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

[Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>]

On Thu, Mar 12, 2015 at 1:57 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:

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.

Ack.

Â

[..]

> 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

Ack, however, all other comments here followed this style even for multi-line comments. If I change the style only on my comments it will be mixed (and ugly) IMHO.

Â

Regards,

--
Julien Grall

Thanks,

Tamas

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