* [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode
@ 2010-11-29 15:38 Joerg Roedel
2010-11-29 15:38 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Joerg Roedel @ 2010-11-29 15:38 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel
Hi Avi, Hi Marcelo,
this patch-set introduces a generic notion of guest-mode for VCPUs in
KVM. This is already useful as seen in patch 3/3. Nested-VMX also has a
guest-mode, so it will make sense for this code too.
Regards,
Joerg
As usual:
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/kvm_cache_regs.h | 15 +++++++++++++
arch/x86/kvm/svm.c | 44 ++++++++++++++++++--------------------
arch/x86/kvm/x86.c | 14 ++++++++---
Joerg Roedel (3):
KVM: X86: Introduce generic guest-mode representation
KVM: SVM: Make Use of the generic guest-mode functions
KVM: X86: Don't report L2 emulation failures to user-space
4 files changed, 47 insertions(+), 27 deletions(-)
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation 2010-11-29 15:38 [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Joerg Roedel @ 2010-11-29 15:38 ` Joerg Roedel 2010-11-29 16:10 ` Avi Kivity 2010-11-29 15:38 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 15:38 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch introduces a generic representation of guest-mode fpr a vcpu. This currently only exists in the SVM code. Having this representation generic will help making the non-svm code aware of nesting when this is necessary. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/kvm_cache_regs.h | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 54e42c8..d2a66be 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -784,6 +784,7 @@ enum { #define HF_VINTR_MASK (1 << 2) #define HF_NMI_MASK (1 << 3) #define HF_IRET_MASK (1 << 4) +#define HF_GUEST_MASK (1 << 5) /* VCPU is in guest-mode */ /* * Hardware virtualization extension instructions may fault if a diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h index 975bb45..7e7c52d 100644 --- a/arch/x86/kvm/kvm_cache_regs.h +++ b/arch/x86/kvm/kvm_cache_regs.h @@ -84,4 +84,19 @@ static inline u64 kvm_read_edx_eax(struct kvm_vcpu *vcpu) | ((u64)(kvm_register_read(vcpu, VCPU_REGS_RDX) & -1u) << 32); } +static inline void kvm_vcpu_enter_gm(struct kvm_vcpu *vcpu) +{ + vcpu->arch.hflags |= HF_GUEST_MASK; +} + +static inline void kvm_vcpu_leave_gm(struct kvm_vcpu *vcpu) +{ + vcpu->arch.hflags &= ~HF_GUEST_MASK; +} + +static inline bool kvm_vcpu_is_gm(struct kvm_vcpu *vcpu) +{ + return !!(vcpu->arch.hflags & HF_GUEST_MASK); +} + #endif -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation 2010-11-29 15:38 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel @ 2010-11-29 16:10 ` Avi Kivity 0 siblings, 0 replies; 16+ messages in thread From: Avi Kivity @ 2010-11-29 16:10 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, kvm, linux-kernel On 11/29/2010 05:38 PM, Joerg Roedel wrote: > This patch introduces a generic representation of guest-mode > fpr a vcpu. This currently only exists in the SVM code. > Having this representation generic will help making the > non-svm code aware of nesting when this is necessary. > > > > +static inline void kvm_vcpu_enter_gm(struct kvm_vcpu *vcpu) > +{ > + vcpu->arch.hflags |= HF_GUEST_MASK; > +} I don't like the name much - the "meat" is just two letters. Please spell it out. I guess we could do is_guest_mode() like we do is_long_mode(). > + > +static inline bool kvm_vcpu_is_gm(struct kvm_vcpu *vcpu) > +{ > + return !!(vcpu->arch.hflags& HF_GUEST_MASK); > +} > + !! unneeded with bool. Note we need to live migrate this bit, but that's outside the scope of this patchset. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions 2010-11-29 15:38 [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Joerg Roedel 2010-11-29 15:38 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel @ 2010-11-29 15:38 ` Joerg Roedel 2010-11-29 15:38 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity 3 siblings, 0 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 15:38 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch replaces the is_nested logic in the SVM module with the generic notion of guest-mode. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/kvm/svm.c | 44 +++++++++++++++++++++----------------------- 1 files changed, 21 insertions(+), 23 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 2fd2f4d..3376fca 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -192,11 +192,6 @@ static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu) return container_of(vcpu, struct vcpu_svm, vcpu); } -static inline bool is_nested(struct vcpu_svm *svm) -{ - return svm->nested.vmcb; -} - static inline void enable_gif(struct vcpu_svm *svm) { svm->vcpu.arch.hflags |= HF_GIF_MASK; @@ -727,7 +722,7 @@ static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) struct vcpu_svm *svm = to_svm(vcpu); u64 g_tsc_offset = 0; - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(vcpu)) { g_tsc_offset = svm->vmcb->control.tsc_offset - svm->nested.hsave->control.tsc_offset; svm->nested.hsave->control.tsc_offset = offset; @@ -741,7 +736,7 @@ static void svm_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment) struct vcpu_svm *svm = to_svm(vcpu); svm->vmcb->control.tsc_offset += adjustment; - if (is_nested(svm)) + if (kvm_vcpu_is_gm(vcpu)) svm->nested.hsave->control.tsc_offset += adjustment; } @@ -1209,7 +1204,7 @@ static void update_cr0_intercept(struct vcpu_svm *svm) if (gcr0 == *hcr0 && svm->vcpu.fpu_active) { vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK; vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK; - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(&svm->vcpu)) { struct vmcb *hsave = svm->nested.hsave; hsave->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK; @@ -1220,7 +1215,7 @@ static void update_cr0_intercept(struct vcpu_svm *svm) } else { svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK; svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK; - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(&svm->vcpu)) { struct vmcb *hsave = svm->nested.hsave; hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK; @@ -1233,7 +1228,7 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(vcpu)) { /* * We are here because we run in nested mode, the host kvm * intercepts cr0 writes but the l1 hypervisor does not. @@ -1471,7 +1466,7 @@ static void svm_fpu_activate(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); u32 excp; - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(vcpu)) { u32 h_excp, n_excp; h_excp = svm->nested.hsave->control.intercept_exceptions; @@ -1700,7 +1695,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr, { int vmexit; - if (!is_nested(svm)) + if (!kvm_vcpu_is_gm(&svm->vcpu)) return 0; svm->vmcb->control.exit_code = SVM_EXIT_EXCP_BASE + nr; @@ -1718,7 +1713,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr, /* This function returns true if it is save to enable the irq window */ static inline bool nested_svm_intr(struct vcpu_svm *svm) { - if (!is_nested(svm)) + if (!kvm_vcpu_is_gm(&svm->vcpu)) return true; if (!(svm->vcpu.arch.hflags & HF_VINTR_MASK)) @@ -1757,7 +1752,7 @@ static inline bool nested_svm_intr(struct vcpu_svm *svm) /* This function returns true if it is save to enable the nmi window */ static inline bool nested_svm_nmi(struct vcpu_svm *svm) { - if (!is_nested(svm)) + if (!kvm_vcpu_is_gm(&svm->vcpu)) return true; if (!(svm->nested.intercept & (1ULL << INTERCEPT_NMI))) @@ -1994,7 +1989,8 @@ static int nested_svm_vmexit(struct vcpu_svm *svm) if (!nested_vmcb) return 1; - /* Exit nested SVM mode */ + /* Exit Guest-Mode */ + kvm_vcpu_leave_gm(&svm->vcpu); svm->nested.vmcb = 0; /* Give the current vmcb to the guest */ @@ -2302,7 +2298,9 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) nested_svm_unmap(page); - /* nested_vmcb is our indicator if nested SVM is activated */ + /* Enter Guest-Mode */ + kvm_vcpu_enter_gm(&svm->vcpu); + svm->nested.vmcb = vmcb_gpa; enable_gif(svm); @@ -2588,7 +2586,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data) case MSR_IA32_TSC: { u64 tsc_offset; - if (is_nested(svm)) + if (kvm_vcpu_is_gm(vcpu)) tsc_offset = svm->nested.hsave->control.tsc_offset; else tsc_offset = svm->vmcb->control.tsc_offset; @@ -3002,7 +3000,7 @@ static int handle_exit(struct kvm_vcpu *vcpu) return 1; } - if (is_nested(svm)) { + if (kvm_vcpu_is_gm(vcpu)) { int vmexit; trace_kvm_nested_vmexit(svm->vmcb->save.rip, exit_code, @@ -3109,7 +3107,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (kvm_vcpu_is_gm(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; if (irr == -1) @@ -3163,7 +3161,7 @@ static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) ret = !!(vmcb->save.rflags & X86_EFLAGS_IF); - if (is_nested(svm)) + if (kvm_vcpu_is_gm(vcpu)) return ret && !(svm->vcpu.arch.hflags & HF_VINTR_MASK); return ret; @@ -3220,7 +3218,7 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (kvm_vcpu_is_gm(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR8_MASK)) { @@ -3234,7 +3232,7 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); u64 cr8; - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (kvm_vcpu_is_gm(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; cr8 = kvm_get_cr8(vcpu); @@ -3614,7 +3612,7 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); svm->vmcb->control.intercept_exceptions |= 1 << NM_VECTOR; - if (is_nested(svm)) + if (kvm_vcpu_is_gm(vcpu)) svm->nested.hsave->control.intercept_exceptions |= 1 << NM_VECTOR; update_cr0_intercept(svm); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space 2010-11-29 15:38 [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Joerg Roedel 2010-11-29 15:38 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel 2010-11-29 15:38 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel @ 2010-11-29 15:38 ` Joerg Roedel 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity 3 siblings, 0 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 15:38 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch prevents that emulation failures which result from emulating an instruction for an L2-Guest results in being reported to userspace. Without this patch a malicious L2-Guest would be able to kill the L1 by triggering a race-condition between an vmexit and the instruction emulator. With this patch the L2 will most likely only kill itself in this situation. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/kvm/x86.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 410d2d1..78329dd 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4320,13 +4320,19 @@ EXPORT_SYMBOL_GPL(kvm_inject_realmode_interrupt); static int handle_emulation_failure(struct kvm_vcpu *vcpu) { + int r = EMULATE_DONE; + ++vcpu->stat.insn_emulation_fail; trace_kvm_emulate_insn_failed(vcpu); - vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; - vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; - vcpu->run->internal.ndata = 0; + if (!kvm_vcpu_is_gm(vcpu)) { + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; + vcpu->run->internal.ndata = 0; + r = EMULATE_FAIL; + } kvm_queue_exception(vcpu, UD_VECTOR); - return EMULATE_FAIL; + + return r; } static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode 2010-11-29 15:38 [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Joerg Roedel ` (2 preceding siblings ...) 2010-11-29 15:38 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel @ 2010-11-29 16:10 ` Avi Kivity 2010-11-29 16:25 ` Roedel, Joerg ` (4 more replies) 3 siblings, 5 replies; 16+ messages in thread From: Avi Kivity @ 2010-11-29 16:10 UTC (permalink / raw) To: Joerg Roedel; +Cc: Marcelo Tosatti, kvm, linux-kernel On 11/29/2010 05:38 PM, Joerg Roedel wrote: > Hi Avi, Hi Marcelo, > > this patch-set introduces a generic notion of guest-mode for VCPUs in > KVM. This is already useful as seen in patch 3/3. Nested-VMX also has a > guest-mode, so it will make sense for this code too. > Looks good, apart from the trivial comments on patch 1. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity @ 2010-11-29 16:25 ` Roedel, Joerg 2010-11-29 16:51 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 Joerg Roedel ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Roedel, Joerg @ 2010-11-29 16:25 UTC (permalink / raw) To: Avi Kivity Cc: Marcelo Tosatti, kvm@vger.kernel.org, linux-kernel@vger.kernel.org On Mon, Nov 29, 2010 at 11:10:59AM -0500, Avi Kivity wrote: > On 11/29/2010 05:38 PM, Joerg Roedel wrote: > > Hi Avi, Hi Marcelo, > > > > this patch-set introduces a generic notion of guest-mode for VCPUs in > > KVM. This is already useful as seen in patch 3/3. Nested-VMX also has a > > guest-mode, so it will make sense for this code too. > > > > Looks good, apart from the trivial comments on patch 1. Okay, I do a re-spin and fix this. Thanks for your comments. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity 2010-11-29 16:25 ` Roedel, Joerg @ 2010-11-29 16:51 ` Joerg Roedel 2010-12-01 8:01 ` Nadav Har'El 2010-12-02 2:42 ` Marcelo Tosatti 2010-11-29 16:51 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel ` (2 subsequent siblings) 4 siblings, 2 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 16:51 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel Hi Avi, Hi Marcelo, here is the re-spin I promised. The change to V1 are essentially the renames: kvm_vcpu_enter_gm -> enter_guest_mode kvm_vcpu_leave_gm -> leave_guest_mode kvm_vcpu_is_gm -> is_guest_mode No other changes are in this patch-set compared to V1. Regards, Joerg arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/kvm_cache_regs.h | 15 +++++++++++++ arch/x86/kvm/svm.c | 44 ++++++++++++++++++-------------------- arch/x86/kvm/x86.c | 14 ++++++++--- 4 files changed, 47 insertions(+), 27 deletions(-) Joerg Roedel (3): KVM: X86: Introduce generic guest-mode representation KVM: SVM: Make Use of the generic guest-mode functions KVM: X86: Don't report L2 emulation failures to user-space ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-11-29 16:51 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 Joerg Roedel @ 2010-12-01 8:01 ` Nadav Har'El 2010-12-01 10:03 ` Roedel, Joerg 2010-12-02 2:42 ` Marcelo Tosatti 1 sibling, 1 reply; 16+ messages in thread From: Nadav Har'El @ 2010-12-01 8:01 UTC (permalink / raw) To: Joerg Roedel; +Cc: Avi Kivity, Marcelo Tosatti, kvm, linux-kernel On Mon, Nov 29, 2010, Joerg Roedel wrote about "[PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2": > Hi Avi, Hi Marcelo, > > here is the re-spin I promised. The change to V1 are essentially the > renames: > > kvm_vcpu_enter_gm -> enter_guest_mode > kvm_vcpu_leave_gm -> leave_guest_mode > kvm_vcpu_is_gm -> is_guest_mode I like this concept, and will be happy to change the nested VMX code to use it as well. One small thing: After the name change, it might not be obvious on first sight that these functions refer to the state of the vcpu, not the state of the actual CPU (which, if you think about it, is never in guest mode while KVM code is running ;-)). I think that a short comment before the definition of these functions might be useful - perhaps saying that they pertain to a hypervisor running in the vcpu (i.e., nested virtualization). Nadav. -- Nadav Har'El | Wednesday, Dec 1 2010, 24 Kislev 5771 nyh@math.technion.ac.il |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |How do you get holy water? Boil the hell http://nadav.harel.org.il |out of it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-12-01 8:01 ` Nadav Har'El @ 2010-12-01 10:03 ` Roedel, Joerg 2010-12-01 11:38 ` Nadav Har'El 0 siblings, 1 reply; 16+ messages in thread From: Roedel, Joerg @ 2010-12-01 10:03 UTC (permalink / raw) To: Nadav Har'El Cc: Avi Kivity, Marcelo Tosatti, kvm@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Dec 01, 2010 at 03:01:49AM -0500, Nadav Har'El wrote: > On Mon, Nov 29, 2010, Joerg Roedel wrote about "[PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2": > > Hi Avi, Hi Marcelo, > > > > here is the re-spin I promised. The change to V1 are essentially the > > renames: > > > > kvm_vcpu_enter_gm -> enter_guest_mode > > kvm_vcpu_leave_gm -> leave_guest_mode > > kvm_vcpu_is_gm -> is_guest_mode > > I like this concept, and will be happy to change the nested VMX code to use > it as well. > > One small thing: After the name change, it might not be obvious on first > sight that these functions refer to the state of the vcpu, not the state > of the actual CPU (which, if you think about it, is never in guest mode while > KVM code is running ;-)). I think that a short comment before the definition > of these functions might be useful - perhaps saying that they pertain to a > hypervisor running in the vcpu (i.e., nested virtualization). Yes, right. Thats a good thing. I sent a follow-on patch adding the comments. Btw, another idea which came up recently was to concentrate the actuall vmexit emulation at a single point. Every code place which does the exit directly today will be changed to only set a request-bit and the real exit is then done later. Your code might already do this, I havn't checked. In fact the idea is from the neste-VMX patchset for Xen :) This would fit very well in the generic code because it already has request-bit infrastructure. What do you think, can nested VMX also make use of that too? Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-12-01 10:03 ` Roedel, Joerg @ 2010-12-01 11:38 ` Nadav Har'El 2010-12-01 13:20 ` Roedel, Joerg 0 siblings, 1 reply; 16+ messages in thread From: Nadav Har'El @ 2010-12-01 11:38 UTC (permalink / raw) To: Roedel, Joerg Cc: Avi Kivity, Marcelo Tosatti, kvm@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Dec 01, 2010, Roedel, Joerg wrote about "Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2": > Btw, another idea which came up recently was to concentrate the actuall > vmexit emulation at a single point. Every code place which does the exit > directly today will be changed to only set a request-bit and the real > exit is then done later. Your code might already do this, I havn't In my current patches, there is single function nested_vmx_vmexit() which emulates the exit (exits from L2 to L1), but it is called in several places, the most significant are of course in vmx_handle_exit (when L1 asked an exit on the given event), and vmx_interrupt_allowed (when we inject an interrupt and L1 asked to exit on interrupts). This area of my code definitely needs some reorganization, as Gleb pointed out in his review. > This would fit very well in the generic code because it already has > request-bit infrastructure. What do you think, can nested VMX also make > use of that too? Can you please say a few words why you'd want to move this nested-exit request bit to x86.c? Do you want to move some of the exit logic to x86.c - e.g., for the injection logic? Nadav. -- Nadav Har'El | Wednesday, Dec 1 2010, 24 Kislev 5771 nyh@math.technion.ac.il |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |Fame: when your name is in everything but http://nadav.harel.org.il |the phone book. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-12-01 11:38 ` Nadav Har'El @ 2010-12-01 13:20 ` Roedel, Joerg 0 siblings, 0 replies; 16+ messages in thread From: Roedel, Joerg @ 2010-12-01 13:20 UTC (permalink / raw) To: Nadav Har'El Cc: Avi Kivity, Marcelo Tosatti, kvm@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Dec 01, 2010 at 06:38:30AM -0500, Nadav Har'El wrote: > Can you please say a few words why you'd want to move this nested-exit > request bit to x86.c? I don't want to move the actual exit-code itself into generic code. This code is different between svm and vmx. I think we could implement a call-back in kvm_x86_ops which is called when a vmexit is requested. The benefit is that we have a single and well-defined place where we emulate a vmexit. SVM already as a similar mechanism internally because nested_svm_vmexit may sleep and can't be called from certain places. Another reason is that emulating a vmexit at a wrong plase may have side-effects (for example when called from within the instruction emulator). With a generic request-bit I can remove the SVM internal implementation and nested vmx could use it too. I am certain you will need something similar in nested-vmx too. > Do you want to move some of the exit logic to x86.c - e.g., for the > injection logic? Thats another and probably more complex topic. I need a better understanding of (nested-)vmx before we discuss how this can be done. But a vmexit-callback may be helpful there as well. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 2010-11-29 16:51 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 Joerg Roedel 2010-12-01 8:01 ` Nadav Har'El @ 2010-12-02 2:42 ` Marcelo Tosatti 1 sibling, 0 replies; 16+ messages in thread From: Marcelo Tosatti @ 2010-12-02 2:42 UTC (permalink / raw) To: Joerg Roedel; +Cc: Avi Kivity, kvm, linux-kernel On Mon, Nov 29, 2010 at 05:51:46PM +0100, Joerg Roedel wrote: > Hi Avi, Hi Marcelo, > > here is the re-spin I promised. The change to V1 are essentially the > renames: > > kvm_vcpu_enter_gm -> enter_guest_mode > kvm_vcpu_leave_gm -> leave_guest_mode > kvm_vcpu_is_gm -> is_guest_mode > > No other changes are in this patch-set compared to V1. > > Regards, > Joerg > > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/kvm_cache_regs.h | 15 +++++++++++++ > arch/x86/kvm/svm.c | 44 ++++++++++++++++++-------------------- > arch/x86/kvm/x86.c | 14 ++++++++--- > 4 files changed, 47 insertions(+), 27 deletions(-) > > Joerg Roedel (3): > KVM: X86: Introduce generic guest-mode representation > KVM: SVM: Make Use of the generic guest-mode functions > KVM: X86: Don't report L2 emulation failures to user-space Applied, thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity 2010-11-29 16:25 ` Roedel, Joerg 2010-11-29 16:51 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 Joerg Roedel @ 2010-11-29 16:51 ` Joerg Roedel 2010-11-29 16:51 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel 2010-11-29 16:51 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel 4 siblings, 0 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 16:51 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch introduces a generic representation of guest-mode fpr a vcpu. This currently only exists in the SVM code. Having this representation generic will help making the non-svm code aware of nesting when this is necessary. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/kvm_cache_regs.h | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 54e42c8..d2a66be 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -784,6 +784,7 @@ enum { #define HF_VINTR_MASK (1 << 2) #define HF_NMI_MASK (1 << 3) #define HF_IRET_MASK (1 << 4) +#define HF_GUEST_MASK (1 << 5) /* VCPU is in guest-mode */ /* * Hardware virtualization extension instructions may fault if a diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h index 975bb45..95ac3af 100644 --- a/arch/x86/kvm/kvm_cache_regs.h +++ b/arch/x86/kvm/kvm_cache_regs.h @@ -84,4 +84,19 @@ static inline u64 kvm_read_edx_eax(struct kvm_vcpu *vcpu) | ((u64)(kvm_register_read(vcpu, VCPU_REGS_RDX) & -1u) << 32); } +static inline void enter_guest_mode(struct kvm_vcpu *vcpu) +{ + vcpu->arch.hflags |= HF_GUEST_MASK; +} + +static inline void leave_guest_mode(struct kvm_vcpu *vcpu) +{ + vcpu->arch.hflags &= ~HF_GUEST_MASK; +} + +static inline bool is_guest_mode(struct kvm_vcpu *vcpu) +{ + return vcpu->arch.hflags & HF_GUEST_MASK; +} + #endif -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity ` (2 preceding siblings ...) 2010-11-29 16:51 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel @ 2010-11-29 16:51 ` Joerg Roedel 2010-11-29 16:51 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel 4 siblings, 0 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 16:51 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch replaces the is_nested logic in the SVM module with the generic notion of guest-mode. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/kvm/svm.c | 44 +++++++++++++++++++++----------------------- 1 files changed, 21 insertions(+), 23 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 2fd2f4d..bff391e 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -192,11 +192,6 @@ static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu) return container_of(vcpu, struct vcpu_svm, vcpu); } -static inline bool is_nested(struct vcpu_svm *svm) -{ - return svm->nested.vmcb; -} - static inline void enable_gif(struct vcpu_svm *svm) { svm->vcpu.arch.hflags |= HF_GIF_MASK; @@ -727,7 +722,7 @@ static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) struct vcpu_svm *svm = to_svm(vcpu); u64 g_tsc_offset = 0; - if (is_nested(svm)) { + if (is_guest_mode(vcpu)) { g_tsc_offset = svm->vmcb->control.tsc_offset - svm->nested.hsave->control.tsc_offset; svm->nested.hsave->control.tsc_offset = offset; @@ -741,7 +736,7 @@ static void svm_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment) struct vcpu_svm *svm = to_svm(vcpu); svm->vmcb->control.tsc_offset += adjustment; - if (is_nested(svm)) + if (is_guest_mode(vcpu)) svm->nested.hsave->control.tsc_offset += adjustment; } @@ -1209,7 +1204,7 @@ static void update_cr0_intercept(struct vcpu_svm *svm) if (gcr0 == *hcr0 && svm->vcpu.fpu_active) { vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK; vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK; - if (is_nested(svm)) { + if (is_guest_mode(&svm->vcpu)) { struct vmcb *hsave = svm->nested.hsave; hsave->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK; @@ -1220,7 +1215,7 @@ static void update_cr0_intercept(struct vcpu_svm *svm) } else { svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK; svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK; - if (is_nested(svm)) { + if (is_guest_mode(&svm->vcpu)) { struct vmcb *hsave = svm->nested.hsave; hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK; @@ -1233,7 +1228,7 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm)) { + if (is_guest_mode(vcpu)) { /* * We are here because we run in nested mode, the host kvm * intercepts cr0 writes but the l1 hypervisor does not. @@ -1471,7 +1466,7 @@ static void svm_fpu_activate(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); u32 excp; - if (is_nested(svm)) { + if (is_guest_mode(vcpu)) { u32 h_excp, n_excp; h_excp = svm->nested.hsave->control.intercept_exceptions; @@ -1700,7 +1695,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr, { int vmexit; - if (!is_nested(svm)) + if (!is_guest_mode(&svm->vcpu)) return 0; svm->vmcb->control.exit_code = SVM_EXIT_EXCP_BASE + nr; @@ -1718,7 +1713,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr, /* This function returns true if it is save to enable the irq window */ static inline bool nested_svm_intr(struct vcpu_svm *svm) { - if (!is_nested(svm)) + if (!is_guest_mode(&svm->vcpu)) return true; if (!(svm->vcpu.arch.hflags & HF_VINTR_MASK)) @@ -1757,7 +1752,7 @@ static inline bool nested_svm_intr(struct vcpu_svm *svm) /* This function returns true if it is save to enable the nmi window */ static inline bool nested_svm_nmi(struct vcpu_svm *svm) { - if (!is_nested(svm)) + if (!is_guest_mode(&svm->vcpu)) return true; if (!(svm->nested.intercept & (1ULL << INTERCEPT_NMI))) @@ -1994,7 +1989,8 @@ static int nested_svm_vmexit(struct vcpu_svm *svm) if (!nested_vmcb) return 1; - /* Exit nested SVM mode */ + /* Exit Guest-Mode */ + leave_guest_mode(&svm->vcpu); svm->nested.vmcb = 0; /* Give the current vmcb to the guest */ @@ -2302,7 +2298,9 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm) nested_svm_unmap(page); - /* nested_vmcb is our indicator if nested SVM is activated */ + /* Enter Guest-Mode */ + enter_guest_mode(&svm->vcpu); + svm->nested.vmcb = vmcb_gpa; enable_gif(svm); @@ -2588,7 +2586,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data) case MSR_IA32_TSC: { u64 tsc_offset; - if (is_nested(svm)) + if (is_guest_mode(vcpu)) tsc_offset = svm->nested.hsave->control.tsc_offset; else tsc_offset = svm->vmcb->control.tsc_offset; @@ -3002,7 +3000,7 @@ static int handle_exit(struct kvm_vcpu *vcpu) return 1; } - if (is_nested(svm)) { + if (is_guest_mode(vcpu)) { int vmexit; trace_kvm_nested_vmexit(svm->vmcb->save.rip, exit_code, @@ -3109,7 +3107,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; if (irr == -1) @@ -3163,7 +3161,7 @@ static int svm_interrupt_allowed(struct kvm_vcpu *vcpu) ret = !!(vmcb->save.rflags & X86_EFLAGS_IF); - if (is_nested(svm)) + if (is_guest_mode(vcpu)) return ret && !(svm->vcpu.arch.hflags & HF_VINTR_MASK); return ret; @@ -3220,7 +3218,7 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR8_MASK)) { @@ -3234,7 +3232,7 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); u64 cr8; - if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK)) + if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) return; cr8 = kvm_get_cr8(vcpu); @@ -3614,7 +3612,7 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu) struct vcpu_svm *svm = to_svm(vcpu); svm->vmcb->control.intercept_exceptions |= 1 << NM_VECTOR; - if (is_nested(svm)) + if (is_guest_mode(vcpu)) svm->nested.hsave->control.intercept_exceptions |= 1 << NM_VECTOR; update_cr0_intercept(svm); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity ` (3 preceding siblings ...) 2010-11-29 16:51 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel @ 2010-11-29 16:51 ` Joerg Roedel 4 siblings, 0 replies; 16+ messages in thread From: Joerg Roedel @ 2010-11-29 16:51 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, linux-kernel, Joerg Roedel This patch prevents that emulation failures which result from emulating an instruction for an L2-Guest results in being reported to userspace. Without this patch a malicious L2-Guest would be able to kill the L1 by triggering a race-condition between an vmexit and the instruction emulator. With this patch the L2 will most likely only kill itself in this situation. Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/kvm/x86.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 410d2d1..4337a8b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4320,13 +4320,19 @@ EXPORT_SYMBOL_GPL(kvm_inject_realmode_interrupt); static int handle_emulation_failure(struct kvm_vcpu *vcpu) { + int r = EMULATE_DONE; + ++vcpu->stat.insn_emulation_fail; trace_kvm_emulate_insn_failed(vcpu); - vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; - vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; - vcpu->run->internal.ndata = 0; + if (!is_guest_mode(vcpu)) { + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; + vcpu->run->internal.ndata = 0; + r = EMULATE_FAIL; + } kvm_queue_exception(vcpu, UD_VECTOR); - return EMULATE_FAIL; + + return r; } static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-12-02 2:50 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-29 15:38 [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Joerg Roedel 2010-11-29 15:38 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel 2010-11-29 16:10 ` Avi Kivity 2010-11-29 15:38 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel 2010-11-29 15:38 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel 2010-11-29 16:10 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode Avi Kivity 2010-11-29 16:25 ` Roedel, Joerg 2010-11-29 16:51 ` [PATCH 0/3] KVM: Introduce VCPU-wide notion of guest-mode V2 Joerg Roedel 2010-12-01 8:01 ` Nadav Har'El 2010-12-01 10:03 ` Roedel, Joerg 2010-12-01 11:38 ` Nadav Har'El 2010-12-01 13:20 ` Roedel, Joerg 2010-12-02 2:42 ` Marcelo Tosatti 2010-11-29 16:51 ` [PATCH 1/3] KVM: X86: Introduce generic guest-mode representation Joerg Roedel 2010-11-29 16:51 ` [PATCH 2/3] KVM: SVM: Make Use of the generic guest-mode functions Joerg Roedel 2010-11-29 16:51 ` [PATCH 3/3] KVM: X86: Don't report L2 emulation failures to user-space Joerg Roedel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).