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

2016-09-02 Thread Tamas K Lengyel 
On Sep 2, 2016 09:17, "Razvan Cojocaru"  wrote:
>
> On 09/02/2016 06:13 PM, Tamas K Lengyel wrote:
> > On Sep 2, 2016 09:03, "Jan Beulich"  > > wrote:
> >>
> >> >>> On 02.09.16 at 16:50,  > > wrote:
> >> > On Sep 2, 2016 05:45, "Jan Beulich"  > > wrote:
> >> >>
> >> >> >>> On 02.09.16 at 13:18,  > > wrote:
> >> >> > On 09/02/2016 01:02 PM, Jan Beulich wrote:
> >> >> > On 02.09.16 at 10:51,  > > wrote:
> >> >> >>> Changes since V1 / RFC:
> >> >> >>>  - Renamed xc_set_mem_access_sparse() to
xc_set_mem_access_multi(),
> >> >> >>>and XENMEM_access_op_set_access_sparse to
> >> >> >>>XENMEM_access_op_set_access_multi.
> >> >> >>>  - Renamed the 'nr' parameter to 'size'.
> >> >> >>
> >> >> >> Why?
> >> >> >
> >> >> > Tamas suggested it, size sounded more intuitive to him. I have no
> >> >> > problem with either nr or size.
> >> >>
> >> >> Size to me means something in bytes, which clearly isn't the case
> >> >> here. There's not even support for other then 4k pages so far.
> >> >
> >> > Lets make it array_size then to clarify?
> >>
> >> What's wrong with "nr", matching the other (existing) function?
> >>
> >
> > IMHO it's too generic. So either a more descriptive name or a comment is
> > warranted to explain the inputs.
>
> If this satisfies everybody, I'll keep 'nr' and add a comment describing
> the function and parameters (which is a good thing anyway).
>
>

SGTM.

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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 06:13 PM, Tamas K Lengyel wrote:
> On Sep 2, 2016 09:03, "Jan Beulich"  > wrote:
>>
>> >>> On 02.09.16 at 16:50,  > wrote:
>> > On Sep 2, 2016 05:45, "Jan Beulich"  > wrote:
>> >>
>> >> >>> On 02.09.16 at 13:18,  > wrote:
>> >> > On 09/02/2016 01:02 PM, Jan Beulich wrote:
>> >> > On 02.09.16 at 10:51,  > wrote:
>> >> >>> Changes since V1 / RFC:
>> >> >>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>> >> >>>and XENMEM_access_op_set_access_sparse to
>> >> >>>XENMEM_access_op_set_access_multi.
>> >> >>>  - Renamed the 'nr' parameter to 'size'.
>> >> >>
>> >> >> Why?
>> >> >
>> >> > Tamas suggested it, size sounded more intuitive to him. I have no
>> >> > problem with either nr or size.
>> >>
>> >> Size to me means something in bytes, which clearly isn't the case
>> >> here. There's not even support for other then 4k pages so far.
>> >
>> > Lets make it array_size then to clarify?
>>
>> What's wrong with "nr", matching the other (existing) function?
>>
> 
> IMHO it's too generic. So either a more descriptive name or a comment is
> warranted to explain the inputs.

If this satisfies everybody, I'll keep 'nr' and add a comment describing
the function and parameters (which is a good thing anyway).


Thanks,
Razvan

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

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

2016-09-02 Thread Tamas K Lengyel 
On Sep 2, 2016 09:03, "Jan Beulich"  wrote:
>
> >>> On 02.09.16 at 16:50,  wrote:
> > On Sep 2, 2016 05:45, "Jan Beulich"  wrote:
> >>
> >> >>> On 02.09.16 at 13:18,  wrote:
> >> > On 09/02/2016 01:02 PM, Jan Beulich wrote:
> >> > On 02.09.16 at 10:51,  wrote:
> >> >>> Changes since V1 / RFC:
> >> >>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
> >> >>>and XENMEM_access_op_set_access_sparse to
> >> >>>XENMEM_access_op_set_access_multi.
> >> >>>  - Renamed the 'nr' parameter to 'size'.
> >> >>
> >> >> Why?
> >> >
> >> > Tamas suggested it, size sounded more intuitive to him. I have no
> >> > problem with either nr or size.
> >>
> >> Size to me means something in bytes, which clearly isn't the case
> >> here. There's not even support for other then 4k pages so far.
> >
> > Lets make it array_size then to clarify?
>
> What's wrong with "nr", matching the other (existing) function?
>

IMHO it's too generic. So either a more descriptive name or a comment is
warranted to explain the inputs.

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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 16:50,  wrote:
> On Sep 2, 2016 05:45, "Jan Beulich"  wrote:
>>
>> >>> On 02.09.16 at 13:18,  wrote:
>> > On 09/02/2016 01:02 PM, Jan Beulich wrote:
>> > On 02.09.16 at 10:51,  wrote:
>> >>> Changes since V1 / RFC:
>> >>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>> >>>and XENMEM_access_op_set_access_sparse to
>> >>>XENMEM_access_op_set_access_multi.
>> >>>  - Renamed the 'nr' parameter to 'size'.
>> >>
>> >> Why?
>> >
>> > Tamas suggested it, size sounded more intuitive to him. I have no
>> > problem with either nr or size.
>>
>> Size to me means something in bytes, which clearly isn't the case
>> here. There's not even support for other then 4k pages so far.
> 
> Lets make it array_size then to clarify?

What's wrong with "nr", matching the other (existing) function?

Jan


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

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

2016-09-02 Thread Tamas K Lengyel 
On Sep 2, 2016 05:45, "Jan Beulich"  wrote:
>
> >>> On 02.09.16 at 13:18,  wrote:
> > On 09/02/2016 01:02 PM, Jan Beulich wrote:
> > On 02.09.16 at 10:51,  wrote:
> >>> Changes since V1 / RFC:
> >>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
> >>>and XENMEM_access_op_set_access_sparse to
> >>>XENMEM_access_op_set_access_multi.
> >>>  - Renamed the 'nr' parameter to 'size'.
> >>
> >> Why?
> >
> > Tamas suggested it, size sounded more intuitive to him. I have no
> > problem with either nr or size.
>
> Size to me means something in bytes, which clearly isn't the case
> here. There's not even support for other then 4k pages so far.
>

Lets make it array_size then to clarify?

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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 04:26 PM, Julien Grall wrote:
> 
> 
> On 02/09/16 14:22, Razvan Cojocaru wrote:
>> On 09/02/2016 04:17 PM, Julien Grall wrote:
>>> Hello Razvan,
>>
>> Hello Julien, thanks for the email!
>>
>>> On 02/09/16 09:51, 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 

 ---
 Changes since V1 / RFC:
  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
and XENMEM_access_op_set_access_sparse to
XENMEM_access_op_set_access_multi.
  - Renamed the 'nr' parameter to 'size'.
  - Wrapped long line in the implementation of xc_set_mem_access_multi().
  - Factored out common code in _p2m_set_mem_access() (and modified
p2m_set_altp2m_mem_access() in the process, to take an unsigned
long argument instead of a gfn_t).
  - Factored out xenmem_access_t to p2m_access_t conversion code in
p2m_xenmem_access_to_p2m_access().
  - Added hypercall continuation support.
  - Added compat translation support.
  - No longer allocating buffers (now using copy_from_guest_offset()).
  - Added support for setting an array of access restrictions, as
suggested by Tamas Lengyel.
  - This patch incorporates Jan Beulich's "memory: fix compat handling
of XENMEM_access_op".
 ---
  tools/libxc/include/xenctrl.h |   4 ++
  tools/libxc/xc_mem_access.c   |  38 ++
  xen/arch/x86/mm/p2m.c | 161
 --
  xen/common/compat/memory.c|  24 +--
  xen/common/mem_access.c   |  11 +++
  xen/include/public/memory.h   |  14 +++-
  xen/include/xen/p2m-common.h  |   6 ++
>>>
>>> p2m-common.h contains prototype common between ARM and x86. I would have
>>> expected to see a change in the ARM port because of that.
>>
>> The only change to p2m-common.h is the addition of a single function,
>> p2m_set_mem_access_multi(). As long as the ARM bits don't make use of
>> it, there's nothing to modify there.
> 
> p2m_set_mem_access_multi is called from common code. If you had tried
> to build Xen on ARM you would have noticed a compilation error:
> 
> prelink.o: In function `mem_access_memop':
> /local/home/julieng/works/xen/xen/common/mem_access.c:80: undefined reference 
> to `p2m_set_mem_access_multi'
> aarch64-linux-gnu-ld: /local/home/julieng/works/xen/xen/.xen-syms.0: hidden 
> symbol `p2m_set_mem_access_multi' isn't defined

I see. Sorry about that. I'll fix that in V3.


Thanks,
Razvan


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

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

2016-09-02 Thread Julien Grall 


On 02/09/16 14:22, Razvan Cojocaru wrote:
> On 09/02/2016 04:17 PM, Julien Grall wrote:
>> Hello Razvan,
> 
> Hello Julien, thanks for the email!
> 
>> On 02/09/16 09:51, 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 
>>>
>>> ---
>>> Changes since V1 / RFC:
>>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>>>and XENMEM_access_op_set_access_sparse to
>>>XENMEM_access_op_set_access_multi.
>>>  - Renamed the 'nr' parameter to 'size'.
>>>  - Wrapped long line in the implementation of xc_set_mem_access_multi().
>>>  - Factored out common code in _p2m_set_mem_access() (and modified
>>>p2m_set_altp2m_mem_access() in the process, to take an unsigned
>>>long argument instead of a gfn_t).
>>>  - Factored out xenmem_access_t to p2m_access_t conversion code in
>>>p2m_xenmem_access_to_p2m_access().
>>>  - Added hypercall continuation support.
>>>  - Added compat translation support.
>>>  - No longer allocating buffers (now using copy_from_guest_offset()).
>>>  - Added support for setting an array of access restrictions, as
>>>suggested by Tamas Lengyel.
>>>  - This patch incorporates Jan Beulich's "memory: fix compat handling
>>>of XENMEM_access_op".
>>> ---
>>>  tools/libxc/include/xenctrl.h |   4 ++
>>>  tools/libxc/xc_mem_access.c   |  38 ++
>>>  xen/arch/x86/mm/p2m.c | 161
>>> --
>>>  xen/common/compat/memory.c|  24 +--
>>>  xen/common/mem_access.c   |  11 +++
>>>  xen/include/public/memory.h   |  14 +++-
>>>  xen/include/xen/p2m-common.h  |   6 ++
>>
>> p2m-common.h contains prototype common between ARM and x86. I would have
>> expected to see a change in the ARM port because of that.
> 
> The only change to p2m-common.h is the addition of a single function,
> p2m_set_mem_access_multi(). As long as the ARM bits don't make use of
> it, there's nothing to modify there.

p2m_set_mem_access_multi is called from common code. If you had tried
to build Xen on ARM you would have noticed a compilation error:

prelink.o: In function `mem_access_memop':
/local/home/julieng/works/xen/xen/common/mem_access.c:80: undefined reference 
to `p2m_set_mem_access_multi'
aarch64-linux-gnu-ld: /local/home/julieng/works/xen/xen/.xen-syms.0: hidden 
symbol `p2m_set_mem_access_multi' isn't defined

