On Wed, Jun 01, 2016 at 10:32:56AM -0400, Dave Anderson wrote:
>
> ----- Original Message -----
> > On Tue, May 31, 2016 at 03:30:44PM -0400, Dave Anderson wrote:
> > >
> > > This patch looks good -- if it wasn't layered on top of the KASLR patch,
> > > I would check it into github.
> >
> > This patch is almost independent from KASLR support, except that
> > adding new flags, VM_L4_4K and NEW_VMEMMAP, is conflicting.
> >
> > When you merge it, please make a change.
> >
> > -Takahiro AKASHI
>
> OK, I see that now. But upon further inspection, I've got another question
> about this piece:
>
> + * 4-levels / 4K pages
> + * 48-bit VA
> + */
> +#define PTRS_PER_PGD_L4_4K ((1UL) << (48 - 39))
> +#define PTRS_PER_PUD_L4_4K (512)
> +#define PTRS_PER_PMD_L4_4K (512)
> +#define PTRS_PER_PTE_L4_4K (512)
> +#define PGDIR_SHIFT_L4_4K (39)
> +#define PGDIR_SIZE_L4_4K ((1UL) << PGDIR_SHIFT_L4_4K)
> +#define PGDIR_MASK_L4_4K (~(PGDIR_SIZE_L4_4K-1))
> +#define PUD_SHIFT_L4_4K (30)
> +#define PUD_SIZE_L4_4K ((1UL) << PUD_SHIFT_L4_4K)
> +#define PUD_MASK_L4_4K (~(PUD_SIZE_L4_4K-1))
> +#define PMD_SHIFT_L4_4K (21)
> +#define PMD_SIZE_L4_4K (1UL << PMD_SHIFT_L4_4K)
> +#define PMD_MASK_L4_4K (~(PMD_SIZE_L4_4K-1))
> +
> +#define PGDIR_SIZE (1UL << ((48 - 39) + 3))
> +#define PGDIR_MASK (~(PGDIR_SIZE - 1))
> +#define PGDIR_OFFSET(X) (((ulong)(X)) & (PGDIR_SIZE - 1))
> +
>
> You have defined PGDIR_SIZE_L4_4K and PGDIR_MASK_L4_4K, but they
> are not used in arm64.c:
>
> $ grep -e PGDIR_SIZE_L4_4K -e PGDIR_MASK_L4_4K arm64.c
> $
It is kinda copy-and-paste from L3_4K.
PGDIR_MASK_L3_4K/PMD_SIZE_L3_4K/PMD_MASK_L3_4K are not used neither.
> Instead you define PGDIR_SIZE and PGDIR_MASK without the _L4_4K qualifiers,
> and use them and PGDIR_OFFSET() in arm64.c.
>
> Preferably convention should be followed such that the _L4_4K versions would
> be used, but I see that PGDIR_SIZE_L4_4K is not equal to PGDIR_SIZE, and that
> PGDIR_MASK_L4_4K is not equal to PGDIR_MASK. What's going on there?
As you can see, those macros are used to calculate pgd_val and pud_base.
I'd like to use the same formula as in the other level of tables,
but the size and mask can be varied depending on a value of VA_BITS
for 4-level translation.
For 48-bit VA, luckily or not, PAGEOFFSET() would work instead of
PGDIR_OFFSET(), and likewise machdep->pagemask instead of PGDIR_MASK,
but we should not use them.
That is why I define the macros as
#define PGDIR_SIZE (1UL << ((48 - 39) + 3))
But I will agree that we'd better give them different names to avoid
any confusion.
Any suggestions?
Thanks,
-Takahiro AKASHI
> Dave
>
>
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
--
Thanks,
-Takahiro AKASHI
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
