All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
@ 2010-11-18 12:17 Avi Kivity
  2010-11-18 13:48 ` Andi Kleen
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-11-18 12:17 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm; +Cc: Andi Kleen

cea15c2 ("KVM: Move KVM context switch into own function") split vmx_vcpu_run()
to prevent multiple copies of the context switch from being generated (causing
problems due to a label).  This patch folds them back together again and adds
the __noclone attribute to prevent the label from being duplicated.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/vmx.c |   63 ++++++++++++++++++++-------------------------------
 1 files changed, 25 insertions(+), 38 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9367abc..b4b66a8 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3902,17 +3902,33 @@ static void vmx_cancel_injection(struct kvm_vcpu *vcpu)
 #define Q "l"
 #endif
 
-/*
- * We put this into a separate noinline function to prevent the compiler
- * from duplicating the code. This is needed because this code
- * uses non local labels that cannot be duplicated.
- * Do not put any flow control into this function.
- * Better would be to put this whole monstrosity into a .S file.
- */
-static void noinline do_vmx_vcpu_run(struct kvm_vcpu *vcpu)
+static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	asm volatile(
+
+	/* Record the guest's net vcpu time for enforced NMI injections. */
+	if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
+		vmx->entry_time = ktime_get();
+
+	/* Don't enter VMX if guest state is invalid, let the exit handler
+	   start emulation until we arrive back to a valid state */
+	if (vmx->emulation_required && emulate_invalid_guest_state)
+		return;
+
+	if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
+		vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
+	if (test_bit(VCPU_REGS_RIP, (unsigned long *)&vcpu->arch.regs_dirty))
+		vmcs_writel(GUEST_RIP, vcpu->arch.regs[VCPU_REGS_RIP]);
+
+	/* When single-stepping over STI and MOV SS, we must clear the
+	 * corresponding interruptibility bits in the guest state. Otherwise
+	 * vmentry fails as it then expects bit 14 (BS) in pending debug
+	 * exceptions being set, but that's not correct for the guest debugging
+	 * case. */
+	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+		vmx_set_interrupt_shadow(vcpu, 0);
+
+	asm(
 		/* Store host registers */
 		"push %%"R"dx; push %%"R"bp;"
 		"push %%"R"cx \n\t"
@@ -4007,35 +4023,6 @@ static void noinline do_vmx_vcpu_run(struct kvm_vcpu *vcpu)
 		, "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
 #endif
 	      );
-}
-
-static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
-{
-	struct vcpu_vmx *vmx = to_vmx(vcpu);
-
-	/* Record the guest's net vcpu time for enforced NMI injections. */
-	if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
-		vmx->entry_time = ktime_get();
-
-	/* Don't enter VMX if guest state is invalid, let the exit handler
-	   start emulation until we arrive back to a valid state */
-	if (vmx->emulation_required && emulate_invalid_guest_state)
-		return;
-
-	if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
-		vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
-	if (test_bit(VCPU_REGS_RIP, (unsigned long *)&vcpu->arch.regs_dirty))
-		vmcs_writel(GUEST_RIP, vcpu->arch.regs[VCPU_REGS_RIP]);
-
-	/* When single-stepping over STI and MOV SS, we must clear the
-	 * corresponding interruptibility bits in the guest state. Otherwise
-	 * vmentry fails as it then expects bit 14 (BS) in pending debug
-	 * exceptions being set, but that's not correct for the guest debugging
-	 * case. */
-	if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
-		vmx_set_interrupt_shadow(vcpu, 0);
-
-	do_vmx_vcpu_run(vcpu);
 
 	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP)
 				  | (1 << VCPU_EXREG_PDPTR));
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
  2010-11-18 12:17 [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run() Avi Kivity
@ 2010-11-18 13:48 ` Andi Kleen
  2010-11-18 14:32   ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2010-11-18 13:48 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On 11/18/2010 1:17 PM, Avi Kivity wrote:
> cea15c2 ("KVM: Move KVM context switch into own function") split vmx_vcpu_run()
> to prevent multiple copies of the context switch from being generated (causing
> problems due to a label).  This patch folds them back together again and adds
> the __noclone attribute to prevent the label from being duplicated.

That won't work on gcc versions that didn't have __noclone yet. Noclone 
is fairly recent
(4.5 or 4.4)

