On 5/16/24 10:39, Andy Shevchenko wrote:
The initial change to set x86_virt_bits to the correct value straight away broke boot on Intel Quark X1000 CPUs (which are family 5, model 9, stepping 0)
Do you know what _actually_ broke? Like was there a crash somewhere?
With deeper investigation it appears that the Quark doesn't have the bit 19 set in 0x01 CPUID leaf, which means it doesn't provide any clflush instructions and hence the cache alignment is set to 0. The actual cache line size is 16 bytes, hence we may set the alignment accordingly. At the same time the physical and virtual address bits are retrieved via 0x80000008 CPUID leaf.
This seems to be saying that ->x86_clflush_size must come from CPUID. But there _are_ CPUID-independent defaults set in identify_cpu(). How do those fit in?
Note, we don't really care about the value of x86_clflush_size as it is either used with a proper check for the instruction to be present, or, like in PCI case, it assumes 32 bytes for all supported 32-bit CPUs that have actually smaller cache line sizes and don't advertise it.
Are you trying to say that having ->x86_clflush_size==0 is not fatal while having ->x86_cache_alignment==0 _is_ fatal?
The commit fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach") basically revealed the issue that has been present from day 1 of introducing the Quark support.
How did it do that, exactly? It's still not crystal clear.
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index be30d7fa2e66..2bffae158dd5 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -321,6 +321,15 @@ static void early_init_intel(struct cpuinfo_x86 *c) #ifdef CONFIG_X86_64 set_cpu_cap(c, X86_FEATURE_SYSENTER32); #else
- /*
* The Quark doesn't have bit 19 set in 0x01 CPUID leaf, which means* it doesn't provide any clflush instructions and hence the cache* alignment is set to 0. The actual cache line size is 16 bytes,* hence set the alignment accordingly. At the same time the physical* and virtual address bits are retrieved via 0x80000008 CPUID leaf.*/- if (c->x86 == 5 && c->x86_model == 9)
c->x86_cache_alignment = 16;
What are the odds that another CPU has this issue? I'm thinking we should just set a default in addition to hacking around this for Quark.