* [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs
@ 2018-03-13 17:48 Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 1/2] x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread Vitaly Kuznetsov
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2018-03-13 17:48 UTC (permalink / raw)
To: kvm, x86
Cc: linux-kernel, Paolo Bonzini, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Lutomirski
Changes since v1:
- Merged patches 1 and 2, skip savesegment() for FS and FS for x86_64
as save_fsgs() already does that [Paolo Bonzini]
- Rename save_current_fsgs() -> save_fsgs_for_kvm() [Andy Lutomirski]
- Trimmed comments [Andy Lutomirski]
- Add Andy's A-b to what is now PATCH2
Some time ago Paolo suggested to take a look at probably unneeded expensive
rdmsrs for FS/GS base MSR in vmx_save_host_state(). This is called on every
vcpu run when we need to handle vmexit in userspace.
vmx_save_host_state() is always called in a very well defined context
(ioctl from userspace) so we may try to get the required values for FS/GS
bases from in-kernel variables and avoid expensive rdmsrs.
My debug shows we're shaving off 240 cpu cycles (E5-2603 v3).
Vitaly Kuznetsov (2):
x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread
x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE
arch/x86/include/asm/processor.h | 10 ++++++++++
arch/x86/kernel/cpu/common.c | 3 ++-
arch/x86/kernel/process_64.c | 14 ++++++++++++++
arch/x86/kvm/vmx.c | 16 ++++++++++++----
4 files changed, 38 insertions(+), 5 deletions(-)
--
2.14.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread
2018-03-13 17:48 [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Vitaly Kuznetsov
@ 2018-03-13 17:48 ` Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 2/2] x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE Vitaly Kuznetsov
2018-03-14 16:12 ` [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2018-03-13 17:48 UTC (permalink / raw)
To: kvm, x86
Cc: linux-kernel, Paolo Bonzini, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Lutomirski
vmx_save_host_state() is only called from kvm_arch_vcpu_ioctl_run() so
the context is pretty well defined. Read MSR_{FS,KERNEL_GS}_BASE from
current->thread after calling save_fsgs() which takes care of
X86_BUG_NULL_SEG case now and will do RD[FG,GS]BASE when FSGSBASE
extensions are exposed to userspace (currently they are not).
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/include/asm/processor.h | 5 +++++
arch/x86/kernel/process_64.c | 14 ++++++++++++++
arch/x86/kvm/vmx.c | 13 ++++++++++---
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index b0ccd4847a58..1d816a3325a1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -410,6 +410,11 @@ DECLARE_INIT_PER_CPU(irq_stack_union);
DECLARE_PER_CPU(char *, irq_stack_ptr);
DECLARE_PER_CPU(unsigned int, irq_count);
extern asmlinkage void ignore_sysret(void);
+
+#if IS_ENABLED(CONFIG_KVM)
+/* Save actual FS/GS selectors and bases to current->thread */
+void save_fsgs_for_kvm(void);
+#endif
#else /* X86_64 */
#ifdef CONFIG_CC_STACKPROTECTOR
/*
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 9eb448c7859d..4b100fe0f508 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -205,6 +205,20 @@ static __always_inline void save_fsgs(struct task_struct *task)
save_base_legacy(task, task->thread.gsindex, GS);
}
+#if IS_ENABLED(CONFIG_KVM)
+/*
+ * While a process is running,current->thread.fsbase and current->thread.gsbase
+ * may not match the corresponding CPU registers (see save_base_legacy()). KVM
+ * wants an efficient way to save and restore FSBASE and GSBASE.
+ * When FSGSBASE extensions are enabled, this will have to use RD{FS,GS}BASE.
+ */
+void save_fsgs_for_kvm(void)
+{
+ save_fsgs(current);
+}
+EXPORT_SYMBOL_GPL(save_fsgs_for_kvm);
+#endif
+
static __always_inline void loadseg(enum which_selector which,
unsigned short sel)
{
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 051dab74e4e9..189a347f3952 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2135,7 +2135,15 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
*/
vmx->host_state.ldt_sel = kvm_read_ldt();
vmx->host_state.gs_ldt_reload_needed = vmx->host_state.ldt_sel;
+
+#ifdef CONFIG_X86_64
+ save_fsgs_for_kvm();
+ vmx->host_state.fs_sel = current->thread.fsindex;
+ vmx->host_state.gs_sel = current->thread.gsindex;
+#else
savesegment(fs, vmx->host_state.fs_sel);
+ savesegment(gs, vmx->host_state.gs_sel);
+#endif
if (!(vmx->host_state.fs_sel & 7)) {
vmcs_write16(HOST_FS_SELECTOR, vmx->host_state.fs_sel);
vmx->host_state.fs_reload_needed = 0;
@@ -2143,7 +2151,6 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
vmcs_write16(HOST_FS_SELECTOR, 0);
vmx->host_state.fs_reload_needed = 1;
}
- savesegment(gs, vmx->host_state.gs_sel);
if (!(vmx->host_state.gs_sel & 7))
vmcs_write16(HOST_GS_SELECTOR, vmx->host_state.gs_sel);
else {
@@ -2157,7 +2164,7 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
#endif
#ifdef CONFIG_X86_64
- vmcs_writel(HOST_FS_BASE, read_msr(MSR_FS_BASE));
+ vmcs_writel(HOST_FS_BASE, current->thread.fsbase);
vmcs_writel(HOST_GS_BASE, read_msr(MSR_GS_BASE));
#else
vmcs_writel(HOST_FS_BASE, segment_base(vmx->host_state.fs_sel));
@@ -2165,7 +2172,7 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
#endif
#ifdef CONFIG_X86_64
- rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
+ vmx->msr_host_kernel_gs_base = current->thread.gsbase;
if (is_long_mode(&vmx->vcpu))
wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
#endif
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE
2018-03-13 17:48 [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 1/2] x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread Vitaly Kuznetsov
@ 2018-03-13 17:48 ` Vitaly Kuznetsov
2018-03-14 16:12 ` [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2018-03-13 17:48 UTC (permalink / raw)
To: kvm, x86
Cc: linux-kernel, Paolo Bonzini, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Lutomirski
vmx_save_host_state() is only called from kvm_arch_vcpu_ioctl_run() so
the context is pretty well defined and as we're past 'swapgs' MSR_GS_BASE
should contain kernel's GS base which we point to irq_stack_union.
Add new kernelmode_gs_base() API, irq_stack_union needs to be exported
as KVM can be build as module.
Acked-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/include/asm/processor.h | 5 +++++
arch/x86/kernel/cpu/common.c | 3 ++-
arch/x86/kvm/vmx.c | 3 ++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 1d816a3325a1..4fa4206029e3 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -407,6 +407,11 @@ union irq_stack_union {
DECLARE_PER_CPU_FIRST(union irq_stack_union, irq_stack_union) __visible;
DECLARE_INIT_PER_CPU(irq_stack_union);
+static inline unsigned long cpu_kernelmode_gs_base(int cpu)
+{
+ return (unsigned long)per_cpu(irq_stack_union.gs_base, cpu);
+}
+
DECLARE_PER_CPU(char *, irq_stack_ptr);
DECLARE_PER_CPU(unsigned int, irq_count);
extern asmlinkage void ignore_sysret(void);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 348cf4821240..4702fbd98f92 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -487,7 +487,7 @@ void load_percpu_segment(int cpu)
loadsegment(fs, __KERNEL_PERCPU);
#else
__loadsegment_simple(gs, 0);
- wrmsrl(MSR_GS_BASE, (unsigned long)per_cpu(irq_stack_union.gs_base, cpu));
+ wrmsrl(MSR_GS_BASE, cpu_kernelmode_gs_base(cpu));
#endif
load_stack_canary_segment();
}
@@ -1398,6 +1398,7 @@ __setup("clearcpuid=", setup_clearcpuid);
#ifdef CONFIG_X86_64
DEFINE_PER_CPU_FIRST(union irq_stack_union,
irq_stack_union) __aligned(PAGE_SIZE) __visible;
+EXPORT_PER_CPU_SYMBOL_GPL(irq_stack_union);
/*
* The following percpu variables are hot. Align current_task to
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 189a347f3952..49953e362e92 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2123,6 +2123,7 @@ static unsigned long segment_base(u16 selector)
static void vmx_save_host_state(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
+ int cpu = raw_smp_processor_id();
int i;
if (vmx->host_state.loaded)
@@ -2165,7 +2166,7 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
#ifdef CONFIG_X86_64
vmcs_writel(HOST_FS_BASE, current->thread.fsbase);
- vmcs_writel(HOST_GS_BASE, read_msr(MSR_GS_BASE));
+ vmcs_writel(HOST_GS_BASE, cpu_kernelmode_gs_base(cpu));
#else
vmcs_writel(HOST_FS_BASE, segment_base(vmx->host_state.fs_sel));
vmcs_writel(HOST_GS_BASE, segment_base(vmx->host_state.gs_sel));
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs
2018-03-13 17:48 [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 1/2] x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 2/2] x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE Vitaly Kuznetsov
@ 2018-03-14 16:12 ` Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2018-03-14 16:12 UTC (permalink / raw)
To: Vitaly Kuznetsov, kvm, x86
Cc: linux-kernel, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Lutomirski
On 13/03/2018 18:48, Vitaly Kuznetsov wrote:
> Changes since v1:
> - Merged patches 1 and 2, skip savesegment() for FS and FS for x86_64
> as save_fsgs() already does that [Paolo Bonzini]
> - Rename save_current_fsgs() -> save_fsgs_for_kvm() [Andy Lutomirski]
> - Trimmed comments [Andy Lutomirski]
> - Add Andy's A-b to what is now PATCH2
>
> Some time ago Paolo suggested to take a look at probably unneeded expensive
> rdmsrs for FS/GS base MSR in vmx_save_host_state(). This is called on every
> vcpu run when we need to handle vmexit in userspace.
>
> vmx_save_host_state() is always called in a very well defined context
> (ioctl from userspace) so we may try to get the required values for FS/GS
> bases from in-kernel variables and avoid expensive rdmsrs.
>
> My debug shows we're shaving off 240 cpu cycles (E5-2603 v3).
>
> Vitaly Kuznetsov (2):
> x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread
> x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE
>
> arch/x86/include/asm/processor.h | 10 ++++++++++
> arch/x86/kernel/cpu/common.c | 3 ++-
> arch/x86/kernel/process_64.c | 14 ++++++++++++++
> arch/x86/kvm/vmx.c | 16 ++++++++++++----
> 4 files changed, 38 insertions(+), 5 deletions(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-14 16:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 17:48 [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 1/2] x86/kvm/vmx: read MSR_{FS,KERNEL_GS}_BASE from current->thread Vitaly Kuznetsov
2018-03-13 17:48 ` [PATCH v2 2/2] x86/kvm/vmx: avoid expensive rdmsr for MSR_GS_BASE Vitaly Kuznetsov
2018-03-14 16:12 ` [PATCH v2 0/2] x86/kvm: avoid expensive rdmsrs for FS/GS base MSRs 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).