On Thu, Aug 15, 2024 at 11:56 AM Kuan-Ying Lee <kuan-ying....@canonical.com>
wrote:

> On Tue, Aug 13, 2024 at 12:04:30PM +0800, lijiang wrote:
> > Thank you for the patch, Kuan-Ying.
> >
> > On Thu, Aug 8, 2024 at 9:09 PM <
> devel-requ...@lists.crash-utility.osci.io>
> > wrote:
> >
> > > Date: Thu,  8 Aug 2024 13:35:41 +0800
> > > From: Kuan-Ying Lee <kuan-ying....@canonical.com>
> > > Subject: [Crash-utility] [PATCH 1/3] arm64: Introduction of support
> > >         for 16K page with 2-level table support
> > > To: kuan-ying....@canonical.com,
> > >         devel@lists.crash-utility.osci.io
> > > Message-ID: <20240808053543.10910-2-kuan-ying....@canonical.com>
> > >
> > > Introduction of ARM64 support for 16K page size with 2-level page
> > > table and 36 VA bits.
> > >
> > > Signed-off-by: Kuan-Ying Lee <kuan-ying....@canonical.com>
> > > ---
> > >  arm64.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  defs.h  | 11 +++++++
> > >  2 files changed, 98 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arm64.c b/arm64.c
> > > index 8ed1aaf919d6..002cc0078dad 100644
> > > --- a/arm64.c
> > > +++ b/arm64.c
> > > @@ -42,6 +42,7 @@ static int arm64_kvtop(struct task_context *, ulong,
> > > physaddr_t *, int);
> > >  static int arm64_uvtop(struct task_context *, ulong, physaddr_t *,
> int);
> > >  static int arm64_vtop_2level_64k(ulong, ulong, physaddr_t *, int);
> > >  static int arm64_vtop_3level_64k(ulong, ulong, physaddr_t *, int);
> > > +static int arm64_vtop_2level_16k(ulong, ulong, physaddr_t *, int);
> > >  static int arm64_vtop_3level_16k(ulong, ulong, physaddr_t *, int);
> > >  static int arm64_vtop_3level_4k(ulong, ulong, physaddr_t *, int);
> > >  static int arm64_vtop_4level_4k(ulong, ulong, physaddr_t *, int);
> > > @@ -426,7 +427,16 @@ arm64_init(int when)
> > >                                     (char *)malloc(PTRS_PER_PTE_L3_16K
> *
> > > 8)) == NULL)
> > >                                         error(FATAL, "cannot malloc
> ptbl
> > > space.");
> > >                         } else {
> > > -                               error(FATAL, "we only support 47 bits,
> 3
> > > level for 16K page now.");
> > >
> >
> > I copied it from kernel code here:
> >
> > config PGTABLE_LEVELS
> >         int
> >         default 2 if ARM64_16K_PAGES && ARM64_VA_BITS_36
> >         default 2 if ARM64_64K_PAGES && ARM64_VA_BITS_42
> >         default 3 if ARM64_64K_PAGES && (ARM64_VA_BITS_48 ||
> > ARM64_VA_BITS_52)
> >         default 3 if ARM64_4K_PAGES && ARM64_VA_BITS_39
> >         default 3 if ARM64_16K_PAGES && ARM64_VA_BITS_47
> >         default 4 if ARM64_16K_PAGES && (ARM64_VA_BITS_48 ||
> > ARM64_VA_BITS_52)
> >         default 4 if !ARM64_64K_PAGES && ARM64_VA_BITS_48
> >         default 5 if ARM64_4K_PAGES && ARM64_VA_BITS_52
> >
> > Crash tools do not support 16K pages with 4-level tables and 48 or 52 VA
> > bits, is it possible to add a similar check for this one? I'm not sure if
> > you have any plans to support it.
>
> Yes, it's on my to-do list. I will find time to implement it later.
>

Thank you for working on it, Kuan-Ying.

I have a request: Can you help add a check in the current patch and say
that crash tools do not support 48(or 52)bits, 4-level page tables for 16K
pages?

For example:
...
error(FATAL, "Only support [47 bits, 3-level] or [36 bits, 2-level] for 16K
page now.");
 ...

or
error(FATAL, "Do not support 48 bits or 52 bits, 4-level for 16K page
now.");


Thanks
Lianbo

Thank you.
>
> >
> > BTW:  It also has the same issue in the [PATCH 3/3].
> >
> > +                               machdep->flags |= VM_L2_16K;
> > > +                               if (!machdep->ptrs_per_pgd)
> > > +                                       machdep->ptrs_per_pgd =
> > > PTRS_PER_PGD_L2_16K;
> > > +                               if ((machdep->pgd =
> > > +                                   (char
> *)malloc(machdep->ptrs_per_pgd *
> > > 8)) == NULL)
> > > +                                       error(FATAL, "cannot malloc pgd
> > > space.");
> > > +                               if ((machdep->ptbl =
> > > +                                   (char *)malloc(PTRS_PER_PTE_L2_16K
> *
> > > 8)) == NULL)
> > > +                                       error(FATAL, "cannot malloc
> ptbl
> > > space.");
> > > +                               machdep->pmd = NULL;  /* not used */
> > >                         }
> > >                         machdep->pud = NULL;  /* not used */
> > >                         break;
> > > @@ -1064,6 +1074,8 @@ arm64_dump_machdep_table(ulong arg)
> > >                 fprintf(fp, "%sVM_L2_64K", others++ ? "|" : "");
> > >         if (machdep->flags & VM_L3_64K)
> > >                 fprintf(fp, "%sVM_L3_64K", others++ ? "|" : "");
> > > +       if (machdep->flags & VM_L2_16K)
> > > +               fprintf(fp, "%sVM_L2_16K", others++ ? "|" : "");
> > >         if (machdep->flags & VM_L3_16K)
> > >                 fprintf(fp, "%sVM_L3_16K", others++ ? "|" : "");
> > >         if (machdep->flags & VM_L3_4K)
> > > @@ -1113,6 +1125,8 @@ arm64_dump_machdep_table(ulong arg)
> > >                 "arm64_vtop_3level_4k" :
> > >                 machdep->flags & VM_L4_4K ?
> > >                 "arm64_vtop_4level_4k" :
> > > +               machdep->flags & VM_L2_16K ?
> > > +               "arm64_vtop_2level_16k" :
> > >                 machdep->flags & VM_L3_16K ?
> > >                 "arm64_vtop_3level_16k" :
> > >                 machdep->flags & VM_L3_64K ?
> > > @@ -1122,6 +1136,8 @@ arm64_dump_machdep_table(ulong arg)
> > >                 "arm64_vtop_3level_4k" :
> > >                 machdep->flags & VM_L4_4K ?
> > >                 "arm64_vtop_4level_4k" :
> > > +               machdep->flags & VM_L2_16K ?
> > > +               "arm64_vtop_2level_16k" :
> > >                 machdep->flags & VM_L3_16K ?
> > >                 "arm64_vtop_3level_16k" :
> > >                 machdep->flags & VM_L3_64K ?
> > > @@ -1814,7 +1830,7 @@ arm64_kvtop(struct task_context *tc, ulong
> kvaddr,
> > > physaddr_t *paddr, int verbos
> > >         kernel_pgd = vt->kernel_pgd[0];
> > >         *paddr = 0;
> > >
> > > -       switch (machdep->flags &
> > > (VM_L2_64K|VM_L3_64K|VM_L3_4K|VM_L4_4K|VM_L3_16K))
> > > +       switch (machdep->flags &
> > > (VM_L2_64K|VM_L3_64K|VM_L3_4K|VM_L4_4K|VM_L2_16K|VM_L3_16K))
> > >         {
> > >         case VM_L2_64K:
> > >                 return arm64_vtop_2level_64k(kernel_pgd, kvaddr, paddr,
> > > verbose);
> > > @@ -1824,6 +1840,8 @@ arm64_kvtop(struct task_context *tc, ulong
> kvaddr,
> > > physaddr_t *paddr, int verbos
> > >                 return arm64_vtop_3level_4k(kernel_pgd, kvaddr, paddr,
> > > verbose);
> > >         case VM_L4_4K:
> > >                 return arm64_vtop_4level_4k(kernel_pgd, kvaddr, paddr,
> > > verbose);
> > > +       case VM_L2_16K:
> > > +               return arm64_vtop_2level_16k(kernel_pgd, kvaddr, paddr,
> > > verbose);
> > >         case VM_L3_16K:
> > >                 return arm64_vtop_3level_16k(kernel_pgd, kvaddr, paddr,
> > > verbose);
> > >         default:
> > > @@ -1841,7 +1859,7 @@ arm64_uvtop(struct task_context *tc, ulong
> uvaddr,
> > > physaddr_t *paddr, int verbos
> > >
> > >         *paddr = 0;
> > >
> > > -       switch (machdep->flags &
> > > (VM_L2_64K|VM_L3_64K|VM_L3_4K|VM_L4_4K|VM_L3_16K))
> > > +       switch (machdep->flags &
> > > (VM_L2_64K|VM_L3_64K|VM_L3_4K|VM_L4_4K|VM_L2_16K|VM_L3_16K))
> > >         {
> > >         case VM_L2_64K:
> > >                 return arm64_vtop_2level_64k(user_pgd, uvaddr, paddr,
> > > verbose);
> > > @@ -1851,6 +1869,8 @@ arm64_uvtop(struct task_context *tc, ulong
> uvaddr,
> > > physaddr_t *paddr, int verbos
> > >                 return arm64_vtop_3level_4k(user_pgd, uvaddr, paddr,
> > > verbose);
> > >         case VM_L4_4K:
> > >                 return arm64_vtop_4level_4k(user_pgd, uvaddr, paddr,
> > > verbose);
> > > +       case VM_L2_16K:
> > > +               return arm64_vtop_2level_16k(user_pgd, uvaddr, paddr,
> > > verbose);
> > >         case VM_L3_16K:
> > >                 return arm64_vtop_3level_16k(user_pgd, uvaddr, paddr,
> > > verbose);
> > >         default:
> > > @@ -2012,6 +2032,70 @@ no_page:
> > >         return FALSE;
> > >  }
> > >
> > > +static int
> > > +arm64_vtop_2level_16k(ulong pgd, ulong vaddr, physaddr_t *paddr, int
> > > verbose)
> > > +{
> > > +       ulong *pgd_base, *pgd_ptr, pgd_val;
> > > +       ulong *pte_base, *pte_ptr, pte_val;
> > > +
> > > +        if (verbose)
> > > +                fprintf(fp, "PAGE DIRECTORY: %lx\n", pgd);
> > > +
> > > +       pgd_base = (ulong *)pgd;
> > > +       FILL_PGD(pgd_base, KVADDR, machdep->ptrs_per_pgd *
> sizeof(ulong));
> > > +       pgd_ptr = pgd_base + (((vaddr) >> PGDIR_SHIFT_L2_16K) &
> > > (machdep->ptrs_per_pgd - 1));
> > > +        pgd_val = ULONG(machdep->pgd + PAGEOFFSET(pgd_ptr));
> > > +        if (verbose)
> > > +                fprintf(fp, "   PGD: %lx => %lx\n", (ulong)pgd_ptr,
> > > pgd_val);
> > > +       if (!pgd_val)
> > > +               goto no_page;
> > > +
> > > +       /*
> > > +        * #define __PAGETABLE_PUD_FOLDED
> > > +        * #define __PAGETABLE_PMD_FOLDED
> > > +        */
> > > +
> > > +       if ((pgd_val & PMD_TYPE_MASK) == PMD_TYPE_SECT) {
> > > +               ulong sectionbase = (pgd_val & SECTION_PAGE_MASK_32MB)
> &
> > > PHYS_MASK;
> > > +               if (verbose) {
> > > +                       fprintf(fp, "  PAGE: %lx  (32MB%s)\n\n",
> > > sectionbase,
> > > +                               IS_ZEROPAGE(sectionbase) ? ", ZERO
> PAGE" :
> > > "");
> > > +                       arm64_translate_pte(pgd_val, 0, 0);
> > > +               }
> > > +               *paddr = sectionbase + (vaddr &
> ~SECTION_PAGE_MASK_32MB);
> > > +               return TRUE;
> > > +       }
> > > +
> > > +       pte_base = (ulong *)PTOV(pgd_val & PHYS_MASK &
> > > (s32)machdep->pagemask);
> > > +       FILL_PTBL(pte_base, KVADDR, PTRS_PER_PTE_L2_16K *
> sizeof(ulong));
> > > +       pte_ptr = pte_base + (((vaddr) >> machdep->pageshift) &
> > > (PTRS_PER_PTE_L2_16K - 1));
> > > +        pte_val = ULONG(machdep->ptbl + PAGEOFFSET(pte_ptr));
> > > +        if (verbose)
> > > +                fprintf(fp, "   PTE: %lx => %lx\n", (ulong)pte_ptr,
> > > pte_val);
> > > +       if (!pte_val)
> > > +               goto no_page;
> > > +
> > > +       if (pte_val & PTE_VALID) {
> > > +               *paddr = (PAGEBASE(pte_val) & PHYS_MASK) +
> > > PAGEOFFSET(vaddr);
> > > +               if (verbose) {
> > > +                       fprintf(fp, "  PAGE: %lx  %s\n\n",
> > > PAGEBASE(*paddr),
> > > +                               IS_ZEROPAGE(PAGEBASE(*paddr)) ? "(ZERO
> > > PAGE)" : "");
> > > +                       arm64_translate_pte(pte_val, 0, 0);
> > > +               }
> > > +       } else {
> > > +               if (IS_UVADDR(vaddr, NULL))
> > > +                       *paddr = pte_val;
> > > +               if (verbose) {
> > > +                       fprintf(fp, "\n");
> > > +                       arm64_translate_pte(pte_val, 0, 0);
> > > +               }
> > > +               goto no_page;
> > > +       }
> > > +
> > > +       return TRUE;
> > > +no_page:
> > > +       return FALSE;
> > > +}
> > >
> >
> > The above code seems to have an indentation issue, other changes are fine
> > to me.
> >
> > Thanks
> > Lianbo
> >
> >
> > >  static int
> > >  arm64_vtop_3level_16k(ulong pgd, ulong vaddr, physaddr_t *paddr, int
> > > verbose)
> > >  {
> > > diff --git a/defs.h b/defs.h
> > > index 1b7649d9f05c..dfbd2419f969 100644
> > > --- a/defs.h
> > > +++ b/defs.h
> > > @@ -3302,6 +3302,16 @@ typedef signed int s32;
> > >  #define PGDIR_MASK_48VA      (~(PGDIR_SIZE_48VA - 1))
> > >  #define PGDIR_OFFSET_48VA(X) (((ulong)(X)) & (PGDIR_SIZE_48VA - 1))
> > >
> > > +/*
> > > + * 2-levels / 16K pages
> > > + * 36-bit VA
> > > + */
> > > +#define PTRS_PER_PGD_L2_16K  (2048)
> > > +#define PTRS_PER_PTE_L2_16K  (2048)
> > > +#define PGDIR_SHIFT_L2_16K   (25)
> > > +#define PGDIR_SIZE_L2_16K    ((1UL) << PGDIR_SHIFT_L2_16K)
> > > +#define PGDIR_MASK_L2_16K    (~(PGDIR_SIZE_L2_16K-1))
> > > +
> > >  /*
> > >   * 3-levels / 16K pages
> > >   * 47-bit VA
> > > @@ -3383,6 +3393,7 @@ typedef signed int s32;
> > >  #define OVERFLOW_STACKS     (0x1000)
> > >  #define ARM64_MTE     (0x2000)
> > >  #define VM_L3_16K     (0x4000)
> > > +#define VM_L2_16K     (0x8000)
> > >
> > >  /*
> > >   * Get kimage_voffset from /dev/crash
> > > --
> > > 2.34.1
> > >
>
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to