Re: [PATCH v5 1/3] xen/arm: mpu: Create boot-time MPU protection regions

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 16/11/2024 10:30, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
> On 07/11/2024 15:03, Ayan Kumar Halder wrote:
> > +/*
> > + * Macro to prepare and set a EL2 MPU memory region.
> > + * We will also create an according MPU memory region entry, which
> > + * is a structure of pr_t,  in table \prmap.
> > + *
> > + * Inputs:
This needs to be removed as some parameters are both input and output 
(eg sel) and some parameters are for temporary storage (eg prbar, prlar).
> > + * sel:         region selector
> ``sel`` is now also an output. But looking at the description, we 
> don't seem to say which registers are preserved or not. For instance, 
> both ..
> > + * base:        reg storing base address (should be page-aligned) > 
> > + * limit:       reg storing limit address
> ... the two registers above will be clobbered. On the other hand...
>
> > + * prbar:       store computed PRBAR_EL2 value
> > + * prlar:       store computed PRLAR_EL2 value
> > + * maxcount:    maximum number of EL2 regions supported
> ... this will be preserved. So I think we need to clarify it. For 
> instance, this is the description of create_table_entry_from_paddr:
Actually prbar, prlar are similar to tmp1, tmp2 (ie scratch registers). 
I used the names so as to make it more meaningful.
> /*
>  * Macro to create a page table entry in \ptbl to \tbl
>  * ptbl:    table symbol where the entry will be created
>  * tbl:     physical address of the table to point to
>  * virt:    virtual address
>  * lvl:     page-table level
>  * tmp1:    scratch register
>  * tmp2:    scratch register
>  *
>  * Preserves \virt
>  * Clobbers \tbl, \tmp1, \tmp2
>  *
>  * Note that all parameters using registers should be distinct.
>  */
In our case, it will be

/*
 * Macro to prepare and set a EL2 MPU memory region.
 * We will also create an according MPU memory region entry, which
 * is a structure of pr_t,  in table \prmap.
 *
 * sel:         region selector
 * base:        reg storing base address
 * limit:       reg storing limit address
 * prbar:       store computed PRBAR_EL2 value
 * prlar:       store computed PRLAR_EL2 value
 * maxcount:    maximum number of EL2 regions supported
 * attr_prbar:  PRBAR_EL2-related memory attributes. If not specified 
it will be
 * REGION_DATA_PRBAR
 * attr_prlar:  PRLAR_EL2-related memory attributes. If not specified 
it will be
 *              REGION_NORMAL_PRLAR
 *
 * Preserves \maxcount
 * Clobbers \sel, \base, \limit, \prbar, \prlar
 *
 * Note that all parameters using registers should be distinct.
 */

- Ayan