All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Cleanups around the 'current_machine' global variable
@ 2020-01-21 11:03 Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 01/10] hw/ppc/spapr_rtas: Use local MachineState variable Philippe Mathieu-Daudé
                   ` (11 more replies)
  0 siblings, 12 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

v1 was "Replace current_machine by qdev_get_machine()":
https://www.mail-archive.com/qemu-devel@nongnu.org/msg669611.html

But Markus objected, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg670122.html
and older discussion:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg611338.html

This series salvage patches from v1, and add other trivial cleanups.

Can the ARM/PPC/S390 patches could go via their own tree, and the
rest via Paolo's 'misc' tree?

Supersedes: <20200109152133.23649-1-philmd@redhat.com>

Philippe Mathieu-Daudé (10):
  hw/ppc/spapr_rtas: Use local MachineState variable
  hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument
  hw/ppc/spapr_rtas: Remove local variable
  target/arm/kvm: Use CPUState::kvm_state in kvm_arm_pmu_supported()
  target/s390x: Remove duplicated ifdef macro
  qom/object: Display more helpful message when a parent is missing
  qdev: Abort if the root machine container is missing
  accel: Introduce the current_accel() wrapper
  accel: Replace current_machine->accelerator by current_accel() wrapper
  accel/tcg: Sanitize include path

 include/sysemu/accel.h     | 2 ++
 accel/accel.c              | 5 +++++
 accel/kvm/kvm-all.c        | 4 ++--
 accel/tcg/tcg-all.c        | 8 ++++----
 hw/core/qdev.c             | 1 +
 hw/ppc/spapr_rtas.c        | 9 ++++-----
 memory.c                   | 2 +-
 qom/object.c               | 6 +++++-
 target/arm/kvm.c           | 4 +---
 target/arm/kvm64.c         | 5 ++---
 target/i386/kvm.c          | 2 +-
 target/ppc/kvm.c           | 2 +-
 target/s390x/excp_helper.c | 7 +++----
 vl.c                       | 2 +-
 14 files changed, 33 insertions(+), 26 deletions(-)

-- 
2.21.1



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 01/10] hw/ppc/spapr_rtas: Use local MachineState variable
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 02/10] hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster, Greg Kurz,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

Since we have the MachineState already available locally,
use it instead of the global current_machine.

Reviewed-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ppc/spapr_rtas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 8d8d8cdfcb..e88bb1930e 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -281,7 +281,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           "DesProcs=%d,"
                                           "MaxPlatProcs=%d",
                                           max_cpus,
-                                          current_machine->ram_size / MiB,
+                                          ms->ram_size / MiB,
                                           ms->smp.cpus,
                                           max_cpus);
         if (pcc->n_host_threads > 0) {
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 02/10] hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 01/10] hw/ppc/spapr_rtas: Use local MachineState variable Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster, Greg Kurz,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

We received a SpaprMachineState argument. Since SpaprMachineState
inherits of MachineState, use it instead of calling qdev_get_machine.

Reviewed-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ppc/spapr_rtas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index e88bb1930e..6f06e9d7fe 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -267,7 +267,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           uint32_t nret, target_ulong rets)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
-    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineState *ms = MACHINE(spapr);
     unsigned int max_cpus = ms->smp.max_cpus;
     target_ulong parameter = rtas_ld(args, 0);
     target_ulong buffer = rtas_ld(args, 1);
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 01/10] hw/ppc/spapr_rtas: Use local MachineState variable Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 02/10] hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-22  3:27   ` David Gibson
  2020-01-21 11:03 ` [PATCH v2 04/10] target/arm/kvm: Use CPUState::kvm_state in kvm_arm_pmu_supported() Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster, Greg Kurz,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

We only access this variable in the RTAS_SYSPARM_SPLPAR_CHARACTERISTICS
case. Use it in place and remove the local declaration.

Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Do not reduce MachineState variable scope (David Gibson)
---
 hw/ppc/spapr_rtas.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 6f06e9d7fe..85135e0e1a 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -268,7 +268,6 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
     MachineState *ms = MACHINE(spapr);
-    unsigned int max_cpus = ms->smp.max_cpus;
     target_ulong parameter = rtas_ld(args, 0);
     target_ulong buffer = rtas_ld(args, 1);
     target_ulong length = rtas_ld(args, 2);
@@ -280,10 +279,10 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           "DesMem=%" PRIu64 ","
                                           "DesProcs=%d,"
                                           "MaxPlatProcs=%d",
-                                          max_cpus,
+                                          ms->smp.max_cpus,
                                           ms->ram_size / MiB,
                                           ms->smp.cpus,
-                                          max_cpus);
+                                          ms->smp.max_cpus);
         if (pcc->n_host_threads > 0) {
             char *hostthr_val, *old = param_val;
 
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 04/10] target/arm/kvm: Use CPUState::kvm_state in kvm_arm_pmu_supported()
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Alistair Francis, Cornelia Huck, Eduardo Habkost,
	Like Xu, David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

KVMState is already accessible via CPUState::kvm_state, use it.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/arm/kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index b87b59a02a..8d82889150 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -181,9 +181,7 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
 
 bool kvm_arm_pmu_supported(CPUState *cpu)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
-
-    return kvm_check_extension(s, KVM_CAP_ARM_PMU_V3);
+    return kvm_check_extension(cpu->kvm_state, KVM_CAP_ARM_PMU_V3);
 }
 
 int kvm_arm_get_max_vm_ipa_size(MachineState *ms)
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 04/10] target/arm/kvm: Use CPUState::kvm_state in kvm_arm_pmu_supported() Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 13:52   ` Cornelia Huck
  2020-01-21 16:45   ` Cornelia Huck
  2020-01-21 11:03 ` [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

Commit ae71ed8610 replaced the use of global max_cpus variable
with a machine property, but introduced a unnecessary ifdef, as
this block is already in the 'not CONFIG_USER_ONLY' branch part:

   86 #if defined(CONFIG_USER_ONLY)
   87
  ...
  106 #else /* !CONFIG_USER_ONLY */
  107
  ...
  292 static void do_ext_interrupt(CPUS390XState *env)
  293 {
  ...
  313 #ifndef CONFIG_USER_ONLY
  314         MachineState *ms = MACHINE(qdev_get_machine());
  315         unsigned int max_cpus = ms->smp.max_cpus;
  316 #endif

To ease code review, remove the duplicated preprocessor macro,
and move the declarations at the beginning of the statement.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: New patch
Cc: Like Xu <like.xu@linux.intel.com>
---
 target/s390x/excp_helper.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index e70c20d363..1e9d6f20c1 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -305,15 +305,14 @@ static void do_ext_interrupt(CPUS390XState *env)
 
     if ((env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) &&
         (env->cregs[0] & CR0_EMERGENCY_SIGNAL_SC)) {
+        MachineState *ms = MACHINE(qdev_get_machine());
+        unsigned int max_cpus = ms->smp.max_cpus;
+
         lowcore->ext_int_code = cpu_to_be16(EXT_EMERGENCY);
         cpu_addr = find_first_bit(env->emergency_signals, S390_MAX_CPUS);
         g_assert(cpu_addr < S390_MAX_CPUS);
         lowcore->cpu_addr = cpu_to_be16(cpu_addr);
         clear_bit(cpu_addr, env->emergency_signals);
-#ifndef CONFIG_USER_ONLY
-        MachineState *ms = MACHINE(qdev_get_machine());
-        unsigned int max_cpus = ms->smp.max_cpus;
-#endif
         if (bitmap_empty(env->emergency_signals, max_cpus)) {
             env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
         }
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 13:54   ` Cornelia Huck
  2020-01-21 11:03 ` [PATCH v2 07/10] qdev: Abort if the root machine container " Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Daniel P. Berrangé,
	Eduardo Habkost, Like Xu, David Hildenbrand, Marcelo Tosatti,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini,
	Richard Henderson, Philippe Mathieu-Daudé,
	David Gibson

QEMU object model is scarse in documentation. Some calls are
recursive, and it might be hard to figure out even trivial issues.

We can avoid developers to waste time in a debugging session by
displaying a simple error message.

This commit is also similar to e02bdf1cecd2 ("Display more helpful
message when an object type is missing").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: New patch
---
 qom/object.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 0d971ca897..bc13a55094 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -173,7 +173,11 @@ static TypeImpl *type_get_parent(TypeImpl *type)
 {
     if (!type->parent_type && type->parent) {
         type->parent_type = type_get_by_name(type->parent);
-        g_assert(type->parent_type != NULL);
+        if (!type->parent_type) {
+            fprintf(stderr, "Object '%s' missing its parent '%s'\n",
+                    type->name, type->parent);
+            abort();
+        }
     }
 
     return type->parent_type;
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 07/10] qdev: Abort if the root machine container is missing
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 12:45   ` Markus Armbruster
  2020-01-21 11:03 ` [PATCH v2 08/10] accel: Introduce the current_accel() wrapper Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Daniel P. Berrangé,
	Eduardo Habkost, Like Xu, David Hildenbrand, Marcelo Tosatti,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini,
	Richard Henderson, Philippe Mathieu-Daudé,
	David Gibson

The QEMU device API (qdev) relies on having the '/machine'
container always available.

If it is missing, QEMU will later crash dereferencing a NULL
pointer, we will get a SEGV, open a debugger, look at the
backtrace, and figure out we messed with QOM.
Or we can use g_assert() which abort, displaying the filename
and line number, so we can quickly open our favorite IDE.
Prefer the later, to save time to developers.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: New patch
---
 hw/core/qdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 58e87d336d..d30cf6320b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1143,6 +1143,7 @@ Object *qdev_get_machine(void)
 
     if (dev == NULL) {
         dev = container_get(object_get_root(), "/machine");
+        g_assert(dev != NULL);
     }
 
     return dev;
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 08/10] accel: Introduce the current_accel() wrapper
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 07/10] qdev: Abort if the root machine container " Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 13:56   ` Cornelia Huck
  2020-01-21 11:03   ` Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Alistair Francis, Cornelia Huck, Eduardo Habkost,
	Like Xu, David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

The accel/ code only access the MachineState::accel field.
As we simply want to access the accelerator, not the machine,
add a current_accel() wrapper.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Reworded description using 'wrapper'
---
 include/sysemu/accel.h | 2 ++
 accel/accel.c          | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index d4c1429711..47e5788530 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -70,4 +70,6 @@ int accel_init_machine(AccelState *accel, MachineState *ms);
 /* Called just before os_setup_post (ie just before drop OS privs) */
 void accel_setup_post(MachineState *ms);
 
+AccelState *current_accel(void);
+
 #endif
diff --git a/accel/accel.c b/accel/accel.c
index 1c5c3a6abb..cb555e3b06 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -63,6 +63,11 @@ int accel_init_machine(AccelState *accel, MachineState *ms)
     return ret;
 }
 
+AccelState *current_accel(void)
+{
+    return current_machine->accelerator;
+}
+
 void accel_setup_post(MachineState *ms)
 {
     AccelState *accel = ms->accelerator;
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
@ 2020-01-21 11:03   ` Philippe Mathieu-Daudé
  2020-01-21 11:03 ` [PATCH v2 02/10] hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument Philippe Mathieu-Daudé
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Marcelo Tosatti, Cornelia Huck, David Gibson,
	qemu-s390x, David Hildenbrand, qemu-ppc, Eduardo Habkost,
	Like Xu, Markus Armbruster, qemu-arm, Paolo Bonzini,
	Richard Henderson, Philippe Mathieu-Daudé,
	Alistair Francis, open list:Overall KVM CPUs

We actually want to access the accelerator, not the machine, so
use the current_accel() wrapper instead.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
- Reworded description
- Remove unused include in arm/kvm64
---
 accel/kvm/kvm-all.c | 4 ++--
 accel/tcg/tcg-all.c | 2 +-
 memory.c            | 2 +-
 target/arm/kvm64.c  | 5 ++---
 target/i386/kvm.c   | 2 +-
 target/ppc/kvm.c    | 2 +-
 vl.c                | 2 +-
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 1ada2f4ecb..c111312dfd 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -164,7 +164,7 @@ static NotifierList kvm_irqchip_change_notifiers =
 
 int kvm_get_max_memslots(void)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return s->nr_slots;
 }
@@ -1848,7 +1848,7 @@ static int kvm_max_vcpu_id(KVMState *s)
 
 bool kvm_vcpu_id_is_valid(int vcpu_id)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
     return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
 }
 
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 1dc384c8d2..1802ce02f6 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -124,7 +124,7 @@ static void tcg_accel_instance_init(Object *obj)
 
 static int tcg_init(MachineState *ms)
 {
-    TCGState *s = TCG_STATE(current_machine->accelerator);
+    TCGState *s = TCG_STATE(current_accel());
 
     tcg_exec_init(s->tb_size * 1024 * 1024);
     cpu_interrupt_handler = tcg_handle_interrupt;
diff --git a/memory.c b/memory.c
index d7b9bb6951..854798791e 100644
--- a/memory.c
+++ b/memory.c
@@ -3104,7 +3104,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner)
         };
         GArray *fv_address_spaces;
         GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
-        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
 
         if (ac->has_memory) {
             fvi.ac = ac;
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 876184b8fe..e3c580e749 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -26,7 +26,6 @@
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_int.h"
 #include "kvm_arm.h"
-#include "hw/boards.h"
 #include "internals.h"
 
 static bool have_guest_debug;
@@ -613,14 +612,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 
 bool kvm_arm_aarch32_supported(CPUState *cpu)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT);
 }
 
 bool kvm_arm_sve_supported(CPUState *cpu)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return kvm_check_extension(s, KVM_CAP_ARM_SVE);
 }
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 7ee3202634..eddb930065 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -147,7 +147,7 @@ bool kvm_allows_irq0_override(void)
 
 static bool kvm_x2apic_api_set_flags(uint64_t flags)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
 }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index b5799e62b4..45ede6b6d9 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
 
 struct ppc_radix_page_info *kvm_get_radix_page_info(void)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
     struct ppc_radix_page_info *radix_page_info;
     struct kvm_ppc_rmmu_info rmmu_info;
     int i;
diff --git a/vl.c b/vl.c
index 71d3e7eefb..a8ea36f4f8 100644
--- a/vl.c
+++ b/vl.c
@@ -2812,7 +2812,7 @@ static void configure_accelerators(const char *progname)
     }
 
     if (init_failed) {
-        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
         error_report("falling back to %s", ac->name);
     }
 
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
@ 2020-01-21 11:03   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Alistair Francis, Cornelia Huck, Eduardo Habkost,
	Like Xu, David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, open list:Overall KVM CPUs,
	Paolo Bonzini, Richard Henderson, Philippe Mathieu-Daudé,
	David Gibson

We actually want to access the accelerator, not the machine, so
use the current_accel() wrapper instead.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
- Reworded description
- Remove unused include in arm/kvm64
---
 accel/kvm/kvm-all.c | 4 ++--
 accel/tcg/tcg-all.c | 2 +-
 memory.c            | 2 +-
 target/arm/kvm64.c  | 5 ++---
 target/i386/kvm.c   | 2 +-
 target/ppc/kvm.c    | 2 +-
 vl.c                | 2 +-
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 1ada2f4ecb..c111312dfd 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -164,7 +164,7 @@ static NotifierList kvm_irqchip_change_notifiers =
 
 int kvm_get_max_memslots(void)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return s->nr_slots;
 }
@@ -1848,7 +1848,7 @@ static int kvm_max_vcpu_id(KVMState *s)
 
 bool kvm_vcpu_id_is_valid(int vcpu_id)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
     return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
 }
 
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 1dc384c8d2..1802ce02f6 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -124,7 +124,7 @@ static void tcg_accel_instance_init(Object *obj)
 
 static int tcg_init(MachineState *ms)
 {
-    TCGState *s = TCG_STATE(current_machine->accelerator);
+    TCGState *s = TCG_STATE(current_accel());
 
     tcg_exec_init(s->tb_size * 1024 * 1024);
     cpu_interrupt_handler = tcg_handle_interrupt;
diff --git a/memory.c b/memory.c
index d7b9bb6951..854798791e 100644
--- a/memory.c
+++ b/memory.c
@@ -3104,7 +3104,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner)
         };
         GArray *fv_address_spaces;
         GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
-        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
 
         if (ac->has_memory) {
             fvi.ac = ac;
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 876184b8fe..e3c580e749 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -26,7 +26,6 @@
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_int.h"
 #include "kvm_arm.h"
-#include "hw/boards.h"
 #include "internals.h"
 
 static bool have_guest_debug;
@@ -613,14 +612,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 
 bool kvm_arm_aarch32_supported(CPUState *cpu)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT);
 }
 
 bool kvm_arm_sve_supported(CPUState *cpu)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return kvm_check_extension(s, KVM_CAP_ARM_SVE);
 }
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 7ee3202634..eddb930065 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -147,7 +147,7 @@ bool kvm_allows_irq0_override(void)
 
 static bool kvm_x2apic_api_set_flags(uint64_t flags)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
 
     return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
 }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index b5799e62b4..45ede6b6d9 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
 
 struct ppc_radix_page_info *kvm_get_radix_page_info(void)
 {
-    KVMState *s = KVM_STATE(current_machine->accelerator);
+    KVMState *s = KVM_STATE(current_accel());
     struct ppc_radix_page_info *radix_page_info;
     struct kvm_ppc_rmmu_info rmmu_info;
     int i;
diff --git a/vl.c b/vl.c
index 71d3e7eefb..a8ea36f4f8 100644
--- a/vl.c
+++ b/vl.c
@@ -2812,7 +2812,7 @@ static void configure_accelerators(const char *progname)
     }
 
     if (init_failed) {
-        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
         error_report("falling back to %s", ac->name);
     }
 
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 10/10] accel/tcg: Sanitize include path
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2020-01-21 11:03   ` Philippe Mathieu-Daudé
@ 2020-01-21 11:03 ` Philippe Mathieu-Daudé
  2020-01-21 14:06   ` Cornelia Huck
  2020-01-21 12:47 ` [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Markus Armbruster
  2020-01-21 16:27 ` Paolo Bonzini
  11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

Commit af0440ae852 moved the qemu_tcg_configure() function,
but introduced extraneous 'include/' in the includes path.
As it is not necessary, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: New patch
---
 accel/tcg/tcg-all.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 1802ce02f6..acfdcfdf59 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -31,9 +31,9 @@
 #include "sysemu/cpus.h"
 #include "qemu/main-loop.h"
 #include "tcg/tcg.h"
-#include "include/qapi/error.h"
-#include "include/qemu/error-report.h"
-#include "include/hw/boards.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/boards.h"
 #include "qapi/qapi-builtin-visit.h"
 
 typedef struct TCGState {
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 07/10] qdev: Abort if the root machine container is missing
  2020-01-21 11:03 ` [PATCH v2 07/10] qdev: Abort if the root machine container " Philippe Mathieu-Daudé
@ 2020-01-21 12:45   ` Markus Armbruster
  0 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2020-01-21 12:45 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Marcelo Tosatti, Daniel P. Berrangé,
	Eduardo Habkost, Like Xu, David Hildenbrand, Cornelia Huck,
	qemu-devel, qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini,
	David Gibson, Richard Henderson

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> The QEMU device API (qdev) relies on having the '/machine'
> container always available.
>
> If it is missing, QEMU will later crash dereferencing a NULL
> pointer, we will get a SEGV, open a debugger, look at the
> backtrace, and figure out we messed with QOM.
> Or we can use g_assert() which abort, displaying the filename
> and line number, so we can quickly open our favorite IDE.
> Prefer the later, to save time to developers.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: New patch
> ---
>  hw/core/qdev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 58e87d336d..d30cf6320b 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1143,6 +1143,7 @@ Object *qdev_get_machine(void)
>  
>      if (dev == NULL) {
>          dev = container_get(object_get_root(), "/machine");
> +        g_assert(dev != NULL);
>      }
>  
>      return dev;

container_get()'s contract promises it won't return null.  I think the
assertion belongs there instead.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/10] Cleanups around the 'current_machine' global variable
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2020-01-21 11:03 ` [PATCH v2 10/10] accel/tcg: Sanitize include path Philippe Mathieu-Daudé
@ 2020-01-21 12:47 ` Markus Armbruster
  2020-01-21 12:49   ` Philippe Mathieu-Daudé
  2020-01-21 16:27 ` Paolo Bonzini
  11 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2020-01-21 12:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Marcelo Tosatti, Eduardo Habkost, Like Xu,
	David Hildenbrand, Cornelia Huck, qemu-devel, qemu-s390x,
	qemu-arm, qemu-ppc, Paolo Bonzini, David Gibson,
	Richard Henderson

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> v1 was "Replace current_machine by qdev_get_machine()":
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg669611.html
>
> But Markus objected, see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg670122.html
> and older discussion:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg611338.html
>
> This series salvage patches from v1, and add other trivial cleanups.
>
> Can the ARM/PPC/S390 patches could go via their own tree, and the
> rest via Paolo's 'misc' tree?
>
> Supersedes: <20200109152133.23649-1-philmd@redhat.com>

Separating off uncontroversial patches is always a good idea.  These all
look okay to me on first glance, with one minor remark on PATCH 7.  I
think you got or are likely to get competent review for all of them.  If
you need me to have a closer look, just ask.

Thanks!



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/10] Cleanups around the 'current_machine' global variable
  2020-01-21 12:47 ` [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Markus Armbruster
@ 2020-01-21 12:49   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-21 12:49 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Peter Maydell, Marcelo Tosatti, Eduardo Habkost, Like Xu,
	David Hildenbrand, Cornelia Huck, qemu-devel, qemu-s390x,
	qemu-arm, qemu-ppc, Paolo Bonzini, David Gibson,
	Richard Henderson

On 1/21/20 1:47 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> v1 was "Replace current_machine by qdev_get_machine()":
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg669611.html
>>
>> But Markus objected, see:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg670122.html
>> and older discussion:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg611338.html
>>
>> This series salvage patches from v1, and add other trivial cleanups.
>>
>> Can the ARM/PPC/S390 patches could go via their own tree, and the
>> rest via Paolo's 'misc' tree?
>>
>> Supersedes: <20200109152133.23649-1-philmd@redhat.com>
> 
> Separating off uncontroversial patches is always a good idea.  These all
> look okay to me on first glance, with one minor remark on PATCH 7.  I
> think you got or are likely to get competent review for all of them.  If
> you need me to have a closer look, just ask.

Good :) Thanks!



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro
  2020-01-21 11:03 ` [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro Philippe Mathieu-Daudé
@ 2020-01-21 13:52   ` Cornelia Huck
  2020-01-21 16:45   ` Cornelia Huck
  1 sibling, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 13:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Eduardo Habkost, Like Xu, David Hildenbrand,
	Marcelo Tosatti, qemu-devel, Markus Armbruster, qemu-s390x,
	qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	David Gibson

On Tue, 21 Jan 2020 12:03:44 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Commit ae71ed8610 replaced the use of global max_cpus variable
> with a machine property, but introduced a unnecessary ifdef, as
> this block is already in the 'not CONFIG_USER_ONLY' branch part:
> 
>    86 #if defined(CONFIG_USER_ONLY)
>    87
>   ...
>   106 #else /* !CONFIG_USER_ONLY */
>   107
>   ...
>   292 static void do_ext_interrupt(CPUS390XState *env)
>   293 {
>   ...
>   313 #ifndef CONFIG_USER_ONLY
>   314         MachineState *ms = MACHINE(qdev_get_machine());
>   315         unsigned int max_cpus = ms->smp.max_cpus;
>   316 #endif
> 
> To ease code review, remove the duplicated preprocessor macro,
> and move the declarations at the beginning of the statement.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: New patch
> Cc: Like Xu <like.xu@linux.intel.com>
> ---
>  target/s390x/excp_helper.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Thanks, applied.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing
  2020-01-21 11:03 ` [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing Philippe Mathieu-Daudé
@ 2020-01-21 13:54   ` Cornelia Huck
  0 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 13:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Like Xu, David Hildenbrand, Marcelo Tosatti,
	qemu-devel, Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc,
	Paolo Bonzini, Richard Henderson, David Gibson

On Tue, 21 Jan 2020 12:03:45 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> QEMU object model is scarse in documentation. Some calls are
> recursive, and it might be hard to figure out even trivial issues.
> 
> We can avoid developers to waste time in a debugging session by
> displaying a simple error message.
> 
> This commit is also similar to e02bdf1cecd2 ("Display more helpful
> message when an object type is missing").
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: New patch
> ---
>  qom/object.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 0d971ca897..bc13a55094 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -173,7 +173,11 @@ static TypeImpl *type_get_parent(TypeImpl *type)
>  {
>      if (!type->parent_type && type->parent) {
>          type->parent_type = type_get_by_name(type->parent);
> -        g_assert(type->parent_type != NULL);
> +        if (!type->parent_type) {
> +            fprintf(stderr, "Object '%s' missing its parent '%s'\n",

Maybe s/missing/is missing/ ?

Anyway, a real improvement :)

> +                    type->name, type->parent);
> +            abort();
> +        }
>      }
>  
>      return type->parent_type;

Reviewed-by: Cornelia Huck <cohuck@redhat.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 08/10] accel: Introduce the current_accel() wrapper
  2020-01-21 11:03 ` [PATCH v2 08/10] accel: Introduce the current_accel() wrapper Philippe Mathieu-Daudé
@ 2020-01-21 13:56   ` Cornelia Huck
  0 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 13:56 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Alistair Francis, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, qemu-devel,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini,
	Richard Henderson, David Gibson

On Tue, 21 Jan 2020 12:03:47 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> The accel/ code only access the MachineState::accel field.

s/access/accesses/

> As we simply want to access the accelerator, not the machine,
> add a current_accel() wrapper.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Reworded description using 'wrapper'
> ---
>  include/sysemu/accel.h | 2 ++
>  accel/accel.c          | 5 +++++
>  2 files changed, 7 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
  2020-01-21 11:03   ` Philippe Mathieu-Daudé
@ 2020-01-21 14:03     ` Cornelia Huck
  -1 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 14:03 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Peter Maydell, Marcelo Tosatti, David Gibson,
	qemu-s390x, David Hildenbrand, qemu-ppc, Eduardo Habkost,
	Like Xu, Markus Armbruster, qemu-arm, Paolo Bonzini,
	Richard Henderson, Alistair Francis, open list:Overall KVM CPUs

On Tue, 21 Jan 2020 12:03:48 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> We actually want to access the accelerator, not the machine, so
> use the current_accel() wrapper instead.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2:
> - Reworded description
> - Remove unused include in arm/kvm64
> ---
>  accel/kvm/kvm-all.c | 4 ++--
>  accel/tcg/tcg-all.c | 2 +-
>  memory.c            | 2 +-
>  target/arm/kvm64.c  | 5 ++---
>  target/i386/kvm.c   | 2 +-
>  target/ppc/kvm.c    | 2 +-
>  vl.c                | 2 +-
>  7 files changed, 9 insertions(+), 10 deletions(-)

> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b5799e62b4..45ede6b6d9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
>  
>  struct ppc_radix_page_info *kvm_get_radix_page_info(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      struct ppc_radix_page_info *radix_page_info;
>      struct kvm_ppc_rmmu_info rmmu_info;
>      int i;

What about the usage in kvmppc_svm_off()?


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
@ 2020-01-21 14:03     ` Cornelia Huck
  0 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 14:03 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Alistair Francis, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, qemu-devel,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc,
	open list:Overall KVM CPUs, Paolo Bonzini, Richard Henderson,
	David Gibson

On Tue, 21 Jan 2020 12:03:48 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> We actually want to access the accelerator, not the machine, so
> use the current_accel() wrapper instead.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2:
> - Reworded description
> - Remove unused include in arm/kvm64
> ---
>  accel/kvm/kvm-all.c | 4 ++--
>  accel/tcg/tcg-all.c | 2 +-
>  memory.c            | 2 +-
>  target/arm/kvm64.c  | 5 ++---
>  target/i386/kvm.c   | 2 +-
>  target/ppc/kvm.c    | 2 +-
>  vl.c                | 2 +-
>  7 files changed, 9 insertions(+), 10 deletions(-)

> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b5799e62b4..45ede6b6d9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
>  
>  struct ppc_radix_page_info *kvm_get_radix_page_info(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      struct ppc_radix_page_info *radix_page_info;
>      struct kvm_ppc_rmmu_info rmmu_info;
>      int i;

What about the usage in kvmppc_svm_off()?



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 10/10] accel/tcg: Sanitize include path
  2020-01-21 11:03 ` [PATCH v2 10/10] accel/tcg: Sanitize include path Philippe Mathieu-Daudé
@ 2020-01-21 14:06   ` Cornelia Huck
  0 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 14:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Eduardo Habkost, Like Xu, David Hildenbrand,
	Marcelo Tosatti, qemu-devel, Markus Armbruster, qemu-s390x,
	qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	David Gibson

On Tue, 21 Jan 2020 12:03:49 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Commit af0440ae852 moved the qemu_tcg_configure() function,
> but introduced extraneous 'include/' in the includes path.
> As it is not necessary, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: New patch
> ---
>  accel/tcg/tcg-all.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 1802ce02f6..acfdcfdf59 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -31,9 +31,9 @@
>  #include "sysemu/cpus.h"
>  #include "qemu/main-loop.h"
>  #include "tcg/tcg.h"
> -#include "include/qapi/error.h"
> -#include "include/qemu/error-report.h"
> -#include "include/hw/boards.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "hw/boards.h"
>  #include "qapi/qapi-builtin-visit.h"
>  
>  typedef struct TCGState {

Reviewed-by: Cornelia Huck <cohuck@redhat.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/10] Cleanups around the 'current_machine' global variable
  2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2020-01-21 12:47 ` [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Markus Armbruster
@ 2020-01-21 16:27 ` Paolo Bonzini
  2020-01-21 16:44   ` Cornelia Huck
  11 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2020-01-21 16:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, Markus Armbruster,
	qemu-s390x, qemu-arm, qemu-ppc, Richard Henderson, David Gibson

On 21/01/20 12:03, Philippe Mathieu-Daudé wrote:
> v1 was "Replace current_machine by qdev_get_machine()":
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg669611.html
> 
> But Markus objected, see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg670122.html
> and older discussion:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg611338.html
> 
> This series salvage patches from v1, and add other trivial cleanups.
> 
> Can the ARM/PPC/S390 patches could go via their own tree, and the
> rest via Paolo's 'misc' tree?

I think they're trivial enough so (with the exception of patch 7) I have
queued them.

Paolo



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 00/10] Cleanups around the 'current_machine' global variable
  2020-01-21 16:27 ` Paolo Bonzini
@ 2020-01-21 16:44   ` Cornelia Huck
  0 siblings, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 16:44 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Eduardo Habkost, Like Xu, David Hildenbrand,
	Marcelo Tosatti, qemu-devel, Markus Armbruster, qemu-s390x,
	qemu-arm, qemu-ppc, Richard Henderson,
	Philippe Mathieu-Daudé,
	David Gibson

On Tue, 21 Jan 2020 17:27:09 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 21/01/20 12:03, Philippe Mathieu-Daudé wrote:
> > v1 was "Replace current_machine by qdev_get_machine()":
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg669611.html
> > 
> > But Markus objected, see:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg670122.html
> > and older discussion:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg611338.html
> > 
> > This series salvage patches from v1, and add other trivial cleanups.
> > 
> > Can the ARM/PPC/S390 patches could go via their own tree, and the
> > rest via Paolo's 'misc' tree?  
> 
> I think they're trivial enough so (with the exception of patch 7) I have
> queued them.

I had already queued the s390 patch; let's just see who gets their pull
req out first, I can also unqueue it again.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro
  2020-01-21 11:03 ` [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro Philippe Mathieu-Daudé
  2020-01-21 13:52   ` Cornelia Huck
@ 2020-01-21 16:45   ` Cornelia Huck
  1 sibling, 0 replies; 27+ messages in thread
From: Cornelia Huck @ 2020-01-21 16:45 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Eduardo Habkost, Like Xu, David Hildenbrand,
	Marcelo Tosatti, qemu-devel, Markus Armbruster, qemu-s390x,
	qemu-arm, qemu-ppc, Paolo Bonzini, Richard Henderson,
	David Gibson

On Tue, 21 Jan 2020 12:03:44 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Commit ae71ed8610 replaced the use of global max_cpus variable
> with a machine property, but introduced a unnecessary ifdef, as
> this block is already in the 'not CONFIG_USER_ONLY' branch part:
> 
>    86 #if defined(CONFIG_USER_ONLY)
>    87
>   ...
>   106 #else /* !CONFIG_USER_ONLY */
>   107
>   ...
>   292 static void do_ext_interrupt(CPUS390XState *env)
>   293 {
>   ...
>   313 #ifndef CONFIG_USER_ONLY
>   314         MachineState *ms = MACHINE(qdev_get_machine());
>   315         unsigned int max_cpus = ms->smp.max_cpus;
>   316 #endif
> 
> To ease code review, remove the duplicated preprocessor macro,
> and move the declarations at the beginning of the statement.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: New patch
> Cc: Like Xu <like.xu@linux.intel.com>
> ---
>  target/s390x/excp_helper.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

I case this goes via the misc tree:

Reviewed-by: Cornelia Huck <cohuck@redhat.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable
  2020-01-21 11:03 ` [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable Philippe Mathieu-Daudé
@ 2020-01-22  3:27   ` David Gibson
  0 siblings, 0 replies; 27+ messages in thread
From: David Gibson @ 2020-01-22  3:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Cornelia Huck, Eduardo Habkost, Like Xu,
	David Hildenbrand, Marcelo Tosatti, qemu-devel,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc, Paolo Bonzini,
	Greg Kurz, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]

On Tue, Jan 21, 2020 at 12:03:42PM +0100, Philippe Mathieu-Daudé wrote:
> We only access this variable in the RTAS_SYSPARM_SPLPAR_CHARACTERISTICS
> case. Use it in place and remove the local declaration.
> 
> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>


> ---
> v2: Do not reduce MachineState variable scope (David Gibson)
> ---
>  hw/ppc/spapr_rtas.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 6f06e9d7fe..85135e0e1a 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -268,7 +268,6 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>  {
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>      MachineState *ms = MACHINE(spapr);
> -    unsigned int max_cpus = ms->smp.max_cpus;
>      target_ulong parameter = rtas_ld(args, 0);
>      target_ulong buffer = rtas_ld(args, 1);
>      target_ulong length = rtas_ld(args, 2);
> @@ -280,10 +279,10 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>                                            "DesMem=%" PRIu64 ","
>                                            "DesProcs=%d,"
>                                            "MaxPlatProcs=%d",
> -                                          max_cpus,
> +                                          ms->smp.max_cpus,
>                                            ms->ram_size / MiB,
>                                            ms->smp.cpus,
> -                                          max_cpus);
> +                                          ms->smp.max_cpus);
>          if (pcc->n_host_threads > 0) {
>              char *hostthr_val, *old = param_val;
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
  2020-01-21 11:03   ` Philippe Mathieu-Daudé
@ 2020-01-22  3:28     ` David Gibson
  -1 siblings, 0 replies; 27+ messages in thread
From: David Gibson @ 2020-01-22  3:28 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Peter Maydell, Marcelo Tosatti, Cornelia Huck,
	qemu-s390x, David Hildenbrand, qemu-ppc, Eduardo Habkost,
	Like Xu, Markus Armbruster, qemu-arm, Paolo Bonzini,
	Richard Henderson, Alistair Francis, open list:Overall KVM CPUs

[-- Attachment #1: Type: text/plain, Size: 5124 bytes --]

On Tue, Jan 21, 2020 at 12:03:48PM +0100, Philippe Mathieu-Daudé wrote:
> We actually want to access the accelerator, not the machine, so
> use the current_accel() wrapper instead.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2:
> - Reworded description
> - Remove unused include in arm/kvm64
> ---
>  accel/kvm/kvm-all.c | 4 ++--
>  accel/tcg/tcg-all.c | 2 +-
>  memory.c            | 2 +-
>  target/arm/kvm64.c  | 5 ++---
>  target/i386/kvm.c   | 2 +-
>  target/ppc/kvm.c    | 2 +-
>  vl.c                | 2 +-
>  7 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 1ada2f4ecb..c111312dfd 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -164,7 +164,7 @@ static NotifierList kvm_irqchip_change_notifiers =
>  
>  int kvm_get_max_memslots(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return s->nr_slots;
>  }
> @@ -1848,7 +1848,7 @@ static int kvm_max_vcpu_id(KVMState *s)
>  
>  bool kvm_vcpu_id_is_valid(int vcpu_id)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
>  }
>  
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 1dc384c8d2..1802ce02f6 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -124,7 +124,7 @@ static void tcg_accel_instance_init(Object *obj)
>  
>  static int tcg_init(MachineState *ms)
>  {
> -    TCGState *s = TCG_STATE(current_machine->accelerator);
> +    TCGState *s = TCG_STATE(current_accel());
>  
>      tcg_exec_init(s->tb_size * 1024 * 1024);
>      cpu_interrupt_handler = tcg_handle_interrupt;
> diff --git a/memory.c b/memory.c
> index d7b9bb6951..854798791e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -3104,7 +3104,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner)
>          };
>          GArray *fv_address_spaces;
>          GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> -        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
> +        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
>  
>          if (ac->has_memory) {
>              fvi.ac = ac;
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 876184b8fe..e3c580e749 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -26,7 +26,6 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/kvm_int.h"
>  #include "kvm_arm.h"
> -#include "hw/boards.h"
>  #include "internals.h"
>  
>  static bool have_guest_debug;
> @@ -613,14 +612,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>  
>  bool kvm_arm_aarch32_supported(CPUState *cpu)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT);
>  }
>  
>  bool kvm_arm_sve_supported(CPUState *cpu)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return kvm_check_extension(s, KVM_CAP_ARM_SVE);
>  }
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 7ee3202634..eddb930065 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -147,7 +147,7 @@ bool kvm_allows_irq0_override(void)
>  
>  static bool kvm_x2apic_api_set_flags(uint64_t flags)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
>  }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b5799e62b4..45ede6b6d9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
>  
>  struct ppc_radix_page_info *kvm_get_radix_page_info(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      struct ppc_radix_page_info *radix_page_info;
>      struct kvm_ppc_rmmu_info rmmu_info;
>      int i;
> diff --git a/vl.c b/vl.c
> index 71d3e7eefb..a8ea36f4f8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2812,7 +2812,7 @@ static void configure_accelerators(const char *progname)
>      }
>  
>      if (init_failed) {
> -        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
> +        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
>          error_report("falling back to %s", ac->name);
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/10] accel: Replace current_machine->accelerator by current_accel() wrapper
@ 2020-01-22  3:28     ` David Gibson
  0 siblings, 0 replies; 27+ messages in thread
From: David Gibson @ 2020-01-22  3:28 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Alistair Francis, Cornelia Huck, Eduardo Habkost,
	Like Xu, David Hildenbrand, Marcelo Tosatti, qemu-devel,
	Markus Armbruster, qemu-s390x, qemu-arm, qemu-ppc,
	open list:Overall KVM CPUs, Paolo Bonzini, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 5124 bytes --]

On Tue, Jan 21, 2020 at 12:03:48PM +0100, Philippe Mathieu-Daudé wrote:
> We actually want to access the accelerator, not the machine, so
> use the current_accel() wrapper instead.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2:
> - Reworded description
> - Remove unused include in arm/kvm64
> ---
>  accel/kvm/kvm-all.c | 4 ++--
>  accel/tcg/tcg-all.c | 2 +-
>  memory.c            | 2 +-
>  target/arm/kvm64.c  | 5 ++---
>  target/i386/kvm.c   | 2 +-
>  target/ppc/kvm.c    | 2 +-
>  vl.c                | 2 +-
>  7 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 1ada2f4ecb..c111312dfd 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -164,7 +164,7 @@ static NotifierList kvm_irqchip_change_notifiers =
>  
>  int kvm_get_max_memslots(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return s->nr_slots;
>  }
> @@ -1848,7 +1848,7 @@ static int kvm_max_vcpu_id(KVMState *s)
>  
>  bool kvm_vcpu_id_is_valid(int vcpu_id)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
>  }
>  
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 1dc384c8d2..1802ce02f6 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -124,7 +124,7 @@ static void tcg_accel_instance_init(Object *obj)
>  
>  static int tcg_init(MachineState *ms)
>  {
> -    TCGState *s = TCG_STATE(current_machine->accelerator);
> +    TCGState *s = TCG_STATE(current_accel());
>  
>      tcg_exec_init(s->tb_size * 1024 * 1024);
>      cpu_interrupt_handler = tcg_handle_interrupt;
> diff --git a/memory.c b/memory.c
> index d7b9bb6951..854798791e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -3104,7 +3104,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner)
>          };
>          GArray *fv_address_spaces;
>          GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> -        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
> +        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
>  
>          if (ac->has_memory) {
>              fvi.ac = ac;
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 876184b8fe..e3c580e749 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -26,7 +26,6 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/kvm_int.h"
>  #include "kvm_arm.h"
> -#include "hw/boards.h"
>  #include "internals.h"
>  
>  static bool have_guest_debug;
> @@ -613,14 +612,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>  
>  bool kvm_arm_aarch32_supported(CPUState *cpu)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT);
>  }
>  
>  bool kvm_arm_sve_supported(CPUState *cpu)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return kvm_check_extension(s, KVM_CAP_ARM_SVE);
>  }
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 7ee3202634..eddb930065 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -147,7 +147,7 @@ bool kvm_allows_irq0_override(void)
>  
>  static bool kvm_x2apic_api_set_flags(uint64_t flags)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>  
>      return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
>  }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b5799e62b4..45ede6b6d9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -258,7 +258,7 @@ static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
>  
>  struct ppc_radix_page_info *kvm_get_radix_page_info(void)
>  {
> -    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    KVMState *s = KVM_STATE(current_accel());
>      struct ppc_radix_page_info *radix_page_info;
>      struct kvm_ppc_rmmu_info rmmu_info;
>      int i;
> diff --git a/vl.c b/vl.c
> index 71d3e7eefb..a8ea36f4f8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2812,7 +2812,7 @@ static void configure_accelerators(const char *progname)
>      }
>  
>      if (init_failed) {
> -        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
> +        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
>          error_report("falling back to %s", ac->name);
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-01-22  3:32 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 11:03 [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Philippe Mathieu-Daudé
2020-01-21 11:03 ` [PATCH v2 01/10] hw/ppc/spapr_rtas: Use local MachineState variable Philippe Mathieu-Daudé
2020-01-21 11:03 ` [PATCH v2 02/10] hw/ppc/spapr_rtas: Access MachineState via SpaprMachineState argument Philippe Mathieu-Daudé
2020-01-21 11:03 ` [PATCH v2 03/10] hw/ppc/spapr_rtas: Remove local variable Philippe Mathieu-Daudé
2020-01-22  3:27   ` David Gibson
2020-01-21 11:03 ` [PATCH v2 04/10] target/arm/kvm: Use CPUState::kvm_state in kvm_arm_pmu_supported() Philippe Mathieu-Daudé
2020-01-21 11:03 ` [PATCH v2 05/10] target/s390x: Remove duplicated ifdef macro Philippe Mathieu-Daudé
2020-01-21 13:52   ` Cornelia Huck
2020-01-21 16:45   ` Cornelia Huck
2020-01-21 11:03 ` [PATCH v2 06/10] qom/object: Display more helpful message when a parent is missing Philippe Mathieu-Daudé
2020-01-21 13:54   ` Cornelia Huck
2020-01-21 11:03 ` [PATCH v2 07/10] qdev: Abort if the root machine container " Philippe Mathieu-Daudé
2020-01-21 12:45   ` Markus Armbruster
2020-01-21 11:03 ` [PATCH v2 08/10] accel: Introduce the current_accel() wrapper Philippe Mathieu-Daudé
2020-01-21 13:56   ` Cornelia Huck
2020-01-21 11:03 ` [PATCH v2 09/10] accel: Replace current_machine->accelerator by " Philippe Mathieu-Daudé
2020-01-21 11:03   ` Philippe Mathieu-Daudé
2020-01-21 14:03   ` Cornelia Huck
2020-01-21 14:03     ` Cornelia Huck
2020-01-22  3:28   ` David Gibson
2020-01-22  3:28     ` David Gibson
2020-01-21 11:03 ` [PATCH v2 10/10] accel/tcg: Sanitize include path Philippe Mathieu-Daudé
2020-01-21 14:06   ` Cornelia Huck
2020-01-21 12:47 ` [PATCH v2 00/10] Cleanups around the 'current_machine' global variable Markus Armbruster
2020-01-21 12:49   ` Philippe Mathieu-Daudé
2020-01-21 16:27 ` Paolo Bonzini
2020-01-21 16:44   ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.