Paul Mackerras <paulus@xxxxxxxxx> writes:
> On Thu, Feb 21, 2013 at 10:17:15PM +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>>
>> We look at both the segment base page size and actual page size and store
>> the pte-lp-encodings in an array per base page size.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>
> This needs more than 2 lines of patch description. In fact what
> you're doing is adding general mixed page-size segment (MPSS)
> support. Doing this should mean that you can also get rid of the
> MMU_PAGE_64K_AP value from the list in asm/mmu.h.
>
Can you elaborate on Admixed pages ? I was not able to find more info on
that.
>> struct mmu_psize_def
>> {
>> unsigned int shift; /* number of bits */
>> - unsigned int penc; /* HPTE encoding */
>> + unsigned int penc[MMU_PAGE_COUNT]; /* HPTE encoding */
>
> I guess this is reasonable, though adding space for 14 page size
> encodings seems a little bit over the top. Also, you don't seem to
> have any way to indicate which encodings are valid, since 0 is a valid
> encoding. Maybe you need to add a valid bit higher up to indicate
> which page sizes are valid.
>
how about penc = { [0 ... MMU_PAGE_COUNT] = -1 } ?, that would make sure
we set all the bits for invalid entries.
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 71d0c90..d2c9932 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -1515,7 +1515,12 @@ static void kvmppc_add_seg_page_size(struct kvm_ppc_one_seg_page_size **sps,
>> (*sps)->page_shift = def->shift;
>> (*sps)->slb_enc = def->sllp;
>> (*sps)->enc[0].page_shift = def->shift;
>> - (*sps)->enc[0].pte_enc = def->penc;
>> + /*
>> + * FIXME!!
>> + * This is returned to user space. Do we need to
>> + * return details of MPSS here ?
>
> Yes, we do, probably a separate entry for each valid base/actual page
> size pair.
>
Ok will do new entries to enc for valid actual page size supported.
for 16MB actual page size
enc[1].page_shift = 24
enc[1].page_shift = x
should this be a seperate patch ?
>> +static inline int hpte_actual_psize(struct hash_pte *hptep, int psize)
>> +{
>> + unsigned int mask;
>> + int i, penc, shift;
>> + /* Look at the 8 bit LP value */
>> + unsigned int lp = (hptep->r >> LP_SHIFT) & ((1 << (LP_BITS + 1)) - 1);
>
> Why LP_BITS + 1 here? You seem to be extracting and comparing 9 bits
> rather than 8. Why is that?
>
My mistake. Will fix
>> @@ -395,12 +422,13 @@ static void hpte_decode(struct hash_pte *hpte, unsigned long slot,
>> /* valid entries have a shift value */
>> if (!mmu_psize_defs[size].shift)
>> continue;
>> -
>> - if (penc == mmu_psize_defs[size].penc)
>> - break;
>> + for (a_size = 0; a_size < MMU_PAGE_COUNT; a_size++)
>> + if (penc == mmu_psize_defs[size].penc[a_size])
>> + goto out;
>
> I think this will get false matches due to unused/invalid entries
> in mmu_psize_defs[size].penc[] containing 0.
Will set the invalid value to 0xff.
-aneesh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>