From 2880ffb08995238714b175db703c13fac4725cc1 Mon Sep 17 00:00:00 2001 From: Mario Smarduch Date: Fri, 26 Jun 2020 13:19:00 -0700 Subject: util/qemu-error: prepend guest name to error message to identify affected VM owner This is followup patch to the one submitted back in Oct, 19 https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg02102.html My mistake here, I took my eyes of the mailing list after I got the initial thumbs up. This patch follows up on Markus comments in the above link. Purpose of this patch: We want to print guest name for errors, warnings and info messages. This was the first of two patches the second being MCE errors targeting a VM with guest name prepended. But in a large fleet we see many other errors that disable a VM or crash it. In a large fleet and centralized logging having the guest name enables identify of owner and customer. Signed-off-by: Mario Smarduch Message-Id: <20200626201900.8876-1-msmarduch@digitalocean.com> Signed-off-by: Paolo Bonzini --- include/qemu/error-report.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 87532d8..a5ad95f 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -75,5 +75,7 @@ void error_init(const char *argv0); const char *error_get_progname(void); extern bool error_with_timestamp; +extern bool error_with_guestname; +extern const char *error_guest_name; #endif -- cgit v1.1 From db57fef1e285a1f56a2d99b83456abf2f0b86e96 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 29 Jun 2020 21:34:22 +0200 Subject: qom: Introduce object_property_try_add_child() object_property_add() does not allow object_property_try_add() to gracefully fail as &error_abort is passed as an error handle. However such failure can easily be triggered from the QMP shell when, for instance, one attempts to create an object with an id that already exists. This is achieved from the following call path: qmp_object_add -> user_creatable_add_dict -> user_creatable_add_type -> object_property_add_child -> object_property_add For instance, from the qmp-shell, call twice: object-add qom-type=memory-backend-ram id=mem1 props.size=1073741824 and QEMU aborts. This behavior is undesired as a user/management application mistake in reusing a property ID shouldn't result in loss of the VM and live data within. This patch introduces a new function, object_property_try_add_child() which takes an error handle and turn object_property_try_add() into a non-static one. Now the call path becomes: user_creatable_add_type -> object_property_try_add_child -> object_property_try_add and the error is returned gracefully to the QMP client. (QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296 {"return": {}} (QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296 {"error": {"class": "GenericError", "desc": "attempt to add duplicate property 'mem2' to object (type 'container')"}} Signed-off-by: Eric Auger Fixes: d2623129a7de ("qom: Drop parameter @errp of object_property_add() & friends") Reviewed-by: Markus Armbruster Reviewed-by: Greg Kurz Tested-by: Greg Kurz Message-Id: <20200629193424.30280-2-eric.auger@redhat.com> Signed-off-by: Paolo Bonzini --- include/qom/object.h | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/qom/object.h b/include/qom/object.h index 10fd4a2..79c8f83 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1047,7 +1047,7 @@ Object *object_ref(Object *obj); void object_unref(Object *obj); /** - * object_property_add: + * object_property_try_add: * @obj: the object to add a property to * @name: the name of the property. This can contain any character except for * a forward slash. In general, you should use hyphens '-' instead of @@ -1064,10 +1064,23 @@ void object_unref(Object *obj); * meant to allow a property to free its opaque upon object * destruction. This may be NULL. * @opaque: an opaque pointer to pass to the callbacks for the property + * @errp: pointer to error object * * Returns: The #ObjectProperty; this can be used to set the @resolve * callback for child and link properties. */ +ObjectProperty *object_property_try_add(Object *obj, const char *name, + const char *type, + ObjectPropertyAccessor *get, + ObjectPropertyAccessor *set, + ObjectPropertyRelease *release, + void *opaque, Error **errp); + +/** + * object_property_add: + * Same as object_property_try_add() with @errp hardcoded to + * &error_abort. + */ ObjectProperty *object_property_add(Object *obj, const char *name, const char *type, ObjectPropertyAccessor *get, @@ -1518,10 +1531,11 @@ Object *object_resolve_path_type(const char *path, const char *typename, Object *object_resolve_path_component(Object *parent, const char *part); /** - * object_property_add_child: + * object_property_try_add_child: * @obj: the object to add a property to * @name: the name of the property * @child: the child object + * @errp: pointer to error object * * Child properties form the composition tree. All objects need to be a child * of another object. Objects can only be a child of one object. @@ -1535,6 +1549,14 @@ Object *object_resolve_path_component(Object *parent, const char *part); * * Returns: The newly added property on success, or %NULL on failure. */ +ObjectProperty *object_property_try_add_child(Object *obj, const char *name, + Object *child, Error **errp); + +/** + * object_property_add_child: + * Same as object_property_try_add_child() with @errp hardcoded to + * &error_abort + */ ObjectProperty *object_property_add_child(Object *obj, const char *name, Object *child); -- cgit v1.1 From 6553aa1d1166b4257f1294b898fc9f09e7276639 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 29 Jun 2020 11:28:04 -0500 Subject: coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Coverity has problems seeing through __builtin_choose_expr, which result in it abandoning analysis of later functions that utilize a definition that used MIN_CONST or MAX_CONST, such as in qemu-file.c: 50 DECLARE_BITMAP(may_free, MAX_IOV_SIZE); CID 1429992 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1. expr_not_constant: expression must have a constant value As has been done in the past (see 07d66672), it's okay to dumb things down when compiling for static analyzers. (Of course, now the syntax-checker has a false positive on our reference to __COVERITY__...) Reported-by: Peter Maydell Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999 Signed-off-by: Eric Blake Message-Id: <20200629162804.1096180-1-eblake@redhat.com> Signed-off-by: Paolo Bonzini --- include/qemu/osdep.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'include') diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 0d26a1b..0fc206a 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -250,7 +250,8 @@ extern int daemon(int, int); * Note that neither form is usable as an #if condition; if you truly * need to write conditional code that depends on a minimum or maximum * determined by the pre-processor instead of the compiler, you'll - * have to open-code it. + * have to open-code it. Sadly, Coverity is severely confused by the + * constant variants, so we have to dumb things down there. */ #undef MIN #define MIN(a, b) \ @@ -258,22 +259,28 @@ extern int daemon(int, int); typeof(1 ? (a) : (b)) _a = (a), _b = (b); \ _a < _b ? _a : _b; \ }) -#define MIN_CONST(a, b) \ - __builtin_choose_expr( \ - __builtin_constant_p(a) && __builtin_constant_p(b), \ - (a) < (b) ? (a) : (b), \ - ((void)0)) #undef MAX #define MAX(a, b) \ ({ \ typeof(1 ? (a) : (b)) _a = (a), _b = (b); \ _a > _b ? _a : _b; \ }) -#define MAX_CONST(a, b) \ + +#ifdef __COVERITY__ +# define MIN_CONST(a, b) ((a) < (b) ? (a) : (b)) +# define MAX_CONST(a, b) ((a) > (b) ? (a) : (b)) +#else +# define MIN_CONST(a, b) \ + __builtin_choose_expr( \ + __builtin_constant_p(a) && __builtin_constant_p(b), \ + (a) < (b) ? (a) : (b), \ + ((void)0)) +# define MAX_CONST(a, b) \ __builtin_choose_expr( \ __builtin_constant_p(a) && __builtin_constant_p(b), \ (a) > (b) ? (a) : (b), \ ((void)0)) +#endif /* * Minimum function that returns zero only if both values are zero. -- cgit v1.1 From 4bb19f98d34302d627a1267b608de4df6d0988f9 Mon Sep 17 00:00:00 2001 From: Roman Bolshakov Date: Tue, 30 Jun 2020 13:28:17 +0300 Subject: i386: hvf: Move synchronize functions to sysemu Cc: Cameron Esfahani Signed-off-by: Roman Bolshakov Message-Id: <20200630102824.77604-3-r.bolshakov@yadro.com> Signed-off-by: Paolo Bonzini --- include/sysemu/hw_accel.h | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'include') diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h index 0ec2372..80bce75 100644 --- a/include/sysemu/hw_accel.h +++ b/include/sysemu/hw_accel.h @@ -14,6 +14,7 @@ #include "hw/core/cpu.h" #include "sysemu/hax.h" #include "sysemu/kvm.h" +#include "sysemu/hvf.h" #include "sysemu/whpx.h" static inline void cpu_synchronize_state(CPUState *cpu) @@ -24,6 +25,9 @@ static inline void cpu_synchronize_state(CPUState *cpu) if (hax_enabled()) { hax_cpu_synchronize_state(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_state(cpu); + } if (whpx_enabled()) { whpx_cpu_synchronize_state(cpu); } @@ -37,6 +41,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu) if (hax_enabled()) { hax_cpu_synchronize_post_reset(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_post_reset(cpu); + } if (whpx_enabled()) { whpx_cpu_synchronize_post_reset(cpu); } @@ -50,6 +57,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) if (hax_enabled()) { hax_cpu_synchronize_post_init(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_post_init(cpu); + } if (whpx_enabled()) { whpx_cpu_synchronize_post_init(cpu); } -- cgit v1.1 From 5536c98e449fe832c6cb59612baf0f2936fb436d Mon Sep 17 00:00:00 2001 From: Roman Bolshakov Date: Tue, 30 Jun 2020 13:28:18 +0300 Subject: i386: hvf: Add hvf_cpu_synchronize_pre_loadvm() hvf lacks an implementation of cpu_synchronize_pre_loadvm(). Cc: Cameron Esfahani Signed-off-by: Roman Bolshakov Message-Id: <20200630102824.77604-4-r.bolshakov@yadro.com> Signed-off-by: Paolo Bonzini --- include/sysemu/hvf.h | 1 + include/sysemu/hw_accel.h | 3 +++ 2 files changed, 4 insertions(+) (limited to 'include') diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h index 5214ed5..1d40a8e 100644 --- a/include/sysemu/hvf.h +++ b/include/sysemu/hvf.h @@ -28,6 +28,7 @@ int hvf_vcpu_exec(CPUState *); void hvf_cpu_synchronize_state(CPUState *); void hvf_cpu_synchronize_post_reset(CPUState *); void hvf_cpu_synchronize_post_init(CPUState *); +void hvf_cpu_synchronize_pre_loadvm(CPUState *); void hvf_vcpu_destroy(CPUState *); void hvf_reset_vcpu(CPUState *); diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h index 80bce75..e128f8b 100644 --- a/include/sysemu/hw_accel.h +++ b/include/sysemu/hw_accel.h @@ -73,6 +73,9 @@ static inline void cpu_synchronize_pre_loadvm(CPUState *cpu) if (hax_enabled()) { hax_cpu_synchronize_pre_loadvm(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_pre_loadvm(cpu); + } if (whpx_enabled()) { whpx_cpu_synchronize_pre_loadvm(cpu); } -- cgit v1.1 From 5009ef22c6bb21aa741e9e354ccaa97edf56911d Mon Sep 17 00:00:00 2001 From: Roman Bolshakov Date: Tue, 30 Jun 2020 13:28:22 +0300 Subject: i386: hvf: Don't duplicate register reset hvf_reset_vcpu() duplicates actions performed by x86_cpu_reset(). The difference is that hvf_reset_vcpu() stores initial values directly to VMCS while x86_cpu_reset() stores it in CPUX86State and then cpu_synchronize_all_post_init() or cpu_synchronize_all_post_reset() flushes CPUX86State into VMCS. That makes hvf_reset_vcpu() a kind of no-op. Here's the trace of CPU state modifications during VM start: hvf_reset_vcpu (resets VMCS) cpu_synchronize_all_post_init (overwrites VMCS fields written by hvf_reset_vcpu()) cpu_synchronize_all_states hvf_reset_vcpu (resets VMCS) cpu_synchronize_all_post_reset (overwrites VMCS fields written by hvf_reset_vcpu()) General purpose registers, system registers, segment descriptors, flags and IP are set by hvf_put_segments() in post-init and post-reset, therefore it's safe to remove them from hvf_reset_vcpu(). PDPTE initialization can be dropped because Intel SDM (26.3.1.6 Checks on Guest Page-Directory-Pointer-Table Entries) doesn't require PDPTE to be clear unless PAE is used: "A VM entry to a guest that does not use PAE paging does not check the validity of any PDPTEs." And if PAE is used, PDPTE's are initialized from CR3 in macvm_set_cr0(). Cc: Cameron Esfahani Signed-off-by: Roman Bolshakov Message-Id: <20200630102824.77604-8-r.bolshakov@yadro.com> Signed-off-by: Paolo Bonzini --- include/sysemu/hvf.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h index 1d40a8e..6d3ee4f 100644 --- a/include/sysemu/hvf.h +++ b/include/sysemu/hvf.h @@ -30,7 +30,6 @@ void hvf_cpu_synchronize_post_reset(CPUState *); void hvf_cpu_synchronize_post_init(CPUState *); void hvf_cpu_synchronize_pre_loadvm(CPUState *); void hvf_vcpu_destroy(CPUState *); -void hvf_reset_vcpu(CPUState *); #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") -- cgit v1.1 From b0c3cf9407e642d74d1bbd18f8846872152a92df Mon Sep 17 00:00:00 2001 From: Claudio Fontana Date: Mon, 29 Jun 2020 11:35:03 +0200 Subject: cpu-throttle: new module, extracted from cpus.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit move the vcpu throttling functionality into its own module. This functionality is not specific to any accelerator, and it is used currently by migration to slow down guests to try to have migrations converge, and by the cocoa MacOS UI to throttle speed. cpu-throttle contains the controls to adjust and inspect throttle settings, start (set) and stop vcpu throttling, and the throttling function itself that is run periodically on vcpus to make them take a nap. Execution of the throttling function on all vcpus is triggered by a timer, registered at module initialization. No functionality change. Signed-off-by: Claudio Fontana Reviewed-by: Alex Bennée Reviewed-by: Laurent Vivier Message-Id: <20200629093504.3228-3-cfontana@suse.de> Signed-off-by: Paolo Bonzini --- include/hw/core/cpu.h | 37 ----------------------- include/qemu/main-loop.h | 5 ++++ include/sysemu/cpu-throttle.h | 68 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 37 deletions(-) create mode 100644 include/sysemu/cpu-throttle.h (limited to 'include') diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index b3f4b79..5542577 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -822,43 +822,6 @@ bool cpu_exists(int64_t id); */ CPUState *cpu_by_arch_id(int64_t id); -/** - * cpu_throttle_set: - * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99. - * - * Throttles all vcpus by forcing them to sleep for the given percentage of - * time. A throttle_percentage of 25 corresponds to a 75% duty cycle roughly. - * (example: 10ms sleep for every 30ms awake). - * - * cpu_throttle_set can be called as needed to adjust new_throttle_pct. - * Once the throttling starts, it will remain in effect until cpu_throttle_stop - * is called. - */ -void cpu_throttle_set(int new_throttle_pct); - -/** - * cpu_throttle_stop: - * - * Stops the vcpu throttling started by cpu_throttle_set. - */ -void cpu_throttle_stop(void); - -/** - * cpu_throttle_active: - * - * Returns: %true if the vcpus are currently being throttled, %false otherwise. - */ -bool cpu_throttle_active(void); - -/** - * cpu_throttle_get_percentage: - * - * Returns the vcpu throttle percentage. See cpu_throttle_set for details. - * - * Returns: The throttle percentage in range 1 to 99. - */ -int cpu_throttle_get_percentage(void); - #ifndef CONFIG_USER_ONLY typedef void (*CPUInterruptHandler)(CPUState *, int); diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h index a6d20b0..8e98613 100644 --- a/include/qemu/main-loop.h +++ b/include/qemu/main-loop.h @@ -303,6 +303,11 @@ void qemu_mutex_unlock_iothread(void); */ void qemu_cond_wait_iothread(QemuCond *cond); +/* + * qemu_cond_timedwait_iothread: like the previous, but with timeout + */ +void qemu_cond_timedwait_iothread(QemuCond *cond, int ms); + /* internal interfaces */ void qemu_fd_register(int fd); diff --git a/include/sysemu/cpu-throttle.h b/include/sysemu/cpu-throttle.h new file mode 100644 index 0000000..d65bdef --- /dev/null +++ b/include/sysemu/cpu-throttle.h @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2012 SUSE LINUX Products GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see + * + */ + +#ifndef SYSEMU_CPU_THROTTLE_H +#define SYSEMU_CPU_THROTTLE_H + +#include "qemu/timer.h" + +/** + * cpu_throttle_init: + * + * Initialize the CPU throttling API. + */ +void cpu_throttle_init(void); + +/** + * cpu_throttle_set: + * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99. + * + * Throttles all vcpus by forcing them to sleep for the given percentage of + * time. A throttle_percentage of 25 corresponds to a 75% duty cycle roughly. + * (example: 10ms sleep for every 30ms awake). + * + * cpu_throttle_set can be called as needed to adjust new_throttle_pct. + * Once the throttling starts, it will remain in effect until cpu_throttle_stop + * is called. + */ +void cpu_throttle_set(int new_throttle_pct); + +/** + * cpu_throttle_stop: + * + * Stops the vcpu throttling started by cpu_throttle_set. + */ +void cpu_throttle_stop(void); + +/** + * cpu_throttle_active: + * + * Returns: %true if the vcpus are currently being throttled, %false otherwise. + */ +bool cpu_throttle_active(void); + +/** + * cpu_throttle_get_percentage: + * + * Returns the vcpu throttle percentage. See cpu_throttle_set for details. + * + * Returns: The throttle percentage in range 1 to 99. + */ +int cpu_throttle_get_percentage(void); + +#endif /* SYSEMU_CPU_THROTTLE_H */ -- cgit v1.1 From 6e083c0de41a606f304168fce75ea77f3c031f98 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 7 Feb 2020 07:43:42 +0100 Subject: apic: Report current_count via 'info lapic' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is helpful when debugging stuck guest timers. As we need apic_get_current_count for that, and it is really not emulation specific, move it to apic_common.c and export it. Fix its style at this chance as well. Signed-off-by: Jan Kiszka Reviewed-by: Philippe Mathieu-Daudé Message-Id: Signed-off-by: Paolo Bonzini --- include/hw/i386/apic_internal.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h index b04bdd9..2597000 100644 --- a/include/hw/i386/apic_internal.h +++ b/include/hw/i386/apic_internal.h @@ -211,6 +211,7 @@ void vapic_report_tpr_access(DeviceState *dev, CPUState *cpu, target_ulong ip, TPRAccess access); int apic_get_ppr(APICCommonState *s); +uint32_t apic_get_current_count(APICCommonState *s); static inline void apic_set_bit(uint32_t *tab, int index) { -- cgit v1.1