All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7] x86/mce: retrieve poison range from hardware
@ 2022-08-02 19:50 Jane Chu
  2022-08-03  8:53 ` Ingo Molnar
  2022-08-23 16:51 ` Borislav Petkov
  0 siblings, 2 replies; 12+ messages in thread
From: Jane Chu @ 2022-08-02 19:50 UTC (permalink / raw)
  To: tony.luck, bp, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
poison granularity") that changed nfit_handle_mce() callback to report
badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
2 back-to-back poisons and the driver ends up logging 8 badblocks,
because 0x1000 bytes is 8 512-byte.

Dan Williams noticed that apei_mce_report_mem_error() hardcode
the LSB field to PAGE_SHIFT instead of consulting the input
struct cper_sec_mem_err record.  So change to rely on hardware whenever
support is available.

Link: https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Jane Chu <jane.chu@oracle.com>
---
 arch/x86/kernel/cpu/mce/apei.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
index 717192915f28..8ed341714686 100644
--- a/arch/x86/kernel/cpu/mce/apei.c
+++ b/arch/x86/kernel/cpu/mce/apei.c
@@ -29,15 +29,26 @@
 void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
 {
 	struct mce m;
+	int lsb;
 
 	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
 
+	/*
+	 * Even if the ->validation_bits are set for address mask,
+	 * to be extra safe, check and reject an error radius '0',
+	 * and fall back to the default page size.
+	 */
+	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
+		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
+	else
+		lsb = PAGE_SHIFT;
+
 	mce_setup(&m);
 	m.bank = -1;
 	/* Fake a memory read error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
-	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
+	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
 
 	if (severity >= GHES_SEV_RECOVERABLE)
 		m.status |= MCI_STATUS_UC;
-- 
2.18.4


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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-02 19:50 [PATCH v7] x86/mce: retrieve poison range from hardware Jane Chu
@ 2022-08-03  8:53 ` Ingo Molnar
  2022-08-08 20:58   ` Jane Chu
  2022-08-23 16:51 ` Borislav Petkov
  1 sibling, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2022-08-03  8:53 UTC (permalink / raw)
  To: Jane Chu
  Cc: tony.luck, bp, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm


* Jane Chu <jane.chu@oracle.com> wrote:

> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine

s/Commit/commit

> poison granularity") that changed nfit_handle_mce() callback to report
> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
> because 0x1000 bytes is 8 512-byte.
> 
> Dan Williams noticed that apei_mce_report_mem_error() hardcode
> the LSB field to PAGE_SHIFT instead of consulting the input
> struct cper_sec_mem_err record.  So change to rely on hardware whenever
> support is available.
> 
> Link: https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Jane Chu <jane.chu@oracle.com>
> ---
>  arch/x86/kernel/cpu/mce/apei.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
> index 717192915f28..8ed341714686 100644
> --- a/arch/x86/kernel/cpu/mce/apei.c
> +++ b/arch/x86/kernel/cpu/mce/apei.c
> @@ -29,15 +29,26 @@
>  void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
>  {
>  	struct mce m;
> +	int lsb;
>  
>  	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
>  		return;
>  
> +	/*
> +	 * Even if the ->validation_bits are set for address mask,
> +	 * to be extra safe, check and reject an error radius '0',
> +	 * and fall back to the default page size.
> +	 */
> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
> +		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
> +	else
> +		lsb = PAGE_SHIFT;
> +
>  	mce_setup(&m);
>  	m.bank = -1;
>  	/* Fake a memory read error with unknown channel */
>  	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
> -	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
> +	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;

LGTM.

I suppose this wants to go upstream via the tree the bug came from (NVDIMM 
tree? ACPI tree?), or should we pick it up into the x86 tree?

Thanks,

	Ingo

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-03  8:53 ` Ingo Molnar
@ 2022-08-08 20:58   ` Jane Chu
  2022-08-08 23:30     ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Jane Chu @ 2022-08-08 20:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: tony.luck, bp, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

On 8/3/2022 1:53 AM, Ingo Molnar wrote:
> 
> * Jane Chu <jane.chu@oracle.com> wrote:
> 
>> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
> 
> s/Commit/commit

Maintainers,
Would you prefer a v8, or take care the comment upon accepting the patch?

> 
>> poison granularity") that changed nfit_handle_mce() callback to report
>> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
>> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
>> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
>> because 0x1000 bytes is 8 512-byte.
>>
>> Dan Williams noticed that apei_mce_report_mem_error() hardcode
>> the LSB field to PAGE_SHIFT instead of consulting the input
>> struct cper_sec_mem_err record.  So change to rely on hardware whenever
>> support is available.
>>
>> Link: https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com
>>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Ingo Molnar <mingo@kernel.org>
>> Signed-off-by: Jane Chu <jane.chu@oracle.com>
>> ---
>>   arch/x86/kernel/cpu/mce/apei.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
>> index 717192915f28..8ed341714686 100644
>> --- a/arch/x86/kernel/cpu/mce/apei.c
>> +++ b/arch/x86/kernel/cpu/mce/apei.c
>> @@ -29,15 +29,26 @@
>>   void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
>>   {
>>   	struct mce m;
>> +	int lsb;
>>   
>>   	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
>>   		return;
>>   
>> +	/*
>> +	 * Even if the ->validation_bits are set for address mask,
>> +	 * to be extra safe, check and reject an error radius '0',
>> +	 * and fall back to the default page size.
>> +	 */
>> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
>> +		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
>> +	else
>> +		lsb = PAGE_SHIFT;
>> +
>>   	mce_setup(&m);
>>   	m.bank = -1;
>>   	/* Fake a memory read error with unknown channel */
>>   	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
>> -	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
>> +	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
> 
> LGTM.
> 
> I suppose this wants to go upstream via the tree the bug came from (NVDIMM
> tree? ACPI tree?), or should we pick it up into the x86 tree?

No idea.  Maintainers?

thanks!
-jane

> 
> Thanks,
> 
> 	Ingo


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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-08 20:58   ` Jane Chu
@ 2022-08-08 23:30     ` Dan Williams
  2022-08-23 16:38       ` Jane Chu
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2022-08-08 23:30 UTC (permalink / raw)
  To: Jane Chu, Ingo Molnar
  Cc: tony.luck, bp, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

Jane Chu wrote:
> On 8/3/2022 1:53 AM, Ingo Molnar wrote:
> > 
> > * Jane Chu <jane.chu@oracle.com> wrote:
> > 
> >> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
> > 
> > s/Commit/commit
> 
> Maintainers,
> Would you prefer a v8, or take care the comment upon accepting the patch?
> 
> > 
> >> poison granularity") that changed nfit_handle_mce() callback to report
> >> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
> >> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
> >> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
> >> because 0x1000 bytes is 8 512-byte.
> >>
> >> Dan Williams noticed that apei_mce_report_mem_error() hardcode
> >> the LSB field to PAGE_SHIFT instead of consulting the input
> >> struct cper_sec_mem_err record.  So change to rely on hardware whenever
> >> support is available.
> >>
> >> Link: https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com
> >>
> >> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> >> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> >> Signed-off-by: Jane Chu <jane.chu@oracle.com>
> >> ---
> >>   arch/x86/kernel/cpu/mce/apei.c | 13 ++++++++++++-
> >>   1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
> >> index 717192915f28..8ed341714686 100644
> >> --- a/arch/x86/kernel/cpu/mce/apei.c
> >> +++ b/arch/x86/kernel/cpu/mce/apei.c
> >> @@ -29,15 +29,26 @@
> >>   void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
> >>   {
> >>   	struct mce m;
> >> +	int lsb;
> >>   
> >>   	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
> >>   		return;
> >>   
> >> +	/*
> >> +	 * Even if the ->validation_bits are set for address mask,
> >> +	 * to be extra safe, check and reject an error radius '0',
> >> +	 * and fall back to the default page size.
> >> +	 */
> >> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
> >> +		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
> >> +	else
> >> +		lsb = PAGE_SHIFT;
> >> +
> >>   	mce_setup(&m);
> >>   	m.bank = -1;
> >>   	/* Fake a memory read error with unknown channel */
> >>   	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
> >> -	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
> >> +	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
> > 
> > LGTM.
> > 
> > I suppose this wants to go upstream via the tree the bug came from (NVDIMM
> > tree? ACPI tree?), or should we pick it up into the x86 tree?
> 
> No idea.  Maintainers?

There's no real NVDIMM dependency here, just a general cleanup of how
APEI error granularities are managed. So I think it is appropriate for
this to go through the x86 tree via the typical path for mce related
topics.

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-08 23:30     ` Dan Williams
@ 2022-08-23 16:38       ` Jane Chu
  0 siblings, 0 replies; 12+ messages in thread
From: Jane Chu @ 2022-08-23 16:38 UTC (permalink / raw)
  To: Dan Williams, Ingo Molnar, ying.huang, Luck, Tony
  Cc: bp, tglx, mingo, dave.hansen, x86, linux-edac, linux-kernel, hch, nvdimm

 >>> I suppose this wants to go upstream via the tree the bug came from 
(NVDIMM
 >>> tree? ACPI tree?), or should we pick it up into the x86 tree?
 >>
 >> No idea.  Maintainers?
 >
 > There's no real NVDIMM dependency here, just a general cleanup of how
 > APEI error granularities are managed. So I think it is appropriate for
 > this to go through the x86 tree via the typical path for mce related
 > topics.

+ Huang, Ying.

x86 maintainers,

Please let me know if you need another revision.

thanks,
-jane


On 8/8/2022 4:30 PM, Dan Williams wrote:
> Jane Chu wrote:
>> On 8/3/2022 1:53 AM, Ingo Molnar wrote:
>>>
>>> * Jane Chu <jane.chu@oracle.com> wrote:
>>>
>>>> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
>>>
>>> s/Commit/commit
>>
>> Maintainers,
>> Would you prefer a v8, or take care the comment upon accepting the patch?
>>
>>>
>>>> poison granularity") that changed nfit_handle_mce() callback to report
>>>> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
>>>> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
>>>> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
>>>> because 0x1000 bytes is 8 512-byte.
>>>>
>>>> Dan Williams noticed that apei_mce_report_mem_error() hardcode
>>>> the LSB field to PAGE_SHIFT instead of consulting the input
>>>> struct cper_sec_mem_err record.  So change to rely on hardware whenever
>>>> support is available.
>>>>
>>>> Link: https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com
>>>>
>>>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>>>> Reviewed-by: Ingo Molnar <mingo@kernel.org>
>>>> Signed-off-by: Jane Chu <jane.chu@oracle.com>
>>>> ---
>>>>    arch/x86/kernel/cpu/mce/apei.c | 13 ++++++++++++-
>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
>>>> index 717192915f28..8ed341714686 100644
>>>> --- a/arch/x86/kernel/cpu/mce/apei.c
>>>> +++ b/arch/x86/kernel/cpu/mce/apei.c
>>>> @@ -29,15 +29,26 @@
>>>>    void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
>>>>    {
>>>>    	struct mce m;
>>>> +	int lsb;
>>>>    
>>>>    	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
>>>>    		return;
>>>>    
>>>> +	/*
>>>> +	 * Even if the ->validation_bits are set for address mask,
>>>> +	 * to be extra safe, check and reject an error radius '0',
>>>> +	 * and fall back to the default page size.
>>>> +	 */
>>>> +	if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK)
>>>> +		lsb = find_first_bit((void *)&mem_err->physical_addr_mask, PAGE_SHIFT);
>>>> +	else
>>>> +		lsb = PAGE_SHIFT;
>>>> +
>>>>    	mce_setup(&m);
>>>>    	m.bank = -1;
>>>>    	/* Fake a memory read error with unknown channel */
>>>>    	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | MCI_STATUS_MISCV | 0x9f;
>>>> -	m.misc = (MCI_MISC_ADDR_PHYS << 6) | PAGE_SHIFT;
>>>> +	m.misc = (MCI_MISC_ADDR_PHYS << 6) | lsb;
>>>
>>> LGTM.
>>>
>>> I suppose this wants to go upstream via the tree the bug came from (NVDIMM
>>> tree? ACPI tree?), or should we pick it up into the x86 tree?
>>
>> No idea.  Maintainers?
> 
> There's no real NVDIMM dependency here, just a general cleanup of how
> APEI error granularities are managed. So I think it is appropriate for
> this to go through the x86 tree via the typical path for mce related
> topics.


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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-02 19:50 [PATCH v7] x86/mce: retrieve poison range from hardware Jane Chu
  2022-08-03  8:53 ` Ingo Molnar
@ 2022-08-23 16:51 ` Borislav Petkov
  2022-08-23 16:58   ` Luck, Tony
  2022-08-25 16:29   ` Jane Chu
  1 sibling, 2 replies; 12+ messages in thread
From: Borislav Petkov @ 2022-08-23 16:51 UTC (permalink / raw)
  To: Jane Chu
  Cc: tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

On Tue, Aug 02, 2022 at 01:50:53PM -0600, Jane Chu wrote:
> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
> poison granularity") that changed nfit_handle_mce() callback to report
> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
> because 0x1000 bytes is 8 512-byte.

What I'm missing from this text here is, what *is* the mce->misc LSB
field in human speak? What does that field denote?

What effect does that field have on error injection?

And so on.

> Dan Williams noticed that apei_mce_report_mem_error() hardcode
> the LSB field to PAGE_SHIFT instead of consulting the input
> struct cper_sec_mem_err record.  So change to rely on hardware whenever
> support is available.

Rely on hardware? You're changing this to rely on what the firmware
reports.

That mem_err thing comes from a BIOS table AFAICT.

...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* RE: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-23 16:51 ` Borislav Petkov
@ 2022-08-23 16:58   ` Luck, Tony
  2022-08-25 16:29   ` Jane Chu
  1 sibling, 0 replies; 12+ messages in thread
From: Luck, Tony @ 2022-08-23 16:58 UTC (permalink / raw)
  To: Borislav Petkov, chu, jane
  Cc: tglx, mingo, dave.hansen, x86, linux-edac, Williams, Dan J,
	linux-kernel, hch, nvdimm

> What I'm missing from this text here is, what *is* the mce->misc LSB
> field in human speak? What does that field denote?

The SDM says:

 Recoverable Address LSB (bits 5:0): The lowest valid recoverable address bit. Indicates the position of the least
 significant bit (LSB) of the recoverable error address. For example, if the processor logs bits [43:9] of the
 address, the LSB sub-field in IA32_MCi_MISC is 01001b (9 decimal). For this example, bits [8:0] of the
 recoverable error address in IA32_MCi_ADDR should be ignored.

So in human speak "how much data did you lose". "6" is a common value saying a cache line (2<<6 == 64)
was lost. Sometimes you see "12' (2<<12 == 4096) for a whole page lost.

-Tony

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-23 16:51 ` Borislav Petkov
  2022-08-23 16:58   ` Luck, Tony
@ 2022-08-25 16:29   ` Jane Chu
  2022-08-25 22:53     ` Borislav Petkov
  1 sibling, 1 reply; 12+ messages in thread
From: Jane Chu @ 2022-08-25 16:29 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

On 8/23/2022 9:51 AM, Borislav Petkov wrote:
> On Tue, Aug 02, 2022 at 01:50:53PM -0600, Jane Chu wrote:
>> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
>> poison granularity") that changed nfit_handle_mce() callback to report
>> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
>> discovered that the mce->misc LSB field is 0x1000 bytes, hence injecting
>> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
>> because 0x1000 bytes is 8 512-byte.
> 
> What I'm missing from this text here is, what *is* the mce->misc LSB
> field in human speak? What does that field denote?
> 
> What effect does that field have on error injection
Tony has replied.

> 
> And so on.
> 
>> Dan Williams noticed that apei_mce_report_mem_error() hardcode
>> the LSB field to PAGE_SHIFT instead of consulting the input
>> struct cper_sec_mem_err record.  So change to rely on hardware whenever
>> support is available.
> 
> Rely on hardware? You're changing this to rely on what the firmware
> reports.
> 
> That mem_err thing comes from a BIOS table AFAICT.
> 

Would fix the comment to indicate "relying on firmware" help?
Is there other concern?

thanks!
-jane

> ...
> 
> Thx.
> 


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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-25 16:29   ` Jane Chu
@ 2022-08-25 22:53     ` Borislav Petkov
  2022-08-26 17:54       ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2022-08-25 22:53 UTC (permalink / raw)
  To: Jane Chu
  Cc: tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

On Thu, Aug 25, 2022 at 04:29:47PM +0000, Jane Chu wrote:
> Tony has replied.

Do you really think that I can't look up what field means?

What I said was

"What I'm missing from this text here is... "

IOW, what I'm trying to say is, you should formulate your commit message
better, more human-friendly. Right now it reads like for insiders only.
But that's not its purpose.

Do you catch my drift?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-25 22:53     ` Borislav Petkov
@ 2022-08-26 17:54       ` Dan Williams
  2022-08-26 18:09         ` Borislav Petkov
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2022-08-26 17:54 UTC (permalink / raw)
  To: Borislav Petkov, Jane Chu
  Cc: tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	dan.j.williams, linux-kernel, hch, nvdimm

Borislav Petkov wrote:
> On Thu, Aug 25, 2022 at 04:29:47PM +0000, Jane Chu wrote:
> > Tony has replied.
> 
> Do you really think that I can't look up what field means?
> 
> What I said was
> 
> "What I'm missing from this text here is... "
> 
> IOW, what I'm trying to say is, you should formulate your commit message
> better, more human-friendly. Right now it reads like for insiders only.
> But that's not its purpose.
> 
> Do you catch my drift?

How about:

---

When memory poison consumption machine checks fire,
mce-notifier-handlers like nfit_handle_mce() record the impacted
physical address range. The error information includes data about blast
radius, i.e. how many cachelines did the hardware determine are
impacted. A recent change, commit 7917f9cdb503 ("acpi/nfit: rely on
mce->misc to determine poison granularity"), updated nfit_handle_mce()
to stop hard coding the blast radius value of 1 cacheline, and instead
rely on the blast radius reported in 'struct mce' which can be up to 4K
(64 cachelines).

It turns out that apei_mce_report_mem_error() had a similar problem in
that it hard coded a blast radius of 4K rather than checking the blast
radius in the error information. Fix apei_mce_report_mem_error() to
convey the proper poison granularity.

---

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-26 17:54       ` Dan Williams
@ 2022-08-26 18:09         ` Borislav Petkov
  2022-08-26 22:11           ` Jane Chu
  0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2022-08-26 18:09 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jane Chu, tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	linux-kernel, hch, nvdimm

On Fri, Aug 26, 2022 at 10:54:31AM -0700, Dan Williams wrote:
> How about:
> 
> ---
> 
> When memory poison consumption machine checks fire,
> mce-notifier-handlers like nfit_handle_mce() record the impacted
> physical address range.

... which is reported by the hardware in the MCi_MISC MSR.

> The error information includes data about blast
> radius, i.e. how many cachelines did the hardware determine are
> impacted.

Yap, nice.

> A recent change, commit 7917f9cdb503 ("acpi/nfit: rely on
> mce->misc to determine poison granularity"), updated nfit_handle_mce()
> to stop hard coding the blast radius value of 1 cacheline, and instead
> rely on the blast radius reported in 'struct mce' which can be up to 4K
> (64 cachelines).
> 
> It turns out that apei_mce_report_mem_error() had a similar problem in
> that it hard coded a blast radius of 4K rather than checking the blast

s/checking/reading/

> radius in the error information. Fix apei_mce_report_mem_error() to

s/in/from/

> convey the proper poison granularity.
> 
> ---

Yap, that's a lot better.

Thanks!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH v7] x86/mce: retrieve poison range from hardware
  2022-08-26 18:09         ` Borislav Petkov
@ 2022-08-26 22:11           ` Jane Chu
  0 siblings, 0 replies; 12+ messages in thread
From: Jane Chu @ 2022-08-26 22:11 UTC (permalink / raw)
  To: Borislav Petkov, Dan Williams
  Cc: tony.luck, tglx, mingo, dave.hansen, x86, linux-edac,
	linux-kernel, hch, nvdimm

On 8/26/2022 11:09 AM, Borislav Petkov wrote:
> On Fri, Aug 26, 2022 at 10:54:31AM -0700, Dan Williams wrote:
>> How about:
>>
>> ---
>>
>> When memory poison consumption machine checks fire,
>> mce-notifier-handlers like nfit_handle_mce() record the impacted
>> physical address range.
> 
> ... which is reported by the hardware in the MCi_MISC MSR.
> 
>> The error information includes data about blast
>> radius, i.e. how many cachelines did the hardware determine are
>> impacted.
> 
> Yap, nice.
> 
>> A recent change, commit 7917f9cdb503 ("acpi/nfit: rely on
>> mce->misc to determine poison granularity"), updated nfit_handle_mce()
>> to stop hard coding the blast radius value of 1 cacheline, and instead
>> rely on the blast radius reported in 'struct mce' which can be up to 4K
>> (64 cachelines).
>>
>> It turns out that apei_mce_report_mem_error() had a similar problem in
>> that it hard coded a blast radius of 4K rather than checking the blast
> 
> s/checking/reading/
> 
>> radius in the error information. Fix apei_mce_report_mem_error() to
> 
> s/in/from/
> 
>> convey the proper poison granularity.
>>
>> ---
> 
> Yap, that's a lot better.
> 
> Thanks!


Got it and points taken.  Thank you both, Boris and Dan.

v8 coming up.

thanks,
-jane



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

end of thread, other threads:[~2022-08-26 22:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 19:50 [PATCH v7] x86/mce: retrieve poison range from hardware Jane Chu
2022-08-03  8:53 ` Ingo Molnar
2022-08-08 20:58   ` Jane Chu
2022-08-08 23:30     ` Dan Williams
2022-08-23 16:38       ` Jane Chu
2022-08-23 16:51 ` Borislav Petkov
2022-08-23 16:58   ` Luck, Tony
2022-08-25 16:29   ` Jane Chu
2022-08-25 22:53     ` Borislav Petkov
2022-08-26 17:54       ` Dan Williams
2022-08-26 18:09         ` Borislav Petkov
2022-08-26 22:11           ` Jane Chu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.