Re: [PATCH for-4.5 v8 06/19] xen: Relocate mem_event_op domctl and access_op memop into common.

From: Tamas K Lengyel <tamas.lengyel@zentific.com>
To: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Ian Campbell <ian.campbell@citrix.com>, 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 06/19] xen: Relocate mem_event_op domctl and access_op memop into common.
Date: Tue, 23 Sep 2014 16:28:52 +0200	[thread overview]
Message-ID: <CAErYnsgt0unZtFPLWjKm0a0Ov7d3vr-s8ZonTVDWa_Ft2A_2bQ@mail.gmail.com> (raw)
In-Reply-To: <5421825E.1040607@bitdefender.com>

[-- Attachment #1.1: Type: text/plain, Size: 3083 bytes --]

On Tue, Sep 23, 2014 at 4:23 PM, Razvan Cojocaru <rcojocaru@bitdefender.com>
wrote:

> On 09/23/2014 05:13 PM, Tamas K Lengyel wrote:
> >
> >
> > On Tue, Sep 23, 2014 at 4:07 PM, Jan Beulich <JBeulich@suse.com
> > <mailto:JBeulich@suse.com>> wrote:
> >
> >     >>> On 23.09.14 at 16:00, <rcojocaru@bitdefender.com <mailto:
> rcojocaru@bitdefender.com>> wrote:
> >     > On 09/23/2014 04:32 PM, Jan Beulich wrote:
> >     >>>>> On 23.09.14 at 15:14, <tklengyel@sec.in.tum.de <mailto:
> tklengyel@sec.in.tum.de>> wrote:
> >     >>> --- a/xen/common/mem_event.c
> >     >>> +++ b/xen/common/mem_event.c
> >     >>> @@ -623,12 +623,9 @@ int mem_event_domctl(struct domain *d,
> >     >>> xen_domctl_mem_event_op_t *mec,
> >     >>>                                      HVM_PARAM_ACCESS_RING_PFN,
> >     >>>                                      mem_access_notification);
> >     >>>
> >     >>> -            if ( mec->op !=
> XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE &&
> >     >>> -                 rc == 0 &&
> hvm_funcs.enable_msr_exit_interception )
> >     >>> -            {
> >     >>> -                d->arch.hvm_domain.introspection_enabled = 1;
> >     >>> -                hvm_funcs.enable_msr_exit_interception(d);
> >     >>> -            }
> >     >>> +            if ( !rc && mec->op !=
> XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE )
> >     >>> +                p2m_enable_msr_exit_interception(d);
> >     >>
> >     >> The name is clearly not suitable for an abstraction - there's
> certainly
> >     >> not going to be MSRs on each and every CPU architecture. Maybe
> >     >> consult with Razvan on an agreeable more suitable name.
> >     >
> >     > P2m_set_up_introspection() perhaps? With the MSR HVM code where
> >     > applicable, nothing (or something else) where not? Would this be
> too
> >     > generic?
> >
> >     I'd be fine with that name provided the != above gets converted
> >     to a == XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE_INTROSPECTION.
> >
> >     Jan
> >
> >
> > My problem with this name is that introspection is really way too
> > generic of a term. You can certainly do all sorts of introspection
> > without having this feature or using this feature.. Ultimately its just
> > a name so if this becomes Xen's terminology to mean this particular
> > feature I'm fine with it but that's going to be confusing when other
> > people talk about 'introspection'.
>
> "Introspection" in general, yes, is a bit generic. However, the
> "MEM_EVENT_OP_ACCESS" part of
> XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE_INTROSPECTION, and the "p2m_" part
> of "p2m_set_up_introspection()" would, I think, narrow it down a bit more.
>
> But it is, of course, ultimately up to you (and the Xen maintainers). It
> was merely a suggestion.
>
>
> Regards,
> Razvan Cojocaru
>

Thanks, I guess we can keep that name for the function with a comment
saying this is used to enable additional arch-specific introspection
options, such as MSR interception on x86. Maybe there will be more in the
future which could be put in here beside what it is used for at the moment.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 4426 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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