[PATCH] x86/mce: Increase maximum number of banks to 64

[PATCH] x86/mce: Increase maximum number of banks to 64

[Yazen Ghannam]

From: Akshay Gupta <Akshay.Gupta@amd.com>

...because future AMD systems will support up to 64 MCA banks per CPU.

MAX_NR_BANKS is used to allocate a number of data structures, and it is
used as a ceiling for values read from MCG_CAP[Count]. Therefore, this
change will have no functional effect on existing systems with 32 or
fewer MCA banks per CPU.

Signed-off-by: Akshay Gupta <Akshay.Gupta@amd.com>
[ Adjust commit message and code comment. ]
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
 arch/x86/include/asm/mce.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 6adced6e7dd3..109af5c7f515 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -200,12 +200,8 @@ void mce_setup(struct mce *m);
 void mce_log(struct mce *m);
 DECLARE_PER_CPU(struct device *, mce_device);
 
-/*
- * Maximum banks number.
- * This is the limit of the current register layout on
- * Intel CPUs.
- */
-#define MAX_NR_BANKS 32
+/* Maximum number of MCA banks per CPU. */
+#define MAX_NR_BANKS 64
 
 #ifdef CONFIG_X86_MCE_INTEL
 void mce_intel_feature_init(struct cpuinfo_x86 *c);
-- 
2.25.1

Re: [PATCH] x86/mce: Increase maximum number of banks to 64

[Borislav Petkov]

On Thu, Aug 20, 2020 at 05:06:24PM +0000, Yazen Ghannam wrote:
> From: Akshay Gupta <Akshay.Gupta@amd.com>
> 
> ...because future AMD systems will support up to 64 MCA banks per CPU.
> 
> MAX_NR_BANKS is used to allocate a number of data structures, and it is
> used as a ceiling for values read from MCG_CAP[Count]. Therefore, this
> change will have no functional effect on existing systems with 32 or
> fewer MCA banks per CPU.

Of course it will, grep for MAX_NR_BANKS and look at all those bitmaps
and arrays which get defined with MAX_NR_BANKS size. With your change,
they will double in size.

How much does vmlinux size grow with your change?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/mce: Increase maximum number of banks to 64

[Yazen Ghannam]

On Thu, Aug 20, 2020 at 07:15:18PM +0200, Borislav Petkov wrote:
> On Thu, Aug 20, 2020 at 05:06:24PM +0000, Yazen Ghannam wrote:
> > From: Akshay Gupta <Akshay.Gupta@amd.com>
> > 
> > ...because future AMD systems will support up to 64 MCA banks per CPU.
> > 
> > MAX_NR_BANKS is used to allocate a number of data structures, and it is
> > used as a ceiling for values read from MCG_CAP[Count]. Therefore, this
> > change will have no functional effect on existing systems with 32 or
> > fewer MCA banks per CPU.
> 
> Of course it will, grep for MAX_NR_BANKS and look at all those bitmaps
> and arrays which get defined with MAX_NR_BANKS size. With your change,
> they will double in size.
> 
> How much does vmlinux size grow with your change?
>

It seems to get smaller.

-rwxrwxr-x   1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks
-rwxrwxr-x   1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks

Any ideas? Maybe there's some alignment change? Or a build issue on my
end?

Thanks,
Yazen

RE: [PATCH] x86/mce: Increase maximum number of banks to 64

[Luck, Tony]

>> How much does vmlinux size grow with your change?
>>
>
> It seems to get smaller.
>
> -rwxrwxr-x   1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks
> -rwxrwxr-x   1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks

You need to run:

$ size vmlinux
   text    data     bss     dec     hex filename
20334755        12569682        14798924        47703361        2d7e541 vmlinux

Likely the extra space is added to the third element ("bss"). That doesn't show
up in the vmlinux file, but does add to memory footprint while running.

-Tony

Re: [PATCH] x86/mce: Increase maximum number of banks to 64

[Yazen Ghannam]

