From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Thu, 10 May 2012 17:57:23 +0000 Subject: Re: [PATCH 2/2] pseries: Correctly create ibm,segment-page-sizes property Message-Id: <4FAC0183.2020602@suse.de> List-Id: References: <1335505422.4578.3.camel@pasglop> <1335505904.4578.10.camel@pasglop> In-Reply-To: <1335505904.4578.10.camel@pasglop> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Benjamin Herrenschmidt Cc: kvm@vger.kernel.org, kvm-ppc 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 > Signed-off-by: David Gibson > Signed-off-by: Benjamin Herrenschmidt > --- > 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