Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584747 > unrolled thread

[PATCH 0/6] KVM TSS cleanups and speedups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] KVM TSS cleanups and speedups Andy Lutomirski <luto@kernel.org> - 2017-02-20 18:00 +0100
    [PATCH 2/6] x86/kvm/vmx: Don't fetch the TSS base from the GDT Andy Lutomirski <luto@kernel.org> - 2017-02-20 18:00 +0100
    [PATCH 5/6] x86/asm/64: Drop __cacheline_aligned from struct x86_hw_tss Andy Lutomirski <luto@kernel.org> - 2017-02-20 18:00 +0100
    [PATCH 1/6] x86/asm: Define the kernel TSS limit in a macro Andy Lutomirski <luto@kernel.org> - 2017-02-20 18:10 +0100
    Re: [PATCH 0/6] KVM TSS cleanups and speedups Paolo Bonzini <pbonzini@redhat.com> - 2017-02-20 19:00 +0100

#1584747 — [PATCH 0/6] KVM TSS cleanups and speedups

The first four patches here are intended to be straightforward
cleanups and to make a better base for Thomas' GDT series.  They may
be a slight speedup, too, because they remove an STR instruction
from the VMX entry path.

The last two patches are a reasonably large speedup but need careful
review.

FWIW, I can see lots of additional easy-ish speedups here.  For example:

 - The GDT reload on VM exit isn't really needed at all.  Instead let's
   just change the kernel limit to 0xFFFF.  Doing that naively would
   waste memory, but doing it carefully on top of Thomas' series would
   be straightforward and almost free.

 - RDMSR from MSR_GS_BASE is totally pointless.

 - Once I or someone finishes the FSGSBASE series, we get a big speedup
   there.

 - The LDT reload code should be split up and optimized better, I think.

Andy Lutomirski (6):
  x86/asm: Define the kernel TSS limit in a macro
  x86/kvm/vmx: Don't fetch the TSS base from the GDT
  x86/kvm/vmx: Get rid of segment_base() on 64-bit kernels
  x86/kvm/vmx: Simplify segment_base()
  x86/asm/64: Drop __cacheline_aligned from struct x86_hw_tss
  x86/kvm/vmx: Defer TR reload after VM exit

 arch/x86/include/asm/desc.h      | 58 ++++++++++++++++++++++++++++++------
 arch/x86/include/asm/processor.h | 12 +++++++-
 arch/x86/kernel/ioport.c         |  5 ++++
 arch/x86/kernel/process.c        | 10 +++++++
 arch/x86/kvm/vmx.c               | 63 +++++++++++++++++-----------------------
 5 files changed, 102 insertions(+), 46 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1584748 — [PATCH 2/6] x86/kvm/vmx: Don't fetch the TSS base from the GDT

The current CPU's TSS base is a foregone conclusion, so there's no need
to parse it out of the segment tables.  This should save a couple cycles
(as STR is surely microcoded and poorly optimized) but, more importantly,
it's a cleanup and it means that segment_base() will never be called on
64-bit kernels.

Cc: Thomas Garnier <thgarnie@google.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kvm/vmx.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a236decb81e4..46420aaf1684 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2088,13 +2088,6 @@ static unsigned long segment_base(u16 selector)
 	return v;
 }
 
