|
On 23/08/2013 15:01, Jan Beulich wrote:
- move stuff easily/better done in C into C code
- re-arrange code paths so that no redundant GET_CURRENT() would remain
on the fast paths
- move long latency operations earlier
- slightly defer disabling interrupts on the VM entry path
- use ENTRY() instead of open coding it
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
This is quite tough reviewing as all the changes are mixed together,
but I think I have got it all now.
WRT moving things to C, and use of ENTRY(), fine.
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -28,109 +28,66 @@
#define VMRESUME .byte 0x0f,0x01,0xc3
#define VMLAUNCH .byte 0x0f,0x01,0xc2
-#define VMREAD(off) .byte 0x0f,0x78,0x47,((off)-UREGS_rip)
-#define VMWRITE(off) .byte 0x0f,0x79,0x47,((off)-UREGS_rip)
-/* VMCS field encodings */
-#define GUEST_RSP 0x681c
-#define GUEST_RIP 0x681e
-#define GUEST_RFLAGS 0x6820
-
- ALIGN
-.globl vmx_asm_vmexit_handler
-vmx_asm_vmexit_handler:
+ENTRY(vmx_asm_vmexit_handler)
push %rdi
push %rsi
push %rdx
push %rcx
push %rax
+ mov %cr2,%rax
I presume this is a long latency instruction. Do you have a source
of numbers for this? (more for interest, as I can easily accept that
it would be a longer operation than the surrounding ones)
push %r8
push %r9
push %r10
push %r11
push %rbx
+ GET_CURRENT(%rbx)
This seems a little less obvious. I presume you are just breaking
true read-after-write data hazard on %rbx ?
push %rbp
push %r12
push %r13
push %r14
push %r15
- GET_CURRENT(%rbx)
-
movb $1,VCPU_vmx_launched(%rbx)
-
- lea UREGS_rip(%rsp),%rdi
- mov $GUEST_RIP,%eax
- /*VMREAD(UREGS_rip)*/
- .byte 0x0f,0x78,0x07 /* vmread %rax,(%rdi) */
- mov $GUEST_RSP,%eax
- VMREAD(UREGS_rsp)
- mov $GUEST_RFLAGS,%eax
- VMREAD(UREGS_eflags)
-
- mov %cr2,%rax
mov %rax,VCPU_hvm_guest_cr2(%rbx)
-#ifndef NDEBUG
- mov $0xbeef,%ax
- mov %ax,UREGS_error_code(%rsp)
- mov %ax,UREGS_entry_vector(%rsp)
- mov %ax,UREGS_saved_upcall_mask(%rsp)
- mov %ax,UREGS_cs(%rsp)
- mov %ax,UREGS_ds(%rsp)
- mov %ax,UREGS_es(%rsp)
- mov %ax,UREGS_fs(%rsp)
- mov %ax,UREGS_gs(%rsp)
- mov %ax,UREGS_ss(%rsp)
-#endif
-
mov %rsp,%rdi
call vmx_vmexit_handler
-.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. However...
+.Lvmx_do_vmentry:
call vmx_intr_assist
call nvmx_switch_guest
ASSERT_NOT_IN_ATOMIC
- GET_CURRENT(%rbx)
- cli
The movement of this cli indicates a possible issue.
If we have softirqs pending, we jump to .Lvmx_process_softirqs,
which calls do_softirq, and then jumps back to the top of
.Lvmx_do_vmentry, which reruns the top of do_vmentry with interrupts
now enabled.
First of all, I cant see anything in vmx_intr_assist or
nvmx_switch_guest which should require calling multiple times on a
vmentry. They are also expecting to be called with interrupts
disabled (although I cant spot anything obvious in the callpath
which would be affected).
Perhaps the jumps vmx_goto_emulator and vmx_process_softirqs should
turn into calls, to prevent repeatedly reruning the top of
vmx_do_vmentry? Then, ENTRY(vmx_do_vmentry) and .Lvmx_do_vmentry
could be collapsed together.
-
mov VCPU_processor(%rbx),%eax
- shl $IRQSTAT_shift,%eax
lea irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
- cmpl $0,(%rdx,%rax,1)
+ xor %ecx,%ecx
+ shl $IRQSTAT_shift,%eax
+ cli
+ cmp %ecx,(%rdx,%rax,1)
jnz .Lvmx_process_softirqs
- testb $0xff,VCPU_vmx_emulate(%rbx)
- jnz .Lvmx_goto_emulator
- testb $0xff,VCPU_vmx_realmode(%rbx)
-UNLIKELY_START(nz, realmode)
- cmpw $0,VCPU_vm86_seg_mask(%rbx)
+ cmp %cl,VCPU_vmx_emulate(%rbx)
+ jne .Lvmx_goto_emulator
+ cmp %cl,VCPU_vmx_realmode(%rbx)
+UNLIKELY_START(ne, realmode)
+ cmp %cx,VCPU_vm86_seg_mask(%rbx)
jnz .Lvmx_goto_emulator
mov %rsp,%rdi
call vmx_enter_realmode
UNLIKELY_END(realmode)
+ mov %rsp,%rdi
call vmx_vmenter_helper
mov VCPU_hvm_guest_cr2(%rbx),%rax
- mov %rax,%cr2
-
- lea UREGS_rip(%rsp),%rdi
- mov $GUEST_RIP,%eax
- /*VMWRITE(UREGS_rip)*/
- .byte 0x0f,0x79,0x07 /* vmwrite (%rdi),%rax */
- mov $GUEST_RSP,%eax
- VMWRITE(UREGS_rsp)
- mov $GUEST_RFLAGS,%eax
- VMWRITE(UREGS_eflags)
- cmpb $0,VCPU_vmx_launched(%rbx)
pop %r15
pop %r14
pop %r13
pop %r12
pop %rbp
+ mov %rax,%cr2
+ cmpb $0,VCPU_vmx_launched(%rbx)
Again, I presume the move of "mov %rax,%cr2" is about the %rax data
hazard?
~Andrew
pop %rbx
pop %r11
pop %r10
@@ -155,13 +112,17 @@ UNLIKELY_END(realmode)
call vm_launch_fail
ud2
+ENTRY(vmx_asm_do_vmentry)
+ GET_CURRENT(%rbx)
+ jmp .Lvmx_do_vmentry
+
.Lvmx_goto_emulator:
sti
mov %rsp,%rdi
call vmx_realmode
- jmp vmx_asm_do_vmentry
+ jmp .Lvmx_do_vmentry
.Lvmx_process_softirqs:
sti
call do_softirq
- jmp vmx_asm_do_vmentry
+ jmp .Lvmx_do_vmentry
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2375,6 +2375,12 @@ void vmx_vmexit_handler(struct cpu_user_
unsigned long exit_qualification, inst_len = 0;
struct vcpu *v = current;
+ regs->rip = __vmread(GUEST_RIP);
+ regs->rsp = __vmread(GUEST_RSP);
+ regs->rflags = __vmread(GUEST_RFLAGS);
+
+ hvm_invalidate_regs_fields(regs);
+
if ( paging_mode_hap(v->domain) && hvm_paging_enabled(v) )
v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3] =
__vmread(GUEST_CR3);
@@ -2868,7 +2874,7 @@ out:
nvmx_idtv_handling();
}
-void vmx_vmenter_helper(void)
+void vmx_vmenter_helper(const struct cpu_user_regs *regs)
{
struct vcpu *curr = current;
u32 new_asid, old_asid;
@@ -2910,6 +2916,10 @@ void vmx_vmenter_helper(void)
out:
HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
+
+ __vmwrite(GUEST_RIP, regs->rip);
+ __vmwrite(GUEST_RSP, regs->rsp);
+ __vmwrite(GUEST_RFLAGS, regs->rflags);
}
/*
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -439,6 +439,21 @@ static inline void hvm_set_info_guest(st
int hvm_debug_op(struct vcpu *v, int32_t op);
+static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
+{
+#ifndef NDEBUG
+ regs->error_code = 0xbeef;
+ regs->entry_vector = 0xbeef;
+ regs->saved_upcall_mask = 0xbf;
+ regs->cs = 0xbeef;
+ regs->ss = 0xbeef;
+ regs->ds = 0xbeef;
+ regs->es = 0xbeef;
+ regs->fs = 0xbeef;
+ regs->gs = 0xbeef;
+#endif
+}
+
int hvm_hap_nested_page_fault(paddr_t gpa,
bool_t gla_valid, unsigned long gla,
bool_t access_r,
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
