On Thu, 11 Feb 2021 11:06:06 +0100
Thomas Huth <thuth@xxxxxxxxxx> wrote:
> On 09/02/2021 15.38, Claudio Imbrenda wrote:
> > Add support for 1M and 2G pages.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> > ---
> > lib/s390x/mmu.h | 73 +++++++++++++-
> > lib/s390x/mmu.c | 246
> > +++++++++++++++++++++++++++++++++++++++++++----- 2 files changed,
> > 294 insertions(+), 25 deletions(-)
> [...]
> > +/*
> > + * Get the pte (page) DAT table entry for the given address and
> > pmd,
> > + * allocating it if necessary.
> > + * The pmd must not be large.
> > + */
> > +static inline pte_t *get_pte(pmd_t *pmd, uintptr_t vaddr)
> > +{
> > pte_t *pte = pte_alloc(pmd, vaddr);
> >
> > - return &pte_val(*pte);
> > + assert(!pmd_large(*pmd));
> > + pte = pte_alloc(pmd, vaddr);
>
> Why is this function doing "pte = pte_alloc(pmd, vaddr)" twice now?
ooops! the first pte_alloc is not supposed to be there!
good catch - will fix
> > + return pte;
> > +}
> [...]
> > + if ((level == 1) && !pgd_none(*(pgd_t *)ptr))
> > + idte_pgdp(va, ptr);
> > + else if ((level == 2) && !p4d_none(*(p4d_t *)ptr))
> > + idte_p4dp(va, ptr);
> > + else if ((level == 3) && !pud_none(*(pud_t *)ptr))
> > + idte_pudp(va, ptr);
> > + else if ((level == 4) && !pmd_none(*(pmd_t *)ptr))
> > + idte_pmdp(va, ptr);
> > + else if (!pte_none(*(pte_t *)ptr))
> > + ipte(va, ptr);
>
> Meta-comment: Being someone who worked quite a bit with the page
> tables on s390x, but never really got in touch with the way it is
> handled in the Linux kernel, I'm always having a hard time to match
> all these TLAs to the PoP: pmd, pud, p4d ...
> Can we please have a proper place in the kvm-unit-tests sources
> somewhere (maybe at the beginning of mmu.c), where the TLAs are
> explained and how they map to the region and segment tables of the Z
> architecture? (I personally would prefer to completely switch to the
> Z arch naming instead, but I guess that's too much of a change right
> now)
makes sense, I can add that
> Thomas
>
