Re: [PATCH v2 4/4] x86: adjustments to guest handle treatment

[Julien Grall <julien@xen.org>]

Hi,

On 22/04/2020 10:32, Jan Beulich wrote:
> On 22.04.2020 10:17, Julien Grall wrote:
>> On 21/04/2020 10:13, Jan Beulich wrote:
>>> First of all avoid excessive conversions. copy_{from,to}_guest(), for
>>> example, work fine with all of XEN_GUEST_HANDLE{,_64,_PARAM}().
>>>
>>> Further
>>> - do_physdev_op_compat() didn't use the param form for its parameter,
>>> - {hap,shadow}_track_dirty_vram() wrongly used the param form,
>>> - compat processor Px logic failed to check compatibility of native and
>>>     compat structures not further converted.
>>>
>>> As this eliminates all users of guest_handle_from_param() and as there's
>>> no real need to allow for conversions in both directions, drop the
>>> macros as well.
>>
>> I was kind of expecting both guest_handle_from_param() and
>> guest_handle_to_param() to be dropped together. May I ask why
>> you still need guest_handle_to_param()?
> 
> There are three (iirc) uses left which I don't really see how
> to sensibly replace. Take a look at the different callers of
> x86's vcpumask_to_pcpumask(), for example.

Oh, const_guest_handle_from_ptr() is returning a GUEST_HANDLE_PARAM. 
This is a bit odd but fair enough.

> 
>>> --- a/xen/include/xen/acpi.h
>>> +++ b/xen/include/xen/acpi.h
>>> @@ -184,8 +184,8 @@ static inline unsigned int acpi_get_csub
>>>    static inline void acpi_set_csubstate_limit(unsigned int new_limit) { return; }
>>>    #endif
>>>    -#ifdef XEN_GUEST_HANDLE_PARAM
>>> -int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32));
>>> +#ifdef XEN_GUEST_HANDLE
>>> +int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE(uint32));
>>>    #endif
>>
>> Do we really need to keep the #ifdef here?
> 
> I think so, yes, or else the original one wouldn't have been
> needed either. (Consider the header getting included without
> any of the public headers having got included first.) Dropping
> (if it was possible) this would be an orthogonal change imo.

Fair point.

Cheers,

-- 
Julien Grall