Re: [PATCH 3/6] x86/mem-paging: use guest handle for XENMEM_paging_op_prep

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.04.2020 10:37, Julien Grall wrote:
> On 16/04/2020 16:46, Jan Beulich wrote:
>> While it should have been this way from the beginning, not doing so will
>> become an actual problem with PVH Dom0.
> 
> I think the current code is also buggy on PV dom0 because the buffer
> is not locked in memory. So you have no promise the buffer will be
> present when calling the hypercall.

Quite possibly; I didn't looks at that aspect at all.

>> --- a/tools/libxc/xc_mem_paging.c
>> +++ b/tools/libxc/xc_mem_paging.c
>> @@ -26,15 +26,33 @@ static int xc_mem_paging_memop(xc_interf
>>                                  unsigned int op, uint64_t gfn, void *buffer)
> 
> NIT: As you switch the handle to use const, would it make to also
> make the buffer const?

A separate change, I would say, but if the tool stack maintainers
agree with doing so at the same time, I certainly can.

>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1779,7 +1779,8 @@ void p2m_mem_paging_populate(struct doma
>>    * mfn if populate was called for  gfn which was nominated but not 
>> evicted. In
>>    * this case only the p2mt needs to be forwarded.
>>    */
>> -int p2m_mem_paging_prep(struct domain *d, unsigned long gfn_l, uint64_t 
>> buffer)
>> +int p2m_mem_paging_prep(struct domain *d, unsigned long gfn_l,
>> +                        XEN_GUEST_HANDLE_PARAM(const_uint8) buffer)
> 
> Shouldn't this technically be XEN_GUEST_HANDLE_64() to match the field?

I think an argument can be made for going either way - as a function
parameter it should have the type chosen. Do you see any (possibly
just latent) breakage from using _PARAM() rather than _64()?

Jan