| Message ID | 1430393772-27208-7-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Paolo Bonzini <pbonzini@redhat.com> writes: > The hflags field will contain information about system management mode > and will be useful for the emulator. Pass the entire field rather than > just the guest-mode information. With respect to maintaining maximum isolation between vcpu internals and the emulator, why not just "bool smm_mode" ? > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/include/asm/kvm_emulate.h | 5 ++++- > arch/x86/kvm/emulate.c | 6 +++--- > arch/x86/kvm/x86.c | 4 +++- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h > index 57a9d94fe160..7410879a41f7 100644 > --- a/arch/x86/include/asm/kvm_emulate.h > +++ b/arch/x86/include/asm/kvm_emulate.h > @@ -262,6 +262,9 @@ enum x86emul_mode { > X86EMUL_MODE_PROT64, /* 64-bit (long) mode. */ > }; > > +/* These match some of the HF_* flags defined in kvm_host.h */ > +#define X86EMUL_GUEST_MASK (1 << 5) /* VCPU is in guest-mode */ > + > struct x86_emulate_ctxt { > const struct x86_emulate_ops *ops; > > @@ -273,8 +276,8 @@ struct x86_emulate_ctxt { > > /* interruptibility state, as a result of execution of STI or MOV SS */ > int interruptibility; > + int emul_flags; > > - bool guest_mode; /* guest running a nested guest */ > bool perm_ok; /* do not check permissions if true */ > bool ud; /* inject an #UD if host doesn't support insn */ > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 630bcb0d7a04..cdb612b50910 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -4871,7 +4871,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) > fetch_possible_mmx_operand(ctxt, &ctxt->dst); > } > > - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { > + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && ctxt->intercept) { > rc = emulator_check_intercept(ctxt, ctxt->intercept, > X86_ICPT_PRE_EXCEPT); > if (rc != X86EMUL_CONTINUE) > @@ -4900,7 +4900,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) > goto done; > } > > - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { > + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && (ctxt->d & Intercept)) { > rc = emulator_check_intercept(ctxt, ctxt->intercept, > X86_ICPT_POST_EXCEPT); > if (rc != X86EMUL_CONTINUE) > @@ -4953,7 +4953,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) > > special_insn: > > - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { > + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && (ctxt->d & Intercept)) { > rc = emulator_check_intercept(ctxt, ctxt->intercept, > X86_ICPT_POST_MEMACCESS); > if (rc != X86EMUL_CONTINUE) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 856598afa6b4..6009e6a0e406 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5132,7 +5132,8 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu) > (cs_l && is_long_mode(vcpu)) ? X86EMUL_MODE_PROT64 : > cs_db ? X86EMUL_MODE_PROT32 : > X86EMUL_MODE_PROT16; > - ctxt->guest_mode = is_guest_mode(vcpu); > + BUILD_BUG_ON(HF_GUEST_MASK != X86EMUL_GUEST_MASK); > + ctxt->emul_flags = vcpu->arch.hflags; > > init_decode_cache(ctxt); > vcpu->arch.emulate_regs_need_sync_from_vcpu = false; > @@ -5500,6 +5501,7 @@ restart: > unsigned long rflags = kvm_x86_ops->get_rflags(vcpu); > toggle_interruptibility(vcpu, ctxt->interruptibility); > vcpu->arch.emulate_regs_need_sync_to_vcpu = false; > + vcpu->arch.hflags = ctxt->emul_flags; > kvm_rip_write(vcpu, ctxt->eip); > if (r == EMULATE_DONE) > kvm_vcpu_check_singlestep(vcpu, rflags, &r); -- 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 05/05/2015 17:47, Bandan Das wrote: > > The hflags field will contain information about system management mode > > and will be useful for the emulator. Pass the entire field rather than > > just the guest-mode information. > > With respect to maintaining maximum isolation between vcpu internals and > the emulator, Isolation is maintained. hflags are simply parts of the processor state that are not visible to the guest, you can choose to include them as separate flags or in a single one. Bundling them in a single flag makes it a bit faster to pass around many of them. While we do not need all of them in the emulator, that's in some cases a limitation of the emulator. For example, if we wanted to emulate CLGI/STGI we would need either HF_GIF_MASK or another . Likewise, HF_NMI_MASK could replace emulator_set_nmi_mask (especially in v2 of the series, which will add kvm_set_hflags). However, if you prefer, I can change it to "bool smm_mode" + a new smm_exit emulator callback. Paolo > why not just "bool smm_mode" ? > -- 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
Paolo Bonzini <pbonzini@redhat.com> writes: > On 05/05/2015 17:47, Bandan Das wrote: >> > The hflags field will contain information about system management mode >> > and will be useful for the emulator. Pass the entire field rather than >> > just the guest-mode information. >> >> With respect to maintaining maximum isolation between vcpu internals and >> the emulator, > > Isolation is maintained. hflags are simply parts of the processor state > that are not visible to the guest, you can choose to include them as > separate flags or in a single one. Bundling them in a single flag makes > it a bit faster to pass around many of them. > > While we do not need all of them in the emulator, that's in some cases a > limitation of the emulator. For example, if we wanted to emulate > CLGI/STGI we would need either HF_GIF_MASK or another . Likewise, > HF_NMI_MASK could replace emulator_set_nmi_mask (especially in v2 of the > series, which will add kvm_set_hflags). > > However, if you prefer, I can change it to "bool smm_mode" + a new > smm_exit emulator callback. No, this makes sense. It's better to just pass hflags then to pass bits individually. > Paolo > >> why not just "bool smm_mode" ? >> -- 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 57a9d94fe160..7410879a41f7 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -262,6 +262,9 @@ enum x86emul_mode { X86EMUL_MODE_PROT64, /* 64-bit (long) mode. */ }; +/* These match some of the HF_* flags defined in kvm_host.h */ +#define X86EMUL_GUEST_MASK (1 << 5) /* VCPU is in guest-mode */ + struct x86_emulate_ctxt { const struct x86_emulate_ops *ops; @@ -273,8 +276,8 @@ struct x86_emulate_ctxt { /* interruptibility state, as a result of execution of STI or MOV SS */ int interruptibility; + int emul_flags; - bool guest_mode; /* guest running a nested guest */ bool perm_ok; /* do not check permissions if true */ bool ud; /* inject an #UD if host doesn't support insn */ diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 630bcb0d7a04..cdb612b50910 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4871,7 +4871,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) fetch_possible_mmx_operand(ctxt, &ctxt->dst); } - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && ctxt->intercept) { rc = emulator_check_intercept(ctxt, ctxt->intercept, X86_ICPT_PRE_EXCEPT); if (rc != X86EMUL_CONTINUE) @@ -4900,7 +4900,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) goto done; } - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && (ctxt->d & Intercept)) { rc = emulator_check_intercept(ctxt, ctxt->intercept, X86_ICPT_POST_EXCEPT); if (rc != X86EMUL_CONTINUE) @@ -4953,7 +4953,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt) special_insn: - if (unlikely(ctxt->guest_mode) && (ctxt->d & Intercept)) { + if (unlikely(ctxt->emul_flags & X86EMUL_GUEST_MASK) && (ctxt->d & Intercept)) { rc = emulator_check_intercept(ctxt, ctxt->intercept, X86_ICPT_POST_MEMACCESS); if (rc != X86EMUL_CONTINUE) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 856598afa6b4..6009e6a0e406 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5132,7 +5132,8 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu) (cs_l && is_long_mode(vcpu)) ? X86EMUL_MODE_PROT64 : cs_db ? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16; - ctxt->guest_mode = is_guest_mode(vcpu); + BUILD_BUG_ON(HF_GUEST_MASK != X86EMUL_GUEST_MASK); + ctxt->emul_flags = vcpu->arch.hflags; init_decode_cache(ctxt); vcpu->arch.emulate_regs_need_sync_from_vcpu = false; @@ -5500,6 +5501,7 @@ restart: unsigned long rflags = kvm_x86_ops->get_rflags(vcpu); toggle_interruptibility(vcpu, ctxt->interruptibility); vcpu->arch.emulate_regs_need_sync_to_vcpu = false; + vcpu->arch.hflags = ctxt->emul_flags; kvm_rip_write(vcpu, ctxt->eip); if (r == EMULATE_DONE) kvm_vcpu_check_singlestep(vcpu, rflags, &r);
The hflags field will contain information about system management mode and will be useful for the emulator. Pass the entire field rather than just the guest-mode information. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/include/asm/kvm_emulate.h | 5 ++++- arch/x86/kvm/emulate.c | 6 +++--- arch/x86/kvm/x86.c | 4 +++- 3 files changed, 10 insertions(+), 5 deletions(-)