Re: [PATCH v1] xen/arm: AArch32-V8R: Add MPU register definitions

[Julien Grall <julien@xxxxxxx>]

Hi Ayan

On 25/04/2025 13:00, Ayan Kumar Halder wrote:
> > > +        unsigned int ns:1;     /* Reserved 0 by hardware */
> > > +        unsigned int res0:1;   /* Reserved 0 by hardware */
> Then, we can change this on Arm32 as well.
> > > +        unsigned int limit:26;  /* Limit Address */
> > NIT: Can we align the comments?
> Ack.
> > > +    } reg;
> > > +    uint32_t bits;
> > > +} prlar_t;
> > > +
> > > +/* Protection Region */
> > > +typedef struct {
> > > +    prbar_t prbar;
> > > +    prlar_t prlar;
> > > +    uint64_t p2m_type;          /* Used to store p2m types. */
> > > +} pr_t;
> > This looks to be common with arm64. If so, I would prefer if the 
> > structure is in a common header.
> No, in arm64 this is
>
> typedef struct {
>       prbar_t prbar;
>       prlar_t prlar;
> } pr_t;
>
> The reason being Arm64 uses unused bits (ie 'pad') to store the type.
Hmmm... They are bits from the system register (prbar), right? If so, 
you can't use them for storing the p2m_type as AFAICT they are RES0.
You could possibly mask the bits before writing them. But then, it will 
become risky if the fields are defined.
Also, the number of MPU regions is fairly small. So, I don't think it is 
worth it to store p2m_type.
Regardless that, I think it would be better to defer the introduction of 
p2m_type until guest support is added. So it will be easier to 
understand how this is going to be used and the size (for instance, I 
doubt 64-bit is necessary...).
Cheers,

--
Julien Grall