linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU.
@ 2021-06-07  2:53 Cody Yao-oc
  2021-06-10  7:20 ` [tip: perf/urgent] x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs tip-bot2 for CodyYao-oc
  2021-06-10  8:13 ` tip-bot2 for CodyYao-oc
  0 siblings, 2 replies; 5+ messages in thread
From: Cody Yao-oc @ 2021-06-07  2:53 UTC (permalink / raw)
  To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
	namhyung, tglx, bp, hpa, x86
  Cc: linux-perf-users, linux-kernel, shyaololo, CodyYao-oc

From: CodyYao-oc <CodyYao-oc@zhaoxin.com>

Fix "obtain wrong msr address" bug in function nmi_perfctr_msr_to_bit
and nmi_eventsel_msr_to_bit. In X86_VENDOR_INTEL switch branch, if all
of the check conditions are not met, code flow will slip to
X86_VENDOR_ZHAOXIN branch which may lead to incorrect information.

Using fallthrough instead of break was completely unintentional,
therefore, "fallthrough" should be changed to "break".

Fixes: 3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")

Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com>
---
 arch/x86/kernel/cpu/perfctr-watchdog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
index 3ef5868ac588..7aecb2fc3186 100644
--- a/arch/x86/kernel/cpu/perfctr-watchdog.c
+++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
@@ -63,7 +63,7 @@ static inline unsigned int nmi_perfctr_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BPU_PERFCTR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_PERFCTR0;
@@ -96,7 +96,7 @@ static inline unsigned int nmi_evntsel_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BSU_ESCR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_EVENTSEL0;
-- 
2.17.1


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

