Re: [PATCH] x86/rapl: Do not load in a guest

On December 4, 2015 2:14:46 PM PST, Peter Zijlstra wrote: >On Fri, Dec 04, 2015 at 09:51:02AM -0800, Jacob Pan wrote: >> On Fri, 4 Dec 2015 09:22:56 +0100 >> Peter Zijlstra wrote: >> >> > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() + >> > wrmsr_on_cpu() all over the place. >>

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 09:51:02AM -0800, Jacob Pan wrote: > On Fri, 4 Dec 2015 09:22:56 +0100 > Peter Zijlstra wrote: > > > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() + > > wrmsr_on_cpu() all over the place. > Can you please be more specific? is the concern related to the >

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 10:37:31AM -0800, Jacob Pan wrote: > who is gonna collect all the DMI strings? I don't think this is > scalable. Let's hope only a small number of platforms is affected. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- To unsubscribe from

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 19:28:07 +0100 Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 10:16:10AM -0800, Jacob Pan wrote: > > CPU model detection is the first level checking. > > And in the case of RAPL, the only checking you can do. This is why it > should've had a CPUID bit. > I am 200% with

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 10:16:10AM -0800, Jacob Pan wrote: > CPU model detection is the first level checking. And in the case of RAPL, the only checking you can do. This is why it should've had a CPUID bit. > The error is about no valid domains (e.g. counters not working). So > the error on

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 19:04:26 +0100 Borislav Petkov wrote: > > This is good for the first level RAPL detection. The only way is to > > base detection on known CPU models. > > ...and yet Hannes sees it on a minnowboard. CCed. > > The CPU model-based detection is ugly and does not always work,

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 09:46:42AM -0800, Jacob Pan wrote: > Since RAPL is not architectural, consistency of hw support needs lots > of improvement at the least. It should have received a CPUID bit, no matter if it is architectural or not. > This error message is valid in other than VM. Domain

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 09:22:56 +0100 Peter Zijlstra wrote: > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() + > wrmsr_on_cpu() all over the place. Can you please be more specific? is the concern related to the overhead of IPI? I am doing these calls based on MSR CPU scope and consider

Re: [PATCH] x86/rapl: Do not load in a guest

On 04/12/2015 18:46, Jacob Pan wrote: > But I still think hypervisor check is sufficient. I don't there will > ever be a use case for VM to control platform level power. A disaster > for sure. There isn't just usual VMs. There are partitioning hypervisors for example. Paolo -- To unsubscribe

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 12:53:35 +0100 Ingo Molnar wrote: > > * Borislav Petkov wrote: > > > On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > > > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > > > there's no RAPL domains, the device doesn't load. On

Re: [PATCH] x86/rapl: Do not load in a guest

* Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > > there's no RAPL domains, the device doesn't load. On hypervisors, > > reading random MSRs is generally safe. > > Well, we could

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > there's no RAPL domains, the device doesn't load. On hypervisors, > reading random MSRs is generally safe. Well, we could not do anything, sure, that's an option

Re: [PATCH] x86/rapl: Do not load in a guest

On 04/12/2015 11:19, Borislav Petkov wrote: > + Paolo. > > On Fri, Dec 04, 2015 at 09:28:23AM +0100, Ingo Molnar wrote: So when a hypervisor starts supporting RAPL we'll disable the driver erroneously? Isn't there any better method to detect RAPL support? So in

Re: [PATCH] x86/rapl: Do not load in a guest

+ Paolo. On Fri, Dec 04, 2015 at 09:28:23AM +0100, Ingo Molnar wrote: > > > So when a hypervisor starts supporting RAPL we'll disable the driver > > > erroneously? > > > > > > Isn't there any better method to detect RAPL support? > > > > > > So in particular in drivers/powercap/intel_rapl.c

Re: [PATCH] x86/rapl: Do not load in a guest

* Peter Zijlstra wrote: > On Fri, Dec 04, 2015 at 08:42:06AM +0100, Ingo Molnar wrote: > > > > * Borislav Petkov wrote: > > > > > From: Borislav Petkov > > > > > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit > > > so check whether we're in a guest instead. > >

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 08:42:06AM +0100, Ingo Molnar wrote: > > * Borislav Petkov wrote: > > > From: Borislav Petkov > > > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit > > so check whether we're in a guest instead. > > So when a hypervisor starts supporting RAPL

Re: [PATCH] x86/rapl: Do not load in a guest

