| Message ID | 20230208120529.22313-9-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for 32 bit physical address | expand |
On Wed, 8 Feb 2023, Ayan Kumar Halder wrote: > When 32 bit physical addresses are used (ie ARM_PA_32=y), > "va >> ZEROETH_SHIFT" causes an overflow. > Also, there is no zeroeth level page table on Arm 32-bit. > > Also took the opportunity to clean up dump_pt_walk(). One could use > DECLARE_OFFSETS() macro instead of declaring the declaring an array > of page table offsets. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes from - > > v1 - Removed the duplicate declaration for DECLARE_OFFSETS. > > v2 - 1. Reworded the commit message. > 2. Use CONFIG_ARM_PA_32 to restrict zeroeth_table_offset. > > xen/arch/arm/include/asm/lpae.h | 4 ++++ > xen/arch/arm/mm.c | 7 +------ > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h > index 3fdd5d0de2..998edeed6e 100644 > --- a/xen/arch/arm/include/asm/lpae.h > +++ b/xen/arch/arm/include/asm/lpae.h > @@ -259,7 +259,11 @@ lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr); > #define first_table_offset(va) TABLE_OFFSET(first_linear_offset(va)) > #define second_table_offset(va) TABLE_OFFSET(second_linear_offset(va)) > #define third_table_offset(va) TABLE_OFFSET(third_linear_offset(va)) > +#ifdef CONFIG_ARM_PA_32 > +#define zeroeth_table_offset(va) 0 > +#else > #define zeroeth_table_offset(va) TABLE_OFFSET(zeroeth_linear_offset(va)) > +#endif > > /* > * Macros to define page-tables: > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index b99806af99..44942c6a4f 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -221,12 +221,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr, > { > static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" }; > const mfn_t root_mfn = maddr_to_mfn(ttbr); > - const unsigned int offsets[4] = { > - zeroeth_table_offset(addr), > - first_table_offset(addr), > - second_table_offset(addr), > - third_table_offset(addr) > - }; > + DECLARE_OFFSETS(offsets, addr); > lpae_t pte, *mapping; > unsigned int level, root_table; > > -- > 2.17.1 > >
Hi, On 08/02/2023 12:05, Ayan Kumar Halder wrote: > When 32 bit physical addresses are used (ie ARM_PA_32=y), > "va >> ZEROETH_SHIFT" causes an overflow. > Also, there is no zeroeth level page table on Arm 32-bit. > > Also took the opportunity to clean up dump_pt_walk(). One could use > DECLARE_OFFSETS() macro instead of declaring the declaring an array > of page table offsets. Technically this is unrelated to the goal of the patch. So this should have been split in a separate patch. No need to split it this time, but in the future please consider to split a patch when a change is unrelated. In general, I would only consider to bundle clean-up if all the below applies: 1) the patch is not doing code movement 2) the change is simple (e.g. typo, coding style fix) 3) the change is in the vicinity of the the rest of code (i.e. it would be visible in the diff). > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> Acked-by: Julien Grall <jgrall@amazon.com> > --- > Changes from - > > v1 - Removed the duplicate declaration for DECLARE_OFFSETS. > > v2 - 1. Reworded the commit message. > 2. Use CONFIG_ARM_PA_32 to restrict zeroeth_table_offset. > > xen/arch/arm/include/asm/lpae.h | 4 ++++ > xen/arch/arm/mm.c | 7 +------ > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h > index 3fdd5d0de2..998edeed6e 100644 > --- a/xen/arch/arm/include/asm/lpae.h > +++ b/xen/arch/arm/include/asm/lpae.h > @@ -259,7 +259,11 @@ lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr); > #define first_table_offset(va) TABLE_OFFSET(first_linear_offset(va)) > #define second_table_offset(va) TABLE_OFFSET(second_linear_offset(va)) > #define third_table_offset(va) TABLE_OFFSET(third_linear_offset(va)) > +#ifdef CONFIG_ARM_PA_32 > +#define zeroeth_table_offset(va) 0 > +#else > #define zeroeth_table_offset(va) TABLE_OFFSET(zeroeth_linear_offset(va)) > +#endif > > /* > * Macros to define page-tables: > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index b99806af99..44942c6a4f 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -221,12 +221,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr, > { > static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" }; > const mfn_t root_mfn = maddr_to_mfn(ttbr); > - const unsigned int offsets[4] = { > - zeroeth_table_offset(addr), > - first_table_offset(addr), > - second_table_offset(addr), > - third_table_offset(addr) > - }; > + DECLARE_OFFSETS(offsets, addr); > lpae_t pte, *mapping; > unsigned int level, root_table; > Cheers,
diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h index 3fdd5d0de2..998edeed6e 100644 --- a/xen/arch/arm/include/asm/lpae.h +++ b/xen/arch/arm/include/asm/lpae.h @@ -259,7 +259,11 @@ lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr); #define first_table_offset(va) TABLE_OFFSET(first_linear_offset(va)) #define second_table_offset(va) TABLE_OFFSET(second_linear_offset(va)) #define third_table_offset(va) TABLE_OFFSET(third_linear_offset(va)) +#ifdef CONFIG_ARM_PA_32 +#define zeroeth_table_offset(va) 0 +#else #define zeroeth_table_offset(va) TABLE_OFFSET(zeroeth_linear_offset(va)) +#endif /* * Macros to define page-tables: diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index b99806af99..44942c6a4f 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -221,12 +221,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr, { static const char *level_strs[4] = { "0TH", "1ST", "2ND", "3RD" }; const mfn_t root_mfn = maddr_to_mfn(ttbr); - const unsigned int offsets[4] = { - zeroeth_table_offset(addr), - first_table_offset(addr), - second_table_offset(addr), - third_table_offset(addr) - }; + DECLARE_OFFSETS(offsets, addr); lpae_t pte, *mapping; unsigned int level, root_table;
When 32 bit physical addresses are used (ie ARM_PA_32=y), "va >> ZEROETH_SHIFT" causes an overflow. Also, there is no zeroeth level page table on Arm 32-bit. Also took the opportunity to clean up dump_pt_walk(). One could use DECLARE_OFFSETS() macro instead of declaring the declaring an array of page table offsets. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from - v1 - Removed the duplicate declaration for DECLARE_OFFSETS. v2 - 1. Reworded the commit message. 2. Use CONFIG_ARM_PA_32 to restrict zeroeth_table_offset. xen/arch/arm/include/asm/lpae.h | 4 ++++ xen/arch/arm/mm.c | 7 +------ 2 files changed, 5 insertions(+), 6 deletions(-)