Regards,

-- 
Julien Grall

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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 04:10 PM, Jan Beulich wrote:
 On 02.09.16 at 15:03,  wrote:
>> On 09/02/2016 03:56 PM, Jan Beulich wrote:
>> On 02.09.16 at 14:41,  wrote:
 On 09/02/2016 03:33 PM, Jan Beulich wrote:
 On 02.09.16 at 14:21,  wrote:
>> On 09/02/2016 01:02 PM, Jan Beulich wrote:
 +/*
> + * Corresponding list of access settings for pfn_list
> + * Used only with XENMEM_access_op_set_access_multi
> + */
> +XEN_GUEST_HANDLE(uint8) access_list;
>>> And for both of them - I don't think the arrays are meant to be
>>> used for output? In which case they should be handles of const
>>> types.
>>
>> Actually I can't seem to be able to find a magic combination that works.
>>
>> XEN_GUEST_HANDLE(const uint8) access_list; tells me:
>>
>> ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
>> name a type
>>  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name
>>
>> which is fair. I've tried:
>>
>> XEN_GUEST_HANDLE(const_uint8) access_list;
>>
>> which does go further with the compilation process, but still kills it 
>> with:
>>
>> xen/include/compat/memory.h:154:5: error: unknown type name
>> '__compat_handle_const_uint8'
>>  COMPAT_HANDLE(const_uint8) access_list;
>>
>> What would be the appropriate const type to use here?
>
> This one. Did you check that handle actually gets created? Per
> my brief checking it doesn't look so. And neither the native one.

 Running ./configure again, followed by 'make clean' and a new 'make
 dist' didn't help, so if it's supposed to be generated automatically it
 doesn't seem to be (or I'm doing something wrong). I'll investigate more.
>>>
>>> The compat one is supposed to get auto-generated once the native
>>> one is there.
>>
>> Changing both handles to XEN_GUEST_HANLDE(const_void) compiles cleanly.
>> As soon as I change to either XEN_GUEST_HANLDE(const_uint8) or
>> XEN_GUEST_HANLDE(const_uint64) I start getting errors.
>>
>> Previously the auto-generation seems to have worked fine, so this is
>> likely something more subtle.
> 
> Oh, I see where the issue is: Both DEFINE_XEN_GUEST_HANDLE()
> and __DEFINE_XEN_GUEST_HANDLE() auto-magically produce const
> counterparts, but while DEFINE_COMPAT_HANDLE() does too,
> __DEFINE_COMPAT_HANDLE() doesn't. That'll need fixing, I think.

I see it, thanks! I'll submit a small patch shortly.


Thanks,
Razvan


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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 04:17 PM, Julien Grall wrote:
> Hello Razvan,

Hello Julien, thanks for the email!

> On 02/09/16 09:51, 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 
>>
>> ---
>> Changes since V1 / RFC:
>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>>and XENMEM_access_op_set_access_sparse to
>>XENMEM_access_op_set_access_multi.
>>  - Renamed the 'nr' parameter to 'size'.
>>  - Wrapped long line in the implementation of xc_set_mem_access_multi().
>>  - Factored out common code in _p2m_set_mem_access() (and modified
>>p2m_set_altp2m_mem_access() in the process, to take an unsigned
>>long argument instead of a gfn_t).
>>  - Factored out xenmem_access_t to p2m_access_t conversion code in
>>p2m_xenmem_access_to_p2m_access().
>>  - Added hypercall continuation support.
>>  - Added compat translation support.
>>  - No longer allocating buffers (now using copy_from_guest_offset()).
>>  - Added support for setting an array of access restrictions, as
>>suggested by Tamas Lengyel.
>>  - This patch incorporates Jan Beulich's "memory: fix compat handling
>>of XENMEM_access_op".
>> ---
>>  tools/libxc/include/xenctrl.h |   4 ++
>>  tools/libxc/xc_mem_access.c   |  38 ++
>>  xen/arch/x86/mm/p2m.c | 161
>> --
>>  xen/common/compat/memory.c|  24 +--
>>  xen/common/mem_access.c   |  11 +++
>>  xen/include/public/memory.h   |  14 +++-
>>  xen/include/xen/p2m-common.h  |   6 ++
> 
> p2m-common.h contains prototype common between ARM and x86. I would have
> expected to see a change in the ARM port because of that.

