Xen devel - Dec 2010 - [PATCH] Enble 6 argument hypercalls for HVMs

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 <ross.philipson@citrix.com>




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 14.12.10 at 23:16, Ross Philipson
<Ross.Philipson@citrix.com> 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 <ross.philipson@citrix.com>
I''m curious what hypercall there is that takes 6 arguments,
particularly on 64-bit (where the maximum so far is 4).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/12/2010 09:07, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> On 14.12.10 at 23:16, Ross Philipson
<Ross.Philipson@citrix.com> 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 <ross.philipson@citrix.com>
> 
> 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.

 -- Keir
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 15.12.10 at 11:06, Keir Fraser <keir@xen.org> wrote:
> On 15/12/2010 09:07, "Jan Beulich" <JBeulich@novell.com>
wrote:
> 
>>>>> On 14.12.10 at 23:16, Ross Philipson
<Ross.Philipson@citrix.com> 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 <ross.philipson@citrix.com>
>> 
>> 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):

        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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/12/2010 10:40, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> On 15.12.10 at 11:06, Keir Fraser <keir@xen.org> wrote:
>> On 15/12/2010 09:07, "Jan Beulich"
<JBeulich@novell.com> wrote:
>> 
>>>>>> On 14.12.10 at 23:16, Ross Philipson
<Ross.Philipson@citrix.com> 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 <ross.philipson@citrix.com>
>>> 
>>> 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
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 15.12.10 at 12:43, Keir Fraser <keir@xen.org> wrote:
> On 15/12/2010 10:40, "Jan Beulich" <JBeulich@novell.com>
wrote:
>> 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!
Oops, of course - it being taken is the problem, as it''ll be statically
mis-predicted.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/12/2010 12:12, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> On 15.12.10 at 12:43, Keir Fraser <keir@xen.org> wrote:
>> On 15/12/2010 10:40, "Jan Beulich"
<JBeulich@novell.com> wrote:
>>> 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!
> 
> Oops, of course - it being taken is the problem, as it''ll be
statically
> mis-predicted.
Well, moving the trace code out of line, perhaps into the fixup section,
would be good for cache locality. And then we would have a rarely taken
forward branch (since the fixup section comes after normal text).

I''m sure there are a bunch of places in our asm code where we could do
this.
Feel free to fix them all up, if you like.

 -- Keir
> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel