Re: [PATCH 1/2] x86/amd_nb: Add AMD Family 19h A0-AF IDs

From: Mario Limonciello
Date: Wed Jun 01 2022 - 19:47:51 EST


On 6/1/22 17:30, Bjorn Helgaas wrote:
On Wed, Jun 01, 2022 at 12:21:18PM -0500, Mario Limonciello wrote:
commit 4fb0abfee424 ("x86/amd_nb: Add AMD Family 19h Models (10h-1Fh)
and (A0h-AFh) PCI IDs") had claimed to add the IDs for models A0h-AFh,
but it appears to only have added the models 10h-1Fh.

Add the actual IDs for A0-AF which are needed for SMN communication to
work properly in amd_nb.

Fixes: 4fb0abfee424 ("x86/amd_nb: Add AMD Family 19h Models (10h-1Fh) and (A0h-AFh) PCI IDs")
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
arch/x86/kernel/amd_nb.c | 5 +++++
include/linux/pci_ids.h | 1 +
2 files changed, 6 insertions(+)

diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 190e0f763375..cc8c7cfa9068 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -25,11 +25,13 @@
#define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494
#define PCI_DEVICE_ID_AMD_17H_M60H_DF_F4 0x144c
#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444
+#define PCI_DEVICE_ID_AMD_19H_MA0H_ROOT 0x14b5
#define PCI_DEVICE_ID_AMD_19H_DF_F4 0x1654
#define PCI_DEVICE_ID_AMD_19H_M10H_DF_F4 0x14b1
#define PCI_DEVICE_ID_AMD_19H_M40H_ROOT 0x14b5
#define PCI_DEVICE_ID_AMD_19H_M40H_DF_F4 0x167d
#define PCI_DEVICE_ID_AMD_19H_M50H_DF_F4 0x166e
+#define PCI_DEVICE_ID_AMD_19H_MA0H_DF_F4 0x1728
/* Protect the PCI config register pairs used for SMN. */
static DEFINE_MUTEX(smn_mutex);
@@ -43,6 +45,7 @@ static const struct pci_device_id amd_root_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M10H_ROOT) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_ROOT) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_MA0H_ROOT) },
{}
};
@@ -67,6 +70,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M10H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_DF_F3) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_MA0H_DF_F3) },
{}
};
@@ -85,6 +89,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M10H_DF_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_DF_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F4) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_MA0H_DF_F4) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
{}
};
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 0178823ce8c2..05b4c67a8a2a 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -560,6 +560,7 @@
#define PCI_DEVICE_ID_AMD_19H_M10H_DF_F3 0x14b0
#define PCI_DEVICE_ID_AMD_19H_M40H_DF_F3 0x167c
#define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d
+#define PCI_DEVICE_ID_AMD_19H_MA0H_DF_F3 0x1727

This file is nominally sorted by numeric device ID inside the vendor
section, but that has really deteriorated over time.

It seemed like it was more oriented to be sorted by the macro definition than the ID.


I can't quite figure out the rationale for deciding whether to put
things in amd_nb.c vs pci_ids.h. The IDs in amd_nb.c look basically
the same as this one.

Normally we put things in pci_ids.h if they are used more than one
place. PCI_DEVICE_ID_AMD_19H_MA0H_DF_F3 looks like it's only used in
one place.

I suppose you weren't on CC for patch 2/2 in this series (thanks scripts/get_maintainer.pl), so it's not obvious to you that this ID is used in a second place in patch 2. I didn't want to ping pong in/out from amd_nb.c in the series.

Here is a lore link to it:
https://lore.kernel.org/linux-kernel/20220601172121.18612-2-mario.limonciello@xxxxxxx/


#define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703
#define PCI_DEVICE_ID_AMD_LANCE 0x2000
#define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001
--
2.34.1