The only change to p2m-common.h is the addition of a single function,
p2m_set_mem_access_multi(). As long as the ARM bits don't make use of
it, there's nothing to modify there.


Thanks,
Razvan

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

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

2016-09-02 Thread Julien Grall 

Hello Razvan,

On 02/09/16 09:51, 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 

---
Changes since V1 / RFC:
 - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
   and XENMEM_access_op_set_access_sparse to
   XENMEM_access_op_set_access_multi.
 - Renamed the 'nr' parameter to 'size'.
 - Wrapped long line in the implementation of xc_set_mem_access_multi().
 - Factored out common code in _p2m_set_mem_access() (and modified
   p2m_set_altp2m_mem_access() in the process, to take an unsigned
   long argument instead of a gfn_t).
 - Factored out xenmem_access_t to p2m_access_t conversion code in
   p2m_xenmem_access_to_p2m_access().
 - Added hypercall continuation support.
 - Added compat translation support.
 - No longer allocating buffers (now using copy_from_guest_offset()).
 - Added support for setting an array of access restrictions, as
   suggested by Tamas Lengyel.
 - This patch incorporates Jan Beulich's "memory: fix compat handling
   of XENMEM_access_op".
---
 tools/libxc/include/xenctrl.h |   4 ++
 tools/libxc/xc_mem_access.c   |  38 ++
 xen/arch/x86/mm/p2m.c | 161 --
 xen/common/compat/memory.c|  24 +--
 xen/common/mem_access.c   |  11 +++
 xen/include/public/memory.h   |  14 +++-
 xen/include/xen/p2m-common.h  |   6 ++


p2m-common.h contains prototype common between ARM and x86. I would have 
expected to see a change in the ARM port because of that.


Regards,

--
Julien Grall

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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 15:03,  wrote:
> On 09/02/2016 03:56 PM, Jan Beulich wrote:
> On 02.09.16 at 14:41,  wrote:
>>> On 09/02/2016 03:33 PM, Jan Beulich wrote:
>>> On 02.09.16 at 14:21,  wrote:
> On 09/02/2016 01:02 PM, Jan Beulich wrote:
>>> +/*
 + * Corresponding list of access settings for pfn_list
 + * Used only with XENMEM_access_op_set_access_multi
 + */
 +XEN_GUEST_HANDLE(uint8) access_list;
>> And for both of them - I don't think the arrays are meant to be
>> used for output? In which case they should be handles of const
>> types.
>
> Actually I can't seem to be able to find a magic combination that works.
>
> XEN_GUEST_HANDLE(const uint8) access_list; tells me:
>
> ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
> name a type
>  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name
>
> which is fair. I've tried:
>
> XEN_GUEST_HANDLE(const_uint8) access_list;
>
> which does go further with the compilation process, but still kills it 
> with:
>
> xen/include/compat/memory.h:154:5: error: unknown type name
> '__compat_handle_const_uint8'
>  COMPAT_HANDLE(const_uint8) access_list;
>
> What would be the appropriate const type to use here?

 This one. Did you check that handle actually gets created? Per
 my brief checking it doesn't look so. And neither the native one.
>>>
>>> Running ./configure again, followed by 'make clean' and a new 'make
>>> dist' didn't help, so if it's supposed to be generated automatically it
>>> doesn't seem to be (or I'm doing something wrong). I'll investigate more.
>> 
>> The compat one is supposed to get auto-generated once the native
>> one is there.
> 
> Changing both handles to XEN_GUEST_HANLDE(const_void) compiles cleanly.
> As soon as I change to either XEN_GUEST_HANLDE(const_uint8) or
> XEN_GUEST_HANLDE(const_uint64) I start getting errors.
> 
> Previously the auto-generation seems to have worked fine, so this is
> likely something more subtle.

Oh, I see where the issue is: Both DEFINE_XEN_GUEST_HANDLE()
and __DEFINE_XEN_GUEST_HANDLE() auto-magically produce const
counterparts, but while DEFINE_COMPAT_HANDLE() does too,
__DEFINE_COMPAT_HANDLE() doesn't. That'll need fixing, I think.

Jan


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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 03:56 PM, Jan Beulich wrote:
 On 02.09.16 at 14:41,  wrote:
>> On 09/02/2016 03:33 PM, Jan Beulich wrote:
>> On 02.09.16 at 14:21,  wrote:
 On 09/02/2016 01:02 PM, Jan Beulich wrote:
>> +/*
>>> + * Corresponding list of access settings for pfn_list
>>> + * Used only with XENMEM_access_op_set_access_multi
>>> + */
>>> +XEN_GUEST_HANDLE(uint8) access_list;
> And for both of them - I don't think the arrays are meant to be
> used for output? In which case they should be handles of const
> types.

 Actually I can't seem to be able to find a magic combination that works.

 XEN_GUEST_HANDLE(const uint8) access_list; tells me:

 ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
 name a type
  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name

 which is fair. I've tried:

 XEN_GUEST_HANDLE(const_uint8) access_list;

 which does go further with the compilation process, but still kills it 
 with:

 xen/include/compat/memory.h:154:5: error: unknown type name
 '__compat_handle_const_uint8'
  COMPAT_HANDLE(const_uint8) access_list;

 What would be the appropriate const type to use here?
>>>
>>> This one. Did you check that handle actually gets created? Per
>>> my brief checking it doesn't look so. And neither the native one.
>>
>> Running ./configure again, followed by 'make clean' and a new 'make
>> dist' didn't help, so if it's supposed to be generated automatically it
>> doesn't seem to be (or I'm doing something wrong). I'll investigate more.
> 
> The compat one is supposed to get auto-generated once the native
> one is there.

Changing both handles to XEN_GUEST_HANLDE(const_void) compiles cleanly.
As soon as I change to either XEN_GUEST_HANLDE(const_uint8) or
XEN_GUEST_HANLDE(const_uint64) I start getting errors.

Previously the auto-generation seems to have worked fine, so this is
likely something more subtle.


Thanks,
Razvan

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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 14:41,  wrote:
> On 09/02/2016 03:33 PM, Jan Beulich wrote:
> On 02.09.16 at 14:21,  wrote:
>>> On 09/02/2016 01:02 PM, Jan Beulich wrote:
> +/*
>> + * Corresponding list of access settings for pfn_list
>> + * Used only with XENMEM_access_op_set_access_multi
>> + */
>> +XEN_GUEST_HANDLE(uint8) access_list;
 And for both of them - I don't think the arrays are meant to be
 used for output? In which case they should be handles of const
 types.
>>>
>>> Actually I can't seem to be able to find a magic combination that works.
>>>
>>> XEN_GUEST_HANDLE(const uint8) access_list; tells me:
>>>
>>> ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
>>> name a type
>>>  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name
>>>
>>> which is fair. I've tried:
>>>
>>> XEN_GUEST_HANDLE(const_uint8) access_list;
>>>
>>> which does go further with the compilation process, but still kills it with:
>>>
>>> xen/include/compat/memory.h:154:5: error: unknown type name
>>> '__compat_handle_const_uint8'
>>>  COMPAT_HANDLE(const_uint8) access_list;
>>>
>>> What would be the appropriate const type to use here?
>> 
>> This one. Did you check that handle actually gets created? Per
>> my brief checking it doesn't look so. And neither the native one.
> 
> Running ./configure again, followed by 'make clean' and a new 'make
> dist' didn't help, so if it's supposed to be generated automatically it
> doesn't seem to be (or I'm doing something wrong). I'll investigate more.

The compat one is supposed to get auto-generated once the native
one is there.

Jan


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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 03:33 PM, Jan Beulich wrote:
 On 02.09.16 at 14:21,  wrote:
>> On 09/02/2016 01:02 PM, Jan Beulich wrote:
 +/*
> + * Corresponding list of access settings for pfn_list
> + * Used only with XENMEM_access_op_set_access_multi
> + */
> +XEN_GUEST_HANDLE(uint8) access_list;
>>> And for both of them - I don't think the arrays are meant to be
>>> used for output? In which case they should be handles of const
>>> types.
>>
>> Actually I can't seem to be able to find a magic combination that works.
>>
>> XEN_GUEST_HANDLE(const uint8) access_list; tells me:
>>
>> ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
>> name a type
>>  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name
>>
>> which is fair. I've tried:
>>
>> XEN_GUEST_HANDLE(const_uint8) access_list;
>>
>> which does go further with the compilation process, but still kills it with:
>>
>> xen/include/compat/memory.h:154:5: error: unknown type name
>> '__compat_handle_const_uint8'
>>  COMPAT_HANDLE(const_uint8) access_list;
>>
>> What would be the appropriate const type to use here?
> 
> This one. Did you check that handle actually gets created? Per
> my brief checking it doesn't look so. And neither the native one.

Running ./configure again, followed by 'make clean' and a new 'make
dist' didn't help, so if it's supposed to be generated automatically it
doesn't seem to be (or I'm doing something wrong). I'll investigate more.


Thanks,
Razvan

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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 14:21,  wrote:
> On 09/02/2016 01:02 PM, Jan Beulich wrote:
>>> +/*
>>> > + * Corresponding list of access settings for pfn_list
>>> > + * Used only with XENMEM_access_op_set_access_multi
>>> > + */
>>> > +XEN_GUEST_HANDLE(uint8) access_list;
>> And for both of them - I don't think the arrays are meant to be
>> used for output? In which case they should be handles of const
>> types.
> 
> Actually I can't seem to be able to find a magic combination that works.
> 
> XEN_GUEST_HANDLE(const uint8) access_list; tells me:
> 
> ./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
> name a type
>  #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name
> 
> which is fair. I've tried:
> 
> XEN_GUEST_HANDLE(const_uint8) access_list;
> 
> which does go further with the compilation process, but still kills it with:
> 
> xen/include/compat/memory.h:154:5: error: unknown type name
> '__compat_handle_const_uint8'
>  COMPAT_HANDLE(const_uint8) access_list;
> 
> What would be the appropriate const type to use here?

This one. Did you check that handle actually gets created? Per
my brief checking it doesn't look so. And neither the native one.

Jan


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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 01:02 PM, Jan Beulich wrote:
>> +/*
>> > + * Corresponding list of access settings for pfn_list
>> > + * Used only with XENMEM_access_op_set_access_multi
>> > + */
>> > +XEN_GUEST_HANDLE(uint8) access_list;
> And for both of them - I don't think the arrays are meant to be
> used for output? In which case they should be handles of const
> types.

