linux-kernel - Re: [PATCH 1/2] x86/mce: Disable preemption for CPER decoding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e6b1af5a-774a-c3ef-223e-4595b7ec532a@amd.com>
Date:   Fri, 23 Jun 2023 09:51:47 -0400
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     "Luck, Tony" <tony.luck@...el.com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>
Cc:     yazen.ghannam@....com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH 1/2] x86/mce: Disable preemption for CPER decoding

On 6/22/2023 3:42 PM, Luck, Tony wrote:
>> 2) Split mce_setup() into global and per_cpu parts. The memset(), cpuid,
>> etc. would be global, and the cpu_data()* and rdmsr() would be per_cpu.
> 
> That sounds good. So global is:
> 
>          memset(m, 0, sizeof(struct mce));
>          /* need the internal __ version to avoid deadlocks */
>          m->time = __ktime_get_real_seconds();
>          m->cpuvendor = boot_cpu_data.x86_vendor;
>          m->mcgcap = __rdmsr(MSR_IA32_MCG_CAP);

MCG_CAP would be per_cpu, because the bank count can vary. But I don't 
think this matters in practice. So leaving it global is okay, I think.

>          m->microcode = boot_cpu_data.microcode;
>          m->cpuid = cpuid_eax(1);
> 
> Though that last one is perhaps per-cpu if you want to allow for mixed-stepping systems.
> Perhaps m->time also? Questionable whether it is useful to log time this record
> was created, when it refers to something much earlier in the BERT case.
>

I agree about m->time. It doesn't seem useful in this case.

But I don't know about m->cpuid. Mixing processor revisions is not 
allowed on AMD systems, and I don't know about other vendors. So I'd 
leave m->cpuid as global unless there's a strong case otherwise.

> and per-cpu is:
> 
>          m->cpu = m->extcpu = smp_processor_id();
>          m->socketid = cpu_data(m->extcpu).phys_proc_id;
>          m->apicid = cpu_data(m->extcpu).initial_apicid;
>          m->ppin = cpu_data(m->extcpu).ppin;
> 
>> Option #2 can also be used in apei_mce_report_mem_error(), I think.
> 
> Agreed.
> 

Okay, I'll update that too.

Thanks,
Yazen