[PATCH] x86/cpu: Provide default cache line size if not enumerated

[PATCH] x86/cpu: Provide default cache line size if not enumerated

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

tl;dr: CPUs with CPUID.80000008H but without CPUID.01H:EDX[CLFSH]
will end up reporting cache_line_size()==0 and bad things happen.
Fill in a default on those to avoid the problem.

Long Story:

The kernel dies a horrible death if c->x86_cache_alignment (aka.
cache_line_size() is 0.  Normally, this value is populated from
c->x86_clflush_size.

Right now the code is set up to get c->x86_clflush_size from two
places.  First, modern CPUs get it from CPUID.  Old CPUs that don't
have leaf 0x80000008 (or CPUID at all) just get some sane defaults
from the kernel in get_cpu_address_sizes().

The vast majority of CPUs that have leaf 0x80000008 also get
->x86_clflush_size from CPUID.  But there are oddballs.

Intel Quark CPUs[1] and others[2] have leaf 0x80000008 but don't set
CPUID.01H:EDX[CLFSH], so they skip over filling in ->x86_clflush_size:

	cpuid(0x00000001, &tfms, &misc, &junk, &cap0);
	if (cap0 & (1<<19))
		c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;

So they: land in get_cpu_address_sizes(), set vp_bits_from_cpuid=0 and
never fill in c->x86_clflush_size, assign c->x86_cache_alignment, and
hilarity ensues in code like:

        buffer = kzalloc(ALIGN(sizeof(*buffer), cache_line_size()),
                         GFP_KERNEL);

To fix this, always provide a sane value for ->x86_clflush_size.

Big thanks to Andy Shevchenko for finding and reporting this and also
providing a first pass at a fix. But his fix was only partial and only
worked on the Quark CPUs.  It would not, for instance, have worked on
the QEMU config.

1. https://raw.githubusercontent.com/InstLatx64/InstLatx64/master/GenuineIntel/GenuineIntel0000590_Clanton_03_CPUID.txt
2. You can also get this behavior if you use "-cpu 486,+clzero"
   in QEMU.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Fixes: fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach")
Link: https://lore.kernel.org/all/20240516173928.3960193-1-andriy.shevchenko@linux.intel.com/
Cc: stable@vger.kernel.org
---

 b/arch/x86/kernel/cpu/common.c |    4 ++++
 1 file changed, 4 insertions(+)

diff -puN arch/x86/kernel/cpu/common.c~default-x86_clflush_size arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c~default-x86_clflush_size	2024-05-17 12:51:25.886169008 -0700
+++ b/arch/x86/kernel/cpu/common.c	2024-05-17 13:03:09.761999885 -0700
@@ -1064,6 +1064,10 @@ void get_cpu_address_sizes(struct cpuinf
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
+
+		/* Provide a sane default if not enumerated: */
+		if (!c->x86_clflush_size)
+			c->x86_clflush_size = 32;
 	} else {
 		if (IS_ENABLED(CONFIG_X86_64)) {
 			c->x86_clflush_size = 64;
_

Re: [PATCH] x86/cpu: Provide default cache line size if not enumerated

[Andy Shevchenko]

On Fri, May 17, 2024 at 01:05:34PM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> tl;dr: CPUs with CPUID.80000008H but without CPUID.01H:EDX[CLFSH]
> will end up reporting cache_line_size()==0 and bad things happen.
> Fill in a default on those to avoid the problem.
> 
> Long Story:
> 
> The kernel dies a horrible death if c->x86_cache_alignment (aka.
> cache_line_size() is 0.  Normally, this value is populated from

Missing ) ?

> c->x86_clflush_size.
> 
> Right now the code is set up to get c->x86_clflush_size from two
> places.  First, modern CPUs get it from CPUID.  Old CPUs that don't
> have leaf 0x80000008 (or CPUID at all) just get some sane defaults
> from the kernel in get_cpu_address_sizes().
> 
> The vast majority of CPUs that have leaf 0x80000008 also get
> ->x86_clflush_size from CPUID.  But there are oddballs.
> 
> Intel Quark CPUs[1] and others[2] have leaf 0x80000008 but don't set
> CPUID.01H:EDX[CLFSH], so they skip over filling in ->x86_clflush_size:
> 
> 	cpuid(0x00000001, &tfms, &misc, &junk, &cap0);
> 	if (cap0 & (1<<19))
> 		c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
> 
> So they: land in get_cpu_address_sizes(), set vp_bits_from_cpuid=0 and
> never fill in c->x86_clflush_size, assign c->x86_cache_alignment, and
> hilarity ensues in code like:
> 
>         buffer = kzalloc(ALIGN(sizeof(*buffer), cache_line_size()),
>                          GFP_KERNEL);
> 
> To fix this, always provide a sane value for ->x86_clflush_size.
> 
> Big thanks to Andy Shevchenko for finding and reporting this and also
> providing a first pass at a fix. But his fix was only partial and only
> worked on the Quark CPUs.  It would not, for instance, have worked on
> the QEMU config.
> 
> 1. https://raw.githubusercontent.com/InstLatx64/InstLatx64/master/GenuineIntel/GenuineIntel0000590_Clanton_03_CPUID.txt
> 2. You can also get this behavior if you use "-cpu 486,+clzero"
>    in QEMU.

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(as this obviously fixes the issue as it makes a partial revert of the culprit
 change).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] x86/cpu: Provide default cache line size if not enumerated

[Andy Shevchenko]

On Mon, May 20, 2024 at 01:43:42PM +0300, Andy Shevchenko wrote:
> On Fri, May 17, 2024 at 01:05:34PM -0700, Dave Hansen wrote:
> > 
> > From: Dave Hansen <dave.hansen@linux.intel.com>
> > 
> > tl;dr: CPUs with CPUID.80000008H but without CPUID.01H:EDX[CLFSH]
> > will end up reporting cache_line_size()==0 and bad things happen.
> > Fill in a default on those to avoid the problem.
> > 
> > Long Story:
> > 
> > The kernel dies a horrible death if c->x86_cache_alignment (aka.
> > cache_line_size() is 0.  Normally, this value is populated from
> 
> Missing ) ?
> 
> > c->x86_clflush_size.
> > 
> > Right now the code is set up to get c->x86_clflush_size from two
> > places.  First, modern CPUs get it from CPUID.  Old CPUs that don't
> > have leaf 0x80000008 (or CPUID at all) just get some sane defaults
> > from the kernel in get_cpu_address_sizes().
> > 
> > The vast majority of CPUs that have leaf 0x80000008 also get
> > ->x86_clflush_size from CPUID.  But there are oddballs.
> > 
> > Intel Quark CPUs[1] and others[2] have leaf 0x80000008 but don't set
> > CPUID.01H:EDX[CLFSH], so they skip over filling in ->x86_clflush_size:
> > 
> > 	cpuid(0x00000001, &tfms, &misc, &junk, &cap0);
> > 	if (cap0 & (1<<19))
> > 		c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
> > 
> > So they: land in get_cpu_address_sizes(), set vp_bits_from_cpuid=0 and
> > never fill in c->x86_clflush_size, assign c->x86_cache_alignment, and
> > hilarity ensues in code like:
> > 
> >         buffer = kzalloc(ALIGN(sizeof(*buffer), cache_line_size()),
> >                          GFP_KERNEL);
> > 
> > To fix this, always provide a sane value for ->x86_clflush_size.
> > 
> > Big thanks to Andy Shevchenko for finding and reporting this and also
> > providing a first pass at a fix. But his fix was only partial and only
> > worked on the Quark CPUs.  It would not, for instance, have worked on
> > the QEMU config.
> > 
> > 1. https://raw.githubusercontent.com/InstLatx64/InstLatx64/master/GenuineIntel/GenuineIntel0000590_Clanton_03_CPUID.txt
> > 2. You can also get this behavior if you use "-cpu 486,+clzero"
> >    in QEMU.
> 
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> (as this obviously fixes the issue as it makes a partial revert of the culprit
>  change).

What's the status of this? (It seems you have to rebase it on top of the
existing patches in the same area).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] x86/cpu: Provide default cache line size if not enumerated

[Dave Hansen]

On 5/30/24 07:13, Andy Shevchenko wrote:
> What's the status of this? (It seems you have to rebase it on top of the
> existing patches in the same area).

Queued as of about 10 seconds ago:

> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=b9210e56d71d9deb1ad692e405f6b2394f7baa4d

Re: [PATCH] x86/cpu: Provide default cache line size if not enumerated

[Andy Shevchenko]

On Thu, May 30, 2024 at 07:16:03AM -0700, Dave Hansen wrote:
> On 5/30/24 07:13, Andy Shevchenko wrote:
> > What's the status of this? (It seems you have to rebase it on top of the
> > existing patches in the same area).
> 
> Queued as of about 10 seconds ago:

Thank you!

> > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=b9210e56d71d9deb1ad692e405f6b2394f7baa4d

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] x86/cpu: Provide default cache line size if not enumerated

[Jörn Heusipp]

Hello!

> From: Dave Hansen <dave.hansen@linux.intel.com>
>
> tl;dr: CPUs with CPUID.80000008H but without CPUID.01H:EDX[CLFSH]
> will end up reporting cache_line_size()==0 and bad things happen.
> Fill in a default on those to avoid the problem.
>
> Long Story:
>
> The kernel dies a horrible death if c->x86_cache_alignment (aka.
> cache_line_size() is 0.  Normally, this value is populated from
> c->x86_clflush_size.
>
> Right now the code is set up to get c->x86_clflush_size from two
> places.  First, modern CPUs get it from CPUID.  Old CPUs that don't
> have leaf 0x80000008 (or CPUID at all) just get some sane defaults
> from the kernel in get_cpu_address_sizes().
>
> The vast majority of CPUs that have leaf 0x80000008 also get
> ->x86_clflush_size from CPUID.  But there are oddballs.
>
> Intel Quark CPUs[1] and others[2] have leaf 0x80000008 but don't set
> CPUID.01H:EDX[CLFSH], so they skip over filling in ->x86_clflush_size:
>
> 	cpuid(0x00000001, &tfms, &misc, &junk, &cap0);
> 	if (cap0 & (1<<19))
> 		c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
>
> So they: land in get_cpu_address_sizes(), set vp_bits_from_cpuid=0 and
> never fill in c->x86_clflush_size, assign c->x86_cache_alignment, and
> hilarity ensues in code like:
>
>         buffer = kzalloc(ALIGN(sizeof(*buffer), cache_line_size()),
>                          GFP_KERNEL);
>
> To fix this, always provide a sane value for ->x86_clflush_size.
>
> Big thanks to Andy Shevchenko for finding and reporting this and also
> providing a first pass at a fix. But his fix was only partial and only
> worked on the Quark CPUs.  It would not, for instance, have worked on
> the QEMU config.
>
> 1. https://raw.githubusercontent.com/InstLatx64/InstLatx64/master/GenuineIntel/GenuineIntel0000590_Clanton_03_CPUID.txt
> 2. You can also get this behavior if you use "-cpu 486,+clzero"
>    in QEMU.

Tested-by: Jörn Heusipp <osmanx@heusipp.de>

See
https://lore.kernel.org/lkml/5e31cad3-ad4d-493e-ab07-724cfbfaba44@heusipp.de/


Best regards,
Jörn