-static inline unsigned long kvm_read_tr_base(void)
-{
-	u16 tr;
-	asm("str %0" : "=g"(tr));
-	return segment_base(tr);
-}
-
 static void vmx_save_host_state(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -2294,10 +2287,11 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 		/*
 		 * Linux uses per-cpu TSS and GDT, so set these when switching
-		 * processors.
+		 * processors.  See 22.2.4.
 		 */
-		vmcs_writel(HOST_TR_BASE, kvm_read_tr_base()); /* 22.2.4 */
-		vmcs_writel(HOST_GDTR_BASE, gdt->address);   /* 22.2.4 */
+		vmcs_writel(HOST_TR_BASE,
+			    (unsigned long)this_cpu_ptr(&cpu_tss));
+		vmcs_writel(HOST_GDTR_BASE, gdt->address);
 
 		rdmsrl(MSR_IA32_SYSENTER_ESP, sysenter_esp);
 		vmcs_writel(HOST_IA32_SYSENTER_ESP, sysenter_esp); /* 22.2.3 */
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1584754 — [PATCH 5/6] x86/asm/64: Drop __cacheline_aligned from struct x86_hw_tss

Historically, the entire TSS + io bitmap structure was cacheline
aligned, but commit ca241c75037b ("x86: unify tss_struct") changed it
(presumably inadvertently) so that the fixed-layout hardware part is
cacheline-aligned and the io bitmap is after the padding.  This wastes
24 bytes (the hardware part should be 104 bytes, but this pads it to
128 bytes) and, serves no purpose, and causes sizeof(struct
x86_hw_tss) to have a confusing value.

Drop the pointless alignment.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index feb2ab95b8f6..f385eca5407a 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -304,7 +304,7 @@ struct x86_hw_tss {
 	u16			reserved5;
 	u16			io_bitmap_base;
 
-} __attribute__((packed)) ____cacheline_aligned;
+} __attribute__((packed));
 #endif
 
 /*
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1584763 — [PATCH 1/6] x86/asm: Define the kernel TSS limit in a macro

Rather than open-coding the kernel TSS limit in set_tss_desc(), make
it a real macro near the TSS layout definition.

This is purely a cleanup.

Cc: Thomas Garnier <thgarnie@google.com>
Cc: Jim Mattson <jmattson@google.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/desc.h      | 10 +---------
 arch/x86/include/asm/processor.h | 10 ++++++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 12080d87da3b..2e781bcc5e12 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -177,16 +177,8 @@ static inline void __set_tss_desc(unsigned cpu, unsigned int entry, void *addr)
 	struct desc_struct *d = get_cpu_gdt_table(cpu);
 	tss_desc tss;
 
-	/*
-	 * sizeof(unsigned long) coming from an extra "long" at the end
-	 * of the iobitmap. See tss_struct definition in processor.h
-	 *
-	 * -1? seg base+limit should be pointing to the address of the
-	 * last valid byte
-	 */
 	set_tssldt_descriptor(&tss, (unsigned long)addr, DESC_TSS,
-			      IO_BITMAP_OFFSET + IO_BITMAP_BYTES +
-			      sizeof(unsigned long) - 1);
+			      __KERNEL_TSS_LIMIT);
 	write_gdt_entry(d, entry, &tss, DESC_TSS);
 }
 
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index e6cfe7ba2d65..feb2ab95b8f6 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -342,6 +342,16 @@ struct tss_struct {
 
 DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss);
 
+/*
+ * sizeof(unsigned long) coming from an extra "long" at the end
+ * of the iobitmap.
+ *
+ * -1? seg base+limit should be pointing to the address of the
+ * last valid byte
+ */
+#define __KERNEL_TSS_LIMIT	\
+	(IO_BITMAP_OFFSET + IO_BITMAP_BYTES + sizeof(unsigned long) - 1)
+
 #ifdef CONFIG_X86_32
 DECLARE_PER_CPU(unsigned long, cpu_current_top_of_stack);
 #endif
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1584803

On 20/02/2017 17:56, Andy Lutomirski wrote:
> The first four patches here are intended to be straightforward
> cleanups and to make a better base for Thomas' GDT series.  They may
> be a slight speedup, too, because they remove an STR instruction
> from the VMX entry path.
> 
> The last two patches are a reasonably large speedup but need careful
> review.

I spotted (I think) a bug in patch 6, but apart from that it looks nice.

Paolo

> FWIW, I can see lots of additional easy-ish speedups here.  For example:
> 
>  - The GDT reload on VM exit isn't really needed at all.  Instead let's
>    just change the kernel limit to 0xFFFF.  Doing that naively would
>    waste memory, but doing it carefully on top of Thomas' series would
>    be straightforward and almost free.
> 
>  - RDMSR from MSR_GS_BASE is totally pointless.
> 
>  - Once I or someone finishes the FSGSBASE series, we get a big speedup
>    there.
> 
>  - The LDT reload code should be split up and optimized better, I think.
> 
> Andy Lutomirski (6):
>   x86/asm: Define the kernel TSS limit in a macro
>   x86/kvm/vmx: Don't fetch the TSS base from the GDT
>   x86/kvm/vmx: Get rid of segment_base() on 64-bit kernels
>   x86/kvm/vmx: Simplify segment_base()
>   x86/asm/64: Drop __cacheline_aligned from struct x86_hw_tss
>   x86/kvm/vmx: Defer TR reload after VM exit
> 
>  arch/x86/include/asm/desc.h      | 58 ++++++++++++++++++++++++++++++------
>  arch/x86/include/asm/processor.h | 12 +++++++-
>  arch/x86/kernel/ioport.c         |  5 ++++
>  arch/x86/kernel/process.c        | 10 +++++++
>  arch/x86/kvm/vmx.c               | 63 +++++++++++++++++-----------------------
>  5 files changed, 102 insertions(+), 46 deletions(-)
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web