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 12:38:07 +0300	[thread overview]
Message-ID: <20180827093807.GA1307@hel> (raw)
In-Reply-To: <5B6049FA02000078001D967A@prv1-mh.provo.novell.com>

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.

> Also, to you, George, and whoever else advocates for this, another
> remark regarding the guest accessibility here (at the risk of getting
> flamed once again): The less capable (afaict)
> XENMEM_access_op_{g,s}et_access (previously
> HVMOP_{g,s}et_mem_access) are tools accessible only. I find such
> an inconsistency rather odd.

I don't really know what to reply.  While there probably isn't, as far
as I'm aware, any use-case for the non-altp2m OPs to be accessed from
the guest via HVMOPs directly, if my understanding is correct, the
scenario involving #VE and an in-guest agent would require this
functionality.

Thanks.

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