Actually I can't seem to be able to find a magic combination that works.

XEN_GUEST_HANDLE(const uint8) access_list; tells me:

./public/arch-x86/xen.h:53:41: error: '__guest_handle_const' does not
name a type
 #define __XEN_GUEST_HANDLE(name)__guest_handle_ ## name

which is fair. I've tried:

XEN_GUEST_HANDLE(const_uint8) access_list;

which does go further with the compilation process, but still kills it with:

xen/include/compat/memory.h:154:5: error: unknown type name
'__compat_handle_const_uint8'
 COMPAT_HANDLE(const_uint8) access_list;

What would be the appropriate const type to use here?


Thanks,
Razvan

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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 13:18,  wrote:
> On 09/02/2016 01:02 PM, Jan Beulich wrote:
> On 02.09.16 at 10:51,  wrote:
>>> Changes since V1 / RFC:
>>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>>>and XENMEM_access_op_set_access_sparse to
>>>XENMEM_access_op_set_access_multi.
>>>  - Renamed the 'nr' parameter to 'size'.
>> 
>> Why?
> 
> Tamas suggested it, size sounded more intuitive to him. I have no
> problem with either nr or size.

Size to me means something in bytes, which clearly isn't the case
here. There's not even support for other then 4k pages so far.

Jan


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

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

2016-09-02 Thread Razvan Cojocaru 
On 09/02/2016 01:02 PM, Jan Beulich wrote:
 On 02.09.16 at 10:51,  wrote:
>> Changes since V1 / RFC:
>>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>>and XENMEM_access_op_set_access_sparse to
>>XENMEM_access_op_set_access_multi.
>>  - Renamed the 'nr' parameter to 'size'.
> 
> Why?

Tamas suggested it, size sounded more intuitive to him. I have no
problem with either nr or size.

>>  - Wrapped long line in the implementation of xc_set_mem_access_multi().
>>  - Factored out common code in _p2m_set_mem_access() (and modified
>>p2m_set_altp2m_mem_access() in the process, to take an unsigned
>>long argument instead of a gfn_t).
>>  - Factored out xenmem_access_t to p2m_access_t conversion code in
>>p2m_xenmem_access_to_p2m_access().
>>  - Added hypercall continuation support.
>>  - Added compat translation support.
>>  - No longer allocating buffers (now using copy_from_guest_offset()).
>>  - Added support for setting an array of access restrictions, as
>>suggested by Tamas Lengyel.
>>  - This patch incorporates Jan Beulich's "memory: fix compat handling
>>of XENMEM_access_op".
> 
> I'm not sure that's okay; I'd have preferred you make the other
> patch a prereq (not the least because that one may want
> backporting, while this one certainly won't). In no event should
> such a bug fix go without mentioning in the commit message.

I agree, this is here simply because looking at the state of staging,
the patch hadn't yet made it in, and I was trying to be efficient and
have an intermediate review anyway. I'll manually apply your patch
before mine and remove that code from this patch.

>> @@ -1772,13 +1773,12 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned 
>> long gla,
>>  static inline
>>  int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
>>struct p2m_domain *ap2m, p2m_access_t a,
>> -  gfn_t gfn)
>> +  unsigned long gfn_l)
>>  {
> 
> I'm not really happy about such a step backwards.

I'll keep p2m_set_altp2m_mem_access() as it is then. I just thought that
going from unsigned long to gfn_t just to have to go back to unsigned
long slightly clutters up the code, but it's certainly not a huge
difference.

>> @@ -1811,21 +1811,39 @@ int p2m_set_altp2m_mem_access(struct domain *d, 
>> struct p2m_domain *hp2m,
>>   (current->domain != d));
>>  }
>>  
>> -/*
>> - * Set access type for a region of gfns.
>> - * If gfn == INVALID_GFN, sets the default access type.
>> - */
>> -long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
>> -uint32_t start, uint32_t mask, xenmem_access_t 
>> access,
>> -unsigned int altp2m_idx)
>> +static inline
>> +int _p2m_set_mem_access(struct domain *d, struct p2m_domain *p2m,
> 
> Considering the file we're in, can't this just be set_mem_access()?

Of course, I'll rename it.

> Also I'd leave the inlining decision completely to the compiler.

I took that cue from p2m_set_altp2m_mem_access() above. I'll remove the
inline.

>> +struct p2m_domain *ap2m, p2m_access_t a,
>> +unsigned long gfn_l)
>>  {
>> -struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
>> -p2m_access_t a, _a;
>> -p2m_type_t t;
>> -mfn_t mfn;
>> -unsigned long gfn_l;
>> -long rc = 0;
>> +int rc;
>>  
>> +if ( ap2m )
>> +{
>> +rc = p2m_set_altp2m_mem_access(d, p2m, ap2m, a, gfn_l);
>> +/* If the corresponding mfn is invalid we will want to just skip it 
>> */
>> +if ( rc && rc != -ESRCH )
>> +return rc;
> 
> If you just converted -ESRCH to 0 here, you could then ...
> 
>> +}
>> +else
>> +{
>> +mfn_t mfn;
>> +p2m_access_t _a;
>> +p2m_type_t t;
>> +
>> +mfn = p2m->get_entry(p2m, gfn_l, , &_a, 0, NULL, NULL);
>> +rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
>> +return rc;
> 
> ... ditch this extra return path and ...
> 
>> +}
>> +
>> +return 0;
> 
> ... have both cases come here.

I'll do that.

>> @@ -1851,17 +1897,8 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
>> uint32_t nr,
>>  ap2m = d->arch.altp2m_p2m[altp2m_idx];
>>  }
>>  
>> -switch ( access )
>> -{
>> -case 0 ... ARRAY_SIZE(memaccess) - 1:
>> -a = memaccess[access];
>> -break;
>> -case XENMEM_access_default:
>> -a = p2m->default_access;
>> -break;
>> -default:
>> +if ( p2m_xenmem_access_to_p2m_access(p2m, access, ) )
>>  return -EINVAL;
> 
> Either you make the helper function return bool, or you pass on
> whatever value it returned instead of assuming it's -EINVAL.

Fair enough, I'll return bool.

>> +long p2m_set_mem_access_multi(struct domain *d,
>> +

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 12:02,  wrote:
 On 02.09.16 at 10:51,  wrote:
>> @@ -320,6 +321,23 @@ int compat_memory_op(unsigned int cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) compat)
>>  break;
>>  }
>>  
>> +case XENMEM_access_op:
>> +{
>> +#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
>> +
>> +return mem_access_memop(cmd,
>> +guest_handle_cast(compat,
>> +  xen_mem_access_op_t));
>> +}
> 
> You translate into native format, but then cast the compat handle
> to its native counterpart type. Such casting is okay only when
> accompanied by a respectively invoked CHECK_* macro. Please
> follow the model other code here uses.
> 
> And reviewing this I notice that my earlier bug fix also is still not
> really correct: It causes hypercall_xlat_continuation() to be
> bypassed.

