Re: [Xen-devel] [PATCH] common/mem_access: merged mem_access setting interfaces

[Razvan Cojocaru]

On 03/16/2017 08:10 PM, Tamas K Lengyel wrote:
> 
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 6eee0c8..ca53a0c 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -444,6 +444,8 @@ struct xen_mem_access_op {
>  /* xenmem_access_t */
>  uint8_t access;
>  domid_t domid;
> +uint16_t view_id;
> +uint16_t pad;
> 
> 
> I think this will mess with the next uint32_t struct member, so either
> this pad should be uint16_t pad[3], or another pad needs to be added
> after the following uint32_t nr.

Fair enough, I'll need to send V2 anyway as Roger's "x86: remove PVHv1
code" has also bumped XEN_DOMCTL_INTERFACE_VERSION and I now need to
take it out.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/mem_access: merged mem_access setting interfaces

[Tamas K Lengyel]

On Thu, Mar 16, 2017 at 12:33 PM, Razvan Cojocaru  wrote:

> On 03/16/2017 08:10 PM, Tamas K Lengyel wrote:
> >
> >
> > diff --git a/xen/include/public/memory.h
> b/xen/include/public/memory.h
> > index 6eee0c8..ca53a0c 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -444,6 +444,8 @@ struct xen_mem_access_op {
> >  /* xenmem_access_t */
> >  uint8_t access;
> >  domid_t domid;
> > +uint16_t view_id;
> > +uint16_t pad;
> >
> >
> > I think this will mess with the next uint32_t struct member, so either
> > this pad should be uint16_t pad[3], or another pad needs to be added
> > after the following uint32_t nr.
>
> Fair enough, I'll need to send V2 anyway as Roger's "x86: remove PVHv1
> code" has also bumped XEN_DOMCTL_INTERFACE_VERSION and I now need to
> take it out.
>

Sounds good. The patch looks good to me otherwise.

Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/mem_access: merged mem_access setting interfaces

[Tamas K Lengyel]

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 6eee0c8..ca53a0c 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -444,6 +444,8 @@ struct xen_mem_access_op {
>  /* xenmem_access_t */
>  uint8_t access;
>  domid_t domid;
> +uint16_t view_id;
> +uint16_t pad;
>

I think this will mess with the next uint32_t struct member, so either this
pad should be uint16_t pad[3], or another pad needs to be added after the
following uint32_t nr.


>  /*
>   * Number of pages for set op (or size of pfn_list for
>   * XENMEM_access_op_set_access_multi)
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/mem_access: merged mem_access setting interfaces

[Wei Liu]

On Thu, Mar 16, 2017 at 02:32:07PM +0200, Razvan Cojocaru wrote:
> xc_altp2m_set_mem_access() and xc_set_mem_access() end up doing the same thing
> in the hypervisor, but the former is a HVMOP and the latter a DOMCTL. Since
> nobody is currently using, or has stated intent to use, this functionality
> specifically as an HVMOP, this patch removes the HVMOP while adding an extra
> parameter to the more flexible DOMCTL variant, in which the altp2m view can be
> transmitted (0 for the default view, or when altp2m is disabled).
> The xen-access test has been updated in the process.
> 
> Signed-off-by: Razvan Cojocaru 
> ---
>  tools/libxc/include/xenctrl.h   | 10 +++---
>  tools/libxc/xc_altp2m.c | 25 -
>  tools/libxc/xc_mem_access.c | 14 +-
>  tools/tests/xen-access/xen-access.c | 25 -

Subject to an ack on the HV changes:

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] common/mem_access: merged mem_access setting interfaces

[Razvan Cojocaru]

xc_altp2m_set_mem_access() and xc_set_mem_access() end up doing the same thing
in the hypervisor, but the former is a HVMOP and the latter a DOMCTL. Since
nobody is currently using, or has stated intent to use, this functionality
specifically as an HVMOP, this patch removes the HVMOP while adding an extra
parameter to the more flexible DOMCTL variant, in which the altp2m view can be
transmitted (0 for the default view, or when altp2m is disabled).
The xen-access test has been updated in the process.

Signed-off-by: Razvan Cojocaru 
---
 tools/libxc/include/xenctrl.h   | 10 +++---
 tools/libxc/xc_altp2m.c | 25 -
 tools/libxc/xc_mem_access.c | 14 +-
 tools/tests/xen-access/xen-access.c | 25 -
 xen/arch/x86/hvm/hvm.c  | 10 --
 xen/common/mem_access.c |  4 ++--
 xen/include/public/domctl.h |  2 +-
 xen/include/public/hvm/hvm_op.h | 15 +--
 xen/include/public/memory.h |  2 ++
 9 files changed, 26 insertions(+), 81 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index b80d150..8a00fba 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1923,9 +1923,6 @@ int xc_altp2m_destroy_view(xc_interface *handle, domid_t 
domid,
 /* Switch all vCPUs of the domain to the specified altp2m view */
 int xc_altp2m_switch_to_view(xc_interface *handle, domid_t domid,
  uint16_t view_id);
-int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
- uint16_t view_id, xen_pfn_t gfn,
- xenmem_access_t access);
 int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid,
  uint16_t view_id, xen_pfn_t old_gfn,
  xen_pfn_t new_gfn);
@@ -1956,9 +1953,8 @@ int xc_mem_paging_load(xc_interface *xch, domid_t 
domain_id,
  * Allowed types are XENMEM_access_default, XENMEM_access_n, any combination of
  * XENMEM_access_ + (rwx), and XENMEM_access_rx2rw
  */
-int xc_set_mem_access(xc_interface *xch, domid_t domain_id,
-  xenmem_access_t access, uint64_t first_pfn,
-  uint32_t nr);
+int xc_set_mem_access(xc_interface *xch, domid_t domain_id, uint16_t view_id,
+  xenmem_access_t access, uint64_t first_pfn, uint32_t nr);
 
 /*
  * Set an array of pages to their respective access in the access array.
@@ -1966,7 +1962,7 @@ int xc_set_mem_access(xc_interface *xch, domid_t 
domain_id,
  * The same allowed access types as for xc_set_mem_access() apply.
  */
 int xc_set_mem_access_multi(xc_interface *xch, domid_t domain_id,
-uint8_t *access, uint64_t *pages,
+uint16_t view_id, uint8_t *access, uint64_t *pages,
 uint32_t nr);
 
 /*
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index 0639632..4f44a7b 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -163,31 +163,6 @@ int xc_altp2m_switch_to_view(xc_interface *handle, domid_t 
domid,
 return rc;
 }
 
-int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
- uint16_t view_id, xen_pfn_t gfn,
- xenmem_access_t access)
-{
-int rc;
-DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
-
-arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
-if ( arg == NULL )
-return -1;
-
-arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
-arg->cmd = HVMOP_altp2m_set_mem_access;
-arg->domain = domid;
-arg->u.set_mem_access.view = view_id;
-arg->u.set_mem_access.hvmmem_access = access;
-arg->u.set_mem_access.gfn = gfn;
-
-rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
- HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(handle, arg);
-return rc;
-}
-
 int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid,
  uint16_t view_id, xen_pfn_t old_gfn,
  xen_pfn_t new_gfn)
diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index 9536635..4242527 100644
--- a/tools/libxc/xc_mem_access.c
+++ b/tools/libxc/xc_mem_access.c
@@ -25,17 +25,19 @@
 
 int xc_set_mem_access(xc_interface *xch,
   domid_t domain_id,
+  uint16_t view_id,
   xenmem_access_t access,
   uint64_t first_pfn,
   uint32_t nr)
 {
 xen_mem_access_op_t mao =
 {
-.op = XENMEM_access_op_set_access,
-.domid  = domain_id,
-.access = access,
-.pfn= first_pfn,
-.nr = nr
+.op  = XENMEM_access_op_set_access,
+.domid   = domain_id,
+.access  = access,
+.pfn = first_pfn,
+