qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606
@ 2017-06-06  2:51 David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 01/17] migration: remove register_savevm() David Gibson
                   ` (17 more replies)
  0 siblings, 18 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

The following changes since commit 199e19ee538eb61fd08b1c1ee5aa838ebdcc968e:

  Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into staging (2017-06-05 15:28:12 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170606

for you to fetch changes up to 91dcb1ffa67cfa41a13dcc89dd44e2310b5773b0:

  spapr: Remove some non-useful properties on DRC objects (2017-06-06 09:24:25 +1000)

----------------------------------------------------------------
ppc patch queue 2017-06-06

Accumulated patches for ppc targets and the pseries machine type.

The big thing in this batch is a start on a substantial cleanup of the
pseries hotplug mechanisms, which were pretty confusing.  For now
these shouldn't cause substantial behavioural changes, but I am hoping
these lead to clearer code and eventually to fixes for the bugs we
have in hotplug handling, particularly when hotplug and migration are
combined.

The remaining patches are mostly bugfixes.

----------------------------------------------------------------
Aaron Larson (1):
      target-ppc: Fix openpic timer read register offset

Cédric Le Goater (1):
      ppc/pnv: check the return value of fdt_setprop()

David Gibson (10):
      migration: Mark CPU states dirty before incoming migration/loadvm
      spapr: Move DRC RTAS calls into spapr_drc.c
      spapr: Abolish DRC get_fdt method
      spapr: Abolish DRC set_configured method
      spapr: Make DRC get_index and get_type methods into plain functions
      spapr: Introduce DRC subclasses
      spapr: Clean up spapr_dr_connector_by_*()
      spapr: Move configure-connector state into DRC
      spapr: Eliminate spapr_drc_get_type_str()
      spapr: Remove some non-useful properties on DRC objects

Felipe Franciosi (1):
      spapr: Allow boot from vhost-*-scsi backends

Greg Kurz (1):
      spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs

Laurent Vivier (1):
      migration: remove register_savevm()

Peter Maydell (1):
      spapr_nvram: Check return value from blk_getlength()

Suraj Jitindar Singh (1):
      target/ppc: Fixup set_spr error in h_register_process_table

 cpus.c                      |   9 +
 hw/intc/openpic.c           |  22 +-
 hw/net/vmxnet3.c            |   8 +-
 hw/nvram/spapr_nvram.c      |  10 +-
 hw/ppc/pnv.c                |   5 +-
 hw/ppc/spapr.c              |  58 +++--
 hw/ppc/spapr_drc.c          | 573 +++++++++++++++++++++++++++++++-------------
 hw/ppc/spapr_events.c       |  12 +-
 hw/ppc/spapr_hcall.c        |   5 +-
 hw/ppc/spapr_pci.c          |  13 +-
 hw/ppc/spapr_rtas.c         | 304 -----------------------
 hw/s390x/s390-skeys.c       |   9 +-
 hw/s390x/s390-virtio-ccw.c  |   8 +-
 include/hw/ppc/spapr.h      |  14 --
 include/hw/ppc/spapr_drc.h  |  69 +++++-
 include/migration/vmstate.h |   8 -
 include/sysemu/cpus.h       |   1 +
 include/sysemu/hax.h        |   1 +
 include/sysemu/hw_accel.h   |  10 +
 include/sysemu/kvm.h        |   1 +
 kvm-all.c                   |  10 +
 migration/savevm.c          |  18 +-
 slirp/slirp.c               |   8 +-
 target/i386/hax-all.c       |  10 +
 24 files changed, 595 insertions(+), 591 deletions(-)

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

* [Qemu-devel] [PULL 01/17] migration: remove register_savevm()
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06 17:49   ` Peter Maydell
  2017-06-06  2:51 ` [Qemu-devel] [PULL 02/17] migration: Mark CPU states dirty before incoming migration/loadvm David Gibson
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Laurent Vivier, David Gibson

From: Laurent Vivier <lvivier@redhat.com>

We can replace the four remaining calls of register_savevm() by
calls to register_savevm_live(). So we can remove the function and
as we don't allocate anymore the ops pointer with g_new0()
we don't have to free it then.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/net/vmxnet3.c            |  8 ++++++--
 hw/s390x/s390-skeys.c       |  9 +++++++--
 hw/s390x/s390-virtio-ccw.c  |  8 ++++++--
 include/migration/vmstate.h |  8 --------
 migration/savevm.c          | 16 ----------------
 slirp/slirp.c               |  8 ++++++--
 6 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 8b1fab2..4df3110 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2262,6 +2262,11 @@ static const MemoryRegionOps b1_ops = {
     },
 };
 
+static SaveVMHandlers savevm_vmxnet3_msix = {
+    .save_state = vmxnet3_msix_save,
+    .load_state = vmxnet3_msix_load,
+};
+
 static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
 {
     uint64_t dsn_payload;
@@ -2331,8 +2336,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
                               vmxnet3_device_serial_num(s));
     }
 
-    register_savevm(dev, "vmxnet3-msix", -1, 1,
-                    vmxnet3_msix_save, vmxnet3_msix_load, s);
+    register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
 }
 
 static void vmxnet3_instance_init(Object *obj)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 619152c..35e7f63 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -362,6 +362,11 @@ static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp)
     return ss->migration_enabled;
 }
 
+static SaveVMHandlers savevm_s390_storage_keys = {
+    .save_state = s390_storage_keys_save,
+    .load_state = s390_storage_keys_load,
+};
+
 static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
                                             Error **errp)
 {
@@ -375,8 +380,8 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
     ss->migration_enabled = value;
 
     if (ss->migration_enabled) {
-        register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save,
-                        s390_storage_keys_load, ss);
+        register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
+                             &savevm_s390_storage_keys, ss);
     } else {
         unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
     }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index c9021f2..a806345 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -104,6 +104,11 @@ void s390_memory_init(ram_addr_t mem_size)
     s390_skeys_init();
 }
 
+static SaveVMHandlers savevm_gtod = {
+    .save_state = gtod_save,
+    .load_state = gtod_load,
+};
+
 static void ccw_init(MachineState *machine)
 {
     int ret;
@@ -151,8 +156,7 @@ static void ccw_init(MachineState *machine)
     s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
 
     /* Register savevm handler for guest TOD clock */
-    register_savevm(NULL, "todclock", 0, 1,
-                    gtod_save, gtod_load, kvm_state);
+    register_savevm_live(NULL, "todclock", 0, 1, &savevm_gtod, kvm_state);
 }
 
 static void s390_cpu_plug(HotplugHandler *hotplug_dev,
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 6689562..8a3e9e6 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -59,14 +59,6 @@ typedef struct SaveVMHandlers {
     LoadStateHandler *load_state;
 } SaveVMHandlers;
 
-int register_savevm(DeviceState *dev,
-                    const char *idstr,
-                    int instance_id,
-                    int version_id,
-                    SaveStateHandler *save_state,
-                    LoadStateHandler *load_state,
-                    void *opaque);
-
 int register_savevm_live(DeviceState *dev,
                          const char *idstr,
                          int instance_id,
diff --git a/migration/savevm.c b/migration/savevm.c
index 9c320f5..035c127 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -645,21 +645,6 @@ int register_savevm_live(DeviceState *dev,
     return 0;
 }
 
-int register_savevm(DeviceState *dev,
-                    const char *idstr,
-                    int instance_id,
-                    int version_id,
-                    SaveStateHandler *save_state,
-                    LoadStateHandler *load_state,
-                    void *opaque)
-{
-    SaveVMHandlers *ops = g_new0(SaveVMHandlers, 1);
-    ops->save_state = save_state;
-    ops->load_state = load_state;
-    return register_savevm_live(dev, idstr, instance_id, version_id,
-                                ops, opaque);
-}
-
 void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
 {
     SaveStateEntry *se, *new_se;
@@ -679,7 +664,6 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
         if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
             QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
             g_free(se->compat);
-            g_free(se->ops);
             g_free(se);
         }
     }
diff --git a/slirp/slirp.c b/slirp/slirp.c
index e79345b..2386493 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -272,6 +272,11 @@ static void slirp_init_once(void)
 static void slirp_state_save(QEMUFile *f, void *opaque);
 static int slirp_state_load(QEMUFile *f, void *opaque, int version_id);
 
+static SaveVMHandlers savevm_slirp_state = {
+    .save_state = slirp_state_save,
+    .load_state = slirp_state_load,
+};
+
 Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   bool in6_enabled,
@@ -321,8 +326,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
 
     slirp->opaque = opaque;
 
-    register_savevm(NULL, "slirp", 0, 4,
-                    slirp_state_save, slirp_state_load, slirp);
+    register_savevm_live(NULL, "slirp", 0, 4, &savevm_slirp_state, slirp);
 
     QTAILQ_INSERT_TAIL(&slirp_instances, slirp, entry);
 
-- 
2.9.4

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

* [Qemu-devel] [PULL 02/17] migration: Mark CPU states dirty before incoming migration/loadvm
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 01/17] migration: remove register_savevm() David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 03/17] spapr: Move DRC RTAS calls into spapr_drc.c David Gibson
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson, Juan Quintela,
	Dave Gilbert

As a rule, CPU internal state should never be updated when
!cpu->kvm_vcpu_dirty (or the HAX equivalent).  If that is done, then
subsequent calls to cpu_synchronize_state() - usually safe and idempotent -
will clobber state.

However, we routinely do this during a loadvm or incoming migration.
Usually this is called shortly after a reset, which will clear all the cpu
dirty flags with cpu_synchronize_all_post_reset().  Nothing is expected
to set the dirty flags again before the cpu state is loaded from the
incoming stream.

This means that it isn't safe to call cpu_synchronize_state() from a
post_load handler, which is non-obvious and potentially inconvenient.

We could cpu_synchronize_all_state() before the loadvm, but that would be
overkill since a) we expect the state to already be synchronized from the
reset and b) we expect to completely rewrite the state with a call to
cpu_synchronize_all_post_init() at the end of qemu_loadvm_state().

To clear this up, this patch introduces cpu_synchronize_pre_loadvm() and
associated helpers, which simply marks the cpu state as dirty without
actually changing anything.  i.e. it says we want to discard any existing
KVM (or HAX) state and replace it with what we're going to load.

Cc: Juan Quintela <quintela@redhat.com>
Cc: Dave Gilbert <dgilbert@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 cpus.c                    |  9 +++++++++
 include/sysemu/cpus.h     |  1 +
 include/sysemu/hax.h      |  1 +
 include/sysemu/hw_accel.h | 10 ++++++++++
 include/sysemu/kvm.h      |  1 +
 kvm-all.c                 | 10 ++++++++++
 migration/savevm.c        |  2 ++
 target/i386/hax-all.c     | 10 ++++++++++
 8 files changed, 44 insertions(+)

diff --git a/cpus.c b/cpus.c
index 516e5cb..6398439 100644
--- a/cpus.c
+++ b/cpus.c
@@ -921,6 +921,15 @@ void cpu_synchronize_all_post_init(void)
     }
 }
 
+void cpu_synchronize_all_pre_loadvm(void)
+{
+    CPUState *cpu;
+
+    CPU_FOREACH(cpu) {
+        cpu_synchronize_pre_loadvm(cpu);
+    }
+}
+
 static int do_vm_stop(RunState state)
 {
     int ret = 0;
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index a8053f1..731756d 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -27,6 +27,7 @@ void qemu_timer_notify_cb(void *opaque, QEMUClockType type);
 void cpu_synchronize_all_states(void);
 void cpu_synchronize_all_post_reset(void);
 void cpu_synchronize_all_post_init(void);
+void cpu_synchronize_all_pre_loadvm(void);
 
 void qtest_clock_warp(int64_t dest);
 
diff --git a/include/sysemu/hax.h b/include/sysemu/hax.h
index d9f0239..232a68a 100644
--- a/include/sysemu/hax.h
+++ b/include/sysemu/hax.h
@@ -33,6 +33,7 @@ int hax_populate_ram(uint64_t va, uint32_t size);
 void hax_cpu_synchronize_state(CPUState *cpu);
 void hax_cpu_synchronize_post_reset(CPUState *cpu);
 void hax_cpu_synchronize_post_init(CPUState *cpu);
+void hax_cpu_synchronize_pre_loadvm(CPUState *cpu);
 
 #ifdef CONFIG_HAX
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index c9b3105..469ffda 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -45,4 +45,14 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
     }
 }
 
+static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
+{
+    if (kvm_enabled()) {
+        kvm_cpu_synchronize_pre_loadvm(cpu);
+    }
+    if (hax_enabled()) {
+        hax_cpu_synchronize_pre_loadvm(cpu);
+    }
+}
+
 #endif /* QEMU_HW_ACCEL_H */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 5cc83f2..a45c145 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -459,6 +459,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
 void kvm_cpu_synchronize_state(CPUState *cpu);
 void kvm_cpu_synchronize_post_reset(CPUState *cpu);
 void kvm_cpu_synchronize_post_init(CPUState *cpu);
+void kvm_cpu_synchronize_pre_loadvm(CPUState *cpu);
 
 void kvm_init_cpu_signals(CPUState *cpu);
 
diff --git a/kvm-all.c b/kvm-all.c
index 7df27c8..494b925 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1896,6 +1896,16 @@ void kvm_cpu_synchronize_post_init(CPUState *cpu)
     run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
 }
 
+static void do_kvm_cpu_synchronize_pre_loadvm(CPUState *cpu, run_on_cpu_data arg)
+{
+    cpu->kvm_vcpu_dirty = true;
+}
+
+void kvm_cpu_synchronize_pre_loadvm(CPUState *cpu)
+{
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_pre_loadvm, RUN_ON_CPU_NULL);
+}
+
 #ifdef KVM_HAVE_MCE_INJECTION
 static __thread void *pending_sigbus_addr;
 static __thread int pending_sigbus_code;
diff --git a/migration/savevm.c b/migration/savevm.c
index 035c127..1993ca2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1999,6 +1999,8 @@ int qemu_loadvm_state(QEMUFile *f)
         }
     }
 
+    cpu_synchronize_all_pre_loadvm();
+
     ret = qemu_loadvm_state_main(f, mis);
     qemu_event_set(&mis->main_thread_load_event);
 
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index 7346931..097db5c 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -635,6 +635,16 @@ void hax_cpu_synchronize_post_init(CPUState *cpu)
     run_on_cpu(cpu, do_hax_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
 }
 
+static void do_hax_cpu_synchronize_pre_loadvm(CPUState *cpu, run_on_cpu_data arg)
+{
+    cpu->hax_vcpu_dirty = true;
+}
+
+void hax_cpu_synchronize_pre_loadvm(CPUState *cpu)
+{
+    run_on_cpu(cpu, do_hax_cpu_synchronize_pre_loadvm, RUN_ON_CPU_NULL);
+}
+
 int hax_smp_cpu_exec(CPUState *cpu)
 {
     CPUArchState *env = (CPUArchState *) (cpu->env_ptr);
-- 
2.9.4

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

* [Qemu-devel] [PULL 03/17] spapr: Move DRC RTAS calls into spapr_drc.c
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 01/17] migration: remove register_savevm() David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 02/17] migration: Mark CPU states dirty before incoming migration/loadvm David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 04/17] spapr: Abolish DRC get_fdt method David Gibson
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

Currently implementations of the RTAS calls related to DRCs are in
spapr_rtas.c.  They belong better in spapr_drc.c - that way they're closer
to related code, and we'll be able to make some more things local.

spapr_rtas.c was intended to contain the RTAS infrastructure and core calls
that don't belong anywhere else, not every RTAS implementation.

Code motion only.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c  | 322 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/ppc/spapr_rtas.c | 304 -------------------------------------------------
 2 files changed, 315 insertions(+), 311 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index cc2400b..ae8800d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -27,6 +27,34 @@
 #define DRC_INDEX_TYPE_SHIFT 28
 #define DRC_INDEX_ID_MASK ((1ULL << DRC_INDEX_TYPE_SHIFT) - 1)
 
+static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr,
+                                                    uint32_t drc_index)
+{
+    sPAPRConfigureConnectorState *ccs = NULL;
+
+    QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) {
+        if (ccs->drc_index == drc_index) {
+            break;
+        }
+    }
+
+    return ccs;
+}
+
+static void spapr_ccs_add(sPAPRMachineState *spapr,
+                          sPAPRConfigureConnectorState *ccs)
+{
+    g_assert(!spapr_ccs_find(spapr, ccs->drc_index));
+    QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next);
+}
+
+static void spapr_ccs_remove(sPAPRMachineState *spapr,
+                             sPAPRConfigureConnectorState *ccs)
+{
+    QTAILQ_REMOVE(&spapr->ccs_list, ccs, next);
+    g_free(ccs);
+}
+
 static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
 {
     uint32_t shift = 0;
@@ -747,13 +775,6 @@ static const TypeInfo spapr_dr_connector_info = {
     .class_init    = spapr_dr_connector_class_init,
 };
 
-static void spapr_drc_register_types(void)
-{
-    type_register_static(&spapr_dr_connector_info);
-}
-
-type_init(spapr_drc_register_types)
-
 /* helper functions for external users */
 
 sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
@@ -932,3 +953,290 @@ out:
 
     return ret;
 }
+
+/*
+ * RTAS calls
+ */
+
+static bool sensor_type_is_dr(uint32_t sensor_type)
+{
+    switch (sensor_type) {
+    case RTAS_SENSOR_TYPE_ISOLATION_STATE:
+    case RTAS_SENSOR_TYPE_DR:
+    case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
+        return true;
+    }
+
+    return false;
+}
+
+static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                               uint32_t token, uint32_t nargs,
+                               target_ulong args, uint32_t nret,
+                               target_ulong rets)
+{
+    uint32_t sensor_type;
+    uint32_t sensor_index;
+    uint32_t sensor_state;
+    uint32_t ret = RTAS_OUT_SUCCESS;
+    sPAPRDRConnector *drc;
+    sPAPRDRConnectorClass *drck;
+
+    if (nargs != 3 || nret != 1) {
+        ret = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+
+    sensor_type = rtas_ld(args, 0);
+    sensor_index = rtas_ld(args, 1);
+    sensor_state = rtas_ld(args, 2);
+
+    if (!sensor_type_is_dr(sensor_type)) {
+        goto out_unimplemented;
+    }
+
+    /* if this is a DR sensor we can assume sensor_index == drc_index */
+    drc = spapr_dr_connector_by_index(sensor_index);
+    if (!drc) {
+        trace_spapr_rtas_set_indicator_invalid(sensor_index);
+        ret = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
+    switch (sensor_type) {
+    case RTAS_SENSOR_TYPE_ISOLATION_STATE:
+        /* if the guest is configuring a device attached to this
+         * DRC, we should reset the configuration state at this
+         * point since it may no longer be reliable (guest released
+         * device and needs to start over, or unplug occurred so
+         * the FDT is no longer valid)
+         */
+        if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
+            sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr,
+                                                               sensor_index);
+            if (ccs) {
+                spapr_ccs_remove(spapr, ccs);
+            }
+        }
+        ret = drck->set_isolation_state(drc, sensor_state);
+        break;
+    case RTAS_SENSOR_TYPE_DR:
+        ret = drck->set_indicator_state(drc, sensor_state);
+        break;
+    case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
+        ret = drck->set_allocation_state(drc, sensor_state);
+        break;
+    default:
+        goto out_unimplemented;
+    }
+
+out:
+    rtas_st(rets, 0, ret);
+    return;
+
+out_unimplemented:
+    /* currently only DR-related sensors are implemented */
+    trace_spapr_rtas_set_indicator_not_supported(sensor_index, sensor_type);
+    rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+}
+
+static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                                  uint32_t token, uint32_t nargs,
+                                  target_ulong args, uint32_t nret,
+                                  target_ulong rets)
+{
+    uint32_t sensor_type;
+    uint32_t sensor_index;
+    uint32_t sensor_state = 0;
+    sPAPRDRConnector *drc;
+    sPAPRDRConnectorClass *drck;
+    uint32_t ret = RTAS_OUT_SUCCESS;
+
+    if (nargs != 2 || nret != 2) {
+        ret = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+
+    sensor_type = rtas_ld(args, 0);
+    sensor_index = rtas_ld(args, 1);
+
+    if (sensor_type != RTAS_SENSOR_TYPE_ENTITY_SENSE) {
+        /* currently only DR-related sensors are implemented */
+        trace_spapr_rtas_get_sensor_state_not_supported(sensor_index,
+                                                        sensor_type);
+        ret = RTAS_OUT_NOT_SUPPORTED;
+        goto out;
+    }
+
+    drc = spapr_dr_connector_by_index(sensor_index);
+    if (!drc) {
+        trace_spapr_rtas_get_sensor_state_invalid(sensor_index);
+        ret = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+    ret = drck->entity_sense(drc, &sensor_state);
+
+out:
+    rtas_st(rets, 0, ret);
+    rtas_st(rets, 1, sensor_state);
+}
+
+/* configure-connector work area offsets, int32_t units for field
+ * indexes, bytes for field offset/len values.
+ *
+ * as documented by PAPR+ v2.7, 13.5.3.5
+ */
+#define CC_IDX_NODE_NAME_OFFSET 2
+#define CC_IDX_PROP_NAME_OFFSET 2
+#define CC_IDX_PROP_LEN 3
+#define CC_IDX_PROP_DATA_OFFSET 4
+#define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
+#define CC_WA_LEN 4096
+
+static void configure_connector_st(target_ulong addr, target_ulong offset,
+                                   const void *buf, size_t len)
+{
+    cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
+                              buf, MIN(len, CC_WA_LEN - offset));
+}
+
+void spapr_ccs_reset_hook(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+    sPAPRConfigureConnectorState *ccs, *ccs_tmp;
+
+    QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) {
+        spapr_ccs_remove(spapr, ccs);
+    }
+}
+
+static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
+                                         sPAPRMachineState *spapr,
+                                         uint32_t token, uint32_t nargs,
+                                         target_ulong args, uint32_t nret,
+                                         target_ulong rets)
+{
+    uint64_t wa_addr;
+    uint64_t wa_offset;
+    uint32_t drc_index;
+    sPAPRDRConnector *drc;
+    sPAPRDRConnectorClass *drck;
+    sPAPRConfigureConnectorState *ccs;
+    sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
+    int rc;
+    const void *fdt;
+
+    if (nargs != 2 || nret != 1) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0);
+
+    drc_index = rtas_ld(wa_addr, 0);
+    drc = spapr_dr_connector_by_index(drc_index);
+    if (!drc) {
+        trace_spapr_rtas_ibm_configure_connector_invalid(drc_index);
+        rc = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+
+    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+    fdt = drck->get_fdt(drc, NULL);
+    if (!fdt) {
+        trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
+        rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
+        goto out;
+    }
+
+    ccs = spapr_ccs_find(spapr, drc_index);
+    if (!ccs) {
+        ccs = g_new0(sPAPRConfigureConnectorState, 1);
+        (void)drck->get_fdt(drc, &ccs->fdt_offset);
+        ccs->drc_index = drc_index;
+        spapr_ccs_add(spapr, ccs);
+    }
+
+    do {
+        uint32_t tag;
+        const char *name;
+        const struct fdt_property *prop;
+        int fdt_offset_next, prop_len;
+
+        tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next);
+
+        switch (tag) {
+        case FDT_BEGIN_NODE:
+            ccs->fdt_depth++;
+            name = fdt_get_name(fdt, ccs->fdt_offset, NULL);
+
+            /* provide the name of the next OF node */
+            wa_offset = CC_VAL_DATA_OFFSET;
+            rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
+            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
+            resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
+            break;
+        case FDT_END_NODE:
+            ccs->fdt_depth--;
+            if (ccs->fdt_depth == 0) {
+                /* done sending the device tree, don't need to track
+                 * the state anymore
+                 */
+                drck->set_configured(drc);
+                spapr_ccs_remove(spapr, ccs);
+                ccs = NULL;
+                resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
+            } else {
+                resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT;
+            }
+            break;
+        case FDT_PROP:
+            prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset,
+                                              &prop_len);
+            name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+
+            /* provide the name of the next OF property */
+            wa_offset = CC_VAL_DATA_OFFSET;
+            rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
+            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
+
+            /* provide the length and value of the OF property. data gets
+             * placed immediately after NULL terminator of the OF property's
+             * name string
+             */
+            wa_offset += strlen(name) + 1,
+            rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
+            rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
+            configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
+            resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
+            break;
+        case FDT_END:
+            resp = SPAPR_DR_CC_RESPONSE_ERROR;
+        default:
+            /* keep seeking for an actionable tag */
+            break;
+        }
+        if (ccs) {
+            ccs->fdt_offset = fdt_offset_next;
+        }
+    } while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE);
+
+    rc = resp;
+out:
+    rtas_st(rets, 0, rc);
+}
+
+static void spapr_drc_register_types(void)
+{
+    type_register_static(&spapr_dr_connector_info);
+
+    spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
+                        rtas_set_indicator);
+    spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state",
+                        rtas_get_sensor_state);
+    spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector",
+                        rtas_ibm_configure_connector);
+}
+type_init(spapr_drc_register_types)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index b666a4c..707c4d4 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -47,44 +47,6 @@
 #include "trace.h"
 #include "hw/ppc/fdt.h"
 
-static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr,
-                                                    uint32_t drc_index)
-{
-    sPAPRConfigureConnectorState *ccs = NULL;
-
-    QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) {
-        if (ccs->drc_index == drc_index) {
-            break;
-        }
-    }
-
-    return ccs;
-}
-
-static void spapr_ccs_add(sPAPRMachineState *spapr,
-                          sPAPRConfigureConnectorState *ccs)
-{
-    g_assert(!spapr_ccs_find(spapr, ccs->drc_index));
-    QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next);
-}
-
-static void spapr_ccs_remove(sPAPRMachineState *spapr,
-                             sPAPRConfigureConnectorState *ccs)
-{
-    QTAILQ_REMOVE(&spapr->ccs_list, ccs, next);
-    g_free(ccs);
-}
-
-void spapr_ccs_reset_hook(void *opaque)
-{
-    sPAPRMachineState *spapr = opaque;
-    sPAPRConfigureConnectorState *ccs, *ccs_tmp;
-
-    QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) {
-        spapr_ccs_remove(spapr, ccs);
-    }
-}
-
 static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                                    uint32_t token, uint32_t nargs,
                                    target_ulong args,
@@ -389,266 +351,6 @@ static void rtas_get_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     rtas_st(rets, 1, 100);
 }
 
-static bool sensor_type_is_dr(uint32_t sensor_type)
-{
-    switch (sensor_type) {
-    case RTAS_SENSOR_TYPE_ISOLATION_STATE:
-    case RTAS_SENSOR_TYPE_DR:
-    case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
-        return true;
-    }
-
-    return false;
-}
-
-static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                               uint32_t token, uint32_t nargs,
-                               target_ulong args, uint32_t nret,
-                               target_ulong rets)
-{
-    uint32_t sensor_type;
-    uint32_t sensor_index;
-    uint32_t sensor_state;
-    uint32_t ret = RTAS_OUT_SUCCESS;
-    sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
-
-    if (nargs != 3 || nret != 1) {
-        ret = RTAS_OUT_PARAM_ERROR;
-        goto out;
-    }
-
-    sensor_type = rtas_ld(args, 0);
-    sensor_index = rtas_ld(args, 1);
-    sensor_state = rtas_ld(args, 2);
-
-    if (!sensor_type_is_dr(sensor_type)) {
-        goto out_unimplemented;
-    }
-
-    /* if this is a DR sensor we can assume sensor_index == drc_index */
-    drc = spapr_dr_connector_by_index(sensor_index);
-    if (!drc) {
-        trace_spapr_rtas_set_indicator_invalid(sensor_index);
-        ret = RTAS_OUT_PARAM_ERROR;
-        goto out;
-    }
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-
-    switch (sensor_type) {
-    case RTAS_SENSOR_TYPE_ISOLATION_STATE:
-        /* if the guest is configuring a device attached to this
-         * DRC, we should reset the configuration state at this
-         * point since it may no longer be reliable (guest released
-         * device and needs to start over, or unplug occurred so
-         * the FDT is no longer valid)
-         */
-        if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-            sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr,
-                                                               sensor_index);
-            if (ccs) {
-                spapr_ccs_remove(spapr, ccs);
-            }
-        }
-        ret = drck->set_isolation_state(drc, sensor_state);
-        break;
-    case RTAS_SENSOR_TYPE_DR:
-        ret = drck->set_indicator_state(drc, sensor_state);
-        break;
-    case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
-        ret = drck->set_allocation_state(drc, sensor_state);
-        break;
-    default:
-        goto out_unimplemented;
-    }
-
-out:
-    rtas_st(rets, 0, ret);
-    return;
-
-out_unimplemented:
-    /* currently only DR-related sensors are implemented */
-    trace_spapr_rtas_set_indicator_not_supported(sensor_index, sensor_type);
-    rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
-}
-
-static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                                  uint32_t token, uint32_t nargs,
-                                  target_ulong args, uint32_t nret,
-                                  target_ulong rets)
-{
-    uint32_t sensor_type;
-    uint32_t sensor_index;
-    uint32_t sensor_state = 0;
-    sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
-    uint32_t ret = RTAS_OUT_SUCCESS;
-
-    if (nargs != 2 || nret != 2) {
-        ret = RTAS_OUT_PARAM_ERROR;
-        goto out;
-    }
-
-    sensor_type = rtas_ld(args, 0);
-    sensor_index = rtas_ld(args, 1);
-
-    if (sensor_type != RTAS_SENSOR_TYPE_ENTITY_SENSE) {
-        /* currently only DR-related sensors are implemented */
-        trace_spapr_rtas_get_sensor_state_not_supported(sensor_index,
-                                                        sensor_type);
-        ret = RTAS_OUT_NOT_SUPPORTED;
-        goto out;
-    }
-
-    drc = spapr_dr_connector_by_index(sensor_index);
-    if (!drc) {
-        trace_spapr_rtas_get_sensor_state_invalid(sensor_index);
-        ret = RTAS_OUT_PARAM_ERROR;
-        goto out;
-    }
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    ret = drck->entity_sense(drc, &sensor_state);
-
-out:
-    rtas_st(rets, 0, ret);
-    rtas_st(rets, 1, sensor_state);
-}
-
-/* configure-connector work area offsets, int32_t units for field
- * indexes, bytes for field offset/len values.
- *
- * as documented by PAPR+ v2.7, 13.5.3.5
- */
-#define CC_IDX_NODE_NAME_OFFSET 2
-#define CC_IDX_PROP_NAME_OFFSET 2
-#define CC_IDX_PROP_LEN 3
-#define CC_IDX_PROP_DATA_OFFSET 4
-#define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
-#define CC_WA_LEN 4096
-
-static void configure_connector_st(target_ulong addr, target_ulong offset,
-                                   const void *buf, size_t len)
-{
-    cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
-                              buf, MIN(len, CC_WA_LEN - offset));
-}
-
-static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
-                                         sPAPRMachineState *spapr,
-                                         uint32_t token, uint32_t nargs,
-                                         target_ulong args, uint32_t nret,
-                                         target_ulong rets)
-{
-    uint64_t wa_addr;
-    uint64_t wa_offset;
-    uint32_t drc_index;
-    sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
-    sPAPRConfigureConnectorState *ccs;
-    sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
-    int rc;
-    const void *fdt;
-
-    if (nargs != 2 || nret != 1) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0);
-
-    drc_index = rtas_ld(wa_addr, 0);
-    drc = spapr_dr_connector_by_index(drc_index);
-    if (!drc) {
-        trace_spapr_rtas_ibm_configure_connector_invalid(drc_index);
-        rc = RTAS_OUT_PARAM_ERROR;
-        goto out;
-    }
-
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    fdt = drck->get_fdt(drc, NULL);
-    if (!fdt) {
-        trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
-        rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
-        goto out;
-    }
-
-    ccs = spapr_ccs_find(spapr, drc_index);
-    if (!ccs) {
-        ccs = g_new0(sPAPRConfigureConnectorState, 1);
-        (void)drck->get_fdt(drc, &ccs->fdt_offset);
-        ccs->drc_index = drc_index;
-        spapr_ccs_add(spapr, ccs);
-    }
-
-    do {
-        uint32_t tag;
-        const char *name;
-        const struct fdt_property *prop;
-        int fdt_offset_next, prop_len;
-
-        tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next);
-
-        switch (tag) {
-        case FDT_BEGIN_NODE:
-            ccs->fdt_depth++;
-            name = fdt_get_name(fdt, ccs->fdt_offset, NULL);
-
-            /* provide the name of the next OF node */
-            wa_offset = CC_VAL_DATA_OFFSET;
-            rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
-            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
-            resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
-            break;
-        case FDT_END_NODE:
-            ccs->fdt_depth--;
-            if (ccs->fdt_depth == 0) {
-                /* done sending the device tree, don't need to track
-                 * the state anymore
-                 */
-                drck->set_configured(drc);
-                spapr_ccs_remove(spapr, ccs);
-                ccs = NULL;
-                resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
-            } else {
-                resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT;
-            }
-            break;
-        case FDT_PROP:
-            prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset,
-                                              &prop_len);
-            name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
-
-            /* provide the name of the next OF property */
-            wa_offset = CC_VAL_DATA_OFFSET;
-            rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
-            configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
-
-            /* provide the length and value of the OF property. data gets
-             * placed immediately after NULL terminator of the OF property's
-             * name string
-             */
-            wa_offset += strlen(name) + 1,
-            rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
-            rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
-            configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
-            resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
-            break;
-        case FDT_END:
-            resp = SPAPR_DR_CC_RESPONSE_ERROR;
-        default:
-            /* keep seeking for an actionable tag */
-            break;
-        }
-        if (ccs) {
-            ccs->fdt_offset = fdt_offset_next;
-        }
-    } while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE);
-
-    rc = resp;
-out:
-    rtas_st(rets, 0, rc);
-}
-
 static struct rtas_call {
     const char *name;
     spapr_rtas_fn fn;
@@ -790,12 +492,6 @@ static void core_rtas_register_types(void)
                         rtas_set_power_level);
     spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
                         rtas_get_power_level);
-    spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
-                        rtas_set_indicator);
-    spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state",
-                        rtas_get_sensor_state);
-    spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector",
-                        rtas_ibm_configure_connector);
 }
 
 type_init(core_rtas_register_types)
-- 
2.9.4

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

* [Qemu-devel] [PULL 04/17] spapr: Abolish DRC get_fdt method
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (2 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 03/17] spapr: Move DRC RTAS calls into spapr_drc.c David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 05/17] spapr: Abolish DRC set_configured method David Gibson
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

The DRConnectorClass includes a get_fdt method.  However
  * There's only one implementation, and there's only likely to ever be one
  * Both callers are local to spapr_drc
  * Each caller only uses one half of the actual implementation

So abolish get_fdt() entirely, and just open-code what we need.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c         | 23 ++++++-----------------
 include/hw/ppc/spapr_drc.h |  1 -
 2 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index ae8800d..f5b7b68 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -199,14 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
     return drc->name;
 }
 
-static const void *get_fdt(sPAPRDRConnector *drc, int *fdt_start_offset)
-{
-    if (fdt_start_offset) {
-        *fdt_start_offset = drc->fdt_start_offset;
-    }
-    return drc->fdt;
-}
-
 static void set_configured(sPAPRDRConnector *drc)
 {
     trace_spapr_drc_set_configured(get_index(drc));
@@ -753,7 +745,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     drck->get_index = get_index;
     drck->get_type = get_type;
     drck->get_name = get_name;
-    drck->get_fdt = get_fdt;
     drck->set_configured = set_configured;
     drck->entity_sense = entity_sense;
     drck->attach = attach;
@@ -1126,7 +1117,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     sPAPRConfigureConnectorState *ccs;
     sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
     int rc;
-    const void *fdt;
 
     if (nargs != 2 || nret != 1) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -1144,8 +1134,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     }
 
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    fdt = drck->get_fdt(drc, NULL);
-    if (!fdt) {
+    if (!drc->fdt) {
         trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
         rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
         goto out;
@@ -1154,7 +1143,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     ccs = spapr_ccs_find(spapr, drc_index);
     if (!ccs) {
         ccs = g_new0(sPAPRConfigureConnectorState, 1);
-        (void)drck->get_fdt(drc, &ccs->fdt_offset);
+        ccs->fdt_offset = drc->fdt_start_offset;
         ccs->drc_index = drc_index;
         spapr_ccs_add(spapr, ccs);
     }
@@ -1165,12 +1154,12 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         const struct fdt_property *prop;
         int fdt_offset_next, prop_len;
 
-        tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next);
+        tag = fdt_next_tag(drc->fdt, ccs->fdt_offset, &fdt_offset_next);
 
         switch (tag) {
         case FDT_BEGIN_NODE:
             ccs->fdt_depth++;
-            name = fdt_get_name(fdt, ccs->fdt_offset, NULL);
+            name = fdt_get_name(drc->fdt, ccs->fdt_offset, NULL);
 
             /* provide the name of the next OF node */
             wa_offset = CC_VAL_DATA_OFFSET;
@@ -1193,9 +1182,9 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
             }
             break;
         case FDT_PROP:
-            prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset,
+            prop = fdt_get_property_by_offset(drc->fdt, ccs->fdt_offset,
                                               &prop_len);
-            name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+            name = fdt_string(drc->fdt, fdt32_to_cpu(prop->nameoff));
 
             /* provide the name of the next OF property */
             wa_offset = CC_VAL_DATA_OFFSET;
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 813b9ff..80db955 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -178,7 +178,6 @@ typedef struct sPAPRDRConnectorClass {
     uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
 
     /* QEMU interfaces for managing FDT/configure-connector */
-    const void *(*get_fdt)(sPAPRDRConnector *drc, int *fdt_start_offset);
     void (*set_configured)(sPAPRDRConnector *drc);
 
     /* QEMU interfaces for managing hotplug operations */
-- 
2.9.4

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

* [Qemu-devel] [PULL 05/17] spapr: Abolish DRC set_configured method
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (3 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 04/17] spapr: Abolish DRC get_fdt method David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 06/17] spapr: Make DRC get_index and get_type methods into plain functions David Gibson
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

DRConnectorClass has a set_configured method, however:
  * There is only one implementation, and only ever likely to be one
  * There's exactly one caller, and that's (now) local
  * The implementation is very straightforward

So abolish the method entirely, and just open-code what we need.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c         | 24 ++++++++----------------
 include/hw/ppc/spapr_drc.h |  3 ---
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index f5b7b68..693571a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc)
     return drc->name;
 }
 
-static void set_configured(sPAPRDRConnector *drc)
-{
-    trace_spapr_drc_set_configured(get_index(drc));
-
-    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
-        /* guest should be not configuring an isolated device */
-        trace_spapr_drc_set_configured_skipping(get_index(drc));
-        return;
-    }
-    drc->configured = true;
-}
-
 /* has the guest been notified of device attachment? */
 static void set_signalled(sPAPRDRConnector *drc)
 {
@@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     drck->get_index = get_index;
     drck->get_type = get_type;
     drck->get_name = get_name;
-    drck->set_configured = set_configured;
     drck->entity_sense = entity_sense;
     drck->attach = attach;
     drck->detach = detach;
@@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     uint64_t wa_offset;
     uint32_t drc_index;
     sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
     sPAPRConfigureConnectorState *ccs;
     sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
     int rc;
@@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         goto out;
     }
 
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     if (!drc->fdt) {
         trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index);
         rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
@@ -1170,10 +1155,17 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         case FDT_END_NODE:
             ccs->fdt_depth--;
             if (ccs->fdt_depth == 0) {
+                sPAPRDRIsolationState state = drc->isolation_state;
                 /* done sending the device tree, don't need to track
                  * the state anymore
                  */
-                drck->set_configured(drc);
+                trace_spapr_drc_set_configured(get_index(drc));
+                if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+                    drc->configured = true;
+                } else {
+                    /* guest should be not configuring an isolated device */
+                    trace_spapr_drc_set_configured_skipping(get_index(drc));
+                }
                 spapr_ccs_remove(spapr, ccs);
                 ccs = NULL;
                 resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 80db955..90f4953 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass {
 
     uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
 
-    /* QEMU interfaces for managing FDT/configure-connector */
-    void (*set_configured)(sPAPRDRConnector *drc);
-
     /* QEMU interfaces for managing hotplug operations */
     void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
                    int fdt_start_offset, bool coldplug, Error **errp);
-- 
2.9.4

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

* [Qemu-devel] [PULL 06/17] spapr: Make DRC get_index and get_type methods into plain functions
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (4 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 05/17] spapr: Abolish DRC set_configured method David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 07/17] target-ppc: Fix openpic timer read register offset David Gibson
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

These two methods only have one implementation, and the spec they're
implementing means any other implementation is unlikely, verging on
impossible.

So replace them with simple functions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c             | 13 +++------
 hw/ppc/spapr_drc.c         | 66 ++++++++++++++++++++++------------------------
 hw/ppc/spapr_events.c      | 10 +++----
 hw/ppc/spapr_pci.c         |  4 +--
 include/hw/ppc/spapr_drc.h |  5 ++--
 5 files changed, 44 insertions(+), 54 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ab3aab1..5d10366 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -456,15 +456,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
     uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
     int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
     sPAPRDRConnector *drc;
-    sPAPRDRConnectorClass *drck;
     int drc_index;
     uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ];
     int i;
 
     drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
     if (drc) {
-        drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-        drc_index = drck->get_index(drc);
+        drc_index = spapr_drc_index(drc);
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
     }
 
@@ -654,15 +652,13 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
 
         if (i >= hotplug_lmb_start) {
             sPAPRDRConnector *drc;
-            sPAPRDRConnectorClass *drck;
 
             drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, i);
             g_assert(drc);
-            drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
             dynamic_memory[0] = cpu_to_be32(addr >> 32);
             dynamic_memory[1] = cpu_to_be32(addr & 0xffffffff);
-            dynamic_memory[2] = cpu_to_be32(drck->get_index(drc));
+            dynamic_memory[2] = cpu_to_be32(spapr_drc_index(drc));
             dynamic_memory[3] = cpu_to_be32(0); /* reserved */
             dynamic_memory[4] = cpu_to_be32(numa_get_node(addr, NULL));
             if (memory_region_present(get_system_memory(), addr)) {
@@ -2560,7 +2556,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
             drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
             spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
                                                    nr_lmbs,
-                                                   drck->get_index(drc));
+                                                   spapr_drc_index(drc));
         } else {
             spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB,
                                            nr_lmbs);
@@ -2770,8 +2766,7 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev,
                                    addr_start / SPAPR_MEMORY_BLOCK_SIZE);
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                                              nr_lmbs,
-                                              drck->get_index(drc));
+                                              nr_lmbs, spapr_drc_index(drc));
 out:
     error_propagate(errp, local_err);
 }
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 693571a..a35314e 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -70,7 +70,7 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
     return shift;
 }
 
-static uint32_t get_index(sPAPRDRConnector *drc)
+uint32_t spapr_drc_index(sPAPRDRConnector *drc)
 {
     /* no set format for a drc index: it only needs to be globally
      * unique. this is how we encode the DRC type on bare-metal
@@ -85,7 +85,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
 {
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
-    trace_spapr_drc_set_isolation_state(get_index(drc), state);
+    trace_spapr_drc_set_isolation_state(spapr_drc_index(drc), state);
 
     if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
         /* cannot unisolate a non-existent resource, and, or resources
@@ -126,11 +126,12 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
          * PAPR+ 2.7, 13.4
          */
         if (drc->awaiting_release) {
+            uint32_t drc_index = spapr_drc_index(drc);
             if (drc->configured) {
-                trace_spapr_drc_set_isolation_state_finalizing(get_index(drc));
+                trace_spapr_drc_set_isolation_state_finalizing(drc_index);
                 drck->detach(drc, DEVICE(drc->dev), NULL);
             } else {
-                trace_spapr_drc_set_isolation_state_deferring(get_index(drc));
+                trace_spapr_drc_set_isolation_state_deferring(drc_index);
             }
         }
         drc->configured = false;
@@ -142,7 +143,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
 static uint32_t set_indicator_state(sPAPRDRConnector *drc,
                                     sPAPRDRIndicatorState state)
 {
-    trace_spapr_drc_set_indicator_state(get_index(drc), state);
+    trace_spapr_drc_set_indicator_state(spapr_drc_index(drc), state);
     drc->indicator_state = state;
     return RTAS_OUT_SUCCESS;
 }
@@ -152,7 +153,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
 {
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
-    trace_spapr_drc_set_allocation_state(get_index(drc), state);
+    trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state);
 
     if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
         /* if there's no resource/device associated with the DRC, there's
@@ -180,7 +181,8 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
         drc->allocation_state = state;
         if (drc->awaiting_release &&
             drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
-            trace_spapr_drc_set_allocation_state_finalizing(get_index(drc));
+            uint32_t drc_index = spapr_drc_index(drc);
+            trace_spapr_drc_set_allocation_state_finalizing(drc_index);
             drck->detach(drc, DEVICE(drc->dev), NULL);
         } else if (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
             drc->awaiting_allocation = false;
@@ -189,7 +191,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
     return RTAS_OUT_SUCCESS;
 }
 
-static uint32_t get_type(sPAPRDRConnector *drc)
+sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
 {
     return drc->type;
 }
@@ -243,7 +245,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
         }
     }
 
-    trace_spapr_drc_entity_sense(get_index(drc), *state);
+    trace_spapr_drc_entity_sense(spapr_drc_index(drc), *state);
     return RTAS_OUT_SUCCESS;
 }
 
@@ -251,8 +253,7 @@ static void prop_get_index(Object *obj, Visitor *v, const char *name,
                            void *opaque, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    uint32_t value = (uint32_t)drck->get_index(drc);
+    uint32_t value = spapr_drc_index(drc);
     visit_type_uint32(v, name, &value, errp);
 }
 
@@ -260,8 +261,7 @@ static void prop_get_type(Object *obj, Visitor *v, const char *name,
                           void *opaque, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    uint32_t value = (uint32_t)drck->get_type(drc);
+    uint32_t value = (uint32_t)spapr_drc_type(drc);
     visit_type_uint32(v, name, &value, errp);
 }
 
@@ -362,7 +362,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
 static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
                    int fdt_start_offset, bool coldplug, Error **errp)
 {
-    trace_spapr_drc_attach(get_index(drc));
+    trace_spapr_drc_attach(spapr_drc_index(drc));
 
     if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
         error_setg(errp, "an attached device is still awaiting release");
@@ -413,7 +413,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
 
 static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
 {
-    trace_spapr_drc_detach(get_index(drc));
+    trace_spapr_drc_detach(spapr_drc_index(drc));
 
     /* if we've signalled device presence to the guest, or if the guest
      * has gone ahead and configured the device (via manually-executed
@@ -436,14 +436,14 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
     }
 
     if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-        trace_spapr_drc_awaiting_isolated(get_index(drc));
+        trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc));
         drc->awaiting_release = true;
         return;
     }
 
     if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
         drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
-        trace_spapr_drc_awaiting_unusable(get_index(drc));
+        trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
         drc->awaiting_release = true;
         return;
     }
@@ -451,7 +451,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
     if (drc->awaiting_allocation) {
         if (!drc->awaiting_allocation_skippable) {
             drc->awaiting_release = true;
-            trace_spapr_drc_awaiting_allocation(get_index(drc));
+            trace_spapr_drc_awaiting_allocation(spapr_drc_index(drc));
             return;
         }
     }
@@ -495,7 +495,7 @@ static void reset(DeviceState *d)
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     sPAPRDREntitySense state;
 
-    trace_spapr_drc_reset(drck->get_index(drc));
+    trace_spapr_drc_reset(spapr_drc_index(drc));
     /* immediately upon reset we can safely assume DRCs whose devices
      * are pending removal can be safely removed, and that they will
      * subsequently be left in an ISOLATED state. move the DRC to this
@@ -584,13 +584,12 @@ static const VMStateDescription vmstate_spapr_drc = {
 static void realize(DeviceState *d, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     Object *root_container;
     char link_name[256];
     gchar *child_name;
     Error *err = NULL;
 
-    trace_spapr_drc_realize(drck->get_index(drc));
+    trace_spapr_drc_realize(spapr_drc_index(drc));
     /* NOTE: we do this as part of realize/unrealize due to the fact
      * that the guest will communicate with the DRC via RTAS calls
      * referencing the global DRC index. By unlinking the DRC
@@ -599,9 +598,9 @@ static void realize(DeviceState *d, Error **errp)
      * existing in the composition tree
      */
     root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
-    snprintf(link_name, sizeof(link_name), "%x", drck->get_index(drc));
+    snprintf(link_name, sizeof(link_name), "%x", spapr_drc_index(drc));
     child_name = object_get_canonical_path_component(OBJECT(drc));
-    trace_spapr_drc_realize_child(drck->get_index(drc), child_name);
+    trace_spapr_drc_realize_child(spapr_drc_index(drc), child_name);
     object_property_add_alias(root_container, link_name,
                               drc->owner, child_name, &err);
     if (err) {
@@ -609,22 +608,21 @@ static void realize(DeviceState *d, Error **errp)
         object_unref(OBJECT(drc));
     }
     g_free(child_name);
-    vmstate_register(DEVICE(drc), drck->get_index(drc), &vmstate_spapr_drc,
+    vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
                      drc);
-    trace_spapr_drc_realize_complete(drck->get_index(drc));
+    trace_spapr_drc_realize_complete(spapr_drc_index(drc));
 }
 
 static void unrealize(DeviceState *d, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     Object *root_container;
     char name[256];
     Error *err = NULL;
 
-    trace_spapr_drc_unrealize(drck->get_index(drc));
+    trace_spapr_drc_unrealize(spapr_drc_index(drc));
     root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
-    snprintf(name, sizeof(name), "%x", drck->get_index(drc));
+    snprintf(name, sizeof(name), "%x", spapr_drc_index(drc));
     object_property_del(root_container, name, &err);
     if (err) {
         error_report_err(err);
@@ -645,7 +643,8 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
     drc->type = type;
     drc->id = id;
     drc->owner = owner;
-    prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", get_index(drc));
+    prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
+                                spapr_drc_index(drc));
     object_property_add_child(owner, prop_name, OBJECT(drc), NULL);
     object_property_set_bool(OBJECT(drc), true, "realized", NULL);
     g_free(prop_name);
@@ -730,8 +729,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     drck->set_isolation_state = set_isolation_state;
     drck->set_indicator_state = set_indicator_state;
     drck->set_allocation_state = set_allocation_state;
-    drck->get_index = get_index;
-    drck->get_type = get_type;
     drck->get_name = get_name;
     drck->entity_sense = entity_sense;
     drck->attach = attach;
@@ -868,7 +865,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
         drc_count++;
 
         /* ibm,drc-indexes */
-        drc_index = cpu_to_be32(drck->get_index(drc));
+        drc_index = cpu_to_be32(spapr_drc_index(drc));
         g_array_append_val(drc_indexes, drc_index);
 
         /* ibm,drc-power-domains */
@@ -1156,15 +1153,16 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
             ccs->fdt_depth--;
             if (ccs->fdt_depth == 0) {
                 sPAPRDRIsolationState state = drc->isolation_state;
+                uint32_t drc_index = spapr_drc_index(drc);
                 /* done sending the device tree, don't need to track
                  * the state anymore
                  */
-                trace_spapr_drc_set_configured(get_index(drc));
+                trace_spapr_drc_set_configured(drc_index);
                 if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
                     drc->configured = true;
                 } else {
                     /* guest should be not configuring an isolated device */
-                    trace_spapr_drc_set_configured_skipping(get_index(drc));
+                    trace_spapr_drc_set_configured_skipping(drc_index);
                 }
                 spapr_ccs_remove(spapr, ccs);
                 ccs = NULL;
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 57acd85..d349ae5 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -570,22 +570,20 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
 
 void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc)
 {
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    sPAPRDRConnectorType drc_type = drck->get_type(drc);
+    sPAPRDRConnectorType drc_type = spapr_drc_type(drc);
     union drc_identifier drc_id;
 
-    drc_id.index = drck->get_index(drc);
+    drc_id.index = spapr_drc_index(drc);
     spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
                             RTAS_LOG_V6_HP_ACTION_ADD, drc_type, &drc_id);
 }
 
 void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc)
 {
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    sPAPRDRConnectorType drc_type = drck->get_type(drc);
+    sPAPRDRConnectorType drc_type = spapr_drc_type(drc);
     union drc_identifier drc_id;
 
-    drc_id.index = drck->get_index(drc);
+    drc_id.index = spapr_drc_index(drc);
     spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
                             RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
 }
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index e4daf8d..7a208a7 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1417,14 +1417,12 @@ static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb,
                                             PCIDevice *pdev)
 {
     sPAPRDRConnector *drc = spapr_phb_get_pci_drc(phb, pdev);
-    sPAPRDRConnectorClass *drck;
 
     if (!drc) {
         return 0;
     }
 
-    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    return drck->get_index(drc);
+    return spapr_drc_index(drc);
 }
 
 static void spapr_phb_hot_plug_child(HotplugHandler *plug_handler,
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 90f4953..10e7c24 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -171,8 +171,6 @@ typedef struct sPAPRDRConnectorClass {
                                     sPAPRDRIndicatorState state);
     uint32_t (*set_allocation_state)(sPAPRDRConnector *drc,
                                      sPAPRDRAllocationState state);
-    uint32_t (*get_index)(sPAPRDRConnector *drc);
-    uint32_t (*get_type)(sPAPRDRConnector *drc);
     const char *(*get_name)(sPAPRDRConnector *drc);
 
     uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state);
@@ -185,6 +183,9 @@ typedef struct sPAPRDRConnectorClass {
     void (*set_signalled)(sPAPRDRConnector *drc);
 } sPAPRDRConnectorClass;
 
+uint32_t spapr_drc_index(sPAPRDRConnector *drc);
+sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
+
 sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
                                          sPAPRDRConnectorType type,
                                          uint32_t id);
-- 
2.9.4

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

* [Qemu-devel] [PULL 07/17] target-ppc: Fix openpic timer read register offset
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (5 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 06/17] spapr: Make DRC get_index and get_type methods into plain functions David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 08/17] target/ppc: Fixup set_spr error in h_register_process_table David Gibson
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Aaron Larson, David Gibson

From: Aaron Larson <alarson@ddci.com>

openpic_tmr_read() is incorrectly computing register offset of the
TCCR, TBCR, TVPR, and TDR registers when accessing the open pic timer
registers.  Specifically the offset of timer registers for
openpic_tmr_read() is not accounting for the timer frequency reporting
register (TFFR) which is the first register in the "tmr" memory
region.

openpic_tmr_write() *is* correctly computing the offset by adding
0x10f0 to the address prior to computing the register index.  This
patch instead subtracts 0x10 in both the read and write routines and
eliminates some other gratuitous differences between the functions.

Signed-off-by: Aaron Larson <alarson@ddci.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/openpic.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 4349e45..f966d06 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -796,27 +796,24 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len)
 }
 
 static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val,
-                                unsigned len)
+                              unsigned len)
 {
     OpenPICState *opp = opaque;
     int idx;
 
-    addr += 0x10f0;
-
     DPRINTF("%s: addr %#" HWADDR_PRIx " <= %08" PRIx64 "\n",
-            __func__, addr, val);
+            __func__, (addr + 0x10f0), val);
     if (addr & 0xF) {
         return;
     }
 
-    if (addr == 0x10f0) {
+    if (addr == 0) {
         /* TFRR */
         opp->tfrr = val;
         return;
     }
-
+    addr -= 0x10;  /* correct for TFRR */
     idx = (addr >> 6) & 0x3;
-    addr = addr & 0x30;
 
     switch (addr & 0x30) {
     case 0x00: /* TCCR */
@@ -844,16 +841,17 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
     uint32_t retval = -1;
     int idx;
 
-    DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr);
+    DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr + 0x10f0);
     if (addr & 0xF) {
         goto out;
     }
-    idx = (addr >> 6) & 0x3;
-    if (addr == 0x0) {
+    if (addr == 0) {
         /* TFRR */
         retval = opp->tfrr;
         goto out;
     }
+    addr -= 0x10;  /* correct for TFRR */
+    idx = (addr >> 6) & 0x3;
     switch (addr & 0x30) {
     case 0x00: /* TCCR */
         retval = opp->timers[idx].tccr;
@@ -861,10 +859,10 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len)
     case 0x10: /* TBCR */
         retval = opp->timers[idx].tbcr;
         break;
-    case 0x20: /* TIPV */
+    case 0x20: /* TVPR */
         retval = read_IRQreg_ivpr(opp, opp->irq_tim0 + idx);
         break;
-    case 0x30: /* TIDE (TIDR) */
+    case 0x30: /* TDR */
         retval = read_IRQreg_idr(opp, opp->irq_tim0 + idx);
         break;
     }
-- 
2.9.4

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

* [Qemu-devel] [PULL 08/17] target/ppc: Fixup set_spr error in h_register_process_table
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (6 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 07/17] target-ppc: Fix openpic timer read register offset David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 09/17] spapr_nvram: Check return value from blk_getlength() David Gibson
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Suraj Jitindar Singh,
	David Gibson

From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

set_spr is used in the function h_register_process_table() to update the
LPCR_GTSE and LPCR_UPRT values based on the flags passed by the guest.
The set_spr function takes the last two arguments mask and value used to
mask and set the value of the spr respectively.

The current call site passes these arguments in the wrong order and thus
bot GTSE and UPRT will be set irrespective, which is obviously
incorrect.

Rearrange the function call so that these arguments are passed in the
correct order and the correct behaviour is exhibited.

It is worth noting that this wasn't detected earlier since these were
always both set in all cases where this H_CALL was made.

Fixes: 6de833070ca2 ("target/ppc: Set UPRT and GTSE on all cpus in H_REGISTER_PROCESS_TABLE")

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_hcall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index aae5a62..aa1ffea 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -992,9 +992,10 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
 
     /* Update the UPRT and GTSE bits in the LPCR for all cpus */
     CPU_FOREACH(cs) {
-        set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE,
+        set_spr(cs, SPR_LPCR,
                 ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) |
-                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0));
+                ((flags & FLAG_GTSE) ? LPCR_GTSE : 0),
+                LPCR_UPRT | LPCR_GTSE);
     }
 
     if (kvm_enabled()) {
-- 
2.9.4

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

* [Qemu-devel] [PULL 09/17] spapr_nvram: Check return value from blk_getlength()
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (7 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 08/17] target/ppc: Fixup set_spr error in h_register_process_table David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 10/17] ppc/pnv: check the return value of fdt_setprop() David Gibson
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

From: Peter Maydell <peter.maydell@linaro.org>

The blk_getlength() function can return an error value if the
image size cannot be determined. Check for this rather than
ploughing on and trying to g_malloc0() a negative number.
(Spotted by Coverity, CID 1288484.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/nvram/spapr_nvram.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index aa5d2c1..bc355a4 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -144,7 +144,15 @@ static void spapr_nvram_realize(VIOsPAPRDevice *dev, Error **errp)
     int ret;
 
     if (nvram->blk) {
-        nvram->size = blk_getlength(nvram->blk);
+        int64_t len = blk_getlength(nvram->blk);
+
+        if (len < 0) {
+            error_setg_errno(errp, -len,
+                             "could not get length of backing image");
+            return;
+        }
+
+        nvram->size = len;
 
         ret = blk_set_perm(nvram->blk,
                            BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
-- 
2.9.4

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

* [Qemu-devel] [PULL 10/17] ppc/pnv: check the return value of fdt_setprop()
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (8 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 09/17] spapr_nvram: Check return value from blk_getlength() David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 11/17] spapr: Allow boot from vhost-*-scsi backends David Gibson
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Cédric Le Goater,
	David Gibson

From: Cédric Le Goater <clg@kaod.org>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
[dwg: Correct typo in commit message]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/pnv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 231ed97..89b6801 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -378,8 +378,9 @@ static void powernv_populate_ipmi_bt(ISADevice *d, void *fdt, int lpc_off)
     _FDT(node);
     g_free(name);
 
-    fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs));
-    fdt_setprop(fdt, node, "compatible", compatible, sizeof(compatible));
+    _FDT((fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs))));
+    _FDT((fdt_setprop(fdt, node, "compatible", compatible,
+                      sizeof(compatible))));
 
     /* Mark it as reserved to avoid Linux trying to claim it */
     _FDT((fdt_setprop_string(fdt, node, "status", "reserved")));
-- 
2.9.4

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

* [Qemu-devel] [PULL 11/17] spapr: Allow boot from vhost-*-scsi backends
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (9 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 10/17] ppc/pnv: check the return value of fdt_setprop() David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 12/17] spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs David Gibson
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Felipe Franciosi, Mike Cui,
	David Gibson

From: Felipe Franciosi <felipe@nutanix.com>

The current implementation of spapr_get_fw_dev_path() doesn't take into
consideration vhost-*-scsi devices. This makes said devices unbootable
on PPC as SLOF is unable to work out the path to scan boot disks.

This makes VMs bootable on spapr when using vhost-*-scsi by implementing
a disk path for VHostSCSICommon (which currently includes both
vhost-user-scsi and vhost-scsi).

Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Signed-off-by: Mike Cui <cui@nutanix.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5d10366..c1c0951 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -57,6 +57,7 @@
 #include "hw/pci/pci.h"
 #include "hw/scsi/scsi.h"
 #include "hw/virtio/virtio-scsi.h"
+#include "hw/virtio/vhost-scsi-common.h"
 
 #include "exec/address-spaces.h"
 #include "hw/usb.h"
@@ -2384,6 +2385,7 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
     ((type *)object_dynamic_cast(OBJECT(obj), (name)))
     SCSIDevice *d = CAST(SCSIDevice,  dev, TYPE_SCSI_DEVICE);
     sPAPRPHBState *phb = CAST(sPAPRPHBState, dev, TYPE_SPAPR_PCI_HOST_BRIDGE);
+    VHostSCSICommon *vsc = CAST(VHostSCSICommon, dev, TYPE_VHOST_SCSI_COMMON);
 
     if (d) {
         void *spapr = CAST(void, bus->parent, "spapr-vscsi");
@@ -2440,6 +2442,12 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
         return g_strdup_printf("pci@%"PRIX64, phb->buid);
     }
 
+    if (vsc) {
+        /* Same logic as virtio above */
+        unsigned id = 0x1000000 | (vsc->target << 16) | vsc->lun;
+        return g_strdup_printf("disk@%"PRIX64, (uint64_t)id << 32);
+    }
+
     return NULL;
 }
 
-- 
2.9.4

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

* [Qemu-devel] [PULL 12/17] spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (10 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 11/17] spapr: Allow boot from vhost-*-scsi backends David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 13/17] spapr: Introduce DRC subclasses David Gibson
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, qemu-ppc, qemu-devel, mdroth, Greg Kurz, David Gibson

From: Greg Kurz <groug@kaod.org>

As explained in commit 5c0139a8c2f0 ("spapr: fix default DRC state for
coldplugged LMBs"), guests expect cold-plugged LMBs to be pre-allocated
and unisolated. The same goes for cold-plugged CPUs.

While here, let's convert g_assert(false) to the better self documenting
g_assert_not_reached().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_drc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index a35314e..c1d4b10 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -546,20 +546,16 @@ static bool spapr_drc_needed(void *opaque)
      */
     switch (drc->type) {
     case SPAPR_DR_CONNECTOR_TYPE_PCI:
-        rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) &&
-               (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) &&
-               drc->configured && drc->signalled && !drc->awaiting_release);
-        break;
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
     case SPAPR_DR_CONNECTOR_TYPE_LMB:
-        rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
-               (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) &&
+        rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) &&
+               (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) &&
                drc->configured && drc->signalled && !drc->awaiting_release);
         break;
     case SPAPR_DR_CONNECTOR_TYPE_PHB:
     case SPAPR_DR_CONNECTOR_TYPE_VIO:
     default:
-        g_assert(false);
+        g_assert_not_reached();
     }
     return rc;
 }
-- 
2.9.4

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

* [Qemu-devel] [PULL 13/17] spapr: Introduce DRC subclasses
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (11 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 12/17] spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 14/17] spapr: Clean up spapr_dr_connector_by_*() David Gibson
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

Currently we only have a single QOM type for all DRCs, but lots of
places where we switch behaviour based on the DRC's PAPR defined type.
This is a poor use of our existing type system.

So, instead create QOM subclasses for each PAPR defined DRC type.  We
also introduce intermediate subclasses for physical and logical DRCs,
a division which will be useful later on.

Instead of being stored in the DRC object itself, the PAPR type is now
stored in the class structure.  There are still many places where we
switch directly on the PAPR type value, but this at least provides the
basis to start to remove those.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c             |   5 +-
 hw/ppc/spapr_drc.c         | 123 +++++++++++++++++++++++++++++++++------------
 hw/ppc/spapr_pci.c         |   3 +-
 include/hw/ppc/spapr_drc.h |  47 +++++++++++++++--
 4 files changed, 139 insertions(+), 39 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c1c0951..11a04d1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1912,7 +1912,7 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
         uint64_t addr;
 
         addr = i * lmb_size + spapr->hotplug_memory.base;
-        drc = spapr_dr_connector_new(OBJECT(spapr), SPAPR_DR_CONNECTOR_TYPE_LMB,
+        drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
                                      addr/lmb_size);
         qemu_register_reset(spapr_drc_reset, drc);
     }
@@ -2009,8 +2009,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
 
         if (mc->has_hotpluggable_cpus) {
             sPAPRDRConnector *drc =
-                spapr_dr_connector_new(OBJECT(spapr),
-                                       SPAPR_DR_CONNECTOR_TYPE_CPU,
+                spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
                                        (core_id / smp_threads) * smt);
 
             qemu_register_reset(spapr_drc_reset, drc);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index c1d4b10..969d727 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -70,14 +70,23 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
     return shift;
 }
 
+sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
+    return 1 << drck->typeshift;
+}
+
 uint32_t spapr_drc_index(sPAPRDRConnector *drc)
 {
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
     /* no set format for a drc index: it only needs to be globally
      * unique. this is how we encode the DRC type on bare-metal
      * however, so might as well do that here
      */
-    return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) |
-            (drc->id & DRC_INDEX_ID_MASK);
+    return (drck->typeshift << DRC_INDEX_TYPE_SHIFT)
+        | (drc->id & DRC_INDEX_ID_MASK);
 }
 
 static uint32_t set_isolation_state(sPAPRDRConnector *drc,
@@ -107,7 +116,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
      * If the LMB being removed doesn't belong to a DIMM device that is
      * actually being unplugged, fail the isolation request here.
      */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_LMB) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
         if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
              !drc->awaiting_release) {
             return RTAS_OUT_HW_ERROR;
@@ -177,7 +186,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
         }
     }
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->allocation_state = state;
         if (drc->awaiting_release &&
             drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
@@ -191,11 +200,6 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
     return RTAS_OUT_SUCCESS;
 }
 
-sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
-{
-    return drc->type;
-}
-
 static const char *get_name(sPAPRDRConnector *drc)
 {
     return drc->name;
@@ -217,7 +221,7 @@ static void set_signalled(sPAPRDRConnector *drc)
 static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
 {
     if (drc->dev) {
-        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
             drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
             /* for logical DR, we return a state of UNUSABLE
              * iff the allocation state UNUSABLE.
@@ -235,7 +239,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
             *state = SPAPR_DR_ENTITY_SENSE_PRESENT;
         }
     } else {
-        if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+        if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
             /* PCI devices, and only PCI devices, use EMPTY
              * in cases where we'd otherwise use UNUSABLE
              */
@@ -368,7 +372,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
         error_setg(errp, "an attached device is still awaiting release");
         return;
     }
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE);
     }
     g_assert(fdt || coldplug);
@@ -380,7 +384,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
      * may be accessing the device, we can easily crash the guest, so we
      * we defer completion of removal in such cases to the reset() hook.
      */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
     }
     drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
@@ -398,10 +402,10 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
      * 'physical' DR resources such as PCI where each device/resource is
      * signalled individually.
      */
-    drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI)
+    drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI)
                      ? true : coldplug;
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->awaiting_allocation = true;
     }
 
@@ -441,7 +445,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
         return;
     }
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
         drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
         trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
         drc->awaiting_release = true;
@@ -458,8 +462,8 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
 
     drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
 
-    /* Calling release callbacks based on drc->type. */
-    switch (drc->type) {
+    /* Calling release callbacks based on spapr_drc_type(drc). */
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
         spapr_core_release(drc->dev);
         break;
@@ -515,7 +519,7 @@ static void reset(DeviceState *d)
         }
 
         /* non-PCI devices may be awaiting a transition to UNUSABLE */
-        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
             drc->awaiting_release) {
             drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
         }
@@ -544,7 +548,7 @@ static bool spapr_drc_needed(void *opaque)
      * If there is dev plugged in, we need to migrate the DRC state when
      * it is different from cold-plugged state
      */
-    switch (drc->type) {
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_PCI:
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
     case SPAPR_DR_CONNECTOR_TYPE_LMB:
@@ -626,17 +630,12 @@ static void unrealize(DeviceState *d, Error **errp)
     }
 }
 
-sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
-                                         sPAPRDRConnectorType type,
+sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
                                          uint32_t id)
 {
-    sPAPRDRConnector *drc =
-        SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
+    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type));
     char *prop_name;
 
-    g_assert(type);
-
-    drc->type = type;
     drc->id = id;
     drc->owner = owner;
     prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
@@ -666,7 +665,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
      * DRC names as documented by PAPR+ v2.7, 13.5.2.4
      * location codes as documented by PAPR+ v2.7, 12.3.1.5
      */
-    switch (drc->type) {
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
         drc->name = g_strdup_printf("CPU %d", id);
         break;
@@ -685,7 +684,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
     }
 
     /* PCI slot always start in a USABLE state, and stay there */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
     }
 
@@ -737,6 +736,27 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     dk->user_creatable = false;
 }
 
+static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
+}
+
+static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
+}
+
+static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
+}
+
 static const TypeInfo spapr_dr_connector_info = {
     .name          = TYPE_SPAPR_DR_CONNECTOR,
     .parent        = TYPE_DEVICE,
@@ -744,6 +764,42 @@ static const TypeInfo spapr_dr_connector_info = {
     .instance_init = spapr_dr_connector_instance_init,
     .class_size    = sizeof(sPAPRDRConnectorClass),
     .class_init    = spapr_dr_connector_class_init,
+    .abstract      = true,
+};
+
+static const TypeInfo spapr_drc_physical_info = {
+    .name          = TYPE_SPAPR_DRC_PHYSICAL,
+    .parent        = TYPE_SPAPR_DR_CONNECTOR,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .abstract      = true,
+};
+
+static const TypeInfo spapr_drc_logical_info = {
+    .name          = TYPE_SPAPR_DRC_LOGICAL,
+    .parent        = TYPE_SPAPR_DR_CONNECTOR,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .abstract      = true,
+};
+
+static const TypeInfo spapr_drc_cpu_info = {
+    .name          = TYPE_SPAPR_DRC_CPU,
+    .parent        = TYPE_SPAPR_DRC_LOGICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_cpu_class_init,
+};
+
+static const TypeInfo spapr_drc_pci_info = {
+    .name          = TYPE_SPAPR_DRC_PCI,
+    .parent        = TYPE_SPAPR_DRC_PHYSICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_pci_class_init,
+};
+
+static const TypeInfo spapr_drc_lmb_info = {
+    .name          = TYPE_SPAPR_DRC_LMB,
+    .parent        = TYPE_SPAPR_DRC_LOGICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_lmb_class_init,
 };
 
 /* helper functions for external users */
@@ -854,7 +910,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
             continue;
         }
 
-        if ((drc->type & drc_type_mask) == 0) {
+        if ((spapr_drc_type(drc) & drc_type_mask) == 0) {
             continue;
         }
 
@@ -874,7 +930,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
 
         /* ibm,drc-types */
         drc_types = g_string_append(drc_types,
-                                    spapr_drc_get_type_str(drc->type));
+                                    spapr_drc_get_type_str(spapr_drc_type(drc)));
         drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
     }
 
@@ -1206,6 +1262,11 @@ out:
 static void spapr_drc_register_types(void)
 {
     type_register_static(&spapr_dr_connector_info);
+    type_register_static(&spapr_drc_physical_info);
+    type_register_static(&spapr_drc_logical_info);
+    type_register_static(&spapr_drc_cpu_info);
+    type_register_static(&spapr_drc_pci_info);
+    type_register_static(&spapr_drc_lmb_info);
 
     spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
                         rtas_set_indicator);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 7a208a7..50d673b 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1761,8 +1761,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     /* allocate connectors for child PCI devices */
     if (sphb->dr_enabled) {
         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
-            spapr_dr_connector_new(OBJECT(phb),
-                                   SPAPR_DR_CONNECTOR_TYPE_PCI,
+            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
                                    (sphb->index << 16) | i);
         }
     }
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 10e7c24..969c16d 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -26,6 +26,48 @@
 #define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
                                              TYPE_SPAPR_DR_CONNECTOR)
 
+#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
+#define SPAPR_DRC_PHYSICAL_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHYSICAL)
+#define SPAPR_DRC_PHYSICAL_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
+                           TYPE_SPAPR_DRC_PHYSICAL)
+#define SPAPR_DRC_PHYSICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                             TYPE_SPAPR_DRC_PHYSICAL)
+
+#define TYPE_SPAPR_DRC_LOGICAL "spapr-drc-logical"
+#define SPAPR_DRC_LOGICAL_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LOGICAL)
+#define SPAPR_DRC_LOGICAL_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
+                           TYPE_SPAPR_DRC_LOGICAL)
+#define SPAPR_DRC_LOGICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                             TYPE_SPAPR_DRC_LOGICAL)
+
+#define TYPE_SPAPR_DRC_CPU "spapr-drc-cpu"
+#define SPAPR_DRC_CPU_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_CPU)
+#define SPAPR_DRC_CPU_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_CPU)
+#define SPAPR_DRC_CPU(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_CPU)
+
+#define TYPE_SPAPR_DRC_PCI "spapr-drc-pci"
+#define SPAPR_DRC_PCI_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PCI)
+#define SPAPR_DRC_PCI_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PCI)
+#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_PCI)
+
+#define TYPE_SPAPR_DRC_LMB "spapr-drc-lmb"
+#define SPAPR_DRC_LMB_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LMB)
+#define SPAPR_DRC_LMB_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_LMB)
+#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_LMB)
+
 /*
  * Various hotplug types managed by sPAPRDRConnector
  *
@@ -134,7 +176,6 @@ typedef struct sPAPRDRConnector {
     /*< private >*/
     DeviceState parent;
 
-    sPAPRDRConnectorType type;
     uint32_t id;
     Object *owner;
     const char *name;
@@ -163,6 +204,7 @@ typedef struct sPAPRDRConnectorClass {
     DeviceClass parent;
 
     /*< public >*/
+    sPAPRDRConnectorTypeShift typeshift;
 
     /* accessors for guest-visible (generally via RTAS) DR state */
     uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
@@ -186,8 +228,7 @@ typedef struct sPAPRDRConnectorClass {
 uint32_t spapr_drc_index(sPAPRDRConnector *drc);
 sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
 
-sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
-                                         sPAPRDRConnectorType type,
+sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
                                          uint32_t id);
 sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
 sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
-- 
2.9.4

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

* [Qemu-devel] [PULL 14/17] spapr: Clean up spapr_dr_connector_by_*()
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (12 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 13/17] spapr: Introduce DRC subclasses David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 15/17] spapr: Move configure-connector state into DRC David Gibson
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

 * Change names to something less ludicrously verbose
 * Now that we have QOM subclasses for the different DRC types, use a QOM
   typename instead of a PAPR type value parameter

The latter allows removal of the get_type_shift() helper.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c             | 28 ++++++++++++++--------------
 hw/ppc/spapr_drc.c         | 34 ++++++++++------------------------
 hw/ppc/spapr_events.c      |  2 +-
 hw/ppc/spapr_pci.c         |  6 ++----
 include/hw/ppc/spapr_drc.h |  5 ++---
 5 files changed, 29 insertions(+), 46 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 11a04d1..3fa9263 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -461,7 +461,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
     uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ];
     int i;
 
-    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index);
     if (drc) {
         drc_index = spapr_drc_index(drc);
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
@@ -654,7 +654,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
         if (i >= hotplug_lmb_start) {
             sPAPRDRConnector *drc;
 
-            drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, i);
+            drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, i);
             g_assert(drc);
 
             dynamic_memory[0] = cpu_to_be32(addr >> 32);
@@ -2536,8 +2536,8 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     uint64_t addr = addr_start;
 
     for (i = 0; i < nr_lmbs; i++) {
-        drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                addr/SPAPR_MEMORY_BLOCK_SIZE);
+        drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                              addr / SPAPR_MEMORY_BLOCK_SIZE);
         g_assert(drc);
 
         fdt = create_device_tree(&fdt_size);
@@ -2558,8 +2558,8 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
      */
     if (dev->hotplugged) {
         if (dedicated_hp_event_source) {
-            drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                    addr_start / SPAPR_MEMORY_BLOCK_SIZE);
+            drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                                  addr_start / SPAPR_MEMORY_BLOCK_SIZE);
             drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
             spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
                                                    nr_lmbs,
@@ -2676,8 +2676,8 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
 
     addr = addr_start;
     for (i = 0; i < nr_lmbs; i++) {
-        drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                                       addr / SPAPR_MEMORY_BLOCK_SIZE);
+        drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                              addr / SPAPR_MEMORY_BLOCK_SIZE);
         g_assert(drc);
         if (drc->indicator_state != SPAPR_DR_INDICATOR_STATE_INACTIVE) {
             avail_lmbs++;
@@ -2760,8 +2760,8 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev,
 
     addr = addr_start;
     for (i = 0; i < nr_lmbs; i++) {
-        drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                addr / SPAPR_MEMORY_BLOCK_SIZE);
+        drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                              addr / SPAPR_MEMORY_BLOCK_SIZE);
         g_assert(drc);
 
         drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -2769,8 +2769,8 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev,
         addr += SPAPR_MEMORY_BLOCK_SIZE;
     }
 
-    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
-                                   addr_start / SPAPR_MEMORY_BLOCK_SIZE);
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                          addr_start / SPAPR_MEMORY_BLOCK_SIZE);
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
                                               nr_lmbs, spapr_drc_index(drc));
@@ -2841,7 +2841,7 @@ void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev,
         return;
     }
 
-    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index * smt);
     g_assert(drc);
 
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -2876,7 +2876,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
                    cc->core_id);
         return;
     }
-    drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index * smt);
 
     g_assert(drc || !mc->has_hotpluggable_cpus);
 
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 969d727..7ed2de1 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -55,21 +55,6 @@ static void spapr_ccs_remove(sPAPRMachineState *spapr,
     g_free(ccs);
 }
 
-static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
-{
-    uint32_t shift = 0;
-
-    /* make sure this isn't SPAPR_DR_CONNECTOR_TYPE_ANY, or some
-     * other wonky value.
-     */
-    g_assert(is_power_of_2(type));
-
-    while (type != (1 << shift)) {
-        shift++;
-    }
-    return shift;
-}
-
 sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
 {
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -804,7 +789,7 @@ static const TypeInfo spapr_drc_lmb_info = {
 
 /* helper functions for external users */
 
-sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
+sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
 {
     Object *obj;
     char name[256];
@@ -815,12 +800,13 @@ sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
     return !obj ? NULL : SPAPR_DR_CONNECTOR(obj);
 }
 
-sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
-                                           uint32_t id)
+sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id)
 {
-    return spapr_dr_connector_by_index(
-            (get_type_shift(type) << DRC_INDEX_TYPE_SHIFT) |
-            (id & DRC_INDEX_ID_MASK));
+    sPAPRDRConnectorClass *drck
+        = SPAPR_DR_CONNECTOR_CLASS(object_class_by_name(type));
+
+    return spapr_drc_by_index(drck->typeshift << DRC_INDEX_TYPE_SHIFT
+                              | (id & DRC_INDEX_ID_MASK));
 }
 
 /* generate a string the describes the DRC to encode into the
@@ -1023,7 +1009,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* if this is a DR sensor we can assume sensor_index == drc_index */
-    drc = spapr_dr_connector_by_index(sensor_index);
+    drc = spapr_drc_by_index(sensor_index);
     if (!drc) {
         trace_spapr_rtas_set_indicator_invalid(sensor_index);
         ret = RTAS_OUT_PARAM_ERROR;
@@ -1096,7 +1082,7 @@ static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         goto out;
     }
 
-    drc = spapr_dr_connector_by_index(sensor_index);
+    drc = spapr_drc_by_index(sensor_index);
     if (!drc) {
         trace_spapr_rtas_get_sensor_state_invalid(sensor_index);
         ret = RTAS_OUT_PARAM_ERROR;
@@ -1161,7 +1147,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0);
 
     drc_index = rtas_ld(wa_addr, 0);
-    drc = spapr_dr_connector_by_index(drc_index);
+    drc = spapr_drc_by_index(drc_index);
     if (!drc) {
         trace_spapr_rtas_ibm_configure_connector_invalid(drc_index);
         rc = RTAS_OUT_PARAM_ERROR;
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index d349ae5..171aedc 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -477,7 +477,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
 
 static void spapr_hotplug_set_signalled(uint32_t drc_index)
 {
-    sPAPRDRConnector *drc = spapr_dr_connector_by_index(drc_index);
+    sPAPRDRConnector *drc = spapr_drc_by_index(drc_index);
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     drck->set_signalled(drc);
 }
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 50d673b..0c181bb 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1400,10 +1400,8 @@ static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
                                                     uint32_t busnr,
                                                     int32_t devfn)
 {
-    return spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_PCI,
-                                    (phb->index << 16) |
-                                    (busnr << 8) |
-                                    devfn);
+    return spapr_drc_by_id(TYPE_SPAPR_DRC_PCI,
+                           (phb->index << 16) | (busnr << 8) | devfn);
 }
 
 static sPAPRDRConnector *spapr_phb_get_pci_drc(sPAPRPHBState *phb,
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 969c16d..e4a25c8 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -230,9 +230,8 @@ sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
 
 sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
                                          uint32_t id);
-sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
-sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
-                                           uint32_t id);
+sPAPRDRConnector *spapr_drc_by_index(uint32_t index);
+sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id);
 int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
                           uint32_t drc_type_mask);
 
-- 
2.9.4

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

* [Qemu-devel] [PULL 15/17] spapr: Move configure-connector state into DRC
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (13 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 14/17] spapr: Clean up spapr_dr_connector_by_*() David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 16/17] spapr: Eliminate spapr_drc_get_type_str() David Gibson
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

Currently the sPAPRMachineState contains a list of sPAPRConfigureConnector
structures which store intermediate state for the ibm,configure-connector
RTAS call.

This was an attempt to separate this state from the core of the DRC state.
However the configure connector process is intimately tied to the DRC
model, so there's really no point trying to have two levels of interface
here.

Moving the configure-connector state into its corresponding DRC allows
removal of a number of helpers for maintaining the anciliary list.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c             |  4 ---
 hw/ppc/spapr_drc.c         | 73 ++++++++++++----------------------------------
 include/hw/ppc/spapr.h     | 14 ---------
 include/hw/ppc/spapr_drc.h |  7 +++++
 4 files changed, 25 insertions(+), 73 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3fa9263..86e6228 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2340,10 +2340,6 @@ static void ppc_spapr_init(MachineState *machine)
     register_savevm_live(NULL, "spapr/htab", -1, 1,
                          &savevm_htab_handlers, spapr);
 
-    /* used by RTAS */
-    QTAILQ_INIT(&spapr->ccs_list);
-    qemu_register_reset(spapr_ccs_reset_hook, spapr);
-
     qemu_register_boot_set(spapr_boot_set, spapr);
 
     if (kvm_enabled()) {
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 7ed2de1..783c621 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -27,34 +27,6 @@
 #define DRC_INDEX_TYPE_SHIFT 28
 #define DRC_INDEX_ID_MASK ((1ULL << DRC_INDEX_TYPE_SHIFT) - 1)
 
-static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr,
-                                                    uint32_t drc_index)
-{
-    sPAPRConfigureConnectorState *ccs = NULL;
-
-    QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) {
-        if (ccs->drc_index == drc_index) {
-            break;
-        }
-    }
-
-    return ccs;
-}
-
-static void spapr_ccs_add(sPAPRMachineState *spapr,
-                          sPAPRConfigureConnectorState *ccs)
-{
-    g_assert(!spapr_ccs_find(spapr, ccs->drc_index));
-    QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next);
-}
-
-static void spapr_ccs_remove(sPAPRMachineState *spapr,
-                             sPAPRConfigureConnectorState *ccs)
-{
-    QTAILQ_REMOVE(&spapr->ccs_list, ccs, next);
-    g_free(ccs);
-}
-
 sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
 {
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -81,6 +53,16 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
 
     trace_spapr_drc_set_isolation_state(spapr_drc_index(drc), state);
 
+    /* if the guest is configuring a device attached to this DRC, we
+     * should reset the configuration state at this point since it may
+     * no longer be reliable (guest released device and needs to start
+     * over, or unplug occurred so the FDT is no longer valid)
+     */
+    if (state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
+        g_free(drc->ccs);
+        drc->ccs = NULL;
+    }
+
     if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
         /* cannot unisolate a non-existent resource, and, or resources
          * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, 13.5.3.5)
@@ -485,6 +467,10 @@ static void reset(DeviceState *d)
     sPAPRDREntitySense state;
 
     trace_spapr_drc_reset(spapr_drc_index(drc));
+
+    g_free(drc->ccs);
+    drc->ccs = NULL;
+
     /* immediately upon reset we can safely assume DRCs whose devices
      * are pending removal can be safely removed, and that they will
      * subsequently be left in an ISOLATED state. move the DRC to this
@@ -1019,19 +1005,6 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     switch (sensor_type) {
     case RTAS_SENSOR_TYPE_ISOLATION_STATE:
-        /* if the guest is configuring a device attached to this
-         * DRC, we should reset the configuration state at this
-         * point since it may no longer be reliable (guest released
-         * device and needs to start over, or unplug occurred so
-         * the FDT is no longer valid)
-         */
-        if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-            sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr,
-                                                               sensor_index);
-            if (ccs) {
-                spapr_ccs_remove(spapr, ccs);
-            }
-        }
         ret = drck->set_isolation_state(drc, sensor_state);
         break;
     case RTAS_SENSOR_TYPE_DR:
@@ -1115,16 +1088,6 @@ static void configure_connector_st(target_ulong addr, target_ulong offset,
                               buf, MIN(len, CC_WA_LEN - offset));
 }
 
-void spapr_ccs_reset_hook(void *opaque)
-{
-    sPAPRMachineState *spapr = opaque;
-    sPAPRConfigureConnectorState *ccs, *ccs_tmp;
-
-    QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) {
-        spapr_ccs_remove(spapr, ccs);
-    }
-}
-
 static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
                                          sPAPRMachineState *spapr,
                                          uint32_t token, uint32_t nargs,
@@ -1160,12 +1123,11 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
         goto out;
     }
 
-    ccs = spapr_ccs_find(spapr, drc_index);
+    ccs = drc->ccs;
     if (!ccs) {
         ccs = g_new0(sPAPRConfigureConnectorState, 1);
         ccs->fdt_offset = drc->fdt_start_offset;
-        ccs->drc_index = drc_index;
-        spapr_ccs_add(spapr, ccs);
+        drc->ccs = ccs;
     }
 
     do {
@@ -1202,7 +1164,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
                     /* guest should be not configuring an isolated device */
                     trace_spapr_drc_set_configured_skipping(drc_index);
                 }
-                spapr_ccs_remove(spapr, ccs);
+                g_free(ccs);
+                drc->ccs = NULL;
                 ccs = NULL;
                 resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
             } else {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 98fb78b..f973b02 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -11,7 +11,6 @@
 struct VIOsPAPRBus;
 struct sPAPRPHBState;
 struct sPAPRNVRAM;
-typedef struct sPAPRConfigureConnectorState sPAPRConfigureConnectorState;
 typedef struct sPAPREventLogEntry sPAPREventLogEntry;
 typedef struct sPAPREventSource sPAPREventSource;
 
@@ -102,9 +101,6 @@ struct sPAPRMachineState {
     bool htab_first_pass;
     int htab_fd;
 
-    /* RTAS state */
-    QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
-
     /* Pending DIMM unplug cache. It is populated when a LMB
      * unplug starts. It can be regenerated if a migration
      * occurs during the unplug process. */
@@ -646,16 +642,6 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
 void spapr_core_release(DeviceState *dev);
 void spapr_lmb_release(DeviceState *dev);
 
-/* rtas-configure-connector state */
-struct sPAPRConfigureConnectorState {
-    uint32_t drc_index;
-    int fdt_offset;
-    int fdt_depth;
-    QTAILQ_ENTRY(sPAPRConfigureConnectorState) next;
-};
-
-void spapr_ccs_reset_hook(void *opaque);
-
 void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns);
 int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
 
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index e4a25c8..7dbb478 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -172,6 +172,12 @@ typedef enum {
     SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE = -9003,
 } sPAPRDRCCResponse;
 
+/* rtas-configure-connector state */
+typedef struct sPAPRConfigureConnectorState {
+    int fdt_offset;
+    int fdt_depth;
+} sPAPRConfigureConnectorState;
+
 typedef struct sPAPRDRConnector {
     /*< private >*/
     DeviceState parent;
@@ -189,6 +195,7 @@ typedef struct sPAPRDRConnector {
     void *fdt;
     int fdt_start_offset;
     bool configured;
+    sPAPRConfigureConnectorState *ccs;
 
     bool awaiting_release;
     bool signalled;
-- 
2.9.4

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

* [Qemu-devel] [PULL 16/17] spapr: Eliminate spapr_drc_get_type_str()
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (14 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 15/17] spapr: Move configure-connector state into DRC David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06  2:51 ` [Qemu-devel] [PULL 17/17] spapr: Remove some non-useful properties on DRC objects David Gibson
  2017-06-06 14:37 ` [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 Peter Maydell
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

This function was used in generating the device tree.  However, now that
we have different QOM types for different DRC types we can easily store
the information we need in the class structure and avoid this specialized
lookup function.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c         | 31 ++++---------------------------
 include/hw/ppc/spapr_drc.h |  1 +
 2 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 783c621..06df5d0 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -712,6 +712,7 @@ static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
 
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
+    drck->typename = "CPU";
 }
 
 static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
@@ -719,6 +720,7 @@ static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
 
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
+    drck->typename = "28";
 }
 
 static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
@@ -726,6 +728,7 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
 
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
+    drck->typename = "MEM";
 }
 
 static const TypeInfo spapr_dr_connector_info = {
@@ -795,31 +798,6 @@ sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id)
                               | (id & DRC_INDEX_ID_MASK));
 }
 
-/* generate a string the describes the DRC to encode into the
- * device tree.
- *
- * as documented by PAPR+ v2.7, 13.5.2.6 and C.6.1
- */
-static const char *spapr_drc_get_type_str(sPAPRDRConnectorType type)
-{
-    switch (type) {
-    case SPAPR_DR_CONNECTOR_TYPE_CPU:
-        return "CPU";
-    case SPAPR_DR_CONNECTOR_TYPE_PHB:
-        return "PHB";
-    case SPAPR_DR_CONNECTOR_TYPE_VIO:
-        return "SLOT";
-    case SPAPR_DR_CONNECTOR_TYPE_PCI:
-        return "28";
-    case SPAPR_DR_CONNECTOR_TYPE_LMB:
-        return "MEM";
-    default:
-        g_assert(false);
-    }
-
-    return NULL;
-}
-
 /**
  * spapr_drc_populate_dt
  *
@@ -901,8 +879,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
         drc_names = g_string_insert_len(drc_names, -1, "\0", 1);
 
         /* ibm,drc-types */
-        drc_types = g_string_append(drc_types,
-                                    spapr_drc_get_type_str(spapr_drc_type(drc)));
+        drc_types = g_string_append(drc_types, drck->typename);
         drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
     }
 
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 7dbb478..c88e1be 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -212,6 +212,7 @@ typedef struct sPAPRDRConnectorClass {
 
     /*< public >*/
     sPAPRDRConnectorTypeShift typeshift;
+    const char *typename; /* used in device tree, PAPR 13.5.2.6 & C.6.1 */
 
     /* accessors for guest-visible (generally via RTAS) DR state */
     uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
-- 
2.9.4

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

* [Qemu-devel] [PULL 17/17] spapr: Remove some non-useful properties on DRC objects
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (15 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 16/17] spapr: Eliminate spapr_drc_get_type_str() David Gibson
@ 2017-06-06  2:51 ` David Gibson
  2017-06-06 14:37 ` [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 Peter Maydell
  17 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-06-06  2:51 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, qemu-ppc, qemu-devel, mdroth, David Gibson

 * 'connector_type' is easily derived from the 'index' property, so there's
   no point to it (it's also implicit in the QOM type of the DRC)
 * 'isolation-state', 'indicator-state' and 'allocation-state' are
   part of the transaction between qemu and guest during PAPR hotplug
   operations, and outside tools really have no business looking at it
   (especially not changing, and these were RW properties)
 * 'entity-sense' is basically just a weird PAPR encoding of whether there
   is a device connected to this DRC

Strictly speaking removing these properties is breaking the qemu interface.
However, I'm pretty sure no management tools have ever used these.  For
debugging there are better alternatives.  Therefore, I think removing these
broken interfaces is the better option.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr_drc.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 06df5d0..39e7f30 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -228,14 +228,6 @@ static void prop_get_index(Object *obj, Visitor *v, const char *name,
     visit_type_uint32(v, name, &value, errp);
 }
 
-static void prop_get_type(Object *obj, Visitor *v, const char *name,
-                          void *opaque, Error **errp)
-{
-    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
-    uint32_t value = (uint32_t)spapr_drc_type(drc);
-    visit_type_uint32(v, name, &value, errp);
-}
-
 static char *prop_get_name(Object *obj, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
@@ -243,17 +235,6 @@ static char *prop_get_name(Object *obj, Error **errp)
     return g_strdup(drck->get_name(drc));
 }
 
-static void prop_get_entity_sense(Object *obj, Visitor *v, const char *name,
-                                  void *opaque, Error **errp)
-{
-    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
-    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-    uint32_t value;
-
-    drck->entity_sense(drc, &value);
-    visit_type_uint32(v, name, &value, errp);
-}
-
 static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
                          void *opaque, Error **errp)
 {
@@ -666,20 +647,10 @@ static void spapr_dr_connector_instance_init(Object *obj)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
 
-    object_property_add_uint32_ptr(obj, "isolation-state",
-                                   &drc->isolation_state, NULL);
-    object_property_add_uint32_ptr(obj, "indicator-state",
-                                   &drc->indicator_state, NULL);
-    object_property_add_uint32_ptr(obj, "allocation-state",
-                                   &drc->allocation_state, NULL);
     object_property_add_uint32_ptr(obj, "id", &drc->id, NULL);
     object_property_add(obj, "index", "uint32", prop_get_index,
                         NULL, NULL, NULL, NULL);
-    object_property_add(obj, "connector_type", "uint32", prop_get_type,
-                        NULL, NULL, NULL, NULL);
     object_property_add_str(obj, "name", prop_get_name, NULL, NULL);
-    object_property_add(obj, "entity-sense", "uint32", prop_get_entity_sense,
-                        NULL, NULL, NULL, NULL);
     object_property_add(obj, "fdt", "struct", prop_get_fdt,
                         NULL, NULL, NULL, NULL);
 }
-- 
2.9.4

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

* Re: [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606
  2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
                   ` (16 preceding siblings ...)
  2017-06-06  2:51 ` [Qemu-devel] [PULL 17/17] spapr: Remove some non-useful properties on DRC objects David Gibson
@ 2017-06-06 14:37 ` Peter Maydell
  17 siblings, 0 replies; 22+ messages in thread
