Re: [PATCH 2/2] xen: arm: Set all bits in mfn_to_xen_entry()

[Julien Grall <julien.grall@citrix.com>]

Hi Chris,

On 14/08/2015 14:42, Chris (Christopher) Brand wrote:
> Ensure that every bit has a specific value.
>
> Reported-by: Julien Grall <julien.grall@citrix.com>
> Signed-off-by: Chris Brand <chris.brand@broadcom.com>
> ---
>   xen/include/asm-arm/page.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 01628f3e96cb..7a56b2cb463a 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -202,9 +202,14 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>               .ai = attr,
>               .ns = 1,              /* Hyp mode is in the non-secure world */
>               .user = 1,            /* See below */
> +            .ro = 0,              /* Assume read-write */
>               .af = 1,              /* No need for access tracking */
>               .ng = 1,              /* Makes TLB flushes easier */
> +            .sbz = 0,
> +            .contig = 0,          /* Assume non-contiguous */
> +            .pxn = 0,

I would add a comment to explain that this bit is reserved for PL2 stage 
1 page table.

>               .xn = 1,              /* No need to execute outside .text */
> +            .avail = 0,

I don't think this one is necessary. avail is not used by the hardware 
neither Xen.

>           }};;

what about *t fields (pxnt, xnt, apt,...)?

>       /* Setting the User bit is strange, but the ATS1H[RW] instructions
>        * don't seem to work otherwise, and since we never run on Xen
>

Regards,


-- 
Julien Grall