Re: [XEN v1 7/9] xen/arm: Restrict zeroeth_table_offset for ARM_64

[Julien Grall <julien@xxxxxxx>]

Hi,

On 15/12/2022 19:32, Ayan Kumar Halder wrote:
> zeroeth_table_offset is not accessed for ARM_32.
Right, but what is the problem with keep it? With your proposal we need 
to duplicate the macro DECLARE_OFFSETS() which is not great.
So you want to provide a more compelling reason to have the duplication.

> This is a left over of the following commit
I am not sure why you are saying this is a left-over.

DECLARE_OFFSETS was introduced in 2019 so...

> "
> commit 5fa6e9abfb11
> Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Date:   Thu Sep 18 01:09:48 2014 +0100

... 5 years after there.

>      xen: arm: Implement variable levels in dump_pt_walk
> "
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
>   xen/arch/arm/include/asm/lpae.h | 10 ++++++++++
>   xen/arch/arm/mm.c               |  4 ++++
>   2 files changed, 14 insertions(+)
>
> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
> index 3fdd5d0de2..35769debf9 100644
> --- a/xen/arch/arm/include/asm/lpae.h
> +++ b/xen/arch/arm/include/asm/lpae.h
> @@ -161,6 +161,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned 
> int level)
>   #define lpae_set_mfn(pte, mfn)  ((pte).walk.base = mfn_x(mfn))
>   
>   /* Generate an array @var containing the offset for each level from @addr */
> +#ifdef CONFIG_ARM_64
>   #define DECLARE_OFFSETS(var, addr)          \
>       const unsigned int var[4] = {           \
>           zeroeth_table_offset(addr),         \
> @@ -168,6 +169,15 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned 
> int level)
>           second_table_offset(addr),          \
>           third_table_offset(addr)            \
>       }
> +#else
> +#define DECLARE_OFFSETS(var, addr)          \
> +    const unsigned int var[4] = {           \
> +        0,                                  \
> +        first_table_offset(addr),           \
> +        second_table_offset(addr),          \
> +        third_table_offset(addr)            \
> +    }
> +#endif
>   
>   /*
>    * Standard entry type that we'll use to build Xen's own pagetables.
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 630175276f..be939fb106 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -184,7 +184,11 @@ 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] = {
> +#ifdef CONFIG_ARM_64
>           zeroeth_table_offset(addr),
> +#else
> +        0,
> +#endif
>           first_table_offset(addr),
>           second_table_offset(addr),
>           third_table_offset(addr)
Please use DECLARE_OFFSETS() here.

Cheers,

--
Julien Grall