On 27.11.2025 13:36, Alejandro Vallejo wrote:
> On Thu Nov 27, 2025 at 10:43 AM CET, Jan Beulich wrote:
>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/Kconfig.cpu
>>> +++ b/xen/arch/x86/Kconfig.cpu
>>> @@ -19,4 +19,49 @@ config INTEL
>>> May be turned off in builds targetting other vendors. Otherwise,
>>> must be enabled for Xen to work suitably on Intel platforms.
>>>
>>> +config HYGON
>>> + bool "Support Hygon CPUs"
>>> + depends on AMD
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Hygon platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Hygon platforms.
>>> +
>>> +
>>> +config CENTAUR
>>> + bool "Support Centaur CPUs"
>>> + depends on INTEL
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Centaur platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Centaur platforms.
>>> +
>>> +config SHANGHAI
>>> + bool "Support Shanghai CPUs"
>>> + depends on INTEL
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Shanghai platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Shanghai platforms.
>>> +
>>> +config UNKNOWN_CPU
>>> + bool "Support unknown CPUs"
>>
>> "Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly
>> support,
>> and such of vendors we do explicitly support, but where we aren't aware of
>> the
>> particular model. This needs to be unambiguous here, perhaps by it becoming
>> UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
>
> Right, what I do in this RFC is have compiled-out vendors fall back onto the
> unknown vendor path. Because it really is unknown to the binary.
>
> I could call it GENERIC_CPU_VENDOR, or anything else, but the main question
> is whether a toggle for this seems acceptable upstream. I don't see obvious
> drawbacks.
I'd recommend against "generic" or anything alike, as it'll rather suggest any
vendor's CPU will work reasonably. I'm fine with "unknown", just that the nature
of the unknown-ness needs making unambiguous.
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 *
>>> c)
>>> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>>> }
>>>
>>> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
>>> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
>>
>> This change isn't explained in the description. __used here was introduced
>> not
>> all this long ago together with __initconst_cf_clobber. Maybe this really was
>> a mistake, but if so it's correction should be explained.
>
> It wasn't clear to me why __used was there (I assume it shouldn't have been),
> but it definitely clashes with the intent of having it gone when
> UNKNOWN_CPU=n.
>
> If __used was misplaced to begin with I'm happy to get rid of it in a separate
> patch. I don't think it warrants a Fixes tag, though.
I can only vaguely reconstruct that it may have been put there so the
.init.rodata.cf_clobber entry wouldn't go away. But as long as the compiler
also eliminates the function pointed at, that would be of no concern.
>>> @@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
>>> *(u32 *)&c->x86_vendor_id[8] = ecx;
>>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>>
>>> - c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>>> + c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
>>> + X86_ENABLED_VENDORS;
>>
>> May I suggest the & to move ...
>>
>>> switch (c->x86_vendor) {
>>
>> ... here? Yes, you panic() below, but I see no reason to store inaccurate
>> data when that's easy to avoid.
>
> That's intentional. Otherwise further checks of c->x86_vendor in other parts
> of
> the code may not go through the same branch as the one here.
Hmm. I would kind of expect x86_vendor_is() to be capable of dealing with
that.
Jan
