Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] x86/MCE: Fix CPU microcode version output
To:     "Luck, Tony" <tony.luck@intel.com>
Cc:     linux-kernel@vger.kernel.org, Borislav Petkov <bp@alien8.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org,
        linux-edac@vger.kernel.org
References: <20180703164844.9742-1-prarit@redhat.com>
 <20180703165849.GA17773@agluck-desk>
From:   Prarit Bhargava <prarit@redhat.com>
Message-ID: <5ed43b85-a92f-8d6c-4565-d2e88d6d3f08@redhat.com>
Date:   Tue, 3 Jul 2018 16:08:41 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <20180703165849.GA17773@agluck-desk>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 07/03/2018 12:58 PM, Luck, Tony wrote:
> On Tue, Jul 03, 2018 at 12:48:44PM -0400, Prarit Bhargava wrote:
>> On systems where a runtime microcode update has occurred the
>> microcode version is wrong because boot_cpu_data.microcode is
>> not updated during runtime.
>>
>> Use the per-CPU microcode version in the MCE message.
> 
> Other option would be to fix the microcode update code to make
> sure that boot_cpu_data.microcode is right.

I thought about that too [1] but TBH I've always been a bit confused by
boot_cpu_data vs. cpu_data().  At one point I thought that boot_cpu_data is
valid only up to a specific point in boot and from that point on the cpu_data()
should be used but I'm no longer sure if that's correct.

If that is not correct when should boot_cpu_data be used vs cpu_data()?  Is
boot_cpu_data truly supposed to represent the general state of the boot cpu at
boot time?

Look at the same function mce_setup() that I modified in my patch.   There is
another reference to boot_cpu_data.x86_vendor (which obviously will never change
during runtime) that likely should be cpu_data() if my understanding above is
correct.

P.

[1] Since the kernel now uses a cpu rendezvous model to update the microcode it
would be trivial to set boot_cpu_data.microcode to the new value.

> 
> But if we go this route we should add:
> 
> Fixes: fa94d0c6e0f3 ("x86/MCE: Save microcode revision in machine check records")
> Cc: stable@vger.kernel.org
> 
> Can also get a:
> 
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> 
> -Tony Luck
>