[PATCH] perf/x86: Disable uncore on virtualized CPU.

[PATCH] perf/x86: Disable uncore on virtualized CPU.

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

Initializing uncore PMU on virtualized CPU may hang the kernel.
This is because kvm does not emulate the entire hardware. Thers
are lots of uncore related MSRs, making kvm enumerate them all
is a non-trival task. So just disable uncore on virtualized CPU.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 0a55710..2f005ba 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
 
+	if (cpu_has_hypervisor)
+		return -ENODEV;
+
 	ret = uncore_pci_init();
 	if (ret)
 		goto fail;
-- 
1.7.11.2

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Pekka Enberg]

On Tue, Aug 21, 2012 at 12:08 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>
> Initializing uncore PMU on virtualized CPU may hang the kernel.
> This is because kvm does not emulate the entire hardware. Thers
> are lots of uncore related MSRs, making kvm enumerate them all
> is a non-trival task. So just disable uncore on virtualized CPU.
>
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0a55710..2f005ba 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
>         if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>                 return -ENODEV;
>
> +       if (cpu_has_hypervisor)
> +               return -ENODEV;
> +
>         ret = uncore_pci_init();
>         if (ret)
>                 goto fail;

Tested-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Andi Kleen]

On Tue, Aug 21, 2012 at 05:08:37PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> Initializing uncore PMU on virtualized CPU may hang the kernel.
> This is because kvm does not emulate the entire hardware. Thers
> are lots of uncore related MSRs, making kvm enumerate them all
> is a non-trival task. So just disable uncore on virtualized CPU.

I'm not sure cpu_has_hypervisor is reliable enough for this.
Better find out why it hangs and fix that.

Early rdmsrls should be rdmsrl_safe()
Early writes in the driver should check if anything was written,
and if not then disable itself.

-Andi

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Pekka Enberg]

On Tue, Aug 21, 2012 at 05:08:37PM +0800, Yan, Zheng wrote:
>> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>>
>> Initializing uncore PMU on virtualized CPU may hang the kernel.
>> This is because kvm does not emulate the entire hardware. Thers
>> are lots of uncore related MSRs, making kvm enumerate them all
>> is a non-trival task. So just disable uncore on virtualized CPU.

On Tue, Aug 21, 2012 at 5:31 PM, Andi Kleen <andi@firstfloor.org> wrote:
> I'm not sure cpu_has_hypervisor is reliable enough for this.
> Better find out why it hangs and fix that.
>
> Early rdmsrls should be rdmsrl_safe()
> Early writes in the driver should check if anything was written,
> and if not then disable itself.

It's unfortunate that the changelog does not include any reference to
the actual thread this was discussed in:

http://www.mail-archive.com/kvm@vger.kernel.org/msg77524.html

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Pekka Enberg]

On Tue, Aug 21, 2012 at 12:08 PM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
>> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>>
>> Initializing uncore PMU on virtualized CPU may hang the kernel.
>> This is because kvm does not emulate the entire hardware. Thers
>> are lots of uncore related MSRs, making kvm enumerate them all
>> is a non-trival task. So just disable uncore on virtualized CPU.
>>
>> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
>> ---
>>  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> index 0a55710..2f005ba 100644
>> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
>>         if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>>                 return -ENODEV;
>>
>> +       if (cpu_has_hypervisor)
>> +               return -ENODEV;
>> +
>>         ret = uncore_pci_init();
>>         if (ret)
>>                 goto fail;

On Tue, Aug 21, 2012 at 3:55 PM, Pekka Enberg <penberg@kernel.org> wrote:
> Tested-by: Pekka Enberg <penberg@kernel.org>

Ping? I have not seen a tip bot email for this. Is the patch queued?

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Ingo Molnar]

* Yan, Zheng <zheng.z.yan@intel.com> wrote:

> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> Initializing uncore PMU on virtualized CPU may hang the kernel.
> This is because kvm does not emulate the entire hardware. Thers
> are lots of uncore related MSRs, making kvm enumerate them all
> is a non-trival task. So just disable uncore on virtualized CPU.
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0a55710..2f005ba 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>  		return -ENODEV;
>  
> +	if (cpu_has_hypervisor)
> +		return -ENODEV;
> +
>  	ret = uncore_pci_init();
>  	if (ret)
>  		goto fail;

Cannot the presence of the uncore hardware be detected in a 
cleaner fashion, via the PCI config space and such?

Thanks,

	Ingo

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Peter Zijlstra]

On Wed, 2012-09-05 at 08:35 +0200, Ingo Molnar wrote:
> * Yan, Zheng <zheng.z.yan@intel.com> wrote:
> 
> > From: "Yan, Zheng" <zheng.z.yan@intel.com>
> > 
> > Initializing uncore PMU on virtualized CPU may hang the kernel.
> > This is because kvm does not emulate the entire hardware. Thers
> > are lots of uncore related MSRs, making kvm enumerate them all
> > is a non-trival task. So just disable uncore on virtualized CPU.
> > 
> > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> > ---
> >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > index 0a55710..2f005ba 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
> >  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> >  		return -ENODEV;
> >  
> > +	if (cpu_has_hypervisor)
> > +		return -ENODEV;
> > +
> >  	ret = uncore_pci_init();
> >  	if (ret)
> >  		goto fail;
> 
> Cannot the presence of the uncore hardware be detected in a 
> cleaner fashion, via the PCI config space and such?

No, part of the uncore PMUs are in MSR space and aren't discoverable.
CPUID model checks + hard assumptions of presence are all that we are
left with.

Now Avi suggested we teach KVM about these MSRs and then modify the
uncore driver to test if the MSRs actually work -- as in retain values
written to them and aren't always 0.

That's a larger patch though, partly because enumerating the gazillion
MSRs consumed by the various uncore PMUs is a tedious job, and we can
always do this later.

This patch is a minimal patch to at least make things 'work' for now.

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Wed, 2012-09-05 at 08:35 +0200, Ingo Molnar wrote:
> > * Yan, Zheng <zheng.z.yan@intel.com> wrote:
> > 
> > > From: "Yan, Zheng" <zheng.z.yan@intel.com>
> > > 
> > > Initializing uncore PMU on virtualized CPU may hang the kernel.
> > > This is because kvm does not emulate the entire hardware. Thers
> > > are lots of uncore related MSRs, making kvm enumerate them all
> > > is a non-trival task. So just disable uncore on virtualized CPU.
> > > 
> > > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> > > ---
> > >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > > index 0a55710..2f005ba 100644
> > > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > > @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
> > >  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> > >  		return -ENODEV;
> > >  
> > > +	if (cpu_has_hypervisor)
> > > +		return -ENODEV;
> > > +
> > >  	ret = uncore_pci_init();
> > >  	if (ret)
> > >  		goto fail;
> > 
> > Cannot the presence of the uncore hardware be detected in a 
> > cleaner fashion, via the PCI config space and such?
> 
> No, part of the uncore PMUs are in MSR space and aren't 
> discoverable. CPUID model checks + hard assumptions of 
> presence are all that we are left with.
>
> Now Avi suggested we teach KVM about these MSRs and then 
> modify the uncore driver to test if the MSRs actually work -- 
> as in retain values written to them and aren't always 0.
> 
> That's a larger patch though, partly because enumerating the 
> gazillion MSRs consumed by the various uncore PMUs is a 
> tedious job, and we can always do this later.
> 
> This patch is a minimal patch to at least make things 'work' 
> for now.

Ok, no objections.

Thanks,

	Ingo

Re: [PATCH] perf/x86: Disable uncore on virtualized CPU.

[Andi Kleen]

On Wed, Sep 05, 2012 at 08:35:24AM +0200, Ingo Molnar wrote:
> 
> * Yan, Zheng <zheng.z.yan@intel.com> wrote:
> 
> > From: "Yan, Zheng" <zheng.z.yan@intel.com>
> > 
> > Initializing uncore PMU on virtualized CPU may hang the kernel.
> > This is because kvm does not emulate the entire hardware. Thers
> > are lots of uncore related MSRs, making kvm enumerate them all
> > is a non-trival task. So just disable uncore on virtualized CPU.
> > 
> > Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> > ---
> >  arch/x86/kernel/cpu/perf_event_intel_uncore.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > index 0a55710..2f005ba 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> > @@ -2898,6 +2898,9 @@ static int __init intel_uncore_init(void)
> >  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> >  		return -ENODEV;
> >  
> > +	if (cpu_has_hypervisor)
> > +		return -ENODEV;
> > +
> >  	ret = uncore_pci_init();
> >  	if (ret)
> >  		goto fail;
> 
> Cannot the presence of the uncore hardware be detected in a 
> cleaner fashion, via the PCI config space and such?

Probably just by reading the registers and see that they are 0 and 
when writing read back and check.

And the code should not hang when that happens, then this check wouldn't
be needed.

-Andi

[tip:perf/urgent] perf/x86: Disable uncore on virtualized CPUs

[tip-bot for Yan, Zheng]

Commit-ID:  a05123bdd1b9ba961ed262864924a5b3ee81afe8
Gitweb:     http://git.kernel.org/tip/a05123bdd1b9ba961ed262864924a5b3ee81afe8
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Tue, 21 Aug 2012 17:08:37 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 20 Oct 2012 10:07:02 +0200

perf/x86: Disable uncore on virtualized CPUs

Initializing uncore PMU on virtualized CPU may hang the kernel.
This is because kvm does not emulate the entire hardware. Thers
are lots of uncore related MSRs, making kvm enumerate them all
is a non-trival task. So just disable uncore on virtualized CPU.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Tested-by: Pekka Enberg <penberg@kernel.org>
Cc: a.p.zijlstra@chello.nl
Cc: eranian@google.com
Cc: andi@firstfloor.org
Cc: avi@redhat.com
Link: http://lkml.kernel.org/r/1345540117-14164-1-git-send-email-zheng.z.yan@intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 99d96a4..5df8d32 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2926,6 +2926,9 @@ static int __init intel_uncore_init(void)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
 
+	if (cpu_has_hypervisor)
+		return -ENODEV;
+
 	ret = uncore_pci_init();
 	if (ret)
 		goto fail;