On 27/01/2026 11:35 am, Jan Beulich wrote:
> On 27.01.2026 12:08, Andrew Cooper wrote:
>> On 27/01/2026 10:37 am, Jan Beulich wrote:
>>> On 26.01.2026 18:53, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/cpu/common.c
>>>> +++ b/xen/arch/x86/cpu/common.c
>>>> @@ -319,8 +319,6 @@ void __init early_cpu_init(bool verbose)
>>>> uint64_t val;
>>>> u32 eax, ebx, ecx, edx;
>>>>
>>>> - c->x86_cache_alignment = 32;
>>>> -
>>>> /* Get vendor name */
>>>> cpuid(0x00000000, &c->cpuid_level, &ebx, &ecx, &edx);
>>>> *(u32 *)&c->x86_vendor_id[0] = ebx;
>>>> @@ -352,6 +350,7 @@ void __init early_cpu_init(bool verbose)
>>>> if (edx & cpufeat_mask(X86_FEATURE_CLFLUSH)) {
>>>> unsigned int size = ((ebx >> 8) & 0xff) * 8;
>>>>
>>>> + c->x86_clflush_size = size;
>>>> c->x86_cache_alignment = size;
>>> With this change, can't the writing of the field in generic_identify()
>>> go away? CPU_DATA_INIT() in particular doesn't invalidate it.
>> No, it can't. The value needs setting up on every AP, right now at least.
> Are you sure? APs inherit part of the BSP's data (initialize_cpu_data()),
> and reset_cpuinfo() doesn't clear ->x86_clflush_size afaics.
Every time I look at that, it gets more insane.
For every CPU, initialize_cpu_data() clobbers boot_cpu_data, *then*
copies the result into cpu_data[] array.
This cannot possibly be correct. Why on earth did I ack it?
>>> Tangentially, "cpuid=no-clflush" didn't have any effect on any of this so
>>> far, and also isn't going to have with the changes you make.
>> The line immediately out of context above will applies the clear cap
>> mask, so will cause cpuid=no-clflush to take effect.
> This concerns me. With your change, "cpuid=no-clflush" will lead to an
> unconditional panic() then.
So will no-cmpxchg8b. The only reason no-lm won't is because that's
evaluated before parsing the cmdline.
> Whereas previously, with cleared_caps[] being
> applied by identify_cpu() only after generic_identify() has already
> evaluated the CLFLUSH bit, there was no effect at all.
That wasn't no effect. The effect (upon request of an impossible thing)
would be that part of Xen would have ignored the request and functioned,
but another part of Xen would have propagated that to guests, which will
probably have equally rude things to say.
> I don't think this panic()ing is desirable, but as an absolute minimum this
> (drastic) change in behavior would want calling out in the description.
>
> Further, if the panic() was to stay, there's no point having cpu_has_clflush
> evaluate to anything other than constant true anymore.
I'm not overly interested in users complaining about a panic() if they
ask for an impossible thing. Better that than the prior behaviour we had.
Talking of other impossible things, cpuid=no-$foo does nothing for
FPU/DE/PSE/PGE or MMX which are the features hard wired to 1 already,
and with 0 users in the tree.
Again, Xen will assume the safe thing, but pass the impossible request
on to guests.
> Again tangentially (and partly the reason why I overlooked that aspect
> originally): While early_cpu_init() respects cleared_caps[] for leaf 1, it
> doesn't for any of leaf 7's subleaves, nor for ARCH_CAPS.
No idea. I stopped doing archaeology on all the wrong-looking things I
found, because there are just too many of them.
~Andrew