* Peter Zijlstra wrote: > On Fri, Dec 04, 2015 at 08:42:06AM +0100, Ingo Molnar wrote: > > > > * Borislav Petkov wrote: > > > > > From: Borislav Petkov > > > > > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit >

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 08:42:06AM +0100, Ingo Molnar wrote: > > * Borislav Petkov wrote: > > > From: Borislav Petkov > > > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit > > so check whether we're in a guest instead. > > So when a

Re: [PATCH] x86/rapl: Do not load in a guest

+ Paolo. On Fri, Dec 04, 2015 at 09:28:23AM +0100, Ingo Molnar wrote: > > > So when a hypervisor starts supporting RAPL we'll disable the driver > > > erroneously? > > > > > > Isn't there any better method to detect RAPL support? > > > > > > So in particular in drivers/powercap/intel_rapl.c

Re: [PATCH] x86/rapl: Do not load in a guest

* Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > > there's no RAPL domains, the device doesn't load. On hypervisors, > > reading random MSRs is generally safe. > >

Re: [PATCH] x86/rapl: Do not load in a guest

On 04/12/2015 11:19, Borislav Petkov wrote: > + Paolo. > > On Fri, Dec 04, 2015 at 09:28:23AM +0100, Ingo Molnar wrote: So when a hypervisor starts supporting RAPL we'll disable the driver erroneously? Isn't there any better method to detect RAPL support? So in

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > there's no RAPL domains, the device doesn't load. On hypervisors, > reading random MSRs is generally safe. Well, we could not do anything, sure, that's an option

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 12:53:35 +0100 Ingo Molnar wrote: > > * Borislav Petkov wrote: > > > On Fri, Dec 04, 2015 at 11:41:03AM +0100, Paolo Bonzini wrote: > > > No, please don't. Why do you need a wrmsr instead of a rdmsr? If > > > there's no RAPL domains, the

Re: [PATCH] x86/rapl: Do not load in a guest

On 04/12/2015 18:46, Jacob Pan wrote: > But I still think hypervisor check is sufficient. I don't there will > ever be a use case for VM to control platform level power. A disaster > for sure. There isn't just usual VMs. There are partitioning hypervisors for example. Paolo -- To unsubscribe

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 09:22:56 +0100 Peter Zijlstra wrote: > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() + > wrmsr_on_cpu() all over the place. Can you please be more specific? is the concern related to the overhead of IPI? I am doing these calls based on MSR

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 10:37:31AM -0800, Jacob Pan wrote: > who is gonna collect all the DMI strings? I don't think this is > scalable. Let's hope only a small number of platforms is affected. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- To unsubscribe from

Re: [PATCH] x86/rapl: Do not load in a guest

On December 4, 2015 2:14:46 PM PST, Peter Zijlstra wrote: >On Fri, Dec 04, 2015 at 09:51:02AM -0800, Jacob Pan wrote: >> On Fri, 4 Dec 2015 09:22:56 +0100 >> Peter Zijlstra wrote: >> >> > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() +

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 09:51:02AM -0800, Jacob Pan wrote: > On Fri, 4 Dec 2015 09:22:56 +0100 > Peter Zijlstra wrote: > > > Also, yuck @ powercap/intel_rapl.c for doing rdmsr_on_cpu() + > > wrmsr_on_cpu() all over the place. > Can you please be more specific? is the

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 09:46:42AM -0800, Jacob Pan wrote: > Since RAPL is not architectural, consistency of hw support needs lots > of improvement at the least. It should have received a CPUID bit, no matter if it is architectural or not. > This error message is valid in other than VM. Domain

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 19:04:26 +0100 Borislav Petkov wrote: > > This is good for the first level RAPL detection. The only way is to > > base detection on known CPU models. > > ...and yet Hannes sees it on a minnowboard. CCed. > > The CPU model-based detection is ugly and does

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, 4 Dec 2015 19:28:07 +0100 Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 10:16:10AM -0800, Jacob Pan wrote: > > CPU model detection is the first level checking. > > And in the case of RAPL, the only checking you can do. This is why it > should've had a CPUID bit. >

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 10:16:10AM -0800, Jacob Pan wrote: > CPU model detection is the first level checking. And in the case of RAPL, the only checking you can do. This is why it should've had a CPUID bit. > The error is about no valid domains (e.g. counters not working). So > the error on

Re: [PATCH] x86/rapl: Do not load in a guest

* Borislav Petkov wrote: > From: Borislav Petkov > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit > so check whether we're in a guest instead. So when a hypervisor starts supporting RAPL we'll disable the driver erroneously? Isn't there any better method to

Re: [PATCH] x86/rapl: Do not load in a guest

