On Thu, Feb 21, 2013 at 10:17:15PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com>
> 
> 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.ku...@linux.vnet.ibm.com>

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.

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

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

> +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?

> @@ -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.

Paul.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to