On Thu, Mar 12, 2015 at 1:57 PM, Julien Grall <julien.gr...@linaro.org>
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@lists.xen.org
http://lists.xen.org/xen-devel

Reply via email to