KVM: VMX: Improve handle_external_interrupt_irqoff inline assembly

PATCH Superseded Uros Bizjak 2 participants 4 messages Apr 17, 2020

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Uros Bizjak

author

2020-04-17 11:35 UTC #0 lore raw

PATCH 1/1 KVM: VMX: Improve handle_external_interrupt_irqoff inline assembly

Improve handle_external_interrupt_irqoff inline assembly in several ways:
- use __stringify to use __KERNEL_DS and __KERNEL_CS directly in assembler
- use %rsp instead of _ASM_SP, since we are in CONFIG_X86_64 part
- use $-16 immediate to align %rsp
- avoid earlyclobber by using "l" GCC input operand constraint
- avoid temporary by using current_stack_pointer

The patch introduces no functional change.

Cc Paolo Bonzini <[email protected]>

Cc Sean Christopherson <[email protected]>

Signed-off-by Uros Bizjak <[email protected]>

---
 arch/x86/kvm/vmx/vmx.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

arch/x86/kvm/vmx/vmx.c +6 ~~-13~~

index 01330096ff3e..4b0d5f0044ff 100644
@@ -6233,9 +6233,6 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
 {
 	unsigned int vector;
 	unsigned long entry;
-#ifdef CONFIG_X86_64
-	unsigned long tmp;
-#endif
 	gate_desc *desc;
 	u32 intr_info;

@@ -6252,23 +6249,19 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
 
 	asm volatile(
 #ifdef CONFIG_X86_64
-		"mov %%" _ASM_SP ", %[sp]\n\t"
-		"and $0xfffffffffffffff0, %%" _ASM_SP "\n\t"
-		"push $%c[ss]\n\t"
+		"and $-16, %%rsp\n\t"
+		"push $" __stringify(__KERNEL_DS) "\n\t"
 		"push %[sp]\n\t"
 #endif
 		"pushf\n\t"
-		__ASM_SIZE(push) " $%c[cs]\n\t"
+		__ASM_SIZE(push) " $" __stringify(__KERNEL_CS) "\n\t"
 		CALL_NOSPEC
+		: ASM_CALL_CONSTRAINT
 		:
 #ifdef CONFIG_X86_64
-		[sp]"=&r"(tmp),
+		[sp]"l"(current_stack_pointer),
 #endif
-		ASM_CALL_CONSTRAINT
-		:
-		[thunk_target]"r"(entry),
-		[ss]"i"(__KERNEL_DS),
-		[cs]"i"(__KERNEL_CS)
+		[thunk_target]"r"(entry)
 	);
 
 	kvm_after_interrupt(vcpu);

Signature · 1 line

2.25.2

Paolo Bonzini

maintainer

2020-04-17 12:08 UTC #1 lore raw

Quoted 2 lines · Uros Bizjak

Improve handle_external_interrupt_irqoff inline assembly in several ways:
- use __stringify to use __KERNEL_DS and __KERNEL_CS directly in assembler

What's the advantage of this to pushing cs and ss? We are faking an interrupt gate, and interrupts push cs and ss not __KERNEL_DS and __KERNEL_CS.

Quoted 3 lines

- use %rsp instead of _ASM_SP, since we are in CONFIG_X86_64 part
- use $-16 immediate to align %rsp
- avoid earlyclobber by using "l" GCC input operand constraint

What is this and where is it documented?!? :)

Paolo

Quoted 42 lines

- avoid temporary by using current_stack_pointer

The patch introduces no functional change.

Cc Paolo Bonzini <[email protected]>

Cc Sean Christopherson <[email protected]>

Signed-off-by Uros Bizjak <[email protected]>

---
 arch/x86/kvm/vmx/vmx.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

arch/x86/kvm/vmx/vmx.c +4 -7

index 01330096ff3e..4b0d5f0044ff 100644
@@ -6233,9 +6233,6 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
 {
 	unsigned int vector;
 	unsigned long entry;
-#ifdef CONFIG_X86_64
-	unsigned long tmp;
-#endif
 	gate_desc *desc;
 	u32 intr_info;

@@ -6252,23 +6249,19 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
 
 	asm volatile(
 #ifdef CONFIG_X86_64
-		"mov %%" _ASM_SP ", %[sp]\n\t"
-		"and $0xfffffffffffffff0, %%" _ASM_SP "\n\t"
-		"push $%c[ss]\n\t"
+		"and $-16, %%rsp\n\t"
+		"push $" __stringify(__KERNEL_DS) "\n\t"
 		"push %[sp]\n\t"
 #endif
 		"pushf\n\t"
-		__ASM_SIZE(push) " $%c[cs]\n\t"
+		__ASM_SIZE(push) " $" __stringify(__KERNEL_CS) "\n\t"
 		CALL_NOSPEC
+		: ASM_CALL_CONSTRAINT
 		:

Uros Bizjak

author

2020-04-17 12:25 UTC #2 lore raw

Quoted 8 lines · Paolo Bonzini

Quoted 2 lines · Uros Bizjak
Improve handle_external_interrupt_irqoff inline assembly in several ways:
- use __stringify to use __KERNEL_DS and __KERNEL_CS directly in assembler
What's the advantage of this to pushing cs and ss? We are faking an interrupt gate, and interrupts push cs and ss not __KERNEL_DS and __KERNEL_CS.

I see. If the current form is documenting the above, then there is indeed no compelling reason for the change.

Quoted 6 lines

Quoted 4 lines
- use %rsp instead of _ASM_SP, since we are in CONFIG_X86_64 part
- use $-16 immediate to align %rsp
- avoid earlyclobber by using "l" GCC input operand constraint
What is this and where is it documented?!? :)

I was going to say that it is documented in [1] under x86 family section, but indeed, there is no description of "l" constraint. It is internal to the compiler. :(

(define_register_constraint "l" "INDEX_REGS"
"@internal Any register that can be used as the index in a base+index
memory access: that is, any general register except the stack pointer.")

Other changes are minor and really not worth the maintainer's time to handle the patch. Please consider the patch retracted.

[1] https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html

Uros.

Paolo Bonzini

maintainer

2020-04-17 12:32 UTC #3 lore raw

Quoted 3 lines · Uros Bizjak

I was going to say that it is documented in [1] under x86 family section, but indeed, there is no description of "l" constraint. It is internal to the compiler. :(

Yes, I looked there. It's worth documenting it in GCC, I think.

Paolo

Quoted 3 lines

(define_register_constraint "l" "INDEX_REGS"
"@internal Any register that can be used as the index in a base+index
memory access: that is, any general register except the stack pointer.")