On 18.10.21 16:28, Jan Beulich wrote:
> On 18.10.2021 15:27, Juergen Gross wrote:
>> On 18.10.21 14:31, Jan Beulich wrote:
>>> On 15.10.2021 14:51, Juergen Gross wrote:
>>>> hvm_memory_op() should take an unsigned long as cmd, like
>>>> do_memory_op().
>>>>
>>>> As hvm_memory_op() is basically just calling do_memory_op() (or
>>>> compat_memory_op()) passing through the parameters the cmd parameter
>>>> should have no smaller size than that of the called functions.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Nevertheless ...
>>>
>>>> --- a/xen/arch/x86/hvm/hypercall.c
>>>> +++ b/xen/arch/x86/hvm/hypercall.c
>>>> @@ -31,7 +31,7 @@
>>>> #include <public/hvm/hvm_op.h>
>>>> #include <public/hvm/params.h>
>>>>
>>>> -static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>> +static long hvm_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>> {
>>>> long rc;
>>>
>>> ... I think this would even better be dealt with by splitting the
>>> function into a native one (using unsigned long) and a compat one
>>> (using unsigned int).
>>
>> Why? In 32-bit case the value is naturally limited to 32 bits width
>> zero-extending perfectly fine to unsigned long.
>
> It all ends up working fine, yes. Else I wouldn't have given R-b.
> But the .compat slot of the hypercall table really should use a
> prototype without unsigned long, and then the calls wouldn't
> zero-extend the arguments anymore. And then the declaration would
> be wrong, as then it would need to be the callee to zero-extend if
> it wants to use 64-bit values.
>
>> Otherwise I couldn't use the same definition later.
>
> Right. And this will be less of a problem once the function pointer
> tables are gone, as then the compiler sees the real parameter types
> for the individual functions.
Okay, I understand that.
I'd prefer to do that as a followup patch (series) then.
Juergen