[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
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |