[Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170711

[Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170711

[David Gibson]

The following changes since commit 6b06e3e49eb8c91cc286c16d6bf3181ac296f33d:

  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-07-10-v2' into staging (2017-07-10 16:12:47 +0100)

are available in the git repository at:

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

for you to fetch changes up to b87680427e8a3ff682f66514e99a8344e7437247:

  spapr: populate device tree depending on XIVE_EXPLOIT option (2017-07-11 11:04:02 +1000)

----------------------------------------------------------------
ppc patch queue 2017-07-11

  * Several minor cleanups from Greg Kurz
  * Fix for migration of pseries-2.7 and earlier machine types
  * More reworking of the DRC hotplug code, fixing several problems
    though there are still more to go
  * Fixes for CPU family / alias handling on POWER9
  * Preliminary patches for POWER9 XIVE (new interrupt controller)
    support
  * Assorted other fixes

----------------------------------------------------------------
Aaron Larson (1):
      target-ppc: SPR_BOOKE_ESR not set on FP exceptions

Cédric Le Goater (2):
      spapr: introduce the XIVE_EXPLOIT option in CAS
      spapr: populate device tree depending on XIVE_EXPLOIT option

David Gibson (6):
      spapr: Leave DR-indicator management to the guest
      spapr: Uniform DRC reset paths
      spapr: Add DRC release method
      spapr: Remove unnecessary differences between hotplug and coldplug paths
      spapr: Use unplug_request for PCI hot unplug
      spapr: Only report host/guest IOMMU page size mismatches on KVM

Greg Kurz (5):
      spapr: make spapr_populate_hotplug_cpu_dt() static
      spapr: refresh "platform-specific" hcalls comment
      spapr: fix bogus function name in comment
      spapr: fix memory hotplug error path
      ppc/kvm: have the "family" CPU alias to point to TYPE_HOST_POWERPC_CPU

Laurent Vivier (1):
      spapr: fix migration to pseries machine < 2.8

Suraj Jitindar Singh (2):
      target/ppc: Refactor tcg radix mmu code
      target/ppc: Add debug function for radix mmu translation

 hw/ppc/spapr.c              | 95 ++++++++++++++++++++-------------------------
 hw/ppc/spapr_drc.c          | 37 ++++++------------
 hw/ppc/spapr_pci.c          | 47 +++++++++++-----------
 include/hw/ppc/spapr.h      |  7 +---
 include/hw/ppc/spapr_drc.h  |  3 +-
 include/hw/ppc/spapr_ovec.h |  1 +
 target/ppc/excp_helper.c    |  1 +
 target/ppc/kvm.c            |  5 ++-
 target/ppc/mmu-radix64.c    | 67 +++++++++++++++++++++++++-------
 target/ppc/mmu-radix64.h    |  1 +
 target/ppc/mmu_helper.c     |  3 +-
 11 files changed, 145 insertions(+), 122 deletions(-)

[Qemu-devel] [PULL 01/17] spapr: make spapr_populate_hotplug_cpu_dt() static

[David Gibson]

From: Greg Kurz <groug@kaod.org>

Since commit ff9006ddbfd1 ("spapr: move spapr_core_[foo]plug() callbacks
close to machine code in spapr.c"), this function doesn't need to be extern
anymore.

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c         | 4 ++--
 include/hw/ppc/spapr.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ee9fac..65d8ad2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2863,8 +2863,8 @@ out:
     error_propagate(errp, local_err);
 }
 
-void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
-                                    sPAPRMachineState *spapr)
+static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
+                                           sPAPRMachineState *spapr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     DeviceClass *dc = DEVICE_GET_CLASS(cs);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a66bbac..12bf969 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -640,8 +640,6 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
 void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
                                                uint32_t count, uint32_t index);
 void spapr_cpu_parse_features(sPAPRMachineState *spapr);
-void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
-                                    sPAPRMachineState *spapr);
 
 /* CPU and LMB DRC release callbacks. */
 void spapr_core_release(DeviceState *dev);
-- 
2.9.4

[Qemu-devel] [PULL 02/17] spapr: refresh "platform-specific" hcalls comment

[David Gibson]

From: Greg Kurz <groug@kaod.org>

We have more of these since the addition of KVMPPC_H_LOGICAL_MEMOP in 2012.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/hw/ppc/spapr.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 12bf969..a184ffa 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -377,9 +377,8 @@ struct sPAPRMachineState {
  * as well.
  *
  * We also need some hcalls which are specific to qemu / KVM-on-POWER.
- * So far we just need one for H_RTAS, but in future we'll need more
- * for extensions like virtio.  We put those into the 0xf000-0xfffc
- * range which is reserved by PAPR for "platform-specific" hcalls.
+ * We put those into the 0xf000-0xfffc range which is reserved by PAPR
+ * for "platform-specific" hcalls.
  */
 #define KVMPPC_HCALL_BASE       0xf000
 #define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
-- 
2.9.4

[Qemu-devel] [PULL 03/17] spapr: fix bogus function name in comment

[David Gibson]

From: Greg Kurz <groug@kaod.org>

$ git grep spapr_ppc_reset
hw/ppc/spapr.c: * as part of spapr_ppc_reset().

$ git grep ppc_spapr_reset
hw/ppc/spapr.c:static void ppc_spapr_reset(void)
hw/ppc/spapr.c:    mc->reset = ppc_spapr_reset;
hw/ppc/spapr_hcall.c:        /* If ppc_spapr_reset() did not set up a HPT
 but one is necessary

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 65d8ad2..5acfb47 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1973,7 +1973,7 @@ static void spapr_boot_set(void *opaque, const char *boot_device,
  * Unlike PCI DR devices, LMB DR devices explicitly register this reset
  * routine. Reset for PCI DR devices will be handled by PHB reset routine
  * when it walks all its children devices. LMB devices reset occurs
- * as part of spapr_ppc_reset().
+ * as part of ppc_spapr_reset().
  */
 static void spapr_drc_reset(void *opaque)
 {
-- 
2.9.4

[Qemu-devel] [PULL 04/17] spapr: fix migration to pseries machine < 2.8

[David Gibson]

From: Laurent Vivier <lvivier@redhat.com>

since commit 5c4537bd ("spapr: Fix 2.7<->2.8 migration of PCI host bridge"),
some migration fields are forged from the new ones in spapr_pci_pre_save().

It works well, except when the number of MSI devices is 0,
because in this case the function exits immediately.

This fix moves the migration code before the exit code.

The problem can be reproduced with these commands:

source qemu-2.9:

    qemu-system-ppc64 -monitor stdio -M pseries-2.6 -nodefaults -S

destination qemu-2.6:

    qemu-system-ppc64 -monitor stdio -M pseries-2.6 -nodefaults \
                      -incoming tcp:0:4444

on the source:

    migrate tcp:localhost:4444

Destination fails with the following error:

    qemu-system-ppc64: error while loading state for
                       instance 0x0 of device 'spapr_pci'
    qemu-system-ppc64: load of migration failed: Invalid argument

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_pci.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 3b37dcd..f09b4e1 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1873,20 +1873,6 @@ static void spapr_pci_pre_save(void *opaque)
     gpointer key, value;
     int i;
 
-    g_free(sphb->msi_devs);
-    sphb->msi_devs = NULL;
-    sphb->msi_devs_num = g_hash_table_size(sphb->msi);
-    if (!sphb->msi_devs_num) {
-        return;
-    }
-    sphb->msi_devs = g_malloc(sphb->msi_devs_num * sizeof(spapr_pci_msi_mig));
-
-    g_hash_table_iter_init(&iter, sphb->msi);
-    for (i = 0; g_hash_table_iter_next(&iter, &key, &value); ++i) {
-        sphb->msi_devs[i].key = *(uint32_t *) key;
-        sphb->msi_devs[i].value = *(spapr_pci_msi *) value;
-    }
-
     if (sphb->pre_2_8_migration) {
         sphb->mig_liobn = sphb->dma_liobn[0];
         sphb->mig_mem_win_addr = sphb->mem_win_addr;
@@ -1900,6 +1886,20 @@ static void spapr_pci_pre_save(void *opaque)
             sphb->mig_mem_win_size += sphb->mem64_win_size;
         }
     }
+
+    g_free(sphb->msi_devs);
+    sphb->msi_devs = NULL;
+    sphb->msi_devs_num = g_hash_table_size(sphb->msi);
+    if (!sphb->msi_devs_num) {
+        return;
+    }
+    sphb->msi_devs = g_malloc(sphb->msi_devs_num * sizeof(spapr_pci_msi_mig));
+
+    g_hash_table_iter_init(&iter, sphb->msi);
+    for (i = 0; g_hash_table_iter_next(&iter, &key, &value); ++i) {
+        sphb->msi_devs[i].key = *(uint32_t *) key;
+        sphb->msi_devs[i].value = *(spapr_pci_msi *) value;
+    }
 }
 
 static int spapr_pci_post_load(void *opaque, int version_id)
-- 
2.9.4

[Qemu-devel] [PULL 05/17] target-ppc: SPR_BOOKE_ESR not set on FP exceptions

[David Gibson]

From: Aaron Larson <alarson@ddci.com>

Properly set the book E exception syndrome register when a floating
point exception occurs.

Currently on a book E processor, the POWERPC_EXCP_FP exception handler
fails to set "env->spr[SPR_BOOKE_ESR] = ESR_FP;" as required by the
book E specification.

Signed-off-by: Aaron Larson <alarson@ddci.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/excp_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 3a9f086..e6009e7 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -283,6 +283,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
              * precise in the MSR.
              */
             msr |= 0x00100000;
+            env->spr[SPR_BOOKE_ESR] = ESR_FP;
             break;
         case POWERPC_EXCP_INVAL:
             LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip);
-- 
2.9.4

[Qemu-devel] [PULL 06/17] spapr: Leave DR-indicator management to the guest

[David Gibson]

The DR-indicator is essentially a "virtual LED" attached to a hotpluggable
device, which the guest can set to various states for the attention of
the operator or management layers.

It's mostly guest managed, except that we once-off set it to
ACTIVE/INACTIVE in the attach/detach path.  While that makes certain sense,
there's no indication in PAPR that the hypervisor should do this, and the
drmgr code on the guest side doesn't appear to need it (it will already set
the indicator to ACTIVE on hotplug, and INACTIVE on remove).

So, leave the DR-indicator entirely to the guest; the only thing we need
to do is ensure it's in a sane state on reset.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_drc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index bd40b84..22d4d81 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -353,8 +353,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
     }
     g_assert(fdt || coldplug);
 
-    drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
-
     drc->dev = d;
     drc->fdt = fdt;
     drc->fdt_start_offset = fdt_start_offset;
@@ -372,8 +370,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
 
 static void spapr_drc_release(sPAPRDRConnector *drc)
 {
-    drc->dr_indicator = SPAPR_DR_INDICATOR_INACTIVE;
-
     /* Calling release callbacks based on spapr_drc_type(drc). */
     switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
@@ -454,12 +450,14 @@ static void reset(DeviceState *d)
         if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
             drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
         }
+        drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
     } else {
         /* Otherwise device is absent, but might be hotplugged */
         drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED;
         if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
             drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_UNUSABLE;
         }
+        drc->dr_indicator = SPAPR_DR_INDICATOR_INACTIVE;
     }
 }
 
-- 
2.9.4

[Qemu-devel] [PULL 07/17] spapr: Uniform DRC reset paths

[David Gibson]

DRC objects have a regular device reset method.  However, it only gets
called in the usual way for PCI DRCs.  Because of where CPU and LMB DRCs
are in the QOM tree, their device reset method isn't automatically called.
So, the machine manually registers reset handlers to call device_reset().

This patch removes the device reset method, and instead always explicitly
registers the reset handler from realize().  This means the callers don't
have to worry about the two cases, and we always get proper resets.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/ppc/spapr.c     | 31 ++++---------------------------
 hw/ppc/spapr_drc.c |  6 +++---
 2 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5acfb47..4fa982d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1967,24 +1967,6 @@ static void spapr_boot_set(void *opaque, const char *boot_device,
     machine->boot_order = g_strdup(boot_device);
 }
 
-/*
- * Reset routine for LMB DR devices.
- *
- * Unlike PCI DR devices, LMB DR devices explicitly register this reset
- * routine. Reset for PCI DR devices will be handled by PHB reset routine
- * when it walks all its children devices. LMB devices reset occurs
- * as part of ppc_spapr_reset().
- */
-static void spapr_drc_reset(void *opaque)
-{
-    sPAPRDRConnector *drc = opaque;
-    DeviceState *d = DEVICE(drc);
-
-    if (d) {
-        device_reset(d);
-    }
-}
-
 static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
 {
     MachineState *machine = MACHINE(spapr);
@@ -1993,13 +1975,11 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
     int i;
 
     for (i = 0; i < nr_lmbs; i++) {
-        sPAPRDRConnector *drc;
         uint64_t addr;
 
         addr = i * lmb_size + spapr->hotplug_memory.base;
-        drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
-                                     addr/lmb_size);
-        qemu_register_reset(spapr_drc_reset, drc);
+        spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
+                               addr / lmb_size);
     }
 }
 
@@ -2093,11 +2073,8 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
         int core_id = i * smp_threads;
 
         if (mc->has_hotpluggable_cpus) {
-            sPAPRDRConnector *drc =
-                spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
-                                       (core_id / smp_threads) * smt);
-
-            qemu_register_reset(spapr_drc_reset, drc);
+            spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
+                                   (core_id / smp_threads) * smt);
         }
 
         if (i < boot_cores_nr) {
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 22d4d81..c831aa3 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -426,9 +426,9 @@ static bool release_pending(sPAPRDRConnector *drc)
     return drc->awaiting_release;
 }
 
-static void reset(DeviceState *d)
+static void drc_reset(void *opaque)
 {
-    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
+    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(opaque);
 
     trace_spapr_drc_reset(spapr_drc_index(drc));
 
@@ -538,6 +538,7 @@ static void realize(DeviceState *d, Error **errp)
     g_free(child_name);
     vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
                      drc);
+    qemu_register_reset(drc_reset, drc);
     trace_spapr_drc_realize_complete(spapr_drc_index(drc));
 }
 
@@ -596,7 +597,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     DeviceClass *dk = DEVICE_CLASS(k);
     sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
 
-    dk->reset = reset;
     dk->realize = realize;
     dk->unrealize = unrealize;
     drck->release_pending = release_pending;
-- 
2.9.4

[Qemu-devel] [PULL 08/17] spapr: Add DRC release method

[David Gibson]

At the moment, spapr_drc_release() has an ugly switch on the DRC type to
call the right, device-specific release function.  This cleans it up by
doing that via a proper QOM method.

It's still arguably an abstraction violation for the DRC code to call into
the specific device code, but one mess at a time.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_drc.c         | 22 ++++++----------------
 include/hw/ppc/spapr_drc.h |  1 +
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index c831aa3..aa37a47 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -370,22 +370,9 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
 
 static void spapr_drc_release(sPAPRDRConnector *drc)
 {
-    /* 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;
-    case SPAPR_DR_CONNECTOR_TYPE_PCI:
-        spapr_phb_remove_pci_device_cb(drc->dev);
-        break;
-    case SPAPR_DR_CONNECTOR_TYPE_LMB:
-        spapr_lmb_release(drc->dev);
-        break;
-    case SPAPR_DR_CONNECTOR_TYPE_PHB:
-    case SPAPR_DR_CONNECTOR_TYPE_VIO:
-    default:
-        g_assert(false);
-    }
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
+    drck->release(drc->dev);
 
     drc->awaiting_release = false;
     g_free(drc->fdt);
@@ -631,6 +618,7 @@ static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
     drck->typename = "CPU";
     drck->drc_name_prefix = "CPU ";
+    drck->release = spapr_core_release;
 }
 
 static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
@@ -640,6 +628,7 @@ static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
     drck->typename = "28";
     drck->drc_name_prefix = "C";
+    drck->release = spapr_phb_remove_pci_device_cb;
 }
 
 static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
@@ -649,6 +638,7 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
     drck->typename = "MEM";
     drck->drc_name_prefix = "LMB ";
+    drck->release = spapr_lmb_release;
 }
 
 static const TypeInfo spapr_dr_connector_info = {
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index d9cacb3..6fd84d1 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -217,6 +217,7 @@ typedef struct sPAPRDRConnectorClass {
     sPAPRDREntitySense (*dr_entity_sense)(sPAPRDRConnector *drc);
     uint32_t (*isolate)(sPAPRDRConnector *drc);
     uint32_t (*unisolate)(sPAPRDRConnector *drc);
+    void (*release)(DeviceState *dev);
 
     /* QEMU interfaces for managing hotplug operations */
     bool (*release_pending)(sPAPRDRConnector *drc);
-- 
2.9.4

[Qemu-devel] [PULL 09/17] spapr: Remove unnecessary differences between hotplug and coldplug paths

[David Gibson]

spapr_drc_attach() has a 'coldplug' parameter which sets the DRC into
configured state initially, instead of the usual ISOLATED/UNUSABLE state.
It turns out this is unnecessary: although coldplugged devices do need to
be in CONFIGURED state once the guest starts, that will already be
accomplished by the reset code which will move DRCs for already plugged
devices into a coldplug equivalent state.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c             | 13 +++----------
 hw/ppc/spapr_drc.c         |  5 ++---
 hw/ppc/spapr_pci.c         |  3 +--
 include/hw/ppc/spapr_drc.h |  2 +-
 4 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4fa982d..70b3fd3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2611,7 +2611,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
         fdt_offset = spapr_populate_memory_node(fdt, node, addr,
                                                 SPAPR_MEMORY_BLOCK_SIZE);
 
-        spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
+        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
         addr += SPAPR_MEMORY_BLOCK_SIZE;
     }
     /* send hotplug notification to the
@@ -2956,17 +2956,10 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 
     g_assert(drc || !mc->has_hotpluggable_cpus);
 
-    /*
-     * Setup CPU DT entries only for hotplugged CPUs. For boot time or
-     * coldplugged CPUs DT entries are setup in spapr_build_fdt().
-     */
-    if (dev->hotplugged) {
-        fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
-    }
+    fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
 
     if (drc) {
-        spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged,
-                         &local_err);
+        spapr_drc_attach(drc, dev, fdt, fdt_offset, &local_err);
         if (local_err) {
             g_free(fdt);
             error_propagate(errp, local_err);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index aa37a47..f34355d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -340,7 +340,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
 }
 
 void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
