| Message ID | 20250213161423.449435-5-riel@surriel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD broadcast TLB invalidation | expand |
On 2/13/25 08:13, Rik van Riel wrote: > + /* Max number of pages INVLPGB can invalidate in one shot */ > + if (boot_cpu_has(X86_FEATURE_INVLPGB)) { > + cpuid(0x80000008, &eax, &ebx, &ecx, &edx); > + invlpgb_count_max = (edx & 0xffff) + 1; > + } > } My only nit with this is that this adds a seventh place that the kernel reads leaf 0x80000008. The users seem really random, though. Some cross-vendor stuff, some very AMD-specific stuff. So, I'll leave that at a nit since I don't have a good suggestion for solving it: Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
On Thu, Feb 13, 2025 at 11:13:55AM -0500, Rik van Riel wrote: > The CPU advertises the maximum number of pages that can be shot down > with one INVLPGB instruction in the CPUID data. > > Save that information for later use. > > Signed-off-by: Rik van Riel <riel@surriel.com> > Tested-by: Manali Shukla <Manali.Shukla@amd.com> > Tested-by: Brendan Jackman <jackmanb@google.com> > Tested-by: Michael Kelley <mhklinux@outlook.com> > --- > arch/x86/Kconfig.cpu | 5 +++++ > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/tlbflush.h | 7 +++++++ > arch/x86/kernel/cpu/amd.c | 8 ++++++++ > 4 files changed, 21 insertions(+) > > diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu > index 2a7279d80460..abe013a1b076 100644 > --- a/arch/x86/Kconfig.cpu > +++ b/arch/x86/Kconfig.cpu > @@ -395,6 +395,10 @@ config X86_VMX_FEATURE_NAMES > def_bool y > depends on IA32_FEAT_CTL > > +config X86_BROADCAST_TLB_FLUSH > + def_bool y > + depends on CPU_SUP_AMD && 64BIT > + > menuconfig PROCESSOR_SELECT > bool "Supported processor vendors" if EXPERT > help > @@ -431,6 +435,7 @@ config CPU_SUP_CYRIX_32 > config CPU_SUP_AMD > default y > bool "Support AMD processors" if PROCESSOR_SELECT > + select X86_BROADCAST_TLB_FLUSH CPU_SUP_AMD selects X86_BROADCAST_TLB_FLUSH which depends on CPU_SUP_AMD which selects X86_BROADCAST_TLB_FLUSH which depends on CPU_SUP_AMD... Why do you really need yet another Kconfig symbol? Just whack X86_BROADCAST_TLB_FLUSH - it'll be enabled by default on everything anyway. > @@ -1139,6 +1141,12 @@ static void cpu_detect_tlb_amd(struct cpuinfo_x86 *c) > tlb_lli_2m[ENTRIES] = eax & mask; > > tlb_lli_4m[ENTRIES] = tlb_lli_2m[ENTRIES] >> 1; > + > + /* Max number of pages INVLPGB can invalidate in one shot */ > + if (boot_cpu_has(X86_FEATURE_INVLPGB)) { > + cpuid(0x80000008, &eax, &ebx, &ecx, &edx); > + invlpgb_count_max = (edx & 0xffff) + 1; > + } get_cpu_cap() already reads that leaf. You don't need to do it here again.
On Wed, 2025-02-19 at 12:56 +0100, Borislav Petkov wrote: > On Thu, Feb 13, 2025 at 11:13:55AM -0500, Rik van Riel wrote: > > The CPU advertises the maximum number of pages that can be shot > > down > > with one INVLPGB instruction in the CPUID data. > > > > Save that information for later use. > > > > Signed-off-by: Rik van Riel <riel@surriel.com> > > Tested-by: Manali Shukla <Manali.Shukla@amd.com> > > Tested-by: Brendan Jackman <jackmanb@google.com> > > Tested-by: Michael Kelley <mhklinux@outlook.com> > > --- > > arch/x86/Kconfig.cpu | 5 +++++ > > arch/x86/include/asm/cpufeatures.h | 1 + > > arch/x86/include/asm/tlbflush.h | 7 +++++++ > > arch/x86/kernel/cpu/amd.c | 8 ++++++++ > > 4 files changed, 21 insertions(+) > > > > diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu > > index 2a7279d80460..abe013a1b076 100644 > > --- a/arch/x86/Kconfig.cpu > > +++ b/arch/x86/Kconfig.cpu > > @@ -395,6 +395,10 @@ config X86_VMX_FEATURE_NAMES > > def_bool y > > depends on IA32_FEAT_CTL > > > > +config X86_BROADCAST_TLB_FLUSH > > + def_bool y > > + depends on CPU_SUP_AMD && 64BIT > > + > > menuconfig PROCESSOR_SELECT > > bool "Supported processor vendors" if EXPERT > > help > > @@ -431,6 +435,7 @@ config CPU_SUP_CYRIX_32 > > config CPU_SUP_AMD > > default y > > bool "Support AMD processors" if PROCESSOR_SELECT > > + select X86_BROADCAST_TLB_FLUSH > > CPU_SUP_AMD selects X86_BROADCAST_TLB_FLUSH which depends on > CPU_SUP_AMD which > selects X86_BROADCAST_TLB_FLUSH which depends on CPU_SUP_AMD... > > Why do you really need yet another Kconfig symbol? Just whack > X86_BROADCAST_TLB_FLUSH - it'll be enabled by default on everything > anyway. Dave specifically asked me to do things that way. > > > @@ -1139,6 +1141,12 @@ static void cpu_detect_tlb_amd(struct > > cpuinfo_x86 *c) > > tlb_lli_2m[ENTRIES] = eax & mask; > > > > tlb_lli_4m[ENTRIES] = tlb_lli_2m[ENTRIES] >> 1; > > + > > + /* Max number of pages INVLPGB can invalidate in one shot > > */ > > + if (boot_cpu_has(X86_FEATURE_INVLPGB)) { > > + cpuid(0x80000008, &eax, &ebx, &ecx, &edx); > > + invlpgb_count_max = (edx & 0xffff) + 1; > > + } > > get_cpu_cap() already reads that leaf. You don't need to do it here > again. > Should I modify get_cpu_cap() to store c->x86_capabilities[CPUID_8000_0008_EDX] ? Currently only the EBX data is stored there, while invlpgb_count_max comes from EDX.
On Wed, Feb 19, 2025 at 12:52:56PM -0500, Rik van Riel wrote: > Should I modify get_cpu_cap() to store > c->x86_capabilities[CPUID_8000_0008_EDX] ? > > Currently only the EBX data is stored there, > while invlpgb_count_max comes from EDX. No, you should simply assign invlpgb_count_max there.
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu index 2a7279d80460..abe013a1b076 100644 --- a/arch/x86/Kconfig.cpu +++ b/arch/x86/Kconfig.cpu @@ -395,6 +395,10 @@ config X86_VMX_FEATURE_NAMES def_bool y depends on IA32_FEAT_CTL +config X86_BROADCAST_TLB_FLUSH + def_bool y + depends on CPU_SUP_AMD && 64BIT + menuconfig PROCESSOR_SELECT bool "Supported processor vendors" if EXPERT help @@ -431,6 +435,7 @@ config CPU_SUP_CYRIX_32 config CPU_SUP_AMD default y bool "Support AMD processors" if PROCESSOR_SELECT + select X86_BROADCAST_TLB_FLUSH help This enables detection, tunings and quirks for AMD processors diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 508c0dad116b..b5c66b7465ba 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -338,6 +338,7 @@ #define X86_FEATURE_CLZERO (13*32+ 0) /* "clzero" CLZERO instruction */ #define X86_FEATURE_IRPERF (13*32+ 1) /* "irperf" Instructions Retired Count */ #define X86_FEATURE_XSAVEERPTR (13*32+ 2) /* "xsaveerptr" Always save/restore FP error pointers */ +#define X86_FEATURE_INVLPGB (13*32+ 3) /* INVLPGB and TLBSYNC instruction supported. */ #define X86_FEATURE_RDPRU (13*32+ 4) /* "rdpru" Read processor register at user level */ #define X86_FEATURE_WBNOINVD (13*32+ 9) /* "wbnoinvd" WBNOINVD instruction */ #define X86_FEATURE_AMD_IBPB (13*32+12) /* Indirect Branch Prediction Barrier */ diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h index 3da645139748..e026a5cc388e 100644 --- a/arch/x86/include/asm/tlbflush.h +++ b/arch/x86/include/asm/tlbflush.h @@ -183,6 +183,13 @@ static inline void cr4_init_shadow(void) extern unsigned long mmu_cr4_features; extern u32 *trampoline_cr4_features; +/* How many pages can we invalidate with one INVLPGB. */ +#ifdef CONFIG_X86_BROADCAST_TLB_FLUSH +extern u16 invlpgb_count_max; +#else +#define invlpgb_count_max 1 +#endif + extern void initialize_tlbstate_and_flush(void); /* diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 54194f5995de..3e8180354303 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -29,6 +29,8 @@ #include "cpu.h" +u16 invlpgb_count_max __ro_after_init; + static inline int rdmsrl_amd_safe(unsigned msr, unsigned long long *p) { u32 gprs[8] = { 0 }; @@ -1139,6 +1141,12 @@ static void cpu_detect_tlb_amd(struct cpuinfo_x86 *c) tlb_lli_2m[ENTRIES] = eax & mask; tlb_lli_4m[ENTRIES] = tlb_lli_2m[ENTRIES] >> 1; + + /* Max number of pages INVLPGB can invalidate in one shot */ + if (boot_cpu_has(X86_FEATURE_INVLPGB)) { + cpuid(0x80000008, &eax, &ebx, &ecx, &edx); + invlpgb_count_max = (edx & 0xffff) + 1; + } } static const struct cpu_dev amd_cpu_dev = {