RE: [PATCH v4 2/4] makedumpfile/arm64: Add support for ARMv8.2-LPA (52-bit PA support)
> -Original Message-
> Hi Kazu,
>
> On Wed, Dec 4, 2019 at 11:07 PM Kazuhito Hagio wrote:
> >
> > > -Original Message-
> > > ARMv8.2-LPA architecture extension (if available on underlying hardware)
> > > can support 52-bit physical addresses, while the kernel virtual
> > > addresses remain 48-bit.
> > >
> > > Make sure that we read the 52-bit PA address capability from
> > > 'MAX_PHYSMEM_BITS' variable (if available in vmcoreinfo) and
> > > accordingly change the pte_to_phy() mask values and also traverse
> > > the page-table walk accordingly.
> > >
> > > Also make sure that it works well for the existing 48-bit PA address
> > > platforms and also on environments which use newer kernels with 52-bit
> > > PA support but hardware which is not ARM8.2-LPA compliant.
> > >
> > > I have sent a kernel patch upstream to add 'MAX_PHYSMEM_BITS' to
> > > vmcoreinfo for arm64 (see [0]).
> > >
> > > This patch is in accordance with ARMv8 Architecture Reference Manual
> > > version D.a
> > >
> > > [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html
> > >
> > > Cc: Kazuhito Hagio
> > > Cc: John Donnelly
> > > Cc: kexec@lists.infradead.org
> > > Signed-off-by: Bhupesh Sharma
> > > ---
> > > arch/arm64.c | 292
> > > +--
> > > 1 file changed, 204 insertions(+), 88 deletions(-)
> > >
> > > diff --git a/arch/arm64.c b/arch/arm64.c
> > > index 3516b340adfd..ecb19139e178 100644
> > > --- a/arch/arm64.c
> > > +++ b/arch/arm64.c
> > > @@ -39,72 +39,184 @@ typedef struct {
> > > unsigned long pte;
> > > } pte_t;
> > >
> >
> > > +#define __pte(x) ((pte_t) { (x) } )
> > > +#define __pmd(x) ((pmd_t) { (x) } )
> > > +#define __pud(x) ((pud_t) { (x) } )
> > > +#define __pgd(x) ((pgd_t) { (x) } )
> >
> > Is it possible to remove these macros?
>
> Ok, will fix in v5.
>
> > > +
> > > +static int lpa_52_bit_support_available;
> > > static int pgtable_level;
> > > static int va_bits;
> > > static unsigned long kimage_voffset;
> > >
> > > -#define SZ_4K(4 * 1024)
> > > -#define SZ_16K (16 * 1024)
> > > -#define SZ_64K (64 * 1024)
> > > -#define SZ_128M (128 * 1024 * 1024)
> > > +#define SZ_4K4096
> > > +#define SZ_16K 16384
> > > +#define SZ_64K 65536
> > >
> > > -#define PAGE_OFFSET_36 ((0xUL) << 36)
> > > -#define PAGE_OFFSET_39 ((0xUL) << 39)
> > > -#define PAGE_OFFSET_42 ((0xUL) << 42)
> > > -#define PAGE_OFFSET_47 ((0xUL) << 47)
> > > -#define PAGE_OFFSET_48 ((0xUL) << 48)
> > > +#define PAGE_OFFSET_36 ((0xUL) << 36)
> > > +#define PAGE_OFFSET_39 ((0xUL) << 39)
> > > +#define PAGE_OFFSET_42 ((0xUL) << 42)
> > > +#define PAGE_OFFSET_47 ((0xUL) << 47)
> > > +#define PAGE_OFFSET_48 ((0xUL) << 48)
> > > +#define PAGE_OFFSET_52 ((0xUL) << 52)
> > >
> > > #define pgd_val(x) ((x).pgd)
> > > #define pud_val(x) (pgd_val((x).pgd))
> > > #define pmd_val(x) (pud_val((x).pud))
> > > #define pte_val(x) ((x).pte)
> > >
> > > -#define PAGE_MASK(~(PAGESIZE() - 1))
> > > -#define PGDIR_SHIFT ((PAGESHIFT() - 3) * pgtable_level + 3)
> > > -#define PTRS_PER_PGD (1 << (va_bits - PGDIR_SHIFT))
> > > -#define PUD_SHIFTget_pud_shift_arm64()
> > > -#define PUD_SIZE (1UL << PUD_SHIFT)
> > > -#define PUD_MASK (~(PUD_SIZE - 1))
> > > -#define PTRS_PER_PTE (1 << (PAGESHIFT() - 3))
> > > -#define PTRS_PER_PUD PTRS_PER_PTE
> > > -#define PMD_SHIFT((PAGESHIFT() - 3) * 2 + 3)
> > > -#define PMD_SIZE (1UL << PMD_SHIFT)
> > > -#define PMD_MASK (~(PMD_SIZE - 1))
> >
> > > +/* See 'include/uapi/linux/const.h' for definitions below */
> > > +#define __AC(X,Y)(X##Y)
> > > +#define _AC(X,Y) __AC(X,Y)
> > > +#define _AT(T,X) ((T)(X))
> > > +
> > > +/* See 'include/asm/pgtable-types.h' for definitions below */
> > > +typedef unsigned long pteval_t;
> > > +typedef unsigned long pmdval_t;
> > > +typedef unsigned long pudval_t;
> > > +typedef unsigned long pgdval_t;
> >
> > Is it possible to remove these macros/typedefs as well?
> > I don't think they make the code easier to read..
>
> Ok. The idea behind it was to keep the makedumpfile(user-space)
> page-table-walk code similar to Linux kernel (as much as possible), so
> that if we identify an issue in user-space code, it is easier to
> correlate it with the corresponding kernel code.
>
> I will try to see how this can be addressed while making the code
> easier to read/understand.
Thank you for understanding.
Yes,
Re: [PATCH v4 2/4] makedumpfile/arm64: Add support for ARMv8.2-LPA (52-bit PA support)
Hi Kazu,
On Wed, Dec 4, 2019 at 11:07 PM Kazuhito Hagio wrote:
>
> > -Original Message-
> > ARMv8.2-LPA architecture extension (if available on underlying hardware)
> > can support 52-bit physical addresses, while the kernel virtual
> > addresses remain 48-bit.
> >
> > Make sure that we read the 52-bit PA address capability from
> > 'MAX_PHYSMEM_BITS' variable (if available in vmcoreinfo) and
> > accordingly change the pte_to_phy() mask values and also traverse
> > the page-table walk accordingly.
> >
> > Also make sure that it works well for the existing 48-bit PA address
> > platforms and also on environments which use newer kernels with 52-bit
> > PA support but hardware which is not ARM8.2-LPA compliant.
> >
> > I have sent a kernel patch upstream to add 'MAX_PHYSMEM_BITS' to
> > vmcoreinfo for arm64 (see [0]).
> >
> > This patch is in accordance with ARMv8 Architecture Reference Manual
> > version D.a
> >
> > [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html
> >
> > Cc: Kazuhito Hagio
> > Cc: John Donnelly
> > Cc: kexec@lists.infradead.org
> > Signed-off-by: Bhupesh Sharma
> > ---
> > arch/arm64.c | 292
> > +--
> > 1 file changed, 204 insertions(+), 88 deletions(-)
> >
> > diff --git a/arch/arm64.c b/arch/arm64.c
> > index 3516b340adfd..ecb19139e178 100644
> > --- a/arch/arm64.c
> > +++ b/arch/arm64.c
> > @@ -39,72 +39,184 @@ typedef struct {
> > unsigned long pte;
> > } pte_t;
> >
>
> > +#define __pte(x) ((pte_t) { (x) } )
> > +#define __pmd(x) ((pmd_t) { (x) } )
> > +#define __pud(x) ((pud_t) { (x) } )
> > +#define __pgd(x) ((pgd_t) { (x) } )
>
> Is it possible to remove these macros?
Ok, will fix in v5.
> > +
> > +static int lpa_52_bit_support_available;
> > static int pgtable_level;
> > static int va_bits;
> > static unsigned long kimage_voffset;
> >
> > -#define SZ_4K(4 * 1024)
> > -#define SZ_16K (16 * 1024)
> > -#define SZ_64K (64 * 1024)
> > -#define SZ_128M (128 * 1024 * 1024)
> > +#define SZ_4K4096
> > +#define SZ_16K 16384
> > +#define SZ_64K 65536
> >
> > -#define PAGE_OFFSET_36 ((0xUL) << 36)
> > -#define PAGE_OFFSET_39 ((0xUL) << 39)
> > -#define PAGE_OFFSET_42 ((0xUL) << 42)
> > -#define PAGE_OFFSET_47 ((0xUL) << 47)
> > -#define PAGE_OFFSET_48 ((0xUL) << 48)
> > +#define PAGE_OFFSET_36 ((0xUL) << 36)
> > +#define PAGE_OFFSET_39 ((0xUL) << 39)
> > +#define PAGE_OFFSET_42 ((0xUL) << 42)
> > +#define PAGE_OFFSET_47 ((0xUL) << 47)
> > +#define PAGE_OFFSET_48 ((0xUL) << 48)
> > +#define PAGE_OFFSET_52 ((0xUL) << 52)
> >
> > #define pgd_val(x) ((x).pgd)
> > #define pud_val(x) (pgd_val((x).pgd))
> > #define pmd_val(x) (pud_val((x).pud))
> > #define pte_val(x) ((x).pte)
> >
> > -#define PAGE_MASK(~(PAGESIZE() - 1))
> > -#define PGDIR_SHIFT ((PAGESHIFT() - 3) * pgtable_level + 3)
> > -#define PTRS_PER_PGD (1 << (va_bits - PGDIR_SHIFT))
> > -#define PUD_SHIFTget_pud_shift_arm64()
> > -#define PUD_SIZE (1UL << PUD_SHIFT)
> > -#define PUD_MASK (~(PUD_SIZE - 1))
> > -#define PTRS_PER_PTE (1 << (PAGESHIFT() - 3))
> > -#define PTRS_PER_PUD PTRS_PER_PTE
> > -#define PMD_SHIFT((PAGESHIFT() - 3) * 2 + 3)
> > -#define PMD_SIZE (1UL << PMD_SHIFT)
> > -#define PMD_MASK (~(PMD_SIZE - 1))
>
> > +/* See 'include/uapi/linux/const.h' for definitions below */
> > +#define __AC(X,Y)(X##Y)
> > +#define _AC(X,Y) __AC(X,Y)
> > +#define _AT(T,X) ((T)(X))
> > +
> > +/* See 'include/asm/pgtable-types.h' for definitions below */
> > +typedef unsigned long pteval_t;
> > +typedef unsigned long pmdval_t;
> > +typedef unsigned long pudval_t;
> > +typedef unsigned long pgdval_t;
>
> Is it possible to remove these macros/typedefs as well?
> I don't think they make the code easier to read..
Ok. The idea behind it was to keep the makedumpfile(user-space)
page-table-walk code similar to Linux kernel (as much as possible), so
that if we identify an issue in user-space code, it is easier to
correlate it with the corresponding kernel code.
I will try to see how this can be addressed while making the code
easier to read/understand.
Regards,
Bhupesh
> > +
> > +#define PAGE_SHIFT PAGESHIFT()
> > +
> > +/* See 'arch/arm64/include/asm/pgtable-hwdef.h' for definitions below */
> > +
> > +#define ARM64_HW_PGTABLE_LEVEL_SHIFT(n) ((PAGE_SHIFT - 3) * (4 - (n))
> > + 3)
> > +
> > +#define PTRS_PER_PTE (1 << (PAGE_SHIFT -
RE: [PATCH v4 2/4] makedumpfile/arm64: Add support for ARMv8.2-LPA (52-bit PA support)
> -Original Message-
> ARMv8.2-LPA architecture extension (if available on underlying hardware)
> can support 52-bit physical addresses, while the kernel virtual
> addresses remain 48-bit.
>
> Make sure that we read the 52-bit PA address capability from
> 'MAX_PHYSMEM_BITS' variable (if available in vmcoreinfo) and
> accordingly change the pte_to_phy() mask values and also traverse
> the page-table walk accordingly.
>
> Also make sure that it works well for the existing 48-bit PA address
> platforms and also on environments which use newer kernels with 52-bit
> PA support but hardware which is not ARM8.2-LPA compliant.
>
> I have sent a kernel patch upstream to add 'MAX_PHYSMEM_BITS' to
> vmcoreinfo for arm64 (see [0]).
>
> This patch is in accordance with ARMv8 Architecture Reference Manual
> version D.a
>
> [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html
>
> Cc: Kazuhito Hagio
> Cc: John Donnelly
> Cc: kexec@lists.infradead.org
> Signed-off-by: Bhupesh Sharma
> ---
> arch/arm64.c | 292
> +--
> 1 file changed, 204 insertions(+), 88 deletions(-)
>
> diff --git a/arch/arm64.c b/arch/arm64.c
> index 3516b340adfd..ecb19139e178 100644
> --- a/arch/arm64.c
> +++ b/arch/arm64.c
> @@ -39,72 +39,184 @@ typedef struct {
> unsigned long pte;
> } pte_t;
>
> +#define __pte(x) ((pte_t) { (x) } )
> +#define __pmd(x) ((pmd_t) { (x) } )
> +#define __pud(x) ((pud_t) { (x) } )
> +#define __pgd(x) ((pgd_t) { (x) } )
Is it possible to remove these macros?
> +
> +static int lpa_52_bit_support_available;
> static int pgtable_level;
> static int va_bits;
> static unsigned long kimage_voffset;
>
> -#define SZ_4K(4 * 1024)
> -#define SZ_16K (16 * 1024)
> -#define SZ_64K (64 * 1024)
> -#define SZ_128M (128 * 1024 * 1024)
> +#define SZ_4K4096
> +#define SZ_16K 16384
> +#define SZ_64K 65536
>
> -#define PAGE_OFFSET_36 ((0xUL) << 36)
> -#define PAGE_OFFSET_39 ((0xUL) << 39)
> -#define PAGE_OFFSET_42 ((0xUL) << 42)
> -#define PAGE_OFFSET_47 ((0xUL) << 47)
> -#define PAGE_OFFSET_48 ((0xUL) << 48)
> +#define PAGE_OFFSET_36 ((0xUL) << 36)
> +#define PAGE_OFFSET_39 ((0xUL) << 39)
> +#define PAGE_OFFSET_42 ((0xUL) << 42)
> +#define PAGE_OFFSET_47 ((0xUL) << 47)
> +#define PAGE_OFFSET_48 ((0xUL) << 48)
> +#define PAGE_OFFSET_52 ((0xUL) << 52)
>
> #define pgd_val(x) ((x).pgd)
> #define pud_val(x) (pgd_val((x).pgd))
> #define pmd_val(x) (pud_val((x).pud))
> #define pte_val(x) ((x).pte)
>
> -#define PAGE_MASK(~(PAGESIZE() - 1))
> -#define PGDIR_SHIFT ((PAGESHIFT() - 3) * pgtable_level + 3)
> -#define PTRS_PER_PGD (1 << (va_bits - PGDIR_SHIFT))
> -#define PUD_SHIFTget_pud_shift_arm64()
> -#define PUD_SIZE (1UL << PUD_SHIFT)
> -#define PUD_MASK (~(PUD_SIZE - 1))
> -#define PTRS_PER_PTE (1 << (PAGESHIFT() - 3))
> -#define PTRS_PER_PUD PTRS_PER_PTE
> -#define PMD_SHIFT((PAGESHIFT() - 3) * 2 + 3)
> -#define PMD_SIZE (1UL << PMD_SHIFT)
> -#define PMD_MASK (~(PMD_SIZE - 1))
> +/* See 'include/uapi/linux/const.h' for definitions below */
> +#define __AC(X,Y)(X##Y)
> +#define _AC(X,Y) __AC(X,Y)
> +#define _AT(T,X) ((T)(X))
> +
> +/* See 'include/asm/pgtable-types.h' for definitions below */
> +typedef unsigned long pteval_t;
> +typedef unsigned long pmdval_t;
> +typedef unsigned long pudval_t;
> +typedef unsigned long pgdval_t;
Is it possible to remove these macros/typedefs as well?
I don't think they make the code easier to read..
Thanks,
Kazu
> +
> +#define PAGE_SHIFT PAGESHIFT()
> +
> +/* See 'arch/arm64/include/asm/pgtable-hwdef.h' for definitions below */
> +
> +#define ARM64_HW_PGTABLE_LEVEL_SHIFT(n) ((PAGE_SHIFT - 3) * (4 - (n)) +
> 3)
> +
> +#define PTRS_PER_PTE (1 << (PAGE_SHIFT - 3))
> +
> +/*
> + * PMD_SHIFT determines the size a level 2 page table entry can map.
> + */
> +#define PMD_SHIFTARM64_HW_PGTABLE_LEVEL_SHIFT(2)
> +#define PMD_SIZE (_AC(1, UL) << PMD_SHIFT)
> +#define PMD_MASK (~(PMD_SIZE-1))
> #define PTRS_PER_PMD PTRS_PER_PTE
>
> -#define PAGE_PRESENT (1 << 0)
> +/*
> + * PUD_SHIFT determines the size a level 1 page table entry can map.
> + */
> +#define PUD_SHIFTARM64_HW_PGTABLE_LEVEL_SHIFT(1)
> +#define PUD_SIZE (_AC(1, UL) << PUD_SHIFT)
> +#define PUD_MASK (~(PUD_SIZE-1))
> +#define PTRS_PER_PUD
