Re: [PATCH v3 31/52] xen/mpu: make early_fdt_map support in MPU systems

[Julien Grall <julien@xxxxxxx>]

On 13/07/2023 04:12, Penny Zheng wrote:
> > > Thanks ayan for pointing out that RES0 is not suitable for storing 
> > > software-only flags, agreed.
> > > Then, maybe we should refine the existing "struct pr_t" to store these
> > > sw bits, like:
> > > ```
> > > typedef union {
> > >      struct {
> > >         uint8_t tran:1; /* Transient region */
> > >         uint8_t p2m_type:4; /* Used to store p2m types.*/
> > Why do you need the p2m_type?
> I inherited the usage from MMU. Right now, in commit "[PATCH v3 46/52] 
> xen/mpu: look up entry in p2m table", we introduce the first usage to
> tell whether it is a valid P2M MPU memory region. In the future,
> we may also use it to check whether it works as RAM region(p2m_ram_rw).
I find a bit odd to use p2m_type to decide whether this is an MPU memory 
region describing guest information. It would make more sense to 
introduce a boolean 'guest' for now. We can add the type if necessary.
> > >      };
> > >      uint8_t bits;
> > > } pr_flags;
> > >
> > > /* MPU Protection Region */
> > > typedef struct {
> > >          prbar_t prbar;
> > >          prlar_t prlar;
> > >      pr_flags flags;
> > > } pr_t;
> > > ```
> > > The drawback is that, considering the padding, "struct pr_t" expands 
> > > from 16 bytes to 24 bytes.
> > For clarifications, pr_t is going to be used to create an array in 
> > Xen, right? If so, what's the expected size of the array?
> Yes, it is going to be an array. And the maximum length is 255.
> MPUIR_EL2 identifies the number of regions supported by the EL2 MPU,
> which is 8-bits wide.
> The original 16 bytes, even with 255 regions at most, will take up
> less than 4KB. One page is enough. The following definition could have 
> covered all scenarios.
> ```
> /* EL2 Xen MPU memory region mapping table. */
> pr_t __aligned(PAGE_SIZE) __section(".data.page_aligned")
>       xen_mpumap[ARM_MAX_MPU_MEMORY_REGIONS];
> ```
Can you explain why you want the array to be page-aligned?

Cheers,

--
Julien Grall