Xen project Mailing List

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

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

On 09/23/2014 05:13 PM, Tamas K Lengyel wrote:
>
>
> On Tue, Sep 23, 2014 at 4:07 PM, Jan Beulich <JBeulich@xxxxxxxx
> <mailto:JBeulich@xxxxxxxx>> wrote:
>
> >>> On 23.09.14 at 16:00, <rcojocaru@xxxxxxxxxxxxxxx <mailto:rcojocaru@xxxxxxxxxxxxxxx>> wrote:
> > On 09/23/2014 04:32 PM, Jan Beulich wrote:

> >>>>> On 23.09.14 at 15:14, <tklengyel@xxxxxxxxxxxxx <mailto:tklengyel@xxxxxxxxxxxxx>> 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

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