All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU)
@ 2018-04-05  2:14 David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names David Gibson
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

Here's a set of cleanups for the ppc cpu code.  Most are related
specifically to the 64-bit hash MMU, but there are some others as
well.

In particular it establishes a new structure PPCHash64Options which
contains details of the hash64 mmu which can vary from one cpu to
another.  This attempts to gather such options in one place, instead
of spreading them around various bits of env->mmu_model as well as
other fields.

Most of these arose while I was looking to improve the way we handle
available page sizes for the pseries machine type, although they're
mostly not closely tied to that.

Changes since RFC:
  * Added an extra patch folding slb_nr into the new scheme
  * Assorted minor fixes based on feedback

David Gibson (13):
  target/ppc: Standardize instance_init and realize function names
  target/ppc: Simplify cpu valid check in ppc_cpu_realize
  target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop()
  target/ppc: Avoid taking "env" parameter to mmu-hash64 functions
  target/ppc: Remove fallback 64k pagesize information
  target/ppc: Move page size setup to helper function
  target/ppc: Split page size information into a separate allocation
  target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs
  target/ppc: Move 1T segment and AMR options to PPCHash64Options
  target/ppc: Fold ci_large_pages flag into PPCHash64Options
  target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model
  target/ppc: Get rid of POWERPC_MMU_VER() macros
  target/ppc: Fold slb_nr into PPCHash64Options

 hw/ppc/fdt.c                |   7 +-
 hw/ppc/pnv.c                |   9 +--
 hw/ppc/spapr.c              |  20 +++---
 include/hw/ppc/fdt.h        |   2 +-
 target/ppc/cpu-qom.h        |  27 ++------
 target/ppc/cpu.h            |  30 ++-------
 target/ppc/kvm.c            |  31 ++++-----
 target/ppc/machine.c        |  23 ++++++-
 target/ppc/mmu-hash64.c     | 152 +++++++++++++++++++++++++++++++-------------
 target/ppc/mmu-hash64.h     |  48 +++++++++++++-
 target/ppc/mmu_helper.c     |  24 +++----
 target/ppc/translate.c      |  12 ++--
 target/ppc/translate_init.c | 126 +++++++-----------------------------
 13 files changed, 263 insertions(+), 248 deletions(-)

-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 02/13] target/ppc: Simplify cpu valid check in ppc_cpu_realize David Gibson
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

Because of the various hooks called some variant on "init" - and the rather
greater number that used to exist, I'm always wondering when a function
called simply "*_init" or "*_initfn" will be called.

To make it easier on myself, and maybe others, rename the instance_init
hooks for ppc cpus to *_instance_init().  While we're at it rename the
realize time hooks to *_realize() (from *_realizefn()) which seems to be
the more common current convention.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/translate_init.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 391b94b97d..56b80a204a 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9726,7 +9726,7 @@ static inline bool ppc_cpu_is_valid(PowerPCCPUClass *pcc)
 #endif
 }
 
-static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
+static void ppc_cpu_realize(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     PowerPCCPU *cpu = POWERPC_CPU(dev);
@@ -9952,7 +9952,7 @@ unrealize:
     cpu_exec_unrealizefn(cs);
 }
 
-static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
+static void ppc_cpu_unrealize(DeviceState *dev, Error **errp)
 {
     PowerPCCPU *cpu = POWERPC_CPU(dev);
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
@@ -10438,7 +10438,7 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
 }
 #endif
 
-static void ppc_cpu_initfn(Object *obj)
+static void ppc_cpu_instance_init(Object *obj)
 {
     CPUState *cs = CPU(obj);
     PowerPCCPU *cpu = POWERPC_CPU(obj);
@@ -10561,9 +10561,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     CPUClass *cc = CPU_CLASS(oc);
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    device_class_set_parent_realize(dc, ppc_cpu_realizefn,
+    device_class_set_parent_realize(dc, ppc_cpu_realize,
                                     &pcc->parent_realize);
-    device_class_set_parent_unrealize(dc, ppc_cpu_unrealizefn,
+    device_class_set_parent_unrealize(dc, ppc_cpu_unrealize,
                                       &pcc->parent_unrealize);
     pcc->pvr_match = ppc_pvr_match_default;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
@@ -10623,7 +10623,7 @@ static const TypeInfo ppc_cpu_type_info = {
     .name = TYPE_POWERPC_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(PowerPCCPU),
-    .instance_init = ppc_cpu_initfn,
+    .instance_init = ppc_cpu_instance_init,
     .abstract = true,
     .class_size = sizeof(PowerPCCPUClass),
     .class_init = ppc_cpu_class_init,
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 02/13] target/ppc: Simplify cpu valid check in ppc_cpu_realize
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 03/13] target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop() David Gibson
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

The #if isn't necessary, because there's a suitable one inside
ppc_cpu_is_valid().  We've already filtered for suitable cpu models in the
functions that search and register them.  So by the time we get to realize
having an invalid one indicates a code error, not a user error, so an
assert() is more appropriate than error_setg().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/translate_init.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 56b80a204a..2ae718242a 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9749,14 +9749,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp)
         }
     }
 
-#if defined(TARGET_PPCEMB)
-    if (!ppc_cpu_is_valid(pcc)) {
-        error_setg(errp, "CPU does not possess a BookE or 4xx MMU. "
-                   "Please use qemu-system-ppc or qemu-system-ppc64 instead "
-                   "or choose another CPU model.");
-        goto unrealize;
-    }
-#endif
+    assert(ppc_cpu_is_valid(pcc));
 
     create_ppc_opcodes(cpu, &local_err);
     if (local_err != NULL) {
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 03/13] target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop()
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 02/13] target/ppc: Simplify cpu valid check in ppc_cpu_realize David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 04/13] target/ppc: Avoid taking "env" parameter to mmu-hash64 functions David Gibson
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

As a rule we prefer to pass PowerPCCPU instead of CPUPPCState, and this
change will make some things simpler later on.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/fdt.c         | 5 +++--
 hw/ppc/pnv.c         | 4 ++--
 hw/ppc/spapr.c       | 4 ++--
 include/hw/ppc/fdt.h | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c
index 2ffc5866e4..2721603ffa 100644
--- a/hw/ppc/fdt.c
+++ b/hw/ppc/fdt.c
@@ -13,9 +13,10 @@
 #include "hw/ppc/fdt.h"
 
 #if defined(TARGET_PPC64)
-size_t ppc_create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
-                                     size_t maxsize)
+size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
+                                  size_t maxsize)
 {
+    CPUPPCState *env = &cpu->env;
     size_t maxcells = maxsize / sizeof(uint32_t);
     int i, j, count;
     uint32_t *p = prop;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 98ee3c607a..5a79b24828 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -209,8 +209,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
     }
 
-    page_sizes_prop_size = ppc_create_page_sizes_prop(env, page_sizes_prop,
-                                                  sizeof(page_sizes_prop));
+    page_sizes_prop_size = ppc_create_page_sizes_prop(cpu, page_sizes_prop,
+                                                      sizeof(page_sizes_prop));
     if (page_sizes_prop_size) {
         _FDT((fdt_setprop(fdt, offset, "ibm,segment-page-sizes",
                            page_sizes_prop, page_sizes_prop_size)));
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e764f999c5..14c31f82fa 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -581,8 +581,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
     }
 
-    page_sizes_prop_size = ppc_create_page_sizes_prop(env, page_sizes_prop,
-                                                  sizeof(page_sizes_prop));
+    page_sizes_prop_size = ppc_create_page_sizes_prop(cpu, page_sizes_prop,
+                                                      sizeof(page_sizes_prop));
     if (page_sizes_prop_size) {
         _FDT((fdt_setprop(fdt, offset, "ibm,segment-page-sizes",
                           page_sizes_prop, page_sizes_prop_size)));
diff --git a/include/hw/ppc/fdt.h b/include/hw/ppc/fdt.h
index bd5b0a8c3d..a8cd85069f 100644
--- a/include/hw/ppc/fdt.h
+++ b/include/hw/ppc/fdt.h
@@ -23,7 +23,7 @@
         }                                                          \
     } while (0)
 
-size_t ppc_create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
+size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
                                   size_t maxsize);
 
 #endif /* PPC_FDT_H */
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 04/13] target/ppc: Avoid taking "env" parameter to mmu-hash64 functions
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (2 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 03/13] target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop() David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 05/13] target/ppc: Remove fallback 64k pagesize information David Gibson
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

In most cases we prefer to pass a PowerPCCPU rather than the (embedded)
CPUPPCState.

For ppc_hash64_update_{rmls,vrma}() change to take "cpu" instead of "env".
For ppc_hash64_set_{dsi,isi}() remove the redundant "env" parameter.

In theory this makes more work for the functions, but since "cs", "cpu"
and "env" are related by at most constant offsets, the compiler should be
able to optimize out the difference at effectively zero cost.

helper_*() functions are left alone - since they're more closely tied to
the TCG generated code, passing "env" is still the standard there.

While we're there, fix an incorrect indentation.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/mmu-hash64.c     | 35 +++++++++++++++++++----------------
 target/ppc/mmu-hash64.h     |  4 ++--
 target/ppc/translate_init.c |  4 ++--
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index c9b72b7429..a87fa7c83f 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -633,9 +633,9 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
     return 0;
 }
 
-static void ppc_hash64_set_isi(CPUState *cs, CPUPPCState *env,
-                               uint64_t error_code)
+static void ppc_hash64_set_isi(CPUState *cs, uint64_t error_code)
 {
+    CPUPPCState *env = &POWERPC_CPU(cs)->env;
     bool vpm;
 
     if (msr_ir) {
@@ -659,9 +659,9 @@ static void ppc_hash64_set_isi(CPUState *cs, CPUPPCState *env,
     env->error_code = error_code;
 }
 
-static void ppc_hash64_set_dsi(CPUState *cs, CPUPPCState *env, uint64_t dar,
-                               uint64_t dsisr)
+static void ppc_hash64_set_dsi(CPUState *cs, uint64_t dar, uint64_t dsisr)
 {
+    CPUPPCState *env = &POWERPC_CPU(cs)->env;
     bool vpm;
 
     if (msr_dr) {
@@ -741,13 +741,13 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
             } else {
                 /* The access failed, generate the approriate interrupt */
                 if (rwx == 2) {
-                    ppc_hash64_set_isi(cs, env, SRR1_PROTFAULT);
+                    ppc_hash64_set_isi(cs, SRR1_PROTFAULT);
                 } else {
                     int dsisr = DSISR_PROTFAULT;
                     if (rwx == 1) {
                         dsisr |= DSISR_ISSTORE;
                     }
-                    ppc_hash64_set_dsi(cs, env, eaddr, dsisr);
+                    ppc_hash64_set_dsi(cs, eaddr, dsisr);
                 }
                 return 1;
             }
@@ -783,7 +783,7 @@ skip_slb_search:
 
     /* 3. Check for segment level no-execute violation */
     if ((rwx == 2) && (slb->vsid & SLB_VSID_N)) {
-        ppc_hash64_set_isi(cs, env, SRR1_NOEXEC_GUARD);
+        ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD);
         return 1;
     }
 
@@ -791,13 +791,13 @@ skip_slb_search:
     ptex = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
     if (ptex == -1) {
         if (rwx == 2) {
-            ppc_hash64_set_isi(cs, env, SRR1_NOPTE);
+            ppc_hash64_set_isi(cs, SRR1_NOPTE);
         } else {
             int dsisr = DSISR_NOPTE;
             if (rwx == 1) {
                 dsisr |= DSISR_ISSTORE;
             }
-            ppc_hash64_set_dsi(cs, env, eaddr, dsisr);
+            ppc_hash64_set_dsi(cs, eaddr, dsisr);
         }
         return 1;
     }
@@ -824,7 +824,7 @@ skip_slb_search:
             if (PAGE_EXEC & ~amr_prot) {
                 srr1 |= SRR1_IAMR; /* Access violates virt pg class key prot */
             }
-            ppc_hash64_set_isi(cs, env, srr1);
+            ppc_hash64_set_isi(cs, srr1);
         } else {
             int dsisr = 0;
             if (need_prot[rwx] & ~pp_prot) {
@@ -836,7 +836,7 @@ skip_slb_search:
             if (need_prot[rwx] & ~amr_prot) {
                 dsisr |= DSISR_AMR;
             }
-            ppc_hash64_set_dsi(cs, env, eaddr, dsisr);
+            ppc_hash64_set_dsi(cs, eaddr, dsisr);
         }
         return 1;
     }
@@ -942,8 +942,9 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
     cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
 }
 
-void ppc_hash64_update_rmls(CPUPPCState *env)
+void ppc_hash64_update_rmls(PowerPCCPU *cpu)
 {
+    CPUPPCState *env = &cpu->env;
     uint64_t lpcr = env->spr[SPR_LPCR];
 
     /*
@@ -976,8 +977,9 @@ void ppc_hash64_update_rmls(CPUPPCState *env)
     }
 }
 
-void ppc_hash64_update_vrma(CPUPPCState *env)
+void ppc_hash64_update_vrma(PowerPCCPU *cpu)
 {
+    CPUPPCState *env = &cpu->env;
     const struct ppc_one_seg_page_size *sps = NULL;
     target_ulong esid, vsid, lpcr;
     ppc_slb_t *slb = &env->vrma_slb;
@@ -1002,7 +1004,7 @@ void ppc_hash64_update_vrma(CPUPPCState *env)
     vsid |= (vrmasd << 4) & (SLB_VSID_L | SLB_VSID_LP);
     esid = SLB_ESID_V;
 
-   for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
+    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
         const struct ppc_one_seg_page_size *sps1 = &env->sps.sps[i];
 
         if (!sps1->page_shift) {
@@ -1028,6 +1030,7 @@ void ppc_hash64_update_vrma(CPUPPCState *env)
 
 void helper_store_lpcr(CPUPPCState *env, target_ulong val)
 {
+    PowerPCCPU *cpu = ppc_env_get_cpu(env);
     uint64_t lpcr = 0;
 
     /* Filter out bits */
@@ -1089,6 +1092,6 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val)
         ;
     }
     env->spr[SPR_LPCR] = lpcr;
-    ppc_hash64_update_rmls(env);
-    ppc_hash64_update_vrma(env);
+    ppc_hash64_update_rmls(cpu);
+    ppc_hash64_update_vrma(cpu);
 }
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index d297b97d37..95a8c330d6 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -17,8 +17,8 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
                                target_ulong pte0, target_ulong pte1);
 unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
                                           uint64_t pte0, uint64_t pte1);
-void ppc_hash64_update_vrma(CPUPPCState *env);
-void ppc_hash64_update_rmls(CPUPPCState *env);
+void ppc_hash64_update_vrma(PowerPCCPU *cpu);
+void ppc_hash64_update_rmls(PowerPCCPU *cpu);
 #endif
 
 /*
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 2ae718242a..29bd6f3654 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8975,8 +8975,8 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
     env->spr[SPR_AMOR] = amor->default_value = 0xffffffffffffffffull;
 
     /* Update some env bits based on new LPCR value */
-    ppc_hash64_update_rmls(env);
-    ppc_hash64_update_vrma(env);
+    ppc_hash64_update_rmls(cpu);
+    ppc_hash64_update_vrma(cpu);
 
     /* Tell KVM that we're in PAPR mode */
     if (kvm_enabled()) {
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 05/13] target/ppc: Remove fallback 64k pagesize information
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (3 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 04/13] target/ppc: Avoid taking "env" parameter to mmu-hash64 functions David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 06/13] target/ppc: Move page size setup to helper function David Gibson
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

CPU definitions for cpus with the 64-bit hash MMU can include a table of
available pagesizes.  If this isn't supplied ppc_cpu_instance_init() will
fill it in a fallback table based on the POWERPC_MMU_64K bit in mmu_model.

However, it turns out all the cpus which support 64K pages already include
an explicit table of page sizes, so there's no point to the fallback table
including 64k pages.

That removes the only place which tests POWERPC_MMU_64K, so we can remove
it.  Which in turn allows some logic to be removed from
kvm_fixup_page_sizes().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/cpu-qom.h        |  4 ----
 target/ppc/kvm.c            |  7 -------
 target/ppc/translate_init.c | 20 ++------------------
 3 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index deaa46a14b..9bbb05cf62 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -70,7 +70,6 @@ enum powerpc_mmu_t {
 #define POWERPC_MMU_64       0x00010000
 #define POWERPC_MMU_1TSEG    0x00020000
 #define POWERPC_MMU_AMR      0x00040000
-#define POWERPC_MMU_64K      0x00080000
 #define POWERPC_MMU_V3       0x00100000 /* ISA V3.00 MMU Support */
     /* 64 bits PowerPC MMU                                     */
     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
@@ -78,15 +77,12 @@ enum powerpc_mmu_t {
     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.07 variant                               */
     POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
-                             | POWERPC_MMU_64K
                              | POWERPC_MMU_AMR | 0x00000004,
     /* Architecture 3.00 variant                               */
     POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
-                             | POWERPC_MMU_64K
                              | POWERPC_MMU_AMR | POWERPC_MMU_V3
                              | 0x00000005,
 };
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 79a436a384..6160356a4a 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -425,7 +425,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
     static bool has_smmu_info;
     CPUPPCState *env = &cpu->env;
     int iq, ik, jq, jk;
-    bool has_64k_pages = false;
 
     /* We only handle page sizes for 64-bit server guests for now */
     if (!(env->mmu_model & POWERPC_MMU_64)) {
@@ -471,9 +470,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
                                      ksps->enc[jk].page_shift)) {
                 continue;
             }
-            if (ksps->enc[jk].page_shift == 16) {
-                has_64k_pages = true;
-            }
             qsps->enc[jq].page_shift = ksps->enc[jk].page_shift;
             qsps->enc[jq].pte_enc = ksps->enc[jk].pte_enc;
             if (++jq >= PPC_PAGE_SIZES_MAX_SZ) {
@@ -488,9 +484,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
     if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
         env->mmu_model &= ~POWERPC_MMU_1TSEG;
     }
-    if (!has_64k_pages) {
-        env->mmu_model &= ~POWERPC_MMU_64K;
-    }
 }
 
 bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 29bd6f3654..99be6fcd68 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10469,7 +10469,7 @@ static void ppc_cpu_instance_init(Object *obj)
         env->sps = *pcc->sps;
     } else if (env->mmu_model & POWERPC_MMU_64) {
         /* Use default sets of page sizes. We don't support MPSS */
-        static const struct ppc_segment_page_sizes defsps_4k = {
+        static const struct ppc_segment_page_sizes defsps = {
             .sps = {
                 { .page_shift = 12, /* 4K */
                   .slb_enc = 0,
@@ -10481,23 +10481,7 @@ static void ppc_cpu_instance_init(Object *obj)
                 },
             },
         };
-        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;
+        env->sps = defsps;
     }
 #endif /* defined(TARGET_PPC64) */
 }
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 06/13] target/ppc: Move page size setup to helper function
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (4 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 05/13] target/ppc: Remove fallback 64k pagesize information David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 07/13] target/ppc: Split page size information into a separate allocation David Gibson
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

Initialization of the env->sps structure at the end of instance_init is
specific to the 64-bit hash MMU, so move the code into a helper function
in mmu-hash64.c.

We also create a corresponding function to be called at finalize time -
it's empty for now, but we'll need it shortly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/mmu-hash64.c     | 29 +++++++++++++++++++++++++++++
 target/ppc/mmu-hash64.h     | 11 +++++++++++
 target/ppc/translate_init.c | 29 +++++++++--------------------
 3 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index a87fa7c83f..4cb7d1cf07 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1095,3 +1095,32 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val)
     ppc_hash64_update_rmls(cpu);
     ppc_hash64_update_vrma(cpu);
 }
+
+void ppc_hash64_init(PowerPCCPU *cpu)
+{
+    CPUPPCState *env = &cpu->env;
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+    if (pcc->sps) {
+        env->sps = *pcc->sps;
+    } else if (env->mmu_model & POWERPC_MMU_64) {
+        /* Use default sets of page sizes. We don't support MPSS */
+        static const struct ppc_segment_page_sizes defsps = {
+            .sps = {
+                { .page_shift = 12, /* 4K */
+                  .slb_enc = 0,
+                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
+                },
+                { .page_shift = 24, /* 16M */
+                  .slb_enc = 0x100,
+                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
+                },
+            },
+        };
+        env->sps = defsps;
+    }
+}
+
+void ppc_hash64_finalize(PowerPCCPU *cpu)
+{
+}
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 95a8c330d6..074ded4c27 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -19,6 +19,8 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
                                           uint64_t pte0, uint64_t pte1);
 void ppc_hash64_update_vrma(PowerPCCPU *cpu);
 void ppc_hash64_update_rmls(PowerPCCPU *cpu);
+void ppc_hash64_init(PowerPCCPU *cpu);
+void ppc_hash64_finalize(PowerPCCPU *cpu);
 #endif
 
 /*
@@ -136,4 +138,13 @@ static inline uint64_t ppc_hash64_hpte1(PowerPCCPU *cpu,
 
 #endif /* CONFIG_USER_ONLY */
 
+#if defined(CONFIG_USER_ONLY) || !defined(TARGET_PPC64)
+static inline void ppc_hash64_init(PowerPCCPU *cpu)
+{
+}
+static inline void ppc_hash64_finalize(PowerPCCPU *cpu)
+{
+}
+#endif
+
 #endif /* MMU_HASH64_H */
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 99be6fcd68..aa63a5dcb3 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10464,26 +10464,14 @@ static void ppc_cpu_instance_init(Object *obj)
     env->has_hv_mode = !!(env->msr_mask & MSR_HVB);
 #endif
 
-#if defined(TARGET_PPC64)
-    if (pcc->sps) {
-        env->sps = *pcc->sps;
-    } else if (env->mmu_model & POWERPC_MMU_64) {
-        /* Use default sets of page sizes. We don't support MPSS */
-        static const struct ppc_segment_page_sizes defsps = {
-            .sps = {
-                { .page_shift = 12, /* 4K */
-                  .slb_enc = 0,
-                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
-                },
-                { .page_shift = 24, /* 16M */
-                  .slb_enc = 0x100,
-                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
-                },
-            },
-        };
-        env->sps = defsps;
-    }
-#endif /* defined(TARGET_PPC64) */
+    ppc_hash64_init(cpu);
+}
+
+static void ppc_cpu_instance_finalize(Object *obj)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(obj);
+
+    ppc_hash64_finalize(cpu);
 }
 
 static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
@@ -10601,6 +10589,7 @@ static const TypeInfo ppc_cpu_type_info = {
     .parent = TYPE_CPU,
     .instance_size = sizeof(PowerPCCPU),
     .instance_init = ppc_cpu_instance_init,
+    .instance_finalize = ppc_cpu_instance_finalize,
     .abstract = true,
     .class_size = sizeof(PowerPCCPUClass),
     .class_init = ppc_cpu_class_init,
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 07/13] target/ppc: Split page size information into a separate allocation
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (5 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 06/13] target/ppc: Move page size setup to helper function David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 08/13] target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs David Gibson
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

env->sps contains page size encoding information as an embedded structure.
Since this information is specific to 64-bit hash MMUs, split it out into
a separately allocated structure, to reduce the basic env size for other
cpus.  Along the way we make a few other cleanups:

    * Rename to PPCHash64Options which is more in line with qemu name
      conventions, and reflects that we're going to merge some more hash64
      mmu specific details in there in future.  Also rename its
      substructures to match qemu conventions.

    * Move structure definitions to the mmu-hash64.[ch] files.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/fdt.c                |  4 +--
 target/ppc/cpu-qom.h        |  4 +--
 target/ppc/cpu.h            | 26 +++----------------
 target/ppc/kvm.c            |  4 +--
 target/ppc/mmu-hash64.c     | 61 +++++++++++++++++++++++++++++++++------------
 target/ppc/mmu-hash64.h     | 22 ++++++++++++++++
 target/ppc/translate_init.c | 36 +++-----------------------
 7 files changed, 80 insertions(+), 77 deletions(-)

diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c
index 2721603ffa..0828ad7254 100644
--- a/hw/ppc/fdt.c
+++ b/hw/ppc/fdt.c
@@ -9,6 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "target/ppc/cpu.h"
+#include "target/ppc/mmu-hash64.h"
 
 #include "hw/ppc/fdt.h"
 
@@ -16,13 +17,12 @@
 size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop,
                                   size_t maxsize)
 {
-    CPUPPCState *env = &cpu->env;
     size_t maxcells = maxsize / sizeof(uint32_t);
     int i, j, count;
     uint32_t *p = prop;
 
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
-        struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
+        PPCHash64SegmentPageSizes *sps = &cpu->hash64_opts->sps[i];
 
         if (!sps->page_shift) {
             break;
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 9bbb05cf62..3e5ef7375f 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -160,7 +160,7 @@ enum powerpc_input_t {
     PPC_FLAGS_INPUT_RCPU,
 };
 
-struct ppc_segment_page_sizes;
+typedef struct PPCHash64Options PPCHash64Options;
 
 /**
  * PowerPCCPUClass:
@@ -194,7 +194,7 @@ typedef struct PowerPCCPUClass {
     uint32_t flags;
     int bfd_mach;
     uint32_t l1_dcache_size, l1_icache_size;
-    const struct ppc_segment_page_sizes *sps;
+    const PPCHash64Options *hash64_opts;
     struct ppc_radix_page_info *radix_page_info;
     void (*init_proc)(CPUPPCState *env);
     int  (*check_pow)(CPUPPCState *env);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c621a6bd5e..1c5c33ca11 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -327,11 +327,13 @@ union ppc_tlb_t {
 #define TLB_MAS                3
 #endif
 
+typedef struct PPCHash64SegmentPageSizes PPCHash64SegmentPageSizes;
+
 typedef struct ppc_slb_t ppc_slb_t;
 struct ppc_slb_t {
     uint64_t esid;
     uint64_t vsid;
-    const struct ppc_one_seg_page_size *sps;
+    const PPCHash64SegmentPageSizes *sps;
 };
 
 #define MAX_SLB_ENTRIES         64
@@ -948,28 +950,8 @@ enum {
 
 #define DBELL_PROCIDTAG_MASK           PPC_BITMASK(44, 63)
 
-/*****************************************************************************/
-/* Segment page size information, used by recent hash MMUs
- * The format of this structure mirrors kvm_ppc_smmu_info
- */
-
 #define PPC_PAGE_SIZES_MAX_SZ   8
 
-struct ppc_one_page_size {
-    uint32_t page_shift;  /* Page shift (or 0) */
-    uint32_t pte_enc;     /* Encoding in the HPTE (>>12) */
-};
-
-struct ppc_one_seg_page_size {
-    uint32_t page_shift;  /* Base page shift of segment (or 0) */
-    uint32_t slb_enc;     /* SLB encoding for BookS */
-    struct ppc_one_page_size enc[PPC_PAGE_SIZES_MAX_SZ];
-};
-
-struct ppc_segment_page_sizes {
-    struct ppc_one_seg_page_size sps[PPC_PAGE_SIZES_MAX_SZ];
-};
-
 struct ppc_radix_page_info {
     uint32_t count;
     uint32_t entries[PPC_PAGE_SIZES_MAX_SZ];
@@ -1106,7 +1088,6 @@ struct CPUPPCState {
     uint64_t insns_flags;
     uint64_t insns_flags2;
 #if defined(TARGET_PPC64)
-    struct ppc_segment_page_sizes sps;
     ppc_slb_t vrma_slb;
     target_ulong rmls;
     bool ci_large_pages;
@@ -1227,6 +1208,7 @@ struct PowerPCCPU {
     PPCVirtualHypervisor *vhyp;
     Object *intc;
     int32_t node_id; /* NUMA node this CPU belongs to */
+    PPCHash64Options *hash64_opts;
 
     /* Fields related to migration compatibility hacks */
     bool pre_2_8_migration;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 6160356a4a..bc6d0a8314 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -442,7 +442,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
     }
 
     /* Convert to QEMU form */
-    memset(&env->sps, 0, sizeof(env->sps));
+    memset(cpu->hash64_opts, 0, sizeof(*cpu->hash64_opts));
 
     /* If we have HV KVM, we need to forbid CI large pages if our
      * host page size is smaller than 64K.
@@ -456,7 +456,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
      *     the selected CPU has with the capabilities that KVM supports.
      */
     for (ik = iq = 0; ik < KVM_PPC_PAGE_SIZES_MAX_SZ; ik++) {
-        struct ppc_one_seg_page_size *qsps = &env->sps.sps[iq];
+        PPCHash64SegmentPageSizes *qsps = &cpu->hash64_opts->sps[iq];
         struct kvm_ppc_one_seg_page_size *ksps = &smmu_info.sps[ik];
 
         if (!kvm_valid_page_size(smmu_info.flags, max_cpu_page_size,
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 4cb7d1cf07..6758afd9de 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -148,7 +148,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
 {
     CPUPPCState *env = &cpu->env;
     ppc_slb_t *slb = &env->slb[slot];
-    const struct ppc_one_seg_page_size *sps = NULL;
+    const PPCHash64SegmentPageSizes *sps = NULL;
     int i;
 
     if (slot >= env->slb_nr) {
@@ -165,7 +165,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
     }
 
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
-        const struct ppc_one_seg_page_size *sps1 = &env->sps.sps[i];
+        const PPCHash64SegmentPageSizes *sps1 = &cpu->hash64_opts->sps[i];
 
         if (!sps1->page_shift) {
             break;
@@ -451,8 +451,8 @@ void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
                         false, n * HASH_PTE_SIZE_64);
 }
 
-static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
-    uint64_t pte0, uint64_t pte1)
+static unsigned hpte_page_shift(const PPCHash64SegmentPageSizes *sps,
+                                uint64_t pte0, uint64_t pte1)
 {
     int i;
 
@@ -466,7 +466,7 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
     }
 
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
-        const struct ppc_one_page_size *ps = &sps->enc[i];
+        const PPCHash64PageSize *ps = &sps->enc[i];
         uint64_t mask;
 
         if (!ps->page_shift) {
@@ -489,7 +489,7 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
 }
 
 static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
-                                     const struct ppc_one_seg_page_size *sps,
+                                     const PPCHash64SegmentPageSizes *sps,
                                      target_ulong ptem,
                                      ppc_hash_pte64_t *pte, unsigned *pshift)
 {
@@ -543,7 +543,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
     CPUPPCState *env = &cpu->env;
     hwaddr hash, ptex;
     uint64_t vsid, epnmask, epn, ptem;
-    const struct ppc_one_seg_page_size *sps = slb->sps;
+    const PPCHash64SegmentPageSizes *sps = slb->sps;
 
     /* The SLB store path should prevent any bad page size encodings
      * getting in there, so: */
@@ -552,7 +552,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
     /* If ISL is set in LPCR we need to clamp the page size to 4K */
     if (env->spr[SPR_LPCR] & LPCR_ISL) {
         /* We assume that when using TCG, 4k is first entry of SPS */
-        sps = &env->sps.sps[0];
+        sps = &cpu->hash64_opts->sps[0];
         assert(sps->page_shift == 12);
     }
 
@@ -605,7 +605,6 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
 unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
                                           uint64_t pte0, uint64_t pte1)
 {
-    CPUPPCState *env = &cpu->env;
     int i;
 
     if (!(pte0 & HPTE64_V_LARGE)) {
@@ -617,7 +616,7 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
      * this gives an unambiguous result.
      */
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
-        const struct ppc_one_seg_page_size *sps = &env->sps.sps[i];
+        const PPCHash64SegmentPageSizes *sps = &cpu->hash64_opts->sps[i];
         unsigned shift;
 
         if (!sps->page_shift) {
@@ -980,7 +979,7 @@ void ppc_hash64_update_rmls(PowerPCCPU *cpu)
 void ppc_hash64_update_vrma(PowerPCCPU *cpu)
 {
     CPUPPCState *env = &cpu->env;
-    const struct ppc_one_seg_page_size *sps = NULL;
+    const PPCHash64SegmentPageSizes *sps = NULL;
     target_ulong esid, vsid, lpcr;
     ppc_slb_t *slb = &env->vrma_slb;
     uint32_t vrmasd;
@@ -1005,7 +1004,7 @@ void ppc_hash64_update_vrma(PowerPCCPU *cpu)
     esid = SLB_ESID_V;
 
     for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
-        const struct ppc_one_seg_page_size *sps1 = &env->sps.sps[i];
+        const PPCHash64SegmentPageSizes *sps1 = &cpu->hash64_opts->sps[i];
 
         if (!sps1->page_shift) {
             break;
@@ -1101,11 +1100,12 @@ void ppc_hash64_init(PowerPCCPU *cpu)
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
 
-    if (pcc->sps) {
-        env->sps = *pcc->sps;
+    if (pcc->hash64_opts) {
+        cpu->hash64_opts = g_memdup(pcc->hash64_opts,
+                                    sizeof(*cpu->hash64_opts));
     } else if (env->mmu_model & POWERPC_MMU_64) {
         /* Use default sets of page sizes. We don't support MPSS */
-        static const struct ppc_segment_page_sizes defsps = {
+        static const PPCHash64Options defopts = {
             .sps = {
                 { .page_shift = 12, /* 4K */
                   .slb_enc = 0,
@@ -1117,10 +1117,39 @@ void ppc_hash64_init(PowerPCCPU *cpu)
                 },
             },
         };
-        env->sps = defsps;
+        cpu->hash64_opts = g_memdup(&defopts, sizeof(*cpu->hash64_opts));
     }
 }
 
 void ppc_hash64_finalize(PowerPCCPU *cpu)
 {
+    g_free(cpu->hash64_opts);
 }
+
+const PPCHash64Options ppc_hash64_opts_POWER7 = {
+    .sps = {
+        {
+            .page_shift = 12, /* 4K */
+            .slb_enc = 0,
+            .enc = { { .page_shift = 12, .pte_enc = 0 },
+                     { .page_shift = 16, .pte_enc = 0x7 },
+                     { .page_shift = 24, .pte_enc = 0x38 }, },
+        },
+        {
+            .page_shift = 16, /* 64K */
+            .slb_enc = SLB_VSID_64K,
+            .enc = { { .page_shift = 16, .pte_enc = 0x1 },
+                     { .page_shift = 24, .pte_enc = 0x8 }, },
+        },
+        {
+            .page_shift = 24, /* 16M */
+            .slb_enc = SLB_VSID_16M,
+            .enc = { { .page_shift = 24, .pte_enc = 0 }, },
+        },
+        {
+            .page_shift = 34, /* 16G */
+            .slb_enc = SLB_VSID_16G,
+            .enc = { { .page_shift = 34, .pte_enc = 0x3 }, },
+        },
+    }
+};
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 074ded4c27..957bd68e33 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -136,6 +136,28 @@ static inline uint64_t ppc_hash64_hpte1(PowerPCCPU *cpu,
     return ldq_p(&(hptes[i].pte1));
 }
 
+/*
+ * MMU Options
+ */
+
+struct PPCHash64PageSize {
+    uint32_t page_shift;  /* Page shift (or 0) */
+    uint32_t pte_enc;     /* Encoding in the HPTE (>>12) */
+};
+typedef struct PPCHash64PageSize PPCHash64PageSize;
+
+struct PPCHash64SegmentPageSizes {
+    uint32_t page_shift;  /* Base page shift of segment (or 0) */
+    uint32_t slb_enc;     /* SLB encoding for BookS */
+    PPCHash64PageSize enc[PPC_PAGE_SIZES_MAX_SZ];
+};
+
+struct PPCHash64Options {
+    PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
+};
+
+extern const PPCHash64Options ppc_hash64_opts_POWER7;
+
 #endif /* CONFIG_USER_ONLY */
 
 #if defined(CONFIG_USER_ONLY) || !defined(TARGET_PPC64)
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index aa63a5dcb3..040d6fbac3 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8368,36 +8368,6 @@ static Property powerpc_servercpu_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-#ifdef CONFIG_SOFTMMU
-static const struct ppc_segment_page_sizes POWER7_POWER8_sps = {
-    .sps = {
-        {
-            .page_shift = 12, /* 4K */
-            .slb_enc = 0,
-            .enc = { { .page_shift = 12, .pte_enc = 0 },
-                     { .page_shift = 16, .pte_enc = 0x7 },
-                     { .page_shift = 24, .pte_enc = 0x38 }, },
-        },
-        {
-            .page_shift = 16, /* 64K */
-            .slb_enc = SLB_VSID_64K,
-            .enc = { { .page_shift = 16, .pte_enc = 0x1 },
-                     { .page_shift = 24, .pte_enc = 0x8 }, },
-        },
-        {
-            .page_shift = 24, /* 16M */
-            .slb_enc = SLB_VSID_16M,
-            .enc = { { .page_shift = 24, .pte_enc = 0 }, },
-        },
-        {
-            .page_shift = 34, /* 16G */
-            .slb_enc = SLB_VSID_16G,
-            .enc = { { .page_shift = 34, .pte_enc = 0x3 }, },
-        },
-    }
-};
-#endif /* CONFIG_SOFTMMU */
-
 static void init_proc_POWER7(CPUPPCState *env)
 {
     /* Common Registers */
@@ -8526,7 +8496,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-    pcc->sps = &POWER7_POWER8_sps;
+    pcc->hash64_opts = &ppc_hash64_opts_POWER7;
 #endif
     pcc->excp_model = POWERPC_EXCP_POWER7;
     pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
@@ -8698,7 +8668,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     pcc->mmu_model = POWERPC_MMU_2_07;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-    pcc->sps = &POWER7_POWER8_sps;
+    pcc->hash64_opts = &ppc_hash64_opts_POWER7;
 #endif
     pcc->excp_model = POWERPC_EXCP_POWER8;
     pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
@@ -8893,7 +8863,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
     /* segment page size remain the same */
-    pcc->sps = &POWER7_POWER8_sps;
+    pcc->hash64_opts = &ppc_hash64_opts_POWER7;
     pcc->radix_page_info = &POWER9_radix_page_info;
 #endif
     pcc->excp_model = POWERPC_EXCP_POWER8;
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 08/13] target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (6 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 07/13] target/ppc: Split page size information into a separate allocation David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options David Gibson
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

Currently some cpus set the hash64_opts field in the class structure, with
specific details of their variant of the 64-bit hash mmu.  For the
remaining cpus with that mmu, ppc_hash64_realize() fills in defaults.

But there are only a couple of cpus that use those fallbacks, so just have
them to set the has64_opts field instead, simplifying the logic.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/mmu-hash64.c     | 36 ++++++++++++++++++------------------
 target/ppc/mmu-hash64.h     |  1 +
 target/ppc/translate_init.c |  2 ++
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 6758afd9de..2809c31170 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1100,25 +1100,12 @@ void ppc_hash64_init(PowerPCCPU *cpu)
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
 
-    if (pcc->hash64_opts) {
-        cpu->hash64_opts = g_memdup(pcc->hash64_opts,
-                                    sizeof(*cpu->hash64_opts));
-    } else if (env->mmu_model & POWERPC_MMU_64) {
-        /* Use default sets of page sizes. We don't support MPSS */
-        static const PPCHash64Options defopts = {
-            .sps = {
-                { .page_shift = 12, /* 4K */
-                  .slb_enc = 0,
-                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
-                },
-                { .page_shift = 24, /* 16M */
-                  .slb_enc = 0x100,
-                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
-                },
-            },
-        };
-        cpu->hash64_opts = g_memdup(&defopts, sizeof(*cpu->hash64_opts));
+    if (!pcc->hash64_opts) {
+        assert(!(env->mmu_model & POWERPC_MMU_64));
+        return;
     }
+
+    cpu->hash64_opts = g_memdup(pcc->hash64_opts, sizeof(*cpu->hash64_opts));
 }
 
 void ppc_hash64_finalize(PowerPCCPU *cpu)
@@ -1126,6 +1113,19 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
     g_free(cpu->hash64_opts);
 }
 
+const PPCHash64Options ppc_hash64_opts_basic = {
+    .sps = {
+        { .page_shift = 12, /* 4K */
+          .slb_enc = 0,
+          .enc = { { .page_shift = 12, .pte_enc = 0 } }
+        },
+        { .page_shift = 24, /* 16M */
+          .slb_enc = 0x100,
+          .enc = { { .page_shift = 24, .pte_enc = 0 } }
+        },
+    },
+};
+
 const PPCHash64Options ppc_hash64_opts_POWER7 = {
     .sps = {
         {
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 957bd68e33..341c1524c2 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -156,6 +156,7 @@ struct PPCHash64Options {
     PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
 };
 
+extern const PPCHash64Options ppc_hash64_opts_basic;
 extern const PPCHash64Options ppc_hash64_opts_POWER7;
 
 #endif /* CONFIG_USER_ONLY */
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 040d6fbac3..ae005b2a54 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8242,6 +8242,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
     pcc->mmu_model = POWERPC_MMU_64B;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
+    pcc->hash64_opts = &ppc_hash64_opts_basic;
 #endif
     pcc->excp_model = POWERPC_EXCP_970;
     pcc->bus_model = PPC_FLAGS_INPUT_970;
@@ -8319,6 +8320,7 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
     pcc->mmu_model = POWERPC_MMU_2_03;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
+    pcc->hash64_opts = &ppc_hash64_opts_basic;
 #endif
     pcc->excp_model = POWERPC_EXCP_970;
     pcc->bus_model = PPC_FLAGS_INPUT_970;
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (7 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 08/13] target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05 12:06   ` Greg Kurz
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 10/13] target/ppc: Fold ci_large_pages flag into PPCHash64Options David Gibson
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

Currently env->mmu_model is a bit of an unholy mess of an enum of distinct
MMU types, with various flag bits as well.  This makes which bits of the
field should be compared pretty confusing.

Make a start on cleaning that up by moving two of the flags bits -
POWERPC_MMU_1TSEG and POWERPC_MMU_AMR - which are specific to the 64-bit
hash MMU into a new flags field in PPCHash64Options structure.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/pnv.c            |  3 ++-
 hw/ppc/spapr.c          |  2 +-
 target/ppc/cpu-qom.h    | 11 +++--------
 target/ppc/kvm.c        |  4 ++--
 target/ppc/mmu-hash64.c |  6 ++++--
 target/ppc/mmu-hash64.h |  8 ++++++++
 6 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 5a79b24828..5905be3f71 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -36,6 +36,7 @@
 #include "monitor/monitor.h"
 #include "hw/intc/intc.h"
 #include "hw/ipmi/ipmi.h"
+#include "target/ppc/mmu-hash64.h"
 
 #include "hw/ppc/xics.h"
 #include "hw/ppc/pnv_xscom.h"
@@ -187,7 +188,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
         _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
     }
 
-    if (env->mmu_model & POWERPC_MMU_1TSEG) {
+    if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
         _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes",
                            segs, sizeof(segs))));
     }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 14c31f82fa..f86cb09080 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -557,7 +557,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
         _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
     }
 
-    if (env->mmu_model & POWERPC_MMU_1TSEG) {
+    if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
         _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes",
                           segs, sizeof(segs))));
     }
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 3e5ef7375f..2bd58b2a84 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -68,22 +68,17 @@ enum powerpc_mmu_t {
     /* PowerPC 601 MMU model (specific BATs format)            */
     POWERPC_MMU_601        = 0x0000000A,
 #define POWERPC_MMU_64       0x00010000
-#define POWERPC_MMU_1TSEG    0x00020000
-#define POWERPC_MMU_AMR      0x00040000
 #define POWERPC_MMU_V3       0x00100000 /* ISA V3.00 MMU Support */
     /* 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_AMR | 0x00000003,
+    POWERPC_MMU_2_06       = POWERPC_MMU_64 | 0x00000003,
     /* Architecture 2.07 variant                               */
-    POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
-                             | POWERPC_MMU_AMR | 0x00000004,
+    POWERPC_MMU_2_07       = POWERPC_MMU_64 | 0x00000004,
     /* Architecture 3.00 variant                               */
-    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
-                             | POWERPC_MMU_AMR | POWERPC_MMU_V3
+    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_V3
                              | 0x00000005,
 };
 #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF))
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index bc6d0a8314..22487cef06 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -302,7 +302,7 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu,
         /* HV KVM has backing store size restrictions */
         info->flags = KVM_PPC_PAGE_SIZES_REAL;
 
-        if (env->mmu_model & POWERPC_MMU_1TSEG) {
+        if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
             info->flags |= KVM_PPC_1T_SEGMENTS;
         }
 
@@ -482,7 +482,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
     }
     env->slb_nr = smmu_info.slb_size;
     if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
-        env->mmu_model &= ~POWERPC_MMU_1TSEG;
+        cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
     }
 }
 
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 2809c31170..c9ee55e1ea 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -160,7 +160,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
     if (vsid & (SLB_VSID_B & ~SLB_VSID_B_1T)) {
         return -1; /* Bad segment size */
     }
-    if ((vsid & SLB_VSID_B) && !(env->mmu_model & POWERPC_MMU_1TSEG)) {
+    if ((vsid & SLB_VSID_B) && !(ppc_hash64_has(cpu, PPC_HASH64_1TSEG))) {
         return -1; /* 1T segment on MMU that doesn't support it */
     }
 
@@ -369,7 +369,7 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte)
     int prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 
     /* Only recent MMUs implement Virtual Page Class Key Protection */
-    if (!(env->mmu_model & POWERPC_MMU_AMR)) {
+    if (!ppc_hash64_has(cpu, PPC_HASH64_AMR)) {
         return prot;
     }
 
@@ -1114,6 +1114,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
 }
 
 const PPCHash64Options ppc_hash64_opts_basic = {
+    .flags = 0,
     .sps = {
         { .page_shift = 12, /* 4K */
           .slb_enc = 0,
@@ -1127,6 +1128,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
 };
 
 const PPCHash64Options ppc_hash64_opts_POWER7 = {
+    .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR,
     .sps = {
         {
             .page_shift = 12, /* 4K */
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 341c1524c2..b2b5d25238 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -153,12 +153,20 @@ struct PPCHash64SegmentPageSizes {
 };
 
 struct PPCHash64Options {
+#define PPC_HASH64_1TSEG        0x00001
+#define PPC_HASH64_AMR          0x00002
+    unsigned flags;
     PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
 };
 
 extern const PPCHash64Options ppc_hash64_opts_basic;
 extern const PPCHash64Options ppc_hash64_opts_POWER7;
 
+static inline bool ppc_hash64_has(PowerPCCPU *cpu, unsigned feature)
+{
+    return !!(cpu->hash64_opts->flags & feature);
+}
+
 #endif /* CONFIG_USER_ONLY */
 
 #if defined(CONFIG_USER_ONLY) || !defined(TARGET_PPC64)
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 10/13] target/ppc: Fold ci_large_pages flag into PPCHash64Options
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (8 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 11/13] target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model David Gibson
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

The ci_large_pages boolean in CPUPPCState is only relevant to 64-bit hash
MMU machines, indicating whether it's possible to map large (> 4kiB) pages
as cache-inhibitied (i.e. for IO, rather than memory).  Fold it as another
flag into the PPCHash64Options structure.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c              | 3 +--
 target/ppc/cpu.h            | 1 -
 target/ppc/kvm.c            | 6 +++++-
 target/ppc/mmu-hash64.c     | 2 +-
 target/ppc/mmu-hash64.h     | 1 +
 target/ppc/translate_init.c | 3 ---
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f86cb09080..60bc8417b6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -263,7 +263,6 @@ static void spapr_populate_pa_features(sPAPRMachineState *spapr,
                                        void *fdt, int offset,
                                        bool legacy_guest)
 {
-    CPUPPCState *env = &cpu->env;
     uint8_t pa_features_206[] = { 6, 0,
         0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
     uint8_t pa_features_207[] = { 24, 0,
@@ -315,7 +314,7 @@ static void spapr_populate_pa_features(sPAPRMachineState *spapr,
         return;
     }
 
-    if (env->ci_large_pages) {
+    if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
         /*
          * Note: we keep CI large pages off by default because a 64K capable
          * guest provisioned with large pages might otherwise try to map a qemu
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 1c5c33ca11..c0c44fb91d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1090,7 +1090,6 @@ struct CPUPPCState {
 #if defined(TARGET_PPC64)
     ppc_slb_t vrma_slb;
     target_ulong rmls;
-    bool ci_large_pages;
 #endif
 
 #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 22487cef06..fff2c601e0 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -448,7 +448,11 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
      * host page size is smaller than 64K.
      */
     if (smmu_info.flags & KVM_PPC_PAGE_SIZES_REAL) {
-        env->ci_large_pages = getpagesize() >= 0x10000;
+        if (getpagesize() >= 0x10000) {
+            cpu->hash64_opts->flags |= PPC_HASH64_CI_LARGEPAGE;
+        } else {
+            cpu->hash64_opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
+        }
     }
 
     /*
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index c9ee55e1ea..f341714550 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1128,7 +1128,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
 };
 
 const PPCHash64Options ppc_hash64_opts_POWER7 = {
-    .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR,
+    .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
     .sps = {
         {
             .page_shift = 12, /* 4K */
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index b2b5d25238..f1babb0afc 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -155,6 +155,7 @@ struct PPCHash64SegmentPageSizes {
 struct PPCHash64Options {
 #define PPC_HASH64_1TSEG        0x00001
 #define PPC_HASH64_AMR          0x00002
+#define PPC_HASH64_CI_LARGEPAGE 0x00004
     unsigned flags;
     PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
 };
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index ae005b2a54..a925cf5cd3 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8392,7 +8392,6 @@ static void init_proc_POWER7(CPUPPCState *env)
 #if !defined(CONFIG_USER_ONLY)
     env->slb_nr = 32;
 #endif
-    env->ci_large_pages = true;
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8547,7 +8546,6 @@ static void init_proc_POWER8(CPUPPCState *env)
 #if !defined(CONFIG_USER_ONLY)
     env->slb_nr = 32;
 #endif
-    env->ci_large_pages = true;
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8748,7 +8746,6 @@ static void init_proc_POWER9(CPUPPCState *env)
 #if !defined(CONFIG_USER_ONLY)
     env->slb_nr = 32;
 #endif
-    env->ci_large_pages = true;
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 11/13] target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (9 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 10/13] target/ppc: Fold ci_large_pages flag into PPCHash64Options David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 12/13] target/ppc: Get rid of POWERPC_MMU_VER() macros David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options David Gibson
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

The only place we test this flag is in conjunction with
ppc64_use_proc_tbl().  That checks for the LPCR_UPRT bit, which we already
ensure can't be set except on a machine with a v3 MMU (i.e. POWER9).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/cpu-qom.h    | 4 +---
 target/ppc/mmu-hash64.c | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 2bd58b2a84..ef96d42cf2 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -68,7 +68,6 @@ enum powerpc_mmu_t {
     /* PowerPC 601 MMU model (specific BATs format)            */
     POWERPC_MMU_601        = 0x0000000A,
 #define POWERPC_MMU_64       0x00010000
-#define POWERPC_MMU_V3       0x00100000 /* ISA V3.00 MMU Support */
     /* 64 bits PowerPC MMU                                     */
     POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
     /* Architecture 2.03 and later (has LPCR) */
@@ -78,8 +77,7 @@ enum powerpc_mmu_t {
     /* Architecture 2.07 variant                               */
     POWERPC_MMU_2_07       = POWERPC_MMU_64 | 0x00000004,
     /* Architecture 3.00 variant                               */
-    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_V3
-                             | 0x00000005,
+    POWERPC_MMU_3_00       = POWERPC_MMU_64 | 0x00000005,
 };
 #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF))
 #define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B)
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index f341714550..df26a03c15 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -761,7 +761,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
     slb = slb_lookup(cpu, eaddr);
     if (!slb) {
         /* No entry found, check if in-memory segment tables are in use */
-        if ((env->mmu_model & POWERPC_MMU_V3) && ppc64_use_proc_tbl(cpu)) {
+        if (ppc64_use_proc_tbl(cpu)) {
             /* TODO - Unsupported */
             error_report("Segment Table Support Unimplemented");
             exit(1);
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 12/13] target/ppc: Get rid of POWERPC_MMU_VER() macros
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (10 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 11/13] target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options David Gibson
  12 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

These macros were introduced to deal with the fact that the mmu_model
field has bit flags mixed in with what's otherwise an enum of various mmu
types.

We've now eliminated all those flags except for one, and that one -
POWERPC_MMU_64 - is already included/compared in the MMU_VER macros.  So,
we can get rid of those macros and just directly compare mmu_model values
in the places it was used.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/cpu-qom.h    |  6 ------
 target/ppc/kvm.c        |  8 ++++----
 target/ppc/mmu-hash64.c | 12 ++++++------
 target/ppc/mmu_helper.c | 24 ++++++++++++------------
 target/ppc/translate.c  | 12 ++++++------
 5 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index ef96d42cf2..433a71e484 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -79,12 +79,6 @@ enum powerpc_mmu_t {
     /* Architecture 3.00 variant                               */
     POWERPC_MMU_3_00       = POWERPC_MMU_64 | 0x00000005,
 };
-#define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF))
-#define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B)
-#define POWERPC_MMU_VER_2_03 POWERPC_MMU_VER(POWERPC_MMU_2_03)
-#define POWERPC_MMU_VER_2_06 POWERPC_MMU_VER(POWERPC_MMU_2_06)
-#define POWERPC_MMU_VER_2_07 POWERPC_MMU_VER(POWERPC_MMU_2_07)
-#define POWERPC_MMU_VER_3_00 POWERPC_MMU_VER(POWERPC_MMU_3_00)
 
 /*****************************************************************************/
 /* Exception model                                                           */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index fff2c601e0..b329cd8173 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -306,8 +306,8 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu,
             info->flags |= KVM_PPC_1T_SEGMENTS;
         }
 
-        if (POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_06 ||
-           POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_07) {
+        if (env->mmu_model == POWERPC_MMU_2_06 ||
+            env->mmu_model == POWERPC_MMU_2_07) {
             info->slb_size = 32;
         } else {
             info->slb_size = 64;
@@ -321,8 +321,8 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu,
         i++;
 
         /* 64K on MMU 2.06 and later */
-        if (POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_06 ||
-            POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_07) {
+        if (env->mmu_model == POWERPC_MMU_2_06 ||
+            env->mmu_model == POWERPC_MMU_2_07) {
             info->sps[i].page_shift = 16;
             info->sps[i].slb_enc = 0x110;
             info->sps[i].enc[0].page_shift = 16;
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index df26a03c15..a5570c8774 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1033,8 +1033,8 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val)
     uint64_t lpcr = 0;
 
     /* Filter out bits */
-    switch (POWERPC_MMU_VER(env->mmu_model)) {
-    case POWERPC_MMU_VER_64B: /* 970 */
+    switch (env->mmu_model) {
+    case POWERPC_MMU_64B: /* 970 */
         if (val & 0x40) {
             lpcr |= LPCR_LPES0;
         }
@@ -1060,26 +1060,26 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val)
          * to dig HRMOR out of HID5
          */
         break;
-    case POWERPC_MMU_VER_2_03: /* P5p */
+    case POWERPC_MMU_2_03: /* P5p */
         lpcr = val & (LPCR_RMLS | LPCR_ILE |
                       LPCR_LPES0 | LPCR_LPES1 |
                       LPCR_RMI | LPCR_HDICE);
         break;
-    case POWERPC_MMU_VER_2_06: /* P7 */
+    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_VER_2_07: /* P8 */
+    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;
-    case POWERPC_MMU_VER_3_00: /* P9 */
+    case POWERPC_MMU_3_00: /* P9 */
         lpcr = val & (LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
                       (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
                       LPCR_UPRT | LPCR_EVIRT | LPCR_ONL |
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 5568d1642b..8075b7149a 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1266,7 +1266,7 @@ static void mmu6xx_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
 
 void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env)
 {
-    switch (POWERPC_MMU_VER(env->mmu_model)) {
+    switch (env->mmu_model) {
     case POWERPC_MMU_BOOKE:
         mmubooke_dump_mmu(f, cpu_fprintf, env);
         break;
@@ -1278,13 +1278,13 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env)
         mmu6xx_dump_mmu(f, cpu_fprintf, env);
         break;
 #if defined(TARGET_PPC64)
-    case POWERPC_MMU_VER_64B:
-    case POWERPC_MMU_VER_2_03:
-    case POWERPC_MMU_VER_2_06:
-    case POWERPC_MMU_VER_2_07:
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_03:
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_07:
         dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env));
         break;
-    case POWERPC_MMU_VER_3_00:
+    case POWERPC_MMU_3_00:
         if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
             /* TODO - Unsupported */
         } else {
@@ -1423,14 +1423,14 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     CPUPPCState *env = &cpu->env;
     mmu_ctx_t ctx;
 
-    switch (POWERPC_MMU_VER(env->mmu_model)) {
+    switch (env->mmu_model) {
 #if defined(TARGET_PPC64)
-    case POWERPC_MMU_VER_64B:
-    case POWERPC_MMU_VER_2_03:
-    case POWERPC_MMU_VER_2_06:
-    case POWERPC_MMU_VER_2_07:
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_03:
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_07:
         return ppc_hash64_get_phys_page_debug(cpu, addr);
-    case POWERPC_MMU_VER_3_00:
+    case POWERPC_MMU_3_00:
         if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
             return ppc_radix64_get_phys_page_debug(cpu, addr);
         } else {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b0d79a3fb9..3beaa1e2f0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7121,17 +7121,17 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
     if (env->spr_cb[SPR_LPCR].name)
         cpu_fprintf(f, " LPCR " TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
 
-    switch (POWERPC_MMU_VER(env->mmu_model)) {
+    switch (env->mmu_model) {
     case POWERPC_MMU_32B:
     case POWERPC_MMU_601:
     case POWERPC_MMU_SOFT_6xx:
     case POWERPC_MMU_SOFT_74xx:
 #if defined(TARGET_PPC64)
-    case POWERPC_MMU_VER_64B:
-    case POWERPC_MMU_VER_2_03:
-    case POWERPC_MMU_VER_2_06:
-    case POWERPC_MMU_VER_2_07:
-    case POWERPC_MMU_VER_3_00:
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_03:
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_07:
+    case POWERPC_MMU_3_00:
 #endif
         if (env->spr_cb[SPR_SDR1].name) { /* SDR1 Exists */
             cpu_fprintf(f, " SDR1 " TARGET_FMT_lx " ", env->spr[SPR_SDR1]);
-- 
2.14.3

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

* [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
  2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
                   ` (11 preceding siblings ...)
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 12/13] target/ppc: Get rid of POWERPC_MMU_VER() macros David Gibson
@ 2018-04-05  2:14 ` David Gibson
  2018-04-05 13:12   ` Greg Kurz
  12 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2018-04-05  2:14 UTC (permalink / raw)
  To: qemu-ppc, groug; +Cc: qemu-devel, clg, bharata, David Gibson

The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
This is another static-after-initialization parameter of the specific
version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
into PPCHash64Options with the other hash MMU options.

This is a bit more complicated that the things previously put in there,
because slb_nr was foolishly included in the migration stream.  So we need
some of the usual dance to handle backwards compatible migration.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/pnv.c                |  2 +-
 hw/ppc/spapr.c              | 11 ++++++++---
 target/ppc/cpu.h            |  3 ++-
 target/ppc/kvm.c            |  2 +-
 target/ppc/machine.c        | 23 ++++++++++++++++++++---
 target/ppc/mmu-hash64.c     | 15 +++++++++------
 target/ppc/mmu-hash64.h     |  1 +
 target/ppc/translate_init.c | 15 ---------------
 8 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 5905be3f71..53f672afa8 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
     _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
-    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
+    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
     _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
     _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 60bc8417b6..6021631722 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
     _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
-    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
-    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
+    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
+    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
     _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
     _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
@@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
  * pseries-2.12
  */
 #define SPAPR_COMPAT_2_12                                              \
-    HW_COMPAT_2_12
+    HW_COMPAT_2_12                                                     \
+    {                                                                  \
+        .driver = TYPE_POWERPC_CPU,                                    \
+            .property = "pre-2.13-migration",                          \
+            .value    = "on",                                          \
+    },
 
 static void spapr_machine_2_12_instance_options(MachineState *machine)
 {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c0c44fb91d..8c9e03f54d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1025,7 +1025,6 @@ struct CPUPPCState {
 #if defined(TARGET_PPC64)
     /* PowerPC 64 SLB area */
     ppc_slb_t slb[MAX_SLB_ENTRIES];
-    int32_t slb_nr;
     /* tcg TLB needs flush (deferred slb inval instruction typically) */
 #endif
     /* segment registers */
@@ -1216,6 +1215,8 @@ struct PowerPCCPU {
     uint64_t mig_insns_flags2;
     uint32_t mig_nb_BATs;
     bool pre_2_10_migration;
+    bool pre_2_13_migration;
+    int32_t mig_slb_nr;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index b329cd8173..1bd38c6a90 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
             break;
         }
     }
-    env->slb_nr = smmu_info.slb_size;
+    cpu->hash64_opts->slb_size = smmu_info.slb_size;
     if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
         cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
     }
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 0634cdb295..3d6434a006 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     unsigned int i, j;
     target_ulong sdr1;
     uint32_t fpscr;
+#if defined(TARGET_PPC64)
+    int32_t slb_nr;
+#endif
     target_ulong xer;
 
     for (i = 0; i < 32; i++)
@@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     qemu_get_sbe32s(f, &env->access_type);
 #if defined(TARGET_PPC64)
     qemu_get_betls(f, &env->spr[SPR_ASR]);
-    qemu_get_sbe32s(f, &env->slb_nr);
+    qemu_get_sbe32s(f, &slb_nr);
 #endif
     qemu_get_betls(f, &sdr1);
     for (i = 0; i < 32; i++)
@@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)
     return cpu->pre_2_8_migration;
 }
 
+#if defined(TARGET_PPC64)
+static bool cpu_pre_2_13_migration(void *opaque, int version_id)
+{
+    PowerPCCPU *cpu = opaque;
+
+    return cpu->pre_2_13_migration;
+}
+#endif
+
 static int cpu_pre_save(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -203,6 +215,11 @@ static int cpu_pre_save(void *opaque)
         cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
         cpu->mig_nb_BATs = env->nb_BATs;
     }
+    if (cpu->pre_2_13_migration) {
+        if (cpu->hash64_opts) {
+            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
+        }
+    }
 
     return 0;
 }
@@ -478,7 +495,7 @@ static int slb_post_load(void *opaque, int version_id)
 
     /* We've pulled in the raw esid and vsid values from the migration
      * stream, but we need to recompute the page size pointers */
-    for (i = 0; i < env->slb_nr; i++) {
+    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
         if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
             /* Migration source had bad values in its SLB */
             return -1;
@@ -495,7 +512,7 @@ static const VMStateDescription vmstate_slb = {
     .needed = slb_needed,
     .post_load = slb_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
+        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
         VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
         VMSTATE_END_OF_LIST()
     }
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index a5570c8774..7e0adecfd9 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -52,7 +52,7 @@ static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
     esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
     esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
 
-    for (n = 0; n < env->slb_nr; n++) {
+    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         LOG_SLB("%s: slot %d %016" PRIx64 " %016"
@@ -80,7 +80,7 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
     cpu_synchronize_state(CPU(cpu));
 
     cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
-    for (i = 0; i < env->slb_nr; i++) {
+    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
         slbe = env->slb[i].esid;
         slbv = env->slb[i].vsid;
         if (slbe == 0 && slbv == 0) {
@@ -93,10 +93,11 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
 
 void helper_slbia(CPUPPCState *env)
 {
+    PowerPCCPU *cpu = ppc_env_get_cpu(env);
     int n;
 
     /* XXX: Warning: slbia never invalidates the first segment */
-    for (n = 1; n < env->slb_nr; n++) {
+    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         if (slb->esid & SLB_ESID_V) {
@@ -151,7 +152,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
     const PPCHash64SegmentPageSizes *sps = NULL;
     int i;
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1; /* Bad slot number */
     }
     if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
@@ -202,7 +203,7 @@ static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
     int slot = rb & 0xfff;
     ppc_slb_t *slb = &env->slb[slot];
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1;
     }
 
@@ -217,7 +218,7 @@ static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
     int slot = rb & 0xfff;
     ppc_slb_t *slb = &env->slb[slot];
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1;
     }
 
@@ -1115,6 +1116,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
 
 const PPCHash64Options ppc_hash64_opts_basic = {
     .flags = 0,
+    .slb_size = 64,
     .sps = {
         { .page_shift = 12, /* 4K */
           .slb_enc = 0,
@@ -1129,6 +1131,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
 
 const PPCHash64Options ppc_hash64_opts_POWER7 = {
     .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
+    .slb_size = 32,
     .sps = {
         {
             .page_shift = 12, /* 4K */
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index f1babb0afc..d5fc03441d 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -157,6 +157,7 @@ struct PPCHash64Options {
 #define PPC_HASH64_AMR          0x00002
 #define PPC_HASH64_CI_LARGEPAGE 0x00004
     unsigned flags;
+    unsigned slb_size;
     PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
 };
 
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index a925cf5cd3..2f63acd310 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8195,9 +8195,6 @@ static void init_proc_970(CPUPPCState *env)
     gen_spr_970_dbg(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 64;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8272,9 +8269,6 @@ static void init_proc_power5plus(CPUPPCState *env)
     gen_spr_power5p_ear(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 64;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8389,9 +8383,6 @@ static void init_proc_POWER7(CPUPPCState *env)
     gen_spr_power7_book4(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8543,9 +8534,6 @@ static void init_proc_POWER8(CPUPPCState *env)
     gen_spr_power8_rpr(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8743,9 +8731,6 @@ static void init_proc_POWER9(CPUPPCState *env)
                         KVM_REG_PPC_PSSCR, 0);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
-- 
2.14.3

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

* Re: [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options David Gibson
@ 2018-04-05 12:06   ` Greg Kurz
  2018-04-06  4:58     ` David Gibson
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kurz @ 2018-04-05 12:06 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, clg, bharata

On Thu,  5 Apr 2018 12:14:33 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently env->mmu_model is a bit of an unholy mess of an enum of distinct
> MMU types, with various flag bits as well.  This makes which bits of the
> field should be compared pretty confusing.
> 
> Make a start on cleaning that up by moving two of the flags bits -
> POWERPC_MMU_1TSEG and POWERPC_MMU_AMR - which are specific to the 64-bit
> hash MMU into a new flags field in PPCHash64Options structure.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---

While investigating a migration failure from an older QEMU, I realized
this patch has a problem. The *cpu->hash64_opts structure is zeroed in
kvm_fixup_page_sizes(), which has now the unwanted effect of clearing
the cpu->hash64_opts->flags as well.

We only need to zero the segment page sizes actually. The following
fixes migration:

@@ -442,7 +442,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
     }
 
     /* Convert to QEMU form */
-    memset(cpu->hash64_opts, 0, sizeof(*cpu->hash64_opts));
+    memset(&cpu->hash64_opts->sps, 0, sizeof(cpu->hash64_opts->sps));
 
     /* If we have HV KVM, we need to forbid CI large pages if our
      * host page size is smaller than 64K.

>  hw/ppc/pnv.c            |  3 ++-
>  hw/ppc/spapr.c          |  2 +-
>  target/ppc/cpu-qom.h    | 11 +++--------
>  target/ppc/kvm.c        |  4 ++--
>  target/ppc/mmu-hash64.c |  6 ++++--
>  target/ppc/mmu-hash64.h |  8 ++++++++
>  6 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 5a79b24828..5905be3f71 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -36,6 +36,7 @@
>  #include "monitor/monitor.h"
>  #include "hw/intc/intc.h"
>  #include "hw/ipmi/ipmi.h"
> +#include "target/ppc/mmu-hash64.h"
>  
>  #include "hw/ppc/xics.h"
>  #include "hw/ppc/pnv_xscom.h"
> @@ -187,7 +188,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>          _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
>      }
>  
> -    if (env->mmu_model & POWERPC_MMU_1TSEG) {
> +    if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
>          _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes",
>                             segs, sizeof(segs))));
>      }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 14c31f82fa..f86cb09080 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -557,7 +557,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>          _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
>      }
>  
> -    if (env->mmu_model & POWERPC_MMU_1TSEG) {
> +    if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
>          _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes",
>                            segs, sizeof(segs))));
>      }
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 3e5ef7375f..2bd58b2a84 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -68,22 +68,17 @@ enum powerpc_mmu_t {
>      /* PowerPC 601 MMU model (specific BATs format)            */
>      POWERPC_MMU_601        = 0x0000000A,
>  #define POWERPC_MMU_64       0x00010000
> -#define POWERPC_MMU_1TSEG    0x00020000
> -#define POWERPC_MMU_AMR      0x00040000
>  #define POWERPC_MMU_V3       0x00100000 /* ISA V3.00 MMU Support */
>      /* 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_AMR | 0x00000003,
> +    POWERPC_MMU_2_06       = POWERPC_MMU_64 | 0x00000003,
>      /* Architecture 2.07 variant                               */
> -    POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> -                             | POWERPC_MMU_AMR | 0x00000004,
> +    POWERPC_MMU_2_07       = POWERPC_MMU_64 | 0x00000004,
>      /* Architecture 3.00 variant                               */
> -    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> -                             | POWERPC_MMU_AMR | POWERPC_MMU_V3
> +    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_V3
>                               | 0x00000005,
>  };
>  #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF))
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index bc6d0a8314..22487cef06 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -302,7 +302,7 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu,
>          /* HV KVM has backing store size restrictions */
>          info->flags = KVM_PPC_PAGE_SIZES_REAL;
>  
> -        if (env->mmu_model & POWERPC_MMU_1TSEG) {
> +        if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
>              info->flags |= KVM_PPC_1T_SEGMENTS;
>          }
>  
> @@ -482,7 +482,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>      }
>      env->slb_nr = smmu_info.slb_size;
>      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
> -        env->mmu_model &= ~POWERPC_MMU_1TSEG;
> +        cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
>      }
>  }
>  
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 2809c31170..c9ee55e1ea 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -160,7 +160,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>      if (vsid & (SLB_VSID_B & ~SLB_VSID_B_1T)) {
>          return -1; /* Bad segment size */
>      }
> -    if ((vsid & SLB_VSID_B) && !(env->mmu_model & POWERPC_MMU_1TSEG)) {
> +    if ((vsid & SLB_VSID_B) && !(ppc_hash64_has(cpu, PPC_HASH64_1TSEG))) {
>          return -1; /* 1T segment on MMU that doesn't support it */
>      }
>  
> @@ -369,7 +369,7 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte)
>      int prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>  
>      /* Only recent MMUs implement Virtual Page Class Key Protection */
> -    if (!(env->mmu_model & POWERPC_MMU_AMR)) {
> +    if (!ppc_hash64_has(cpu, PPC_HASH64_AMR)) {
>          return prot;
>      }
>  
> @@ -1114,6 +1114,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
>  }
>  
>  const PPCHash64Options ppc_hash64_opts_basic = {
> +    .flags = 0,
>      .sps = {
>          { .page_shift = 12, /* 4K */
>            .slb_enc = 0,
> @@ -1127,6 +1128,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
>  };
>  
>  const PPCHash64Options ppc_hash64_opts_POWER7 = {
> +    .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR,
>      .sps = {
>          {
>              .page_shift = 12, /* 4K */
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 341c1524c2..b2b5d25238 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -153,12 +153,20 @@ struct PPCHash64SegmentPageSizes {
>  };
>  
>  struct PPCHash64Options {
> +#define PPC_HASH64_1TSEG        0x00001
> +#define PPC_HASH64_AMR          0x00002
> +    unsigned flags;
>      PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
>  };
>  
>  extern const PPCHash64Options ppc_hash64_opts_basic;
>  extern const PPCHash64Options ppc_hash64_opts_POWER7;
>  
> +static inline bool ppc_hash64_has(PowerPCCPU *cpu, unsigned feature)
> +{
> +    return !!(cpu->hash64_opts->flags & feature);
> +}
> +
>  #endif /* CONFIG_USER_ONLY */
>  
>  #if defined(CONFIG_USER_ONLY) || !defined(TARGET_PPC64)

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

* Re: [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
  2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options David Gibson
@ 2018-04-05 13:12   ` Greg Kurz
  2018-04-05 13:27     ` Cornelia Huck
  2018-04-06  0:49     ` David Gibson
  0 siblings, 2 replies; 20+ messages in thread
From: Greg Kurz @ 2018-04-05 13:12 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, clg, bharata, Cornelia Huck

On Thu,  5 Apr 2018 12:14:37 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
> This is another static-after-initialization parameter of the specific
> version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
> into PPCHash64Options with the other hash MMU options.
> 
> This is a bit more complicated that the things previously put in there,
> because slb_nr was foolishly included in the migration stream.  So we need
> some of the usual dance to handle backwards compatible migration.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/pnv.c                |  2 +-
>  hw/ppc/spapr.c              | 11 ++++++++---
>  target/ppc/cpu.h            |  3 ++-
>  target/ppc/kvm.c            |  2 +-
>  target/ppc/machine.c        | 23 ++++++++++++++++++++---
>  target/ppc/mmu-hash64.c     | 15 +++++++++------
>  target/ppc/mmu-hash64.h     |  1 +
>  target/ppc/translate_init.c | 15 ---------------
>  8 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 5905be3f71..53f672afa8 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 60bc8417b6..6021631722 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
>   * pseries-2.12
>   */
>  #define SPAPR_COMPAT_2_12                                              \
> -    HW_COMPAT_2_12

This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...

It looks like a patch to introduce the 2.13 machine type is missing.

FWIW, Connie has already queued a patch to do so for s390x, that also
introduces HW_COMPAT_2_12.

https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951

Maybe the HW_COMPAT_ macros should be added in a standalone patch ?

Cc'ing Connie for insights.

> +    HW_COMPAT_2_12                                                     \
> +    {                                                                  \
> +        .driver = TYPE_POWERPC_CPU,                                    \
> +            .property = "pre-2.13-migration",                          \
> +            .value    = "on",                                          \

indentation ?

Also, this property must be added to the TYPE_POWERPC_CPU class.

--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10471,6 +10471,8 @@ static Property ppc_cpu_properties[] = {
     DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
     DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
                      false),
+    DEFINE_PROP_BOOL("pre-2.13-migration", PowerPCCPU, pre_2_13_migration,
+                     false),
     DEFINE_PROP_END_OF_LIST(),
 };

Appart from that, the patch looks good, so:

Reviewed-by: Greg Kurz <groug@kaod.org>

With all the above points addressed, I could successfully migrate from an
older QEMU and back, so:

Tested-by: Greg Kurz <groug@kaod.org>

> +    },
>  
>  static void spapr_machine_2_12_instance_options(MachineState *machine)
>  {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index c0c44fb91d..8c9e03f54d 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1025,7 +1025,6 @@ struct CPUPPCState {
>  #if defined(TARGET_PPC64)
>      /* PowerPC 64 SLB area */
>      ppc_slb_t slb[MAX_SLB_ENTRIES];
> -    int32_t slb_nr;
>      /* tcg TLB needs flush (deferred slb inval instruction typically) */
>  #endif
>      /* segment registers */
> @@ -1216,6 +1215,8 @@ struct PowerPCCPU {
>      uint64_t mig_insns_flags2;
>      uint32_t mig_nb_BATs;
>      bool pre_2_10_migration;
> +    bool pre_2_13_migration;
> +    int32_t mig_slb_nr;
>  };
>  
>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b329cd8173..1bd38c6a90 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>              break;
>          }
>      }
> -    env->slb_nr = smmu_info.slb_size;
> +    cpu->hash64_opts->slb_size = smmu_info.slb_size;
>      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
>          cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
>      }
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 0634cdb295..3d6434a006 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
>      unsigned int i, j;
>      target_ulong sdr1;
>      uint32_t fpscr;
> +#if defined(TARGET_PPC64)
> +    int32_t slb_nr;
> +#endif
>      target_ulong xer;
>  
>      for (i = 0; i < 32; i++)
> @@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
>      qemu_get_sbe32s(f, &env->access_type);
>  #if defined(TARGET_PPC64)
>      qemu_get_betls(f, &env->spr[SPR_ASR]);
> -    qemu_get_sbe32s(f, &env->slb_nr);
> +    qemu_get_sbe32s(f, &slb_nr);
>  #endif
>      qemu_get_betls(f, &sdr1);
>      for (i = 0; i < 32; i++)
> @@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)
>      return cpu->pre_2_8_migration;
>  }
>  
> +#if defined(TARGET_PPC64)
> +static bool cpu_pre_2_13_migration(void *opaque, int version_id)
> +{
> +    PowerPCCPU *cpu = opaque;
> +
> +    return cpu->pre_2_13_migration;
> +}
> +#endif
> +
>  static int cpu_pre_save(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -203,6 +215,11 @@ static int cpu_pre_save(void *opaque)
>          cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
>          cpu->mig_nb_BATs = env->nb_BATs;
>      }
> +    if (cpu->pre_2_13_migration) {
> +        if (cpu->hash64_opts) {
> +            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
> +        }
> +    }
>  
>      return 0;
>  }
> @@ -478,7 +495,7 @@ static int slb_post_load(void *opaque, int version_id)
>  
>      /* We've pulled in the raw esid and vsid values from the migration
>       * stream, but we need to recompute the page size pointers */
> -    for (i = 0; i < env->slb_nr; i++) {
> +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
>          if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
>              /* Migration source had bad values in its SLB */
>              return -1;
> @@ -495,7 +512,7 @@ static const VMStateDescription vmstate_slb = {
>      .needed = slb_needed,
>      .post_load = slb_post_load,
>      .fields = (VMStateField[]) {
> -        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
> +        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
>          VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
>          VMSTATE_END_OF_LIST()
>      }
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index a5570c8774..7e0adecfd9 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -52,7 +52,7 @@ static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
>      esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
>      esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
>  
> -    for (n = 0; n < env->slb_nr; n++) {
> +    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
>          ppc_slb_t *slb = &env->slb[n];
>  
>          LOG_SLB("%s: slot %d %016" PRIx64 " %016"
> @@ -80,7 +80,7 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
>      cpu_synchronize_state(CPU(cpu));
>  
>      cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> -    for (i = 0; i < env->slb_nr; i++) {
> +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
>          slbe = env->slb[i].esid;
>          slbv = env->slb[i].vsid;
>          if (slbe == 0 && slbv == 0) {
> @@ -93,10 +93,11 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
>  
>  void helper_slbia(CPUPPCState *env)
>  {
> +    PowerPCCPU *cpu = ppc_env_get_cpu(env);
>      int n;
>  
>      /* XXX: Warning: slbia never invalidates the first segment */
> -    for (n = 1; n < env->slb_nr; n++) {
> +    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
>          ppc_slb_t *slb = &env->slb[n];
>  
>          if (slb->esid & SLB_ESID_V) {
> @@ -151,7 +152,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>      const PPCHash64SegmentPageSizes *sps = NULL;
>      int i;
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1; /* Bad slot number */
>      }
>      if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
> @@ -202,7 +203,7 @@ static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
>      int slot = rb & 0xfff;
>      ppc_slb_t *slb = &env->slb[slot];
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1;
>      }
>  
> @@ -217,7 +218,7 @@ static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
>      int slot = rb & 0xfff;
>      ppc_slb_t *slb = &env->slb[slot];
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1;
>      }
>  
> @@ -1115,6 +1116,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
>  
>  const PPCHash64Options ppc_hash64_opts_basic = {
>      .flags = 0,
> +    .slb_size = 64,
>      .sps = {
>          { .page_shift = 12, /* 4K */
>            .slb_enc = 0,
> @@ -1129,6 +1131,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
>  
>  const PPCHash64Options ppc_hash64_opts_POWER7 = {
>      .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
> +    .slb_size = 32,
>      .sps = {
>          {
>              .page_shift = 12, /* 4K */
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index f1babb0afc..d5fc03441d 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -157,6 +157,7 @@ struct PPCHash64Options {
>  #define PPC_HASH64_AMR          0x00002
>  #define PPC_HASH64_CI_LARGEPAGE 0x00004
>      unsigned flags;
> +    unsigned slb_size;
>      PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
>  };
>  
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index a925cf5cd3..2f63acd310 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8195,9 +8195,6 @@ static void init_proc_970(CPUPPCState *env)
>      gen_spr_970_dbg(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 64;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8272,9 +8269,6 @@ static void init_proc_power5plus(CPUPPCState *env)
>      gen_spr_power5p_ear(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 64;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8389,9 +8383,6 @@ static void init_proc_POWER7(CPUPPCState *env)
>      gen_spr_power7_book4(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8543,9 +8534,6 @@ static void init_proc_POWER8(CPUPPCState *env)
>      gen_spr_power8_rpr(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8743,9 +8731,6 @@ static void init_proc_POWER9(CPUPPCState *env)
>                          KVM_REG_PPC_PSSCR, 0);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  

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

* Re: [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
  2018-04-05 13:12   ` Greg Kurz
@ 2018-04-05 13:27     ` Cornelia Huck
  2018-04-06  1:09       ` David Gibson
  2018-04-06  0:49     ` David Gibson
  1 sibling, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-04-05 13:27 UTC (permalink / raw)
  To: Greg Kurz; +Cc: David Gibson, qemu-ppc, qemu-devel, clg, bharata

On Thu, 5 Apr 2018 15:12:55 +0200
Greg Kurz <groug@kaod.org> wrote:

> On Thu,  5 Apr 2018 12:14:37 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:

> > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> >   * pseries-2.12
> >   */
> >  #define SPAPR_COMPAT_2_12                                              \
> > -    HW_COMPAT_2_12  
> 
> This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...
> 
> It looks like a patch to introduce the 2.13 machine type is missing.
> 
> FWIW, Connie has already queued a patch to do so for s390x, that also
> introduces HW_COMPAT_2_12.
> 
> https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> 
> Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> 
> Cc'ing Connie for insights.
> 
> > +    HW_COMPAT_2_12                                                     \
> > +    {                                                                  \
> > +        .driver = TYPE_POWERPC_CPU,                                    \
> > +            .property = "pre-2.13-migration",                          \
> > +            .value    = "on",                                          \  

I think the usual procedure is

- every arch that uses compat machines queues a patch that creates the
  new compat machine(s) and adds an empty HW_COMPAT_<version>
- whoever has their queue pulled first wins wrt hw_compat

So, I'm happy with anyone adding the empty HW_COMPAT_2_12 -- it needn't
be me :)

[We could also introduce the 2.13 machines for all architectures in one
sweep, but I think that would be generating needless churn for arch
maintainers.]

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

* Re: [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
  2018-04-05 13:12   ` Greg Kurz
  2018-04-05 13:27     ` Cornelia Huck
@ 2018-04-06  0:49     ` David Gibson
  1 sibling, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-06  0:49 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, qemu-devel, clg, bharata, Cornelia Huck

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

On Thu, Apr 05, 2018 at 03:12:55PM +0200, Greg Kurz wrote:
> On Thu,  5 Apr 2018 12:14:37 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
> > This is another static-after-initialization parameter of the specific
> > version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
> > into PPCHash64Options with the other hash MMU options.
> > 
> > This is a bit more complicated that the things previously put in there,
> > because slb_nr was foolishly included in the migration stream.  So we need
> > some of the usual dance to handle backwards compatible migration.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/pnv.c                |  2 +-
> >  hw/ppc/spapr.c              | 11 ++++++++---
> >  target/ppc/cpu.h            |  3 ++-
> >  target/ppc/kvm.c            |  2 +-
> >  target/ppc/machine.c        | 23 ++++++++++++++++++++---
> >  target/ppc/mmu-hash64.c     | 15 +++++++++------
> >  target/ppc/mmu-hash64.h     |  1 +
> >  target/ppc/translate_init.c | 15 ---------------
> >  8 files changed, 42 insertions(+), 30 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 5905be3f71..53f672afa8 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 60bc8417b6..6021631722 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> >   * pseries-2.12
> >   */
> >  #define SPAPR_COMPAT_2_12                                              \
> > -    HW_COMPAT_2_12
> 
> This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...

Uh.. I think you need to pull the ppc-for-2.13 brnch again, then..

> It looks like a patch to introduce the 2.13 machine type is missing.

..since I added a patch to do exactly that.

> FWIW, Connie has already queued a patch to do so for s390x, that also
> introduces HW_COMPAT_2_12.
> 
> https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> 
> Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> 
> Cc'ing Connie for insights.
> 
> > +    HW_COMPAT_2_12                                                     \
> > +    {                                                                  \
> > +        .driver = TYPE_POWERPC_CPU,                                    \
> > +            .property = "pre-2.13-migration",                          \
> > +            .value    = "on",                                          \
> 
> indentation ?

Oops, adjusted.

> 
> Also, this property must be added to the TYPE_POWERPC_CPU class.
> 
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -10471,6 +10471,8 @@ static Property ppc_cpu_properties[] = {
>      DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
>      DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
>                       false),
> +    DEFINE_PROP_BOOL("pre-2.13-migration", PowerPCCPU, pre_2_13_migration,
> +                     false),
>      DEFINE_PROP_END_OF_LIST(),
>  };

Oops again, adjusted.

> Appart from that, the patch looks good, so:
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> With all the above points addressed, I could successfully migrate from an
> older QEMU and back, so:
> 
> Tested-by: Greg Kurz <groug@kaod.org>
> 
> > +    },
> >  
> >  static void spapr_machine_2_12_instance_options(MachineState *machine)
> >  {
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index c0c44fb91d..8c9e03f54d 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1025,7 +1025,6 @@ struct CPUPPCState {
> >  #if defined(TARGET_PPC64)
> >      /* PowerPC 64 SLB area */
> >      ppc_slb_t slb[MAX_SLB_ENTRIES];
> > -    int32_t slb_nr;
> >      /* tcg TLB needs flush (deferred slb inval instruction typically) */
> >  #endif
> >      /* segment registers */
> > @@ -1216,6 +1215,8 @@ struct PowerPCCPU {
> >      uint64_t mig_insns_flags2;
> >      uint32_t mig_nb_BATs;
> >      bool pre_2_10_migration;
> > +    bool pre_2_13_migration;
> > +    int32_t mig_slb_nr;
> >  };
> >  
> >  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index b329cd8173..1bd38c6a90 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >              break;
> >          }
> >      }
> > -    env->slb_nr = smmu_info.slb_size;
> > +    cpu->hash64_opts->slb_size = smmu_info.slb_size;
> >      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
> >          cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
> >      }
> > diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> > index 0634cdb295..3d6434a006 100644
> > --- a/target/ppc/machine.c
> > +++ b/target/ppc/machine.c
> > @@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> >      unsigned int i, j;
> >      target_ulong sdr1;
> >      uint32_t fpscr;
> > +#if defined(TARGET_PPC64)
> > +    int32_t slb_nr;
> > +#endif
> >      target_ulong xer;
> >  
> >      for (i = 0; i < 32; i++)
> > @@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> >      qemu_get_sbe32s(f, &env->access_type);
> >  #if defined(TARGET_PPC64)
> >      qemu_get_betls(f, &env->spr[SPR_ASR]);
> > -    qemu_get_sbe32s(f, &env->slb_nr);
> > +    qemu_get_sbe32s(f, &slb_nr);
> >  #endif
> >      qemu_get_betls(f, &sdr1);
> >      for (i = 0; i < 32; i++)
> > @@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)
> >      return cpu->pre_2_8_migration;
> >  }
> >  
> > +#if defined(TARGET_PPC64)
> > +static bool cpu_pre_2_13_migration(void *opaque, int version_id)
> > +{
> > +    PowerPCCPU *cpu = opaque;
> > +
> > +    return cpu->pre_2_13_migration;
> > +}
> > +#endif
> > +
> >  static int cpu_pre_save(void *opaque)
> >  {
> >      PowerPCCPU *cpu = opaque;
> > @@ -203,6 +215,11 @@ static int cpu_pre_save(void *opaque)
> >          cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
> >          cpu->mig_nb_BATs = env->nb_BATs;
> >      }
> > +    if (cpu->pre_2_13_migration) {
> > +        if (cpu->hash64_opts) {
> > +            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
> > +        }
> > +    }
> >  
> >      return 0;
> >  }
> > @@ -478,7 +495,7 @@ static int slb_post_load(void *opaque, int version_id)
> >  
> >      /* We've pulled in the raw esid and vsid values from the migration
> >       * stream, but we need to recompute the page size pointers */
> > -    for (i = 0; i < env->slb_nr; i++) {
> > +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
> >          if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
> >              /* Migration source had bad values in its SLB */
> >              return -1;
> > @@ -495,7 +512,7 @@ static const VMStateDescription vmstate_slb = {
> >      .needed = slb_needed,
> >      .post_load = slb_post_load,
> >      .fields = (VMStateField[]) {
> > -        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
> > +        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
> >          VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
> >          VMSTATE_END_OF_LIST()
> >      }
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index a5570c8774..7e0adecfd9 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -52,7 +52,7 @@ static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
> >      esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
> >      esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
> >  
> > -    for (n = 0; n < env->slb_nr; n++) {
> > +    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
> >          ppc_slb_t *slb = &env->slb[n];
> >  
> >          LOG_SLB("%s: slot %d %016" PRIx64 " %016"
> > @@ -80,7 +80,7 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
> >      cpu_synchronize_state(CPU(cpu));
> >  
> >      cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> > -    for (i = 0; i < env->slb_nr; i++) {
> > +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
> >          slbe = env->slb[i].esid;
> >          slbv = env->slb[i].vsid;
> >          if (slbe == 0 && slbv == 0) {
> > @@ -93,10 +93,11 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
> >  
> >  void helper_slbia(CPUPPCState *env)
> >  {
> > +    PowerPCCPU *cpu = ppc_env_get_cpu(env);
> >      int n;
> >  
> >      /* XXX: Warning: slbia never invalidates the first segment */
> > -    for (n = 1; n < env->slb_nr; n++) {
> > +    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
> >          ppc_slb_t *slb = &env->slb[n];
> >  
> >          if (slb->esid & SLB_ESID_V) {
> > @@ -151,7 +152,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
> >      const PPCHash64SegmentPageSizes *sps = NULL;
> >      int i;
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1; /* Bad slot number */
> >      }
> >      if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
> > @@ -202,7 +203,7 @@ static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
> >      int slot = rb & 0xfff;
> >      ppc_slb_t *slb = &env->slb[slot];
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1;
> >      }
> >  
> > @@ -217,7 +218,7 @@ static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
> >      int slot = rb & 0xfff;
> >      ppc_slb_t *slb = &env->slb[slot];
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1;
> >      }
> >  
> > @@ -1115,6 +1116,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
> >  
> >  const PPCHash64Options ppc_hash64_opts_basic = {
> >      .flags = 0,
> > +    .slb_size = 64,
> >      .sps = {
> >          { .page_shift = 12, /* 4K */
> >            .slb_enc = 0,
> > @@ -1129,6 +1131,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
> >  
> >  const PPCHash64Options ppc_hash64_opts_POWER7 = {
> >      .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
> > +    .slb_size = 32,
> >      .sps = {
> >          {
> >              .page_shift = 12, /* 4K */
> > diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> > index f1babb0afc..d5fc03441d 100644
> > --- a/target/ppc/mmu-hash64.h
> > +++ b/target/ppc/mmu-hash64.h
> > @@ -157,6 +157,7 @@ struct PPCHash64Options {
> >  #define PPC_HASH64_AMR          0x00002
> >  #define PPC_HASH64_CI_LARGEPAGE 0x00004
> >      unsigned flags;
> > +    unsigned slb_size;
> >      PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
> >  };
> >  
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index a925cf5cd3..2f63acd310 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -8195,9 +8195,6 @@ static void init_proc_970(CPUPPCState *env)
> >      gen_spr_970_dbg(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 64;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8272,9 +8269,6 @@ static void init_proc_power5plus(CPUPPCState *env)
> >      gen_spr_power5p_ear(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 64;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8389,9 +8383,6 @@ static void init_proc_POWER7(CPUPPCState *env)
> >      gen_spr_power7_book4(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8543,9 +8534,6 @@ static void init_proc_POWER8(CPUPPCState *env)
> >      gen_spr_power8_rpr(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8743,9 +8731,6 @@ static void init_proc_POWER9(CPUPPCState *env)
> >                          KVM_REG_PPC_PSSCR, 0);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> 

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

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

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

* Re: [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
  2018-04-05 13:27     ` Cornelia Huck
@ 2018-04-06  1:09       ` David Gibson
  0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-06  1:09 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Greg Kurz, qemu-ppc, qemu-devel, clg, bharata

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

On Thu, Apr 05, 2018 at 03:27:34PM +0200, Cornelia Huck wrote:
> On Thu, 5 Apr 2018 15:12:55 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Thu,  5 Apr 2018 12:14:37 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> > >   * pseries-2.12
> > >   */
> > >  #define SPAPR_COMPAT_2_12                                              \
> > > -    HW_COMPAT_2_12  
> > 
> > This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...
> > 
> > It looks like a patch to introduce the 2.13 machine type is missing.
> > 
> > FWIW, Connie has already queued a patch to do so for s390x, that also
> > introduces HW_COMPAT_2_12.
> > 
> > https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> > 
> > Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> > 
> > Cc'ing Connie for insights.
> > 
> > > +    HW_COMPAT_2_12                                                     \
> > > +    {                                                                  \
> > > +        .driver = TYPE_POWERPC_CPU,                                    \
> > > +            .property = "pre-2.13-migration",                          \
> > > +            .value    = "on",                                          \  
> 
> I think the usual procedure is
> 
> - every arch that uses compat machines queues a patch that creates the
>   new compat machine(s) and adds an empty HW_COMPAT_<version>
> - whoever has their queue pulled first wins wrt hw_compat

That's my understanding as well.  It's an easy conflict to resolve.

> So, I'm happy with anyone adding the empty HW_COMPAT_2_12 -- it needn't
> be me :)

Likewise.  I'm planning to keep it in my tree for the time being, so
as not to rely on external patches, but when the 2.13 tree opens, who
wins the race is mostly chance, and that's fine.

> [We could also introduce the 2.13 machines for all architectures in one
> sweep, but I think that would be generating needless churn for arch
> maintainers.]
> 

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

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

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

* Re: [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options
  2018-04-05 12:06   ` Greg Kurz
@ 2018-04-06  4:58     ` David Gibson
  0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2018-04-06  4:58 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, qemu-devel, clg, bharata

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

On Thu, Apr 05, 2018 at 02:06:19PM +0200, Greg Kurz wrote:
> On Thu,  5 Apr 2018 12:14:33 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Currently env->mmu_model is a bit of an unholy mess of an enum of distinct
> > MMU types, with various flag bits as well.  This makes which bits of the
> > field should be compared pretty confusing.
> > 
> > Make a start on cleaning that up by moving two of the flags bits -
> > POWERPC_MMU_1TSEG and POWERPC_MMU_AMR - which are specific to the 64-bit
> > hash MMU into a new flags field in PPCHash64Options structure.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> 
> While investigating a migration failure from an older QEMU, I realized
> this patch has a problem. The *cpu->hash64_opts structure is zeroed in
> kvm_fixup_page_sizes(), which has now the unwanted effect of clearing
> the cpu->hash64_opts->flags as well.
> 
> We only need to zero the segment page sizes actually. The following
> fixes migration:
> 
> @@ -442,7 +442,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>      }
>  
>      /* Convert to QEMU form */
> -    memset(cpu->hash64_opts, 0, sizeof(*cpu->hash64_opts));
> +    memset(&cpu->hash64_opts->sps, 0, sizeof(cpu->hash64_opts->sps));
>  
>      /* If we have HV KVM, we need to forbid CI large pages if our
>       * host page size is smaller than 64K.

Ah, good catch.  I mistakenly thought that kvm_fixup_page_sizes() was
completely rewriting those flags as well.  I've made this fix now.

I'm also currently working on some other patches which amongst other
things get rid of kvm_fixup_page_sizes(), because it's completely
broken by design.  But might as well not break things excessively in
the meantime.

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

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

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

end of thread, other threads:[~2018-04-06  4:58 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05  2:14 [Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU) David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 02/13] target/ppc: Simplify cpu valid check in ppc_cpu_realize David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 03/13] target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop() David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 04/13] target/ppc: Avoid taking "env" parameter to mmu-hash64 functions David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 05/13] target/ppc: Remove fallback 64k pagesize information David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 06/13] target/ppc: Move page size setup to helper function David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 07/13] target/ppc: Split page size information into a separate allocation David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 08/13] target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options David Gibson
2018-04-05 12:06   ` Greg Kurz
2018-04-06  4:58     ` David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 10/13] target/ppc: Fold ci_large_pages flag into PPCHash64Options David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 11/13] target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 12/13] target/ppc: Get rid of POWERPC_MMU_VER() macros David Gibson
2018-04-05  2:14 ` [Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options David Gibson
2018-04-05 13:12   ` Greg Kurz
2018-04-05 13:27     ` Cornelia Huck
2018-04-06  1:09       ` David Gibson
2018-04-06  0:49     ` David Gibson

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.