diff mbox series

[03/16] x86/amd_nb: Clean up early_is_amd_nb()

Message ID 20241023172150.659002-4-yazen.ghannam@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series AMD NB and SMN rework | expand

Commit Message

Yazen Ghannam Oct. 23, 2024, 5:21 p.m. UTC
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(-)

Comments

Borislav Petkov Oct. 25, 2024, 3:58 p.m. UTC | #1
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.
Yazen Ghannam Oct. 29, 2024, 2:39 p.m. UTC | #2
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
Borislav Petkov Oct. 29, 2024, 3:08 p.m. UTC | #3
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...
Luck, Tony Oct. 29, 2024, 4:15 p.m. UTC | #4
> > > > -       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
Yazen Ghannam Oct. 30, 2024, 2:21 p.m. UTC | #5
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
Sohil Mehta Oct. 31, 2024, 12:53 a.m. UTC | #6
> 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.
>
Borislav Petkov Oct. 31, 2024, 10:24 a.m. UTC | #7
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 mbox series

Patch

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;