On Tue, Apr 21, 2015 at 5:13 PM, Jan Beulich <jbeul...@suse.com> wrote:
> >>> On 21.04.15 at 16:42, <tkleng...@sec.in.tum.de> wrote: > > On Tue, Apr 21, 2015 at 4:29 PM, Jan Beulich <jbeul...@suse.com> wrote: > > > >> >>> On 21.04.15 at 16:24, <ian.campb...@citrix.com> wrote: > >> > On Tue, 2015-04-21 at 15:14 +0100, Jan Beulich wrote: > >> >> >>> On 21.04.15 at 15:23, <ian.campb...@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 <tkleng...@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)? > >> > >> 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? > > > > At the moment I just followed Andrew's advice and will introduce a new > > XENMEM_maximum_gpfn2 memop that returns the gpfn in a struct as > xen_pfn_t. > > The old memops I'll leave untouched if that's OK. > > For this specific one - is there a reasonable use case? Other than > for host PFN, we have control over guest ones, and I'm not sure > managing a guest with GPFNs extending past 4 billion can be > expected to work if only this one hypercall got fixed. IOW I'm > expecting to NAK any such addition without proper rationale. > > Jan > AFAIK everything works for me as it is already without this patch. I'm not sure if the cornercase of pfn's > 32-bit wide on ARM64 is actually something that would work/is supported at the moment. Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
