[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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.


--- 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. 
    * 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 :(.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.