Re: [Xen-devel] [PATCH 1/4] VMX: streamline entry.S code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.08.13 at 13:48, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 26/08/2013 12:01, Jan Beulich wrote:
>>>> -.globl vmx_asm_do_vmentry
>>>> -vmx_asm_do_vmentry:
>>> If you move the ENTRY(vmx_asm_do_vmentry) up from below, you should be
>>> able to completely drop the jmp in it.
>> That would be possible, at the expense of added padding. I prefer
>> it the way it is now, as vmx_asm_do_vmentry is not performance
>> critical (as being used exactly once per HVM vCPU).
> 
> There are a number of places where we have ENTRY()-like constructs but
> don't want the padding with it.
> 
> Would an __ENTRY() macro go down well?  I can spin a patch for it.

x86 Linux has GLOBAL() for that purpose - I'd like this better than
__ENTRY() both from a name space perspective and from
describing its purpose.

> My point about re-executing it does still apply.  Looking at the code, I
> do not believe it is correct to be executing vmx_intr_assist or
> nvmx_switch_guest multiple times on a context switch to an HVM VCPU. 
> vmx_intr_assist at the very least has a huge amount of work to do before
> it considers exiting.
> 
> It does appear that there is possible interaction between do_softirq()
> and vmx_intr_assist(), at which point vmx_intr_assist() should be run
> after do_softirq(), which removes the apparently redundant run with
> interrupts enabled.

None of this seems related to the patch anymore - if you think
there's more stuff that needs changing, let's discuss this in a
separate thread.

>> The %cr2 write's move is indeed debatable - I tried to get it farther
>> away from the producer of the data in %rax, but it's not clear
>> whether that's very useful. The second purpose was to get
>> something interleaved with the many "pop"s, so that the CPU can
>> get busy other than just its memory load ports. If controversial
>> I'm fine with undoing that change.
> 
> From my understanding of a serialising instruction, it forces the
> completion of all previous instructions before starting, and prevents
> the issue of any subsequent instructions until it itself has completed.
> 
> Therefore, I doubt it has the intended effect.

Wait - this is again also a separation from the producer of the
data. Whether modern CPUs can deal with that I'm not sure,
but it surely doesn't hurt to hide eventual latency.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel