Re: [PATCH V13 1/7] xen/arm: p2m changes for mem_access support

[Julien Grall <julien.grall@linaro.org>]

Hi Tamas,

On 06/03/15 21:24, Tamas K Lengyel wrote:
> @@ -1090,6 +1098,8 @@ void p2m_teardown(struct domain *d)
>  
>      p2m_free_vmid(d);
>  
> +    radix_tree_destroy(&p2m->mem_access_settings, NULL);
> +
>      spin_unlock(&p2m->lock);
>  }
>  
> @@ -1115,6 +1125,10 @@ int p2m_init(struct domain *d)
>      p2m->max_mapped_gfn = 0;
>      p2m->lowest_mapped_gfn = ULONG_MAX;
>  
> +    p2m->default_access = p2m_access_rwx;
> +    p2m->mem_access_enabled = false;

false is defined for bool not bool_t.
Please use 0 here.

[..]

> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index da36504..7583d9b 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -2,8 +2,9 @@
>  #define _XEN_P2M_H
>  
>  #include <xen/mm.h>
> -
> +#include <xen/radix-tree.h>
>  #include <xen/p2m-common.h>
> +#include <public/memory.h>
>  
>  #define paddr_bits PADDR_BITS
>  
> @@ -48,6 +49,18 @@ struct p2m_domain {
>      /* If true, and an access fault comes in and there is no mem_event listener,
>       * pause domain. Otherwise, remove access restrictions. */
>      bool_t access_required;
> +
> +    /* Defines if mem_access is in use for the domain. */
> +    bool_t mem_access_enabled;
> +
> +    /* Default P2M access type for each page in the the domain: new pages,
> +     * swapped in pages, cleared pages, and pages that are ambiguously
> +     * retyped get this access type. See definition of p2m_access_t. */

Coding style. It should be:

/*
 * Default ...
 * ....
 */

> +    p2m_access_t default_access;
> +
> +    /* Radix tree to store the p2m_access_t settings as the pte's don't have
> +     * enough available bits to store this information. */

Ditto

> +    struct radix_tree_root mem_access_settings;
>  };
>  
>  /* List of possible type for each page in the p2m entry.
> @@ -217,6 +230,26 @@ static inline int get_page_and_type(struct page_info *page,
>  /* get host p2m table */
>  #define p2m_get_hostp2m(d) (&(d)->arch.p2m)
>  
> +/* mem_event and mem_access are supported on any ARM guest */
> +static inline bool_t p2m_mem_access_sanity_check(struct domain *d)
> +{
> +    return 1;
> +}
> +
> +static inline bool_t p2m_mem_event_sanity_check(struct domain *d)
> +{
> +    return 1;
> +}
> +
> +/* Get access type for a pfn
> + * If pfn == -1ul, gets the default access type */

Ditto

Regards,

-- 
Julien Grall