All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701
@ 2016-07-01  6:41 David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 01/23] ppc: Add a bunch of hypervisor SPRs to Book3s David Gibson
                   ` (23 more replies)
  0 siblings, 24 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, benh, qemu-devel, qemu-ppc, David Gibson

The following changes since commit 8a0b4de048e20215415b24c7b42514c27b9d6ef3:

  pcspk: fix KVM (2016-06-30 19:00:02 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.7-20160701

for you to fetch changes up to 13f5e8003e7b67039cb7a19e41b4f7f7ac669cb3:

  qmp: fix spapr example of query-hotpluggable-cpus (2016-07-01 13:41:47 +1000)

----------------------------------------------------------------
ppc patch queue 2016-07-01

Here's the current ppc patch queue.  This is a fairly large batch,
containing:
    * A number of further preliminary patches towards full hypervisor
      mode emulation
    * Some further fixes / cleanups for the recently merged device_add
      based CPU hotplug
    * Preliminary patches towards supporting a native (rather than
      paravirtualized) XICS device.  This will be needed to emulate a
      physical Power machine, including hypervisor capabilities
    * Assorted bug fixes

----------------------------------------------------------------
Aaron Larson (2):
      target-ppc: Eliminate redundant and incorrect function booke206_page_size_to_tlb
      target-ppc: gen_pause for instructions: yield, mdoio, mdoom, miso

Benjamin Herrenschmidt (13):
      ppc: Add a bunch of hypervisor SPRs to Book3s
      ppc: Update LPCR definitions
      ppc: Use a helper to filter writes to LPCR
      ppc: Fix conditions for delivering external interrupts to a guest
      ppc: Enforce setting MSR:EE,IR and DR when MSR:PR is set
      ppc: Initial HDEC support
      ppc: LPCR is a HV resource
      ppc: Print HSRR0/HSRR1 in "info registers"
      ppc: Fix 64K pages support in full emulation
      ppc/xics: Rename existing xics to xics_spapr
      ppc/xics: Move SPAPR specific code to a separate file
      ppc/xics: Implement H_IPOLL using an accessor
      ppc/xics: Replace "icp" with "xics" in most places

Bharata B Rao (2):
      spapr: Restore support for older PowerPC CPU cores
      spapr: Restore support for 970MP and POWER8NVL CPU cores

Greg Kurz (4):
      spapr: fix write-past-end-of-array error in cpu core device init code
      spapr: drop reference on child object during core realization
      spapr: do proper error propagation in spapr_cpu_core_realize_child()
      spapr: drop duplicate variable in spapr_core_release()

Igor Mammedov (1):
      qmp: fix spapr example of query-hotpluggable-cpus

Thomas Huth (1):
      hw/ppc/spapr: Add some missing hcall function set strings

 default-configs/ppc64-softmmu.mak |   1 +
 hw/intc/Makefile.objs             |   1 +
 hw/intc/xics.c                    | 537 ++++++--------------------------------
 hw/intc/xics_kvm.c                |  63 ++---
 hw/intc/xics_spapr.c              | 434 ++++++++++++++++++++++++++++++
 hw/ppc/e500.c                     |   2 +-
 hw/ppc/e500.h                     |   2 +
 hw/ppc/ppc.c                      |  17 +-
 hw/ppc/ppce500_spin.c             |   7 +-
 hw/ppc/spapr.c                    |  22 +-
 hw/ppc/spapr_cpu_core.c           |  48 +++-
 hw/ppc/spapr_events.c             |   8 +-
 hw/ppc/spapr_pci.c                |  13 +-
 hw/ppc/spapr_vio.c                |   2 +-
 include/hw/pci-host/spapr.h       |   2 +-
 include/hw/ppc/spapr.h            |   2 +-
 include/hw/ppc/spapr_vio.h        |   2 +-
 include/hw/ppc/xics.h             |  54 +++-
 qmp-commands.hx                   |   4 +-
 target-ppc/cpu-qom.h              |   3 +
 target-ppc/cpu.h                  |  16 +-
 target-ppc/excp_helper.c          |  41 ++-
 target-ppc/helper.h               |   3 +
 target-ppc/helper_regs.h          |   4 +
 target-ppc/mmu-hash64.c           |  96 ++++++-
 target-ppc/timebase_helper.c      |  10 +
 target-ppc/translate.c            |  22 +-
 target-ppc/translate_init.c       | 216 +++++++++++++--
 28 files changed, 1022 insertions(+), 610 deletions(-)
 create mode 100644 hw/intc/xics_spapr.c

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

* [Qemu-devel] [PULL 01/23] ppc: Add a bunch of hypervisor SPRs to Book3s
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 02/23] ppc: Update LPCR definitions David Gibson
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We don't give them a KVM reg number yet as no current KVM version
supports HV mode.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: SPRs AMOR,DAWR,DARWX were already included in commit f401dd32cb8e9]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate_init.c | 119 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 116 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 08bdd07..d328455 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7793,15 +7793,116 @@ static void gen_spr_power5p_lpar(CPUPPCState *env)
     spr_register_kvm(env, SPR_LPCR, "LPCR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
-                     KVM_REG_PPC_LPCR, 0x00000000);
+                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
 }
 
+#if !defined(CONFIG_USER_ONLY)
+static void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
+{
+    TCGv hmer = tcg_temp_new();
+
+    gen_load_spr(hmer, sprn);
+    tcg_gen_and_tl(hmer, cpu_gpr[gprn], hmer);
+    gen_store_spr(sprn, hmer);
+    spr_store_dump_spr(sprn);
+    tcg_temp_free(hmer);
+}
+#endif
+
 static void gen_spr_book3s_ids(CPUPPCState *env)
 {
+    /* FIXME: Will need to deal with thread vs core only SPRs */
+
     /* Processor identification */
-    spr_register(env, SPR_PIR, "PIR",
+    spr_register_hv(env, SPR_PIR, "PIR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_pir,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, NULL,
+                 0x00000000);
+    spr_register_hv(env, SPR_HID0, "HID0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TSCR, "TSCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMER, "HMER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_hmer,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMEER, "HMEER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TFMR, "TFMR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_LPIDR, "LPIDR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HFSCR, "HFSCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_MMCRC, "MMCRC",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_MMCRH, "MMCRH",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSPRG0, "HSPRG0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSPRG1, "HSPRG1",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSRR0, "HSRR0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSRR1, "HSRR1",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HDAR, "HDAR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HDSISR, "HDSISR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_RMOR, "RMOR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HRMOR, "HRMOR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
                  0x00000000);
 }
 
@@ -8060,6 +8161,17 @@ static void gen_spr_power7_book4(CPUPPCState *env)
 #endif
 }
 
+static void gen_spr_power8_rpr(CPUPPCState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+    spr_register_hv(env, SPR_RPR, "RPR",
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    &spr_read_generic, &spr_write_generic,
+                    0x00000103070F1F3F);
+#endif
+}
+
 static void init_proc_book3s_64(CPUPPCState *env, int version)
 {
     gen_spr_ne_601(env);
@@ -8117,6 +8229,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
         gen_spr_vtb(env);
         gen_spr_power8_ic(env);
         gen_spr_power8_book4(env);
+        gen_spr_power8_rpr(env);
     }
     if (version < BOOK3S_CPU_POWER8) {
         gen_spr_book3s_dbg(env);
-- 
2.7.4

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

* [Qemu-devel] [PULL 02/23] ppc: Update LPCR definitions
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 01/23] ppc: Add a bunch of hypervisor SPRs to Book3s David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 03/23] ppc: Use a helper to filter writes to LPCR David Gibson
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Includes all the bits up to ISA 2.07

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 534381e..af73bce 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -377,12 +377,16 @@ struct ppc_slb_t {
 #define LPCR_VPM1         (1ull << (63 - 1))
 #define LPCR_ISL          (1ull << (63 - 2))
 #define LPCR_KBV          (1ull << (63 - 3))
+#define LPCR_DPFD_SHIFT   (63 - 11)
+#define LPCR_DPFD         (0x3ull << LPCR_DPFD_SHIFT)
+#define LPCR_VRMASD_SHIFT (63 - 16)
+#define LPCR_VRMASD       (0x1full << LPCR_VRMASD_SHIFT)
+#define LPCR_RMLS_SHIFT   (63 - 37)
+#define LPCR_RMLS         (0xfull << LPCR_RMLS_SHIFT)
 #define LPCR_ILE          (1ull << (63 - 38))
-#define LPCR_MER          (1ull << (63 - 52))
-#define LPCR_LPES0        (1ull << (63 - 60))
-#define LPCR_LPES1        (1ull << (63 - 61))
 #define LPCR_AIL_SHIFT    (63 - 40)      /* Alternate interrupt location */
 #define LPCR_AIL          (3ull << LPCR_AIL_SHIFT)
+#define LPCR_ONL          (1ull << (63 - 45))
 #define LPCR_P7_PECE0     (1ull << (63 - 49))
 #define LPCR_P7_PECE1     (1ull << (63 - 50))
 #define LPCR_P7_PECE2     (1ull << (63 - 51))
@@ -391,6 +395,12 @@ struct ppc_slb_t {
 #define LPCR_P8_PECE2     (1ull << (63 - 49))
 #define LPCR_P8_PECE3     (1ull << (63 - 50))
 #define LPCR_P8_PECE4     (1ull << (63 - 51))
+#define LPCR_MER          (1ull << (63 - 52))
+#define LPCR_TC           (1ull << (63 - 54))
+#define LPCR_LPES0        (1ull << (63 - 60))
+#define LPCR_LPES1        (1ull << (63 - 61))
+#define LPCR_RMI          (1ull << (63 - 62))
+#define LPCR_HDICE        (1ull << (63 - 63))
 
 #define msr_sf   ((env->msr >> MSR_SF)   & 1)
 #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
-- 
2.7.4

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

* [Qemu-devel] [PULL 03/23] ppc: Use a helper to filter writes to LPCR
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 01/23] ppc: Add a bunch of hypervisor SPRs to Book3s David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 02/23] ppc: Update LPCR definitions David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 04/23] ppc: Fix conditions for delivering external interrupts to a guest David Gibson
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This handles filtering bits based on what is implemented by a
given architecture version. We also use it to copy to LPCR
some of the relevant 970 HID4 bits.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/helper.h         |  1 +
 target-ppc/mmu-hash64.c     | 57 +++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate_init.c | 56 +++++++++++++++++++++++++++++---------------
 3 files changed, 95 insertions(+), 19 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 5056ac2..c532b44 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -16,6 +16,7 @@ DEF_HELPER_1(rfmci, void, env)
 DEF_HELPER_2(pminsn, void, env, i32)
 DEF_HELPER_1(rfid, void, env)
 DEF_HELPER_1(hrfid, void, env)
+DEF_HELPER_2(store_lpcr, void, env, tl)
 #endif
 DEF_HELPER_1(check_tlb_flush, void, env)
 #endif
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 5b7b5e9..6d6f26c 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -851,3 +851,60 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
      */
     tlb_flush(CPU(cpu), 1);
 }
+
+void helper_store_lpcr(CPUPPCState *env, target_ulong val)
+{
+    uint64_t lpcr = 0;
+
+    /* Filter out bits */
+    switch (env->mmu_model) {
+    case POWERPC_MMU_64B: /* 970 */
+        if (val & 0x40) {
+            lpcr |= LPCR_LPES0;
+        }
+        if (val & 0x8000000000000000ull) {
+            lpcr |= LPCR_LPES1;
+        }
+        if (val & 0x20) {
+            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
+        }
+        if (val & 0x4000000000000000ull) {
+            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
+        }
+        if (val & 0x2000000000000000ull) {
+            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
+        }
+        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
+
+        /* XXX We could also write LPID from HID4 here
+         * but since we don't tag any translation on it
+         * it doesn't actually matter
+         */
+        /* XXX For proper emulation of 970 we also need
+         * to dig HRMOR out of HID5
+         */
+        break;
+    case POWERPC_MMU_2_03: /* P5p */
+        lpcr = val & (LPCR_RMLS | LPCR_ILE |
+                      LPCR_LPES0 | LPCR_LPES1 |
+                      LPCR_RMI | LPCR_HDICE);
+        break;
+    case POWERPC_MMU_2_06: /* P7 */
+        lpcr = val & (LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_DPFD |
+                      LPCR_VRMASD | LPCR_RMLS | LPCR_ILE |
+                      LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2 |
+                      LPCR_MER | LPCR_TC |
+                      LPCR_LPES0 | LPCR_LPES1 | LPCR_HDICE);
+        break;
+    case POWERPC_MMU_2_07: /* P8 */
+        lpcr = val & (LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV |
+                      LPCR_DPFD | LPCR_VRMASD | LPCR_RMLS | LPCR_ILE |
+                      LPCR_AIL | LPCR_ONL | LPCR_P8_PECE0 | LPCR_P8_PECE1 |
+                      LPCR_P8_PECE2 | LPCR_P8_PECE3 | LPCR_P8_PECE4 |
+                      LPCR_MER | LPCR_TC | LPCR_LPES0 | LPCR_HDICE);
+        break;
+    default:
+        ;
+    }
+    env->spr[SPR_LPCR] = lpcr;
+}
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d328455..af7a790 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7525,16 +7525,6 @@ static void gen_spr_970_hior(CPUPPCState *env)
                  0x00000000);
 }
 
-static void gen_spr_970_lpar(CPUPPCState *env)
-{
-    /* Logical partitionning */
-    /* PPC970: HID4 is effectively the LPCR */
-    spr_register(env, SPR_970_HID4, "HID4",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
-                 0x00000000);
-}
-
 static void gen_spr_book3s_common(CPUPPCState *env)
 {
     spr_register(env, SPR_CTRL, "SPR_CTRL",
@@ -7787,15 +7777,6 @@ static void gen_spr_power5p_ear(CPUPPCState *env)
                  0x00000000);
 }
 
-static void gen_spr_power5p_lpar(CPUPPCState *env)
-{
-    /* Logical partitionning */
-    spr_register_kvm(env, SPR_LPCR, "LPCR",
-                     SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
-                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
-}
-
 #if !defined(CONFIG_USER_ONLY)
 static void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
 {
@@ -7807,7 +7788,44 @@ static void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
     spr_store_dump_spr(sprn);
     tcg_temp_free(hmer);
 }
+
+static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
+{
+    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
+}
+
+static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
+{
+#if defined(TARGET_PPC64)
+    spr_write_generic(ctx, sprn, gprn);
+    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
+#endif
+}
+
+#endif /* !defined(CONFIG_USER_ONLY) */
+
+static void gen_spr_970_lpar(CPUPPCState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+    /* Logical partitionning */
+    /* PPC970: HID4 is effectively the LPCR */
+    spr_register(env, SPR_970_HID4, "HID4",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_970_hid4,
+                 0x00000000);
+#endif
+}
+
+static void gen_spr_power5p_lpar(CPUPPCState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+    /* Logical partitionning */
+    spr_register_kvm(env, SPR_LPCR, "LPCR",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_generic, &spr_write_lpcr,
+                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
 #endif
+}
 
 static void gen_spr_book3s_ids(CPUPPCState *env)
 {
-- 
2.7.4

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

* [Qemu-devel] [PULL 04/23] ppc: Fix conditions for delivering external interrupts to a guest
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (2 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 03/23] ppc: Use a helper to filter writes to LPCR David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set David Gibson
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

External interrupts can bypass the MSR_EE test if they occur in guest
mode and LPES0 is clear. In that case they are directed to the hypervisor

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/excp_helper.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 533866b..26adda4 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -794,6 +794,14 @@ static void ppc_hw_interrupt(CPUPPCState *env)
             return;
         }
     }
+    /* Extermal interrupt can ignore MSR:EE under some circumstances */
+    if (env->pending_interrupts & (1 << PPC_INTERRUPT_EXT)) {
+        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
+        if (msr_ee != 0 || (env->has_hv_mode && msr_hv == 0 && !lpes0)) {
+            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_EXTERNAL);
+            return;
+        }
+    }
     if (msr_ce != 0) {
         /* External critical interrupt */
         if (env->pending_interrupts & (1 << PPC_INTERRUPT_CEXT)) {
@@ -839,17 +847,6 @@ static void ppc_hw_interrupt(CPUPPCState *env)
             powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DECR);
             return;
         }
-        /* External interrupt */
-        if (env->pending_interrupts & (1 << PPC_INTERRUPT_EXT)) {
-            /* Taking an external interrupt does not clear the external
-             * interrupt status
-             */
-#if 0
-            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EXT);
-#endif
-            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_EXTERNAL);
-            return;
-        }
         if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
             env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL);
             powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI);
-- 
2.7.4

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

* [Qemu-devel] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (3 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 04/23] ppc: Fix conditions for delivering external interrupts to a guest David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-09  0:43   ` [Qemu-devel] [Qemu-ppc] " Mark Cave-Ayland
  2016-07-01  6:41 ` [Qemu-devel] [PULL 06/23] ppc: Initial HDEC support David Gibson
                   ` (18 subsequent siblings)
  23 siblings, 1 reply; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The architecture specifies that any instruction that sets MSR:PR will also
set MSR:EE, IR and DR.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/helper_regs.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 8fc0934..8fdfa5c 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
         /* Change the exception prefix on PowerPC 601 */
         env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
     }
