| Message ID | 20241023172150.659002-4-yazen.ghannam@amd.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | AMD NB and SMN rework | expand |
On Wed, Oct 23, 2024 at 05:21:37PM +0000, Yazen Ghannam wrote: > @@ -393,11 +392,11 @@ bool __init early_is_amd_nb(u32 device) > boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) > return false; > > - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) > - misc_ids = hygon_nb_misc_ids; > + if (boot_cpu_has(X86_FEATURE_ZEN)) check_for_deprecated_apis: WARNING: arch/x86/kernel/amd_nb.c:395: Do not use boot_cpu_has() - use cpu_feature_enabled() instead.
On Fri, Oct 25, 2024 at 05:58:30PM +0200, Borislav Petkov wrote: > On Wed, Oct 23, 2024 at 05:21:37PM +0000, Yazen Ghannam wrote: > > @@ -393,11 +392,11 @@ bool __init early_is_amd_nb(u32 device) > > boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) > > return false; > > > > - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) > > - misc_ids = hygon_nb_misc_ids; > > + if (boot_cpu_has(X86_FEATURE_ZEN)) > > check_for_deprecated_apis: WARNING: arch/x86/kernel/amd_nb.c:395: Do not use boot_cpu_has() - use cpu_feature_enabled() instead. Sure thing. How can I enable this check myself? Thanks, Yazen
On Tue, Oct 29, 2024 at 10:39:28AM -0400, Yazen Ghannam wrote:
> How can I enable this check myself?
It is part of my silly patch checking script:
https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=vp
in here:
https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/tree/.tip/bin/vp.py?h=vp
but it probably isn't ready for public consumption yet.
I probably should try to package it properly when there's time...
> > > > - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) > > > > - misc_ids = hygon_nb_misc_ids; > > > > + if (boot_cpu_has(X86_FEATURE_ZEN)) > > > > > > check_for_deprecated_apis: WARNING: arch/x86/kernel/amd_nb.c:395: Do not use boot_cpu_has() - use cpu_feature_enabled() instead. > > > > Sure thing. > > > > How can I enable this check myself? > It is part of my silly patch checking script: > > https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=vp > > in here: > > https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/tree/.tip/bin/vp.py?h=vp > > but it probably isn't ready for public consumption yet. > > I probably should try to package it properly when there's time... Sounds like it would be a valuable addition to checkpatch. Maybe Joe or Andy will find time before you do. -Tony
On Tue, Oct 29, 2024 at 04:15:33PM +0000, Luck, Tony wrote: > > > > > - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) > > > > > - misc_ids = hygon_nb_misc_ids; > > > > > + if (boot_cpu_has(X86_FEATURE_ZEN)) > > > > > > > > check_for_deprecated_apis: WARNING: arch/x86/kernel/amd_nb.c:395: Do not use boot_cpu_has() - use cpu_feature_enabled() instead. > > > > > > Sure thing. > > > > > > How can I enable this check myself? > > It is part of my silly patch checking script: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=vp > > > > in here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/tree/.tip/bin/vp.py?h=vp > > > > but it probably isn't ready for public consumption yet. > > > > I probably should try to package it properly when there's time... > > Sounds like it would be a valuable addition to checkpatch. > > Maybe Joe or Andy will find time before you do. > And if not to checkpatch, then maybe it can be included in the TIP maintainers' handbook? That is, if others are using it or something similar. Thanks, Yazen
> On Tue, Oct 29, 2024 at 04:15:33PM +0000, Luck, Tony wrote: >>>>>> - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) >>>>>> - misc_ids = hygon_nb_misc_ids; >>>>>> + if (boot_cpu_has(X86_FEATURE_ZEN)) >>>>> >>>>> check_for_deprecated_apis: WARNING: arch/x86/kernel/amd_nb.c:395: Do not use boot_cpu_has() - use cpu_feature_enabled() instead. >>>> Do the comments in cpufeature.h need updating? It seems to recommend boot_cpu_has() in most cases and suggests using static_cpu_has() (which is used by cpu_feature_enabled()) only in fast paths. /* * Static testing of CPU features. Used the same as boot_cpu_has(). It * statically patches the target code for additional performance. Use * static_cpu_has() only in fast paths, where every cycle counts. Which * means that the boot_cpu_has() variant is already fast enough for the * majority of cases and you should stick to using it as it is generally * only two instructions: a RIP-relative MOV and a TEST. * ... */ static __always_inline bool _static_cpu_has(u16 bit) > > And if not to checkpatch, then maybe it can be included in the TIP > maintainers' handbook? That is, if others are using it or something > similar. >
On Wed, Oct 30, 2024 at 10:21:38AM -0400, Yazen Ghannam wrote: > And if not to checkpatch, then maybe it can be included in the TIP > maintainers' handbook? That is, if others are using it or something > similar. Nah, this needs to be a normal janitor task. tglx is working on it. :)
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index 37b8244899d8..65884d0613f8 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -385,7 +385,6 @@ static int amd_cache_northbridges(void) */ bool __init early_is_amd_nb(u32 device) { - const struct pci_device_id *misc_ids = amd_nb_misc_ids; const struct pci_device_id *id; u32 vendor = device & 0xffff; @@ -393,11 +392,11 @@ bool __init early_is_amd_nb(u32 device) boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) return false; - if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) - misc_ids = hygon_nb_misc_ids; + if (boot_cpu_has(X86_FEATURE_ZEN)) + return false; device >>= 16; - for (id = misc_ids; id->vendor; id++) + for (id = amd_nb_misc_ids; id->vendor; id++) if (vendor == id->vendor && device == id->device) return true; return false;
The check for early_is_amd_nb() is only useful for systems with GART or the NB_CFG register. Zen-based systems (both AMD and Hygon) have neither, so return early for them. Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com> --- arch/x86/kernel/amd_nb.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)