lkml.org 
[lkml]   [2020]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5] cper, apei, mce: Pass x86 CPER through the MCA handling chain
From
Date
On 11/6/20 6:09 AM, Borislav Petkov wrote:

> On Tue, Nov 03, 2020 at 10:49:52AM -0600, Smita Koralahalli wrote:
>> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
>> index af8d37962586..f56f0bc147e2 100644
>> --- a/arch/x86/kernel/cpu/mce/apei.c
>> +++ b/arch/x86/kernel/cpu/mce/apei.c
>> @@ -51,6 +51,62 @@ void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
>> }
>> EXPORT_SYMBOL_GPL(apei_mce_report_mem_error);
>>
>> +int apei_smca_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
>> +{
>> + const u64 *i_mce = ((const u64 *) (ctx_info + 1));
>> + unsigned int cpu;
>> + struct mce m;
>> +
>> + if (!boot_cpu_has(X86_FEATURE_SMCA))
>> + return -EINVAL;
>> +
>> + /*
>> + * The starting address of the Register Array extracted from BERT
>> + * must match with the first expected register in the register
>> + * layout of MCAX address space. In SMCA systems this address
>> + * corresponds to banks's MCA_STATUS register.
> So which is it "MCAX" or "SMCA"? They both denote the same thing but
> let's stick to one to avoid unnecessary confusion. I'm guessing to
> "SMCA" because it is more wide-spread in the kernel...

Okay I will change it to SMCA.

>> + *
>> + * The Register array size must be large enough to include all
>> + * the SMCA registers which we want to extract.
>> + *
>> + * The number of registers in the Register Array is determined
>> + * by Register Array Size/8 as defined in UEFI spec v2.8, sec
>> + * N.2.4.2.2. The register layout is fixed and currently the raw
>> + * data in the register array includes 6 SMCA registers which the
>> + * kernel can extract.
>> + */
>> +
>> + if ((ctx_info->msr_addr & MSR_AMD64_SMCA_MC0_STATUS) !=
>> + MSR_AMD64_SMCA_MC0_STATUS || ctx_info->reg_arr_size < 48)
>> + return -EINVAL;
> Split that if in two consecutive if-statements.

Okay.

>
> Also, why the ANDing and not simply do:
>
> if (ctx_info->msr_addr == MSR_AMD64_SMCA_MC0_STATUS)
>
> ?
>
> I'm guessing you wanna match *all* MCi_STATUS MSRs - not only MC0, yes?
>
> If so, document that with in the comment above it.
>
> Thx.

ANDing with MC0 avoids bank check by turning off the bits corresponding
to the bank number.

For example: MCA_STATUS in SMCA space is 0xC0002YY1 where YY is the bank
number. The bank number is "Dont care" so ANDing with MC0_STATUS
(0xC0002001) should match on any MCi_STATUS MSR.

I will include that in comments above it.

Thanks,

>

\
 
 \ /
  Last update: 2020-11-09 19:38    [W:1.012 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site