+    /* If PR=1 then EE, IR and DR must be 1 */
+    if ((value >> MSR_PR) & 1) {
+        value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
+    }
 #endif
     env->msr = value;
     hreg_compute_hflags(env);
-- 
2.7.4

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

* [Qemu-devel] [PULL 06/23] ppc: Initial HDEC support
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (4 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 07/23] ppc: LPCR is a HV resource David Gibson
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The current behaviour isn't completely right, as for the DEC, we
don't properly re-arm when wrapping around, but I will fix this
in a separate patch.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/ppc.c                 | 17 ++++++++++++-----
 target-ppc/excp_helper.c     | 22 ++++++++++++----------
 target-ppc/helper.h          |  2 ++
 target-ppc/timebase_helper.c | 10 ++++++++++
 target-ppc/translate_init.c  | 30 ++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 1bcf740..e425252 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -699,9 +699,18 @@ static inline void cpu_ppc_decr_lower(PowerPCCPU *cpu)
 
 static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
 {
+    CPUPPCState *env = &cpu->env;
+
     /* Raise it */
-    LOG_TB("raise decrementer exception\n");
-    ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 1);
+    LOG_TB("raise hv decrementer exception\n");
+
+    /* The architecture specifies that we don't deliver HDEC
+     * interrupts in a PM state. Not only they don't cause a
+     * wakeup but they also get effectively discarded.
+     */
+    if (!env->in_pm_state) {
+        ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 1);
+    }
 }
 
 static inline void cpu_ppc_hdecr_lower(PowerPCCPU *cpu)
@@ -928,9 +937,7 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq)
     }
     /* Create new timer */
     tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_ppc_decr_cb, cpu);
-    if (0) {
-        /* XXX: find a suitable condition to enable the hypervisor decrementer
-         */
+    if (env->has_hv_mode) {
         tb_env->hdecr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_ppc_hdecr_cb,
                                                 cpu);
     } else {
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 26adda4..d6e1678 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -753,7 +753,6 @@ void ppc_cpu_do_interrupt(CPUState *cs)
 static void ppc_hw_interrupt(CPUPPCState *env)
 {
     PowerPCCPU *cpu = ppc_env_get_cpu(env);
-    int hdice;
 #if 0
     CPUState *cs = CPU(cpu);
 
@@ -781,15 +780,13 @@ static void ppc_hw_interrupt(CPUPPCState *env)
         return;
     }
 #endif
-    if (0) {
-        /* XXX: find a suitable condition to enable the hypervisor mode */
-        hdice = env->spr[SPR_LPCR] & 1;
-    } else {
-        hdice = 0;
-    }
-    if ((msr_ee != 0 || msr_hv == 0 || msr_pr != 0) && hdice != 0) {
-        /* Hypervisor decrementer exception */
-        if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
+    /* Hypervisor decrementer exception */
+    if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
+        /* LPCR will be clear when not supported so this will work */
+        bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
+        if ((msr_ee != 0 || msr_hv == 0) && hdice) {
+            /* HDEC clears on delivery */
+            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
             powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_HDECR);
             return;
         }
@@ -941,6 +938,11 @@ void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
     cs->halted = 1;
     env->in_pm_state = true;
 
+    /* The architecture specifies that HDEC interrupts are
+     * discarded in PM states
+     */
+    env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
+
     /* Technically, nap doesn't set EE, but if we don't set it
      * then ppc_hw_interrupt() won't deliver. We could add some
      * other tests there based on LPCR but it's simpler to just
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index c532b44..1f5cfd0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -600,6 +600,8 @@ DEF_HELPER_2(store_601_rtcl, void, env, tl)
 DEF_HELPER_2(store_601_rtcu, void, env, tl)
 DEF_HELPER_1(load_decr, tl, env)
 DEF_HELPER_2(store_decr, void, env, tl)
+DEF_HELPER_1(load_hdecr, tl, env)
+DEF_HELPER_2(store_hdecr, void, env, tl)
 DEF_HELPER_2(store_hid0_601, void, env, tl)
 DEF_HELPER_3(store_403_pbr, void, env, i32, tl)
 DEF_HELPER_1(load_40x_pit, tl, env)
diff --git a/target-ppc/timebase_helper.c b/target-ppc/timebase_helper.c
index 66de313..a07faa4 100644
--- a/target-ppc/timebase_helper.c
+++ b/target-ppc/timebase_helper.c
@@ -102,6 +102,16 @@ void helper_store_decr(CPUPPCState *env, target_ulong val)
     cpu_ppc_store_decr(env, val);
 }
 
+target_ulong helper_load_hdecr(CPUPPCState *env)
+{
+    return cpu_ppc_load_hdecr(env);
+}
+
+void helper_store_hdecr(CPUPPCState *env, target_ulong val)
+{
+    cpu_ppc_store_hdecr(env, val);
+}
+
 target_ulong helper_load_40x_pit(CPUPPCState *env)
 {
     return load_40x_pit(env);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index af7a790..a2d9ff2 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -277,6 +277,32 @@ static void spr_read_purr (DisasContext *ctx, int gprn, int sprn)
 {
     gen_helper_load_purr(cpu_gpr[gprn], cpu_env);
 }
+
+/* HDECR */
+static void spr_read_hdecr(DisasContext *ctx, int gprn, int sprn)
+{
+    if (ctx->tb->cflags & CF_USE_ICOUNT) {
+        gen_io_start();
+    }
+    gen_helper_load_hdecr(cpu_gpr[gprn], cpu_env);
+    if (ctx->tb->cflags & CF_USE_ICOUNT) {
+        gen_io_end();
+        gen_stop_exception(ctx);
+    }
+}
+
+static void spr_write_hdecr(DisasContext *ctx, int sprn, int gprn)
+{
+    if (ctx->tb->cflags & CF_USE_ICOUNT) {
+        gen_io_start();
+    }
+    gen_helper_store_hdecr(cpu_env, cpu_gpr[gprn]);
+    if (ctx->tb->cflags & CF_USE_ICOUNT) {
+        gen_io_end();
+        gen_stop_exception(ctx);
+    }
+}
+
 #endif
 #endif
 
@@ -7824,6 +7850,10 @@ static void gen_spr_power5p_lpar(CPUPPCState *env)
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_lpcr,
                      KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
+    spr_register_hv(env, SPR_HDEC, "HDEC",
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    &spr_read_hdecr, &spr_write_hdecr, 0);
 #endif
 }
 
-- 
2.7.4

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

* [Qemu-devel] [PULL 07/23] ppc: LPCR is a HV resource
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (5 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 06/23] ppc: Initial HDEC support David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 08/23] ppc: Print HSRR0/HSRR1 in "info registers" David Gibson
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Don't allow access in guest mode

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate_init.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a2d9ff2..55d1bfa 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7846,10 +7846,11 @@ static void gen_spr_power5p_lpar(CPUPPCState *env)
 {
 #if !defined(CONFIG_USER_ONLY)
     /* Logical partitionning */
-    spr_register_kvm(env, SPR_LPCR, "LPCR",
-                     SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_lpcr,
-                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
+    spr_register_kvm_hv(env, SPR_LPCR, "LPCR",
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        &spr_read_generic, &spr_write_lpcr,
+                        KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
     spr_register_hv(env, SPR_HDEC, "HDEC",
                     SPR_NOACCESS, SPR_NOACCESS,
                     SPR_NOACCESS, SPR_NOACCESS,
-- 
2.7.4

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

* [Qemu-devel] [PULL 08/23] ppc: Print HSRR0/HSRR1 in "info registers"
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (6 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 07/23] ppc: LPCR is a HV resource David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 09/23] hw/ppc/spapr: Add some missing hcall function set strings David Gibson
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

They are generally useful when debugging HV mode stuff

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 2f1c591..49fe761 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11407,6 +11407,13 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                 env->spr[SPR_SPRG4], env->spr[SPR_SPRG5],
                 env->spr[SPR_SPRG6], env->spr[SPR_SPRG7]);
 
+#if defined(TARGET_PPC64)
+    if (env->excp_model == POWERPC_EXCP_POWER7 ||
+        env->excp_model == POWERPC_EXCP_POWER8) {
+        cpu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
+                    env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
+    }
+#endif
     if (env->excp_model == POWERPC_EXCP_BOOKE) {
         cpu_fprintf(f, "CSRR0 " TARGET_FMT_lx " CSRR1 " TARGET_FMT_lx
                        " MCSRR0 " TARGET_FMT_lx " MCSRR1 " TARGET_FMT_lx "\n",
-- 
2.7.4

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

* [Qemu-devel] [PULL 09/23] hw/ppc/spapr: Add some missing hcall function set strings
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (7 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 08/23] ppc: Print HSRR0/HSRR1 in "info registers" David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 10/23] spapr: fix write-past-end-of-array error in cpu core device init code David Gibson
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Thomas Huth, David Gibson

From: Thomas Huth <thuth@redhat.com>

Add "hcall-sprg0" (for H_SET_SPRG0), "hcall-copy" (for H_PAGE_INIT)
and "hcall-debug" (for H_LOGICAL_CI_LOAD/STORE) to the property
"ibm,hypertas-functions" to indicate that we support these hypercalls.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0b6bb9c..d26b4c2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -339,6 +339,9 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     add_str(hypertas, "hcall-splpar");
     add_str(hypertas, "hcall-bulk");
     add_str(hypertas, "hcall-set-mode");
+    add_str(hypertas, "hcall-sprg0");
+    add_str(hypertas, "hcall-copy");
+    add_str(hypertas, "hcall-debug");
     add_str(qemu_hypertas, "hcall-memop1");
 
     fdt = g_malloc0(FDT_MAX_SIZE);
-- 
2.7.4

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

* [Qemu-devel] [PULL 10/23] spapr: fix write-past-end-of-array error in cpu core device init code
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (8 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 09/23] hw/ppc/spapr: Add some missing hcall function set strings David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 11/23] spapr: Restore support for older PowerPC CPU cores David Gibson
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, benh, qemu-devel, qemu-ppc, Greg Kurz, David Gibson

From: Greg Kurz <groug@kaod.org>

This fixes a potential QEMU crash introduced by commit 3b542549661.

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

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 3a5da09..8b802a6 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -309,10 +309,9 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
     }
 
 err:
-    while (i >= 0) {
+    while (--i >= 0) {
         obj = sc->threads + i * size;
         object_unparent(obj);
-        i--;
     }
     g_free(sc->threads);
     error_propagate(errp, local_err);
-- 
2.7.4

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

* [Qemu-devel] [PULL 11/23] spapr: Restore support for older PowerPC CPU cores
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (9 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 10/23] spapr: fix write-past-end-of-array error in cpu core device init code David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 12/23] target-ppc: Eliminate redundant and incorrect function booke206_page_size_to_tlb David Gibson
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Bharata B Rao, David Gibson

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

Introduction of core based CPU hotplug for PowerPC sPAPR didn't
add support for 970 and POWER5+ based core types. Add support for
the same.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_cpu_core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 8b802a6..cebeef5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -325,7 +325,6 @@ static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
 
 /*
  * instance_init routines from different flavours of sPAPR CPU cores.
- * TODO: Add support for 'host' core type.
  */
 #define SPAPR_CPU_CORE_INITFN(_type, _fname) \
 static void glue(glue(spapr_cpu_core_, _fname), _initfn(Object *obj)) \
@@ -338,6 +337,8 @@ static void glue(glue(spapr_cpu_core_, _fname), _initfn(Object *obj)) \
     core->cpu_class = oc; \
 }
 
+SPAPR_CPU_CORE_INITFN(970_v2.2, 970);
+SPAPR_CPU_CORE_INITFN(POWER5+_v2.1, POWER5plus);
 SPAPR_CPU_CORE_INITFN(POWER7_v2.3, POWER7);
 SPAPR_CPU_CORE_INITFN(POWER7+_v2.1, POWER7plus);
 SPAPR_CPU_CORE_INITFN(POWER8_v2.0, POWER8);
@@ -349,6 +350,12 @@ typedef struct SPAPRCoreInfo {
 } SPAPRCoreInfo;
 
 static const SPAPRCoreInfo spapr_cores[] = {
+    /* 970 */
+    { .name = "970", .initfn = spapr_cpu_core_970_initfn },
+
+    /* POWER5 */
+    { .name = "POWER5+", .initfn = spapr_cpu_core_POWER5plus_initfn },
+
     /* POWER7 and aliases */
     { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn },
     { .name = "POWER7", .initfn = spapr_cpu_core_POWER7_initfn },
-- 
2.7.4

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

* [Qemu-devel] [PULL 12/23] target-ppc: Eliminate redundant and incorrect function booke206_page_size_to_tlb
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (10 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 11/23] spapr: Restore support for older PowerPC CPU cores David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 13/23] ppc: Fix 64K pages support in full emulation David Gibson
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Aaron Larson, David Gibson

From: Aaron Larson <alarson@ddci.com>

Eliminate redundant and incorrect booke206_page_size_to_tlb function
from ppce500_spin.c in preference to previously existing but newly
exported definition from e500.c

Defect analysis:

The booke206_page_size_to_tlb function in e500.c was updated in commit
2bd9543 "ppc: booke206: use MAV=2.0 TSIZE definition, fix 4G pages" to
reflect a change in the definition of MAS1_TSIZE_SHIFT from 8
(corresponding to a min TLB page size of 4kb) to a value of 7 (TLB
page size 2k).  The booke206_page_size_to_tlb() function defined in
ppce500_spin.c was never updated to reflect the change in
MAS1_TSIZE_SHIFT.

In http://lists.nongnu.org/archive/html/qemu-ppc/2016-06/msg00533.html,
Scott Wood suggested this "root cause" explanation:

SW> The patch that changed MAS1_TSIZE_SHIFT from 8 to 7 was around the
SW> same time as the patch that added this code, which is probably why
SW> adjusting it got missed.  Commit 2bd9543cd3 did update the
SW> equivalent code in ppce500_mpc8544ds.c, which now resides in
SW> hw/ppc/e500.c and has been changed to not assume a power-of-2
SW> size.  The ppce500_spin version should be eliminated.

Signed-off-by: Aaron Larson <alarson@ddci.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/e500.c         | 2 +-
 hw/ppc/e500.h         | 2 ++
 hw/ppc/ppce500_spin.c | 7 +------
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index ee1c60b..0cd534d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -601,7 +601,7 @@ static int ppce500_prep_device_tree(MachineState *machine,
 }
 
 /* Create -kernel TLB entries for BookE.  */
-static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
+hwaddr booke206_page_size_to_tlb(uint64_t size)
 {
     return 63 - clz64(size >> 10);
 }
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index ef224ea..70ba1d8 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -26,4 +26,6 @@ typedef struct PPCE500Params {
 
 void ppce500_init(MachineState *machine, PPCE500Params *params);
 
+hwaddr booke206_page_size_to_tlb(uint64_t size);
+
 #endif
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index 225177b..22c584e 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -32,6 +32,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
+#include "e500.h"
 
 #define MAX_CPUS 32
 
@@ -72,12 +73,6 @@ static void spin_reset(void *opaque)
     }
 }
 
-/* Create -kernel TLB entries for BookE, linearly spanning 256MB.  */
-static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
-{
-    return ctz32(size >> 10) >> 1;
-}
-
 static void mmubooke_create_initial_mapping(CPUPPCState *env,
                                      target_ulong va,
                                      hwaddr pa,
-- 
2.7.4

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

* [Qemu-devel] [PULL 13/23] ppc: Fix 64K pages support in full emulation
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (11 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 12/23] target-ppc: Eliminate redundant and incorrect function booke206_page_size_to_tlb David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 14/23] ppc/xics: Rename existing xics to xics_spapr David Gibson
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We were always advertising only 4K & 16M. Additionally the code wasn't
properly matching the page size with the PTE content, which meant we
could potentially hit an incorrect PTE if the guest used multiple sizes.

Finally, honor the CPU capabilities when decoding the size from the SLB
so we don't try to use 64K pages on 970.

This still doesn't add support for MPSS (Multiple Page Sizes per Segment)

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors
      commits 61a36c9b5a12 and 1114e712c998 reworked the hpte code
      doing insertion/removal in hw/ppc/spapr_hcall.c. The hunks
      modifying these areas were removed. ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu-qom.h        |  3 +++
 target-ppc/mmu-hash64.c     | 39 +++++++++++++++++++++++++++++++++++----
 target-ppc/translate_init.c | 22 +++++++++++++++++++---
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 0fad2de..2864105 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -70,18 +70,21 @@ enum powerpc_mmu_t {
 #define POWERPC_MMU_64       0x00010000
 #define POWERPC_MMU_1TSEG    0x00020000
 #define POWERPC_MMU_AMR      0x00040000
+#define POWERPC_MMU_64K      0x00080000
     /* 64 bits PowerPC MMU                                     */
     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
     /* Architecture 2.03 and later (has LPCR) */
     POWERPC_MMU_2_03       = POWERPC_MMU_64 | 0x00000002,
     /* Architecture 2.06 variant                               */
     POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
+                             | POWERPC_MMU_64K
                              | POWERPC_MMU_AMR | 0x00000003,
     /* Architecture 2.06 "degraded" (no 1T segments)           */
     POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
                              | 0x00000003,
     /* Architecture 2.07 variant                               */
     POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
+                             | POWERPC_MMU_64K
                              | POWERPC_MMU_AMR | 0x00000004,
     /* Architecture 2.07 "degraded" (no 1T segments)           */
     POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 6d6f26c..3b1357a 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -450,9 +450,31 @@ void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
     }
 }
 
+/* Returns the effective page shift or 0. MPSS isn't supported yet so
+ * this will always be the slb_pshift or 0
+ */
+static uint32_t ppc_hash64_pte_size_decode(uint64_t pte1, uint32_t slb_pshift)
+{
+    switch (slb_pshift) {
+    case 12:
+        return 12;
+    case 16:
+        if ((pte1 & 0xf000) == 0x1000) {
+            return 16;
+        }
+        return 0;
+    case 24:
+        if ((pte1 & 0xff000) == 0) {
+            return 24;
+        }
+        return 0;
+    }
+    return 0;
+}
+
 static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
-                                     bool secondary, target_ulong ptem,
-                                     ppc_hash_pte64_t *pte)
+                                     uint32_t slb_pshift, bool secondary,
+                                     target_ulong ptem, ppc_hash_pte64_t *pte)
 {
     CPUPPCState *env = &cpu->env;
     int i;
@@ -472,6 +494,13 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
         if ((pte0 & HPTE64_V_VALID)
             && (secondary == !!(pte0 & HPTE64_V_SECONDARY))
             && HPTE64_V_COMPARE(pte0, ptem)) {
+            uint32_t pshift = ppc_hash64_pte_size_decode(pte1, slb_pshift);
+            if (pshift == 0) {
+                continue;
+            }
+            /* We don't do anything with pshift yet as qemu TLB only deals
+             * with 4K pages anyway
+             */
             pte->pte0 = pte0;
             pte->pte1 = pte1;
             ppc_hash64_stop_access(cpu, token);
@@ -525,7 +554,8 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
             " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
             " hash=" TARGET_FMT_plx "\n",
             env->htab_base, env->htab_mask, vsid, ptem,  hash);
-    pte_offset = ppc_hash64_pteg_search(cpu, hash, 0, ptem, pte);
+    pte_offset = ppc_hash64_pteg_search(cpu, hash, slb->sps->page_shift,
+                                        0, ptem, pte);
 
     if (pte_offset == -1) {
         /* Secondary PTEG lookup */
@@ -535,7 +565,8 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
                 " hash=" TARGET_FMT_plx "\n", env->htab_base,
                 env->htab_mask, vsid, ptem, ~hash);
 
-        pte_offset = ppc_hash64_pteg_search(cpu, ~hash, 1, ptem, pte);
+        pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb->sps->page_shift, 1,
+                                            ptem, pte);
     }
 
     return pte_offset;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 55d1bfa..843f19b 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10293,8 +10293,8 @@ static void ppc_cpu_initfn(Object *obj)
     if (pcc->sps) {
         env->sps = *pcc->sps;
     } else if (env->mmu_model & POWERPC_MMU_64) {
-        /* Use default sets of page sizes */
-        static const struct ppc_segment_page_sizes defsps = {
+        /* Use default sets of page sizes. We don't support MPSS */
+        static const struct ppc_segment_page_sizes defsps_4k = {
             .sps = {
                 { .page_shift = 12, /* 4K */
                   .slb_enc = 0,
@@ -10306,7 +10306,23 @@ static void ppc_cpu_initfn(Object *obj)
                 },
             },
         };
-        env->sps = defsps;
+        static const struct ppc_segment_page_sizes defsps_64k = {
+            .sps = {
+                { .page_shift = 12, /* 4K */
+                  .slb_enc = 0,
+                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
+                },
+                { .page_shift = 16, /* 64K */
+                  .slb_enc = 0x110,
+                  .enc = { { .page_shift = 16, .pte_enc = 1 } }
+                },
+                { .page_shift = 24, /* 16M */
+                  .slb_enc = 0x100,
+                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
+                },
+            },
+        };
+        env->sps = (env->mmu_model & POWERPC_MMU_64K) ? defsps_64k : defsps_4k;
     }
 #endif /* defined(TARGET_PPC64) */
 
-- 
2.7.4

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

* [Qemu-devel] [PULL 14/23] ppc/xics: Rename existing xics to xics_spapr
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (12 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 13/23] ppc: Fix 64K pages support in full emulation David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 15/23] ppc/xics: Move SPAPR specific code to a separate file David Gibson
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Nikunj A Dadhania, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The common class doesn't change, the KVM one is sPAPR specific. Rename
variables and functions to xics_spapr.

Retain the type name as "xics" to preserve migration for existing sPAPR
guests.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c        | 33 +++++++++++++++++----------------
 hw/intc/xics_kvm.c    | 14 +++++++-------
 hw/ppc/spapr.c        |  7 ++++---
 hw/ppc/spapr_events.c |  2 +-
 hw/ppc/spapr_pci.c    | 10 +++++-----
 hw/ppc/spapr_vio.c    |  2 +-
 include/hw/ppc/xics.h | 30 ++++++++++++++++++------------
 7 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 2e83d41..bf5eb56 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -718,7 +718,8 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
     return -1;
 }
 
-int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp)
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+                     Error **errp)
 {
     ICSState *ics = &icp->ics[src];
     int irq;
@@ -749,8 +750,8 @@ int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp)
  * Allocate block of consecutive IRQs, and return the number of the first IRQ in the block.
  * If align==true, aligns the first IRQ number to num.
  */
-int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align,
-                     Error **errp)
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp)
 {
     int i, first = -1;
     ICSState *ics = &icp->ics[src];
@@ -799,7 +800,7 @@ static void ics_free(ICSState *ics, int srcno, int num)
     }
 }
 
-void xics_free(XICSState *icp, int irq, int num)
+void xics_spapr_free(XICSState *icp, int irq, int num)
 {
     int src = xics_find_source(icp, irq);
 
@@ -1018,9 +1019,9 @@ static void xics_set_nr_servers(XICSState *icp, uint32_t nr_servers,
     }
 }
 
-static void xics_realize(DeviceState *dev, Error **errp)
+static void xics_spapr_realize(DeviceState *dev, Error **errp)
 {
-    XICSState *icp = XICS(dev);
+    XICSState *icp = XICS_SPAPR(dev);
     Error *error = NULL;
     int i;
 
@@ -1057,38 +1058,38 @@ static void xics_realize(DeviceState *dev, Error **errp)
     }
 }
 
-static void xics_initfn(Object *obj)
+static void xics_spapr_initfn(Object *obj)
 {
-    XICSState *xics = XICS(obj);
+    XICSState *xics = XICS_SPAPR(obj);
 
     xics->ics = ICS(object_new(TYPE_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
     xics->ics->icp = xics;
 }
 
-static void xics_class_init(ObjectClass *oc, void *data)
+static void xics_spapr_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    XICSStateClass *xsc = XICS_CLASS(oc);
+    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
 
-    dc->realize = xics_realize;
+    dc->realize = xics_spapr_realize;
     xsc->set_nr_irqs = xics_set_nr_irqs;
     xsc->set_nr_servers = xics_set_nr_servers;
 }
 
-static const TypeInfo xics_info = {
-    .name          = TYPE_XICS,
+static const TypeInfo xics_spapr_info = {
+    .name          = TYPE_XICS_SPAPR,
     .parent        = TYPE_XICS_COMMON,
     .instance_size = sizeof(XICSState),
     .class_size = sizeof(XICSStateClass),
-    .class_init    = xics_class_init,
-    .instance_init = xics_initfn,
+    .class_init    = xics_spapr_class_init,
+    .instance_init = xics_spapr_initfn,
 };
 
 static void xics_register_types(void)
 {
     type_register_static(&xics_common_info);
-    type_register_static(&xics_info);
+    type_register_static(&xics_spapr_info);
     type_register_static(&ics_info);
     type_register_static(&icp_info);
 }
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index b17d6a9..a533e3d 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -145,7 +145,7 @@ static const TypeInfo icp_kvm_info = {
  */
 static void ics_get_kvm_state(ICSState *ics)
 {
-    KVMXICSState *icpkvm = KVM_XICS(ics->icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -204,7 +204,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
 static int ics_set_kvm_state(ICSState *ics, int version_id)
 {
-    KVMXICSState *icpkvm = KVM_XICS(ics->icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -328,7 +328,7 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
 {
     CPUState *cs;
     ICPState *ss;
-    KVMXICSState *icpkvm = KVM_XICS(icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(icp);
 
     cs = CPU(cpu);
     ss = &icp->ss[cs->cpu_index];
@@ -394,7 +394,7 @@ static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
 static void xics_kvm_realize(DeviceState *dev, Error **errp)
 {
-    KVMXICSState *icpkvm = KVM_XICS(dev);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(dev);
     XICSState *icp = XICS_COMMON(dev);
     int i, rc;
     Error *error = NULL;
@@ -495,8 +495,8 @@ static void xics_kvm_class_init(ObjectClass *oc, void *data)
     xsc->set_nr_servers = xics_kvm_set_nr_servers;
 }
 
-static const TypeInfo xics_kvm_info = {
-    .name          = TYPE_KVM_XICS,
+static const TypeInfo xics_spapr_kvm_info = {
+    .name          = TYPE_XICS_SPAPR_KVM,
     .parent        = TYPE_XICS_COMMON,
     .instance_size = sizeof(KVMXICSState),
     .class_init    = xics_kvm_class_init,
@@ -505,7 +505,7 @@ static const TypeInfo xics_kvm_info = {
 
 static void xics_kvm_register_types(void)
 {
-    type_register_static(&xics_kvm_info);
+    type_register_static(&xics_spapr_kvm_info);
     type_register_static(&ics_kvm_info);
     type_register_static(&icp_kvm_info);
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d26b4c2..3f0ea03 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -122,7 +122,8 @@ static XICSState *xics_system_init(MachineState *machine,
         Error *err = NULL;
 
         if (machine_kernel_irqchip_allowed(machine)) {
-            icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs, &err);
+            icp = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
+                                  &err);
         }
         if (machine_kernel_irqchip_required(machine) && !icp) {
             error_reportf_err(err,
@@ -133,7 +134,7 @@ static XICSState *xics_system_init(MachineState *machine,
     }
 
     if (!icp) {
-        icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, errp);
+        icp = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
     }
 
     return icp;
@@ -1784,7 +1785,7 @@ static void ppc_spapr_init(MachineState *machine)
     /* Set up Interrupt Controller before we create the VCPUs */
     spapr->icp = xics_system_init(machine,
                                   DIV_ROUND_UP(max_cpus * smt, smp_threads),
-                                  XICS_IRQS, &error_fatal);
+                                  XICS_IRQS_SPAPR, &error_fatal);
 
     if (smc->dr_lmb_enabled) {
         spapr_validate_node_memory(machine, &error_fatal);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index af80992..0585f8a 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -603,7 +603,7 @@ out_no_events:
 void spapr_events_init(sPAPRMachineState *spapr)
 {
     QTAILQ_INIT(&spapr->pending_events);
-    spapr->check_exception_irq = xics_alloc(spapr->icp, 0, 0, false,
+    spapr->check_exception_irq = xics_spapr_alloc(spapr->icp, 0, 0, false,
                                             &error_fatal);
     spapr->epow_notifier.notify = spapr_powerdown_req;
     qemu_register_powerdown_notifier(&spapr->epow_notifier);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 9f28fb3..451651d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -322,7 +322,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
             return;
         }
 
-        xics_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
         if (msi_present(pdev)) {
             spapr_msi_setmsg(pdev, 0, false, 0, 0);
         }
@@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* Allocate MSIs */
-    irq = xics_alloc_block(spapr->icp, 0, req_num, false,
+    irq = xics_spapr_alloc_block(spapr->icp, 0, req_num, false,
                            ret_intr_type == RTAS_TYPE_MSI, &err);
     if (err) {
         error_reportf_err(err, "Can't allocate MSIs for device %x: ",
@@ -371,7 +371,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     /* Release previous MSIs */
     if (msi) {
-        xics_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
         g_hash_table_remove(phb->msi, &config_addr);
     }
 
@@ -1442,7 +1442,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         uint32_t irq;
         Error *local_err = NULL;
 
-        irq = xics_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
+        irq = xics_spapr_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             error_prepend(errp, "can't allocate LSIs: ");
@@ -1801,7 +1801,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges));
     _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg)));
     _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1));
-    _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS));
+    _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS_SPAPR));
 
     /* Build the interrupt-map, this must matches what is done
      * in pci_spapr_map_irq
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index ae40db8..7ffd23e 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
         dev->qdev.id = id;
     }
 
-    dev->irq = xics_alloc(spapr->icp, 0, dev->irq, false, &local_err);
+    dev->irq = xics_spapr_alloc(spapr->icp, 0, dev->irq, false, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 6925677..c946770 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -32,20 +32,25 @@
 #define TYPE_XICS_COMMON "xics-common"
 #define XICS_COMMON(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS_COMMON)
 
-#define TYPE_XICS "xics"
-#define XICS(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS)
+/*
+ * Retain xics as the type name to be compatible for migration. Rest all the
+ * functions, class and variables are renamed as xics_spapr.
+ */
+#define TYPE_XICS_SPAPR "xics"
+#define XICS_SPAPR(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS_SPAPR)
 
-#define TYPE_KVM_XICS "xics-kvm"
-#define KVM_XICS(obj) OBJECT_CHECK(KVMXICSState, (obj), TYPE_KVM_XICS)
+#define TYPE_XICS_SPAPR_KVM "xics-spapr-kvm"
+#define XICS_SPAPR_KVM(obj) \
+     OBJECT_CHECK(KVMXICSState, (obj), TYPE_XICS_SPAPR_KVM)
 
 #define XICS_COMMON_CLASS(klass) \
      OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_COMMON)
-#define XICS_CLASS(klass) \
-     OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS)
+#define XICS_SPAPR_CLASS(klass) \
+     OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_SPAPR)
 #define XICS_COMMON_GET_CLASS(obj) \
      OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_COMMON)
-#define XICS_GET_CLASS(obj) \
-     OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS)
+#define XICS_SPAPR_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_SPAPR)
 
 #define XICS_IPI        0x2
 #define XICS_BUID       0x1
@@ -157,13 +162,14 @@ struct ICSIRQState {
     uint8_t flags;
 };
 
-#define XICS_IRQS               1024
+#define XICS_IRQS_SPAPR               1024
 
 qemu_irq xics_get_qirq(XICSState *icp, int irq);
-int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp);
-int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align,
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
                      Error **errp);
-void xics_free(XICSState *icp, int irq, int num);
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp);
+void xics_spapr_free(XICSState *icp, int irq, int num);
 
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
-- 
2.7.4

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

* [Qemu-devel] [PULL 15/23] ppc/xics: Move SPAPR specific code to a separate file
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (13 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 14/23] ppc/xics: Rename existing xics to xics_spapr David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 16/23] ppc/xics: Implement H_IPOLL using an accessor David Gibson
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Nikunj A Dadhania, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Leave the core ICP/ICS logic in xics.c and move the top level
class wrapper, hypercall and RTAS handlers to xics_spapr.c

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[add cpu.h in xics_spapr.c, move set_nr_irqs and set_nr_servers to
 xics_spapr.c]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 default-configs/ppc64-softmmu.mak |   1 +
 hw/intc/Makefile.objs             |   1 +
 hw/intc/xics.c                    | 418 +-----------------------------------
 hw/intc/xics_spapr.c              | 432 ++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/xics.h             |  21 ++
 5 files changed, 464 insertions(+), 409 deletions(-)
 create mode 100644 hw/intc/xics_spapr.c

diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index bb71b23..c4be59f 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -49,6 +49,7 @@ CONFIG_ETSEC=y
 CONFIG_LIBDECNUMBER=y
 # For pSeries
 CONFIG_XICS=$(CONFIG_PSERIES)
+CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index c7bbf88..530df2e 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -30,6 +30,7 @@ obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
 obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
 obj-$(CONFIG_SH4) += sh_intc.o
 obj-$(CONFIG_XICS) += xics.o
+obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
 obj-$(CONFIG_XICS_KVM) += xics_kvm.o
 obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
 obj-$(CONFIG_S390_FLIC) += s390_flic.o
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index bf5eb56..f01af08 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -32,12 +32,11 @@
 #include "hw/hw.h"
 #include "trace.h"
 #include "qemu/timer.h"
-#include "hw/ppc/spapr.h"
 #include "hw/ppc/xics.h"
 #include "qemu/error-report.h"
 #include "qapi/visitor.h"
 
-static int get_cpu_index_by_dt_id(int cpu_dt_id)
+int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
 {
     PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
 
@@ -242,7 +241,7 @@ static void icp_resend(XICSState *icp, int server)
     ics_resend(icp->ics);
 }
 
-static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
+void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
 {
     ICPState *ss = icp->ss + server;
     uint8_t old_cppr;
@@ -266,7 +265,7 @@ static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
     }
 }
 
-static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
+void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
 {
     ICPState *ss = icp->ss + server;
 
@@ -276,7 +275,7 @@ static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
     }
 }
 