* [tip: perf/urgent] x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs
  2021-06-07  2:53 [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU Cody Yao-oc
@ 2021-06-10  7:20 ` tip-bot2 for CodyYao-oc
  2021-06-10  8:13 ` tip-bot2 for CodyYao-oc
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for CodyYao-oc @ 2021-06-10  7:20 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: CodyYao-oc, Ingo Molnar, x86, linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     89326c72c9b343da20a221a7234850e2e94161b7
Gitweb:        https://git.kernel.org/tip/89326c72c9b343da20a221a7234850e2e94161b7
Author:        CodyYao-oc <CodyYao-oc@zhaoxin.com>
AuthorDate:    Mon, 07 Jun 2021 10:53:35 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 10 Jun 2021 09:19:00 +02:00

x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs

The following commit:

   3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")

Got the old-style NMI watchdog logic wrong and broke it for basically every
Intel CPU where it was active. Which is only truly old CPUs, so few people noticed.

On CPUs with perf events support we turn off the old-style NMI watchdog, so it
was pretty pointless to add the logic for X86_VENDOR_ZHAOXIN to begin with ... :-/

Anyway, the fix is to restore the old logic and add a 'break'.

[ mingo: Wrote a new changelog. ]

Fixes: 3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")
Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20210607025335.9643-1-CodyYao-oc@zhaoxin.com
---
 arch/x86/kernel/cpu/perfctr-watchdog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
index 3ef5868..7aecb2f 100644
--- a/arch/x86/kernel/cpu/perfctr-watchdog.c
+++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
@@ -63,7 +63,7 @@ static inline unsigned int nmi_perfctr_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BPU_PERFCTR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_PERFCTR0;
@@ -96,7 +96,7 @@ static inline unsigned int nmi_evntsel_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BSU_ESCR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_EVENTSEL0;

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

* [tip: perf/urgent] x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs
  2021-06-07  2:53 [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU Cody Yao-oc
  2021-06-10  7:20 ` [tip: perf/urgent] x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs tip-bot2 for CodyYao-oc
@ 2021-06-10  8:13 ` tip-bot2 for CodyYao-oc
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for CodyYao-oc @ 2021-06-10  8:13 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: CodyYao-oc, Ingo Molnar, Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     a8383dfb2138742a1bb77b481ada047aededa2ba
Gitweb:        https://git.kernel.org/tip/a8383dfb2138742a1bb77b481ada047aededa2ba
Author:        CodyYao-oc <CodyYao-oc@zhaoxin.com>
AuthorDate:    Mon, 07 Jun 2021 10:53:35 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 10 Jun 2021 10:04:40 +02:00

x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs

The following commit:

   3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")

Got the old-style NMI watchdog logic wrong and broke it for basically every
Intel CPU where it was active. Which is only truly old CPUs, so few people noticed.

On CPUs with perf events support we turn off the old-style NMI watchdog, so it
was pretty pointless to add the logic for X86_VENDOR_ZHAOXIN to begin with ... :-/

Anyway, the fix is to restore the old logic and add a 'break'.

[ mingo: Wrote a new changelog. ]

Fixes: 3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")
Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210607025335.9643-1-CodyYao-oc@zhaoxin.com
---
 arch/x86/kernel/cpu/perfctr-watchdog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
index 3ef5868..7aecb2f 100644
--- a/arch/x86/kernel/cpu/perfctr-watchdog.c
+++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
@@ -63,7 +63,7 @@ static inline unsigned int nmi_perfctr_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BPU_PERFCTR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_PERFCTR0;
@@ -96,7 +96,7 @@ static inline unsigned int nmi_evntsel_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BSU_ESCR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_EVENTSEL0;

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

* Re: [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU.
  2021-06-04  5:54 [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU Cody Yao-oc
@ 2021-06-04 16:48 ` Sean Christopherson
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2021-06-04 16:48 UTC (permalink / raw)
  To: Cody Yao-oc
  Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
	namhyung, tglx, bp, hpa, x86, linux-perf-users, linux-kernel

On Fri, Jun 04, 2021, Cody Yao-oc wrote:
> From: CodyYao-oc <CodyYao-oc@zhaoxin.com>
> 
> Fix "obtain wrong msr address" bug in function nmi_perfctr_msr_to_bit
> and nmi_eventsel_msr_to_bit. In X86_VENDOR_INTEL switch branch, if all
> of the check conditions are not met, code flow will slip to
> X86_VENDOR_ZHAOXIN branch which may lead to incorrect information.
> 
> Therefore, "fallthrough" should be changed to "break" to avoid it.

This should explicitly state that using fallthrough instead of break was
completely unintentional, assuming that's indeed the case.

Fixes: 3a4ac121c2ca ("x86/perf: Add hardware performance events support for Zhaoxin CPU.")

> Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com>
> ---
>  arch/x86/kernel/cpu/perfctr-watchdog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
> index 3ef5868ac588..7aecb2fc3186 100644
> --- a/arch/x86/kernel/cpu/perfctr-watchdog.c
> +++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
> @@ -63,7 +63,7 @@ static inline unsigned int nmi_perfctr_msr_to_bit(unsigned int msr)
>  		case 15:
>  			return msr - MSR_P4_BPU_PERFCTR0;
>  		}
> -		fallthrough;
> +		break;
>  	case X86_VENDOR_ZHAOXIN:
>  	case X86_VENDOR_CENTAUR:
>  		return msr - MSR_ARCH_PERFMON_PERFCTR0;
> @@ -96,7 +96,7 @@ static inline unsigned int nmi_evntsel_msr_to_bit(unsigned int msr)
>  		case 15:
>  			return msr - MSR_P4_BSU_ESCR0;
>  		}
> -		fallthrough;
> +		break;
>  	case X86_VENDOR_ZHAOXIN:
>  	case X86_VENDOR_CENTAUR:
>  		return msr - MSR_ARCH_PERFMON_EVENTSEL0;
> -- 
> 2.17.1
> 

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

* [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU.
@ 2021-06-04  5:54 Cody Yao-oc
  2021-06-04 16:48 ` Sean Christopherson
  0 siblings, 1 reply; 5+ messages in thread
From: Cody Yao-oc @ 2021-06-04  5:54 UTC (permalink / raw)
  To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
	namhyung, tglx, bp, hpa, x86
  Cc: linux-perf-users, linux-kernel, CodyYao-oc

From: CodyYao-oc <CodyYao-oc@zhaoxin.com>

Fix "obtain wrong msr address" bug in function nmi_perfctr_msr_to_bit
and nmi_eventsel_msr_to_bit. In X86_VENDOR_INTEL switch branch, if all
of the check conditions are not met, code flow will slip to
X86_VENDOR_ZHAOXIN branch which may lead to incorrect information.

Therefore, "fallthrough" should be changed to "break" to avoid it.

Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com>
---
 arch/x86/kernel/cpu/perfctr-watchdog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perfctr-watchdog.c b/arch/x86/kernel/cpu/perfctr-watchdog.c
index 3ef5868ac588..7aecb2fc3186 100644
--- a/arch/x86/kernel/cpu/perfctr-watchdog.c
+++ b/arch/x86/kernel/cpu/perfctr-watchdog.c
@@ -63,7 +63,7 @@ static inline unsigned int nmi_perfctr_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BPU_PERFCTR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_PERFCTR0;
@@ -96,7 +96,7 @@ static inline unsigned int nmi_evntsel_msr_to_bit(unsigned int msr)
 		case 15:
 			return msr - MSR_P4_BSU_ESCR0;
 		}
-		fallthrough;
+		break;
 	case X86_VENDOR_ZHAOXIN:
 	case X86_VENDOR_CENTAUR:
 		return msr - MSR_ARCH_PERFMON_EVENTSEL0;
-- 
2.17.1


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

end of thread, other threads:[~2021-06-10  8:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07  2:53 [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU Cody Yao-oc
2021-06-10  7:20 ` [tip: perf/urgent] x86/nmi_watchdog: Fix old-style NMI watchdog regression on old Intel CPUs tip-bot2 for CodyYao-oc
2021-06-10  8:13 ` tip-bot2 for CodyYao-oc
  -- strict thread matches above, loose matches on Subject: below --
2021-06-04  5:54 [PATCH] x86/perf: Fixed obtaining address error about performance monitor MSR on old Inel CPU Cody Yao-oc
2021-06-04 16:48 ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).