Re: [PATCH 2/3] x86/amd: Drop vestigial PBE logic in init_amd()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/11/2025 8:44 am, Jan Beulich wrote:
> On 26.11.2025 18:15, Andrew Cooper wrote:
>> The comment was adjusted by myself in commit 51bd4bbdfdba ("x86: drop
>> X86_FEATURE_3DNOW_ALT"), on the presumption that the underlying logic existed
>> for a good reason.
>>
>> Having done further archaeology, it turns out to be vestigial technical debt
>> from the leadup to Linux 2.4 in November 2000.
>>
>> Prior to "Massive cleanup of CPU detection and bug handling",
>> c->x86_capability was a single uint32_t containing cpuid(1).edx,
>> cpuid(0x80000001).edx, or a synthesis thereof.  X86_FEATURE_AMD3D was defined
>> as the top bit this single uint32_t.
>>
>> After "Massive cleanup of CPU detection and bug handling",
>> c->x86_capability became an array with AMD's extended feature leaf split
>> away from Intel's basic feature leaf.
>>
>> AMD doc #20734-G states that 3DNow is only enumerated in the extended
>> feature leaf, and that other vendors where using this bit too.  i.e. AMD
>> never produced a CPU which set bit 31 in the basic leaf, meaning that
>> there's nothing to clear out in the first place.
>>
>> This logic looks like it was relevant in the pre-"Massive cleanup" world
>> but ought to have been dropped when c->x86_capability was properly split.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> albeit I think that, along the lines of a comment I made on Matrix, that ...
>
>> --- a/xen/arch/x86/cpu/amd.c
>> +++ b/xen/arch/x86/cpu/amd.c
>> @@ -1131,13 +1131,6 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
>>              wrmsrl(MSR_K8_HWCR, value);
>>      }
>>  
>> -    /*
>> -     * Some AMD CPUs duplicate the 3DNow bit in base and extended CPUID
>> -     * leaves.  Unfortunately, this aliases PBE on Intel CPUs. Clobber the
>> -     * alias, leaving 3DNow in the extended leaf.
>> -     */
>> -    __clear_bit(X86_FEATURE_PBE, c->x86_capability);
> ... while the justification applies to what the CPUs surface on their own,
> the basic leaf masking MSR could (likely) still have undue bits set, and
> hence may want pruning in exchange / addition.

As said, the ability to control CPUID in this way postdates by a decade
this logic in Linux, and this logic came about because of a logical
issue/shortcut taken by Linux, not because of a CPU which really
mis-reported this bit.

We do have logic which adjusts the override MSRs on AMD based on errata.

~Andrew