Re: [Xen-devel] [RFC PATCH V3 10/12] xen: Introduce monitor_op domctl

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

>>
>> Ack, the plan was actually to replace all references to
>> arch.hvm_domain.introspection_enabled with
>> arch.monitor_options.mov_to_msr.extended_capture. I see I forgot to
>> actually fully follow through that plan but that's the intention at
>> least. So the functionality would remain, it would just be worked into
>> the coherent settings field with every other type of events.
>
> I see, but is it conceivable that some vm_event consumer does want to
> enable arch.monitor_options.mov_to_msr.extended_capture but not be
> interested in doing full-blown introspection (for example, is fine with
> having the REP optimization enabled)?

I'm not sure I follow what the difference here between
mov_to_msr.extended_capture and what you refer to as "full-blown
introspection". My understanding was that without gating some MSR's
via introspection_enabled, they are never trapped into the hypervisor
when they get written to, while other MSR do. Thus the two options:
mov_to_msr.enabled for basic trapping, and mov_to_msr.extended_capture
for gating the extended set of MSRs to be also trappable.

>
> What you're proposing here (as far as introspection_enabled is
> concerned) is, if I understand correctly, basically renaming
> introspection_enabled to mov_to_msr.extended_capture. I can see how that
> would seem to simplify some things, but it might not look very intuitive
> to future developers, and it will definitely complicate other things
> down the road.

No, not just simply renaming it. So far the options for the various VM
events were scattered all over the place, yours defined directly on
hvm_domain, others in hvm parameters. Now there is an assigned spot
for all current - and future - events to store their settings: in
arch_domain monitor_settings. This will work for PV domains as well,
while keeping it on the arch_domain layer will avoid defining options
on ARM that are architecture specific (mov_to_cr0/3/4 etc.).

Furthermore, setting these options was also a mess which I try to
address in this patch: some were enabled via hvm_op hypercalls, yours
via domctl. Now everything is moved into one spot: monitor_op domctl.

>
> An example is havig the guest trigger a vm_event, requested by the
> privileged userspace application. In our case, it was VMCALL in the
> original series, and the patch has been eventually dropped from the
> conversation - to be reworked at a later time. But we do need it, and
> our patch now does its thing gated on introspection_enabled. It's hard
> to connect that logic to mov_to_msr.extended_capture.
>
> I guess we could keep the flag and move it to arch.monitor_options if
> you prefer? And setting it could cause mov_to_msr.extended_capture and
> assorted flags to be set also (some sort of umbrella setting)?

Yes, that would be the logic going forward - all VM event related
options and settings should be stored in this structure. Also, I don't
see a problem with having an event or setting that enables multiple at
the same time.

Thanks,
Tamas

>
>
> Thanks,
> Razvan

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