linux-kernel - Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model 60h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240718154323.GA54785@yaz-khff2.amd.com>
Date: Thu, 18 Jul 2024 11:43:23 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: Shyam Sundar S K <Shyam-sundar.S-k@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Peter Anvin <hpa@...or.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Muralidhara M K <muralidhara.mk@....com>,
	Avadhut Naik <Avadhut.Naik@....com>,
	Mario Limonciello <mario.limonciello@....com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	x86@...nel.org
Subject: Re: [PATCH v1] x86/amd_nb: Add new PCI IDs for AMD family 0x1a model
 60h

On Thu, Jul 18, 2024 at 07:32:58PM +0530, Shyam Sundar S K wrote:
> Add the new PCI Device IDs to the root IDs and misc ids list to support
> new generation of AMD 1Ah family 60h Models of processors.

Please be consistent with formatting.

"Device" -> "device"

"misc ids" -> "misc IDs" 

"Models" -> "models"

Also, you have "0x1A"  in the $SUBJECT, but you have "1Ah" in the commit
message. I suggest staying with "1Ah" as that is the format used in AMD
documentation.

And "v1" is not necessary in the "[PATCH]" prefix.

Furthermore, if you CC the "x86" alias, then you don't need to CC the
individual x86 maintainers.

> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
> ---
> (As the amd_nb functions are used by PMC and PMF drivers, without these IDs
> being present AMD PMF/PMC probe shall fail.)

This comment can go in the commit message. Otherwise, it'll be lost from
the git history.

The comment is helpful in that it gives a reason *why* these new IDs are
needed.

> 
>  arch/x86/kernel/amd_nb.c | 3 +++
>  include/linux/pci_ids.h  | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 059e5c16af05..61eadde08511 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -26,6 +26,7 @@
>  #define PCI_DEVICE_ID_AMD_19H_M70H_ROOT		0x14e8
>  #define PCI_DEVICE_ID_AMD_1AH_M00H_ROOT		0x153a
>  #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT		0x1507
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT		0x1122
>  #define PCI_DEVICE_ID_AMD_MI200_ROOT		0x14bb
>  #define PCI_DEVICE_ID_AMD_MI300_ROOT		0x14f8
>  
> @@ -63,6 +64,7 @@ static const struct pci_device_id amd_root_ids[] = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M70H_ROOT) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_ROOT) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_ROOT) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_ROOT) },
>  	{}
> @@ -95,6 +97,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M78H_DF_F3) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI200_DF_F3) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_MI300_DF_F3) },
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 76a8f2d6bd64..bbe8f3dfa813 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -580,6 +580,7 @@
>  #define PCI_DEVICE_ID_AMD_19H_M78H_DF_F3 0x12fb
>  #define PCI_DEVICE_ID_AMD_1AH_M00H_DF_F3 0x12c3
>  #define PCI_DEVICE_ID_AMD_1AH_M20H_DF_F3 0x16fb
> +#define PCI_DEVICE_ID_AMD_1AH_M60H_DF_F3 0x124b
>  #define PCI_DEVICE_ID_AMD_1AH_M70H_DF_F3 0x12bb
>  #define PCI_DEVICE_ID_AMD_MI200_DF_F3	0x14d3
>  #define PCI_DEVICE_ID_AMD_MI300_DF_F3	0x152b
> -- 

I can confirm that the IDs are correct.

Besides the formatting issues, this looks good to me.

Reviewed-by: Yazen Ghannam <yazen.ghannam@....com>

Thanks,
Yazen