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

Re: [PATCH 04/12] xen/x86: modify hvm_memory_op() prototype


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 18 Oct 2021 16:28:38 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=VYuhlC88UZgMhO4pNH4L3Gtp9dKel0eI9TFhqa0Kdu4=; b=UoPHpqc8qCsuWexuGLkiQkFcXgvgSuVLDgZqme5pVOXYooFo3PaF5AJ4UDAnV2qt1Ha0sp+IVyiAROGvSV1UoAZkxdWIn6y854TfDYXMg0fCsOqbY6Yg+WQYSmFMry/LmsDEh7UpImEFZWNdxWZv9Vg8M7wgcViNeYjVzA47BLEUQc8L1jdlW5vfFcLUKdQylfBYBw1/0jppfWySCXwGCAQdTJ0XS0n5c/TBHJkdruGnx4HEU8BWNZvDFNSTu+NJNdXAY0UvE0Yf6xMQYxIUZdlsZJD1sPr6Fz5OP6H7/qUzRlOMKNW1fW/6IDJKOSCTUotOfIpW2+zZigzvh+fA1Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JojYliXSvpzXmLB6/Poeum+fu2QTGe2GfEli4QK+WBI8TeKtj6cPFi2aLljdtJlgmr+37jDjESFSBogGptE0h+ztiIBLHwdAGAO2tqFF6gOAsxDyCU6JNeelzIJCdLWeUYjDH8eWO7cfcapWsocPNnaxNVWsHBHNR5MwRr+oiwH66bUAb0frHanHUBkcH1yBEWrKQQD/dV13PkdoD5XdzOnIk14cAdjOHFSkwZTNEzH9E5Kz6ne2XK89U+B0dDzv+jE8B5vxdIu+Sd8VCWSGK3StaHtvr8ZBWBNc+HxhSFDF1rX5MNpw776yULXJHefsnDSzYfrSLUm+WGJ2e+BMdg==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 18 Oct 2021 14:28:46 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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@xxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> 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.

Jan




 


Rackspace

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