| Message ID | f3c57792-d372-a70f-691b-87681b83e898@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/mem-paging: misc cleanup | expand |
Hi Jan, 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. > The interface change is binary > compatible, but requires tools side producers to be re-built. > > Drop the bogus/unnecessary page alignment restriction on the input > buffer at the same time. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Is there really no way to avoid the buffer copying in libxc? > > --- 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? > { > xen_mem_paging_op_t mpo; > + DECLARE_HYPERCALL_BOUNCE(buffer, XC_PAGE_SIZE, > + XC_HYPERCALL_BUFFER_BOUNCE_IN); > + int rc; > > memset(&mpo, 0, sizeof(mpo)); > > mpo.op = op; > mpo.domain = domain_id; > mpo.gfn = gfn; > - mpo.buffer = (unsigned long) buffer; > > - return do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo)); > + if ( buffer ) > + { > + if ( xc_hypercall_bounce_pre(xch, buffer) ) > + { > + PERROR("Could not bounce memory for XENMEM_paging_op %u", op); > + return -1; > + } > + > + set_xen_guest_handle(mpo.buffer, buffer); > + } > + > + rc = do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo)); > + > + if ( buffer ) > + xc_hypercall_bounce_post(xch, buffer); > + > + return rc; > } > > int xc_mem_paging_enable(xc_interface *xch, uint32_t domain_id, > @@ -92,28 +110,13 @@ int xc_mem_paging_prep(xc_interface *xch > int xc_mem_paging_load(xc_interface *xch, uint32_t domain_id, > uint64_t gfn, void *buffer) > { > - int rc, old_errno; > - > errno = EINVAL; > > if ( !buffer ) > return -1; > > - if ( ((unsigned long) buffer) & (XC_PAGE_SIZE - 1) ) > - return -1; > - > - if ( mlock(buffer, XC_PAGE_SIZE) ) > - return -1; > - > - rc = xc_mem_paging_memop(xch, domain_id, > - XENMEM_paging_op_prep, > - gfn, buffer); > - > - old_errno = errno; > - munlock(buffer, XC_PAGE_SIZE); > - errno = old_errno; > - > - return rc; > + return xc_mem_paging_memop(xch, domain_id, XENMEM_paging_op_prep, > + gfn, buffer); > } > > > --- 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? Cheers,
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
Hi Jan, On 17/04/2020 10:44, Jan Beulich wrote: > 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. Ok. > >>> --- 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()? I know they are the same on x86, but from an abstract PoV they are fundamentally different. XEN_GUEST_HANDLE_PARAM() represents a guest pointer, when pased as an hypercall argument. XEN_GUEST_HANDLE() represents a guest pointer, when passed as a field in a struct in memory. In this case, the guest pointer was part of a structure. So I think you want to use XEN_GUEST_HANDLE(). FWIW, the different matters on Arm. Although, it looks like the compiler will not warn you if you are using the wrong handler :(. Cheers,
On 17.04.2020 19:13, Julien Grall wrote: > On 17/04/2020 10:44, Jan Beulich wrote: >> On 17.04.2020 10:37, Julien Grall wrote: >>> On 16/04/2020 16:46, Jan Beulich wrote: >>>> --- 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()? > I know they are the same on x86, but from an abstract PoV they are fundamentally different. > > XEN_GUEST_HANDLE_PARAM() represents a guest pointer, when pased as an > hypercall argument. > > XEN_GUEST_HANDLE() represents a guest pointer, when passed as a field > in a struct in memory. > > In this case, the guest pointer was part of a structure. So I think > you want to use XEN_GUEST_HANDLE(). Hmm, looks like I was confused about what the two mean. So far I was under the impression that _PARAM() was to be used for function parameters in general, not just hypercall ones. While the text near the macro definitions is quite clear in this regard, I'm afraid Stefano's original series (first and foremost commit abf06ea91d12's playing with e.g. handle_iomem_range()) was rather confusing than helpful - it looks to me as if quite a bit of the "casting" could actually be dropped (I'll see about doing some cleanup there). Plus I'm afraid other mixing of plain vs param has been introduced on x86, at least for dm.c:track_dirty_vram()'s calls to {hap,shadow}_track_dirty_vram(); this is just the first instance I've found - there may be more. > FWIW, the different matters on Arm. Although, it looks like the > compiler will not warn you if you are using the wrong handler :(. I find this highly suspicious, but can't check myself until back in the office - these are distinct compound types after all, so this shouldn't just be a warning, but an error. Or did you merely mean there's no warning on x86? Jan
Hi Jan, On 20/04/2020 08:26, Jan Beulich wrote: > On 17.04.2020 19:13, Julien Grall wrote: >> On 17/04/2020 10:44, Jan Beulich wrote: >>> On 17.04.2020 10:37, Julien Grall wrote: >>>> On 16/04/2020 16:46, Jan Beulich wrote: >>>>> --- 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()? >> I know they are the same on x86, but from an abstract PoV they are fundamentally different. >> >> XEN_GUEST_HANDLE_PARAM() represents a guest pointer, when pased as an >> hypercall argument. >> >> XEN_GUEST_HANDLE() represents a guest pointer, when passed as a field >> in a struct in memory. >> >> In this case, the guest pointer was part of a structure. So I think >> you want to use XEN_GUEST_HANDLE(). > > Hmm, looks like I was confused about what the two mean. So far I was > under the impression that _PARAM() was to be used for function > parameters in general, not just hypercall ones. While the text near > the macro definitions is quite clear in this regard, I'm afraid > Stefano's original series (first and foremost commit abf06ea91d12's > playing with e.g. handle_iomem_range()) was rather confusing than > helpful - it looks to me as if quite a bit of the "casting" could > actually be dropped (I'll see about doing some cleanup there). Plus > I'm afraid other mixing of plain vs param has been introduced on > x86, at least for dm.c:track_dirty_vram()'s calls to > {hap,shadow}_track_dirty_vram(); this is just the first instance > I've found - there may be more. I agree the commit you mention above is confusing. If we follow the definition, then the conversion between the two internally should never have been done. Maybe Stefano can clarify the intention? >> FWIW, the different matters on Arm. Although, it looks like the >> compiler will not warn you if you are using the wrong handler :(. > > I find this highly suspicious, but can't check myself until back > in the office - these are distinct compound types after all, so > this shouldn't just be a warning, but an error. Or did you merely > mean there's no warning on x86? I mean on Arm 32-bit. I have changed one of the function to use XEN_GUEST_HANDLE_PARAM() rather than XEN_GUEST_HANDLE() but not changing the caller. It is probably because they are both defined using an union. Interestly, the type will also not be checked, so the code a function will happily accept a XEN_GUEST_HANDLE_PARAM(uint8) even if the prototype requested XEN_GUEST_HANDLE_PARAM(uint64). This looks rather messy, maybe we should use a structure (and some alignment) to add more safety. Cheers,
On 20.04.2020 14:08, Julien Grall wrote: > On 20/04/2020 08:26, Jan Beulich wrote: >> On 17.04.2020 19:13, Julien Grall wrote: >>> FWIW, the different matters on Arm. Although, it looks like the >>> compiler will not warn you if you are using the wrong handler :(. >> >> I find this highly suspicious, but can't check myself until back >> in the office - these are distinct compound types after all, so >> this shouldn't just be a warning, but an error. Or did you merely >> mean there's no warning on x86? > > I mean on Arm 32-bit. I have changed one of the function to use XEN_GUEST_HANDLE_PARAM() rather than XEN_GUEST_HANDLE() but not changing the caller. > > It is probably because they are both defined using an union. Interestly, the type will also not be checked, so the code a function will happily accept a XEN_GUEST_HANDLE_PARAM(uint8) even if the prototype requested XEN_GUEST_HANDLE_PARAM(uint64). > > This looks rather messy, maybe we should use a structure (and some alignment) to add more safety. Are the unions plain ones? I could see room for behavior like the one you describe with transparent unions, albeit still not quite like you describe it. Getting handle types to be properly type-checked by the compiler is pretty imperative imo. Jan
Hi, On 20/04/2020 13:12, Jan Beulich wrote: > On 20.04.2020 14:08, Julien Grall wrote: >> On 20/04/2020 08:26, Jan Beulich wrote: >>> On 17.04.2020 19:13, Julien Grall wrote: >>>> FWIW, the different matters on Arm. Although, it looks like the >>>> compiler will not warn you if you are using the wrong handler :(. >>> >>> I find this highly suspicious, but can't check myself until back >>> in the office - these are distinct compound types after all, so >>> this shouldn't just be a warning, but an error. Or did you merely >>> mean there's no warning on x86? >> >> I mean on Arm 32-bit. I have changed one of the function to use XEN_GUEST_HANDLE_PARAM() rather than XEN_GUEST_HANDLE() but not changing the caller. >> >> It is probably because they are both defined using an union. Interestly, the type will also not be checked, so the code a function will happily accept a XEN_GUEST_HANDLE_PARAM(uint8) even if the prototype requested XEN_GUEST_HANDLE_PARAM(uint64). >> >> This looks rather messy, maybe we should use a structure (and some alignment) to add more safety. > > Are the unions plain ones? I could see room for behavior like > the one you describe with transparent unions, albeit still > not quite like you describe it. Getting handle types to be > properly type-checked by the compiler is pretty imperative imo. It looks like x86 is using structure, but arm is using plain union: #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ typedef union { type *p; unsigned long q; } \ __guest_handle_ ## name; \ typedef union { type *p; uint64_aligned_t q; } \ __guest_handle_64_ ## name I will look at introducing a union on Arm. Cheers,
On 20.04.2020 14:20, Julien Grall wrote: > Hi, > > On 20/04/2020 13:12, Jan Beulich wrote: >> On 20.04.2020 14:08, Julien Grall wrote: >>> On 20/04/2020 08:26, Jan Beulich wrote: >>>> On 17.04.2020 19:13, Julien Grall wrote: >>>>> FWIW, the different matters on Arm. Although, it looks like the >>>>> compiler will not warn you if you are using the wrong handler :(. >>>> >>>> I find this highly suspicious, but can't check myself until back >>>> in the office - these are distinct compound types after all, so >>>> this shouldn't just be a warning, but an error. Or did you merely >>>> mean there's no warning on x86? >>> >>> I mean on Arm 32-bit. I have changed one of the function to use XEN_GUEST_HANDLE_PARAM() rather than XEN_GUEST_HANDLE() but not changing the caller. >>> >>> It is probably because they are both defined using an union. Interestly, the type will also not be checked, so the code a function will happily accept a XEN_GUEST_HANDLE_PARAM(uint8) even if the prototype requested XEN_GUEST_HANDLE_PARAM(uint64). >>> >>> This looks rather messy, maybe we should use a structure (and some alignment) to add more safety. >> >> Are the unions plain ones? I could see room for behavior like >> the one you describe with transparent unions, albeit still >> not quite like you describe it. Getting handle types to be >> properly type-checked by the compiler is pretty imperative imo. > > It looks like x86 is using structure, but arm is using plain union: > > #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ > typedef union { type *p; unsigned long q; } \ > __guest_handle_ ## name; \ > typedef union { type *p; uint64_aligned_t q; } \ > __guest_handle_64_ ## name I don't see how this would make a difference, and hence ... > I will look at introducing a union on Arm. ... how this would help. I must be missing something, or there must be a very curious bug in only the Arm gcc. Jan
On 20.04.2020 14:31, Jan Beulich wrote: > On 20.04.2020 14:20, Julien Grall wrote: >> On 20/04/2020 13:12, Jan Beulich wrote: >>> On 20.04.2020 14:08, Julien Grall wrote: >>> Are the unions plain ones? I could see room for behavior like >>> the one you describe with transparent unions, albeit still >>> not quite like you describe it. Getting handle types to be >>> properly type-checked by the compiler is pretty imperative imo. >> >> It looks like x86 is using structure, but arm is using plain union: >> >> #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ >> typedef union { type *p; unsigned long q; } \ >> __guest_handle_ ## name; \ >> typedef union { type *p; uint64_aligned_t q; } \ >> __guest_handle_64_ ## name > > I don't see how this would make a difference, and hence ... > >> I will look at introducing a union on Arm. > > ... how this would help. I must be missing something, or there > must be a very curious bug in only the Arm gcc. Compiling this (for x86) properly raises two errors: union u1 { void *p; unsigned long v; }; union u2 { void *p; unsigned long v; }; void test(union u1 u1, union u2 u2) { test(u1, u1); test(u2, u2); } Jan
On 20/04/2020 13:35, Jan Beulich wrote: > On 20.04.2020 14:31, Jan Beulich wrote: >> On 20.04.2020 14:20, Julien Grall wrote: >>> On 20/04/2020 13:12, Jan Beulich wrote: >>>> On 20.04.2020 14:08, Julien Grall wrote: >>>> Are the unions plain ones? I could see room for behavior like >>>> the one you describe with transparent unions, albeit still >>>> not quite like you describe it. Getting handle types to be >>>> properly type-checked by the compiler is pretty imperative imo. >>> >>> It looks like x86 is using structure, but arm is using plain union: >>> >>> #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ >>> typedef union { type *p; unsigned long q; } \ >>> __guest_handle_ ## name; \ >>> typedef union { type *p; uint64_aligned_t q; } \ >>> __guest_handle_64_ ## name >> >> I don't see how this would make a difference, and hence ... >> >>> I will look at introducing a union on Arm. >> >> ... how this would help. I must be missing something, or there >> must be a very curious bug in only the Arm gcc. > > Compiling this (for x86) properly raises two errors: > > union u1 { void *p; unsigned long v; }; > union u2 { void *p; unsigned long v; }; > > void test(union u1 u1, union u2 u2) { > test(u1, u1); > test(u2, u2); > } You are right. It is just me not compiling properly. Sorry for the noise :( Cheers,
--- 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) { xen_mem_paging_op_t mpo; + DECLARE_HYPERCALL_BOUNCE(buffer, XC_PAGE_SIZE, + XC_HYPERCALL_BUFFER_BOUNCE_IN); + int rc; memset(&mpo, 0, sizeof(mpo)); mpo.op = op; mpo.domain = domain_id; mpo.gfn = gfn; - mpo.buffer = (unsigned long) buffer; - return do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo)); + if ( buffer ) + { + if ( xc_hypercall_bounce_pre(xch, buffer) ) + { + PERROR("Could not bounce memory for XENMEM_paging_op %u", op); + return -1; + } + + set_xen_guest_handle(mpo.buffer, buffer); + } + + rc = do_memory_op(xch, XENMEM_paging_op, &mpo, sizeof(mpo)); + + if ( buffer ) + xc_hypercall_bounce_post(xch, buffer); + + return rc; } int xc_mem_paging_enable(xc_interface *xch, uint32_t domain_id, @@ -92,28 +110,13 @@ int xc_mem_paging_prep(xc_interface *xch int xc_mem_paging_load(xc_interface *xch, uint32_t domain_id, uint64_t gfn, void *buffer) { - int rc, old_errno; - errno = EINVAL; if ( !buffer ) return -1; - if ( ((unsigned long) buffer) & (XC_PAGE_SIZE - 1) ) - return -1; - - if ( mlock(buffer, XC_PAGE_SIZE) ) - return -1; - - rc = xc_mem_paging_memop(xch, domain_id, - XENMEM_paging_op_prep, - gfn, buffer); - - old_errno = errno; - munlock(buffer, XC_PAGE_SIZE); - errno = old_errno; - - return rc; + return xc_mem_paging_memop(xch, domain_id, XENMEM_paging_op_prep, + gfn, buffer); } --- 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) { struct page_info *page = NULL; p2m_type_t p2mt; @@ -1788,13 +1789,9 @@ int p2m_mem_paging_prep(struct domain *d mfn_t mfn; struct p2m_domain *p2m = p2m_get_hostp2m(d); int ret, page_extant = 1; - const void *user_ptr = (const void *) buffer; - if ( user_ptr ) - /* Sanity check the buffer and bail out early if trouble */ - if ( (buffer & (PAGE_SIZE - 1)) || - (!access_ok(user_ptr, PAGE_SIZE)) ) - return -EINVAL; + if ( !guest_handle_okay(buffer, PAGE_SIZE) ) + return -EINVAL; gfn_lock(p2m, gfn, 0); @@ -1812,7 +1809,7 @@ int p2m_mem_paging_prep(struct domain *d /* If the user did not provide a buffer, we disallow */ ret = -EINVAL; - if ( unlikely(user_ptr == NULL) ) + if ( unlikely(guest_handle_is_null(buffer)) ) goto out; /* Get a free page */ ret = -ENOMEM; @@ -1834,7 +1831,7 @@ int p2m_mem_paging_prep(struct domain *d ASSERT( mfn_valid(mfn) ); guest_map = map_domain_page(mfn); - ret = copy_from_user(guest_map, user_ptr, PAGE_SIZE); + ret = copy_from_guest(guest_map, buffer, PAGE_SIZE); unmap_domain_page(guest_map); if ( ret ) { --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -741,7 +741,8 @@ void p2m_mem_paging_drop_page(struct dom /* Start populating a paged out frame */ void p2m_mem_paging_populate(struct domain *d, unsigned long gfn); /* Prepare the p2m for paging a frame in */ -int p2m_mem_paging_prep(struct domain *d, unsigned long gfn, uint64_t buffer); +int p2m_mem_paging_prep(struct domain *d, unsigned long gfn, + XEN_GUEST_HANDLE_PARAM(const_uint8) buffer); /* Resume normal operation (in case a domain was paused) */ struct vm_event_st; void p2m_mem_paging_resume(struct domain *d, struct vm_event_st *rsp); --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -396,10 +396,10 @@ struct xen_mem_paging_op { uint8_t op; /* XENMEM_paging_op_* */ domid_t domain; - /* PAGING_PREP IN: buffer to immediately fill page in */ - uint64_aligned_t buffer; - /* Other OPs */ - uint64_aligned_t gfn; /* IN: gfn of page being operated on */ + /* IN: (XENMEM_paging_op_prep) buffer to immediately fill page from */ + XEN_GUEST_HANDLE_64(const_uint8) buffer; + /* IN: gfn of page being operated on */ + uint64_aligned_t gfn; }; typedef struct xen_mem_paging_op xen_mem_paging_op_t; DEFINE_XEN_GUEST_HANDLE(xen_mem_paging_op_t);
While it should have been this way from the beginning, not doing so will become an actual problem with PVH Dom0. The interface change is binary compatible, but requires tools side producers to be re-built. Drop the bogus/unnecessary page alignment restriction on the input buffer at the same time. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Is there really no way to avoid the buffer copying in libxc?