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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

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.


Thanks,
Razvan

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