linux-kernel - Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca

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: <CAGXwabSySjX0cU4we5v85KDv4C08iSQrQHTWQ6cD1qJ3N7zENw@mail.gmail.com>
Date:   Fri, 30 Jun 2017 10:57:57 -0500
From:   Jack Miller <jack@...ezen.org>
To:     Yazen Ghannam <Yazen.Ghannam@....com>
Cc:     linux-edac@...r.kernel.org, Borislav Petkov <bp@...e.de>,
        Tony Luck <tony.luck@...el.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Jack Miller <jack@...ezen.org>
Subject: Re: [PATCH] x86/mce/AMD: Allow any CPU to initialize smca_banks array

This patch works for me, thanks!

Jack

On Thu, Jun 29, 2017 at 1:08 PM, Yazen Ghannam <Yazen.Ghannam@....com> wrote:
> From: Yazen Ghannam <yazen.ghannam@....com>
>
> Current SMCA implementations have the same banks on each CPU with the
> non-core banks only visible to a "master thread" on each Die. Practically,
> this means the smca_banks array, which describes the banks, only needs to
> be populated once by a single master thread.
>
> CPU0 seemed like a good candidate to do the populating. However, it's
> possible that CPU0 is not enabled in which case the smca_banks array won't
> be populated.
>
> Rather than try to figure out another master thread to do the populating,
> we should just allow any CPU to populate the array.
>
> Drop the CPU0 check and return early if the bank was already initialized.
> Also, drop the WARNing about an already initialized bank, since this will
> be a common, expected occurrence.
>
> The smca_banks array is only populated at boot time and CPUs are brought
> online sequentially. So there's no need for locking around the array.
>
> If the first CPU up is a master thread, then it will populate the array
> with all banks, core and non-core. Every CPU afterwards will return early.
>
> If the first CPU up is not a master thread, then it will populate the array
> with all core banks. The first CPU afterwards that is a master thread will
> skip populating the core banks and continue populating the non-core banks.
> Every CPU afterwards will then return early.
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> ---
>  arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index b41466723f71..37dc43f820b4 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -214,8 +214,8 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>                 wrmsr(smca_config, low, high);
>         }
>
> -       /* Collect bank_info using CPU 0 for now. */
> -       if (cpu)
> +       /* Return early if this bank was already initialized. */
> +       if (smca_banks[bank].hwid)
>                 return;
>
>         if (rdmsr_safe_on_cpu(cpu, MSR_AMD64_SMCA_MCx_IPID(bank), &low, &high)) {
> @@ -229,11 +229,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
>         for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
>                 s_hwid = &smca_hwid_mcatypes[i];
>                 if (hwid_mcatype == s_hwid->hwid_mcatype) {
> -
> -                       WARN(smca_banks[bank].hwid,
> -                            "Bank %s already initialized!\n",
> -                            smca_get_name(s_hwid->bank_type));
> -
>                         smca_banks[bank].hwid = s_hwid;
>                         smca_banks[bank].id = low;
>                         smca_banks[bank].sysfs_id = s_hwid->count++;
> --
> 2.7.4
>