-Andi


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
  2010-11-18 13:48 ` Andi Kleen
@ 2010-11-18 14:32   ` Avi Kivity
  2010-11-18 15:00     ` Andi Kleen
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-11-18 14:32 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Marcelo Tosatti, kvm

On 11/18/2010 03:48 PM, Andi Kleen wrote:
> On 11/18/2010 1:17 PM, Avi Kivity wrote:
>> cea15c2 ("KVM: Move KVM context switch into own function") split 
>> vmx_vcpu_run()
>> to prevent multiple copies of the context switch from being generated 
>> (causing
>> problems due to a label).  This patch folds them back together again 
>> and adds
>> the __noclone attribute to prevent the label from being duplicated.
>
> That won't work on gcc versions that didn't have __noclone yet. 
> Noclone is fairly recent
> (4.5 or 4.4)

Are the gcc versions that don't have noclone susceptible to cloning?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
  2010-11-18 14:32   ` Avi Kivity
@ 2010-11-18 15:00     ` Andi Kleen
  2010-11-18 15:08       ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2010-11-18 15:00 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On 11/18/2010 3:32 PM, Avi Kivity wrote:
> On 11/18/2010 03:48 PM, Andi Kleen wrote:
>> On 11/18/2010 1:17 PM, Avi Kivity wrote:
>>> cea15c2 ("KVM: Move KVM context switch into own function") split 
>>> vmx_vcpu_run()
>>> to prevent multiple copies of the context switch from being 
>>> generated (causing
>>> problems due to a label).  This patch folds them back together again 
>>> and adds
>>> the __noclone attribute to prevent the label from being duplicated.
>>
>> That won't work on gcc versions that didn't have __noclone yet. 
>> Noclone is fairly recent
>> (4.5 or 4.4)
>
> Are the gcc versions that don't have noclone susceptible to cloning?

I believe the problem can happen due to inlining already

-Andi



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
  2010-11-18 15:00     ` Andi Kleen
@ 2010-11-18 15:08       ` Avi Kivity
  2010-11-22  9:12         ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-11-18 15:08 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Marcelo Tosatti, kvm

On 11/18/2010 05:00 PM, Andi Kleen wrote:
> On 11/18/2010 3:32 PM, Avi Kivity wrote:
>> On 11/18/2010 03:48 PM, Andi Kleen wrote:
>>> On 11/18/2010 1:17 PM, Avi Kivity wrote:
>>>> cea15c2 ("KVM: Move KVM context switch into own function") split 
>>>> vmx_vcpu_run()
>>>> to prevent multiple copies of the context switch from being 
>>>> generated (causing
>>>> problems due to a label).  This patch folds them back together 
>>>> again and adds
>>>> the __noclone attribute to prevent the label from being duplicated.
>>>
>>> That won't work on gcc versions that didn't have __noclone yet. 
>>> Noclone is fairly recent
>>> (4.5 or 4.4)
>>
>> Are the gcc versions that don't have noclone susceptible to cloning?
>
> I believe the problem can happen due to inlining already

vmx_vcpu_run() cannot be inlined (it is only called via a function 
pointer; call site is in a different module)

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run()
  2010-11-18 15:08       ` Avi Kivity
@ 2010-11-22  9:12         ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2010-11-22  9:12 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Marcelo Tosatti, kvm

On 11/18/2010 05:08 PM, Avi Kivity wrote:
> On 11/18/2010 05:00 PM, Andi Kleen wrote:
>> On 11/18/2010 3:32 PM, Avi Kivity wrote:
>>> On 11/18/2010 03:48 PM, Andi Kleen wrote:
>>>> On 11/18/2010 1:17 PM, Avi Kivity wrote:
>>>>> cea15c2 ("KVM: Move KVM context switch into own function") split 
>>>>> vmx_vcpu_run()
>>>>> to prevent multiple copies of the context switch from being 
>>>>> generated (causing
>>>>> problems due to a label).  This patch folds them back together 
>>>>> again and adds
>>>>> the __noclone attribute to prevent the label from being duplicated.
>>>>
>>>> That won't work on gcc versions that didn't have __noclone yet. 
>>>> Noclone is fairly recent
>>>> (4.5 or 4.4)
>>>
>>> Are the gcc versions that don't have noclone susceptible to cloning?
>>
>> I believe the problem can happen due to inlining already
>
> vmx_vcpu_run() cannot be inlined (it is only called via a function 
> pointer; call site is in a different module)
>

Well, I've applied the patch; if it breaks again, let me know, and I'll 
revert it.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-11-22  9:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-18 12:17 [PATCH] KVM: VMX: Fold __vmx_vcpu_run() into vmx_vcpu_run() Avi Kivity
2010-11-18 13:48 ` Andi Kleen
2010-11-18 14:32   ` Avi Kivity
2010-11-18 15:00     ` Andi Kleen
2010-11-18 15:08       ` Avi Kivity
2010-11-22  9:12         ` Avi Kivity

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.