On Thu, Aug 20, 2020 at 06:15:15PM +0000, Luck, Tony wrote:
> >> How much does vmlinux size grow with your change?
> >>
> >
> > It seems to get smaller.
> >
> > -rwxrwxr-x   1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks
> > -rwxrwxr-x   1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks
> 
> You need to run:
> 
> $ size vmlinux
>    text    data     bss     dec     hex filename
> 20334755        12569682        14798924        47703361        2d7e541 vmlinux
> 
> Likely the extra space is added to the third element ("bss"). That doesn't show
> up in the vmlinux file, but does add to memory footprint while running.

Thanks. Yeah, they're identical:
   text    data     bss     dec     hex filename
   15710076        13519306        5398528 34627910        2106146   vmlinux-32banks
   15710076        13519306        5398528 34627910        2106146   vmlinux-64banks

I did a quick audit of the statically allocated data structures which
use MAX_NR_BANKS.

Global bitmaps:
- core.c / mce_banks_ce_disabled
- core.c / all_banks
- core.c / valid_banks
- core.c / toclear
- Total: 32 new bits * 4 bitmaps = 16 new bytes

Per-CPU bitmaps:
- core.c / mce_poll_banks
- intel.c / mce_banks_owned
- Total: 32 new bits * 2 bitmaps = 8 new bytes

The bitmaps are arrays of longs. So this change will only affect 32-bit
execution (I assume), since there will be one additional long used.
There will be no additional memory use on 64-bit execution, because the
size of long is 64 bits.

Global structs:
- amd.c / struct smca_bank smca_banks[]: 16 bytes per bank
- core.c / struct mce_bank_dev mce_bank_devs[]: 56 bytes per bank
- Total: 32 new banks * (16 + 56) bytes = 2304 new bytes

Per-CPU structs:
- core.c / struct mce_bank mce_banks_array[]: 16 bytes per bank
- Total: 32 new banks * 16 bytes = 512 new bytes

32-bit
Total global size increase: 2320 bytes
Total per-CPU size increase: 520 bytes

64-bit
Total global size increase: 2304 bytes
Total per-CPU size increase: 512 bytes

Is this okay?

Thanks,
Yazen

Re: [PATCH] x86/mce: Increase maximum number of banks to 64

[Borislav Petkov]

On Mon, Aug 24, 2020 at 10:11:05AM -0500, Yazen Ghannam wrote:
> Thanks. Yeah, they're identical:
>    text    data     bss     dec     hex filename
>    15710076        13519306        5398528 34627910        2106146   vmlinux-32banks
>    15710076        13519306        5398528 34627910        2106146   vmlinux-64banks

So I went crazy here and built allmodconfigs for hypothetical CPUs which
support up to 4K MCA banks:

   text    data     bss     dec     hex filename
24618040        27211906        35409988        87239934        5332cfe vmlinux-32banks
24618040        27211906        35409988        87239934        5332cfe vmlinux-64banks
24618040        27211906        35409988        87239934        5332cfe vmlinux-128banks

For the first three - up to 128 banks, the sizes don't change.

24618000        27216002        35401796        87235798        5331cd6 vmlinux-256banks

Somewhere after 128 banks, it grows the data section but by exactly 4K,
i.e., one more page:

27216002 - 27211906 = 4096

The last page is not full, of course.

27216002 % 4096 = 2178
27211906 % 4096 = 2178

24618000        27220098        35401796        87239894        5332cd6 vmlinux-512banks

The same happens here.

27220098 - 27216002 = 4096
27220098 % 4096 = 2178

And so on:

24618000        27228290        35393604        87239894        5332cd6 vmlinux-1024banks
24618000        27244674        35377220        87239894        5332cd6 vmlinux-2048banks
24618000        27277442        35344452        87239894        5332cd6 vmlinux-4Kbanks

So what I think happens is, the first 128 banks fit within the same
.data section page thus vmlinux doesn't increase.

I guess there might be a .config which could cause that data increase
when the 32->64 banks move won't fit in the last .data page. It depends
on where those ~2K you computed and I snipped, land.

But since most kernels are allmodconfig builds, I guess we should be
fine.

So yeah, I guess we can take this one after you summarize the findings
of this thread in v2's commit message. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette