Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-21 Thread Razvan Cojocaru 
On 09/21/2016 03:03 PM, Jan Beulich wrote:
 On 07.09.16 at 11:12,  wrote:
>> +long p2m_set_mem_access_multi(struct domain *d,
>> +  const XEN_GUEST_HANDLE(const_uint64) pfn_list,
>> +  const XEN_GUEST_HANDLE(const_uint8) 
>> access_list,
>> +  uint32_t nr, uint32_t start, uint32_t mask,
>> +  unsigned int altp2m_idx)
>> +{
>> +struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
>> +long rc = 0;
>> +
>> +/* altp2m view 0 is treated as the hostp2m */
>> +if ( altp2m_idx )
>> +{
>> +if ( altp2m_idx >= MAX_ALTP2M ||
>> + d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
>> +return -EINVAL;
>> +
>> +ap2m = d->arch.altp2m_p2m[altp2m_idx];
>> +}
>> +
>> +p2m_lock(p2m);
>> +if ( ap2m )
>> +p2m_lock(ap2m);
>> +
>> +while ( start < nr )
>> +{
>> +p2m_access_t a;
>> +uint8_t access;
>> +uint64_t gfn_l;
>> +
>> +copy_from_guest_offset(_l, pfn_list, start, 1);
>> +copy_from_guest_offset(, access_list, start, 1);
> 
> Coverity validly complains about the missing error checks here
> (IDs 1373105 and 1373106). I have no idea how none of us who
> have looked at the patch noticed this before it went in, but please
> submit a fix (mentioning the two IDs).

Sent. Sorry for the omission.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-21 Thread Jan Beulich 
>>> On 07.09.16 at 11:12,  wrote:
> +long p2m_set_mem_access_multi(struct domain *d,
> +  const XEN_GUEST_HANDLE(const_uint64) pfn_list,
> +  const XEN_GUEST_HANDLE(const_uint8) 
> access_list,
> +  uint32_t nr, uint32_t start, uint32_t mask,
> +  unsigned int altp2m_idx)
> +{
> +struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
> +long rc = 0;
> +
> +/* altp2m view 0 is treated as the hostp2m */
> +if ( altp2m_idx )
> +{
> +if ( altp2m_idx >= MAX_ALTP2M ||
> + d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> +return -EINVAL;
> +
> +ap2m = d->arch.altp2m_p2m[altp2m_idx];
> +}
> +
> +p2m_lock(p2m);
> +if ( ap2m )
> +p2m_lock(ap2m);
> +
> +while ( start < nr )
> +{
> +p2m_access_t a;
> +uint8_t access;
> +uint64_t gfn_l;
> +
> +copy_from_guest_offset(_l, pfn_list, start, 1);
> +copy_from_guest_offset(, access_list, start, 1);

Coverity validly complains about the missing error checks here
(IDs 1373105 and 1373106). I have no idea how none of us who
have looked at the patch noticed this before it went in, but please
submit a fix (mentioning the two IDs).

Jan


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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-19 Thread George Dunlap 
On 07/09/16 10:12, Razvan Cojocaru wrote:
> Currently it is only possible to set mem_access restrictions only for
> a contiguous range of GFNs (or, as a particular case, for a single GFN).
> This patch introduces a new libxc function taking an array of GFNs.
> The alternative would be to set each page in turn, using a userspace-HV
> roundtrip for each call, and triggering a TLB flush per page set.
> 
> Signed-off-by: Razvan Cojocaru 
> Acked-by: Wei Liu 

Looks good:

Acked-by: George Dunlap 

> 
> ---
> Changes since V3:
>  - Fixed ARM compilation (replaced ENOTSUP with EOPNOTSUPP, which is
>#defined in the in-tree errno.h). The multi code remains
>unimplemented for ARM (it depends on " [RFC 21/22] xen/arm: p2m:
>Re-implement p2m_set_mem_access using p2m_{set, get}_entry", and
>Julien Grall has gracefully accepted to defer implementation
>until after both patches go in).
>  - Reordered the xen/guest_access.h #include in p2m.c.
>  - Now passing a gfn_t to set_mem_access() instead of unsigned long.
>  - Removed the p2m prefix from p2m_xenmem_access_to_p2m_access().
>  - Switched from bool_t to bool.
>  - Moved the XENMEM_access_op case up with the other do-nothing
>XENMEM_* cases.
> ---
>  tools/libxc/include/xenctrl.h |   9 +++
>  tools/libxc/xc_mem_access.c   |  38 +++
>  xen/arch/arm/p2m.c|  10 +++
>  xen/arch/x86/mm/p2m.c | 150 
> --
>  xen/common/compat/memory.c|  23 +--
>  xen/common/mem_access.c   |  11 
>  xen/include/public/memory.h   |  14 +++-
>  xen/include/xen/p2m-common.h  |   6 ++
>  xen/include/xlat.lst  |   2 +-
>  9 files changed, 224 insertions(+), 39 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 560ce7b..5e685a6 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2126,6 +2126,15 @@ int xc_set_mem_access(xc_interface *xch, domid_t 
> domain_id,
>uint32_t nr);
>  
>  /*
> + * Set an array of pages to their respective access in the access array.
> + * The nr parameter specifies the size of the pages and access arrays.
> + * 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,
> +uint32_t nr);
> +
> +/*
>   * Gets the mem access for the given page (returned in access on success)
>   */
>  int xc_get_mem_access(xc_interface *xch, domid_t domain_id,
> diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
> index eee088c..9536635 100644
> --- a/tools/libxc/xc_mem_access.c
> +++ b/tools/libxc/xc_mem_access.c
> @@ -41,6 +41,44 @@ int xc_set_mem_access(xc_interface *xch,
>  return do_memory_op(xch, XENMEM_access_op, , sizeof(mao));
>  }
>  
> +int xc_set_mem_access_multi(xc_interface *xch,
> +domid_t domain_id,
> +uint8_t *access,
> +uint64_t *pages,
> +uint32_t nr)
> +{
> +DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t),
> + XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +int rc;
> +
> +xen_mem_access_op_t mao =
> +{
> +.op   = XENMEM_access_op_set_access_multi,
> +.domid= domain_id,
> +.access   = XENMEM_access_default + 1, /* Invalid value */
> +.pfn  = ~0UL, /* Invalid GFN */
> +.nr   = nr,
> +};
> +
> +if ( xc_hypercall_bounce_pre(xch, pages) ||
> + xc_hypercall_bounce_pre(xch, access) )
> +{
> +PERROR("Could not bounce memory for 
> XENMEM_access_op_set_access_multi");
> +return -1;
> +}
> +
> +set_xen_guest_handle(mao.pfn_list, pages);
> +set_xen_guest_handle(mao.access_list, access);
> +
> +rc = do_memory_op(xch, XENMEM_access_op, , sizeof(mao));
> +
> +xc_hypercall_bounce_post(xch, access);
> +xc_hypercall_bounce_post(xch, pages);
> +
> +return rc;
> +}
> +
>  int xc_get_mem_access(xc_interface *xch,
>domid_t domain_id,
>uint64_t pfn,
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index b648a9d..5c5049f 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1836,6 +1836,16 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
> uint32_t nr,
>  return 0;
>  }
>  
> +long p2m_set_mem_access_multi(struct domain *d,
> +  const XEN_GUEST_HANDLE(const_uint64) pfn_list,
> +  const XEN_GUEST_HANDLE(const_uint8) 
> access_list,
> +  uint32_t nr, uint32_t start, uint32_t mask,
> +

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-15 Thread Razvan Cojocaru 
On 09/15/2016 04:55 PM, Wei Liu wrote:
> On Thu, Sep 15, 2016 at 04:52:32PM +0300, Razvan Cojocaru wrote:
>> On 09/15/2016 04:49 PM, Wei Liu wrote:
>>> On Thu, Sep 15, 2016 at 04:39:47PM +0300, Razvan Cojocaru wrote:
 On 09/07/2016 07:01 PM, Jan Beulich wrote:
 On 07.09.16 at 11:12,  wrote:
>> Currently it is only possible to set mem_access restrictions only for
>> a contiguous range of GFNs (or, as a particular case, for a single GFN).
>> This patch introduces a new libxc function taking an array of GFNs.
>> The alternative would be to set each page in turn, using a userspace-HV
>> roundtrip for each call, and triggering a TLB flush per page set.
>>
>> Signed-off-by: Razvan Cojocaru 
>> Acked-by: Wei Liu 
>
> Hypervisor parts (without ARM and x86/mm)
> Acked-by: Jan Beulich 
>
> albeit I spotted one more cosmetic issue (which I guess could be
> fixed up during commit, if no other reason for a v5 arises):
>
>> @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) compat)
>>  }
>>  
>>  case XENMEM_access_op:
>> -return mem_access_memop(cmd,
>> -guest_handle_cast(compat,
>> -  
>> xen_mem_access_op_t));
>> +{
>> +if ( copy_from_guest(, compat, 1) )
>> +return -EFAULT;
>> +
>> +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
>> +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
>> +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
>> +guest_from_compat_handle((_d_)->access_list, 
>> (_s_)->access_list)
>> +
>> +XLAT_mem_access_op(nat.mao, );
>> +
>> +#undef XLAT_mem_access_op_HNDL_pfn_list
>> +#undef XLAT_mem_access_op_HNDL_access_list
>> +
>> +break;
>> +}
>
> There are no local variables declared here, so I don't see the need
> for the braces.

 There have only been Acked-by replies so far, but if you'd prefer I have
 no problem sending a V5 removing the braces.

>>>
>>> Does this patch have all the necessary acks? If so I don't mind fixing
>>> it up and committing it.
>>
>> In addition to your ack, it's:
>>
>> Acked-by: Jan Beulich 
>> Acked-by: Tamas K Lengyel 
>> Acked-by: Julien Grall 
>>
>> for the hypervisor parts (without ARM and x86/mm), vm_event and ARM
>> respectively.
>>
> 
> I think it still needs an ack from George for x86/mm changes.

Fair enough.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-15 Thread Wei Liu 
On Thu, Sep 15, 2016 at 04:52:32PM +0300, Razvan Cojocaru wrote:
> On 09/15/2016 04:49 PM, Wei Liu wrote:
> > On Thu, Sep 15, 2016 at 04:39:47PM +0300, Razvan Cojocaru wrote:
> >> On 09/07/2016 07:01 PM, Jan Beulich wrote:
> >> On 07.09.16 at 11:12,  wrote:
>  Currently it is only possible to set mem_access restrictions only for
>  a contiguous range of GFNs (or, as a particular case, for a single GFN).
>  This patch introduces a new libxc function taking an array of GFNs.
>  The alternative would be to set each page in turn, using a userspace-HV
>  roundtrip for each call, and triggering a TLB flush per page set.
> 
>  Signed-off-by: Razvan Cojocaru 
>  Acked-by: Wei Liu 
> >>>
> >>> Hypervisor parts (without ARM and x86/mm)
> >>> Acked-by: Jan Beulich 
> >>>
> >>> albeit I spotted one more cosmetic issue (which I guess could be
> >>> fixed up during commit, if no other reason for a v5 arises):
> >>>
>  @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
>  XEN_GUEST_HANDLE_PARAM(void) compat)
>   }
>   
>   case XENMEM_access_op:
>  -return mem_access_memop(cmd,
>  -guest_handle_cast(compat,
>  -  
>  xen_mem_access_op_t));
>  +{
>  +if ( copy_from_guest(, compat, 1) )
>  +return -EFAULT;
>  +
>  +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
>  +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
>  +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
>  +guest_from_compat_handle((_d_)->access_list, 
>  (_s_)->access_list)
>  +
>  +XLAT_mem_access_op(nat.mao, );
>  +
>  +#undef XLAT_mem_access_op_HNDL_pfn_list
>  +#undef XLAT_mem_access_op_HNDL_access_list
>  +
>  +break;
>  +}
> >>>
> >>> There are no local variables declared here, so I don't see the need
> >>> for the braces.
> >>
> >> There have only been Acked-by replies so far, but if you'd prefer I have
> >> no problem sending a V5 removing the braces.
> >>
> > 
> > Does this patch have all the necessary acks? If so I don't mind fixing
> > it up and committing it.
> 
> In addition to your ack, it's:
> 
> Acked-by: Jan Beulich 
> Acked-by: Tamas K Lengyel 
> Acked-by: Julien Grall 
> 
> for the hypervisor parts (without ARM and x86/mm), vm_event and ARM
> respectively.
> 

I think it still needs an ack from George for x86/mm changes.

Wei.

> 
> Thanks,
> Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-15 Thread Razvan Cojocaru 
On 09/15/2016 04:49 PM, Wei Liu wrote:
> On Thu, Sep 15, 2016 at 04:39:47PM +0300, Razvan Cojocaru wrote:
>> On 09/07/2016 07:01 PM, Jan Beulich wrote:
>> On 07.09.16 at 11:12,  wrote:
 Currently it is only possible to set mem_access restrictions only for
 a contiguous range of GFNs (or, as a particular case, for a single GFN).
 This patch introduces a new libxc function taking an array of GFNs.
 The alternative would be to set each page in turn, using a userspace-HV
 roundtrip for each call, and triggering a TLB flush per page set.

 Signed-off-by: Razvan Cojocaru 
 Acked-by: Wei Liu 
>>>
>>> Hypervisor parts (without ARM and x86/mm)
>>> Acked-by: Jan Beulich 
>>>
>>> albeit I spotted one more cosmetic issue (which I guess could be
>>> fixed up during commit, if no other reason for a v5 arises):
>>>
 @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
 XEN_GUEST_HANDLE_PARAM(void) compat)
  }
  
  case XENMEM_access_op:
 -return mem_access_memop(cmd,
 -guest_handle_cast(compat,
 -  
 xen_mem_access_op_t));
 +{
 +if ( copy_from_guest(, compat, 1) )
 +return -EFAULT;
 +
 +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
 +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
 +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
 +guest_from_compat_handle((_d_)->access_list, 
 (_s_)->access_list)
 +
 +XLAT_mem_access_op(nat.mao, );
 +
 +#undef XLAT_mem_access_op_HNDL_pfn_list
 +#undef XLAT_mem_access_op_HNDL_access_list
 +
 +break;
 +}
>>>
>>> There are no local variables declared here, so I don't see the need
>>> for the braces.
>>
>> There have only been Acked-by replies so far, but if you'd prefer I have
>> no problem sending a V5 removing the braces.
>>
> 
> Does this patch have all the necessary acks? If so I don't mind fixing
> it up and committing it.

In addition to your ack, it's:

Acked-by: Jan Beulich 
Acked-by: Tamas K Lengyel 
Acked-by: Julien Grall 

for the hypervisor parts (without ARM and x86/mm), vm_event and ARM
respectively.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-15 Thread Wei Liu 
On Thu, Sep 15, 2016 at 04:39:47PM +0300, Razvan Cojocaru wrote:
> On 09/07/2016 07:01 PM, Jan Beulich wrote:
>  On 07.09.16 at 11:12,  wrote:
> >> Currently it is only possible to set mem_access restrictions only for
> >> a contiguous range of GFNs (or, as a particular case, for a single GFN).
> >> This patch introduces a new libxc function taking an array of GFNs.
> >> The alternative would be to set each page in turn, using a userspace-HV
> >> roundtrip for each call, and triggering a TLB flush per page set.
> >>
> >> Signed-off-by: Razvan Cojocaru 
> >> Acked-by: Wei Liu 
> > 
> > Hypervisor parts (without ARM and x86/mm)
> > Acked-by: Jan Beulich 
> > 
> > albeit I spotted one more cosmetic issue (which I guess could be
> > fixed up during commit, if no other reason for a v5 arises):
> > 
> >> @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
> >> XEN_GUEST_HANDLE_PARAM(void) compat)
> >>  }
> >>  
> >>  case XENMEM_access_op:
> >> -return mem_access_memop(cmd,
> >> -guest_handle_cast(compat,
> >> -  
> >> xen_mem_access_op_t));
> >> +{
> >> +if ( copy_from_guest(, compat, 1) )
> >> +return -EFAULT;
> >> +
> >> +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
> >> +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
> >> +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
> >> +guest_from_compat_handle((_d_)->access_list, 
> >> (_s_)->access_list)
> >> +
> >> +XLAT_mem_access_op(nat.mao, );
> >> +
> >> +#undef XLAT_mem_access_op_HNDL_pfn_list
> >> +#undef XLAT_mem_access_op_HNDL_access_list
> >> +
> >> +break;
> >> +}
> > 
> > There are no local variables declared here, so I don't see the need
> > for the braces.
> 
> There have only been Acked-by replies so far, but if you'd prefer I have
> no problem sending a V5 removing the braces.
> 

Does this patch have all the necessary acks? If so I don't mind fixing
it up and committing it.

Wei.

> 
> Thanks,
> Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-15 Thread Razvan Cojocaru 
On 09/07/2016 07:01 PM, Jan Beulich wrote:
 On 07.09.16 at 11:12,  wrote:
>> Currently it is only possible to set mem_access restrictions only for
>> a contiguous range of GFNs (or, as a particular case, for a single GFN).
>> This patch introduces a new libxc function taking an array of GFNs.
>> The alternative would be to set each page in turn, using a userspace-HV
>> roundtrip for each call, and triggering a TLB flush per page set.
>>
>> Signed-off-by: Razvan Cojocaru 
>> Acked-by: Wei Liu 
> 
> Hypervisor parts (without ARM and x86/mm)
> Acked-by: Jan Beulich 
> 
> albeit I spotted one more cosmetic issue (which I guess could be
> fixed up during commit, if no other reason for a v5 arises):
> 
>> @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) compat)
>>  }
>>  
>>  case XENMEM_access_op:
>> -return mem_access_memop(cmd,
>> -guest_handle_cast(compat,
>> -  xen_mem_access_op_t));
>> +{
>> +if ( copy_from_guest(, compat, 1) )
>> +return -EFAULT;
>> +
>> +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
>> +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
>> +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
>> +guest_from_compat_handle((_d_)->access_list, (_s_)->access_list)
>> +
>> +XLAT_mem_access_op(nat.mao, );
>> +
>> +#undef XLAT_mem_access_op_HNDL_pfn_list
>> +#undef XLAT_mem_access_op_HNDL_access_list
>> +
>> +break;
>> +}
> 
> There are no local variables declared here, so I don't see the need
> for the braces.

