Re: [PATCH v3 28/52] xen/mpu: plump virt/maddr conversion in MPU system

[Julien Grall <julien@xxxxxxx>]

Hi,

On 29/06/2023 15:44, Ayan Kumar Halder wrote:
> On 29/06/2023 15:23, Andrew Cooper wrote:
> > On 29/06/2023 3:20 pm, Ayan Kumar Halder wrote:
> > > On 26/06/2023 04:34, Penny Zheng wrote:
> > > > diff --git a/xen/arch/arm/include/asm/mm.h
> > > > b/xen/arch/arm/include/asm/mm.h
> > > > index eb520b49e3..ea4847c12b 100644
> > > > --- a/xen/arch/arm/include/asm/mm.h
> > > > +++ b/xen/arch/arm/include/asm/mm.h
> > > > @@ -292,6 +301,12 @@ static inline void *maddr_to_virt(paddr_t ma)
> > > >                         ((ma & ma_top_mask) >> pfn_pdx_hole_shift)));
> > > >    }
> > > >    #endif
> > > > +#else /* CONFIG_HAS_MPU */
> > > > +static inline void *maddr_to_virt(paddr_t ma)
> > > > +{
> > > > +    return (void *)(unsigned long)ma;
> > > Why do you need "(unsigned long)ma" ? Both "unsigned long" and
> > > "paddr_t" are u64.
> > For when paddr_t really isn't u64.
> Sorry, I am missing something
>
>  From 
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/include/asm/types.h;h=fb6618ef247fe8e3abe472e50b4877e11cc8a96c;hb=refs/heads/staging
> In CONFIG_ARM_64
>
> typedef unsigned long u64;
>
> typedef u64 paddr_t;
>
> So, why do we need to typecast "paddr_t" to "unsigned long" as they are 
> the same ?
We may decide to restrict paddr_t to 32-bit in the future on Arm64.

Also, when CONFIG_PHYS_ADDR_T_32=n, paddr_t is 64-bit on 32-bit Xen. So 
casting directly to (void *) is not possible. Although, this is not a 
problem here because for the MPU on 32-bit, you would select 
CONFIG_PHYS_ADDR_T_32.
On a side note, I agree with Andrew that we should switch to _p().

Cheers,

--
Julien Grall