[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241023172150.659002-11-yazen.ghannam@amd.com>
Date: Wed, 23 Oct 2024 17:21:44 +0000
From: Yazen Ghannam <yazen.ghannam@....com>
To: <linux-edac@...r.kernel.org>
CC: <linux-kernel@...r.kernel.org>, <tony.luck@...el.com>, <x86@...nel.org>,
<avadhut.naik@....com>, <john.allen@....com>, <mario.limonciello@....com>,
<bhelgaas@...gle.com>, <Shyam-sundar.S-k@....com>, <richard.gong@....com>,
<jdelvare@...e.com>, <linux@...ck-us.net>, <clemens@...isch.de>,
<hdegoede@...hat.com>, <ilpo.jarvinen@...ux.intel.com>,
<linux-pci@...r.kernel.org>, <linux-hwmon@...r.kernel.org>,
<platform-driver-x86@...r.kernel.org>, <naveenkrishna.chatradhi@....com>,
<carlos.bilbao.osdev@...il.com>, Yazen Ghannam <yazen.ghannam@....com>
Subject: [PATCH 10/16] x86/amd_nb: Move SMN access code to a new amd_smn driver
From: Mario Limonciello <mario.limonciello@....com>
SMN access was bolted into amd_nb mostly as convenience. This has
limitations though that require incurring tech debt to keep it working.
Move SMN access to its own driver.
[Yazen: Adjust commit message and add MAINTAINERS entry]
Signed-off-by: Mario Limonciello <mario.limonciello@....com>
Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
---
MAINTAINERS | 8 +++
arch/x86/Kconfig | 3 +
arch/x86/include/asm/amd_nb.h | 3 -
arch/x86/include/asm/amd_smn.h | 11 +++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/amd_nb.c | 89 -----------------------
arch/x86/kernel/amd_smn.c | 101 +++++++++++++++++++++++++++
arch/x86/pci/fixup.c | 4 +-
drivers/edac/Kconfig | 1 +
drivers/edac/amd64_edac.c | 1 +
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/k10temp.c | 2 +-
drivers/platform/x86/amd/pmc/Kconfig | 2 +-
drivers/platform/x86/amd/pmc/pmc.c | 2 +-
drivers/platform/x86/amd/pmf/Kconfig | 2 +-
drivers/platform/x86/amd/pmf/core.c | 2 +-
drivers/ras/amd/atl/Kconfig | 1 +
drivers/ras/amd/atl/internal.h | 1 +
18 files changed, 136 insertions(+), 100 deletions(-)
create mode 100644 arch/x86/include/asm/amd_smn.h
create mode 100644 arch/x86/kernel/amd_smn.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ca246aef7ab..cb9af4353b31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1180,6 +1180,14 @@ S: Maintained
F: Documentation/hid/amd-sfh*
F: drivers/hid/amd-sfh-hid/
+AMD SMN DRIVER
+M: Mario Limonciello <mario.limonciello@....com>
+M: Yazen Ghannam <yazen.ghannam@....com>
+L: linux-kernel@...r.kernel.org
+S: Supported
+F: arch/x86/include/asm/amd_smn.h
+F: arch/x86/kernel/amd_smn.c
+
AMD SPI DRIVER
M: Sanjay R Mehta <sanju.mehta@....com>
S: Maintained
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ba5252d8e21c..a03ffa5b6bb1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -3128,6 +3128,9 @@ config AMD_NODE
def_bool y
depends on CPU_SUP_AMD && PCI
+config AMD_SMN
+ def_bool y
+ depends on AMD_NODE
endmenu
menu "Binary Emulations"
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index b3b42e585655..55c03d3495bc 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -21,9 +21,6 @@ extern int amd_numa_init(void);
extern int amd_get_subcaches(int);
extern int amd_set_subcaches(int, unsigned long);
-int __must_check amd_smn_read(u16 node, u32 address, u32 *value);
-int __must_check amd_smn_write(u16 node, u32 address, u32 value);
-
struct amd_l3_cache {
unsigned indices;
u8 subcaches[4];
diff --git a/arch/x86/include/asm/amd_smn.h b/arch/x86/include/asm/amd_smn.h
new file mode 100644
index 000000000000..6850de69f863
--- /dev/null
+++ b/arch/x86/include/asm/amd_smn.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_AMD_SMN_H
+#define _ASM_X86_AMD_SMN_H
+
+#include <linux/types.h>
+#include <asm/amd_node.h>
+
+int __must_check amd_smn_read(u16 node, u32 address, u32 *value);
+int __must_check amd_smn_write(u16 node, u32 address, u32 value);
+
+#endif /* _ASM_X86_AMD_SMN_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index b43eb7e384eb..1df22821f72d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_HPET_TIMER) += hpet.o
obj-$(CONFIG_AMD_NB) += amd_nb.o
obj-$(CONFIG_AMD_NODE) += amd_node.o
+obj-$(CONFIG_AMD_SMN) += amd_smn.o
obj-$(CONFIG_DEBUG_NMI_SELFTEST) += nmi_selftest.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvmclock.o
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 9b159f9b4a11..10cdeddeda02 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -15,9 +15,6 @@
#include <linux/pci_ids.h>
#include <asm/amd_nb.h>
-/* Protect the PCI config register pairs used for SMN. */
-static DEFINE_MUTEX(smn_mutex);
-
static u32 *flush_words;
static const struct pci_device_id amd_nb_misc_ids[] = {
@@ -59,92 +56,6 @@ struct amd_northbridge *node_to_amd_nb(int node)
}
EXPORT_SYMBOL_GPL(node_to_amd_nb);
-/*
- * SMN accesses may fail in ways that are difficult to detect here in the called
- * functions amd_smn_read() and amd_smn_write(). Therefore, callers must do
- * their own checking based on what behavior they expect.
- *
- * For SMN reads, the returned value may be zero if the register is Read-as-Zero.
- * Or it may be a "PCI Error Response", e.g. all 0xFFs. The "PCI Error Response"
- * can be checked here, and a proper error code can be returned.
- *
- * But the Read-as-Zero response cannot be verified here. A value of 0 may be
- * correct in some cases, so callers must check that this correct is for the
- * register/fields they need.
- *
- * For SMN writes, success can be determined through a "write and read back"
- * However, this is not robust when done here.
- *
- * Possible issues:
- *
- * 1) Bits that are "Write-1-to-Clear". In this case, the read value should
- * *not* match the write value.
- *
- * 2) Bits that are "Read-as-Zero"/"Writes-Ignored". This information cannot be
- * known here.
- *
- * 3) Bits that are "Reserved / Set to 1". Ditto above.
- *
- * Callers of amd_smn_write() should do the "write and read back" check
- * themselves, if needed.
- *
- * For #1, they can see if their target bits got cleared.
- *
- * For #2 and #3, they can check if their target bits got set as intended.
- *
- * This matches what is done for RDMSR/WRMSR. As long as there's no #GP, then
- * the operation is considered a success, and the caller does their own
- * checking.
- */
-static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write)
-{
- struct pci_dev *root;
- int err = -ENODEV;
-
- if (node >= amd_northbridges.num)
- goto out;
-
- root = node_to_amd_nb(node)->root;
- if (!root)
- goto out;
-
- mutex_lock(&smn_mutex);
-
- err = pci_write_config_dword(root, 0x60, address);
- if (err) {
- pr_warn("Error programming SMN address 0x%x.\n", address);
- goto out_unlock;
- }
-
- err = (write ? pci_write_config_dword(root, 0x64, *value)
- : pci_read_config_dword(root, 0x64, value));
-
-out_unlock:
- mutex_unlock(&smn_mutex);
-
-out:
- return err;
-}
-
-int __must_check amd_smn_read(u16 node, u32 address, u32 *value)
-{
- int err = __amd_smn_rw(node, address, value, false);
-
- if (PCI_POSSIBLE_ERROR(*value)) {
- err = -ENODEV;
- *value = 0;
- }
-
- return err;
-}
-EXPORT_SYMBOL_GPL(amd_smn_read);
-
-int __must_check amd_smn_write(u16 node, u32 address, u32 value)
-{
- return __amd_smn_rw(node, address, &value, true);
-}
-EXPORT_SYMBOL_GPL(amd_smn_write);
-
static int amd_cache_northbridges(void)
{
struct amd_northbridge *nb;
diff --git a/arch/x86/kernel/amd_smn.c b/arch/x86/kernel/amd_smn.c
new file mode 100644
index 000000000000..06160a9e2444
--- /dev/null
+++ b/arch/x86/kernel/amd_smn.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AMD SMN driver, used for register access
+ * Copyright 2024 Advanced Micro Devices, Inc.
+ */
+
+#define pr_fmt(fmt) "amd_smn: " fmt
+
+#include <linux/pci.h>
+
+#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
+
+/* Protect the PCI config register pairs used for SMN. */
+static DEFINE_MUTEX(smn_mutex);
+
+/*
+ * SMN accesses may fail in ways that are difficult to detect here in the called
+ * functions amd_smn_read() and amd_smn_write(). Therefore, callers must do
+ * their own checking based on what behavior they expect.
+ *
+ * For SMN reads, the returned value may be zero if the register is Read-as-Zero.
+ * Or it may be a "PCI Error Response", e.g. all 0xFFs. The "PCI Error Response"
+ * can be checked here, and a proper error code can be returned.
+ *
+ * But the Read-as-Zero response cannot be verified here. A value of 0 may be
+ * correct in some cases, so callers must check that this correct is for the
+ * register/fields they need.
+ *
+ * For SMN writes, success can be determined through a "write and read back"
+ * However, this is not robust when done here.
+ *
+ * Possible issues:
+ *
+ * 1) Bits that are "Write-1-to-Clear". In this case, the read value should
+ * *not* match the write value.
+ *
+ * 2) Bits that are "Read-as-Zero"/"Writes-Ignored". This information cannot be
+ * known here.
+ *
+ * 3) Bits that are "Reserved / Set to 1". Ditto above.
+ *
+ * Callers of amd_smn_write() should do the "write and read back" check
+ * themselves, if needed.
+ *
+ * For #1, they can see if their target bits got cleared.
+ *
+ * For #2 and #3, they can check if their target bits got set as intended.
+ *
+ * This matches what is done for RDMSR/WRMSR. As long as there's no #GP, then
+ * the operation is considered a success, and the caller does their own
+ * checking.
+ */
+static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write)
+{
+ struct pci_dev *root;
+ int err = -ENODEV;
+
+ if (node >= amd_nb_num())
+ goto out;
+
+ root = node_to_amd_nb(node)->root;
+ if (!root)
+ goto out;
+
+ mutex_lock(&smn_mutex);
+
+ err = pci_write_config_dword(root, 0x60, address);
+ if (err) {
+ pr_warn("Error programming SMN address 0x%x.\n", address);
+ goto out_unlock;
+ }
+
+ err = (write ? pci_write_config_dword(root, 0x64, *value)
+ : pci_read_config_dword(root, 0x64, value));
+
+out_unlock:
+ mutex_unlock(&smn_mutex);
+
+out:
+ return err;
+}
+
+int __must_check amd_smn_read(u16 node, u32 address, u32 *value)
+{
+ int err = __amd_smn_rw(node, address, value, false);
+
+ if (PCI_POSSIBLE_ERROR(*value)) {
+ err = -ENODEV;
+ *value = 0;
+ }
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(amd_smn_read);
+
+int __must_check amd_smn_write(u16 node, u32 address, u32 value)
+{
+ return __amd_smn_rw(node, address, &value, true);
+}
+EXPORT_SYMBOL_GPL(amd_smn_write);
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 98a9bb92d75c..e8ea627c22b4 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -9,7 +9,7 @@
#include <linux/pci.h>
#include <linux/suspend.h>
#include <linux/vgaarb.h>
-#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
#include <asm/hpet.h>
#include <asm/pci_x86.h>
@@ -828,7 +828,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7910, rs690_fix_64bit_dma);
#endif
-#ifdef CONFIG_AMD_NB
+#ifdef CONFIG_AMD_SMN
#define AMD_15B8_RCC_DEV2_EPF0_STRAP2 0x10136008
#define AMD_15B8_RCC_DEV2_EPF0_STRAP2_NO_SOFT_RESET_DEV2_F0_MASK 0x00000080L
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 81af6c344d6b..acdd920b8769 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -78,6 +78,7 @@ config EDAC_GHES
config EDAC_AMD64
tristate "AMD64 (Opteron, Athlon64)"
depends on AMD_NB && EDAC_DECODE_MCE
+ depends on AMD_SMN
imply AMD_ATL
help
Support for error detection and correction of DRAM ECC errors on
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index ddfbdb66b794..b0ee0593137b 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2,6 +2,7 @@
#include <linux/ras.h>
#include "amd64_edac.h"
#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
static struct edac_pci_ctl_info *pci_ctl;
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 08a3c863f80a..bf2bd7098f2c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -324,7 +324,7 @@ config SENSORS_K8TEMP
config SENSORS_K10TEMP
tristate "AMD Family 10h+ temperature sensor"
- depends on X86 && PCI && AMD_NB
+ depends on X86 && PCI && AMD_SMN
help
If you say yes here you get support for the temperature
sensor(s) inside your CPU. Supported are later revisions of
diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 7dc19c5d62ac..1a16640d2fa1 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -20,7 +20,7 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/pci_ids.h>
-#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
#include <asm/processor.h>
MODULE_DESCRIPTION("AMD Family 10h+ CPU core temperature monitor");
diff --git a/drivers/platform/x86/amd/pmc/Kconfig b/drivers/platform/x86/amd/pmc/Kconfig
index 94f9563d8be7..d9cae227aff9 100644
--- a/drivers/platform/x86/amd/pmc/Kconfig
+++ b/drivers/platform/x86/amd/pmc/Kconfig
@@ -5,7 +5,7 @@
config AMD_PMC
tristate "AMD SoC PMC driver"
- depends on ACPI && PCI && RTC_CLASS && AMD_NB
+ depends on ACPI && PCI && RTC_CLASS && AMD_SMN
depends on SUSPEND
select SERIO
help
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index bbb8edb62e00..663cf8d671c7 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -10,7 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
#include <linux/acpi.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index f4fa8bd8bda8..67913f64faf3 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -7,7 +7,7 @@ config AMD_PMF
tristate "AMD Platform Management Framework"
depends on ACPI && PCI
depends on POWER_SUPPLY
- depends on AMD_NB
+ depends on AMD_SMN
select ACPI_PLATFORM_PROFILE
depends on TEE && AMDTEE
depends on AMD_SFH_HID
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index d6af0ca036f1..21f9ec19941b 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -8,7 +8,7 @@
* Author: Shyam Sundar S K <Shyam-sundar.S-k@....com>
*/
-#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
#include <linux/debugfs.h>
#include <linux/iopoll.h>
#include <linux/module.h>
diff --git a/drivers/ras/amd/atl/Kconfig b/drivers/ras/amd/atl/Kconfig
index 551680073e43..819ac593e743 100644
--- a/drivers/ras/amd/atl/Kconfig
+++ b/drivers/ras/amd/atl/Kconfig
@@ -10,6 +10,7 @@
config AMD_ATL
tristate "AMD Address Translation Library"
depends on AMD_NB && X86_64 && RAS
+ depends on AMD_SMN
depends on MEMORY_FAILURE
default N
help
diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
index 143d04c779a8..49cd3e95e0c6 100644
--- a/drivers/ras/amd/atl/internal.h
+++ b/drivers/ras/amd/atl/internal.h
@@ -18,6 +18,7 @@
#include <linux/ras.h>
#include <asm/amd_nb.h>
+#include <asm/amd_smn.h>
#include "reg_fields.h"
--
2.43.0
Powered by blists - more mailing lists