[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 09/18] PVH xen: Support privileged op emulation for PVH
On Wed, 03 Jul 2013 11:21:20 +0100 "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >>> On 03.07.13 at 03:38, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > >>> wrote: > > On Fri, 28 Jun 2013 10:20:47 +0100 > > "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > > > >> >>> On 28.06.13 at 01:43, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > >> >>> wrote: ............. > >> The problem is that you think that now and forever this macro > >> will only be used from the MMIO emulation path (or some such, in > >> any case - just from one very specific path). This is an assumption > >> you may make while in an early development phase, but not in > >> patches that you intend to be committed: Someone adding another > >> use of the macro is _very_ unlikely to go and check what contraints > >> apply to that use. The macro has to work in the general case. > > > > Hmm.. Ok, I still fail to see the difference, caching upfront > > always is such a low overhead. > > Even if it really is (which I doubt), you still would make PVH > different from both PV and HVM, which both don't populate the > selector fields of the frame (PV obviously has ->cs and ->ss > populated [by the CPU], but HVM avoids even that). And what's wrong with PVH being little different? If anything, that makes the code easier than to make it like PVH/HVM IMHO. HVM may not cache selectors, but caches other things like CR3 upfront: vmx_vmexit_handler(): v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3] = __vmread(GUEST_CR3); > We'll have to see - at the first glance I don't follow... Here's what I am talking about: diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 16e25e9..ab1953f 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -1241,10 +1241,10 @@ static void save_segments(struct vcpu *v) struct cpu_user_regs *regs = &v->arch.user_regs; unsigned int dirty_segment_mask = 0; - regs->ds = read_segment_register(v, regs, ds); - regs->es = read_segment_register(v, regs, es); - regs->fs = read_segment_register(v, regs, fs); - regs->gs = read_segment_register(v, regs, gs); + regs->ds = read_segment_register(v, regs, x86_seg_ds); + regs->es = read_segment_register(v, regs, x86_seg_es); + regs->fs = read_segment_register(v, regs, x86_seg_fs); + regs->gs = read_segment_register(v, regs, x86_seg_gs); if ( regs->ds ) dirty_segment_mask |= DIRTY_DS; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index e0d84af..5318126 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -676,6 +676,45 @@ static void vmx_ctxt_switch_to(struct vcpu *v) .fields = { .type = 0xb, .s = 0, .dpl = 0, .p = 1, .avl = 0, \ .l = 0, .db = 0, .g = 0, .pad = 0 } }).bytes) +u16 vmx_get_selector(struct vcpu *v, enum x86_segment seg) +{ + u16 sel = 0; + + vmx_vmcs_enter(v); + switch ( seg ) + { + case x86_seg_cs: + sel = __vmread(GUEST_CS_SELECTOR); + break; + + case x86_seg_ss: + sel = __vmread(GUEST_SS_SELECTOR); + break; + + case x86_seg_es: + sel = __vmread(GUEST_ES_SELECTOR); + break; + + case x86_seg_ds: + sel = __vmread(GUEST_DS_SELECTOR); + break; + + case x86_seg_fs: + sel = __vmread(GUEST_FS_SELECTOR); + break; + + case x86_seg_gs: + sel = __vmread(GUEST_GS_SELECTOR); + break; + + default: + BUG(); + } + vmx_vmcs_exit(v); + + return sel; +} + void vmx_get_segment_register(struct vcpu *v, enum x86_segment seg, struct segment_register *reg) { diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index b638a6e..6b6989a 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -112,6 +112,52 @@ boolean_param("ler", opt_ler); #define stack_words_per_line 4 #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp) +/* + * We need vcpu because during context switch, going from pure PV to PVH, + * in save_segments(), current has been updated to next, and no longer pointing + * to the pure PV. + */ +u16 read_segment_register(struct vcpu *vcpu, + struct cpu_user_regs *regs, enum x86_segment seg) +{ + u16 sel = 0; + + if ( is_pvh_vcpu(vcpu) && guest_mode(regs) ) + sel = pvh_get_selector(vcpu, seg); + else + { + switch ( seg ) + { + case x86_seg_cs: + asm volatile ( "movw %%cs,%0" : "=r" (sel) ); + break; + + case x86_seg_ss: + asm volatile ( "movw %%ss,%0" : "=r" (sel) ); + break; + + case x86_seg_es: + asm volatile ( "movw %%es,%0" : "=r" (sel) ); + break; + + case x86_seg_ds: + asm volatile ( "movw %%ds,%0" : "=r" (sel) ); + break; + + case x86_seg_fs: + asm volatile ( "movw %%fs,%0" : "=r" (sel) ); + break; + + case x86_seg_gs: + asm volatile ( "movw %%gs,%0" : "=r" (sel) ); + break; + + default: + BUG(); + } + } + return sel; +} static void show_guest_stack(struct vcpu *v, struct cpu_user_regs *regs) { int i; @@ -1940,7 +1986,7 @@ int emulate_privileged_op(struct cpu_user_regs *regs) goto fail; /* emulating only opcodes not allowing SS to be default */ - data_sel = read_segment_register(v, regs, ds); + data_sel = read_segment_register(v, regs, x86_seg_ds); which_sel = x86_seg_ds; /* Legacy prefixes. */ @@ -1960,20 +2006,20 @@ int emulate_privileged_op(struct cpu_user_regs *regs) which_sel = x86_seg_cs; continue; case 0x3e: /* DS override */ - data_sel = read_segment_register(v, regs, ds); + data_sel = read_segment_register(v, regs, x86_seg_ds); which_sel = x86_seg_ds; continue; case 0x26: /* ES override */ - data_sel = read_segment_register(v, regs, es); + data_sel = read_segment_register(v, regs, x86_seg_es); which_sel = x86_seg_es; continue; case 0x64: /* FS override */ - data_sel = read_segment_register(v, regs, fs); + data_sel = read_segment_register(v, regs, x86_seg_fs); which_sel = x86_seg_fs; lm_ovr = lm_seg_fs; continue; case 0x65: /* GS override */ - data_sel = read_segment_register(v, regs, gs); + data_sel = read_segment_register(v, regs, x86_seg_gs); which_sel = x86_seg_gs; lm_ovr = lm_seg_gs; continue; @@ -2022,7 +2068,7 @@ int emulate_privileged_op(struct cpu_user_regs *regs) if ( !(opcode & 2) ) { - data_sel = read_segment_register(v, regs, es); + data_sel = read_segment_register(v, regs, x86_seg_es); which_sel = x86_seg_es; lm_ovr = lm_seg_none; } @@ -2769,22 +2815,22 @@ static void emulate_gate_op(struct cpu_user_regs *regs) ASSERT(opnd_sel); continue; case 0x3e: /* DS override */ - opnd_sel = read_segment_register(v, regs, ds); + opnd_sel = read_segment_register(v, regs, x86_seg_ds); if ( !opnd_sel ) opnd_sel = dpl; continue; case 0x26: /* ES override */ - opnd_sel = read_segment_register(v, regs, es); + opnd_sel = read_segment_register(v, regs, x86_seg_es); if ( !opnd_sel ) opnd_sel = dpl; continue; case 0x64: /* FS override */ - opnd_sel = read_segment_register(v, regs, fs); + opnd_sel = read_segment_register(v, regs, x86_seg_fs); if ( !opnd_sel ) opnd_sel = dpl; continue; case 0x65: /* GS override */ - opnd_sel = read_segment_register(v, regs, gs); + opnd_sel = read_segment_register(v, regs, x86_seg_gs); if ( !opnd_sel ) opnd_sel = dpl; continue; @@ -2837,7 +2883,8 @@ static void emulate_gate_op(struct cpu_user_regs *regs) switch ( modrm & 7 ) { default: - opnd_sel = read_segment_register(v, regs, ds); + opnd_sel = read_segment_register(v, regs, + x86_seg_ds); break; case 4: case 5: opnd_sel = regs->ss; @@ -2865,7 +2912,8 @@ static void emulate_gate_op(struct cpu_user_regs *regs) break; } if ( !opnd_sel ) - opnd_sel = read_segment_register(v, regs, ds); + opnd_sel = read_segment_register(v, regs, + x86_seg_ds); switch ( modrm & 7 ) { case 0: case 2: case 4: diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c index 0df1e1c..fbf6506 100644 --- a/xen/arch/x86/x86_64/traps.c +++ b/xen/arch/x86/x86_64/traps.c @@ -122,10 +122,10 @@ void show_registers(struct cpu_user_regs *regs) fault_crs[0] = read_cr0(); fault_crs[3] = read_cr3(); fault_crs[4] = read_cr4(); - fault_regs.ds = read_segment_register(v, regs, ds); - fault_regs.es = read_segment_register(v, regs, es); - fault_regs.fs = read_segment_register(v, regs, fs); - fault_regs.gs = read_segment_register(v, regs, gs); + fault_regs.ds = read_segment_register(v, regs, x86_seg_ds); + fault_regs.es = read_segment_register(v, regs, x86_seg_es); + fault_regs.fs = read_segment_register(v, regs, x86_seg_fs); + fault_regs.gs = read_segment_register(v, regs, x86_seg_gs); } print_xen_info(); @@ -240,10 +240,10 @@ void do_double_fault(struct cpu_user_regs *regs) crs[2] = read_cr2(); crs[3] = read_cr3(); crs[4] = read_cr4(); - regs->ds = read_segment_register(current, regs, ds); - regs->es = read_segment_register(current, regs, es); - regs->fs = read_segment_register(current, regs, fs); - regs->gs = read_segment_register(current, regs, gs); + regs->ds = read_segment_register(current, regs, x86_seg_ds); + regs->es = read_segment_register(current, regs, x86_seg_es); + regs->fs = read_segment_register(current, regs, x86_seg_fs); + regs->gs = read_segment_register(current, regs, x86_seg_gs); printk("CPU: %d\n", cpu); _show_registers(regs, crs, CTXT_hypervisor, NULL); diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 7e21ee1..c63988d 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -194,6 +194,7 @@ struct hvm_function_table { bool_t access_w, bool_t access_x); int (*pvh_set_vcpu_info)(struct vcpu *v, struct vcpu_guest_context *ctxtp); + u16 (*vmx_read_selector)(struct vcpu *v, enum x86_segment seg); }; extern struct hvm_function_table hvm_funcs; @@ -308,6 +309,11 @@ static inline void hvm_flush_guest_tlbs(void) void hvm_hypercall_page_initialise(struct domain *d, void *hypercall_page); +static inline u16 pvh_get_selector(struct vcpu *v, enum x86_segment seg) +{ + return hvm_funcs.vmx_read_selector(v, seg); +} + static inline void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg, struct segment_register *reg) diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h index 035944f..d5d90ca 100644 --- a/xen/include/asm-x86/system.h +++ b/xen/include/asm-x86/system.h @@ -4,22 +4,6 @@ #include <xen/lib.h> #include <xen/bitops.h> -/* - * We need vcpu because during context switch, going from pure PV to PVH, - * in save_segments(), current has been updated to next, and no longer pointing - * to the pure PV. Note: for PVH, we update regs->selectors on each vmexit. - */ -#define read_segment_register(vcpu, regs, name) \ -({ u16 __sel; \ - struct cpu_user_regs *_regs = (regs); \ - \ - if ( is_pvh_vcpu(vcpu) && guest_mode(regs) ) \ - __sel = _regs->name; \ - else \ - asm volatile ( "movw %%" #name ",%0" : "=r" (__sel) ); \ - __sel; \ -}) - #define wbinvd() \ asm volatile ( "wbinvd" : : : "memory" ) diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h index 202e3be..c4a2e2e 100644 --- a/xen/include/asm-x86/traps.h +++ b/xen/include/asm-x86/traps.h @@ -50,4 +50,6 @@ extern int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr); int emulate_privileged_op(struct cpu_user_regs *regs); +u16 read_segment_register(struct vcpu *vcpu, + struct cpu_user_regs *regs, enum x86_segment seg); #endif /* ASM_TRAP_H */ _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |