Re: [Xen-devel] [PATCH 1/3] xen/arm: p2m: extend create_p2m_entries to support read-only mapping

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-10-24 at 10:02 +0100, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/p2m.c         | 19 +++++++++++--------
>  xen/include/asm-arm/page.h |  5 +++--
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2d09fef..fdbb07b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -111,7 +111,7 @@ static int p2m_create_table(struct domain *d,
>      clear_page(p);
>      unmap_domain_page(p);
>  
> -    pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM);
> +    pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM, 1);

If this is a bool_t then true/false might be more appropriate? (and all
the other such too)

I have a weak preference for avoiding such boolean parameters, since it
is never obvious what a given 1/0/true/false is actually is without
having to look at the prototype. At least there is only 1 such param
here (now).

We don't need full p2m typing yet but perhaps we can make a start by
defining "typdef enum { p2m_ram_rw = N; p2m_ram_ro = M } p2m_type_t" for
this?

> @@ -213,14 +213,15 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn)
>      return e;
>  }
>  
> -static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
> +static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
> +                                      bool_t rw)
>  {
>      paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
>      lpae_t e = (lpae_t) {
>          .p2m.xn = 0,
>          .p2m.af = 1,
>          .p2m.sh = LPAE_SH_OUTER,
> -        .p2m.write = 1,
> +        .p2m.write = !!rw,

I don't think the !! is strictly necessary for either a bool_t nor a
single bit bitfield.

Ian.


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