On Wed, Sep 24, 2014 at 4:40 PM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:

> On Tue, 2014-09-23 at 15:14 +0200, Tamas K Lengyel wrote:
> > Add p2m_access_t to struct page_info and add necessary changes for
> > page table construction routines to pass the default access information.
> > We store the p2m_access_t info in page_info as the PTE lacks enough
> > software programmable bits.
>
> The last bit of this is now out of date.
>

Ack.


>
> > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > index 787e93c..3d69152 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -17,6 +17,7 @@ struct hvm_domain
> >  {
> >      uint64_t              params[HVM_NR_PARAMS];
> >      struct hvm_iommu      iommu;
> > +    bool_t                introspection_enabled;
>
> I've not found the user of this yet, but is it not somewhat redundant
> with p2m->access_in_use?
>

It's used by mem_event memop for XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE. I
just added this one field to the hvm_domain for compat reasons instead of
abstracting setting that value to 0 and adding an empty inline for ARM.
This looked like less trouble.


>
> >  }  __cacheline_aligned;
> >
> >  #ifdef CONFIG_ARM_64
> > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> > index 27225c4..dba0df5 100644
> > --- a/xen/include/asm-arm/p2m.h
> > +++ b/xen/include/asm-arm/p2m.h
>
> > +/* Look up a GFN and take a reference count on the backing page. */
> > +typedef unsigned int p2m_query_t;
> > +#define P2M_ALLOC    (1u<<0)   /* Populate PoD and paged-out entries */
> > +#define P2M_UNSHARE  (1u<<1)   /* Break CoW sharing */
>
> Are the a result of something going wrong in a rebase? The comment here
> refers to get_page_from_gfn which you've left behind, so it is out of
> context now.
>
> I don't think your other changes here require any of this to move, do
> they?
>

Ack, these are just artifacts from the previous iteration.


>
> > +    /* Defines if mem_access is in use for the domain to avoid
> uneccessary radix
>
> "unnecessary"
>

Ack.


>
> > +     * lookups. */
> > +    bool_t access_in_use;
>
> > @@ -77,6 +97,7 @@ void p2m_mem_event_emulate_check(struct domain *d,
> >      /* Not supported on ARM. */
> >  };
> >
> > +static inline
> >  void p2m_enable_msr_exit_interception(struct domain *d)
>
> I think this must belong in a previous patch which added this fn?
>
> Ian.
>

Yes and has been since renamed and fixed.

Thanks!
Tamas