* [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host}
@ 2019-06-07 17:08 Paolo Bonzini
2019-06-07 17:37 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2019-06-07 17:08 UTC (permalink / raw)
To: linux-kernel, kvm
vmx->loaded_cpu_state can only be NULL or equal to vmx->loaded_vmcs,
so change it to a bool. Because the direction of the bool is
now the opposite of vmx->guest_msrs_dirty, change the direction of
vmx->guest_msrs_dirty so that they match.
Finally, do not imply that MSRs have to be reloaded when
vmx->guest_sregs_loaded is false; instead, set vmx->guest_msrs_loaded
to false explicitly in vmx_prepare_switch_to_host.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Still untested.
arch/x86/kvm/vmx/vmx.c | 26 +++++++++++++-------------
arch/x86/kvm/vmx/vmx.h | 18 ++++++++++++------
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 091610684d28..f1a58e35fa5f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1057,20 +1057,18 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
* when guest state is loaded. This happens when guest transitions
* to/from long-mode by setting MSR_EFER.LMA.
*/
- if (!vmx->loaded_cpu_state || vmx->guest_msrs_dirty) {
- vmx->guest_msrs_dirty = false;
+ if (!vmx->guest_msrs_loaded) {
+ vmx->guest_msrs_loaded = true;
for (i = 0; i < vmx->save_nmsrs; ++i)
kvm_set_shared_msr(vmx->guest_msrs[i].index,
vmx->guest_msrs[i].data,
vmx->guest_msrs[i].mask);
}
-
- if (vmx->loaded_cpu_state)
+ if (vmx->guest_sregs_loaded)
return;
- vmx->loaded_cpu_state = vmx->loaded_vmcs;
- host_state = &vmx->loaded_cpu_state->host_state;
+ host_state = &vmx->loaded_vmcs->host_state;
/*
* Set host fs and gs selectors. Unfortunately, 22.2.3 does not
@@ -1126,20 +1124,20 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
vmcs_writel(HOST_GS_BASE, gs_base);
host_state->gs_base = gs_base;
}
+
+ vmx->guest_sregs_loaded = true;
}
static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
{
struct vmcs_host_state *host_state;
- if (!vmx->loaded_cpu_state)
+ if (!vmx->guest_sregs_loaded)
return;
- WARN_ON_ONCE(vmx->loaded_cpu_state != vmx->loaded_vmcs);
- host_state = &vmx->loaded_cpu_state->host_state;
+ host_state = &vmx->loaded_vmcs->host_state;
++vmx->vcpu.stat.host_state_reload;
- vmx->loaded_cpu_state = NULL;
#ifdef CONFIG_X86_64
rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
@@ -1165,13 +1163,15 @@ static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
#endif
load_fixmap_gdt(raw_smp_processor_id());
+ vmx->guest_sregs_loaded = false;
+ vmx->guest_msrs_loaded = false;
}
#ifdef CONFIG_X86_64
static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
{
preempt_disable();
- if (vmx->loaded_cpu_state)
+ if (vmx->guest_sregs_loaded)
rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
preempt_enable();
return vmx->msr_guest_kernel_gs_base;
@@ -1180,7 +1180,7 @@ static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
static void vmx_write_guest_kernel_gs_base(struct vcpu_vmx *vmx, u64 data)
{
preempt_disable();
- if (vmx->loaded_cpu_state)
+ if (vmx->guest_sregs_loaded)
wrmsrl(MSR_KERNEL_GS_BASE, data);
preempt_enable();
vmx->msr_guest_kernel_gs_base = data;
@@ -1583,7 +1583,7 @@ static void setup_msrs(struct vcpu_vmx *vmx)
move_msr_up(vmx, index, save_nmsrs++);
vmx->save_nmsrs = save_nmsrs;
- vmx->guest_msrs_dirty = true;
+ vmx->guest_msrs_loaded = false;
if (cpu_has_vmx_msr_bitmap())
vmx_update_msr_bitmap(&vmx->vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index ed65999b07a8..fc369473f9df 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -187,13 +187,23 @@ struct vcpu_vmx {
struct kvm_vcpu vcpu;
u8 fail;
u8 msr_bitmap_mode;
+
+ /*
+ * If true, host state has been stored in vmx->loaded_vmcs for
+ * the CPU registers that only need to be switched when transitioning
+ * to/from the kernel, and the registers have been loaded with guest
+ * values. If false, host state is loaded in the CPU registers
+ * and vmx->loaded_vmcs->host_state is invalid.
+ */
+ bool guest_sregs_loaded;
+
u32 exit_intr_info;
u32 idt_vectoring_info;
ulong rflags;
struct shared_msr_entry *guest_msrs;
int nmsrs;
int save_nmsrs;
- bool guest_msrs_dirty;
+ bool guest_msrs_loaded;
#ifdef CONFIG_X86_64
u64 msr_host_kernel_gs_base;
u64 msr_guest_kernel_gs_base;
@@ -208,14 +218,10 @@ struct vcpu_vmx {
/*
* loaded_vmcs points to the VMCS currently used in this vcpu. For a
* non-nested (L1) guest, it always points to vmcs01. For a nested
- * guest (L2), it points to a different VMCS. loaded_cpu_state points
- * to the VMCS whose state is loaded into the CPU registers that only
- * need to be switched when transitioning to/from the kernel; a NULL
- * value indicates that host state is loaded.
+ * guest (L2), it points to a different VMCS.
*/
struct loaded_vmcs vmcs01;
struct loaded_vmcs *loaded_vmcs;
- struct loaded_vmcs *loaded_cpu_state;
struct msr_autoload {
struct vmx_msrs guest;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host}
2019-06-07 17:08 [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host} Paolo Bonzini
@ 2019-06-07 17:37 ` Sean Christopherson
2019-06-07 17:47 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2019-06-07 17:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Fri, Jun 07, 2019 at 07:08:21PM +0200, Paolo Bonzini wrote:
> vmx->loaded_cpu_state can only be NULL or equal to vmx->loaded_vmcs,
> so change it to a bool. Because the direction of the bool is
> now the opposite of vmx->guest_msrs_dirty, change the direction of
> vmx->guest_msrs_dirty so that they match.
>
> Finally, do not imply that MSRs have to be reloaded when
> vmx->guest_sregs_loaded is false; instead, set vmx->guest_msrs_loaded
> to false explicitly in vmx_prepare_switch_to_host.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
...
> @@ -1165,13 +1163,15 @@ static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
> wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
> #endif
> load_fixmap_gdt(raw_smp_processor_id());
> + vmx->guest_sregs_loaded = false;
> + vmx->guest_msrs_loaded = false;
> }
>
> #ifdef CONFIG_X86_64
> static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
> {
> preempt_disable();
> - if (vmx->loaded_cpu_state)
> + if (vmx->guest_sregs_loaded)
> rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
This is the hiccup with naming it sregs_loaded. The split bools is also
kinda wonky since the 32->64 case is a one-off scenario. I think a
cleaner solution would be to remove guest_msrs_dirty and refresh the MSRs
directly from setup_msrs(). Then loaded_cpu_state -> loaded_guest_state
can be a straight conversion from loaded_vmcs -> bool. I'll send patches.
> preempt_enable();
> return vmx->msr_guest_kernel_gs_base;
> @@ -1180,7 +1180,7 @@ static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
> static void vmx_write_guest_kernel_gs_base(struct vcpu_vmx *vmx, u64 data)
> {
> preempt_disable();
> - if (vmx->loaded_cpu_state)
> + if (vmx->guest_sregs_loaded)
Same issue here, one would expect this to check guest_msrs_loaded.
> wrmsrl(MSR_KERNEL_GS_BASE, data);
> preempt_enable();
> vmx->msr_guest_kernel_gs_base = data;
> @@ -1583,7 +1583,7 @@ static void setup_msrs(struct vcpu_vmx *vmx)
> move_msr_up(vmx, index, save_nmsrs++);
>
> vmx->save_nmsrs = save_nmsrs;
> - vmx->guest_msrs_dirty = true;
> + vmx->guest_msrs_loaded = false;
>
> if (cpu_has_vmx_msr_bitmap())
> vmx_update_msr_bitmap(&vmx->vcpu);
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index ed65999b07a8..fc369473f9df 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -187,13 +187,23 @@ struct vcpu_vmx {
> struct kvm_vcpu vcpu;
> u8 fail;
> u8 msr_bitmap_mode;
> +
> + /*
> + * If true, host state has been stored in vmx->loaded_vmcs for
> + * the CPU registers that only need to be switched when transitioning
> + * to/from the kernel, and the registers have been loaded with guest
> + * values. If false, host state is loaded in the CPU registers
> + * and vmx->loaded_vmcs->host_state is invalid.
> + */
> + bool guest_sregs_loaded;
> +
> u32 exit_intr_info;
> u32 idt_vectoring_info;
> ulong rflags;
> struct shared_msr_entry *guest_msrs;
> int nmsrs;
> int save_nmsrs;
> - bool guest_msrs_dirty;
> + bool guest_msrs_loaded;
> #ifdef CONFIG_X86_64
> u64 msr_host_kernel_gs_base;
> u64 msr_guest_kernel_gs_base;
> @@ -208,14 +218,10 @@ struct vcpu_vmx {
> /*
> * loaded_vmcs points to the VMCS currently used in this vcpu. For a
> * non-nested (L1) guest, it always points to vmcs01. For a nested
> - * guest (L2), it points to a different VMCS. loaded_cpu_state points
> - * to the VMCS whose state is loaded into the CPU registers that only
> - * need to be switched when transitioning to/from the kernel; a NULL
> - * value indicates that host state is loaded.
> + * guest (L2), it points to a different VMCS.
> */
> struct loaded_vmcs vmcs01;
> struct loaded_vmcs *loaded_vmcs;
> - struct loaded_vmcs *loaded_cpu_state;
>
> struct msr_autoload {
> struct vmx_msrs guest;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host}
2019-06-07 17:37 ` Sean Christopherson
@ 2019-06-07 17:47 ` Sean Christopherson
2019-06-07 18:13 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2019-06-07 17:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm
On Fri, Jun 07, 2019 at 10:37:10AM -0700, Sean Christopherson wrote:
> On Fri, Jun 07, 2019 at 07:08:21PM +0200, Paolo Bonzini wrote:
> > vmx->loaded_cpu_state can only be NULL or equal to vmx->loaded_vmcs,
> > so change it to a bool. Because the direction of the bool is
> > now the opposite of vmx->guest_msrs_dirty, change the direction of
> > vmx->guest_msrs_dirty so that they match.
> >
> > Finally, do not imply that MSRs have to be reloaded when
> > vmx->guest_sregs_loaded is false; instead, set vmx->guest_msrs_loaded
> > to false explicitly in vmx_prepare_switch_to_host.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> ...
>
> > @@ -1165,13 +1163,15 @@ static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
> > wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
> > #endif
> > load_fixmap_gdt(raw_smp_processor_id());
> > + vmx->guest_sregs_loaded = false;
> > + vmx->guest_msrs_loaded = false;
> > }
> >
> > #ifdef CONFIG_X86_64
> > static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
> > {
> > preempt_disable();
> > - if (vmx->loaded_cpu_state)
> > + if (vmx->guest_sregs_loaded)
> > rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
>
> This is the hiccup with naming it sregs_loaded. The split bools is also
> kinda wonky since the 32->64 case is a one-off scenario. I think a
> cleaner solution would be to remove guest_msrs_dirty and refresh the MSRs
> directly from setup_msrs(). Then loaded_cpu_state -> loaded_guest_state
> can be a straight conversion from loaded_vmcs -> bool. I'll send patches.
Actually, would it be easier on your end if I do a v2 of the series that
would introduce vmx_sync_vmcs_host_state(), and splice these patch into it?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host}
2019-06-07 17:47 ` Sean Christopherson
@ 2019-06-07 18:13 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2019-06-07 18:13 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-kernel, kvm
On 07/06/19 19:47, Sean Christopherson wrote:
>> This is the hiccup with naming it sregs_loaded. The split bools is also
>> kinda wonky since the 32->64 case is a one-off scenario. I think a
>> cleaner solution would be to remove guest_msrs_dirty and refresh the MSRs
>> directly from setup_msrs(). Then loaded_cpu_state -> loaded_guest_state
>> can be a straight conversion from loaded_vmcs -> bool. I'll send patches.
> Actually, would it be easier on your end if I do a v2 of the series that
> would introduce vmx_sync_vmcs_host_state(), and splice these patch into it?
For now I'll just rename it to guest_state_loaded, let's do further
cleanups on top. My plan is to post my version of everything after
testing so that you can do a "git range-diff" between my patches and
yours. (For exposure I'll probably push that to kvm/queue, but I don't
intend to merge them to kvm/next without your review).
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-07 18:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 17:08 [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host} Paolo Bonzini
2019-06-07 17:37 ` Sean Christopherson
2019-06-07 17:47 ` Sean Christopherson
2019-06-07 18:13 ` Paolo Bonzini
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).