-static uint32_t icp_accept(ICPState *ss)
+uint32_t icp_accept(ICPState *ss)
 {
     uint32_t xirr = ss->xirr;
 
@@ -289,7 +288,7 @@ static uint32_t icp_accept(ICPState *ss)
     return xirr;
 }
 
-static void icp_eoi(XICSState *icp, int server, uint32_t xirr)
+void icp_eoi(XICSState *icp, int server, uint32_t xirr)
 {
     ICPState *ss = icp->ss + server;
 
@@ -390,12 +389,6 @@ static const TypeInfo icp_info = {
 /*
  * ICS: Source layer
  */
-static int ics_valid_irq(ICSState *ics, uint32_t nr)
-{
-    return (nr >= ics->offset)
-        && (nr < (ics->offset + ics->nr_irqs));
-}
-
 static void resend_msi(ICSState *ics, int srcno)
 {
     ICSIRQState *irq = ics->irqs + srcno;
@@ -480,8 +473,8 @@ static void write_xive_lsi(ICSState *ics, int srcno)
     resend_lsi(ics, srcno);
 }
 
-static void ics_write_xive(ICSState *ics, int nr, int server,
-                           uint8_t priority, uint8_t saved_priority)
+void ics_write_xive(ICSState *ics, int nr, int server,
+                    uint8_t priority, uint8_t saved_priority)
 {
     int srcno = nr - ics->offset;
     ICSIRQState *irq = ics->irqs + srcno;
@@ -658,7 +651,7 @@ static const TypeInfo ics_info = {
 /*
  * Exported functions
  */
-static int xics_find_source(XICSState *icp, int irq)
+int xics_find_source(XICSState *icp, int irq)
 {
     int sources = 1;
     int src;
@@ -686,7 +679,7 @@ qemu_irq xics_get_qirq(XICSState *icp, int irq)
     return NULL;
 }
 
-static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
+void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
 {
     assert(!(ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MASK));
 
@@ -694,402 +687,9 @@ static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
         lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
 }
 
-#define ICS_IRQ_FREE(ics, srcno)   \
-    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
-
-static int ics_find_free_block(ICSState *ics, int num, int alignnum)
-{
-    int first, i;
-
-    for (first = 0; first < ics->nr_irqs; first += alignnum) {
-        if (num > (ics->nr_irqs - first)) {
-            return -1;
-        }
-        for (i = first; i < first + num; ++i) {
-            if (!ICS_IRQ_FREE(ics, i)) {
-                break;
-            }
-        }
-        if (i == (first + num)) {
-            return first;
-        }
-    }
-
-    return -1;
-}
-
-int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
-                     Error **errp)
-{
-    ICSState *ics = &icp->ics[src];
-    int irq;
-
-    if (irq_hint) {
-        assert(src == xics_find_source(icp, irq_hint));
-        if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
-            error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
-            return -1;
-        }
-        irq = irq_hint;
-    } else {
-        irq = ics_find_free_block(ics, 1, 1);
-        if (irq < 0) {
-            error_setg(errp, "can't allocate IRQ: no IRQ left");
-            return -1;
-        }
-        irq += ics->offset;
-    }
-
-    ics_set_irq_type(ics, irq - ics->offset, lsi);
-    trace_xics_alloc(src, irq);
-
-    return irq;
-}
-
-/*
- * Allocate block of consecutive IRQs, and return the number of the first IRQ in the block.
- * If align==true, aligns the first IRQ number to num.
- */
-int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
-                           bool align, Error **errp)
-{
-    int i, first = -1;
-    ICSState *ics = &icp->ics[src];
-
-    assert(src == 0);
-    /*
-     * MSIMesage::data is used for storing VIRQ so
-     * it has to be aligned to num to support multiple
-     * MSI vectors. MSI-X is not affected by this.
-     * The hint is used for the first IRQ, the rest should
-     * be allocated continuously.
-     */
-    if (align) {
-        assert((num == 1) || (num == 2) || (num == 4) ||
-               (num == 8) || (num == 16) || (num == 32));
-        first = ics_find_free_block(ics, num, num);
-    } else {
-        first = ics_find_free_block(ics, num, 1);
-    }
-    if (first < 0) {
-        error_setg(errp, "can't find a free %d-IRQ block", num);
-        return -1;
-    }
-
-    if (first >= 0) {
-        for (i = first; i < first + num; ++i) {
-            ics_set_irq_type(ics, i, lsi);
-        }
-    }
-    first += ics->offset;
-
-    trace_xics_alloc_block(src, first, num, lsi, align);
-
-    return first;
-}
-
-static void ics_free(ICSState *ics, int srcno, int num)
-{
-    int i;
-
-    for (i = srcno; i < srcno + num; ++i) {
-        if (ICS_IRQ_FREE(ics, i)) {
-            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
-        }
-        memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
-    }
-}
-
-void xics_spapr_free(XICSState *icp, int irq, int num)
-{
-    int src = xics_find_source(icp, irq);
-
-    if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
-
-        /* FIXME: implement multiple sources */
-        assert(src == 0);
-
-        trace_xics_ics_free(ics - icp->ics, irq, num);
-        ics_free(ics, irq - ics->offset, num);
-    }
-}
-
-/*
- * Guest interfaces
- */
-
-static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                           target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    target_ulong cppr = args[0];
-
-    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          target_ulong opcode, target_ulong *args)
-{
-    target_ulong server = get_cpu_index_by_dt_id(args[0]);
-    target_ulong mfrr = args[1];
-
-    if (server >= spapr->icp->nr_servers) {
-        return H_PARAMETER;
-    }
-
-    icp_set_mfrr(spapr->icp, server, mfrr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                           target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
-
-    args[0] = xirr;
-    return H_SUCCESS;
-}
-
-static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                             target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
-    uint32_t xirr = icp_accept(ss);
-
-    args[0] = xirr;
-    args[1] = cpu_get_host_ticks();
-    return H_SUCCESS;
-}
-
-static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    target_ulong xirr = args[0];
-
-    icp_eoi(spapr->icp, cs->cpu_index, xirr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                            target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
-
-    args[0] = ss->xirr;
-    args[1] = ss->mfrr;
-
-    return H_SUCCESS;
-}
-
-static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          uint32_t token,
-                          uint32_t nargs, target_ulong args,
-                          uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr, server, priority;
-
-    if ((nargs != 3) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-    server = get_cpu_index_by_dt_id(rtas_ld(args, 1));
-    priority = rtas_ld(args, 2);
-
-    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
-        || (priority > 0xff)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, server, priority, priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          uint32_t token,
-                          uint32_t nargs, target_ulong args,
-                          uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 3)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
-    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
-}
-
-static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                         uint32_t token,
-                         uint32_t nargs, target_ulong args,
-                         uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
-                   ics->irqs[nr - ics->offset].priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                        uint32_t token,
-                        uint32_t nargs, target_ulong args,
-                        uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
-                   ics->irqs[nr - ics->offset].saved_priority,
-                   ics->irqs[nr - ics->offset].saved_priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-/*
- * XICS
- */
-
-static void xics_set_nr_irqs(XICSState *icp, uint32_t nr_irqs, Error **errp)
-{
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
-}
-
-static void xics_set_nr_servers(XICSState *icp, uint32_t nr_servers,
-                                Error **errp)
-{
-    int i;
-
-    icp->nr_servers = nr_servers;
-
-    icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
-        char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
-        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
-                                  errp);
-    }
-}
-
-static void xics_spapr_realize(DeviceState *dev, Error **errp)
-{
-    XICSState *icp = XICS_SPAPR(dev);
-    Error *error = NULL;
-    int i;
-
-    if (!icp->nr_servers) {
-        error_setg(errp, "Number of servers needs to be greater 0");
-        return;
-    }
-
-    /* Registration of global state belongs into realize */
-    spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
-    spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
-    spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off);
-    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_int_on);
-
-    spapr_register_hypercall(H_CPPR, h_cppr);
-    spapr_register_hypercall(H_IPI, h_ipi);
-    spapr_register_hypercall(H_XIRR, h_xirr);
-    spapr_register_hypercall(H_XIRR_X, h_xirr_x);
-    spapr_register_hypercall(H_EOI, h_eoi);
-    spapr_register_hypercall(H_IPOLL, h_ipoll);
-
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
-        if (error) {
-            error_propagate(errp, error);
-            return;
-        }
-    }
-}
-
-static void xics_spapr_initfn(Object *obj)
-{
-    XICSState *xics = XICS_SPAPR(obj);
-
-    xics->ics = ICS(object_new(TYPE_ICS));
-    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
-}
-
-static void xics_spapr_class_init(ObjectClass *oc, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(oc);
-    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
-
-    dc->realize = xics_spapr_realize;
-    xsc->set_nr_irqs = xics_set_nr_irqs;
-    xsc->set_nr_servers = xics_set_nr_servers;
-}
-
-static const TypeInfo xics_spapr_info = {
-    .name          = TYPE_XICS_SPAPR,
-    .parent        = TYPE_XICS_COMMON,
-    .instance_size = sizeof(XICSState),
-    .class_size = sizeof(XICSStateClass),
-    .class_init    = xics_spapr_class_init,
-    .instance_init = xics_spapr_initfn,
-};
-
 static void xics_register_types(void)
 {
     type_register_static(&xics_common_info);
-    type_register_static(&xics_spapr_info);
     type_register_static(&ics_info);
     type_register_static(&icp_info);
 }
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
new file mode 100644
index 0000000..89190ef
--- /dev/null
+++ b/hw/intc/xics_spapr.c
@@ -0,0 +1,432 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
+ *
+ * PAPR Virtualized Interrupt System, aka ICS/ICP aka xics
+ *
+ * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/hw.h"
+#include "trace.h"
+#include "qemu/timer.h"
+#include "hw/ppc/spapr.h"
+#include "hw/ppc/xics.h"
+#include "qapi/visitor.h"
+#include "qapi/error.h"
+
+/*
+ * Guest interfaces
+ */
+
+static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    target_ulong cppr = args[0];
+
+    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
+    return H_SUCCESS;
+}
+
+static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          target_ulong opcode, target_ulong *args)
+{
+    target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
+    target_ulong mfrr = args[1];
+
+    if (server >= spapr->icp->nr_servers) {
+        return H_PARAMETER;
+    }
+
+    icp_set_mfrr(spapr->icp, server, mfrr);
+    return H_SUCCESS;
+}
+
+static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
+
+    args[0] = xirr;
+    return H_SUCCESS;
+}
+
+static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                             target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    uint32_t xirr = icp_accept(ss);
+
+    args[0] = xirr;
+    args[1] = cpu_get_host_ticks();
+    return H_SUCCESS;
+}
+
+static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    target_ulong xirr = args[0];
+
+    icp_eoi(spapr->icp, cs->cpu_index, xirr);
+    return H_SUCCESS;
+}
+
+static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                            target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+
+    args[0] = ss->xirr;
+    args[1] = ss->mfrr;
+
+    return H_SUCCESS;
+}
+
+static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          uint32_t token,
+                          uint32_t nargs, target_ulong args,
+                          uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr, server, priority;
+
+    if ((nargs != 3) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+    server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
+    priority = rtas_ld(args, 2);
+
+    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
+        || (priority > 0xff)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, server, priority, priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          uint32_t token,
+                          uint32_t nargs, target_ulong args,
+                          uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 3)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
+    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
+}
+
+static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                         uint32_t token,
+                         uint32_t nargs, target_ulong args,
+                         uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
+                   ics->irqs[nr - ics->offset].priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                        uint32_t token,
+                        uint32_t nargs, target_ulong args,
+                        uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
+                   ics->irqs[nr - ics->offset].saved_priority,
+                   ics->irqs[nr - ics->offset].saved_priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void xics_spapr_set_nr_irqs(XICSState *icp, uint32_t nr_irqs,
+                                   Error **errp)
+{
+    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+}
+
+static void xics_spapr_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+                                      Error **errp)
+{
+    int i;
+
+    icp->nr_servers = nr_servers;
+
+    icp->ss = g_malloc0(icp->nr_servers * sizeof(ICPState));
+    for (i = 0; i < icp->nr_servers; i++) {
+        char buffer[32];
+        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
+        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
+        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+                                  errp);
+    }
+}
+
+static void xics_spapr_realize(DeviceState *dev, Error **errp)
+{
+    XICSState *icp = XICS_SPAPR(dev);
+    Error *error = NULL;
+    int i;
+
+    if (!icp->nr_servers) {
+        error_setg(errp, "Number of servers needs to be greater 0");
+        return;
+    }
+
+    /* Registration of global state belongs into realize */
+    spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
+    spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
+    spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off);
+    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_int_on);
+
+    spapr_register_hypercall(H_CPPR, h_cppr);
+    spapr_register_hypercall(H_IPI, h_ipi);
+    spapr_register_hypercall(H_XIRR, h_xirr);
+    spapr_register_hypercall(H_XIRR_X, h_xirr_x);
+    spapr_register_hypercall(H_EOI, h_eoi);
+    spapr_register_hypercall(H_IPOLL, h_ipoll);
+
+    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+
+    for (i = 0; i < icp->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+        if (error) {
+            error_propagate(errp, error);
+            return;
+        }
+    }
+}
+
+static void xics_spapr_initfn(Object *obj)
+{
+    XICSState *xics = XICS_SPAPR(obj);
+
+    xics->ics = ICS(object_new(TYPE_ICS));
+    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
+    xics->ics->icp = xics;
+}
+
+static void xics_spapr_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
+
+    dc->realize = xics_spapr_realize;
+    xsc->set_nr_irqs = xics_spapr_set_nr_irqs;
+    xsc->set_nr_servers = xics_spapr_set_nr_servers;
+}
+
+static const TypeInfo xics_spapr_info = {
+    .name          = TYPE_XICS_SPAPR,
+    .parent        = TYPE_XICS_COMMON,
+    .instance_size = sizeof(XICSState),
+    .class_size = sizeof(XICSStateClass),
+    .class_init    = xics_spapr_class_init,
+    .instance_init = xics_spapr_initfn,
+};
+
+#define ICS_IRQ_FREE(ics, srcno)   \
+    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
+
+static int ics_find_free_block(ICSState *ics, int num, int alignnum)
+{
+    int first, i;
+
+    for (first = 0; first < ics->nr_irqs; first += alignnum) {
+        if (num > (ics->nr_irqs - first)) {
+            return -1;
+        }
+        for (i = first; i < first + num; ++i) {
+            if (!ICS_IRQ_FREE(ics, i)) {
+                break;
+            }
+        }
+        if (i == (first + num)) {
+            return first;
+        }
+    }
+
+    return -1;
+}
+
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+                     Error **errp)
+{
+    ICSState *ics = &icp->ics[src];
+    int irq;
+
+    if (irq_hint) {
+        assert(src == xics_find_source(icp, irq_hint));
+        if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
+            error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
+            return -1;
+        }
+        irq = irq_hint;
+    } else {
+        irq = ics_find_free_block(ics, 1, 1);
+        if (irq < 0) {
+            error_setg(errp, "can't allocate IRQ: no IRQ left");
+            return -1;
+        }
+        irq += ics->offset;
+    }
+
+    ics_set_irq_type(ics, irq - ics->offset, lsi);
+    trace_xics_alloc(src, irq);
+
+    return irq;
+}
+
+/*
+ * Allocate block of consecutive IRQs, and return the number of the first IRQ in
+ * the block. If align==true, aligns the first IRQ number to num.
+ */
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp)
+{
+    int i, first = -1;
+    ICSState *ics = &icp->ics[src];
+
+    assert(src == 0);
+    /*
+     * MSIMesage::data is used for storing VIRQ so
+     * it has to be aligned to num to support multiple
+     * MSI vectors. MSI-X is not affected by this.
+     * The hint is used for the first IRQ, the rest should
+     * be allocated continuously.
+     */
+    if (align) {
+        assert((num == 1) || (num == 2) || (num == 4) ||
+               (num == 8) || (num == 16) || (num == 32));
+        first = ics_find_free_block(ics, num, num);
+    } else {
+        first = ics_find_free_block(ics, num, 1);
+    }
+    if (first < 0) {
+        error_setg(errp, "can't find a free %d-IRQ block", num);
+        return -1;
+    }
+
+    if (first >= 0) {
+        for (i = first; i < first + num; ++i) {
+            ics_set_irq_type(ics, i, lsi);
+        }
+    }
+    first += ics->offset;
+
+    trace_xics_alloc_block(src, first, num, lsi, align);
+
+    return first;
+}
+
+static void ics_free(ICSState *ics, int srcno, int num)
+{
+    int i;
+
+    for (i = srcno; i < srcno + num; ++i) {
+        if (ICS_IRQ_FREE(ics, i)) {
+            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
+        }
+        memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
+    }
+}
+
+void xics_spapr_free(XICSState *icp, int irq, int num)
+{
+    int src = xics_find_source(icp, irq);
+
+    if (src >= 0) {
+        ICSState *ics = &icp->ics[src];
+
+        /* FIXME: implement multiple sources */
+        assert(src == 0);
+
+        trace_xics_ics_free(ics - icp->ics, irq, num);
+        ics_free(ics, irq - ics->offset, num);
+    }
+}
+
+static void xics_spapr_register_types(void)
+{
+    type_register_static(&xics_spapr_info);
+}
+
+type_init(xics_spapr_register_types)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index c946770..3349006 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -146,6 +146,12 @@ struct ICSState {
     XICSState *icp;
 };
 
+static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
+{
+    return (nr >= ics->offset)
+        && (nr < (ics->offset + ics->nr_irqs));
+}
+
 struct ICSIRQState {
     uint32_t server;
     uint8_t priority;
@@ -174,4 +180,19 @@ void xics_spapr_free(XICSState *icp, int irq, int num);
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
 
+/* Internal XICS interfaces */
+int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
+
+void icp_set_cppr(XICSState *icp, int server, uint8_t cppr);
+void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr);
+uint32_t icp_accept(ICPState *ss);
+void icp_eoi(XICSState *icp, int server, uint32_t xirr);
+
+void ics_write_xive(ICSState *ics, int nr, int server,
+                    uint8_t priority, uint8_t saved_priority);
+
+void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
+
+int xics_find_source(XICSState *icp, int irq);
+
 #endif /* __XICS_H__ */
-- 
2.7.4

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

* [Qemu-devel] [PULL 16/23] ppc/xics: Implement H_IPOLL using an accessor
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (14 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 15/23] ppc/xics: Move SPAPR specific code to a separate file David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 17/23] ppc/xics: Replace "icp" with "xics" in most places David Gibson
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Nikunj A Dadhania, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

None of the other presenter functions directly mucks with the
internal state, so don't do it there either.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c        | 8 ++++++++
 hw/intc/xics_spapr.c  | 7 ++++---
 include/hw/ppc/xics.h | 1 +
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index f01af08..f43f98a 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -288,6 +288,14 @@ uint32_t icp_accept(ICPState *ss)
     return xirr;
 }
 
+uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
+{
+    if (mfrr) {
+        *mfrr = ss->mfrr;
+    }
+    return ss->xirr;
+}
+
 void icp_eoi(XICSState *icp, int server, uint32_t xirr)
 {
     ICPState *ss = icp->ss + server;
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 89190ef..94571dd 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -99,10 +99,11 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                             target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    uint32_t mfrr;
+    uint32_t xirr = icp_ipoll(spapr->icp->ss + cs->cpu_index, &mfrr);
 
-    args[0] = ss->xirr;
-    args[1] = ss->mfrr;
+    args[0] = xirr;
+    args[1] = mfrr;
 
     return H_SUCCESS;
 }
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 3349006..7445a14 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -186,6 +186,7 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
 void icp_set_cppr(XICSState *icp, int server, uint8_t cppr);
 void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr);
 uint32_t icp_accept(ICPState *ss);
+uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr);
 void icp_eoi(XICSState *icp, int server, uint32_t xirr);
 
 void ics_write_xive(ICSState *ics, int nr, int server,
-- 
2.7.4

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

* [Qemu-devel] [PULL 17/23] ppc/xics: Replace "icp" with "xics" in most places
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (15 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 16/23] ppc/xics: Implement H_IPOLL using an accessor David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 18/23] target-ppc: gen_pause for instructions: yield, mdoio, mdoom, miso David Gibson
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Nikunj A Dadhania, David Gibson

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The "ICP" is a different object than the "XICS". For historical reasons,
we have a number of places where we name a variable "icp" while it contains
a XICSState pointer. There *is* an ICPState structure too so this makes
the code really confusing.

This is a mechanical replacement of all those instances to use the name
"xics" instead. There should be no functional change.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[spapr_cpu_init has been moved to spapr_cpu_core.c, change there]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c              | 120 ++++++++++++++++++++++----------------------
 hw/intc/xics_kvm.c          |  57 +++++++++++----------
 hw/intc/xics_spapr.c        |  73 ++++++++++++++-------------
 hw/ppc/spapr.c              |  20 ++++----
 hw/ppc/spapr_cpu_core.c     |   4 +-
 hw/ppc/spapr_events.c       |   8 +--
 hw/ppc/spapr_pci.c          |  11 ++--
 hw/ppc/spapr_vio.c          |   2 +-
 include/hw/pci-host/spapr.h |   2 +-
 include/hw/ppc/spapr.h      |   2 +-
 include/hw/ppc/spapr_vio.h  |   2 +-
 include/hw/ppc/xics.h       |   2 +-
 12 files changed, 154 insertions(+), 149 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index f43f98a..cd48f42 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -47,31 +47,31 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
     return -1;
 }
 
-void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu)
+void xics_cpu_destroy(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &icp->ss[cs->cpu_index];
+    ICPState *ss = &xics->ss[cs->cpu_index];
 
-    assert(cs->cpu_index < icp->nr_servers);
+    assert(cs->cpu_index < xics->nr_servers);
     assert(cs == ss->cs);
 
     ss->output = NULL;
     ss->cs = NULL;
 }
 
-void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
+void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
-    ICPState *ss = &icp->ss[cs->cpu_index];
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    ICPState *ss = &xics->ss[cs->cpu_index];
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
 
-    assert(cs->cpu_index < icp->nr_servers);
+    assert(cs->cpu_index < xics->nr_servers);
 
     ss->cs = cs;
 
     if (info->cpu_setup) {
-        info->cpu_setup(icp, cpu);
+        info->cpu_setup(xics, cpu);
     }
 
     switch (PPC_INPUT(env)) {
@@ -95,21 +95,21 @@ void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
  */
 static void xics_common_reset(DeviceState *d)
 {
-    XICSState *icp = XICS_COMMON(d);
+    XICSState *xics = XICS_COMMON(d);
     int i;
 
-    for (i = 0; i < icp->nr_servers; i++) {
-        device_reset(DEVICE(&icp->ss[i]));
+    for (i = 0; i < xics->nr_servers; i++) {
+        device_reset(DEVICE(&xics->ss[i]));
     }
 
-    device_reset(DEVICE(icp->ics));
+    device_reset(DEVICE(xics->ics));
 }
 
 static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    int64_t value = icp->nr_irqs;
+    XICSState *xics = XICS_COMMON(obj);
+    int64_t value = xics->nr_irqs;
 
     visit_type_int(v, name, &value, errp);
 }
@@ -117,8 +117,8 @@ static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
 static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    XICSState *xics = XICS_COMMON(obj);
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
     Error *error = NULL;
     int64_t value;
 
@@ -127,23 +127,23 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
         error_propagate(errp, error);
         return;
     }
-    if (icp->nr_irqs) {
+    if (xics->nr_irqs) {
         error_setg(errp, "Number of interrupts is already set to %u",
-                   icp->nr_irqs);
+                   xics->nr_irqs);
         return;
     }
 
     assert(info->set_nr_irqs);
-    assert(icp->ics);
-    info->set_nr_irqs(icp, value, errp);
+    assert(xics->ics);
+    info->set_nr_irqs(xics, value, errp);
 }
 
 static void xics_prop_get_nr_servers(Object *obj, Visitor *v,
                                      const char *name, void *opaque,
                                      Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    int64_t value = icp->nr_servers;
+    XICSState *xics = XICS_COMMON(obj);
+    int64_t value = xics->nr_servers;
 
     visit_type_int(v, name, &value, errp);
 }
@@ -152,8 +152,8 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
                                      const char *name, void *opaque,
                                      Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    XICSState *xics = XICS_COMMON(obj);
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
     Error *error = NULL;
     int64_t value;
 
@@ -162,14 +162,14 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
         error_propagate(errp, error);
         return;
     }
-    if (icp->nr_servers) {
+    if (xics->nr_servers) {
         error_setg(errp, "Number of servers is already set to %u",
-                   icp->nr_servers);
+                   xics->nr_servers);
         return;
     }
 
     assert(info->set_nr_servers);
-    info->set_nr_servers(icp, value, errp);
+    info->set_nr_servers(xics, value, errp);
 }
 
 static void xics_common_initfn(Object *obj)
@@ -212,9 +212,9 @@ static void ics_reject(ICSState *ics, int nr);
 static void ics_resend(ICSState *ics);
 static void ics_eoi(ICSState *ics, int nr);
 
-static void icp_check_ipi(XICSState *icp, int server)
+static void icp_check_ipi(XICSState *xics, int server)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
         return;
@@ -223,7 +223,7 @@ static void icp_check_ipi(XICSState *icp, int server)
     trace_xics_icp_check_ipi(server, ss->mfrr);
 
     if (XISR(ss)) {
-        ics_reject(icp->ics, XISR(ss));
+        ics_reject(xics->ics, XISR(ss));
     }
 
     ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
@@ -231,19 +231,19 @@ static void icp_check_ipi(XICSState *icp, int server)
     qemu_irq_raise(ss->output);
 }
 
-static void icp_resend(XICSState *icp, int server)
+static void icp_resend(XICSState *xics, int server)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     if (ss->mfrr < CPPR(ss)) {
-        icp_check_ipi(icp, server);
+        icp_check_ipi(xics, server);
     }
-    ics_resend(icp->ics);
+    ics_resend(xics->ics);
 }
 
-void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
+void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
     uint8_t old_cppr;
     uint32_t old_xisr;
 
@@ -256,22 +256,22 @@ void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
             ss->xirr &= ~XISR_MASK; /* Clear XISR */
             ss->pending_priority = 0xff;
             qemu_irq_lower(ss->output);
-            ics_reject(icp->ics, old_xisr);
+            ics_reject(xics->ics, old_xisr);
         }
     } else {
         if (!XISR(ss)) {
-            icp_resend(icp, server);
+            icp_resend(xics, server);
         }
     }
 }
 
-void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
+void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     ss->mfrr = mfrr;
     if (mfrr < CPPR(ss)) {
-        icp_check_ipi(icp, server);
+        icp_check_ipi(xics, server);
     }
 }
 
@@ -296,31 +296,31 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
     return ss->xirr;
 }
 
-void icp_eoi(XICSState *icp, int server, uint32_t xirr)
+void icp_eoi(XICSState *xics, int server, uint32_t xirr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     /* Send EOI -> ICS */
     ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
     trace_xics_icp_eoi(server, xirr, ss->xirr);
-    ics_eoi(icp->ics, xirr & XISR_MASK);
+    ics_eoi(xics->ics, xirr & XISR_MASK);
     if (!XISR(ss)) {
-        icp_resend(icp, server);
+        icp_resend(xics, server);
     }
 }
 
-static void icp_irq(XICSState *icp, int server, int nr, uint8_t priority)
+static void icp_irq(XICSState *xics, int server, int nr, uint8_t priority)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     trace_xics_icp_irq(server, nr, priority);
 
     if ((priority >= CPPR(ss))
         || (XISR(ss) && (ss->pending_priority <= priority))) {
-        ics_reject(icp->ics, nr);
+        ics_reject(xics->ics, nr);
     } else {
         if (XISR(ss)) {
-            ics_reject(icp->ics, XISR(ss));
+            ics_reject(xics->ics, XISR(ss));
         }
         ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
         ss->pending_priority = priority;
@@ -405,7 +405,7 @@ static void resend_msi(ICSState *ics, int srcno)
     if (irq->status & XICS_STATUS_REJECTED) {
         irq->status &= ~XICS_STATUS_REJECTED;
         if (irq->priority != 0xff) {
-            icp_irq(ics->icp, irq->server, srcno + ics->offset,
+            icp_irq(ics->xics, irq->server, srcno + ics->offset,
                     irq->priority);
         }
     }
@@ -419,7 +419,7 @@ static void resend_lsi(ICSState *ics, int srcno)
         && (irq->status & XICS_STATUS_ASSERTED)
         && !(irq->status & XICS_STATUS_SENT)) {
         irq->status |= XICS_STATUS_SENT;
-        icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+        icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
     }
 }
 
@@ -434,7 +434,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
             irq->status |= XICS_STATUS_MASKED_PENDING;
             trace_xics_masked_pending();
         } else  {
-            icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+            icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
         }
     }
 }
@@ -473,7 +473,7 @@ static void write_xive_msi(ICSState *ics, int srcno)
     }
 
     irq->status &= ~XICS_STATUS_MASKED_PENDING;
-    icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+    icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
 }
 
 static void write_xive_lsi(ICSState *ics, int srcno)
@@ -558,8 +558,8 @@ static int ics_post_load(ICSState *ics, int version_id)
 {
     int i;
 
-    for (i = 0; i < ics->icp->nr_servers; i++) {
-        icp_resend(ics->icp, i);
+    for (i = 0; i < ics->xics->nr_servers; i++) {
+        icp_resend(ics->xics, i);
     }
 
     return 0;
@@ -659,14 +659,14 @@ static const TypeInfo ics_info = {
 /*
  * Exported functions
  */
-int xics_find_source(XICSState *icp, int irq)
+int xics_find_source(XICSState *xics, int irq)
 {
     int sources = 1;
     int src;
 
     /* FIXME: implement multiple sources */
     for (src = 0; src < sources; ++src) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
         if (ics_valid_irq(ics, irq)) {
             return src;
         }
@@ -675,12 +675,12 @@ int xics_find_source(XICSState *icp, int irq)
     return -1;
 }
 
-qemu_irq xics_get_qirq(XICSState *icp, int irq)
+qemu_irq xics_get_qirq(XICSState *xics, int irq)
 {
-    int src = xics_find_source(icp, irq);
+    int src = xics_find_source(xics, irq);
 
     if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
         return ics->qirqs[irq - ics->offset];
     }
 
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index a533e3d..edbd62f 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -145,7 +145,7 @@ static const TypeInfo icp_kvm_info = {
  */
 static void ics_get_kvm_state(ICSState *ics)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(ics->xics);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -160,7 +160,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
         attr.attr = i + ics->offset;
 
-        ret = ioctl(icpkvm->kernel_xics_fd, KVM_GET_DEVICE_ATTR, &attr);
+        ret = ioctl(xicskvm->kernel_xics_fd, KVM_GET_DEVICE_ATTR, &attr);
         if (ret != 0) {
             error_report("Unable to retrieve KVM interrupt controller state"
                     " for IRQ %d: %s", i + ics->offset, strerror(errno));
@@ -204,7 +204,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
 static int ics_set_kvm_state(ICSState *ics, int version_id)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(ics->xics);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -238,7 +238,7 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
             }
         }
 
-        ret = ioctl(icpkvm->kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr);
+        ret = ioctl(xicskvm->kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr);
         if (ret != 0) {
             error_report("Unable to restore KVM interrupt controller state"
                     " for IRQs %d: %s", i + ics->offset, strerror(errno));
@@ -324,17 +324,17 @@ static const TypeInfo ics_kvm_info = {
 /*
  * XICS-KVM
  */
-static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
+static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs;
     ICPState *ss;
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(xics);
 
     cs = CPU(cpu);
-    ss = &icp->ss[cs->cpu_index];
+    ss = &xics->ss[cs->cpu_index];
 
-    assert(cs->cpu_index < icp->nr_servers);
-    if (icpkvm->kernel_xics_fd == -1) {
+    assert(cs->cpu_index < xics->nr_servers);
+    if (xicskvm->kernel_xics_fd == -1) {
         abort();
     }
 
@@ -347,11 +347,12 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
         return;
     }
 
-    if (icpkvm->kernel_xics_fd != -1) {
+    if (xicskvm->kernel_xics_fd != -1) {
         int ret;
 
         ret = kvm_vcpu_enable_cap(cs, KVM_CAP_IRQ_XICS, 0,
-                                  icpkvm->kernel_xics_fd, kvm_arch_vcpu_id(cs));
+                                  xicskvm->kernel_xics_fd,
+                                  kvm_arch_vcpu_id(cs));
         if (ret < 0) {
             error_report("Unable to connect CPU%ld to kernel XICS: %s",
                     kvm_arch_vcpu_id(cs), strerror(errno));
@@ -361,24 +362,25 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
     }
 }
 
-static void xics_kvm_set_nr_irqs(XICSState *icp, uint32_t nr_irqs, Error **errp)
+static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
+                                 Error **errp)
 {
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
 }
 
-static void xics_kvm_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                     Error **errp)
 {
     int i;
 
-    icp->nr_servers = nr_servers;
+    xics->nr_servers = nr_servers;
 
-    icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
+    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
+    for (i = 0; i < xics->nr_servers; i++) {
         char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_KVM_ICP);
+        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_KVM_ICP);
         snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
                                   errp);
     }
 }
@@ -394,8 +396,8 @@ static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
 static void xics_kvm_realize(DeviceState *dev, Error **errp)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(dev);
-    XICSState *icp = XICS_COMMON(dev);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
+    XICSState *xics = XICS_COMMON(dev);
     int i, rc;
     Error *error = NULL;
     struct kvm_create_device xics_create_device = {
@@ -445,17 +447,18 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
         goto fail;
     }
 
-    icpkvm->kernel_xics_fd = xics_create_device.fd;
+    xicskvm->kernel_xics_fd = xics_create_device.fd;
 
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
     if (error) {
         error_propagate(errp, error);
         goto fail;
     }
 
-    assert(icp->nr_servers);
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+    assert(xics->nr_servers);
+    for (i = 0; i < xics->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
+                                 &error);
         if (error) {
             error_propagate(errp, error);
             goto fail;
@@ -481,7 +484,7 @@ static void xics_kvm_initfn(Object *obj)
 
     xics->ics = ICS(object_new(TYPE_KVM_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
+    xics->ics->xics = xics;
 }
 
 static void xics_kvm_class_init(ObjectClass *oc, void *data)
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 94571dd..618826d 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -45,7 +45,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUState *cs = CPU(cpu);
     target_ulong cppr = args[0];
 
-    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
+    icp_set_cppr(spapr->xics, cs->cpu_index, cppr);
     return H_SUCCESS;
 }
 
@@ -55,11 +55,11 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
     target_ulong mfrr = args[1];
 
-    if (server >= spapr->icp->nr_servers) {
+    if (server >= spapr->xics->nr_servers) {
         return H_PARAMETER;
     }
 
-    icp_set_mfrr(spapr->icp, server, mfrr);
+    icp_set_mfrr(spapr->xics, server, mfrr);
     return H_SUCCESS;
 }
 
@@ -67,7 +67,7 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
+    uint32_t xirr = icp_accept(spapr->xics->ss + cs->cpu_index);
 
     args[0] = xirr;
     return H_SUCCESS;
@@ -77,7 +77,7 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                              target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    ICPState *ss = &spapr->xics->ss[cs->cpu_index];
     uint32_t xirr = icp_accept(ss);
 
     args[0] = xirr;
@@ -91,7 +91,7 @@ static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUState *cs = CPU(cpu);
     target_ulong xirr = args[0];
 
-    icp_eoi(spapr->icp, cs->cpu_index, xirr);
+    icp_eoi(spapr->xics, cs->cpu_index, xirr);
     return H_SUCCESS;
 }
 
@@ -100,7 +100,7 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 {
     CPUState *cs = CPU(cpu);
     uint32_t mfrr;
-    uint32_t xirr = icp_ipoll(spapr->icp->ss + cs->cpu_index, &mfrr);
+    uint32_t xirr = icp_ipoll(spapr->xics->ss + cs->cpu_index, &mfrr);
 
     args[0] = xirr;
     args[1] = mfrr;
@@ -113,7 +113,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr, server, priority;
 
     if ((nargs != 3) || (nret != 1)) {
@@ -125,7 +125,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
     priority = rtas_ld(args, 2);
 
-    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
+    if (!ics_valid_irq(ics, nr) || (server >= ics->xics->nr_servers)
         || (priority > 0xff)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
@@ -141,7 +141,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 3)) {
@@ -166,7 +166,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                          uint32_t nargs, target_ulong args,
                          uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
@@ -192,7 +192,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                         uint32_t nargs, target_ulong args,
                         uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
@@ -214,36 +214,36 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 
-static void xics_spapr_set_nr_irqs(XICSState *icp, uint32_t nr_irqs,
+static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
                                    Error **errp)
 {
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
 }
 
-static void xics_spapr_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                       Error **errp)
 {
     int i;
 
-    icp->nr_servers = nr_servers;
+    xics->nr_servers = nr_servers;
 
-    icp->ss = g_malloc0(icp->nr_servers * sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
+    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
+    for (i = 0; i < xics->nr_servers; i++) {
         char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
+        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_ICP);
         snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
                                   errp);
     }
 }
 
 static void xics_spapr_realize(DeviceState *dev, Error **errp)
 {
-    XICSState *icp = XICS_SPAPR(dev);
+    XICSState *xics = XICS_SPAPR(dev);
     Error *error = NULL;
     int i;
 
-    if (!icp->nr_servers) {
+    if (!xics->nr_servers) {
         error_setg(errp, "Number of servers needs to be greater 0");
         return;
     }
@@ -261,14 +261,15 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
     spapr_register_hypercall(H_EOI, h_eoi);
     spapr_register_hypercall(H_IPOLL, h_ipoll);
 
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
     if (error) {
         error_propagate(errp, error);
         return;
     }
 
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+    for (i = 0; i < xics->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
+                                 &error);
         if (error) {
             error_propagate(errp, error);
             return;
@@ -282,7 +283,7 @@ static void xics_spapr_initfn(Object *obj)
 
     xics->ics = ICS(object_new(TYPE_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
+    xics->ics->xics = xics;
 }
 
 static void xics_spapr_class_init(ObjectClass *oc, void *data)
@@ -328,14 +329,14 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
     return -1;
 }
 
-int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
                      Error **errp)
 {
-    ICSState *ics = &icp->ics[src];
+    ICSState *ics = &xics->ics[src];
     int irq;
 
     if (irq_hint) {
-        assert(src == xics_find_source(icp, irq_hint));
+        assert(src == xics_find_source(xics, irq_hint));
         if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
             error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
             return -1;
@@ -360,11 +361,11 @@ int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
  * Allocate block of consecutive IRQs, and return the number of the first IRQ in
  * the block. If align==true, aligns the first IRQ number to num.
  */
-int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
                            bool align, Error **errp)
 {
     int i, first = -1;
-    ICSState *ics = &icp->ics[src];
+    ICSState *ics = &xics->ics[src];
 
     assert(src == 0);
     /*
@@ -404,23 +405,23 @@ static void ics_free(ICSState *ics, int srcno, int num)
 
     for (i = srcno; i < srcno + num; ++i) {
         if (ICS_IRQ_FREE(ics, i)) {
-            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
+            trace_xics_ics_free_warn(ics - ics->xics->ics, i + ics->offset);
         }
         memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
     }
 }
 
-void xics_spapr_free(XICSState *icp, int irq, int num)
+void xics_spapr_free(XICSState *xics, int irq, int num)
 {
-    int src = xics_find_source(icp, irq);
+    int src = xics_find_source(xics, irq);
 
     if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
 
         /* FIXME: implement multiple sources */
         assert(src == 0);
 
-        trace_xics_ics_free(ics - icp->ics, irq, num);
+        trace_xics_ics_free(ics - xics->ics, irq, num);
         ics_free(ics, irq - ics->offset, num);
     }
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3f0ea03..78ebd9e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -116,16 +116,16 @@ static XICSState *try_create_xics(const char *type, int nr_servers,
 static XICSState *xics_system_init(MachineState *machine,
                                    int nr_servers, int nr_irqs, Error **errp)
 {
-    XICSState *icp = NULL;
+    XICSState *xics = NULL;
 
     if (kvm_enabled()) {
         Error *err = NULL;
 
         if (machine_kernel_irqchip_allowed(machine)) {
-            icp = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
-                                  &err);
+            xics = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
+                                   &err);
         }
-        if (machine_kernel_irqchip_required(machine) && !icp) {
+        if (machine_kernel_irqchip_required(machine) && !xics) {
             error_reportf_err(err,
                               "kernel_irqchip requested but unavailable: ");
         } else {
@@ -133,11 +133,11 @@ static XICSState *xics_system_init(MachineState *machine,
         }
     }
 
-    if (!icp) {
-        icp = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
+    if (!xics) {
+        xics = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
     }
 
-    return icp;
+    return xics;
 }
 
 static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
@@ -1783,9 +1783,9 @@ static void ppc_spapr_init(MachineState *machine)
     load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
 
     /* Set up Interrupt Controller before we create the VCPUs */
-    spapr->icp = xics_system_init(machine,
-                                  DIV_ROUND_UP(max_cpus * smt, smp_threads),
-                                  XICS_IRQS_SPAPR, &error_fatal);
+    spapr->xics = xics_system_init(machine,
+                                   DIV_ROUND_UP(max_cpus * smt, smp_threads),
+                                   XICS_IRQS_SPAPR, &error_fatal);
 
     if (smc->dr_lmb_enabled) {
         spapr_validate_node_memory(machine, &error_fatal);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index cebeef5..2aa0dc5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -42,7 +42,7 @@ static void spapr_cpu_destroy(PowerPCCPU *cpu)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    xics_cpu_destroy(spapr->icp, cpu);
+    xics_cpu_destroy(spapr->xics, cpu);
     qemu_unregister_reset(spapr_cpu_reset, cpu);
 }
 
@@ -76,7 +76,7 @@ void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
         }
     }
 
-    xics_cpu_setup(spapr->icp, cpu);
+    xics_cpu_setup(spapr->xics, cpu);
 
     qemu_register_reset(spapr_cpu_reset, cpu);
     spapr_cpu_reset(cpu);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0585f8a..b0668b3 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -386,7 +386,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
 
     rtas_event_log_queue(RTAS_LOG_TYPE_EPOW, new_epow, true);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
 }
 
 static void spapr_hotplug_set_signalled(uint32_t drc_index)
@@ -468,7 +468,7 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
 
     rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
 }
 
 void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc)
@@ -551,7 +551,7 @@ static void check_exception(PowerPCCPU *cpu, sPAPRMachineState *spapr,
      * interrupts.
      */
     if (rtas_event_log_contains(mask, true)) {
-        qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+        qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
     }
 
     return;
@@ -603,7 +603,7 @@ out_no_events:
 void spapr_events_init(sPAPRMachineState *spapr)
 {
     QTAILQ_INIT(&spapr->pending_events);
-    spapr->check_exception_irq = xics_spapr_alloc(spapr->icp, 0, 0, false,
+    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, 0, false,
                                             &error_fatal);
     spapr->epow_notifier.notify = spapr_powerdown_req;
     qemu_register_powerdown_notifier(&spapr->epow_notifier);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 451651d..8c1e6b1 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -322,7 +322,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
             return;
         }
 
-        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->xics, msi->first_irq, msi->num);
         if (msi_present(pdev)) {
             spapr_msi_setmsg(pdev, 0, false, 0, 0);
         }
@@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* Allocate MSIs */
-    irq = xics_spapr_alloc_block(spapr->icp, 0, req_num, false,
+    irq = xics_spapr_alloc_block(spapr->xics, 0, req_num, false,
                            ret_intr_type == RTAS_TYPE_MSI, &err);
     if (err) {
         error_reportf_err(err, "Can't allocate MSIs for device %x: ",
@@ -371,7 +371,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     /* Release previous MSIs */
     if (msi) {
-        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->xics, msi->first_irq, msi->num);
         g_hash_table_remove(phb->msi, &config_addr);
     }
 
@@ -733,7 +733,7 @@ static void spapr_msi_write(void *opaque, hwaddr addr,
 
     trace_spapr_pci_msi_write(addr, data, irq);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, irq));
 }
 
 static const MemoryRegionOps spapr_msi_ops = {
@@ -1442,7 +1442,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         uint32_t irq;
         Error *local_err = NULL;
 
-        irq = xics_spapr_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
+        irq = xics_spapr_alloc_block(spapr->xics, 0, 1, true, false,
+                                     &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             error_prepend(errp, "can't allocate LSIs: ");
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 7ffd23e..f93244d 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
         dev->qdev.id = id;
     }
 
-    dev->irq = xics_spapr_alloc(spapr->icp, 0, dev->irq, false, &local_err);
+    dev->irq = xics_spapr_alloc(spapr->xics, 0, dev->irq, false, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 7848366..288b89c 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -93,7 +93,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
+    return xics_get_qirq(spapr->xics, phb->lsi_table[pin].irq);
 }
 
 PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e1f8274..4932555 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -52,7 +52,7 @@ struct sPAPRMachineState {
     struct VIOsPAPRBus *vio_bus;
     QLIST_HEAD(, sPAPRPHBState) phbs;
     struct sPAPRNVRAM *nvram;
-    XICSState *icp;
+    XICSState *xics;
     DeviceState *rtc;
 
     void *htab;
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 5f8b042..bdb5d2f 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -90,7 +90,7 @@ static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    return xics_get_qirq(spapr->icp, dev->irq);
+    return xics_get_qirq(spapr->xics, dev->irq);
 }
 
 static inline bool spapr_vio_dma_valid(VIOsPAPRDevice *dev, uint64_t taddr,
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 7445a14..6189a3b 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -143,7 +143,7 @@ struct ICSState {
     uint32_t offset;
     qemu_irq *qirqs;
     ICSIRQState *irqs;
-    XICSState *icp;
+    XICSState *xics;
 };
 
 static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
-- 
2.7.4

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

* [Qemu-devel] [PULL 18/23] target-ppc: gen_pause for instructions: yield, mdoio, mdoom, miso
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (16 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 17/23] ppc/xics: Replace "icp" with "xics" in most places David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 19/23] spapr: Restore support for 970MP and POWER8NVL CPU cores David Gibson
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Aaron Larson, David Gibson

From: Aaron Larson <alarson@ddci.com>

Call gen_pause for all "or rx,rx,rx" encodings other nop.  This
provides a reasonable implementation for yield, and a better
approximation for mdoio, mdoom, and miso.  The choice to pause for all
encodings !=0 leverages the PowerISA admonition that the reserved
encodings might change program priority, providing a slight "future
proofing".

Signed-off-by: Aaron Larson <alarson@ddci.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 49fe761..92030b6 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1471,7 +1471,7 @@ static void gen_or(DisasContext *ctx)
     } else if (unlikely(Rc(ctx->opcode) != 0)) {
         gen_set_Rc0(ctx, cpu_gpr[rs]);
 #if defined(TARGET_PPC64)
-    } else {
+    } else if (rs != 0) { /* 0 is nop */
         int prio = 0;
 
         switch (rs) {
@@ -1514,7 +1514,6 @@ static void gen_or(DisasContext *ctx)
             break;
 #endif
         default:
-            /* nop */
             break;
         }
         if (prio) {
@@ -1524,13 +1523,15 @@ static void gen_or(DisasContext *ctx)
             tcg_gen_ori_tl(t0, t0, ((uint64_t)prio) << 50);
             gen_store_spr(SPR_PPR, t0);
             tcg_temp_free(t0);
-            /* Pause us out of TCG otherwise spin loops with smt_low
-             * eat too much CPU and the kernel hangs
-             */
+        }
 #if !defined(CONFIG_USER_ONLY)
-            gen_pause(ctx);
+        /* Pause out of TCG otherwise spin loops with smt_low eat too much
+         * CPU and the kernel hangs.  This applies to all encodings other
+         * than no-op, e.g., miso(rs=26), yield(27), mdoio(29), mdoom(30),
+         * and all currently undefined.
+         */
+        gen_pause(ctx);
 #endif
-        }
 #endif
     }
 }
-- 
2.7.4

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

* [Qemu-devel] [PULL 19/23] spapr: Restore support for 970MP and POWER8NVL CPU cores
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (17 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 18/23] target-ppc: gen_pause for instructions: yield, mdoio, mdoom, miso David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 20/23] spapr: drop reference on child object during core realization David Gibson
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Bharata B Rao, David Gibson

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

Introduction of core based CPU hotplug for PowerPC sPAPR didn't
add support for 970MP and POWER8NVL based core types. Add support for
the same.

While we are here, add support for explicit specification of POWER5+_v2.1
core type.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_cpu_core.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2aa0dc5..e30b159 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -337,12 +337,15 @@ static void glue(glue(spapr_cpu_core_, _fname), _initfn(Object *obj)) \
     core->cpu_class = oc; \
 }
 
+SPAPR_CPU_CORE_INITFN(970mp_v1.0, 970MP_v10);
+SPAPR_CPU_CORE_INITFN(970mp_v1.1, 970MP_v11);
 SPAPR_CPU_CORE_INITFN(970_v2.2, 970);
 SPAPR_CPU_CORE_INITFN(POWER5+_v2.1, POWER5plus);
 SPAPR_CPU_CORE_INITFN(POWER7_v2.3, POWER7);
 SPAPR_CPU_CORE_INITFN(POWER7+_v2.1, POWER7plus);
 SPAPR_CPU_CORE_INITFN(POWER8_v2.0, POWER8);
 SPAPR_CPU_CORE_INITFN(POWER8E_v2.1, POWER8E);