-                      int fdt_start_offset, bool coldplug, Error **errp)
+                      int fdt_start_offset, Error **errp)
 {
     trace_spapr_drc_attach(spapr_drc_index(drc));
 
@@ -351,12 +351,11 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
     if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE);
     }
-    g_assert(fdt || coldplug);
+    g_assert(fdt);
 
     drc->dev = d;
     drc->fdt = fdt;
     drc->fdt_start_offset = fdt_start_offset;
-    drc->configured = coldplug;
 
     if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->awaiting_allocation = true;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f09b4e1..49c8db8 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1435,8 +1435,7 @@ static void spapr_phb_hot_plug_child(HotplugHandler *plug_handler,
         goto out;
     }
 
-    spapr_drc_attach(drc, DEVICE(pdev), fdt, fdt_start_offset,
-                     !plugged_dev->hotplugged, &local_err);
+    spapr_drc_attach(drc, DEVICE(pdev), fdt, fdt_start_offset, &local_err);
     if (local_err) {
         goto out;
     }
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 6fd84d1..d15e9eb 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -234,7 +234,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
                           uint32_t drc_type_mask);
 
 void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
-                      int fdt_start_offset, bool coldplug, Error **errp);
+                      int fdt_start_offset, Error **errp);
 void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp);
 
 #endif /* HW_SPAPR_DRC_H */
-- 
2.9.4

Re: [Qemu-devel] [PULL 09/17] spapr: Remove unnecessary differences between hotplug and coldplug paths

[Bharata B Rao]

On Tue, Jul 11, 2017 at 02:39:09PM +1000, David Gibson wrote:
> spapr_drc_attach() has a 'coldplug' parameter which sets the DRC into
> configured state initially, instead of the usual ISOLATED/UNUSABLE state.
> It turns out this is unnecessary: although coldplugged devices do need to
> be in CONFIGURED state once the guest starts, that will already be
> accomplished by the reset code which will move DRCs for already plugged
> devices into a coldplug equivalent state.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr.c             | 13 +++----------
>  hw/ppc/spapr_drc.c         |  5 ++---
>  hw/ppc/spapr_pci.c         |  3 +--
>  include/hw/ppc/spapr_drc.h |  2 +-
>  4 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4fa982d..70b3fd3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2611,7 +2611,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
>          fdt_offset = spapr_populate_memory_node(fdt, node, addr,
>                                                  SPAPR_MEMORY_BLOCK_SIZE);
> 
> -        spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> +        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
>          addr += SPAPR_MEMORY_BLOCK_SIZE;
>      }
>      /* send hotplug notification to the
> @@ -2956,17 +2956,10 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> 
>      g_assert(drc || !mc->has_hotpluggable_cpus);
> 
> -    /*
> -     * Setup CPU DT entries only for hotplugged CPUs. For boot time or
> -     * coldplugged CPUs DT entries are setup in spapr_build_fdt().
> -     */
> -    if (dev->hotplugged) {
> -        fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> -    }
> +    fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);

A harmless (but still good to get rid of) side effect of this change is
that we are now building the CPU device tree for boot time and cold-plugged
CPUs twice:

first from spapr_core_plug() -> spapr_populate_hotplug_cpu_dt()
second from spapr_build_fdt() -> spapr_populate_cpus_dt_node()

The first one is not used while the 2nd one acutally ends up building
the CPUs DT entries for the boot time and cold-plugged CPUs.

Regards,
Bharata.

Re: [Qemu-devel] [PULL 09/17] spapr: Remove unnecessary differences between hotplug and coldplug paths

[Greg Kurz]

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

On Wed, 12 Jul 2017 13:56:37 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Tue, Jul 11, 2017 at 02:39:09PM +1000, David Gibson wrote:
> > spapr_drc_attach() has a 'coldplug' parameter which sets the DRC into
> > configured state initially, instead of the usual ISOLATED/UNUSABLE state.
> > It turns out this is unnecessary: although coldplugged devices do need to
> > be in CONFIGURED state once the guest starts, that will already be
> > accomplished by the reset code which will move DRCs for already plugged
> > devices into a coldplug equivalent state.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/ppc/spapr.c             | 13 +++----------
> >  hw/ppc/spapr_drc.c         |  5 ++---
> >  hw/ppc/spapr_pci.c         |  3 +--
> >  include/hw/ppc/spapr_drc.h |  2 +-
> >  4 files changed, 7 insertions(+), 16 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4fa982d..70b3fd3 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2611,7 +2611,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
> >          fdt_offset = spapr_populate_memory_node(fdt, node, addr,
> >                                                  SPAPR_MEMORY_BLOCK_SIZE);
> > 
> > -        spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> > +        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
> >          addr += SPAPR_MEMORY_BLOCK_SIZE;
> >      }
> >      /* send hotplug notification to the
> > @@ -2956,17 +2956,10 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > 
> >      g_assert(drc || !mc->has_hotpluggable_cpus);
> > 
> > -    /*
> > -     * Setup CPU DT entries only for hotplugged CPUs. For boot time or
> > -     * coldplugged CPUs DT entries are setup in spapr_build_fdt().
> > -     */
> > -    if (dev->hotplugged) {
> > -        fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> > -    }
> > +    fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);  
> 
> A harmless (but still good to get rid of) side effect of this change is

Well, not that harmless... this is leaked for machine types that don't
support CPU hotplug. I'll send a patch.

> that we are now building the CPU device tree for boot time and cold-plugged
> CPUs twice:
> 
> first from spapr_core_plug() -> spapr_populate_hotplug_cpu_dt()
> second from spapr_build_fdt() -> spapr_populate_cpus_dt_node()
> 
> The first one is not used while the 2nd one acutally ends up building
> the CPUs DT entries for the boot time and cold-plugged CPUs.
> 
> Regards,
> Bharata.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PULL 09/17] spapr: Remove unnecessary differences between hotplug and coldplug paths

[David Gibson]

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

On Wed, Jul 12, 2017 at 01:56:37PM +0530, Bharata B Rao wrote:
> On Tue, Jul 11, 2017 at 02:39:09PM +1000, David Gibson wrote:
> > spapr_drc_attach() has a 'coldplug' parameter which sets the DRC into
> > configured state initially, instead of the usual ISOLATED/UNUSABLE state.
> > It turns out this is unnecessary: although coldplugged devices do need to
> > be in CONFIGURED state once the guest starts, that will already be
> > accomplished by the reset code which will move DRCs for already plugged
> > devices into a coldplug equivalent state.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/ppc/spapr.c             | 13 +++----------
> >  hw/ppc/spapr_drc.c         |  5 ++---
> >  hw/ppc/spapr_pci.c         |  3 +--
> >  include/hw/ppc/spapr_drc.h |  2 +-
> >  4 files changed, 7 insertions(+), 16 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4fa982d..70b3fd3 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2611,7 +2611,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
> >          fdt_offset = spapr_populate_memory_node(fdt, node, addr,
> >                                                  SPAPR_MEMORY_BLOCK_SIZE);
> > 
> > -        spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
> > +        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
> >          addr += SPAPR_MEMORY_BLOCK_SIZE;
> >      }
> >      /* send hotplug notification to the
> > @@ -2956,17 +2956,10 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > 
> >      g_assert(drc || !mc->has_hotpluggable_cpus);
> > 
> > -    /*
> > -     * Setup CPU DT entries only for hotplugged CPUs. For boot time or
> > -     * coldplugged CPUs DT entries are setup in spapr_build_fdt().
> > -     */
> > -    if (dev->hotplugged) {
> > -        fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> > -    }
> > +    fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> 
> A harmless (but still good to get rid of) side effect of this change is
> that we are now building the CPU device tree for boot time and cold-plugged
> CPUs twice:
> 
> first from spapr_core_plug() -> spapr_populate_hotplug_cpu_dt()
> second from spapr_build_fdt() -> spapr_populate_cpus_dt_node()
> 
> The first one is not used while the 2nd one acutally ends up building
> the CPUs DT entries for the boot time and cold-plugged CPUs.

Yeah, I know.  I think simpler code is more important than the handful
of wasted cycles.  We might be able to avoid this with some of the DT
generation cleanups I have on the long term radar.

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

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

[Qemu-devel] [PULL 10/17] spapr: Use unplug_request for PCI hot unplug

[David Gibson]

AIUI, ->unplug_request in the HotplugHandler is used for "soft"
unplug, where acknowledgement from the guest is required before
completing the unplug, whereas ->unplug is used for "hard" unplug
where qemu unilaterally removes the device, and the guest just has to
cope with its sudden absence.  For spapr we (correctly) use
->unplug_request for CPU and memory hot unplug but we use ->unplug for
PCI.

While I think it might be possible to support "hard" PCI unplug within
the PAPR model, that's not how it actually works now.  Although it's
called from ->unplug, the PCI unplug path will usually just mark the
device for removal, with completion of the unplug delayed until
userspace responds to the unplug notification. If the guest doesn't
respond as expected, that could delay the unplug completion arbitrarily
long.

To reflect that, change the PCI unplug path to be called from
->unplug_request.  We also rename spapr_phb_hot_plug_child() and
spapr_phb_hot_unplug_child() to spapr_pci_plug() and
spapr_pci_unplug_request() to more obviously reflect the callbacks they're
implementing.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 49c8db8..cc1588d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1388,8 +1388,8 @@ static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb,
     return spapr_drc_index(drc);
 }
 
-static void spapr_phb_hot_plug_child(HotplugHandler *plug_handler,
-                                     DeviceState *plugged_dev, Error **errp)
+static void spapr_pci_plug(HotplugHandler *plug_handler,
+                           DeviceState *plugged_dev, Error **errp)
 {
     sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
     PCIDevice *pdev = PCI_DEVICE(plugged_dev);
@@ -1469,8 +1469,8 @@ out:
     }
 }
 