Actually no, that patch is fine because it (legitimately) uses a cast.
You would introduce the bypassing problem here as soon as you
properly handed the native handle to mem_access_memop().

Jan


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

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

2016-09-02 Thread Jan Beulich 
>>> On 02.09.16 at 10:51,  wrote:
> Changes since V1 / RFC:
>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>and XENMEM_access_op_set_access_sparse to
>XENMEM_access_op_set_access_multi.
>  - Renamed the 'nr' parameter to 'size'.

Why?

>  - Wrapped long line in the implementation of xc_set_mem_access_multi().
>  - Factored out common code in _p2m_set_mem_access() (and modified
>p2m_set_altp2m_mem_access() in the process, to take an unsigned
>long argument instead of a gfn_t).
>  - Factored out xenmem_access_t to p2m_access_t conversion code in
>p2m_xenmem_access_to_p2m_access().
>  - Added hypercall continuation support.
>  - Added compat translation support.
>  - No longer allocating buffers (now using copy_from_guest_offset()).
>  - Added support for setting an array of access restrictions, as
>suggested by Tamas Lengyel.
>  - This patch incorporates Jan Beulich's "memory: fix compat handling
>of XENMEM_access_op".

I'm not sure that's okay; I'd have preferred you make the other
patch a prereq (not the least because that one may want
backporting, while this one certainly won't). In no event should
such a bug fix go without mentioning in the commit message.

> @@ -1772,13 +1773,12 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned 
> long gla,
>  static inline
>  int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
>struct p2m_domain *ap2m, p2m_access_t a,
> -  gfn_t gfn)
> +  unsigned long gfn_l)
>  {

I'm not really happy about such a step backwards.

> @@ -1811,21 +1811,39 @@ int p2m_set_altp2m_mem_access(struct domain *d, 
> struct p2m_domain *hp2m,
>   (current->domain != d));
>  }
>  
> -/*
> - * Set access type for a region of gfns.
> - * If gfn == INVALID_GFN, sets the default access type.
> - */
> -long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> -uint32_t start, uint32_t mask, xenmem_access_t 
> access,
> -unsigned int altp2m_idx)
> +static inline
> +int _p2m_set_mem_access(struct domain *d, struct p2m_domain *p2m,

Considering the file we're in, can't this just be set_mem_access()?

Also I'd leave the inlining decision completely to the compiler.

> +struct p2m_domain *ap2m, p2m_access_t a,
> +unsigned long gfn_l)
>  {
> -struct p2m_domain *p2m = p2m_get_hostp2m(d), *ap2m = NULL;
> -p2m_access_t a, _a;
> -p2m_type_t t;
> -mfn_t mfn;
> -unsigned long gfn_l;
> -long rc = 0;
> +int rc;
>  
> +if ( ap2m )
> +{
> +rc = p2m_set_altp2m_mem_access(d, p2m, ap2m, a, gfn_l);
> +/* If the corresponding mfn is invalid we will want to just skip it 
> */
> +if ( rc && rc != -ESRCH )
> +return rc;

If you just converted -ESRCH to 0 here, you could then ...

> +}
> +else
> +{
> +mfn_t mfn;
> +p2m_access_t _a;
> +p2m_type_t t;
> +
> +mfn = p2m->get_entry(p2m, gfn_l, , &_a, 0, NULL, NULL);
> +rc = p2m->set_entry(p2m, gfn_l, mfn, PAGE_ORDER_4K, t, a, -1);
> +return rc;

... ditch this extra return path and ...

> +}
> +
> +return 0;

... have both cases come here.

> @@ -1851,17 +1897,8 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
> uint32_t nr,
>  ap2m = d->arch.altp2m_p2m[altp2m_idx];
>  }
>  
> -switch ( access )
> -{
> -case 0 ... ARRAY_SIZE(memaccess) - 1:
> -a = memaccess[access];
> -break;
> -case XENMEM_access_default:
> -a = p2m->default_access;
> -break;
> -default:
> +if ( p2m_xenmem_access_to_p2m_access(p2m, access, ) )
>  return -EINVAL;

Either you make the helper function return bool, or you pass on
whatever value it returned instead of assuming it's -EINVAL.

> +long p2m_set_mem_access_multi(struct domain *d,
> +  XEN_GUEST_HANDLE(uint64_t) pfn_list,
> +  XEN_GUEST_HANDLE(uint8) access_list,
> +  uint32_t size, 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 < size )
> +{
> +p2m_access_t a;
> +
> +uint8_t access;
> +uint64_t gfn_l;

Stray blank line between declarations.

> @@ -320,6 +321,23 @@ int compat_memory_op(unsigned

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

2016-09-02 Thread Wei Liu 
On Fri, Sep 02, 2016 at 11:51:06AM +0300, 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 
> 
> ---
> Changes since V1 / RFC:
>  - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
>and XENMEM_access_op_set_access_sparse to
>XENMEM_access_op_set_access_multi.
>  - Renamed the 'nr' parameter to 'size'.
>  - Wrapped long line in the implementation of xc_set_mem_access_multi().
>  - Factored out common code in _p2m_set_mem_access() (and modified
>p2m_set_altp2m_mem_access() in the process, to take an unsigned
>long argument instead of a gfn_t).
>  - Factored out xenmem_access_t to p2m_access_t conversion code in
>p2m_xenmem_access_to_p2m_access().
>  - Added hypercall continuation support.
>  - Added compat translation support.
>  - No longer allocating buffers (now using copy_from_guest_offset()).
>  - Added support for setting an array of access restrictions, as
>suggested by Tamas Lengyel.
>  - This patch incorporates Jan Beulich's "memory: fix compat handling
>of XENMEM_access_op".
> ---
>  tools/libxc/include/xenctrl.h |   4 ++
>  tools/libxc/xc_mem_access.c   |  38 ++

Acked-by: Wei Liu 

FAOD I am happy with hv maintainers handle this patch.

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

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

2016-09-02 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 

---
Changes since V1 / RFC:
 - Renamed xc_set_mem_access_sparse() to xc_set_mem_access_multi(),
   and XENMEM_access_op_set_access_sparse to
   XENMEM_access_op_set_access_multi.
 - Renamed the 'nr' parameter to 'size'.
 - Wrapped long line in the implementation of xc_set_mem_access_multi().
 - Factored out common code in _p2m_set_mem_access() (and modified
   p2m_set_altp2m_mem_access() in the process, to take an unsigned
   long argument instead of a gfn_t).
 - Factored out xenmem_access_t to p2m_access_t conversion code in
   p2m_xenmem_access_to_p2m_access().
 - Added hypercall continuation support.
 - Added compat translation support.
 - No longer allocating buffers (now using copy_from_guest_offset()).
 - Added support for setting an array of access restrictions, as
   suggested by Tamas Lengyel.
 - This patch incorporates Jan Beulich's "memory: fix compat handling
   of XENMEM_access_op".
---
 tools/libxc/include/xenctrl.h |   4 ++
 tools/libxc/xc_mem_access.c   |  38 ++
 xen/arch/x86/mm/p2m.c | 161 --
 xen/common/compat/memory.c|  24 +--
 xen/common/mem_access.c   |  11 +++
 xen/include/public/memory.h   |  14 +++-
 xen/include/xen/p2m-common.h  |   6 ++
 xen/include/xlat.lst  |   2 +-
 8 files changed, 215 insertions(+), 45 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 560ce7b..c2f14a6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2125,6 +2125,10 @@ 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_multi(xc_interface *xch, domid_t domain_id,
+uint8_t *access, uint64_t *pages,
+uint32_t size);
+
 /*
  * Gets the mem access for the given page (returned in access on success)
  */
diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index eee088c..d69d4f9 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 size)
+{
+DECLARE_HYPERCALL_BOUNCE(access, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+DECLARE_HYPERCALL_BOUNCE(pages, size * 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   = size,
+};
+
+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/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 812dbf6..ee6438b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include  /* copy_from_guest() */
 #include 
 #include 
 #include 
@@ -1772,13 +1773,12 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long 
gla,
 static inline
 int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
   struct p2m_domain *ap2m, p2m_access_t a,
-  gfn_t gfn)
+  unsigned long gfn_l)
 {
 mfn_t mfn;
 p2m_type_t t;
 p2m_access_t old_a;
 unsigned int page_order;
-unsigned long gfn_l = gfn_x(gfn);
 int rc;
 
 mfn = ap2m->get_entry(ap2m, gfn_l, , _a, 0, NULL, NULL);
@@ -1811,21 +1811,39 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
p2m_domain *hp2m,
  (current->domain != d));
 }
 
-/*
- * Set