From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH V15 5/9] xen: Make gpfn related memops compatible with wider return values Date: Tue, 21 Apr 2015 15:47:17 +0100 Message-ID: <1429627637.4743.121.camel@citrix.com> References: <1429542384-23905-1-git-send-email-tklengyel@sec.in.tum.de> <1429542384-23905-6-git-send-email-tklengyel@sec.in.tum.de> <553519B5.8090202@citrix.com> <1429622611.4743.83.camel@citrix.com> <5536777B0200007800074579@mail.emea.novell.com> <1429626293.4743.115.camel@citrix.com> <55367AD702000078000745C0@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55367AD702000078000745C0@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: tim@xen.org, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, Andrew Cooper , julien.grall@linaro.org, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, keir@xen.org, Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org On Tue, 2015-04-21 at 15:29 +0100, Jan Beulich wrote: > >>> On 21.04.15 at 16:24, wrote: > > On Tue, 2015-04-21 at 15:14 +0100, Jan Beulich wrote: > >> >>> On 21.04.15 at 15:23, 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 > >> >> > >> >> 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)? > > It is, but do we really want to introduce other than just compat > mode helper interfaces (i.e. leaving the current ones alone, and > perhaps even making the new ones tools only) if we really care > about such setups in the first place? IIRC the original impetus for at least one part of this interface was for in guest use. I don't recall what the use was, I think it was the max pfn one which was used though. Perhaps in that case we can assume that a signed long is sufficient for any pfn they might see. But is that true? A 32-bit guest could see PFN>2^31 I think. Perhaps we should add these fixed version as a tools interface and consider only doing a fixed guest visible version of the max pfn one? Modulo confirming that I'm not misremembering... Ian.