-static void spapr_phb_hot_unplug_child(HotplugHandler *plug_handler,
-                                       DeviceState *plugged_dev, Error **errp)
+static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
+                                     DeviceState *plugged_dev, Error **errp)
 {
     sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
     PCIDevice *pdev = PCI_DEVICE(plugged_dev);
@@ -1485,6 +1485,7 @@ static void spapr_phb_hot_unplug_child(HotplugHandler *plug_handler,
     }
 
     g_assert(drc);
+    g_assert(drc->dev == plugged_dev);
 
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
     if (!drck->release_pending(drc)) {
@@ -1972,8 +1973,8 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
     /* Supported by TYPE_SPAPR_MACHINE */
     dc->user_creatable = true;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    hp->plug = spapr_phb_hot_plug_child;
-    hp->unplug = spapr_phb_hot_unplug_child;
+    hp->plug = spapr_pci_plug;
+    hp->unplug_request = spapr_pci_unplug_request;
 }
 
 static const TypeInfo spapr_phb_info = {
-- 
2.9.4

[Qemu-devel] [PULL 11/17] target/ppc: Refactor tcg radix mmu code

[David Gibson]

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

The mmu-radix64.c file implements functions to enable the radix mmu
emulation in tcg mode. There is a function ppc_radix64_walk_tree() which
performs the radix tree walk and also implicitly checks the pte
protection.

Move the protection checking of the pte from the ppc_radix64_walk_tree()
function into the caller. This means the ppc_radix64_walk_tree() function
can be used without protection checking which is useful for debugging.

ppc_radix64_walk_tree() no longer needs to take the rwx and prot variables.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/mmu-radix64.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 69fde65..1a650fd 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -147,11 +147,10 @@ static void ppc_radix64_set_rc(PowerPCCPU *cpu, int rwx, uint64_t pte,
     }
 }
 
-static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int rwx, vaddr eaddr,
+static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, vaddr eaddr,
                                       uint64_t base_addr, uint64_t nls,
                                       hwaddr *raddr, int *psize,
-                                      int *fault_cause, int *prot,
-                                      hwaddr *pte_addr)
+                                      int *fault_cause, hwaddr *pte_addr)
 {
     CPUState *cs = CPU(cpu);
     uint64_t index, pde;
@@ -177,10 +176,6 @@ static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int rwx, vaddr eaddr,
         uint64_t rpn = pde & R_PTE_RPN;
         uint64_t mask = (1UL << *psize) - 1;
 
-        if (ppc_radix64_check_prot(cpu, rwx, pde, fault_cause, prot)) {
-            return 0; /* Protection Denied Access */
-        }
-
         /* Or high bits of rpn and low bits to ea to form whole real addr */
         *raddr = (rpn & ~mask) | (eaddr & mask);
         *pte_addr = base_addr + (index * sizeof(pde));
@@ -188,9 +183,8 @@ static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int rwx, vaddr eaddr,
     }
 
     /* Next Level of Radix Tree */
-    return ppc_radix64_walk_tree(cpu, rwx, eaddr, pde & R_PDE_NLB,
-                                 pde & R_PDE_NLS, raddr, psize,
-                                 fault_cause, prot, pte_addr);
+    return ppc_radix64_walk_tree(cpu, eaddr, pde & R_PDE_NLB, pde & R_PDE_NLS,
+                                 raddr, psize, fault_cause, pte_addr);
 }
 
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
@@ -241,11 +235,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
 
     /* Walk Radix Tree from Process Table Entry to Convert EA to RA */
     page_size = PRTBE_R_GET_RTS(prtbe0);
-    pte = ppc_radix64_walk_tree(cpu, rwx, eaddr & R_EADDR_MASK,
+    pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
                                 prtbe0 & PRTBE_R_RPDB, prtbe0 & PRTBE_R_RPDS,
-                                &raddr, &page_size, &fault_cause, &prot,
-                                &pte_addr);
-    if (!pte) {
+                                &raddr, &page_size, &fault_cause, &pte_addr);
+    if (!pte || ppc_radix64_check_prot(cpu, rwx, pte, &fault_cause, &prot)) {
+        /* Couldn't get pte or access denied due to protection */
         ppc_radix64_raise_si(cpu, rwx, eaddr, fault_cause);
         return 1;
     }
-- 
2.9.4

[Qemu-devel] [PULL 12/17] target/ppc: Add debug function for radix mmu translation

[David Gibson]

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

In target/ppc/mmu-hash64.c there already exists the function
ppc_hash64_get_phys_page_debug() to get the physical (real) address for
a given effective address in hash mode.

Implement the function ppc_radix64_get_phys_page_debug() to allow a real
address to be obtained for a given effective address in radix mode.
This is used when a debugger is attached to qemu.

Previously we just had a comment saying this is unimplemented which then
fell through to the default case and caused an abort due to
unrecognised mmu model as the default had no case for the V3 mmu, which
was misleading at best.

We reuse ppc_radix64_walk_tree() which is used by the radix fault
handler since the process of walking the radix tree is identical.

Reported-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/mmu-radix64.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 target/ppc/mmu-radix64.h |  1 +
 target/ppc/mmu_helper.c  |  3 ++-
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 1a650fd..bbd37e3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -251,3 +251,48 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
                  prot, mmu_idx, 1UL << page_size);
     return 0;
 }
+
+hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
+{
+    CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
+    PPCVirtualHypervisorClass *vhc =
+        PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+    hwaddr raddr, pte_addr;
+    uint64_t lpid = 0, pid = 0, offset, size, patbe, prtbe0, pte;
+    int page_size, fault_cause = 0;
+
+    /* Handle Real Mode */
+    if (msr_dr == 0) {
+        /* In real mode top 4 effective addr bits (mostly) ignored */
+        return eaddr & 0x0FFFFFFFFFFFFFFFULL;
+    }
+
+    /* Virtual Mode Access - get the fully qualified address */
+    if (!ppc_radix64_get_fully_qualified_addr(env, eaddr, &lpid, &pid)) {
+        return -1;
+    }
+
+    /* Get Process Table */
+    patbe = vhc->get_patbe(cpu->vhyp);
+
+    /* Index Process Table by PID to Find Corresponding Process Table Entry */
+    offset = pid * sizeof(struct prtb_entry);
+    size = 1ULL << ((patbe & PATBE1_R_PRTS) + 12);
+    if (offset >= size) {
+        /* offset exceeds size of the process table */
+        return -1;
+    }
+    prtbe0 = ldq_phys(cs->as, (patbe & PATBE1_R_PRTB) + offset);
+
+    /* Walk Radix Tree from Process Table Entry to Convert EA to RA */
+    page_size = PRTBE_R_GET_RTS(prtbe0);
+    pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
+                                prtbe0 & PRTBE_R_RPDB, prtbe0 & PRTBE_R_RPDS,
+                                &raddr, &page_size, &fault_cause, &pte_addr);
+    if (!pte) {
+        return -1;
+    }
+
+    return raddr & TARGET_PAGE_MASK;
+}
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index 1d5c7cf..0ecf063 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -46,6 +46,7 @@
 
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
                                  int mmu_idx);
+hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
 {
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 65d1c86..b7b9088 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -30,6 +30,7 @@
 #include "helper_regs.h"
 #include "qemu/error-report.h"
 #include "mmu-book3s-v3.h"
+#include "mmu-radix64.h"
 
 //#define DEBUG_MMU
 //#define DEBUG_BATS
@@ -1432,7 +1433,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
         return ppc_hash64_get_phys_page_debug(cpu, addr);
     case POWERPC_MMU_VER_3_00:
         if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
-            /* TODO - Unsupported */
+            return ppc_radix64_get_phys_page_debug(cpu, addr);
         } else {
             return ppc_hash64_get_phys_page_debug(cpu, addr);
         }
-- 
2.9.4

[Qemu-devel] [PULL 13/17] spapr: fix memory hotplug error path

[David Gibson]

From: Greg Kurz <groug@kaod.org>

QEMU shouldn't abort if spapr_add_lmbs()->spapr_drc_attach() fails.
Let's propagate the error instead, like it is done everywhere else
where spapr_drc_attach() is called.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 70b3fd3..ba8f57a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2601,6 +2601,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
     int i, fdt_offset, fdt_size;
     void *fdt;
     uint64_t addr = addr_start;
+    Error *local_err = NULL;
 
     for (i = 0; i < nr_lmbs; i++) {
         drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
@@ -2611,7 +2612,18 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size,
         fdt_offset = spapr_populate_memory_node(fdt, node, addr,
                                                 SPAPR_MEMORY_BLOCK_SIZE);
 
-        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
+        spapr_drc_attach(drc, dev, fdt, fdt_offset, &local_err);
+        if (local_err) {
+            while (addr > addr_start) {
+                addr -= SPAPR_MEMORY_BLOCK_SIZE;
+                drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
+                                      addr / SPAPR_MEMORY_BLOCK_SIZE);
+                spapr_drc_detach(drc, dev, NULL);
+            }
+            g_free(fdt);
+            error_propagate(errp, local_err);
+            return;
+        }
         addr += SPAPR_MEMORY_BLOCK_SIZE;
     }
     /* send hotplug notification to the
@@ -2651,14 +2663,20 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     addr = object_property_get_uint(OBJECT(dimm),
                                     PC_DIMM_ADDR_PROP, &local_err);
     if (local_err) {
-        pc_dimm_memory_unplug(dev, &ms->hotplug_memory, mr);
-        goto out;
+        goto out_unplug;
     }
 
     spapr_add_lmbs(dev, addr, size, node,
                    spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
-                   &error_abort);
+                   &local_err);
+    if (local_err) {
+        goto out_unplug;
+    }
+
+    return;
 
+out_unplug:
+    pc_dimm_memory_unplug(dev, &ms->hotplug_memory, mr);
 out:
     error_propagate(errp, local_err);
 }
-- 
2.9.4

[Qemu-devel] [PULL 14/17] spapr: Only report host/guest IOMMU page size mismatches on KVM

[David Gibson]

We print a warning if the spapr IOMMU isn't configured to support a page
size matching the host page size backing RAM.  When that's the case we need
more complex logic to translate VFIO mappings, which is slower.

But, it's not so slow that it would be at all noticeable against the
general slowness of TCG.  So, only warn when using KVM.  This removes some
noisy and unhelpful warnings from make check on hosts with page sizes
which typically differ from those on POWER (e.g. Sparc).

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index cc1588d..a52dcf8 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1745,7 +1745,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     }
 
     /* DMA setup */
-    if ((sphb->page_size_mask & qemu_getrampagesize()) == 0) {
+    if (((sphb->page_size_mask & qemu_getrampagesize()) == 0)
+        && kvm_enabled()) {
         error_report("System page size 0x%lx is not enabled in page_size_mask "
                      "(0x%"PRIx64"). Performance may be slow",
                      qemu_getrampagesize(), sphb->page_size_mask);
-- 
2.9.4

[Qemu-devel] [PULL 15/17] ppc/kvm: have the "family" CPU alias to point to TYPE_HOST_POWERPC_CPU

[David Gibson]

From: Greg Kurz <groug@kaod.org>

When running KVM on POWER, we allow the user to pass "-cpu POWERx" instead
of "-cpu host". This is achieved by patching the ppc_cpu_aliases[] array
so that "POWERx" points to the CPU class with the same PVR as the host CPU.
This causes CPUs to be instantiated from this CPU class instead of the
TYPE_HOST_POWERPC_CPU class which is used with "-cpu host". These CPUs thus
miss all the KVM specific tuning from kvmppc_host_cpu_class_init().

This currently causes QEMU with "-cpu POWER9" to fail when running KVM on a
POWER9 DD1 host:

qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only
 "-cpu host" is possible
kvm_init_vcpu failed: Invalid argument

Let's have the "POWERx" alias to point to TYPE_HOST_POWERPC_CPU directly,
so that "-cpu POWERx" instantiates CPUs from the same class as "-cpu host".

Signed-off-by: Greg Kurz <groug@kaod.org>
Tested-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/kvm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index f2f7c53..f7a7ea5 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2445,6 +2445,7 @@ static int kvm_ppc_register_host_cpu_type(void)
         .class_init = kvmppc_host_cpu_class_init,
     };
     PowerPCCPUClass *pvr_pcc;
+    ObjectClass *oc;
     DeviceClass *dc;
     int i;
 
@@ -2455,6 +2456,9 @@ static int kvm_ppc_register_host_cpu_type(void)
     type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
     type_register(&type_info);
 
+    oc = object_class_by_name(type_info.name);
+    g_assert(oc);
+
 #if defined(TARGET_PPC64)
     type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
     type_info.parent = TYPE_SPAPR_CPU_CORE,
@@ -2474,7 +2478,6 @@ static int kvm_ppc_register_host_cpu_type(void)
     dc = DEVICE_CLASS(ppc_cpu_get_family_class(pvr_pcc));
     for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
         if (strcmp(ppc_cpu_aliases[i].alias, dc->desc) == 0) {
-            ObjectClass *oc = OBJECT_CLASS(pvr_pcc);
             char *suffix;
 
             ppc_cpu_aliases[i].model = g_strdup(object_class_get_name(oc));
-- 
2.9.4

[Qemu-devel] [PULL 16/17] spapr: introduce the XIVE_EXPLOIT option in CAS

[David Gibson]

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

On POWER9, the Client Architecture Support (CAS) negotiation process
determines whether the guest operates in XIVE Legacy compatibility
(the former POWER8 interrupt model) or in XIVE exploitation mode (the
newer POWER9 interrupt model).

Bit 7 of Byte 23 of vector 5 is used for this purpose.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c              | 13 +++++++------
 include/hw/ppc/spapr_ovec.h |  1 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ba8f57a..ff78a22 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -910,7 +910,8 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
 {
     PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
 
-    char val[2 * 3] = {
+    char val[2 * 4] = {
+        23, 0x00, /* Xive mode: 0 = legacy (as in ISA 2.7), 1 = Exploitation */
         24, 0x00, /* Hash/Radix, filled in below. */
         25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
         26, 0x40, /* Radix options: GTSE == yes. */
@@ -918,19 +919,19 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
 
     if (kvm_enabled()) {
         if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) {
-            val[1] = 0x80; /* OV5_MMU_BOTH */
+            val[3] = 0x80; /* OV5_MMU_BOTH */
         } else if (kvmppc_has_cap_mmu_radix()) {
-            val[1] = 0x40; /* OV5_MMU_RADIX_300 */
+            val[3] = 0x40; /* OV5_MMU_RADIX_300 */
         } else {
-            val[1] = 0x00; /* Hash */
+            val[3] = 0x00; /* Hash */
         }
     } else {
         if (first_ppc_cpu->env.mmu_model & POWERPC_MMU_V3) {
             /* V3 MMU supports both hash and radix (with dynamic switching) */
-            val[1] = 0xC0;
+            val[3] = 0xC0;
         } else {
             /* Otherwise we can only do hash */
-            val[1] = 0x00;
+            val[3] = 0x00;
         }
     }
     _FDT(fdt_setprop(fdt, chosen, "ibm,arch-vec-5-platform-support",
diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
index f088833..0b464e2 100644
--- a/include/hw/ppc/spapr_ovec.h
+++ b/include/hw/ppc/spapr_ovec.h
@@ -50,6 +50,7 @@ typedef struct sPAPROptionVector sPAPROptionVector;
 #define OV5_DRCONF_MEMORY       OV_BIT(2, 2)
 #define OV5_FORM1_AFFINITY      OV_BIT(5, 0)
 #define OV5_HP_EVT              OV_BIT(6, 5)
+#define OV5_XIVE_EXPLOIT        OV_BIT(23, 7)
 
 /* ISA 3.00 MMU features: */
 #define OV5_MMU_BOTH            OV_BIT(24, 0) /* Radix and hash */
-- 
2.9.4

[Qemu-devel] [PULL 17/17] spapr: populate device tree depending on XIVE_EXPLOIT option

[David Gibson]

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

When XIVE is supported, the device tree should be populated
accordingly and the XIVE memory regions mapped to activate MMIOs.

Depending on the design we choose, we could also allocate different
ICS and ICP objects, or switch between objects. This needs to be
discussed.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ff78a22..f4e011c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -778,6 +778,11 @@ static int spapr_dt_cas_updates(sPAPRMachineState *spapr, void *fdt,
         }
     }
 
+    /* /interrupt controller */
+    if (!spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)) {
+        spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
+    }
+
     offset = fdt_path_offset(fdt, "/chosen");
     if (offset < 0) {
         offset = fdt_add_subnode(fdt, 0, "chosen");
@@ -801,7 +806,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
 
     size -= sizeof(hdr);
 
-    /* Create sceleton */
+    /* Create skeleton */
     fdt_skel = g_malloc0(size);
     _FDT((fdt_create(fdt_skel, size)));
     _FDT((fdt_begin_node(fdt_skel, "")));
@@ -1069,9 +1074,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     _FDT(fdt_setprop_cell(fdt, 0, "#address-cells", 2));
     _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
 
-    /* /interrupt controller */
-    spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
-
     ret = spapr_populate_memory(spapr, fdt);
     if (ret < 0) {
         error_report("couldn't setup memory nodes in fdt");
-- 
2.9.4

Re: [Qemu-devel] [PULL 00/17] ppc-for-2.10 queue 20170711

[Peter Maydell]

On 11 July 2017 at 05:39, David Gibson <david@gibson.dropbear.id.au> wrote:
> The following changes since commit 6b06e3e49eb8c91cc286c16d6bf3181ac296f33d:
>
>   Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-07-10-v2' into staging (2017-07-10 16:12:47 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170711
>
> for you to fetch changes up to b87680427e8a3ff682f66514e99a8344e7437247:
>
>   spapr: populate device tree depending on XIVE_EXPLOIT option (2017-07-11 11:04:02 +1000)
>
> ----------------------------------------------------------------
> ppc patch queue 2017-07-11
>
>   * Several minor cleanups from Greg Kurz
>   * Fix for migration of pseries-2.7 and earlier machine types
>   * More reworking of the DRC hotplug code, fixing several problems
>     though there are still more to go
>   * Fixes for CPU family / alias handling on POWER9
>   * Preliminary patches for POWER9 XIVE (new interrupt controller)
>     support
>   * Assorted other fixes
>

Applied, thanks.

-- PMM