Patchwork KVM: PPC: Don't sync timebase when inside KVM

login
register
mail settings
Submitter Alexander Graf
Date Feb. 29, 2012, 2:16 a.m.
Message ID <1330481769-24390-1-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/143657/
State New
Headers show

Comments

Alexander Graf - Feb. 29, 2012, 2:16 a.m.
When we know that we're running inside of a KVM guest, we don't have to
worry about synchronizing timebases between different CPUs, since the
host already took care of that.

This fixes CPU overcommit scenarios where vCPUs could hang forever trying
to sync each other while not being scheduled.

Reported-by: Stuart Yoder <B08248@freescale.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kernel/smp.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Scott Wood - Feb. 29, 2012, 5:50 p.m.
On 02/28/2012 08:16 PM, Alexander Graf wrote:
> When we know that we're running inside of a KVM guest, we don't have to
> worry about synchronizing timebases between different CPUs, since the
> host already took care of that.
> 
> This fixes CPU overcommit scenarios where vCPUs could hang forever trying
> to sync each other while not being scheduled.
> 
> Reported-by: Stuart Yoder <B08248@freescale.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>

This should apply to any hypervisor, not just KVM.  On book3e, Power ISA
says timebase is read-only on virtualized implementations.  My
understanding is that book3s is paravirt-only (guest state is not
considered an implementation of the Power ISA), and it says "Writing the
Time Base is privileged, and can be done only in hypervisor state".

Which platforms are you seeing this on?  If it's on Freescale chips,
U-Boot should be doing the sync and Linux should never do it, even in
the absence of a hypervisor.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - Feb. 29, 2012, 6:28 p.m.
On 29.02.2012, at 18:50, Scott Wood <scottwood@freescale.com> wrote:

> On 02/28/2012 08:16 PM, Alexander Graf wrote:
>> When we know that we're running inside of a KVM guest, we don't have to
>> worry about synchronizing timebases between different CPUs, since the
>> host already took care of that.
>> 
>> This fixes CPU overcommit scenarios where vCPUs could hang forever trying
>> to sync each other while not being scheduled.
>> 
>> Reported-by: Stuart Yoder <B08248@freescale.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> This should apply to any hypervisor, not just KVM.  

Sure, but do you have a generic function to evaluate that? :)

> On book3e, Power ISA
> says timebase is read-only on virtualized implementations.  My
> understanding is that book3s is paravirt-only (guest state is not
> considered an implementation of the Power ISA), and it says "Writing the
> Time Base is privileged, and can be done only in hypervisor state".

For PR non-PAPR KVM, we are non-paravirt, but ignore tb writes iirc.

> 
> Which platforms are you seeing this on?  If it's on Freescale chips,
> U-Boot should be doing the sync and Linux should never do it, even in
> the absence of a hypervisor.

This is on e500mc.

Alex

> 
> -Scott
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Feb. 29, 2012, 7:06 p.m.
On 02/29/2012 12:28 PM, Alexander Graf wrote:
> 
> 
> On 29.02.2012, at 18:50, Scott Wood <scottwood@freescale.com> wrote:
> 
>> On 02/28/2012 08:16 PM, Alexander Graf wrote:
>>> When we know that we're running inside of a KVM guest, we don't have to
>>> worry about synchronizing timebases between different CPUs, since the
>>> host already took care of that.
>>>
>>> This fixes CPU overcommit scenarios where vCPUs could hang forever trying
>>> to sync each other while not being scheduled.
>>>
>>> Reported-by: Stuart Yoder <B08248@freescale.com>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> This should apply to any hypervisor, not just KVM.  
> 
> Sure, but do you have a generic function to evaluate that? :)

The presence of a hypervisor node without testing compatible.  Might not
get them all, but at least it will cover more than just KVM.

>> Which platforms are you seeing this on?  If it's on Freescale chips,
>> U-Boot should be doing the sync and Linux should never do it, even in
>> the absence of a hypervisor.
> 
> This is on e500mc.

On e500mc Linux should never by trying to sync the timebase.  If it is,
let's fix that.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 46695fe..670b453 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -49,6 +49,8 @@ 
 #ifdef CONFIG_PPC64
 #include <asm/paca.h>
 #endif
+#include <linux/kvm_para.h>
+#include <asm/kvm_para.h>
 
 #ifdef DEBUG
 #include <asm/udbg.h>
@@ -541,7 +543,7 @@  int __cpuinit __cpu_up(unsigned int cpu)
 
 	DBG("Processor %u found.\n", cpu);
 
-	if (smp_ops->give_timebase)
+	if (!kvm_para_available() && smp_ops->give_timebase)
 		smp_ops->give_timebase();
 
 	/* Wait until cpu puts itself in the online map */
@@ -626,7 +628,7 @@  void __devinit start_secondary(void *unused)
 
 	if (smp_ops->setup_cpu)
 		smp_ops->setup_cpu(cpu);
-	if (smp_ops->take_timebase)
+	if (!kvm_para_available() && smp_ops->take_timebase)
 		smp_ops->take_timebase();
 
 	secondary_cpu_time_init();