Re: [Xen-devel] [PATCH v7 02/15] argo: introduce the argo_op hypercall boilerplate

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 30 Jan 2019, Christopher Clark wrote:
> +#include <xen/errno.h>
> +#include <xen/guest_access.h>
> +
> +long
> +do_argo_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
> +           XEN_GUEST_HANDLE_PARAM(void) arg2, unsigned long arg3,
> +           unsigned long arg4)
> +{
> +    return -ENOSYS;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +long
> +compat_argo_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
> +               XEN_GUEST_HANDLE_PARAM(void) arg2, unsigned long arg3,
> +               unsigned long arg4)
> +{
> +    return -ENOSYS;
> +}
> +#endif

From an ARM perspective, it is not a good idea to use unsigned long as
hypercall parameters because they are going to be of different size on
arm32 and arm64. On ARM, there is no COMPAT code, and we try to keep a
single stable ABI across 32bit and 64bit hypervisors (pointers size
being the only exception and we deal with that using
XEN_GUEST_HANDLE_PARAM).

For this reason, given that we don't need arg3 and arg4 to actually be
64bit, it would be best to use explicitly sized integers instead. I
would use uint32_t or unsigned int for arg3 and arg4. That way, there
are not going to be any ABI compatibility issues between arm32 and arm64
and we could run, and even migrate, 32bit guests to a 64bit hypervisor
without problems.

I know that Andrew expressed concerns about using unsigned int before,
but don't we just need to make sure we are properly ignoring the top
32bit of arg3 and arg4 when the hypervisor is compiled 64bit?

I am really sorry for pointing this out so late in the review cycle, but
I only spotted it now.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel