[PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property

[PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property

[Benjamin Herrenschmidt]

The core tcg/kvm code for ppc64 now has at least the outline
capability to support pagesizes beyond the standard 4k and 16MB.  The
CPUState is initialized with information advertising the available
pagesizes and their correct encodings, and under the right KVM setup
this will be populated with page sizes beyond the standard.

Obviously guests can't use the extra page sizes unless they know
they're present.  For the pseries machine, at least, there is a
defined method for conveying exactly this information, the
"ibm-segment-page-sizes" property in the guest device tree.

This patch generates this property using the supported page size
information that's already in the CPUState.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 hw/spapr.c       |   42 ++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm.c |   11 +++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index 94a4e1e..7c36903 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -149,6 +149,39 @@ static int spapr_set_associativity(void *fdt, sPAPREnvironment *spapr)
     return ret;
 }
 
+
+static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
+                                     size_t maxsize)
+{
+    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];
+
+        if (!sps->page_shift) {
+            break;
+        }
+        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
+            if (sps->enc[count].page_shift == 0) {
+                break;
+            }
+        }
+        if ((p - prop) >= (maxcells - 3 - count * 2))
+            break;
+        *(p++) = cpu_to_be32(sps->page_shift);
+        *(p++) = cpu_to_be32(sps->slb_enc);
+        *(p++) = cpu_to_be32(count);
+        for (j = 0; j < count; j++) {
+            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
+            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
+        }
+    }
+
+    return (p - prop) * sizeof(uint32_t);
+}
+
 static void *spapr_create_fdt_skel(const char *cpu_model,
                                    target_phys_addr_t rma_size,
                                    target_phys_addr_t initrd_base,
@@ -304,6 +337,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                            0xffffffff, 0xffffffff};
         uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
         uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
+        uint32_t page_sizes_prop[64];
+        size_t page_sizes_prop_size;
 
         if ((index % smt) != 0) {
             continue;
@@ -368,6 +403,13 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
             _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
         }
 
+        page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
+                                                      sizeof(page_sizes_prop));
+        if (page_sizes_prop_size) {
+            _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
+                               page_sizes_prop, page_sizes_prop_size)));
+        }
+
         _FDT((fdt_end_node(fdt)));
     }
 
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 77aa186..860711c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -201,6 +201,7 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
     if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_GET_PVINFO)) {
         /* No flags */
         info->flags = 0;
+        info->slb_size = 64;
 
         /* Standard 4k base page size segment */
         info->sps[0].page_shift = 12;
@@ -218,9 +219,15 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
 
 	/* HV KVM has backing store size restrictions */
         info->flags = KVM_PPC_PAGE_SIZES_REAL;
+        if (env->mmu_model == POWERPC_MMU_2_06) {
+            info->slb_size = 32;
+        } else {
+            info->slb_size = 64;
+        }
 
-        if (env->mmu_model & POWERPC_MMU_1TSEG)
-            info->flags = KVM_PPC_1T_SEGMENTS;
+        if (env->mmu_model & POWERPC_MMU_1TSEG) {
+            info->flags |= KVM_PPC_1T_SEGMENTS;
+        }
 
         /* Standard 4k base page size segment */
         info->sps[i].page_shift = 12;

Re: [PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property

[Alexander Graf]

On 04/27/2012 07:51 AM, Benjamin Herrenschmidt wrote:
> The core tcg/kvm code for ppc64 now has at least the outline
> capability to support pagesizes beyond the standard 4k and 16MB.  The
> CPUState is initialized with information advertising the available
> pagesizes and their correct encodings, and under the right KVM setup
> this will be populated with page sizes beyond the standard.
>
> Obviously guests can't use the extra page sizes unless they know
> they're present.  For the pseries machine, at least, there is a
> defined method for conveying exactly this information, the
> "ibm-segment-page-sizes" property in the guest device tree.
>
> This patch generates this property using the supported page size
> information that's already in the CPUState.
>
> Signed-off-by: Nishanth Aravamudan<nacc@us.ibm.com>
> Signed-off-by: David Gibson<david@gibson.dropbear.id.au>
> Signed-off-by: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> ---
>   hw/spapr.c       |   42 ++++++++++++++++++++++++++++++++++++++++++
>   target-ppc/kvm.c |   11 +++++++++--
>   2 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 94a4e1e..7c36903 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -149,6 +149,39 @@ static int spapr_set_associativity(void *fdt, sPAPREnvironment *spapr)
>       return ret;
>   }
>
> +
> +static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
> +                                     size_t maxsize)
> +{
> +    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];
> +
> +        if (!sps->page_shift) {
> +            break;
> +        }
> +        for (count = 0; count<  PPC_PAGE_SIZES_MAX_SZ; count++) {
> +            if (sps->enc[count].page_shift == 0) {
> +                break;
> +            }
> +        }
> +        if ((p - prop)>= (maxcells - 3 - count * 2))

