mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models
       [not found] <1546594609-22403-1-git-send-email-shirish.s@amd.com>
@ 2019-01-04  9:37 ` S, Shirish
  2019-01-04  9:37 ` [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well S, Shirish
  1 sibling, 0 replies; 14+ messages in thread
From: S, Shirish @ 2019-01-04  9:37 UTC (permalink / raw)
  Cc: S, Shirish, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Tony Luck, Vishal Verma,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

The below patch added this quirk only for the first generation of family
15 processors, over time its noticed that its required for later
generations too.

"575203b4747c x86, MCE, AMD: Disable error thresholding bank 4 on some
models"

This patch extends the quirk to make it applicable till 7th Generation,
so as to address the below warning at boot:

"[Firmware Bug]: cpu 0, invalid threshold interrupt offset ..."

Signed-off-by: Shirish S <shirish.s@amd.com>
---
 arch/x86/kernel/cpu/mce/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 672c722..051b536 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1616,7 +1616,7 @@ static int __mcheck_cpu_apply_quirks(struct cpuinfo_x86 *c)
 		 * they're not supported there.
 		 */
 		if (c->x86 == 0x15 &&
-		    (c->x86_model >= 0x10 && c->x86_model <= 0x1f)) {
+		    (c->x86_model >= 0x10 && c->x86_model <= 0x7f)) {
 			int i;
 			u64 hwcr;
 			bool need_toggle;
-- 
2.7.4


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

* [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
       [not found] <1546594609-22403-1-git-send-email-shirish.s@amd.com>
  2019-01-04  9:37 ` [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models S, Shirish
@ 2019-01-04  9:37 ` S, Shirish
  1 sibling, 0 replies; 14+ messages in thread
From: S, Shirish @ 2019-01-04  9:37 UTC (permalink / raw)
  Cc: S, Shirish, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Ghannam, Yazen, Kees Cook,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

This patch adds threshold quirk applicable for family 15
in resume path as well, since mce_amd_feature_init()
does not have quirks applied when originating from mce_syscore_resume(),
resulting in the below message at every successful resume:

"[Firmware Bug]: cpu 0, invalid threshold interrupt offset ..."

Signed-off-by: Shirish S <shirish.s@amd.com>
---
 arch/x86/kernel/cpu/mce/amd.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 89298c8..27cbf66 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -545,6 +545,34 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
 	return offset;
 }
 
+void disable_err_thresholding(struct cpuinfo_x86 *c)
+{
+	int i;
+	u64 hwcr;
+	bool need_toggle;
+	u32 msrs[] = {
+		0x00000413, /* MC4_MISC0 */
+		0xc0000408, /* MC4_MISC1 */
+	};
+
+	if (c->x86_model >= 0x10 && c->x86_model <= 0x7f) {
+		rdmsrl(MSR_K7_HWCR, hwcr);
+
+		/* McStatusWrEn has to be set */
+		need_toggle = !(hwcr & BIT(18));
+
+		if (need_toggle)
+			wrmsrl(MSR_K7_HWCR, hwcr | BIT(18));
+
+		/* Clear CntP bit safely */
+		for (i = 0; i < ARRAY_SIZE(msrs); i++)
+			msr_clear_bit(msrs[i], 62);
+
+		/* restore old settings */
+		if (need_toggle)
+			wrmsrl(MSR_K7_HWCR, hwcr);
+	}
+}
 /* cpu init entry point, called from mce.c with preempt off */
 void mce_amd_feature_init(struct cpuinfo_x86 *c)
 {
@@ -552,6 +580,12 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
 	unsigned int bank, block, cpu = smp_processor_id();
 	int offset = -1;
 
+	/* Disable error thresholding bank in S3 resume path as well,
+	 * for 15h family
+	 */
+	if (c->x86 == 0x15)
+		disable_err_thresholding(c);
+
 	for (bank = 0; bank < mca_cfg.banks; ++bank) {
 		if (mce_flags.smca)
 			smca_configure(bank, cpu);
-- 
2.7.4


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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-16 15:36                       ` S, Shirish
@ 2019-01-16 15:38                         ` Borislav Petkov
  0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2019-01-16 15:38 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh

On Wed, Jan 16, 2019 at 03:36:04PM +0000, S, Shirish wrote:
> I believe its fixed now in : 
> 
> https://lkml.org/lkml/2019/1/16/507
> https://lkml.org/lkml/2019/1/16/508
> 
> I don’t see S@vger ... in the From field in the above links instead I see "S, Shirish" <>

Yes, it looks correct now.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* RE: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-16 15:31                     ` S, Shirish
@ 2019-01-16 15:36                       ` S, Shirish
  2019-01-16 15:38                         ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: S, Shirish @ 2019-01-16 15:36 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh

I believe its fixed now in : 

https://lkml.org/lkml/2019/1/16/507
https://lkml.org/lkml/2019/1/16/508

I don’t see S@vger ... in the From field in the above links instead I see "S, Shirish" <>


Regards,
Shirish S

-----Original Message-----
From: S, Shirish 
Sent: Wednesday, January 16, 2019 9:01 PM
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H . Peter Anvin <hpa@zytor.com>; maintainer : X86 ARCHITECTURE <x86@kernel.org>; Tony Luck <tony.luck@intel.com>; Vishal Verma <vishal.l.verma@intel.com>; open list : X86 ARCHITECTURE <linux-kernel@vger.kernel.org>; Lendacky, Thomas <Thomas.Lendacky@amd.com>; Singh, Brijesh <brijesh.singh@amd.com>
Subject: RE: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

Nope thats not my email id, am not sure how(S@vger.kernel.org) its getting added.
Do you find the same for the new patchset I have sent? 


Regards,
Shirish S

-----Original Message-----
From: Borislav Petkov <bp@alien8.de>
Sent: Wednesday, January 16, 2019 8:57 PM
To: S, Shirish <Shirish.S@amd.com>
Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H . Peter Anvin <hpa@zytor.com>; maintainer : X86 ARCHITECTURE <x86@kernel.org>; Tony Luck <tony.luck@intel.com>; Vishal Verma <vishal.l.verma@intel.com>; open list : X86 ARCHITECTURE <linux-kernel@vger.kernel.org>; Lendacky, Thomas <Thomas.Lendacky@amd.com>; Singh, Brijesh <brijesh.singh@amd.com>
Subject: Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

On Wed, Jan 16, 2019 at 03:14:29PM +0000, S wrote:
> Done. Hope the mailer is fine now, i have added --from while sending 
> the new patchset and also got it reviewed from folks here.

No, it isn't. And the problem is not git. The problem is your normal emails you're sending. Look at what you just sent:

https://lore.kernel.org/lkml/8bed39c8-df48-dbb9-3674-dfbc4aea2967@amd.com/raw

and especially your From: field:

From:   S@vger.kernel.org

Is that your email address?

--
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* RE: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-16 15:26                   ` Borislav Petkov
@ 2019-01-16 15:31                     ` S, Shirish
  2019-01-16 15:36                       ` S, Shirish
  0 siblings, 1 reply; 14+ messages in thread
From: S, Shirish @ 2019-01-16 15:31 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh

Nope thats not my email id, am not sure how(S@vger.kernel.org) its getting added.
Do you find the same for the new patchset I have sent? 


Regards,
Shirish S

-----Original Message-----
From: Borislav Petkov <bp@alien8.de> 
Sent: Wednesday, January 16, 2019 8:57 PM
To: S, Shirish <Shirish.S@amd.com>
Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H . Peter Anvin <hpa@zytor.com>; maintainer : X86 ARCHITECTURE <x86@kernel.org>; Tony Luck <tony.luck@intel.com>; Vishal Verma <vishal.l.verma@intel.com>; open list : X86 ARCHITECTURE <linux-kernel@vger.kernel.org>; Lendacky, Thomas <Thomas.Lendacky@amd.com>; Singh, Brijesh <brijesh.singh@amd.com>
Subject: Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

On Wed, Jan 16, 2019 at 03:14:29PM +0000, S wrote:
> Done. Hope the mailer is fine now, i have added --from while sending 
> the new patchset and also got it reviewed from folks here.

No, it isn't. And the problem is not git. The problem is your normal emails you're sending. Look at what you just sent:

https://lore.kernel.org/lkml/8bed39c8-df48-dbb9-3674-dfbc4aea2967@amd.com/raw

and especially your From: field:

From:   S@vger.kernel.org

Is that your email address?

--
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-16 15:14                 ` S
@ 2019-01-16 15:26                   ` Borislav Petkov
  2019-01-16 15:31                     ` S, Shirish
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2019-01-16 15:26 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh

On Wed, Jan 16, 2019 at 03:14:29PM +0000, S wrote:
> Done. Hope the mailer is fine now, i have added --from while sending the 
> new patchset and also got it reviewed from folks here.

No, it isn't. And the problem is not git. The problem is your normal emails
you're sending. Look at what you just sent:

https://lore.kernel.org/lkml/8bed39c8-df48-dbb9-3674-dfbc4aea2967@amd.com/raw

and especially your From: field:

From:   S@vger.kernel.org

Is that your email address?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-15 17:26               ` Borislav Petkov
@ 2019-01-16 15:14                 ` S
  2019-01-16 15:26                   ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: S @ 2019-01-16 15:14 UTC (permalink / raw)
  To: Borislav Petkov, S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh


On 1/15/2019 10:56 PM, Borislav Petkov wrote:
> On Tue, Jan 15, 2019 at 03:30:42AM +0000, S wrote:
>> Please talk to your colleagues how to fix that. Adding some of them to
>> CC.
> Your mailer is still broken. How do you manage to have an empty From:?
> Is your exchange client misconfigured?
>
>> Let me know if you want me to fix this and re-send the 3 patches.
> Well, please redo them by moving the quirk function to
>
>    arch/x86/kernel/cpu/mce/amd.c
>
> and call it from mce_amd_feature_init(). This will concentrate the code
> more and won't need the globally visible function.

Done. Hope the mailer is fine now, i have added --from while sending the 
new patchset and also got it reviewed from folks here.

Regards,

Shirish S

> Thx.
>
-- 
Regards,
Shirish S


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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
       [not found]             ` <4f8b532b-8613-feb5-7474-4bb0462cf1c7@amd.com>
@ 2019-01-15 17:26               ` Borislav Petkov
  2019-01-16 15:14                 ` S
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2019-01-15 17:26 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Lendacky, Thomas, Singh, Brijesh

On Tue, Jan 15, 2019 at 03:30:42AM +0000, S wrote:
> Please talk to your colleagues how to fix that. Adding some of them to
> CC.

Your mailer is still broken. How do you manage to have an empty From:?
Is your exchange client misconfigured?

> Let me know if you want me to fix this and re-send the 3 patches.

Well, please redo them by moving the quirk function to

  arch/x86/kernel/cpu/mce/amd.c

and call it from mce_amd_feature_init(). This will concentrate the code
more and won't need the globally visible function.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-14 15:26         ` S
@ 2019-01-14 15:32           ` Borislav Petkov
       [not found]             ` <4f8b532b-8613-feb5-7474-4bb0462cf1c7@amd.com>
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2019-01-14 15:32 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, Tom Lendacky, Brijesh Singh

On Mon, Jan 14, 2019 at 03:26:50PM +0000, S wrote:
> I will get back with a thorough confirmation from the hw folks.
> 
> Since Yazen is on vacation, i will consult with him too once he is back.
> 
> Thanks for all the comments, i shall implement the same in case there 
> would be another re-spin.

Ok, thx.

> > Btw, fix your mailer: It has an empty from:
> >
> > From: S
> >
> > which, when I do "reply-to-all" here, generates an empty To: too.

Btw, your mailer is still broken. What I see here is:

From: S
To: Borislav Petkov <bp@alien8.de>, "S, Shirish" <Shirish.S@amd.com>
CC: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
	...


Please talk to your colleagues how to fix that. Adding some of them to
CC.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-14 10:33       ` Borislav Petkov
@ 2019-01-14 15:26         ` S
  2019-01-14 15:32           ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: S @ 2019-01-14 15:26 UTC (permalink / raw)
  To: Borislav Petkov, S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE


On 1/14/2019 4:03 PM, Borislav Petkov wrote:
> On Thu, Jan 10, 2019 at 08:04:49AM +0000, S wrote:
>> Perhaps no need to check the model altogether, this is applicable to
>> entire family 15, since there is no 8th Gen.
> I wouldn't be so sure. At the time I did this, we excluded models >=
> 0x10 but I don't remember why anymore. You need to check with hw folks
> whether you can really do this for the whole family.

I will get back with a thorough confirmation from the hw folks.

Since Yazen is on vacation, i will consult with him too once he is back.

Thanks for all the comments, i shall implement the same in case there 
would be another re-spin.

Regards,

Shirish S

>> I have summarized this sequence after capturing the call trace in both
>> scenarios.
>>
>> Its clear that the quirks are not applied during S3-> S0 transition,
>> hence the patch.
> Ok.
>
> Btw, fix your mailer: It has an empty from:
>
> From: S
>
> which, when I do "reply-to-all" here, generates an empty To: too.
>
> Thx.
>
-- 
Regards,
Shirish S


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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-10  8:04     ` S
@ 2019-01-14 10:33       ` Borislav Petkov
  2019-01-14 15:26         ` S
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2019-01-14 10:33 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE

On Thu, Jan 10, 2019 at 08:04:49AM +0000, S wrote:
> Perhaps no need to check the model altogether, this is applicable to 
> entire family 15, since there is no 8th Gen.

I wouldn't be so sure. At the time I did this, we excluded models >=
0x10 but I don't remember why anymore. You need to check with hw folks
whether you can really do this for the whole family.

> I have summarized this sequence after capturing the call trace in both 
> scenarios.
> 
> Its clear that the quirks are not applied during S3-> S0 transition, 
> hence the patch.

Ok.

Btw, fix your mailer: It has an empty from:

From: S

which, when I do "reply-to-all" here, generates an empty To: too.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-09 22:08   ` Borislav Petkov
@ 2019-01-10  8:04     ` S
  2019-01-14 10:33       ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: S @ 2019-01-10  8:04 UTC (permalink / raw)
  To: Borislav Petkov, S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE

Hi Borislav,

On 1/10/2019 3:38 AM, Borislav Petkov wrote:
> On Mon, Jan 07, 2019 at 10:37:24AM +0000, S, Shirish wrote:
>> This patch adds threshold quirk applicable for family 15
> Same issue with "This patch" here.
Agree, have avoided the phrase in both patches that originate from this 
discussion.
>
>> in resume path as well, since mce_amd_feature_init()
>> does not have quirks applied when originating from mce_syscore_resume(),
>> resulting in the below message at every successful resume:
>>
>> "[Firmware Bug]: cpu 0, invalid threshold interrupt offset ..."
>>
>> Signed-off-by: Shirish S <shirish.s@amd.com>
>> ---
>>   arch/x86/kernel/cpu/mce/amd.c | 34 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
>> index 89298c8..27cbf66 100644
>> --- a/arch/x86/kernel/cpu/mce/amd.c
>> +++ b/arch/x86/kernel/cpu/mce/amd.c
>> @@ -545,6 +545,34 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
>>   	return offset;
>>   }
>>   
>> +void disable_err_thresholding(struct cpuinfo_x86 *c)
>> +{
>> +	int i;
>> +	u64 hwcr;
>> +	bool need_toggle;
>> +	u32 msrs[] = {
>> +		0x00000413, /* MC4_MISC0 */
>> +		0xc0000408, /* MC4_MISC1 */
>> +	};
>> +
>> +	if (c->x86_model >= 0x10 && c->x86_model <= 0x7f) {
> You can save yourself an indentation level by reversing the logic here:
>
> 	if (c->x86 != 0x15)
> 		return;
>
> Also, I'm wondering if you simply can't do
>
> 	if (c->x86_model < 0x10)
> 		return;
>
> The assumption being that all the models - even after 0x7f - are highly
> unlikely to get MC4_MISC thresholding supported, all of a sudden. Might
> wanna run it by HW guys first though.

Perhaps no need to check the model altogether, this is applicable to 
entire family 15, since there is no 8th Gen.

>
>> +		rdmsrl(MSR_K7_HWCR, hwcr);
>> +
>> +		/* McStatusWrEn has to be set */
>> +		need_toggle = !(hwcr & BIT(18));
>> +
>> +		if (need_toggle)
>> +			wrmsrl(MSR_K7_HWCR, hwcr | BIT(18));
>> +
>> +		/* Clear CntP bit safely */
>> +		for (i = 0; i < ARRAY_SIZE(msrs); i++)
>> +			msr_clear_bit(msrs[i], 62);
>> +
>> +		/* restore old settings */
>> +		if (need_toggle)
>> +			wrmsrl(MSR_K7_HWCR, hwcr);
>> +	}
> So you copied the same code from __mcheck_cpu_apply_quirks().
>
> No.
>
> In a first patch, you carve that CntP clearing code in a separate
> function disable_err_thresholding() like you've done before.
Agree, first patch here:  https://lkml.org/lkml/2019/1/10/70
> Then, in a second patch, you call it from mce/amd.c and you move all the
> family/model checks inside the function so that you have a sole
>
> 	disable_err_thresholding();
>
> calls where you need them.
>
> You don't have to pass in struct cpuinfo_x86 *c - you can use
> boot_cpu_data in the function.
Agree, second patch here: https://lkml.org/lkml/2019/1/10/72
>
>> +}
>>   /* cpu init entry point, called from mce.c with preempt off */
>>   void mce_amd_feature_init(struct cpuinfo_x86 *c)
>>   {
>> @@ -552,6 +580,12 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
>>   	unsigned int bank, block, cpu = smp_processor_id();
>>   	int offset = -1;
>>   
>> +	/* Disable error thresholding bank in S3 resume path as well,
> What S3 resume path? That's the CPU init path.

I have also put in the commit message, but for discussion sake, there 
are 2 code paths leading to mce_amd_feature_init() as below.
1) S5 -> S0: (boot)
secondary_startup_64 -> start_kernel -> identify_boot_cpu ->
identify_cpu ->
mcheck_cpu_init (calls  __mcheck_cpu_apply_quirks before) ->
mce_amd_feature_init

2) S3 -> S0: (resume)
syscore_resume -> mce_syscore_resume -> mce_amd_feature_init

I have summarized this sequence after capturing the call trace in both 
scenarios.

Its clear that the quirks are not applied during S3-> S0 transition, 
hence the patch.

I cannot move the quirk to mce_amd_feature_init() since amd.c is guarded 
by CONFIG_X86_MCE_AMD and the quirk should be applicable in case the 
config is off via

__mcheck_cpu_apply_quirks() at boot.

> Also, kernel comments style is:
>
> 	/*
> 	 * A sentence ending with a full-stop.
> 	 * Another sentence. ...
> 	 * More sentences. ...
> 	 */
>
Agree, followed it in the above mentioned patches.

Thanks.

Regards,

Shirish S

> Good luck!
>
> :-)
>
-- 
Regards,
Shirish S


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

* Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-07 10:37 ` [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well S, Shirish
@ 2019-01-09 22:08   ` Borislav Petkov
  2019-01-10  8:04     ` S
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2019-01-09 22:08 UTC (permalink / raw)
  To: S, Shirish
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE

On Mon, Jan 07, 2019 at 10:37:24AM +0000, S, Shirish wrote:
> This patch adds threshold quirk applicable for family 15

Same issue with "This patch" here.

> in resume path as well, since mce_amd_feature_init()
> does not have quirks applied when originating from mce_syscore_resume(),
> resulting in the below message at every successful resume:
> 
> "[Firmware Bug]: cpu 0, invalid threshold interrupt offset ..."
> 
> Signed-off-by: Shirish S <shirish.s@amd.com>
> ---
>  arch/x86/kernel/cpu/mce/amd.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index 89298c8..27cbf66 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -545,6 +545,34 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
>  	return offset;
>  }
>  
> +void disable_err_thresholding(struct cpuinfo_x86 *c)
> +{
> +	int i;
> +	u64 hwcr;
> +	bool need_toggle;
> +	u32 msrs[] = {
> +		0x00000413, /* MC4_MISC0 */
> +		0xc0000408, /* MC4_MISC1 */
> +	};
> +
> +	if (c->x86_model >= 0x10 && c->x86_model <= 0x7f) {

You can save yourself an indentation level by reversing the logic here:

	if (c->x86 != 0x15)
		return;

Also, I'm wondering if you simply can't do

	if (c->x86_model < 0x10)
		return;

The assumption being that all the models - even after 0x7f - are highly
unlikely to get MC4_MISC thresholding supported, all of a sudden. Might
wanna run it by HW guys first though.

> +		rdmsrl(MSR_K7_HWCR, hwcr);
> +
> +		/* McStatusWrEn has to be set */
> +		need_toggle = !(hwcr & BIT(18));
> +
> +		if (need_toggle)
> +			wrmsrl(MSR_K7_HWCR, hwcr | BIT(18));
> +
> +		/* Clear CntP bit safely */
> +		for (i = 0; i < ARRAY_SIZE(msrs); i++)
> +			msr_clear_bit(msrs[i], 62);
> +
> +		/* restore old settings */
> +		if (need_toggle)
> +			wrmsrl(MSR_K7_HWCR, hwcr);
> +	}

So you copied the same code from __mcheck_cpu_apply_quirks().

No.

In a first patch, you carve that CntP clearing code in a separate
function disable_err_thresholding() like you've done before.

Then, in a second patch, you call it from mce/amd.c and you move all the
family/model checks inside the function so that you have a sole

	disable_err_thresholding();

calls where you need them.

You don't have to pass in struct cpuinfo_x86 *c - you can use
boot_cpu_data in the function.

> +}
>  /* cpu init entry point, called from mce.c with preempt off */
>  void mce_amd_feature_init(struct cpuinfo_x86 *c)
>  {
> @@ -552,6 +580,12 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
>  	unsigned int bank, block, cpu = smp_processor_id();
>  	int offset = -1;
>  
> +	/* Disable error thresholding bank in S3 resume path as well,

What S3 resume path? That's the CPU init path.

Also, kernel comments style is:

	/*
	 * A sentence ending with a full-stop.
	 * Another sentence. ...
	 * More sentences. ...
	 */


Good luck!

:-)

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well
  2019-01-07 10:37 [PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models S, Shirish
@ 2019-01-07 10:37 ` S, Shirish
  2019-01-09 22:08   ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: S, Shirish @ 2019-01-07 10:37 UTC (permalink / raw)
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
	maintainer : X86 ARCHITECTURE, Tony Luck, Vishal Verma,
	open list : X86 ARCHITECTURE, S, Shirish

This patch adds threshold quirk applicable for family 15
in resume path as well, since mce_amd_feature_init()
does not have quirks applied when originating from mce_syscore_resume(),
resulting in the below message at every successful resume:

"[Firmware Bug]: cpu 0, invalid threshold interrupt offset ..."

Signed-off-by: Shirish S <shirish.s@amd.com>
---
 arch/x86/kernel/cpu/mce/amd.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 89298c8..27cbf66 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -545,6 +545,34 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
 	return offset;
 }
 
+void disable_err_thresholding(struct cpuinfo_x86 *c)
+{
+	int i;
+	u64 hwcr;
+	bool need_toggle;
+	u32 msrs[] = {
+		0x00000413, /* MC4_MISC0 */
+		0xc0000408, /* MC4_MISC1 */
+	};
+
+	if (c->x86_model >= 0x10 && c->x86_model <= 0x7f) {
+		rdmsrl(MSR_K7_HWCR, hwcr);
+
+		/* McStatusWrEn has to be set */
+		need_toggle = !(hwcr & BIT(18));
+
+		if (need_toggle)
+			wrmsrl(MSR_K7_HWCR, hwcr | BIT(18));
+
+		/* Clear CntP bit safely */
+		for (i = 0; i < ARRAY_SIZE(msrs); i++)
+			msr_clear_bit(msrs[i], 62);
+
+		/* restore old settings */
+		if (need_toggle)
+			wrmsrl(MSR_K7_HWCR, hwcr);
+	}
+}
 /* cpu init entry point, called from mce.c with preempt off */
 void mce_amd_feature_init(struct cpuinfo_x86 *c)
 {
@@ -552,6 +580,12 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
 	unsigned int bank, block, cpu = smp_processor_id();
 	int offset = -1;
 
+	/* Disable error thresholding bank in S3 resume path as well,
+	 * for 15h family
+	 */
+	if (c->x86 == 0x15)
+		disable_err_thresholding(c);
+
 	for (bank = 0; bank < mca_cfg.banks; ++bank) {
 		if (mce_flags.smca)
 			smca_configure(bank, cpu);
-- 
2.7.4


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

end of thread, other threads:[~2019-01-16 15:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1546594609-22403-1-git-send-email-shirish.s@amd.com>
2019-01-04  9:37 ` [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models S, Shirish
2019-01-04  9:37 ` [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well S, Shirish
2019-01-07 10:37 [PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models S, Shirish
2019-01-07 10:37 ` [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well S, Shirish
2019-01-09 22:08   ` Borislav Petkov
2019-01-10  8:04     ` S
2019-01-14 10:33       ` Borislav Petkov
2019-01-14 15:26         ` S
2019-01-14 15:32           ` Borislav Petkov
     [not found]             ` <4f8b532b-8613-feb5-7474-4bb0462cf1c7@amd.com>
2019-01-15 17:26               ` Borislav Petkov
2019-01-16 15:14                 ` S
2019-01-16 15:26                   ` Borislav Petkov
2019-01-16 15:31                     ` S, Shirish
2019-01-16 15:36                       ` S, Shirish
2019-01-16 15:38                         ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®