There have only been Acked-by replies so far, but if you'd prefer I have
no problem sending a V5 removing the braces.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-09 Thread Julien Grall 

Hello Razvan,

On 07/09/16 10:12, Razvan Cojocaru wrote:

Currently it is only possible to set mem_access restrictions only for
a contiguous range of GFNs (or, as a particular case, for a single GFN).
This patch introduces a new libxc function taking an array of GFNs.
The alternative would be to set each page in turn, using a userspace-HV
roundtrip for each call, and triggering a TLB flush per page set.

Signed-off-by: Razvan Cojocaru 
Acked-by: Wei Liu 

---
Changes since V3:
 - Fixed ARM compilation (replaced ENOTSUP with EOPNOTSUPP, which is
   #defined in the in-tree errno.h). The multi code remains
   unimplemented for ARM (it depends on " [RFC 21/22] xen/arm: p2m:
   Re-implement p2m_set_mem_access using p2m_{set, get}_entry", and
   Julien Grall has gracefully accepted to defer implementation
   until after both patches go in).


For the ARM bits:

Acked-by: Julien Grall 

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-07 Thread Jan Beulich 
>>> On 07.09.16 at 11:12,  wrote:
> Currently it is only possible to set mem_access restrictions only for
> a contiguous range of GFNs (or, as a particular case, for a single GFN).
> This patch introduces a new libxc function taking an array of GFNs.
> The alternative would be to set each page in turn, using a userspace-HV
> roundtrip for each call, and triggering a TLB flush per page set.
> 
> Signed-off-by: Razvan Cojocaru 
> Acked-by: Wei Liu 

Hypervisor parts (without ARM and x86/mm)
Acked-by: Jan Beulich 

albeit I spotted one more cosmetic issue (which I guess could be
fixed up during commit, if no other reason for a v5 arises):

> @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) compat)
>  }
>  
>  case XENMEM_access_op:
> -return mem_access_memop(cmd,
> -guest_handle_cast(compat,
> -  xen_mem_access_op_t));
> +{
> +if ( copy_from_guest(, compat, 1) )
> +return -EFAULT;
> +
> +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
> +guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
> +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
> +guest_from_compat_handle((_d_)->access_list, (_s_)->access_list)
> +
> +XLAT_mem_access_op(nat.mao, );
> +
> +#undef XLAT_mem_access_op_HNDL_pfn_list
> +#undef XLAT_mem_access_op_HNDL_access_list
> +
> +break;
> +}

