Re: [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page

From: Adrian Pop <apop@bitdefender.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Tamas K Lengyel <tamas@tklengyel.com>,
	Sergej Proskurin <proskurin@sec.in.tum.de>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3] x86/altp2m: Add a subop for obtaining the mem access of a page
Date: Mon, 27 Aug 2018 13:47:47 +0300	[thread overview]
Message-ID: <20180827104747.GA7088@hel> (raw)
In-Reply-To: <5B83CB7402000078001E2169@prv1-mh.provo.novell.com>

On Mon, Aug 27, 2018 at 03:59:16AM -0600, Jan Beulich wrote:
> >>> On 27.08.18 at 11:38, <apop@bitdefender.com> wrote:
> > On Tue, Jul 31, 2018 at 05:37:30AM -0600, Jan Beulich wrote:
> >> >>> On 25.07.18 at 13:16, <apop@bitdefender.com> wrote:
> >> > --- a/xen/include/public/hvm/hvm_op.h
> >> > +++ b/xen/include/public/hvm/hvm_op.h
> >> > @@ -234,7 +234,7 @@ struct xen_hvm_altp2m_view {
> >> >  typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> >> >  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >> >  
> >> > -struct xen_hvm_altp2m_set_mem_access {
> >> > +struct xen_hvm_altp2m_mem_access {
> >> >      /* view */
> >> >      uint16_t view;
> >> >      /* Memory type */
> >> > @@ -243,8 +243,8 @@ struct xen_hvm_altp2m_set_mem_access {
> >> >      /* gfn */
> >> >      uint64_t gfn;
> >> >  };
> >> > -typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t;
> >> > -DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> >> > +typedef struct xen_hvm_altp2m_mem_access xen_hvm_altp2m_mem_access_t;
> >> > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_mem_access_t);
> >> >  
> >> >  struct xen_hvm_altp2m_set_mem_access_multi {
> >> >      /* view */
> >> > @@ -296,6 +296,8 @@ struct xen_hvm_altp2m_op {
> >> >  #define HVMOP_altp2m_change_gfn           8
> >> >  /* Set access for an array of pages */
> >> >  #define HVMOP_altp2m_set_mem_access_multi 9
> >> > +/* Get the access of a page of memory from a certain view */
> >> > +#define HVMOP_altp2m_get_mem_access       10
> >> >      domid_t domain;
> >> >      uint16_t pad1;
> >> >      uint32_t pad2;
> >> > @@ -303,7 +305,7 @@ struct xen_hvm_altp2m_op {
> >> >          struct xen_hvm_altp2m_domain_state         domain_state;
> >> >          struct xen_hvm_altp2m_vcpu_enable_notify   enable_notify;
> >> >          struct xen_hvm_altp2m_view                 view;
> >> > -        struct xen_hvm_altp2m_set_mem_access       set_mem_access;
> >> > +        struct xen_hvm_altp2m_mem_access           mem_access;
> >> >          struct xen_hvm_altp2m_change_gfn           change_gfn;
> >> >          struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
> >> >          uint8_t pad[64];
> >> 
> >> This being exposed to guests, the interface has to be considered
> >> stable imo, in which case you can't rename things like this. You'd
> >> need __XEN_INTERFACE_VERSION__ dependent logic (just like is the
> >> case further up in the file).
> > 
> > Right.  Sorry about that.  Maybe just having separate structs for
> > get/set would be cleaner in this case, even though they would be
> > similar.
> 
> Personally I'd prefer to avoid having two structures with identical
> layout but different tags. But if others think differently, I'm not
> meaning to stand in the way.

Ok then.  I have no strong preference either way.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel