Re: [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Tamas K Lengyel <tamas@tklengyel.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	"wei.liu2@citrix.com" <wei.liu2@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>,
	Jun Nakajima <jun.nakajima@intel.com>, Keir Fraser <keir@xen.org>
Subject: Re: [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s
Date: Wed, 13 Apr 2016 16:01:35 +0100	[thread overview]
Message-ID: <570E5F4F.7070506@citrix.com> (raw)
In-Reply-To: <570E5E18.9050309@bitdefender.com>

On 13/04/16 15:56, Razvan Cojocaru wrote:
> On 04/13/2016 05:52 PM, Tamas K Lengyel wrote:
>>     >> diff --git a/xen/include/public/domctl.h
>>     b/xen/include/public/domctl.h
>>     >> index 2457698..875c09a 100644
>>     >> --- a/xen/include/public/domctl.h
>>     >> +++ b/xen/include/public/domctl.h
>>     >> @@ -1107,8 +1107,7 @@ struct xen_domctl_monitor_op {
>>     >>          } mov_to_cr;
>>     >>
>>     >>          struct {
>>     >> -            /* Enable the capture of an extended set of MSRs */
>>     >> -            uint8_t extended_capture;
>>     >> +            uint32_t msr;
>>     >
>>     > Whoa there. Isn't it expanding the structure? Will this be backwards
>>     > compatible? What if somebody is using an older version of xen-access
>>     > against this hypervisor? Will they work?
>>     >
>>     > Perhaps this should have a new struct / sub-ops? And the old
>>     > 'mov_to_msr' will just re-use this new fangled code?
>>
>>     In addition to Andrew's comments, I think simply changing
>>     VM_EVENT_INTERFACE_VERSION should be enough for xen-access-like clients
>>     to figure out the incompatibility.
>>
>>
>>
>> This is an independent system from VM_EVENT, so IMHO the two shouldn't
>> be mixed. The union size right now is 24-bits so if a uint16_t is enough
>> for the bitmask that should be used instead. That way we don't end up
>> growing the struct size.
> Right. Well, MSR-s seem to be passed around as 32-bit unsigned integers
> everywhere in the Xen source code, so unless that also needs correcting
> then unfortunately it'll have to grow.

MSR indices are always 32bits wide, as they live specifically in %ecx
when encoded for instructions.

Only 2K MSRs are currently specified in hardware, with some extra ones
in the hypervisor range, but this doesn't mean that list won't grow in
the future.

~Andrew

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