There are no local variables declared here, so I don't see the need
for the braces.

Jan


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

Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-07 Thread Tamas K Lengyel 
On Wed, Sep 7, 2016 at 3:12 AM, Razvan Cojocaru
 wrote:
> Currently it is only possible to set mem_access restrictions only for
> a contiguous range of GFNs (or, as a particular case, for a single GFN).
> This patch introduces a new libxc function taking an array of GFNs.
> The alternative would be to set each page in turn, using a userspace-HV
> roundtrip for each call, and triggering a TLB flush per page set.
>
> Signed-off-by: Razvan Cojocaru 
> Acked-by: Wei Liu 

Acked-by: Tamas K Lengyel 

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

[Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

2016-09-07 Thread Razvan Cojocaru 
Currently it is only possible to set mem_access restrictions only for
a contiguous range of GFNs (or, as a particular case, for a single GFN).
This patch introduces a new libxc function taking an array of GFNs.
The alternative would be to set each page in turn, using a userspace-HV
roundtrip for each call, and triggering a TLB flush per page set.

Signed-off-by: Razvan Cojocaru 
Acked-by: Wei Liu 

---
Changes since V3:
 - Fixed ARM compilation (replaced ENOTSUP with EOPNOTSUPP, which is
   #defined in the in-tree errno.h). The multi code remains
   unimplemented for ARM (it depends on " [RFC 21/22] xen/arm: p2m:
   Re-implement p2m_set_mem_access using p2m_{set, get}_entry", and
   Julien Grall has gracefully accepted to defer implementation
   until after both patches go in).
 - Reordered the xen/guest_access.h #include in p2m.c.
 - Now passing a gfn_t to set_mem_access() instead of unsigned long.
 - Removed the p2m prefix from p2m_xenmem_access_to_p2m_access().
 - Switched from bool_t to bool.
 - Moved the XENMEM_access_op case up with the other do-nothing
   XENMEM_* cases.
---
 tools/libxc/include/xenctrl.h |   9 +++
 tools/libxc/xc_mem_access.c   |  38 +++
 xen/arch/arm/p2m.c|  10 +++
 xen/arch/x86/mm/p2m.c | 150 --
 xen/common/compat/memory.c|  23 +--
 xen/common/mem_access.c   |  11 
 xen/include/public/memory.h   |  14 +++-
 xen/include/xen/p2m-common.h  |   6 ++
 xen/include/xlat.lst  |   2 +-
 9 files changed, 224 insertions(+), 39 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 560ce7b..5e685a6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2126,6 +2126,15 @@ int xc_set_mem_access(xc_interface *xch, domid_t 
domain_id,
   uint32_t nr);
 
 /*
+ * Set an array of pages to their respective access in the access array.
+ * The nr parameter specifies the size of the pages and access arrays.
+ * 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,
+uint32_t nr);
+
+/*
  * Gets the mem access for the given page (returned in access on success)
  */
 int xc_get_mem_access(xc_interface *xch, domid_t domain_id,
diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index eee088c..9536635 100644
--- a/tools/libxc/xc_mem_access.c
+++ b/tools/libxc/xc_mem_access.c
@@ -41,6 +41,44 @@ int xc_set_mem_access(xc_interface *xch,
 return do_memory_op(xch, XENMEM_access_op, , sizeof(mao));
 }
 
+int xc_set_mem_access_multi(xc_interface *xch,
+domid_t domain_id,
+uint8_t *access,
+uint64_t *pages,
+uint32_t nr)
+{
+DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t),
+ XC_HYPERCALL_BUFFER_BOUNCE_IN);
+int rc;
+
+xen_mem_access_op_t mao =
+{
+.op   = XENMEM_access_op_set_access_multi,
+.domid= domain_id,
+.access   = XENMEM_access_default + 1, /* Invalid value */
+.pfn  = ~0UL, /* Invalid GFN */
+.nr   = nr,
+};
+
+if ( xc_hypercall_bounce_pre(xch, pages) ||
+ xc_hypercall_bounce_pre(xch, access) )
+{
+PERROR("Could not bounce memory for 
XENMEM_access_op_set_access_multi");
+return -1;
+}
+
+set_xen_guest_handle(mao.pfn_list, pages);
+set_xen_guest_handle(mao.access_list, access);
+
+rc = do_memory_op(xch, XENMEM_access_op, , sizeof(mao));
+
+xc_hypercall_bounce_post(xch, access);
+xc_hypercall_bounce_post(xch, pages);
+
+return rc;
+}
+
 int xc_get_mem_access(xc_interface *xch,
   domid_t domain_id,
   uint64_t pfn,
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b648a9d..5c5049f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1836,6 +1836,16 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
uint32_t nr,
 return 0;
 }
 
+long p2m_set_mem_access_multi(struct domain *d,
+  const XEN_GUEST_HANDLE(const_uint64) pfn_list,
+  const XEN_GUEST_HANDLE(const_uint8) access_list,
+  uint32_t nr, uint32_t start, uint32_t mask,
+  unsigned int altp2m_idx)
+{
+/* Not yet implemented on ARM. */
+return -EOPNOTSUPP;
+}
+
 int p2m_get_mem_access(struct domain *d, gfn_t gfn,
xenmem_access_t *access)
 {
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 27f9d26..97c7cfd 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c