[PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models

[PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models

[S, Shirish]

This patch series applies to family 15 CPU's of AMD platforms,
so as to address a consistent warning of
 "[Firmware Bug]: cpu 0, invalid threshold interrupt offset"
at every boot and upon completiong of successful S3 cycle,
due to a missing quirk, which was not extended to newer models
and also not applied in resume path.

Shirish S (2):
  x86/mce/amd: Extend "Disable error thresholding bank 4" to more models
  x86/mce/amd: Ensure quirks are applied in resume path as well

 arch/x86/kernel/cpu/mce/amd.c  | 34 ++++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/mce/core.c |  2 +-
 2 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models

[S, Shirish]

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

Re: [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models

[Borislav Petkov]

On Mon, Jan 07, 2019 at 10:37:17AM +0000, S, Shirish wrote:
> 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,

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models

[S]

Hi Borislav,

On 1/10/2019 3:24 AM, Borislav Petkov wrote:
> On Mon, Jan 07, 2019 at 10:37:17AM +0000, S, Shirish wrote:
>> 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,
> Avoid having "This patch" or "This commit" in the commit message. It is
> tautologically useless.

Agree, have made the changes and re-spun re-spun v2 here: 
https://lkml.org/lkml/2019/1/10/69

I wanted to send the patch with you in recipient field rather than cc, 
unfortunately --cc remained in the git-send command

that i used, let me know i need to re-send it or you can see the patch.

Thanks.

Regards,

Shirish S

> Also, do
>
> $ git grep 'This patch' Documentation/process
>
> for more details.
>
-- 
Regards,
Shirish S

Re: [PATCH 1/2] x86/mce/amd: Extend "Disable error thresholding bank 4" to more models

[Borislav Petkov]

On Thu, Jan 10, 2019 at 08:02:47AM +0000, S wrote:
> I wanted to send the patch with you in recipient field rather than cc, 
> unfortunately --cc remained in the git-send command

Next time look at the manpage:

git send-email --to <recipient> --cc <other recipient> --suppress-cc=all

and the last switch stops git from adding more CCs.

Make sure you add "--dry-run" to check whether it adds the proper
CCs/Tos first and then send it for real.

-- 
Regards/Gruss,
    Boris.

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

[PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[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

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[S]

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

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[S]

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

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[S]

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

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

RE: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[S, Shirish]

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.

RE: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[S, Shirish]

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.

Re: [PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[Borislav Petkov]

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.

Re: [PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models

[Borislav Petkov]

On Mon, Jan 07, 2019 at 10:37:07AM +0000, S, Shirish wrote:
> This patch series applies to family 15 CPU's of AMD platforms,
> so as to address a consistent warning of
>  "[Firmware Bug]: cpu 0, invalid threshold interrupt offset"
> at every boot and upon completiong of successful S3 cycle,
> due to a missing quirk, which was not extended to newer models
> and also not applied in resume path.
> 
> Shirish S (2):
>   x86/mce/amd: Extend "Disable error thresholding bank 4" to more models
>   x86/mce/amd: Ensure quirks are applied in resume path as well
> 
>  arch/x86/kernel/cpu/mce/amd.c  | 34 ++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/mce/core.c |  2 +-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> -- 

You sent those once three days ago. Why are you sending them again?
There's no changelog to explain what the difference is from the last
submission so what's up?

Also, I'd suggest you have more patience. And while you wait, you can
take a look at

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

to read up on all the modalities when sending patches.

Thx.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 0/2] x86/mce/amd: apply missing quirks for family 15 models

[S]

On 1/7/2019 4:54 PM, Borislav Petkov wrote:
> On Mon, Jan 07, 2019 at 10:37:07AM +0000, S, Shirish wrote:
>> This patch series applies to family 15 CPU's of AMD platforms,
>> so as to address a consistent warning of
>>   "[Firmware Bug]: cpu 0, invalid threshold interrupt offset"
>> at every boot and upon completiong of successful S3 cycle,
>> due to a missing quirk, which was not extended to newer models
>> and also not applied in resume path.
>>
>> Shirish S (2):
>>    x86/mce/amd: Extend "Disable error thresholding bank 4" to more models
>>    x86/mce/amd: Ensure quirks are applied in resume path as well
>>
>>   arch/x86/kernel/cpu/mce/amd.c  | 34 ++++++++++++++++++++++++++++++++++
>>   arch/x86/kernel/cpu/mce/core.c |  2 +-
>>   2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> -- 
> You sent those once three days ago. Why are you sending them again?
> There's no changelog to explain what the difference is from the last
> submission so what's up?

I found that the get_maintainers.pl did not add anyone but me for the 
cover letter.

Hence i have re-sent the entire series along with cover letter so that the

reviewers would get context.

> Also, I'd suggest you have more patience.

Sure, I will wait, no worries.

Regards,

Shirish S

>   And while you wait, you can
> take a look at
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> to read up on all the modalities when sending patches.
>
> Thx.
>
-- 
Regards,
Shirish S

[PATCH 2/2] x86/mce/amd: Ensure quirks are applied in resume path as well

[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