From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH for-4.5 v8 12/19] xen/arm: p2m changes for mem_access support Date: Wed, 24 Sep 2014 18:58:42 +0200 Message-ID: References: <1411478070-13836-1-git-send-email-tklengyel@sec.in.tum.de> <1411478070-13836-13-git-send-email-tklengyel@sec.in.tum.de> <1411569648.28127.48.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8098744003291078438==" Return-path: In-Reply-To: <1411569648.28127.48.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Tim Deegan , Julien Grall , Ian Jackson , "xen-devel@lists.xen.org" , Stefano Stabellini , Andres Lagar-Cavilla , Jan Beulich , Daniel De Graaf , Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org --===============8098744003291078438== Content-Type: multipart/alternative; boundary=089e0149d2e8a9f0980503d298a3 --089e0149d2e8a9f0980503d298a3 Content-Type: text/plain; charset=ISO-8859-1 On Wed, Sep 24, 2014 at 4:40 PM, Ian Campbell 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 --089e0149d2e8a9f0980503d298a3 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable


On Wed, Sep 24, 2014 at 4:40 PM, Ian Campbell <Ian.Campbell@citr= ix.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 informatio= n.
> 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.
=A0

> 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
>=A0 {
>=A0 =A0 =A0 uint64_t=A0 =A0 =A0 =A0 =A0 =A0 =A0 params[HVM_NR_PARAMS];<= br> >=A0 =A0 =A0 struct hvm_iommu=A0 =A0 =A0 iommu;
> +=A0 =A0 bool_t=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 introspection_enabled;<= br>
I've not found the user of this yet, but is it not somewhat redu= ndant
with p2m->access_in_use?

It's us= ed by mem_event memop for XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE. I just ad= ded this one field to the hvm_domain for compat reasons instead of abstract= ing setting that value to 0 and adding an empty inline for ARM. This looked= like less trouble.
=A0

>=A0 }=A0 __cacheline_aligned;
>
>=A0 #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 o= n the backing page. */
> +typedef unsigned int p2m_query_t;
> +#define P2M_ALLOC=A0 =A0 (1u<<0)=A0 =A0/* Populate PoD and page= d-out entries */
> +#define P2M_UNSHARE=A0 (1u<<1)=A0 =A0/* Break CoW sharing */
Are the a result of something going wrong in a rebase? The comment h= ere
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 fro= m the previous iteration.
=A0

> +=A0 =A0 /* Defines if mem_access is in use for the domain to avoid un= eccessary radix

"unnecessary"

Ack.
=
=A0

> +=A0 =A0 =A0* lookups. */
> +=A0 =A0 bool_t access_in_use;

> @@ -77,6 +97,7 @@ void p2m_mem_event_emulate_c= heck(struct domain *d,
>=A0 =A0 =A0 /* Not supported on ARM. */
>=A0 };
>
> +static inline
>=A0 void p2m_enable_msr_exit_interception(struct domain *d)

I think this must belong in a previous patch which added this fn?
Ian.

Yes a= nd has been since renamed and fixed.

Thanks!
Tamas
--089e0149d2e8a9f0980503d298a3-- --===============8098744003291078438== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8098744003291078438==--