+SPAPR_CPU_CORE_INITFN(POWER8NVL_v1.0, POWER8NVL);
 
 typedef struct SPAPRCoreInfo {
     const char *name;
@@ -350,10 +353,19 @@ typedef struct SPAPRCoreInfo {
 } SPAPRCoreInfo;
 
 static const SPAPRCoreInfo spapr_cores[] = {
-    /* 970 */
+    /* 970 and aliaes */
+    { .name = "970_v2.2", .initfn = spapr_cpu_core_970_initfn },
     { .name = "970", .initfn = spapr_cpu_core_970_initfn },
 
-    /* POWER5 */
+    /* 970MP variants and aliases */
+    { .name = "970MP_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
+    { .name = "970mp_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
+    { .name = "970MP_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
+    { .name = "970mp_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
+    { .name = "970mp", .initfn = spapr_cpu_core_970MP_v11_initfn },
+
+    /* POWER5 and aliases */
+    { .name = "POWER5+_v2.1", .initfn = spapr_cpu_core_POWER5plus_initfn },
     { .name = "POWER5+", .initfn = spapr_cpu_core_POWER5plus_initfn },
 
     /* POWER7 and aliases */
@@ -373,6 +385,10 @@ static const SPAPRCoreInfo spapr_cores[] = {
     { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn },
     { .name = "POWER8E", .initfn = spapr_cpu_core_POWER8E_initfn },
 
+    /* POWER8NVL and aliases */
+    { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn },
+    { .name = "POWER8NVL", .initfn = spapr_cpu_core_POWER8NVL_initfn },
+
     { .name = NULL }
 };
 
-- 
2.7.4

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

* [Qemu-devel] [PULL 20/23] spapr: drop reference on child object during core realization
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (18 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 19/23] spapr: Restore support for 970MP and POWER8NVL CPU cores David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 21/23] spapr: do proper error propagation in spapr_cpu_core_realize_child() David Gibson
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, benh, qemu-devel, qemu-ppc, Greg Kurz, David Gibson

From: Greg Kurz <groug@kaod.org>

When a core is being realized, we create a child object for each thread
of the core.

The child is first initialized with object_initialize() which sets its ref
count to 1, and then added to the core with object_property_add_child()
which bumps the ref count to 2.

When the core gets released, object_unparent() decreases the ref count to 1,
and we g_free() the object: we hence loose the reference on an unfinalized
object. This is likely to cause random crashes.

Let's drop the extra reference as soon as we don't need it, after the
thread is added to the core.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_cpu_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index e30b159..bba3612 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -300,6 +300,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
         if (local_err) {
             goto err;
         }
+        object_unref(obj);
     }
     object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child, &local_err);
     if (local_err) {
-- 
2.7.4

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

* [Qemu-devel] [PULL 21/23] spapr: do proper error propagation in spapr_cpu_core_realize_child()
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (19 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 20/23] spapr: drop reference on child object during core realization David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 22/23] spapr: drop duplicate variable in spapr_core_release() David Gibson
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, benh, qemu-devel, qemu-ppc, Greg Kurz, David Gibson

From: Greg Kurz <groug@kaod.org>

This patch changes spapr_cpu_core_realize_child() to have a local error
pointer and use error_propagate() as it is supposed to be done.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_cpu_core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index bba3612..2e264fa 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -262,18 +262,20 @@ out:
 
 static int spapr_cpu_core_realize_child(Object *child, void *opaque)
 {
-    Error **errp = opaque;
+    Error **errp = opaque, *local_err = NULL;
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
     CPUState *cs = CPU(child);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
 
-    object_property_set_bool(child, true, "realized", errp);
-    if (*errp) {
+    object_property_set_bool(child, true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return 1;
     }
 
-    spapr_cpu_init(spapr, cpu, errp);
-    if (*errp) {
+    spapr_cpu_init(spapr, cpu, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return 1;
     }
     return 0;
-- 
2.7.4

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

* [Qemu-devel] [PULL 22/23] spapr: drop duplicate variable in spapr_core_release()
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (20 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 21/23] spapr: do proper error propagation in spapr_cpu_core_realize_child() David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01  6:41 ` [Qemu-devel] [PULL 23/23] qmp: fix spapr example of query-hotpluggable-cpus David Gibson
  2016-07-01 13:28 ` [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 Peter Maydell
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell; +Cc: agraf, benh, qemu-devel, qemu-ppc, Greg Kurz, David Gibson

From: Greg Kurz <groug@kaod.org>

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_cpu_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2e264fa..a384db5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -102,7 +102,6 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
     const char *typename = object_class_get_name(sc->cpu_class);
     size_t size = object_type_get_instance_size(typename);
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
-    sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
     CPUCore *cc = CPU_CORE(dev);
     int smt = kvmppc_smt_threads();
     int i;
@@ -120,7 +119,7 @@ static void spapr_core_release(DeviceState *dev, void *opaque)
 
     spapr->cores[cc->core_id / smt] = NULL;
 
-    g_free(core->threads);
+    g_free(sc->threads);
     object_unparent(OBJECT(dev));
 }
 
-- 
2.7.4

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

* [Qemu-devel] [PULL 23/23] qmp: fix spapr example of query-hotpluggable-cpus
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (21 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 22/23] spapr: drop duplicate variable in spapr_core_release() David Gibson
@ 2016-07-01  6:41 ` David Gibson
  2016-07-01 13:28 ` [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 Peter Maydell
  23 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-01  6:41 UTC (permalink / raw)
  To: peter.maydell
  Cc: agraf, benh, qemu-devel, qemu-ppc, Igor Mammedov, David Gibson

From: Igor Mammedov <imammedo@redhat.com>

27393c33 qapi: keep names in 'CpuInstanceProperties' in sync with struct CPUCore
added -id suffix to property names but forgot to fix example in qmp-commands.hx

Fix example to have 'core-id' instead of 'core' to match current code

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 qmp-commands.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index b444c20..6937e83 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4978,8 +4978,8 @@ Example for pseries machine type started with
 
 -> { "execute": "query-hotpluggable-cpus" }
 <- {"return": [
-     { "props": { "core": 8 }, "type": "POWER8-spapr-cpu-core",
+     { "props": { "core-id": 8 }, "type": "POWER8-spapr-cpu-core",
        "vcpus-count": 1 },
-     { "props": { "core": 0 }, "type": "POWER8-spapr-cpu-core",
+     { "props": { "core-id": 0 }, "type": "POWER8-spapr-cpu-core",
        "vcpus-count": 1, "qom-path": "/machine/unattached/device[0]"}
    ]}'
-- 
2.7.4

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

* Re: [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701
  2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
                   ` (22 preceding siblings ...)
  2016-07-01  6:41 ` [Qemu-devel] [PULL 23/23] qmp: fix spapr example of query-hotpluggable-cpus David Gibson
@ 2016-07-01 13:28 ` Peter Maydell
  23 siblings, 0 replies; 42+ messages in thread
From: Peter Maydell @ 2016-07-01 13:28 UTC (permalink / raw)
  To: David Gibson
  Cc: Alexander Graf, Benjamin Herrenschmidt, QEMU Developers, qemu-ppc

On 1 July 2016 at 07:41, David Gibson <david@gibson.dropbear.id.au> wrote:
> The following changes since commit 8a0b4de048e20215415b24c7b42514c27b9d6ef3:
>
>   pcspk: fix KVM (2016-06-30 19:00:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.7-20160701
>
> for you to fetch changes up to 13f5e8003e7b67039cb7a19e41b4f7f7ac669cb3:
>
>   qmp: fix spapr example of query-hotpluggable-cpus (2016-07-01 13:41:47 +1000)
>
> ----------------------------------------------------------------
> ppc patch queue 2016-07-01
>
> Here's the current ppc patch queue.  This is a fairly large batch,
> containing:
>     * A number of further preliminary patches towards full hypervisor
>       mode emulation
>     * Some further fixes / cleanups for the recently merged device_add
>       based CPU hotplug
>     * Preliminary patches towards supporting a native (rather than
>       paravirtualized) XICS device.  This will be needed to emulate a
>       physical Power machine, including hypervisor capabilities
>     * Assorted bug fixes
>

Applied, thanks.

-- PMM

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-01  6:41 ` [Qemu-devel] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set David Gibson
@ 2016-07-09  0:43   ` Mark Cave-Ayland
  2016-07-09  2:46     ` Benjamin Herrenschmidt
  2016-07-09  8:16     ` Cédric Le Goater
  0 siblings, 2 replies; 42+ messages in thread
From: Mark Cave-Ayland @ 2016-07-09  0:43 UTC (permalink / raw)
  To: David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater, Ben Herrenschmidt

On 01/07/16 07:41, David Gibson wrote:

> From: Benjamin Herrenschmidt <b378bb0948277d71c78bc6d0c1ef80a253aafc80>
> 
> The architecture specifies that any instruction that sets MSR:PR will also
> set MSR:EE, IR and DR.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  target-ppc/helper_regs.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
> index 8fc0934..8fdfa5c 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
>          /* Change the exception prefix on PowerPC 601 */
>          env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
>      }
> +    /* If PR=1 then EE, IR and DR must be 1 */
> +    if ((value >> MSR_PR) & 1) {
> +        value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
> +    }
>  #endif
>      env->msr = value;
>      hreg_compute_hflags(env);
> 

Unfortunately this patch causes a regression and breaks booting OS 9 and
OS X under qemu-system-ppc.


ATB,

Mark.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  0:43   ` [Qemu-devel] [Qemu-ppc] " Mark Cave-Ayland
@ 2016-07-09  2:46     ` Benjamin Herrenschmidt
  2016-07-09  2:52       ` Benjamin Herrenschmidt
  2016-07-09  8:16     ` Cédric Le Goater
  1 sibling, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  2:46 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 01:43 +0100, Mark Cave-Ayland wrote:
> On 01/07/16 07:41, David Gibson wrote:
> 
> > From: Benjamin Herrenschmidt 
> > 
> > The architecture specifies that any instruction that sets MSR:PR will also
> > set MSR:EE, IR and DR.

 .../...

> Unfortunately this patch causes a regression and breaks booting OS 9 and
> OS X under qemu-system-ppc.

Any idea what is breaking specifically ? The architecture is pretty clear
here, could it be that they rely on old implementations allowing the
incorrect combination ?

Maybe we can make the restriction 64-bit server only...

Cheers,
Ben.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  2:46     ` Benjamin Herrenschmidt
@ 2016-07-09  2:52       ` Benjamin Herrenschmidt
  2016-07-09  3:00         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  2:52 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 12:46 +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2016-07-09 at 01:43 +0100, Mark Cave-Ayland wrote:
> > On 01/07/16 07:41, David Gibson wrote:
> > 
> > > From: Benjamin Herrenschmidt 
> > > 
> > > The architecture specifies that any instruction that sets MSR:PR
> > > will also
> > > set MSR:EE, IR and DR.
> 
>  .../...
> 
> > Unfortunately this patch causes a regression and breaks booting OS
> > 9 and
> > OS X under qemu-system-ppc.
> 
> Any idea what is breaking specifically ? The architecture is pretty
> clear
> here, could it be that they rely on old implementations allowing the
> incorrect combination ?
> 
> Maybe we can make the restriction 64-bit server only...

Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1
as well ! That means that if those old processors allow PR=1 and IR or
DR=0 and MacOS uses it, we do have a TLB coherency problem in qemu.

Cheers,
Ben.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  2:52       ` Benjamin Herrenschmidt
@ 2016-07-09  3:00         ` Benjamin Herrenschmidt
  2016-07-09  3:08           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:00 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 12:52 +1000, Benjamin Herrenschmidt wrote:
> 
> Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1
> as well ! That means that if those old processors allow PR=1 and IR
> or DR=0 and MacOS uses it, we do have a TLB coherency problem in
> qemu.

Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh.

Cheers,
Ben.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  3:00         ` Benjamin Herrenschmidt
@ 2016-07-09  3:08           ` Benjamin Herrenschmidt
  2016-07-09  3:22             ` [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations Benjamin Herrenschmidt
                               ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:08 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 13:00 +1000, Benjamin Herrenschmidt wrote:
> > Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1
> > as well ! That means that if those old processors allow PR=1 and IR
> > or DR=0 and MacOS uses it, we do have a TLB coherency problem in
> > qemu.
> 
> Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh.

Note that I see that happening with OS 9, but not with Darwin ... are
you sure about OS X ?

Cheers,
Ben.

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

* [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations
  2016-07-09  3:08           ` Benjamin Herrenschmidt
@ 2016-07-09  3:22             ` Benjamin Herrenschmidt
  2016-07-09  3:40               ` Benjamin Herrenschmidt
  2016-07-09  3:41             ` [Qemu-devel] [PATCH v2] " Benjamin Herrenschmidt
  2016-07-09  9:04             ` [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set Mark Cave-Ayland
  2 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:22 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

MacOS uses an architecturally illegal MSR combination that
seems nonetheless supported by 32-bit processors, which is
to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.

This adds support for it. To work properly we need to also
properly include support for PR=1,{I,D}R=0 to the MMU index
used by the qemu TLB.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

This applies on top of the existing "ppc: Enforce setting
MSR:EE, IR and DR when MSR:PR is set" patch, so don't revert it,
and fixes booting MacOS 9.

Mark: I haven't reproduced your problem with Darwin.

 target-ppc/helper_regs.h | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 8fdfa5c..4015ce2 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -41,17 +41,19 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
 
 static inline void hreg_compute_mem_idx(CPUPPCState *env)
 {
-    /* This is our encoding for server processors
+    /* This is our encoding for server processors. The architecture
+     * specifies that there is no such thing as userspace with
+     * translation off, however it appears that MacOS does it and
+     * some 32-bit CPUs support it. Weird...
      *
      *   0 = Guest User space virtual mode
      *   1 = Guest Kernel space virtual mode
-     *   2 = Guest Kernel space real mode
-     *   3 = HV User space virtual mode
-     *   4 = HV Kernel space virtual mode
-     *   5 = HV Kernel space real mode
-     *
-     * The combination PR=1 IR&DR=0 is invalid, we will treat
-     * it as IR=DR=1
+     *   2 = Guest User space real mode
+     *   3 = Guest Kernel space real mode
+     *   4 = HV User space virtual mode
+     *   5 = HV Kernel space virtual mode
+     *   6 = HV User space real mode
+     *   7 = HV Kernel space real mode
      *
      * For BookE, we need 8 MMU modes as follow:
      *
@@ -71,20 +73,11 @@ static inline void hreg_compute_mem_idx(CPUPPCState *env)
         env->immu_idx += msr_gs ? 4 : 0;
         env->dmmu_idx += msr_gs ? 4 : 0;
     } else {
-        /* First calucalte a base value independent of HV */
-        if (msr_pr != 0) {
-            /* User space, ignore IR and DR */
-            env->immu_idx = env->dmmu_idx = 0;
-        } else {
-            /* Kernel, setup a base I/D value */
-            env->immu_idx = msr_ir ? 1 : 2;
-            env->dmmu_idx = msr_dr ? 1 : 2;
-        }
-        /* Then offset it for HV */
-        if (msr_hv) {
-            env->immu_idx += 3;
-            env->dmmu_idx += 3;
-        }
+        env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
+        env->immu_idx += msr_ir ? 0 : 2;
+        env->dmmu_idx += msr_dr ? 0 : 2;
+        env->immu_idx += msr_hv ? 4 : 0;
+        env->dmmu_idx += msr_hv ? 4 : 0;
     }
 }
 
@@ -136,8 +129,13 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
         /* Change the exception prefix on PowerPC 601 */
         env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
     }
-    /* If PR=1 then EE, IR and DR must be 1 */
-    if ((value >> MSR_PR) & 1) {
+    /* If PR=1 then EE, IR and DR must be 1
+     *
+     * Note: We only enforce this on 64-bit processors. It appears that
+     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
+     * exploits it.
+     */
+    if ((env->flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
         value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
     }
 #endif


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

* Re: [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations
  2016-07-09  3:22             ` [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations Benjamin Herrenschmidt
@ 2016-07-09  3:40               ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:40 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 13:22 +1000, Benjamin Herrenschmidt wrote:
> MacOS uses an architecturally illegal MSR combination that
> seems nonetheless supported by 32-bit processors, which is
> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> 
> This adds support for it. To work properly we need to also
> properly include support for PR=1,{I,D}R=0 to the MMU index
> used by the qemu TLB.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---

Oopps, little bug, v2 on its way:
+ 
   if ((env->flags & PPC_64B) && ((value >> MSR_PR) & 1)) {

This should be insns_flags

>          value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
>      }
>  #endif

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

* [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-09  3:08           ` Benjamin Herrenschmidt
  2016-07-09  3:22             ` [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations Benjamin Herrenschmidt
@ 2016-07-09  3:41             ` Benjamin Herrenschmidt
  2016-07-09  3:42               ` Benjamin Herrenschmidt
  2016-07-11  1:55               ` David Gibson
  2016-07-09  9:04             ` [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set Mark Cave-Ayland
  2 siblings, 2 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:41 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

MacOS uses an architecturally illegal MSR combination that
seems nonetheless supported by 32-bit processors, which is
to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.

This adds support for it. To work properly we need to also
properly include support for PR=1,{I,D}R=0 to the MMU index
used by the qemu TLB.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

v2. Use the correct flags

 target-ppc/helper_regs.h | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 8fdfa5c..466ad67 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -41,17 +41,19 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
 
 static inline void hreg_compute_mem_idx(CPUPPCState *env)
 {
-    /* This is our encoding for server processors
+    /* This is our encoding for server processors. The architecture
+     * specifies that there is no such thing as userspace with
+     * translation off, however it appears that MacOS does it and
+     * some 32-bit CPUs support it. Weird...
      *
      *   0 = Guest User space virtual mode
      *   1 = Guest Kernel space virtual mode
-     *   2 = Guest Kernel space real mode
-     *   3 = HV User space virtual mode
-     *   4 = HV Kernel space virtual mode
-     *   5 = HV Kernel space real mode
-     *
-     * The combination PR=1 IR&DR=0 is invalid, we will treat
-     * it as IR=DR=1
+     *   2 = Guest User space real mode
+     *   3 = Guest Kernel space real mode
+     *   4 = HV User space virtual mode
+     *   5 = HV Kernel space virtual mode
+     *   6 = HV User space real mode
+     *   7 = HV Kernel space real mode
      *
      * For BookE, we need 8 MMU modes as follow:
      *
@@ -71,20 +73,11 @@ static inline void hreg_compute_mem_idx(CPUPPCState *env)
         env->immu_idx += msr_gs ? 4 : 0;
         env->dmmu_idx += msr_gs ? 4 : 0;
     } else {
-        /* First calucalte a base value independent of HV */
-        if (msr_pr != 0) {
-            /* User space, ignore IR and DR */
-            env->immu_idx = env->dmmu_idx = 0;
-        } else {
-            /* Kernel, setup a base I/D value */
-            env->immu_idx = msr_ir ? 1 : 2;
-            env->dmmu_idx = msr_dr ? 1 : 2;
-        }
-        /* Then offset it for HV */
-        if (msr_hv) {
-            env->immu_idx += 3;
-            env->dmmu_idx += 3;
-        }
+        env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
+        env->immu_idx += msr_ir ? 0 : 2;
+        env->dmmu_idx += msr_dr ? 0 : 2;
+        env->immu_idx += msr_hv ? 4 : 0;
+        env->dmmu_idx += msr_hv ? 4 : 0;
     }
 }
 
@@ -136,8 +129,13 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
         /* Change the exception prefix on PowerPC 601 */
         env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
     }
-    /* If PR=1 then EE, IR and DR must be 1 */
-    if ((value >> MSR_PR) & 1) {
+    /* If PR=1 then EE, IR and DR must be 1
+     *
+     * Note: We only enforce this on 64-bit processors. It appears that
+     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
+     * exploits it.
+     */
+    if ((env->insns_flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
         value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
     }
 #endif

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

* Re: [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-09  3:41             ` [Qemu-devel] [PATCH v2] " Benjamin Herrenschmidt
@ 2016-07-09  3:42               ` Benjamin Herrenschmidt
  2016-07-09  9:56                 ` Mark Cave-Ayland
  2016-07-11  1:55               ` David Gibson
  1 sibling, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  3:42 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Cédric Le Goater

On Sat, 2016-07-09 at 13:41 +1000, Benjamin Herrenschmidt wrote:
> MacOS uses an architecturally illegal MSR combination that
> seems nonetheless supported by 32-bit processors, which is
> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> 
> This adds support for it. To work properly we need to also
> properly include support for PR=1,{I,D}R=0 to the MMU index
> used by the qemu TLB.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Note Mark that with this patch on top of the dbdma series (which I did
manage to find in your repository :-), my OSX 9.2.2 installer now boots
without any hangs.

Cheers,
Ben.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  0:43   ` [Qemu-devel] [Qemu-ppc] " Mark Cave-Ayland
  2016-07-09  2:46     ` Benjamin Herrenschmidt
@ 2016-07-09  8:16     ` Cédric Le Goater
  2016-07-09  8:25       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 42+ messages in thread
From: Cédric Le Goater @ 2016-07-09  8:16 UTC (permalink / raw)
  To: Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc, Ben Herrenschmidt

On 07/09/2016 02:43 AM, Mark Cave-Ayland wrote:
> On 01/07/16 07:41, David Gibson wrote:
> 
>> From: Benjamin Herrenschmidt <b378bb0948277d71c78bc6d0c1ef80a253aafc80>
>>
>> The architecture specifies that any instruction that sets MSR:PR will also
>> set MSR:EE, IR and DR.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  target-ppc/helper_regs.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
>> index 8fc0934..8fdfa5c 100644
>> --- a/target-ppc/helper_regs.h
>> +++ b/target-ppc/helper_regs.h
>> @@ -136,6 +136,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
>>          /* Change the exception prefix on PowerPC 601 */
>>          env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
>>      }
>> +    /* If PR=1 then EE, IR and DR must be 1 */
>> +    if ((value >> MSR_PR) & 1) {
>> +        value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
>> +    }
>>  #endif
>>      env->msr = value;
>>      hreg_compute_hflags(env);
>>
> 
> Unfortunately this patch causes a regression and breaks booting OS 9 and
> OS X under qemu-system-ppc.

Ah This is curious. 


I used :

	qemu-system-ppc -M g3beige -cdrom darwinppc-602.cdr -boot d 
	qemu-system-ppc -M mac99 -cdrom darwinppc-602.cdr -boot d 
	qemu-system-ppc64 -M g3beige -cdrom darwinppc-602.cdr -boot d

which "work" as they reach the installation prompt :

	The following devices are available for installation.

This one hangs :

	qemu-system-ppc64 -M mac99 -cdrom darwinppc-602.cdr -boot d 

But that is expected for a 970 cpu.

The login prompt is reached with a full Darwin disk image. 

So I must be missing a scenario :/

Thanks,

C.


> 
> ATB,
> 
> Mark.
> 

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  8:16     ` Cédric Le Goater
@ 2016-07-09  8:25       ` Benjamin Herrenschmidt
  2016-07-09  8:28         ` Cédric Le Goater
  0 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-09  8:25 UTC (permalink / raw)
  To: Cédric Le Goater, Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-devel, qemu-ppc

On Sat, 2016-07-09 at 10:16 +0200, Cédric Le Goater wrote:
> The login prompt is reached with a full Darwin disk image. 
> 
> So I must be missing a scenario :/

Yes same here, Darwin worked fine, but I did reproduce the problem with
OS 9. See the fix I sent.

Cheers,
Ben.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  8:25       ` Benjamin Herrenschmidt
@ 2016-07-09  8:28         ` Cédric Le Goater
  0 siblings, 0 replies; 42+ messages in thread
From: Cédric Le Goater @ 2016-07-09  8:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Mark Cave-Ayland, David Gibson, peter.maydell
  Cc: qemu-ppc, qemu-devel

On 07/09/2016 10:25 AM, Benjamin Herrenschmidt wrote:
> On Sat, 2016-07-09 at 10:16 +0200, Cédric Le Goater wrote:
>> The login prompt is reached with a full Darwin disk image. 
>>
>> So I must be missing a scenario :/
> 
> Yes same here, Darwin worked fine, but I did reproduce the problem with
> OS 9. See the fix I sent.

Yep. I need to get one of these installers then.

Thanks, 

C.

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

* Re: [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set
  2016-07-09  3:08           ` Benjamin Herrenschmidt
  2016-07-09  3:22             ` [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations Benjamin Herrenschmidt
  2016-07-09  3:41             ` [Qemu-devel] [PATCH v2] " Benjamin Herrenschmidt
@ 2016-07-09  9:04             ` Mark Cave-Ayland
  2 siblings, 0 replies; 42+ messages in thread
From: Mark Cave-Ayland @ 2016-07-09  9:04 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, David Gibson, peter.maydell
  Cc: qemu-ppc, qemu-devel, Cédric Le Goater

On 09/07/16 04:08, Benjamin Herrenschmidt wrote:

> On Sat, 2016-07-09 at 13:00 +1000, Benjamin Herrenschmidt wrote:
>>> Additionally, hreg_compute_mem_idx() will treat PR=1 as DR=1/IR=1
>>> as well ! That means that if those old processors allow PR=1 and IR
>>> or DR=0 and MacOS uses it, we do have a TLB coherency problem in
>>> qemu.
>>
>> Wow, yes indeed, I see an MSR with PR=1 IR=0, IR=1 and EE=0 .. ugh.
> 
> Note that I see that happening with OS 9, but not with Darwin ... are
> you sure about OS X ?
> 
> Cheers,
> Ben.

Hmmm actually I think OS X might have been a red herring - I
double-checked and it looks like I was accidentally testing an illegal
combination, i.e. OS X 10.2 with -M mac99 which of course doesn't work.


ATB,

Mark.

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

* Re: [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-09  3:42               ` Benjamin Herrenschmidt
@ 2016-07-09  9:56                 ` Mark Cave-Ayland
  0 siblings, 0 replies; 42+ messages in thread
From: Mark Cave-Ayland @ 2016-07-09  9:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, David Gibson, peter.maydell
  Cc: qemu-ppc, qemu-devel, Cédric Le Goater

On 09/07/16 04:42, Benjamin Herrenschmidt wrote:

> On Sat, 2016-07-09 at 13:41 +1000, Benjamin Herrenschmidt wrote:
>> MacOS uses an architecturally illegal MSR combination that
>> seems nonetheless supported by 32-bit processors, which is
>> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
>>
>> This adds support for it. To work properly we need to also
>> properly include support for PR=1,{I,D}R=0 to the MMU index
>> used by the qemu TLB.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Note Mark that with this patch on top of the dbdma series (which I did
> manage to find in your repository :-), my OSX 9.2.2 installer now boots
> without any hangs.

Wow, just wow. This fixes the hard lockup after disk initialization I
was seeing with OS 9.2.2, and I've just done a complete install and boot
cycle of both OS 9.2.1 and OS 9.2.2 with no issues.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

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

* Re: [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-09  3:41             ` [Qemu-devel] [PATCH v2] " Benjamin Herrenschmidt
  2016-07-09  3:42               ` Benjamin Herrenschmidt
@ 2016-07-11  1:55               ` David Gibson
  2016-07-11 18:30                 ` Mark Cave-Ayland
  1 sibling, 1 reply; 42+ messages in thread
From: David Gibson @ 2016-07-11  1:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Mark Cave-Ayland, peter.maydell, qemu-devel, qemu-ppc,
	Cédric Le Goater

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

On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
> MacOS uses an architecturally illegal MSR combination that
> seems nonetheless supported by 32-bit processors, which is
> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> 
> This adds support for it. To work properly we need to also
> properly include support for PR=1,{I,D}R=0 to the MMU index
> used by the qemu TLB.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Applied to ppc-for-2.7, thanks.


> ---
> 
> v2. Use the correct flags
> 
>  target-ppc/helper_regs.h | 46 ++++++++++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
> index 8fdfa5c..466ad67 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -41,17 +41,19 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
>  
>  static inline void hreg_compute_mem_idx(CPUPPCState *env)
>  {
> -    /* This is our encoding for server processors
> +    /* This is our encoding for server processors. The architecture
> +     * specifies that there is no such thing as userspace with
> +     * translation off, however it appears that MacOS does it and
> +     * some 32-bit CPUs support it. Weird...
>       *
>       *   0 = Guest User space virtual mode
>       *   1 = Guest Kernel space virtual mode
> -     *   2 = Guest Kernel space real mode
> -     *   3 = HV User space virtual mode
> -     *   4 = HV Kernel space virtual mode
> -     *   5 = HV Kernel space real mode
> -     *
> -     * The combination PR=1 IR&DR=0 is invalid, we will treat
> -     * it as IR=DR=1
> +     *   2 = Guest User space real mode
> +     *   3 = Guest Kernel space real mode
> +     *   4 = HV User space virtual mode
> +     *   5 = HV Kernel space virtual mode
> +     *   6 = HV User space real mode
> +     *   7 = HV Kernel space real mode
>       *
>       * For BookE, we need 8 MMU modes as follow:
>       *
> @@ -71,20 +73,11 @@ static inline void hreg_compute_mem_idx(CPUPPCState *env)
>          env->immu_idx += msr_gs ? 4 : 0;
>          env->dmmu_idx += msr_gs ? 4 : 0;
>      } else {
> -        /* First calucalte a base value independent of HV */
> -        if (msr_pr != 0) {
> -            /* User space, ignore IR and DR */
> -            env->immu_idx = env->dmmu_idx = 0;
> -        } else {
> -            /* Kernel, setup a base I/D value */
> -            env->immu_idx = msr_ir ? 1 : 2;
> -            env->dmmu_idx = msr_dr ? 1 : 2;
> -        }
> -        /* Then offset it for HV */
> -        if (msr_hv) {
> -            env->immu_idx += 3;
> -            env->dmmu_idx += 3;
> -        }
> +        env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> +        env->immu_idx += msr_ir ? 0 : 2;
> +        env->dmmu_idx += msr_dr ? 0 : 2;
> +        env->immu_idx += msr_hv ? 4 : 0;
> +        env->dmmu_idx += msr_hv ? 4 : 0;
>      }
>  }
>  
> @@ -136,8 +129,13 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
>          /* Change the exception prefix on PowerPC 601 */
>          env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
>      }
> -    /* If PR=1 then EE, IR and DR must be 1 */
> -    if ((value >> MSR_PR) & 1) {
> +    /* If PR=1 then EE, IR and DR must be 1
> +     *
> +     * Note: We only enforce this on 64-bit processors. It appears that
> +     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> +     * exploits it.
> +     */
> +    if ((env->insns_flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
>          value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
>      }
>  #endif
> 
> 

-- 
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 --]

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

* Re: [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-11  1:55               ` David Gibson
@ 2016-07-11 18:30                 ` Mark Cave-Ayland
  2016-07-12  0:57                   ` David Gibson
  0 siblings, 1 reply; 42+ messages in thread
From: Mark Cave-Ayland @ 2016-07-11 18:30 UTC (permalink / raw)
  To: David Gibson, Benjamin Herrenschmidt
  Cc: peter.maydell, qemu-devel, qemu-ppc, Cédric Le Goater

On 11/07/16 02:55, David Gibson wrote:

> On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
>> MacOS uses an architecturally illegal MSR combination that
>> seems nonetheless supported by 32-bit processors, which is
>> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
>>
>> This adds support for it. To work properly we need to also
>> properly include support for PR=1,{I,D}R=0 to the MMU index
>> used by the qemu TLB.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Applied to ppc-for-2.7, thanks.

Hi David,

I can't see this in the ppc-for-2.7 branch on github - does it need a push?


ATB,

Mark.

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

* Re: [Qemu-devel] [PATCH v2] ppc: Fix support for odd MSR combinations
  2016-07-11 18:30                 ` Mark Cave-Ayland
@ 2016-07-12  0:57                   ` David Gibson
  0 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-07-12  0:57 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: Benjamin Herrenschmidt, peter.maydell, qemu-devel, qemu-ppc,
	Cédric Le Goater

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

On Mon, Jul 11, 2016 at 07:30:08PM +0100, Mark Cave-Ayland wrote:
1;4402;0c> On 11/07/16 02:55, David Gibson wrote:
> 
> > On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
> >> MacOS uses an architecturally illegal MSR combination that
> >> seems nonetheless supported by 32-bit processors, which is
> >> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> >>
> >> This adds support for it. To work properly we need to also
> >> properly include support for PR=1,{I,D}R=0 to the MMU index
> >> used by the qemu TLB.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > Applied to ppc-for-2.7, thanks.
> 
> Hi David,
> 
> I can't see this in the ppc-for-2.7 branch on github - does it need
> a push?

Yes it did.  Done now.

-- 
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 --]

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

end of thread, other threads:[~2016-07-12  1:19 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01  6:41 [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 01/23] ppc: Add a bunch of hypervisor SPRs to Book3s David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 02/23] ppc: Update LPCR definitions David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 03/23] ppc: Use a helper to filter writes to LPCR David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 04/23] ppc: Fix conditions for delivering external interrupts to a guest David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set David Gibson
2016-07-09  0:43   ` [Qemu-devel] [Qemu-ppc] " Mark Cave-Ayland
2016-07-09  2:46     ` Benjamin Herrenschmidt
2016-07-09  2:52       ` Benjamin Herrenschmidt
2016-07-09  3:00         ` Benjamin Herrenschmidt
2016-07-09  3:08           ` Benjamin Herrenschmidt
2016-07-09  3:22             ` [Qemu-devel] [PATCH] ppc: Fix support for odd MSR combinations Benjamin Herrenschmidt
2016-07-09  3:40               ` Benjamin Herrenschmidt
2016-07-09  3:41             ` [Qemu-devel] [PATCH v2] " Benjamin Herrenschmidt
2016-07-09  3:42               ` Benjamin Herrenschmidt
2016-07-09  9:56                 ` Mark Cave-Ayland
2016-07-11  1:55               ` David Gibson
2016-07-11 18:30                 ` Mark Cave-Ayland
2016-07-12  0:57                   ` David Gibson
2016-07-09  9:04             ` [Qemu-devel] [Qemu-ppc] [PULL 05/23] ppc: Enforce setting MSR:EE, IR and DR when MSR:PR is set Mark Cave-Ayland
2016-07-09  8:16     ` Cédric Le Goater
2016-07-09  8:25       ` Benjamin Herrenschmidt
2016-07-09  8:28         ` Cédric Le Goater
2016-07-01  6:41 ` [Qemu-devel] [PULL 06/23] ppc: Initial HDEC support David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 07/23] ppc: LPCR is a HV resource David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 08/23] ppc: Print HSRR0/HSRR1 in "info registers" David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 09/23] hw/ppc/spapr: Add some missing hcall function set strings David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 10/23] spapr: fix write-past-end-of-array error in cpu core device init code David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 11/23] spapr: Restore support for older PowerPC CPU cores David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 12/23] target-ppc: Eliminate redundant and incorrect function booke206_page_size_to_tlb David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 13/23] ppc: Fix 64K pages support in full emulation David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 14/23] ppc/xics: Rename existing xics to xics_spapr David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 15/23] ppc/xics: Move SPAPR specific code to a separate file David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 16/23] ppc/xics: Implement H_IPOLL using an accessor David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 17/23] ppc/xics: Replace "icp" with "xics" in most places David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 18/23] target-ppc: gen_pause for instructions: yield, mdoio, mdoom, miso David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 19/23] spapr: Restore support for 970MP and POWER8NVL CPU cores David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 20/23] spapr: drop reference on child object during core realization David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 21/23] spapr: do proper error propagation in spapr_cpu_core_realize_child() David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 22/23] spapr: drop duplicate variable in spapr_core_release() David Gibson
2016-07-01  6:41 ` [Qemu-devel] [PULL 23/23] qmp: fix spapr example of query-hotpluggable-cpus David Gibson
2016-07-01 13:28 ` [Qemu-devel] [PULL 00/23] ppc-for-2.7 queue 20160701 Peter Maydell

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