Sorry for the formatting.
On Tue, 5 Feb 2019, 20:04 Stefano Stabellini, <sstabell...@kernel.org>
wrote:
> On Tue, 5 Feb 2019, Jan Beulich wrote:
> > >>> On 05.02.19 at 01:39, <sstabell...@kernel.org> wrote:
> > > 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?
> >
> > Are you saying that hypercall arguments made by a 32-bit guest on a
> > 64-bit hypervisor do not get zero-extended before reaching the C layer
> > (or more specifically the individual handlers, since on x86 we deal with
> > the necessary zero-extension in C nowadays)? What about
> > do_memory_op()'s first parameter then?
>
> If I remember right, there is no zero-extension, however, they should
> still be zero because they have always been zero -- nothing should
> change them in the VM lifetime. However, it is not great to rely on
> that, that is why I suggested to clear them on entry as an alternative,
> and also to have a single ABI between 32bit and 64bit.
I can't find the wording again on the Arm Arm (the pdf reader on the phone
is not great).
But I am afraid this is not correct. Upper 32-bit of the register will be
zeroed when writing a 32-bit value. So we never rely on the register to be
zeroed on boot.
> FYI do_memory_op is declared as follows on the Linux side for arm32 and
> arm64:
>
> int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
>
> When I went through all existing hypercalls to introduce them on arm32,
> I checked that we didn't actually need 64bit parameters, especially for
> cmd. I introduced them as int instead of long on the Linux side when
> possible (see include/xen/arm/hypercall.h), but I didn't attempt to
> modify all the existing Xen headers.
>
I don't understand your concern with unsigned long. We use them in
__DEFINE_XEN_GUEST_HANDLE that are in turn to describe guest pointer.
The problem with explicitly sized (i.e 32-bit) is you ignore the top
32-bit. This means you can't check the upper bits are always 0 and would
prevent extension.
Cheers,
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
