Re: [PATCH 01/16] x86/cpu: Fix boot time cache flushing

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

>> 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. 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.

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.

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.

Jan