From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] Enble 6 argument hypercalls for HVMs Date: Wed, 15 Dec 2010 11:43:45 +0000 Message-ID: References: <4D08A920020000780002811F@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D08A920020000780002811F@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , Ross Philipson Cc: George Dunlap , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 15/12/2010 10:40, "Jan Beulich" wrote: >>>> On 15.12.10 at 11:06, Keir Fraser wrote: >> On 15/12/2010 09:07, "Jan Beulich" wrote: >> >>>>>> On 14.12.10 at 23:16, Ross Philipson wrote: >>>> Enable 6 argument hypercalls for HVMs. The hypercall code handles a sixth >>>> argument in EBP or R9 but the HVM code is not passing the value. >>>> >>>> Signed-off-by: Ross Philipson >>> >>> I'm curious what hypercall there is that takes 6 arguments, >>> particularly on 64-bit (where the maximum so far is 4). >> >> The v4v hypercalls in XenClient (not as yet submitted upstream) take 6 >> arguments. Multicalls also need fixing up for a sixth argument, making >> everything consistent with existing PV hypercall logic. > > I would generally take this as an indication that this actually works, > but at least with tracing enabled I can't see how it would on 64-bit > (note the last two reloads): Yes, well, obviously noone has tried 6-arg (or 5-arg!) hypercalls from a 64-bit PV guest with tracing enabled. The code appears obviously wrong here. Cc'ing George -- he should be able to test this and submit the obvious patch. This looks like a cut-n-paste error from x86_64/compat/entry.S into x86_64/entry.S -- it wouldn't have been picked up by George because we don't generally run any 64-bit PV guests. > call trace_hypercall > /* Now restore all the registers that trace_hypercall clobbered */ > movq UREGS_rax+SHADOW_BYTES(%rsp),%rax /* Hypercall # */ > movq UREGS_rdi+SHADOW_BYTES(%rsp),%rdi /* Arg 1 */ > movq UREGS_rsi+SHADOW_BYTES(%rsp),%rsi /* Arg 2 */ > movq UREGS_rdx+SHADOW_BYTES(%rsp),%rdx /* Arg 3 */ > movq UREGS_r10+SHADOW_BYTES(%rsp),%rcx /* Arg 4 */ > movq UREGS_rdi+SHADOW_BYTES(%rsp),%r8 /* Arg 5 */ > movq UREGS_rbp+SHADOW_BYTES(%rsp),%r9 /* Arg 6 */ > > Looking at this code also makes me wonder once again whether > it really is a good idea to have a generally not taken forward > branch here. Which generally not-taken branch? The 'je 1f' instruction generally *will* be taken! -- Keir > Jan >