Is this valid C? Can you substract one pointer from another and compare 
the result with an int?

> +            break;

Braces? Please run checkpatch :)

> +        *(p++) = cpu_to_be32(sps->page_shift);
> +        *(p++) = cpu_to_be32(sps->slb_enc);
> +        *(p++) = cpu_to_be32(count);
> +        for (j = 0; j<  count; j++) {
> +            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
> +            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
> +        }
> +    }
> +
> +    return (p - prop) * sizeof(uint32_t);

I'd prefer a second integer counter "len" I think :). Pointer 
arithmentics always make me wary...

> +}
> +
>   static void *spapr_create_fdt_skel(const char *cpu_model,
>                                      target_phys_addr_t rma_size,
>                                      target_phys_addr_t initrd_base,
> @@ -304,6 +337,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
>                              0xffffffff, 0xffffffff};
>           uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
>           uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
> +        uint32_t page_sizes_prop[64];
> +        size_t page_sizes_prop_size;
>
>           if ((index % smt) != 0) {
>               continue;
> @@ -368,6 +403,13 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
>               _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
>           }
>
> +        page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
> +                                                      sizeof(page_sizes_prop));
> +        if (page_sizes_prop_size) {
> +            _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
> +                               page_sizes_prop, page_sizes_prop_size)));
> +        }
> +
>           _FDT((fdt_end_node(fdt)));
>       }
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 77aa186..860711c 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -201,6 +201,7 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
>       if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_GET_PVINFO)) {
>           /* No flags */
>           info->flags = 0;
> +        info->slb_size = 64;

Eh - this one belongs in the first patch, no?

>
>           /* Standard 4k base page size segment */
>           info->sps[0].page_shift = 12;
> @@ -218,9 +219,15 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
>
>   	/* HV KVM has backing store size restrictions */
>           info->flags = KVM_PPC_PAGE_SIZES_REAL;
> +        if (env->mmu_model == POWERPC_MMU_2_06) {
> +            info->slb_size = 32;
> +        } else {
> +            info->slb_size = 64;
> +        }

This assumes that we're always using -cpu host. Is there any more 
reliable way of calculating the slb size? Otherwise maybe we should just 
error out in non-cpu-host cases for HV mode.

>
> -        if (env->mmu_model&  POWERPC_MMU_1TSEG)
> -            info->flags = KVM_PPC_1T_SEGMENTS;
> +        if (env->mmu_model&  POWERPC_MMU_1TSEG) {
> +            info->flags |= KVM_PPC_1T_SEGMENTS;
> +        }

Ahem :)


Alex

Re: [PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property

[Benjamin Herrenschmidt]

> > +        for (count = 0; count<  PPC_PAGE_SIZES_MAX_SZ; count++) {
> > +            if (sps->enc[count].page_shift == 0) {
> > +                break;
> > +            }
> > +        }
> > +        if ((p - prop)>= (maxcells - 3 - count * 2))
> 
> Is this valid C? Can you substract one pointer from another and compare 
> the result with an int?

Ugh ... yeah it's basic stuff ;-) the difference between two pointers is
an integer (there's even a ptrdiff_t nowadays no ?)

> > +            break;
> 
> Braces? Please run checkpatch :)

Ah missed that one.

> > +        *(p++) = cpu_to_be32(sps->page_shift);
> > +        *(p++) = cpu_to_be32(sps->slb_enc);
> > +        *(p++) = cpu_to_be32(count);
> > +        for (j = 0; j<  count; j++) {
> > +            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
> > +            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
> > +        }
> > +    }
> > +
> > +    return (p - prop) * sizeof(uint32_t);
> 
> I'd prefer a second integer counter "len" I think :). Pointer 
> arithmentics always make me wary...
> 
And a separate variable that might accidentally get out of sync makes
_me_ wary :-)

> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> > index 77aa186..860711c 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -201,6 +201,7 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
> >       if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_GET_PVINFO)) {
> >           /* No flags */
> >           info->flags = 0;
> > +        info->slb_size = 64;
> 
> Eh - this one belongs in the first patch, no?

Quite possibly, the split in 2 patches was done by David (I originally
did a single patch), so I'm not 100% sure why he put that there, I'll
have a closer look today.

> >           /* Standard 4k base page size segment */
> >           info->sps[0].page_shift = 12;
> > @@ -218,9 +219,15 @@ static void kvm_get_fallback_smmu_info(CPUPPCState *env,
> >
> >   	/* HV KVM has backing store size restrictions */
> >           info->flags = KVM_PPC_PAGE_SIZES_REAL;
> > +        if (env->mmu_model == POWERPC_MMU_2_06) {
> > +            info->slb_size = 32;
> > +        } else {
> > +            info->slb_size = 64;
> > +        }
> 
> This assumes that we're always using -cpu host. Is there any more 
> reliable way of calculating the slb size? Otherwise maybe we should just 
> error out in non-cpu-host cases for HV mode.

This is a fallback, it's good enough. I don't think there's such a thing
as non-cpu-host on HV anyway, at least for now (we should probably error
out elsewhere). In any case, even if the CPU is configured for backward
compat (which we don't support yet, though might using -cpu in the long
run), the SLB size so far has to be exactly the one implemented by the
host when using HV KVM.

So for anything we can work on today, the above will work.

> > -        if (env->mmu_model&  POWERPC_MMU_1TSEG)
> > -            info->flags = KVM_PPC_1T_SEGMENTS;
> > +        if (env->mmu_model&  POWERPC_MMU_1TSEG) {
> > +            info->flags |= KVM_PPC_1T_SEGMENTS;
> > +        }
> 
> Ahem :)

What's this ? Second patch adding the braces missing in the first one ?
Heh, ok, I'll fix that.

Cheers,
Ben.

[PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property

[Benjamin Herrenschmidt]

The core tcg/kvm code for ppc64 now has at least the outline
capability to support pagesizes beyond the standard 4k and 16MB.  The
CPUState is initialized with information advertising the available
pagesizes and their correct encodings, and under the right KVM setup
this will be populated with page sizes beyond the standard.

Obviously guests can't use the extra page sizes unless they know
they're present.  For the pseries machine, at least, there is a
defined method for conveying exactly this information, the
"ibm-segment-page-sizes" property in the guest device tree.

This patch generates this property using the supported page size
information that's already in the CPUState.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 hw/spapr.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index b20f70d..a22a6b6 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -149,6 +149,40 @@ static int spapr_set_associativity(void *fdt, sPAPREnvironment *spapr)
     return ret;
 }
 
+
+static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
+                                     size_t maxsize)
+{
+    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];
+
+        if (!sps->page_shift) {
+            break;
+        }
+        for (count = 0; count < PPC_PAGE_SIZES_MAX_SZ; count++) {
+            if (sps->enc[count].page_shift == 0) {
+                break;
+            }
+        }
+        if ((p - prop) >= (maxcells - 3 - count * 2)) {
+            break;
+        }
+        *(p++) = cpu_to_be32(sps->page_shift);
+        *(p++) = cpu_to_be32(sps->slb_enc);
+        *(p++) = cpu_to_be32(count);
+        for (j = 0; j < count; j++) {
+            *(p++) = cpu_to_be32(sps->enc[j].page_shift);
+            *(p++) = cpu_to_be32(sps->enc[j].pte_enc);
+        }
+    }
+
+    return (p - prop) * sizeof(uint32_t);
+}
+
 static void *spapr_create_fdt_skel(const char *cpu_model,
                                    target_phys_addr_t rma_size,
                                    target_phys_addr_t initrd_base,
@@ -304,6 +338,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                            0xffffffff, 0xffffffff};
         uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
         uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
+        uint32_t page_sizes_prop[64];
+        size_t page_sizes_prop_size;
 
         if ((index % smt) != 0) {
             continue;
@@ -368,6 +404,13 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
             _FDT((fdt_property_cell(fdt, "ibm,dfp", 1)));
         }
 
+        page_sizes_prop_size = create_page_sizes_prop(env, page_sizes_prop,
+                                                      sizeof(page_sizes_prop));
+        if (page_sizes_prop_size) {
+            _FDT((fdt_property(fdt, "ibm,segment-page-sizes",
+                               page_sizes_prop, page_sizes_prop_size)));
+        }
+
         _FDT((fdt_end_node(fdt)));
     }