From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 1/2] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access Date: Mon, 01 Feb 2016 09:36:52 -0700 Message-ID: <56AF97B402000078000CD1C1@prv-mh.provo.novell.com> 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> <1454344256.28781.113.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aQHTI-00019O-57 for xen-devel@lists.xenproject.org; Mon, 01 Feb 2016 16:37:00 +0000 In-Reply-To: <1454344256.28781.113.camel@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Tamas Lengyel Cc: Wei Liu , Razvan Cojocaru , Stefano Stabellini , GeorgeDunlap , Andrew Cooper , Ian Jackson , Stefano Stabellini , xen-devel@lists.xenproject.org, KeirFraser List-Id: xen-devel@lists.xenproject.org >>> On 01.02.16 at 17:30, wrote: > On Mon, 2016-02-01 at 09:21 -0700, 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), if we go this route we should just retire the old number to >> > straight away to return -ENOSYS (or maybe -EACCESS, which is what a >> > version >> > mismatch would have resulted in). >> >> That actually looks like a reasonable compromise, until we finally >> manage to get around to morph the tools-only HVM-ops into a >> new hvmctl hypercall (leaving only guest accessible ones in the >> current interface). > > Aren't the ones being discussed here xenmem subops rather than hvmops? Oh, yes, right. I'm sorry for confusing things. Jan