This reverts a small part of 7e21b25059ed ("xen: harmonize return types
of hypercall handlers"). The change from "unsigned long" to "unsigned
int" for the native handler function meant that previously invalid
values became valid. While perhaps not a significant issue, strictly
speaking that's still a change to the ABI. Don't go as far as restoring
the compat entry point's type though: That one can't have values passed
which don't fit in 32 bits.

Note that as a side effect this fixes the invocation of
hypercall_create_continuation(), which by mistake wasn't adjusted by the
earlier change.

Also take the opportunity and correct the respective comment in the
public header. (The way it was it really supports that it probably was
pointless to use "long", but that's the way the hypercall was
introduced.)

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -1213,7 +1213,7 @@ static int kexec_status(XEN_GUEST_HANDLE
     return !!test_bit(bit, &kexec_flags);
 }
 
-static int do_kexec_op_internal(unsigned int op,
+static int do_kexec_op_internal(unsigned long op,
                                 XEN_GUEST_HANDLE_PARAM(void) uarg,
                                 bool_t compat)
 {
@@ -1265,7 +1265,7 @@ static int do_kexec_op_internal(unsigned
     return ret;
 }
 
-long do_kexec_op(unsigned int op, XEN_GUEST_HANDLE_PARAM(void) uarg)
+long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     return do_kexec_op_internal(op, uarg, 0);
 }
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -117,9 +117,6 @@ callback_op(int cmd, const void *arg)
 #ifdef CONFIG_ARGO
 argo_op(unsigned int cmd, void *arg1, void *arg2, unsigned long arg3, unsigned long arg4)
 #endif
-#ifdef CONFIG_KEXEC
-kexec_op(unsigned int op, void *uarg)
-#endif
 #ifdef CONFIG_PV
 iret()
 nmi_op(unsigned int cmd, void *arg)
@@ -149,6 +146,9 @@ update_va_mapping_otherdomain(unsigned i
 #ifndef CONFIG_PV_SHIM_EXCLUSIVE
 platform_op(compat_platform_op_t *u_xenpf_op)
 #endif
+#ifdef CONFIG_KEXEC
+kexec_op(unsigned int op, void *uarg)
+#endif
 #endif /* CONFIG_COMPAT */
 
 #if defined(CONFIG_PV) || defined(CONFIG_ARM)
@@ -181,6 +181,9 @@ update_descriptor(uint64_t gaddr, seg_de
 update_va_mapping(unsigned long va, uint64_t val64, unsigned long flags)
 update_va_mapping_otherdomain(unsigned long va, uint64_t val64, unsigned long flags, domid_t domid)
 #endif
+#ifdef CONFIG_KEXEC
+kexec_op(unsigned long op, void *uarg)
+#endif
 #ifdef CONFIG_IOREQ_SERVER
 dm_op(domid_t domid, unsigned int nr_bufs, xen_dm_op_buf_t *bufs)
 #endif
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -64,7 +64,7 @@
 
 /*
  * Prototype for this hypercall is:
- *  int kexec_op(int cmd, void *args)
+ *  int kexec_op(unsigned long cmd, void *args)
  * @cmd  == KEXEC_CMD_...
  *          KEXEC operation to perform
  * @args == Operation-specific extra arguments (NULL if none).

On 07/11/2022 15:09, Jan Beulich wrote:
> This reverts a small part of 7e21b25059ed ("xen: harmonize return types
> of hypercall handlers"). The change from "unsigned long" to "unsigned
> int" for the native handler function meant that previously invalid
> values became valid. While perhaps not a significant issue, strictly
> speaking that's still a change to the ABI. Don't go as far as restoring
> the compat entry point's type though: That one can't have values passed
> which don't fit in 32 bits.
>
> Note that as a side effect this fixes the invocation of
> hypercall_create_continuation(), which by mistake wasn't adjusted by the
> earlier change.
>
> Also take the opportunity and correct the respective comment in the
> public header. (The way it was it really supports that it probably was
> pointless to use "long", but that's the way the hypercall was
> introduced.)
>
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Thankyou for fixing this. 

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

On 07.11.22 16:09, Jan Beulich wrote:
> This reverts a small part of 7e21b25059ed ("xen: harmonize return types
> of hypercall handlers"). The change from "unsigned long" to "unsigned
> int" for the native handler function meant that previously invalid
> values became valid. While perhaps not a significant issue, strictly
> speaking that's still a change to the ABI. Don't go as far as restoring
> the compat entry point's type though: That one can't have values passed
> which don't fit in 32 bits.
> 
> Note that as a side effect this fixes the invocation of
> hypercall_create_continuation(), which by mistake wasn't adjusted by the
> earlier change.
> 
> Also take the opportunity and correct the respective comment in the
> public header. (The way it was it really supports that it probably was
> pointless to use "long", but that's the way the hypercall was
> introduced.)
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Hi Jan,

Thanks for the patch.

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Subject: [PATCH][4.17] kexec: restore hypercall 1st arg's type
> 
> This reverts a small part of 7e21b25059ed ("xen: harmonize return types
> of hypercall handlers"). The change from "unsigned long" to "unsigned
> int" for the native handler function meant that previously invalid
> values became valid. While perhaps not a significant issue, strictly
> speaking that's still a change to the ABI. Don't go as far as restoring
> the compat entry point's type though: That one can't have values passed
> which don't fit in 32 bits.
> 
> Note that as a side effect this fixes the invocation of
> hypercall_create_continuation(), which by mistake wasn't adjusted by the
> earlier change.
> 
> Also take the opportunity and correct the respective comment in the
> public header. (The way it was it really supports that it probably was
> pointless to use "long", but that's the way the hypercall was
> introduced.)
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry