| Message ID | 1430393772-27208-9-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-04-30 13:36+0200, Paolo Bonzini: > This patch adds the interface between x86.c and the emulator: the > SMBASE register, a new emulator flag, the RSM instruction. It also > adds a new request bit that will be used by the KVM_SMI ioctl. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > @@ -2505,7 +2505,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) > vmx->nested.nested_vmx_misc_low &= VMX_MISC_SAVE_EFER_LMA; > vmx->nested.nested_vmx_misc_low |= > VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE | > - VMX_MISC_ACTIVITY_HLT; > + VMX_MISC_ACTIVITY_HLT | VMX_MISC_IA32_SMBASE_MSR; > vmx->nested.nested_vmx_misc_high = 0; > } > > bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer) > @@ -2217,6 +2218,11 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > case MSR_IA32_MISC_ENABLE: > vcpu->arch.ia32_misc_enable_msr = data; > break; > + case MSR_IA32_SMBASE: > + if (!msr_info->host_initiated) > + return 1; > + vcpu->arch.smbase = data; > + break; > case MSR_KVM_WALL_CLOCK_NEW: > case MSR_KVM_WALL_CLOCK: > vcpu->kvm->arch.wall_clock = data; > @@ -2612,6 +2618,11 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > case MSR_IA32_MISC_ENABLE: > msr_info->data = vcpu->arch.ia32_misc_enable_msr; > break; > + case MSR_IA32_SMBASE: > + if (!msr_info->host_initiated && !is_smm(vcpu)) > + return 1; > + msr_info->data = vcpu->arch.smbase; > + break; (I'm not sure if this is supported if IA32_VMX_BASIC[49] = 0. 34.15.6.4 Saving Guest State The SMM-transfer monitor (STM) can also discover the current value of the SMBASE register by using the RDMSR but it's not possible to get into STM without having a support for it noted in IA32_VMX_BASIC[49] and more magic we also don't emulate to actually enable it.) > @@ -7208,6 +7240,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) > vcpu->arch.regs_avail = ~0; > vcpu->arch.regs_dirty = ~0; > > + vcpu->arch.smbase = 0x30000; It's not reset on INIT, only on RESET. (34.11 SMBASE RELOCATION) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/05/2015 19:51, Radim Kr?má? wrote: > (I'm not sure if this is supported if IA32_VMX_BASIC[49] = 0. > 34.15.6.4 Saving Guest State > The SMM-transfer monitor (STM) can also discover the current value of > the SMBASE register by using the RDMSR > > but it's not possible to get into STM without having a support for it > noted in IA32_VMX_BASIC[49] and more magic we also don't emulate to > actually enable it.) I can certainly make the SMBASE MSR host-accessible only. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Radim Kr?má? <rkrcmar@redhat.com> writes: ... >> + break; > > (I'm not sure if this is supported if IA32_VMX_BASIC[49] = 0. > 34.15.6.4 Saving Guest State > The SMM-transfer monitor (STM) can also discover the current value of > the SMBASE register by using the RDMSR > > but it's not possible to get into STM without having a support for it > noted in IA32_VMX_BASIC[49] and more magic we also don't emulate to > actually enable it.) Where does it mention IA32_VMX_BASIC[49] ? I only see "IA32_VMX_MISC[15] should be 1" in 34.15.6.4. Anyway, I think we should do what the spec says.. >> @@ -7208,6 +7240,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) >> vcpu->arch.regs_avail = ~0; >> vcpu->arch.regs_dirty = ~0; >> >> + vcpu->arch.smbase = 0x30000; > > It's not reset on INIT, only on RESET. (34.11 SMBASE RELOCATION) I remember mentioning it elsewhere - IMO kvm_vcpu_reset() and kvm_vcpu_init() should really be two different interfaces. I don't mean code duplication - one can just call the other but different names will be of some help when it comes to the million places where the spec mentions INIT and RESET have different behavior. Bandan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-05-05 14:38-0400, Bandan Das: > Radim Kr?má? <rkrcmar@redhat.com> writes: > ... > >> + break; > > > > (I'm not sure if this is supported if IA32_VMX_BASIC[49] = 0. > > 34.15.6.4 Saving Guest State > > The SMM-transfer monitor (STM) can also discover the current value of > > the SMBASE register by using the RDMSR > > > > but it's not possible to get into STM without having a support for it > > noted in IA32_VMX_BASIC[49] and more magic we also don't emulate to > > actually enable it.) > > Where does it mention IA32_VMX_BASIC[49] ? I only see "IA32_VMX_MISC[15] should be 1" > in 34.15.6.4. Anyway, I think we should do what the spec says.. The relevant part is "SMM-transfer monitor (STM) can" -- you can't be STM without IA32_VMX_MISC[15] and a bunch of other stuff. Testing on real hardware would be the best way to tell ... (Till we know, I'm okay with anything.) > >> @@ -7208,6 +7240,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) > >> vcpu->arch.regs_avail = ~0; > >> vcpu->arch.regs_dirty = ~0; > >> > >> + vcpu->arch.smbase = 0x30000; > > > > It's not reset on INIT, only on RESET. (34.11 SMBASE RELOCATION) > I remember mentioning it elsewhere - IMO kvm_vcpu_reset() and kvm_vcpu_init() > should really be two different interfaces. I don't mean code duplication - one > can just call the other but different names will be of some help when it comes > to the million places where the spec mentions INIT and RESET have different > behavior. Agreed. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index 7410879a41f7..efe9b9565914 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -193,6 +193,7 @@ struct x86_emulate_ops { int (*cpl)(struct x86_emulate_ctxt *ctxt); int (*get_dr)(struct x86_emulate_ctxt *ctxt, int dr, ulong *dest); int (*set_dr)(struct x86_emulate_ctxt *ctxt, int dr, ulong value); + void (*set_smbase)(struct x86_emulate_ctxt *ctxt, u64 smbase); int (*set_msr)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 data); int (*get_msr)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 *pdata); int (*check_pmc)(struct x86_emulate_ctxt *ctxt, u32 pmc); @@ -264,6 +265,7 @@ enum x86emul_mode { /* These match some of the HF_* flags defined in kvm_host.h */ #define X86EMUL_GUEST_MASK (1 << 5) /* VCPU is in guest-mode */ +#define X86EMUL_SMM_MASK (1 << 6) struct x86_emulate_ctxt { const struct x86_emulate_ops *ops; diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 1171e346bb33..f0db43e80d09 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -367,6 +367,7 @@ struct kvm_vcpu_arch { int32_t apic_arb_prio; int mp_state; u64 ia32_misc_enable_msr; + u64 smbase; bool tpr_access_reporting; u64 ia32_xss; diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h index da772edd19ab..e8f998114df0 100644 --- a/arch/x86/include/asm/vmx.h +++ b/arch/x86/include/asm/vmx.h @@ -108,6 +108,7 @@ #define VMX_MISC_PREEMPTION_TIMER_RATE_MASK 0x0000001f #define VMX_MISC_SAVE_EFER_LMA 0x00000020 #define VMX_MISC_ACTIVITY_HLT 0x00000040 +#define VMX_MISC_IA32_SMBASE_MSR 0x00008000 /* VMCS Encodings */ enum vmcs_field { diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index cdb612b50910..f6b641207416 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -2262,6 +2262,14 @@ static int em_lseg(struct x86_emulate_ctxt *ctxt) return rc; } +static int em_rsm(struct x86_emulate_ctxt *ctxt) +{ + if ((ctxt->emul_flags & X86EMUL_SMM_MASK) == 0) + return emulate_ud(ctxt); + + return X86EMUL_UNHANDLEABLE; +} + static void setup_syscalls_segments(struct x86_emulate_ctxt *ctxt, struct desc_struct *cs, struct desc_struct *ss) @@ -4173,7 +4181,7 @@ static const struct opcode twobyte_table[256] = { F(DstMem | SrcReg | Src2CL | ModRM, em_shld), N, N, /* 0xA8 - 0xAF */ I(Stack | Src2GS, em_push_sreg), I(Stack | Src2GS, em_pop_sreg), - DI(ImplicitOps, rsm), + II(No64 | EmulateOnUD | ImplicitOps, em_rsm, rsm), F(DstMem | SrcReg | ModRM | BitOp | Lock | PageTable, em_bts), F(DstMem | SrcReg | Src2ImmByte | ModRM, em_shrd), F(DstMem | SrcReg | Src2CL | ModRM, em_shrd), diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 629af0f1c5c4..0b2e45e3dc4c 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -799,7 +799,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode, break; case APIC_DM_SMI: - apic_debug("Ignoring guest SMI\n"); + result = 1; + kvm_make_request(KVM_REQ_SMI, vcpu); + kvm_vcpu_kick(vcpu); break; case APIC_DM_NMI: diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index f2cc241c71c4..0cbb49b8d555 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -3392,6 +3392,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { [SVM_EXIT_MWAIT] = mwait_interception, [SVM_EXIT_XSETBV] = xsetbv_interception, [SVM_EXIT_NPF] = pf_interception, + [SVM_EXIT_RSM] = emulate_on_interception, }; static void dump_vmcb(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 4e14dfa83ce0..6d296fcb68f4 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2505,7 +2505,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) vmx->nested.nested_vmx_misc_low &= VMX_MISC_SAVE_EFER_LMA; vmx->nested.nested_vmx_misc_low |= VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE | - VMX_MISC_ACTIVITY_HLT; + VMX_MISC_ACTIVITY_HLT | VMX_MISC_IA32_SMBASE_MSR; vmx->nested.nested_vmx_misc_high = 0; } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c088b057aad5..4cd7a2a18e93 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -953,6 +953,7 @@ static const u32 emulated_msrs[] = { MSR_IA32_MISC_ENABLE, MSR_IA32_MCG_STATUS, MSR_IA32_MCG_CTL, + MSR_IA32_SMBASE, }; bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer) @@ -2217,6 +2218,11 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_MISC_ENABLE: vcpu->arch.ia32_misc_enable_msr = data; break; + case MSR_IA32_SMBASE: + if (!msr_info->host_initiated) + return 1; + vcpu->arch.smbase = data; + break; case MSR_KVM_WALL_CLOCK_NEW: case MSR_KVM_WALL_CLOCK: vcpu->kvm->arch.wall_clock = data; @@ -2612,6 +2618,11 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_MISC_ENABLE: msr_info->data = vcpu->arch.ia32_misc_enable_msr; break; + case MSR_IA32_SMBASE: + if (!msr_info->host_initiated && !is_smm(vcpu)) + return 1; + msr_info->data = vcpu->arch.smbase; + break; case MSR_IA32_PERF_STATUS: /* TSC increment by tick */ msr_info->data = 1000ULL; @@ -3034,6 +3045,8 @@ static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu) static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu) { + kvm_make_request(KVM_REQ_SMI, vcpu); + return 0; } @@ -5005,6 +5018,13 @@ static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, return kvm_set_msr(emul_to_vcpu(ctxt), &msr); } +static void emulator_set_smbase(struct x86_emulate_ctxt *ctxt, u64 smbase) +{ + struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt); + + vcpu->arch.smbase = smbase; +} + static int emulator_check_pmc(struct x86_emulate_ctxt *ctxt, u32 pmc) { @@ -5090,6 +5110,7 @@ static const struct x86_emulate_ops emulate_ops = { .cpl = emulator_get_cpl, .get_dr = emulator_get_dr, .set_dr = emulator_set_dr, + .set_smbase = emulator_set_smbase, .set_msr = emulator_set_msr, .get_msr = emulator_get_msr, .check_pmc = emulator_check_pmc, @@ -5152,6 +5173,7 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu) cs_db ? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16; BUILD_BUG_ON(HF_GUEST_MASK != X86EMUL_GUEST_MASK); + BUILD_BUG_ON(HF_SMM_MASK != X86EMUL_SMM_MASK); ctxt->emul_flags = vcpu->arch.hflags; init_decode_cache(ctxt); @@ -6210,6 +6232,14 @@ static void process_nmi(struct kvm_vcpu *vcpu) kvm_make_request(KVM_REQ_EVENT, vcpu); } +static void process_smi(struct kvm_vcpu *vcpu) +{ + if (is_smm(vcpu)) + return; + + printk_once(KERN_DEBUG "Ignoring guest SMI\n"); +} + static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) { u64 eoi_exit_bitmap[4]; @@ -6316,6 +6346,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) } if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) record_steal_time(vcpu); + if (kvm_check_request(KVM_REQ_SMI, vcpu)) + process_smi(vcpu); if (kvm_check_request(KVM_REQ_NMI, vcpu)) process_nmi(vcpu); if (kvm_check_request(KVM_REQ_PMU, vcpu)) @@ -7208,6 +7240,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu) vcpu->arch.regs_avail = ~0; vcpu->arch.regs_dirty = ~0; + vcpu->arch.smbase = 0x30000; + kvm_x86_ops->vcpu_reset(vcpu); } diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 1edb63dbdead..e7cfee09970a 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -134,6 +134,7 @@ static inline bool is_error_page(struct page *page) #define KVM_REQ_ENABLE_IBS 23 #define KVM_REQ_DISABLE_IBS 24 #define KVM_REQ_APIC_PAGE_RELOAD 25 +#define KVM_REQ_SMI 26 #define KVM_USERSPACE_IRQ_SOURCE_ID 0 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID 1
This patch adds the interface between x86.c and the emulator: the SMBASE register, a new emulator flag, the RSM instruction. It also adds a new request bit that will be used by the KVM_SMI ioctl. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/include/asm/kvm_emulate.h | 2 ++ arch/x86/include/asm/kvm_host.h | 1 + arch/x86/include/asm/vmx.h | 1 + arch/x86/kvm/emulate.c | 10 +++++++++- arch/x86/kvm/lapic.c | 4 +++- arch/x86/kvm/svm.c | 1 + arch/x86/kvm/vmx.c | 2 +- arch/x86/kvm/x86.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/kvm_host.h | 1 + 9 files changed, 53 insertions(+), 3 deletions(-)