Re: [PATCH V15 5/9] xen: Make gpfn related memops compatible with wider return values

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-04-21 at 15:14 +0100, Jan Beulich wrote:
> >>> On 21.04.15 at 15:23, <ian.campbell@citrix.com> wrote:
> > On Mon, 2015-04-20 at 16:22 +0100, Andrew Cooper wrote:
> >> On 20/04/15 16:06, Tamas K Lengyel wrote:
> >> > The current implementation of three memops, XENMEM_current_reservation,
> >> > XENMEM_maximum_reservation and XENMEM_maximum_gpfn return values as an
> >> > int. However, in ARM64 we could potentially have 36-bit pfn's, thus
> >> > in preparation for the ARM patch, in this patch we update the existing
> >> > memop routines to use a struct, xen_get_gpfn, to exchange the gpfn info
> >> > as a uin64_t.
> >> >
> >> > This patch also adds error checking on the toolside in case the memop
> >> > fails.
> >> >
> >> > Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
> >> 
> >> XENMEM, unlikely domctls/sysctls is a guest-visible stable ABI/API.
> >> 
> >> You cannot make adjustments like this, but you can add a brand new op
> >> with appropriate parameters and list the old ops as deprecated.
> > 
> > Right. For the benefit of callers using the old API it seems what we
> > usually do is rename the old op XENMEM_foo_compat and use the name with
> > a new number for the new functionality, then use a
> > __XEN_INTERFACE_VERSION__ to #define back to the old name.
> > 
> > The handling of __HYPERVISOR_sched_op in public/xen.h seems like a
> > reasonable example, I couldn't find one specifically for the memory ops.
> 
> And there's no need to afaict: This complication isn't needed in the
> first place. The patch's context already makes this clear:
> 
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -838,12 +838,16 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> 
> Note the "long" return type. Yet the patch description, for
> whatever reason, claims the hypercall to only return an "int".
> Maybe because (as pointed out before) the respective Linux
> hypercall stub wrongly an "int" return type?

Isn't this still an issue for 32-bit toolstack (long == 4 bytes) on a 64
bit host (maximum pfn more than 2^32)?