[PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[Chen Gong]

When GHES error record is logged into mcelog kernel buffer,
a validation check for physical address is necessary, which prevents
invalid physical address is happened in error record.

Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
 arch/x86/kernel/cpu/mcheck/mce-apei.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
index 507ea58..514b77f 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
@@ -49,7 +49,8 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 	m.bank = 1;
 	/* Fake a memory read corrected error with unknown channel */
 	m.status = MCI_STATUS_VAL | MCI_STATUS_EN | MCI_STATUS_ADDRV | 0x9f;
-	m.addr = mem_err->physical_addr;
+	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
+		m.addr = mem_err->physical_addr;
 	mce_log(&m);
 	mce_notify_irq();
 }
-- 
1.7.10.rc2.27.g59012

RE: [PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[Luck, Tony]

+	if (mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS)
+		m.addr = mem_err->physical_addr;

At the moment "addr" is the only useful value in this record ... so
if we find that we can't supply it because we don't know it - then the
record has no value, and we might as well not send it.

E.g. start the function with:

	if (!(mem_err->validation_bits & CPER_MEM_VALID_PHYSICAL_ADDRESS))
		return;

-Tony

Re: [PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[Chen Gong]

于 2012/3/30 1:07, Luck, Tony 写道:
> +	if (mem_err->validation_bits&  CPER_MEM_VALID_PHYSICAL_ADDRESS)
> +		m.addr = mem_err->physical_addr;
>
> At the moment "addr" is the only useful value in this record ... so
> if we find that we can't supply it because we don't know it - then the
> record has no value, and we might as well not send it.
>
> E.g. start the function with:
>
> 	if (!(mem_err->validation_bits&  CPER_MEM_VALID_PHYSICAL_ADDRESS))
> 		return;
>
> -Tony
>
But as Ying said before, in theory we need to support this scenario, 
otherwise, I prefer to
add check condition out of this function, not here.

What's your opinion, Ying?
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[Huang Ying]

On Sat, 2012-03-31 at 09:53 +0800, Chen Gong wrote:
> 于 2012/3/30 1:07, Luck, Tony 写道:
> > +	if (mem_err->validation_bits&  CPER_MEM_VALID_PHYSICAL_ADDRESS)
> > +		m.addr = mem_err->physical_addr;
> >
> > At the moment "addr" is the only useful value in this record ... so
> > if we find that we can't supply it because we don't know it - then the
> > record has no value, and we might as well not send it.
> >
> > E.g. start the function with:
> >
> > 	if (!(mem_err->validation_bits&  CPER_MEM_VALID_PHYSICAL_ADDRESS))
> > 		return;
> >
> > -Tony
> >
> But as Ying said before, in theory we need to support this scenario, 
> otherwise, I prefer to
> add check condition out of this function, not here.
> 
> What's your opinion, Ying?

Yes.  I have said that.  We need to report memory error even without
physical address.  But now I think maybe we need not to report that in
mce in addition to GHES.

So my current idea is as follow:

- For corrected memory error with address collected via GHES
  - report it in GHES and MCE (fake bank 1)

- Others
  - report it in GHES only

How to filter it is a coding style issue.  The filter can be in
ghes_do_proc() or apei_mce_report_mem_error().  Both are OK for me.
Personally, I prefer to filter it in apei_mce_report_mem_error().
Because it is a MCE trick/quirk instead of GHES trick/quirk.

Best Regards,
Huang Ying


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI, APEI: Add validation check before GHES error is recorded

[Chen Gong]

于 2012/3/31 11:24, Huang Ying 写道:
> How to filter it is a coding style issue.  The filter can be in
> ghes_do_proc() or apei_mce_report_mem_error().  Both are OK for me.
> Personally, I prefer to filter it in apei_mce_report_mem_error().
> Because it is a MCE trick/quirk instead of GHES trick/quirk.
Maybe some day channel information can be filtered so I will add it in

apei_mce_report_mem_error

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] ACPI, APEI: Add validation check before GHES error is recorded

[Chen Gong]

When GHES error record is logged into mcelog kernel buffer,
a validation check for physical address is necessary, which prevents
invalid physical address is happened in error record.

v2->v1:
  Only report error to mcelog kernel buffer when physical address is valid.

Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
 arch/x86/kernel/cpu/mcheck/mce-apei.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce-apei.c b/arch/x86/kernel/cpu/mcheck/mce-apei.c
index 507ea58..cd8b166 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-apei.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-apei.c
@@ -42,7 +42,8 @@ void apei_mce_report_mem_error(int corrected, struct cper_sec_mem_err *mem_err)
 	struct mce m;
 
 	/* Only corrected MC is reported */
-	if (!corrected)
+	if (!corrected || !(mem_err->validation_bits &
+				CPER_MEM_VALID_PHYSICAL_ADDRESS))
 		return;
 
 	mce_setup(&m);
-- 
1.7.10.rc2.27.g59012

RE: [PATCH V2] ACPI, APEI: Add validation check before GHES error is recorded

[Luck, Tony]

	/* Only corrected MC is reported */
-	if (!corrected)
+	if (!corrected || !(mem_err->validation_bits &
+				CPER_MEM_VALID_PHYSICAL_ADDRESS))
 		return;

Acked-by: Tony Luck <tony.luck@intel.com>