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

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

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

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.

Okay, I understand that.

I'd prefer to do that as a followup patch (series) then.


Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature



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