From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s Date: Wed, 13 Apr 2016 08:52:49 -0600 Message-ID: References: <1460524280-6902-1-git-send-email-rcojocaru@bitdefender.com> <20160413094726.GA2133@localhost.localdomain> <570E342F.5060606@bitdefender.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7421626838687426808==" Return-path: In-Reply-To: <570E342F.5060606@bitdefender.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Razvan Cojocaru Cc: Kevin Tian , "wei.liu2@citrix.com" , Jan Beulich , Andrew Cooper , Ian Jackson , Xen-devel , Jun Nakajima , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============7421626838687426808== Content-Type: multipart/alternative; boundary=047d7b86c8c07d315c05305eee4c --047d7b86c8c07d315c05305eee4c Content-Type: text/plain; charset=UTF-8 > >> 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. Tamas --047d7b86c8c07d315c05305eee4c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

=
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domc= tl.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 {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } mov_to_cr;
>>
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Enable the capture o= f an extended set of MSRs */
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 uint8_t extended_captur= e;
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 uint32_t msr;
>
> Whoa there. Isn't it expanding the structure? Will this be backwar= ds
> 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=20 now is 24-bits so if a uint16_t is enough for the bitmask that should be=20 used instead. That way we don't end up growing the struct size.

=
Tamas

--047d7b86c8c07d315c05305eee4c-- --===============7421626838687426808== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============7421626838687426808==--