On Friday, December 04, 2015 12:25:34 AM Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 12:32:12AM +0100, Rafael J. Wysocki wrote: > > OK, so who's suppose to apply this? > > It doesn't matter to me. I was planning on routing it through tip next > week if you're fine with it or tip guys could

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 12:32:12AM +0100, Rafael J. Wysocki wrote: > OK, so who's suppose to apply this? It doesn't matter to me. I was planning on routing it through tip next week if you're fine with it or tip guys could ack it too and you could pick it up. Preference? -- Regards/Gruss,

Re: [PATCH] x86/rapl: Do not load in a guest

On Thursday, December 03, 2015 10:59:33 AM Jacob Pan wrote: > On Thu, 3 Dec 2015 19:42:41 +0100 > Borislav Petkov wrote: > > > No, those are going away: > > > > https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de > > > > Next on my TODO is killing the rest of them. > >

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, 3 Dec 2015 19:42:41 +0100 Borislav Petkov wrote: > No, those are going away: > > https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de > > Next on my TODO is killing the rest of them. Fair enough. Acked-by: Jacob Pan -- To unsubscribe from this list: send the line

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, Dec 03, 2015 at 10:38:28AM -0800, Jacob Pan wrote: > or use this? > #define cpu_has_hypervisorboot_cpu_has(X86_FEATURE_HYPERVISOR) No, those are going away: https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de Next on my TODO is killing the rest of them. --

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, 3 Dec 2015 19:27:02 +0100 Borislav Petkov wrote: > > + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) > + return 0; > + or use this? #define cpu_has_hypervisor boot_cpu_has(X86_FEATURE_HYPERVISOR) -- To unsubscribe from this list: send the line "unsubscribe

[PATCH] x86/rapl: Do not load in a guest

From: Borislav Petkov qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit so check whether we're in a guest instead. Reported-by: Hannes Reinecke Signed-off-by: Borislav Petkov Cc: Arnaldo Carvalho de Melo Cc: "H. Peter Anvin" Cc: Ingo Molnar Cc: Jacob Pan Cc: Peter

Re: [PATCH] x86/rapl: Do not load in a guest

* Borislav Petkov wrote: > From: Borislav Petkov > > qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit > so check whether we're in a guest instead. So when a hypervisor starts supporting RAPL we'll disable the driver erroneously? Isn't

Re: [PATCH] x86/rapl: Do not load in a guest

On Thursday, December 03, 2015 10:59:33 AM Jacob Pan wrote: > On Thu, 3 Dec 2015 19:42:41 +0100 > Borislav Petkov wrote: > > > No, those are going away: > > > > https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de > > > > Next on my TODO is killing the rest

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, 3 Dec 2015 19:42:41 +0100 Borislav Petkov wrote: > No, those are going away: > > https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de > > Next on my TODO is killing the rest of them. Fair enough. Acked-by: Jacob Pan

Re: [PATCH] x86/rapl: Do not load in a guest

On Fri, Dec 04, 2015 at 12:32:12AM +0100, Rafael J. Wysocki wrote: > OK, so who's suppose to apply this? It doesn't matter to me. I was planning on routing it through tip next week if you're fine with it or tip guys could ack it too and you could pick it up. Preference? -- Regards/Gruss,

Re: [PATCH] x86/rapl: Do not load in a guest

On Friday, December 04, 2015 12:25:34 AM Borislav Petkov wrote: > On Fri, Dec 04, 2015 at 12:32:12AM +0100, Rafael J. Wysocki wrote: > > OK, so who's suppose to apply this? > > It doesn't matter to me. I was planning on routing it through tip next > week if you're fine with it or tip guys could

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, 3 Dec 2015 19:27:02 +0100 Borislav Petkov wrote: > > + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) > + return 0; > + or use this? #define cpu_has_hypervisor boot_cpu_has(X86_FEATURE_HYPERVISOR) -- To unsubscribe from this list: send the line

[PATCH] x86/rapl: Do not load in a guest

From: Borislav Petkov qemu/kvm doesn't support RAPL and RAPL doesn't have a CPUID feature bit so check whether we're in a guest instead. Reported-by: Hannes Reinecke Signed-off-by: Borislav Petkov Cc: Arnaldo Carvalho de Melo Cc: "H.

Re: [PATCH] x86/rapl: Do not load in a guest

On Thu, Dec 03, 2015 at 10:38:28AM -0800, Jacob Pan wrote: > or use this? > #define cpu_has_hypervisorboot_cpu_has(X86_FEATURE_HYPERVISOR) No, those are going away: https://lkml.kernel.org/r/1448982023-19187-4-git-send-email...@alien8.de Next on my TODO is killing the rest of them. --