Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 12 Jun 2023 17:42:46 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] x86/microcode: Add a "microcode=" command line option
Message-ID: <20230612154246.GLZIc89v6Q2THgsY8N@fat_crate.local>
References: <20230605141332.25948-1-bp@alien8.de>
 <20230605141332.25948-2-bp@alien8.de>
 <87ilbs7lcr.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <87ilbs7lcr.ffs@tglx>
Precedence: bulk

On Mon, Jun 12, 2023 at 05:26:28PM +0200, Thomas Gleixner wrote:
> Why is it suddenly required to prevent late loading on SMT threads?

The intent is, like a chicken bit, to revert to the *old* behavior which
would not load on both threads. In *case* some old configuration of CPU
and microcode cannot handle loading on both threads. Which is from
Bulldozer onwards but I don't think anyone uses Bulldozer anymore.

> That's the exact opposite of what e7ad18d1169c ("x86/microcode/AMD:
> Apply the patch early on every logical thread") is doing.

No, see patch 1 - it does exactly the same what this commit does but for
late loading.

Bottomline: on AMD, we should load on both threads by default.

> no_late_all is a horrible option name.

Yeah, at the time dis_ucode_ldr was horrible too. I tried harder this
time.

What do you suggest?

> Also the explanation is not mentioning that this is only relevant for
> late loading.

Huh, it has "late" in the name. :-)

> Aside of that why is this a kernel side chicken bit and not communicated
> by the microcode header?

See above. This chicken bit is there, just in case, to help in the case
where the user cannot do anything else. It should not be used, judging
by all the combinations I've tested here.

> How should an administrator know which microcode versions need this
> treatment and which do not? How is that supposed to work on a fleet?

None of them should need it. Thus, a chicken bit as a last option in
production.

> global variable name without a proper prefix. Moo.

Lemme fix.

> Where is the documentation which tells me what I'm supposed to write
> into this file? Also this is a generic file, right?

Lemme write some docs about it.

> So what's the meaning for non AMD? I can write this bit into it
> successfully and nothing happens, right?

Yes, this bit is AMD-only.

> Why ULL bits for a unsigned long variable?

There's no BIT_UL() macro.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette