From: Mario Limonciello <mario.limonciello@xxxxxxx> 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@xxxxxxx> Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx> --- 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@xxxxxxx> +M: Yazen Ghannam <yazen.ghannam@xxxxxxx> +L: linux-kernel@xxxxxxxxxxxxxxx +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@xxxxxxx> 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@xxxxxxx> */ -#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