From: Peter Maydell @ 2017-06-06 14:37 UTC (permalink / raw)
  To: David Gibson
  Cc: Alexander Graf, qemu-ppc@nongnu.org, QEMU Developers,
	Michael Roth

On 6 June 2017 at 03:51, David Gibson <david@gibson.dropbear.id.au> wrote:
> The following changes since commit 199e19ee538eb61fd08b1c1ee5aa838ebdcc968e:
>
>   Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into staging (2017-06-05 15:28:12 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170606
>
> for you to fetch changes up to 91dcb1ffa67cfa41a13dcc89dd44e2310b5773b0:
>
>   spapr: Remove some non-useful properties on DRC objects (2017-06-06 09:24:25 +1000)
>
> ----------------------------------------------------------------
> ppc patch queue 2017-06-06
>
> Accumulated patches for ppc targets and the pseries machine type.
>
> The big thing in this batch is a start on a substantial cleanup of the
> pseries hotplug mechanisms, which were pretty confusing.  For now
> these shouldn't cause substantial behavioural changes, but I am hoping
> these lead to clearer code and eventually to fixes for the bugs we
> have in hotplug handling, particularly when hotplug and migration are
> combined.
>
> The remaining patches are mostly bugfixes.
>

Applied, thanks.

-- PMM

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

* Re: [Qemu-devel] [PULL 01/17] migration: remove register_savevm()
  2017-06-06  2:51 ` [Qemu-devel] [PULL 01/17] migration: remove register_savevm() David Gibson
@ 2017-06-06 17:49   ` Peter Maydell
  2017-06-06 22:14     ` Paolo Bonzini
  2017-06-07  8:07     ` Juan Quintela
  0 siblings, 2 replies; 22+ messages in thread
From: Peter Maydell @ 2017-06-06 17:49 UTC (permalink / raw)
  To: David Gibson
  Cc: Alexander Graf, qemu-ppc@nongnu.org, QEMU Developers,
	Michael Roth, Laurent Vivier, Juan Quintela

On 6 June 2017 at 03:51, David Gibson <david@gibson.dropbear.id.au> wrote:
> From: Laurent Vivier <lvivier@redhat.com>
>
> We can replace the four remaining calls of register_savevm() by
> calls to register_savevm_live(). So we can remove the function and
> as we don't allocate anymore the ops pointer with g_new0()
> we don't have to free it then.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/net/vmxnet3.c            |  8 ++++++--
>  hw/s390x/s390-skeys.c       |  9 +++++++--
>  hw/s390x/s390-virtio-ccw.c  |  8 ++++++--
>  include/migration/vmstate.h |  8 --------
>  migration/savevm.c          | 16 ----------------
>  slirp/slirp.c               |  8 ++++++--
>  6 files changed, 25 insertions(+), 32 deletions(-)

Great to see register_savevm() finally disappearing.

Any chance of an update to docs/migration.txt, which still
mentions register_savevm(), but on the other hand doesn't
say anything about register_savevm_live() and unregister_savevm().
(Doc comments in the .h file for those functions would be
nice too...)

Things that would be interesting to explain/document:
 * what is special about vmxnet3 that makes it the only pci device
   that needs to use this rather than having a vmstate struct?
 * why does s390-skeys call the register function with a NULL
   pointer but the unregister pointer with a device pointer?

(Could we replace the uses of these which pass a dev pointer
with vmstate structs and then drop the dev parameter?)

thanks
-- PMM

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

* Re: [Qemu-devel] [PULL 01/17] migration: remove register_savevm()
  2017-06-06 17:49   ` Peter Maydell
@ 2017-06-06 22:14     ` Paolo Bonzini
  2017-06-07  8:07     ` Juan Quintela
  1 sibling, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2017-06-06 22:14 UTC (permalink / raw)
  To: Peter Maydell, David Gibson
  Cc: Laurent Vivier, Juan Quintela, QEMU Developers, Michael Roth,
	Alexander Graf, qemu-ppc@nongnu.org

On 06/06/2017 19:49, Peter Maydell wrote:
> Things that would be interesting to explain/document:
>  * what is special about vmxnet3 that makes it the only pci device
>    that needs to use this rather than having a vmstate struct?

Nothing, it should be replaced by VMSTATE_MSIX (at the cost of breaking
migration from and to older QEMU versions).

Paolo

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

* Re: [Qemu-devel] [PULL 01/17] migration: remove register_savevm()
  2017-06-06 17:49   ` Peter Maydell
  2017-06-06 22:14     ` Paolo Bonzini
@ 2017-06-07  8:07     ` Juan Quintela
  1 sibling, 0 replies; 22+ messages in thread
From: Juan Quintela @ 2017-06-07  8:07 UTC (permalink / raw)
  To: Peter Maydell
  Cc: David Gibson, Alexander Graf, qemu-ppc@nongnu.org,
	QEMU Developers, Michael Roth, Laurent Vivier

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 June 2017 at 03:51, David Gibson <david@gibson.dropbear.id.au> wrote:
>> From: Laurent Vivier <lvivier@redhat.com>
>>
>> We can replace the four remaining calls of register_savevm() by
>> calls to register_savevm_live(). So we can remove the function and
>> as we don't allocate anymore the ops pointer with g_new0()
>> we don't have to free it then.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/net/vmxnet3.c            |  8 ++++++--
>>  hw/s390x/s390-skeys.c       |  9 +++++++--
>>  hw/s390x/s390-virtio-ccw.c  |  8 ++++++--
>>  include/migration/vmstate.h |  8 --------
>>  migration/savevm.c          | 16 ----------------
>>  slirp/slirp.c               |  8 ++++++--
>>  6 files changed, 25 insertions(+), 32 deletions(-)
>
> Great to see register_savevm() finally disappearing.
>
> Any chance of an update to docs/migration.txt, which still
> mentions register_savevm(), but on the other hand doesn't
> say anything about register_savevm_live() and unregister_savevm().
> (Doc comments in the .h file for those functions would be
> nice too...)

Ok, will take a look.

> Things that would be interesting to explain/document:
>  * what is special about vmxnet3 that makes it the only pci device
>    that needs to use this rather than having a vmstate struct?

Will take a look.  vmxnet3 used to be a mess (in relation to migration).

>  * why does s390-skeys call the register function with a NULL
>    pointer but the unregister pointer with a device pointer?

No clue, will left that 

> (Could we replace the uses of these which pass a dev pointer
> with vmstate structs and then drop the dev parameter?)

Not sure, have to take a look.

Thanks, Juan.

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

end of thread, other threads:[~2017-06-07  8:08 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-06  2:51 [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 01/17] migration: remove register_savevm() David Gibson
2017-06-06 17:49   ` Peter Maydell
2017-06-06 22:14     ` Paolo Bonzini
2017-06-07  8:07     ` Juan Quintela
2017-06-06  2:51 ` [Qemu-devel] [PULL 02/17] migration: Mark CPU states dirty before incoming migration/loadvm David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 03/17] spapr: Move DRC RTAS calls into spapr_drc.c David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 04/17] spapr: Abolish DRC get_fdt method David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 05/17] spapr: Abolish DRC set_configured method David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 06/17] spapr: Make DRC get_index and get_type methods into plain functions David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 07/17] target-ppc: Fix openpic timer read register offset David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 08/17] target/ppc: Fixup set_spr error in h_register_process_table David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 09/17] spapr_nvram: Check return value from blk_getlength() David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 10/17] ppc/pnv: check the return value of fdt_setprop() David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 11/17] spapr: Allow boot from vhost-*-scsi backends David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 12/17] spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 13/17] spapr: Introduce DRC subclasses David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 14/17] spapr: Clean up spapr_dr_connector_by_*() David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 15/17] spapr: Move configure-connector state into DRC David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 16/17] spapr: Eliminate spapr_drc_get_type_str() David Gibson
2017-06-06  2:51 ` [Qemu-devel] [PULL 17/17] spapr: Remove some non-useful properties on DRC objects David Gibson
2017-06-06 14:37 ` [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170606 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).