From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Lengyel, Tamas" Subject: Re: [PATCH v2 1/2] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access Date: Mon, 1 Feb 2016 09:36:57 -0700 Message-ID: References: <1454014688-25060-1-git-send-email-tlengyel@novetta.com> <56AB552202000078000CC5E5@prv-mh.provo.novell.com> <56AB9F3102000078000CC78F@prv-mh.provo.novell.com> <56ABA5CA02000078000CC820@prv-mh.provo.novell.com> <1454337922.28781.102.camel@citrix.com> <56AF93FD02000078000CD176@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7157910524587271173==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aQHTb-0001CX-Nu for xen-devel@lists.xenproject.org; Mon, 01 Feb 2016 16:37:19 +0000 Received: by mail-vk0-f43.google.com with SMTP id n1so80205373vkb.3 for ; Mon, 01 Feb 2016 08:37:17 -0800 (PST) In-Reply-To: <56AF93FD02000078000CD176@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Wei Liu , Ian Campbell , Razvan Cojocaru , Stefano Stabellini , GeorgeDunlap , Andrew Cooper , Ian Jackson , Stefano Stabellini , xen-devel@lists.xenproject.org, KeirFraser List-Id: xen-devel@lists.xenproject.org --===============7157910524587271173== Content-Type: multipart/alternative; boundary=001a114406ea7642f9052ab7ff82 --001a114406ea7642f9052ab7ff82 Content-Type: text/plain; charset=UTF-8 On Mon, Feb 1, 2016 at 9:21 AM, Jan Beulich wrote: > >>> On 01.02.16 at 15:45, wrote: > > On Fri, 2016-01-29 at 09:47 -0700, Jan Beulich wrote: > >> > > > On 29.01.16 at 17:32, wrote: > >> > On Fri, Jan 29, 2016 at 9:19 AM, Jan Beulich > wrote: > >> > > > > > On 29.01.16 at 17:12, wrote: > >> > > > On Fri, Jan 29, 2016 at 4:03 AM, Jan Beulich > >> > > > wrote: > >> > > > > > > > On 28.01.16 at 21:58, wrote: > >> > > > > > --- a/xen/include/public/memory.h > >> > > > > > +++ b/xen/include/public/memory.h > >> > > > > > @@ -423,11 +423,14 @@ struct xen_mem_access_op { > >> > > > > > /* xenmem_access_t */ > >> > > > > > uint8_t access; > >> > > > > > domid_t domid; > >> > > > > > + uint16_t altp2m_idx; > >> > > > > > + uint16_t _pad; > >> > > > > > /* > >> > > > > > * Number of pages for set op > >> > > > > > * Ignored on setting default access and other ops > >> > > > > > */ > >> > > > > > uint32_t nr; > >> > > > > > + uint32_t _pad2; > >> > > > > > >> > > > > Repeating what I had said on v1: So this is a tools only > >> > > > > interface, > >> > > > > yes. But it's not versioned (other than e.g. domctl and sysctl), > >> > > > > so > >> > > > > altering the interface structure is at least fragile. > >> > > > > >> > > > Not sure what I can do to address this. > >> > > > >> > > Deprecate the old interface and introduce a new one. But other > >> > > maintainers' opinions would be welcome. > >> > > >> > That seems like a very heavy handed solution to me. > >> > >> I understand that - hence the request for others' opinions. > > > > It's unfortunate that we've found ourselves here, but I think rather than > > deprecating the current and adding a new op alongside we should just > accept > > the one-time fragility this time around, add the version field as part of > > this set of changes and try and remember to include a version number for > > next time we add a tools only interface. I don't think xenaccess is yet > > widely used outside of Tamas and the Bitdfender folks, who I would assume > > can cope with such a change. > > > > I could accept changing the op number would make sense, but I don't think > > we should deprecate the old one (which implies continuing to support it > in > > parallel) > Supporting the two versions in parallel is not much effort, we would just have to add an extra check to the current version to fail if altp2m is enabled. Tamas --001a114406ea7642f9052ab7ff82 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Mon, Feb 1, 2016 at 9:21 AM, Jan Beulich <JBeulich@suse.com>= wrote:
>>> On 01.02.16 at 15:45, <ian.campbell@citrix.com> wrote:
> On Fri, 2016-01-29 at 09:47 -0700, Jan Beulich wrote:
>> > > > On 29.01.16 at 17:32, <tlengyel@novetta.com> wrote:
>> > On Fri, Jan 29, 2016 at 9:19 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> > > > > > On 29.01.16 at 17:12, <tlengyel@novetta.com> wrote:
>> > > > On Fri, Jan 29, 2016 at 4:03 AM, Jan Beulich <JBeulich@suse.com>
>> > > > wrote:
>> > > > > > > > On 28.01.16 at 21:58, <tlengyel@novetta.com> wrote:
>> > > > > > --- a/xen/include/public/memory.h
>> > > > > > +++ b/xen/include/public/memory.h
>> > > > > > @@ -423,11 +423,14 @@ struct xen_mem_acce= ss_op {
>> > > > > >=C2=A0 =C2=A0 =C2=A0 /* xenmem_access_t */=
>> > > > > >=C2=A0 =C2=A0 =C2=A0 uint8_t access;
>> > > > > >=C2=A0 =C2=A0 =C2=A0 domid_t domid;
>> > > > > > +=C2=A0 =C2=A0 uint16_t altp2m_idx;
>> > > > > > +=C2=A0 =C2=A0 uint16_t _pad;
>> > > > > >=C2=A0 =C2=A0 =C2=A0 /*
>> > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0* Number of pag= es for set op
>> > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0* Ignored on se= tting default access and other ops
>> > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>> > > > > >=C2=A0 =C2=A0 =C2=A0 uint32_t nr;
>> > > > > > +=C2=A0 =C2=A0 uint32_t _pad2;
>> > > > >
>> > > > > Repeating what I had said on v1: So this is a = tools only
>> > > > > interface,
>> > > > > yes. But it's not versioned (other than e.= g. domctl and sysctl),
>> > > > > so
>> > > > > altering the interface structure is at least f= ragile.
>> > > >
>> > > > Not sure what I can do to address this.
>> > >
>> > > Deprecate the old interface and introduce a new one. But= other
>> > > maintainers' opinions would be welcome.
>> >
>> > That seems like a very heavy handed solution to me.
>>
>> I understand that - hence the request for others' opinions. >
> It's unfortunate that we've found ourselves here, but I think = rather than
> deprecating the current and adding a new op alongside we should just a= ccept
> the one-time fragility this time around, add the version field as part= of
> this set of changes and try and remember to include a version number f= or
> next time we add a tools only interface. I don't think xenaccess i= s yet
> widely used outside of Tamas and the Bitdfender folks, who I would ass= ume
> can cope with such a change.
>
> I could accept changing the op number would make sense, but I don'= t think
> we should deprecate the old one (which implies continuing to support i= t in
> parallel)

Supporting t= he two versions in parallel is not much effort, we would just have to add a= n extra check to the current version to fail if altp2m is enabled.

<= /div>
Tamas

--001a114406ea7642f9052ab7ff82-- --===============7157910524587271173== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7157910524587271173==--