Re: [Xen-devel] [PATCH v4 08/10] xen/arm: mm: Embed permission in the flags

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 9 Oct 2017, Julien Grall wrote:
> Currently, it is not possible to specify the permission of a new
> mapping. It would be necessary to use the function modify_xen_mappings
> with a different set of flags.
> 
> Introduce a couple of new flags for the permissions (Non-eXecutable,
> Read-Only) and also provides definition that combine the memory attribute
> and permission for common combinations.
> 
> PAGE_HYPERVISOR is now an alias to PAGE_HYPERVISOR_RW (read-write,
> non-executable mappings). This does not affect the current mapping using
> PAGE_HYPERVISOR because Xen is currently forcing all the mapping to be
> non-executable by default (see mfn_to_xen_entry).
> 
> A follow-up patch will change modify_xen_mappings to use the new flags.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
> 
>     Changes in v3:
>         - Add a comment about _PAGE_DEVICE and _PAGE_NORMAL
> 
>     Changes in v2:
>         - Update the commit message
> ---
>  xen/include/asm-arm/page.h | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index aa3e83f5b4..e2b3e402d0 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -69,12 +69,31 @@
>   * Layout of the flags used for updating the hypervisor page tables
>   *
>   * [0:2] Memory Attribute Index
> + * [3:4] Permission flags
>   */
>  #define PAGE_AI_MASK(x) ((x) & 0x7U)
>  
> -#define PAGE_HYPERVISOR         (MT_NORMAL)
> -#define PAGE_HYPERVISOR_NOCACHE (MT_DEVICE_nGnRE)
> -#define PAGE_HYPERVISOR_WC      (MT_NORMAL_NC)
> +#define _PAGE_XN_BIT    3
> +#define _PAGE_RO_BIT    4
> +#define _PAGE_XN    (1U << _PAGE_XN_BIT)
> +#define _PAGE_RO    (1U << _PAGE_RO_BIT)
> +#define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x1U)
> +#define PAGE_RO_MASK(x) (((x) >> _PAGE_RO_BIT) & 0x1U)
> +
> +/*
> + * _PAGE_DEVICE and _PAGE_NORMAL are conveniences defines. They are not
> + * meant to be used outside of the headers.

just grammar NITs:
  _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
  meant to be used outside of this header.
I'll fix on commit

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

> + */
> +#define _PAGE_DEVICE    _PAGE_XN
> +#define _PAGE_NORMAL    MT_NORMAL
> +
> +#define PAGE_HYPERVISOR_RO      (_PAGE_NORMAL|_PAGE_RO|_PAGE_XN)
> +#define PAGE_HYPERVISOR_RX      (_PAGE_NORMAL|_PAGE_RO)
> +#define PAGE_HYPERVISOR_RW      (_PAGE_NORMAL|_PAGE_XN)
> +
> +#define PAGE_HYPERVISOR         PAGE_HYPERVISOR_RW
> +#define PAGE_HYPERVISOR_NOCACHE (_PAGE_DEVICE|MT_DEVICE_nGnRE)
> +#define PAGE_HYPERVISOR_WC      (_PAGE_DEVICE|MT_NORMAL_NC)
>  
>  /*
>   * Defines for changing the hypervisor PTE .ro and .nx bits. This is only to 
> be
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel