Re: [PATCH for-4.5 v8 12/19] xen/arm: p2m changes for mem_access support

From: Razvan Cojocaru <rcojocaru@bitdefender.com>
To: Tamas K Lengyel <tamas.lengyel@zentific.com>,
	Ian Campbell <Ian.Campbell@citrix.com>
Cc: Tim Deegan <tim@xen.org>, Julien Grall <julien.grall@linaro.org>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	Andres Lagar-Cavilla <andres@lagarcavilla.org>,
	Jan Beulich <jbeulich@suse.com>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	Tamas K Lengyel <tklengyel@sec.in.tum.de>
Subject: Re: [PATCH for-4.5 v8 12/19] xen/arm: p2m changes for mem_access support
Date: Wed, 24 Sep 2014 20:14:31 +0300	[thread overview]
Message-ID: <5422FBF7.3050509@bitdefender.com> (raw)
In-Reply-To: <CAErYnsiY2nhDG9=kuT7OB15bqP-8st-5fzOuBpCdpuRik-tbzA@mail.gmail.com>

On 09/24/14 19:58, Tamas K Lengyel wrote:
> 
> 
> On Wed, Sep 24, 2014 at 4:40 PM, Ian Campbell <Ian.Campbell@citrix.com
> <mailto: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.

Indeed, that's used (at the moment) to make sure x86 MSR write
interception doesn't get disabled for memory introspection clients. I
guess on ARM it is less useful now, but since hopefully ARM development
of this feature will happen at some point in the future, it would
explain Tamas' choice.

Regards,
Razvan Cojocaru