From 9aae1baa1c5d8b7229b6de38dc9dc17efcb8c55d Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz Date: Thu, 29 Feb 2024 15:51:00 +0100 Subject: [PATCH 01/15] x86, arm: Add missing license tag to syscall tables files syscall*.tbl files were added to make it easier to check which system calls are supported on each architecture and to check for their numbers. Arm and x86 files lack Linux-syscall-note license exception present in files for all other architectures. Signed-off-by: Marcin Juszkiewicz Signed-off-by: Borislav Petkov (AMD) Cc: linux-arm-kernel@lists.infradead.org Link: https://lore.kernel.org/r/20240229145101.553998-1-marcin@juszkiewicz.com.pl --- arch/arm/tools/syscall.tbl | 1 + arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + 3 files changed, 3 insertions(+) diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl index 2ed7d229c8f9..23c98203c40f 100644 --- a/arch/arm/tools/syscall.tbl +++ b/arch/arm/tools/syscall.tbl @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note # # Linux system call numbers and entry vectors # diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl index 7fd1f57ad3d3..9436b67fe832 100644 --- a/arch/x86/entry/syscalls/syscall_32.tbl +++ b/arch/x86/entry/syscalls/syscall_32.tbl @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note # # 32-bit system call numbers and entry vectors # diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index a396f6e6ab5b..129bdd475cf0 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note # # 64-bit system call numbers and entry vectors # From 1d2a03d26a69e4e781077f5fbb5466143928c737 Mon Sep 17 00:00:00 2001 From: Christian Heusel Date: Fri, 31 May 2024 13:17:58 +0200 Subject: [PATCH 02/15] tools/x86/kcpuid: Add missing dir via Makefile So far the Makefile just installed the csv into $(HWDATADIR)/cpuid.csv, which made it unaware about $DESTDIR. Add $DESTDIR to the install command and while at it also create the directory, should it not exist already. This eases the packaging of kcpuid and allows i.e. for the install on Arch to look like this: $ make BINDIR=/usr/bin DESTDIR="$pkgdir" -C tools/arch/x86/kcpuid install Some background on DESTDIR: DESTDIR is commonly used in packaging for staged installs (regardless of the used package manager): https://www.gnu.org/prep/standards/html_node/DESTDIR.html So the package is built and installed into a directory which the package manager later picks up and creates some archive from it. What is specific to Arch Linux here is only the usage of $pkgdir in the example, DESTDIR itself is widely used. [ bp: Extend the commit message with Christian's info on DESTDIR as a GNU coding standards thing. ] Signed-off-by: Christian Heusel Signed-off-by: Borislav Petkov (AMD) Link: https://lore.kernel.org/r/20240531111757.719528-2-christian@heusel.eu --- tools/arch/x86/kcpuid/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/arch/x86/kcpuid/Makefile b/tools/arch/x86/kcpuid/Makefile index 87b554fab14b..d0b4b0ed10ff 100644 --- a/tools/arch/x86/kcpuid/Makefile +++ b/tools/arch/x86/kcpuid/Makefile @@ -19,6 +19,6 @@ clean : @rm -f kcpuid install : kcpuid - install -d $(DESTDIR)$(BINDIR) + install -d $(DESTDIR)$(BINDIR) $(DESTDIR)$(HWDATADIR) install -m 755 -p kcpuid $(DESTDIR)$(BINDIR)/kcpuid - install -m 444 -p cpuid.csv $(HWDATADIR)/cpuid.csv + install -m 444 -p cpuid.csv $(DESTDIR)$(HWDATADIR)/cpuid.csv From f97a8b9170a0524c5432791cd5852bf797934af4 Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:54 -0500 Subject: [PATCH 03/15] EDAC/amd64: Remove unused register accesses A number of UMC registers are read only for the purpose of debug printing. They are not used in any calculations. Nor do they have any specific debug value. Remove them. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-1-ffde21931c3f@amd.com --- drivers/edac/amd64_edac.c | 18 +----------------- drivers/edac/amd64_edac.h | 4 ---- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index a17f3c0cdfa6..dfc4fb8f7a7a 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -20,7 +20,6 @@ static inline u32 get_umc_reg(struct amd64_pvt *pvt, u32 reg) return reg; switch (reg) { - case UMCCH_ADDR_CFG: return UMCCH_ADDR_CFG_DDR5; case UMCCH_ADDR_MASK_SEC: return UMCCH_ADDR_MASK_SEC_DDR5; case UMCCH_DIMM_CFG: return UMCCH_DIMM_CFG_DDR5; } @@ -1341,22 +1340,15 @@ static void umc_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl) static void umc_dump_misc_regs(struct amd64_pvt *pvt) { struct amd64_umc *umc; - u32 i, tmp, umc_base; + u32 i; for_each_umc(i) { - umc_base = get_umc_base(i); umc = &pvt->umc[i]; edac_dbg(1, "UMC%d DIMM cfg: 0x%x\n", i, umc->dimm_cfg); edac_dbg(1, "UMC%d UMC cfg: 0x%x\n", i, umc->umc_cfg); edac_dbg(1, "UMC%d SDP ctrl: 0x%x\n", i, umc->sdp_ctrl); edac_dbg(1, "UMC%d ECC ctrl: 0x%x\n", i, umc->ecc_ctrl); - - amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ECC_BAD_SYMBOL, &tmp); - edac_dbg(1, "UMC%d ECC bad symbol: 0x%x\n", i, tmp); - - amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_UMC_CAP, &tmp); - edac_dbg(1, "UMC%d UMC cap: 0x%x\n", i, tmp); edac_dbg(1, "UMC%d UMC cap high: 0x%x\n", i, umc->umc_cap_hi); edac_dbg(1, "UMC%d ECC capable: %s, ChipKill ECC capable: %s\n", @@ -1369,14 +1361,6 @@ static void umc_dump_misc_regs(struct amd64_pvt *pvt) edac_dbg(1, "UMC%d x16 DIMMs present: %s\n", i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no"); - if (umc->dram_type == MEM_LRDDR4 || umc->dram_type == MEM_LRDDR5) { - amd_smn_read(pvt->mc_node_id, - umc_base + get_umc_reg(pvt, UMCCH_ADDR_CFG), - &tmp); - edac_dbg(1, "UMC%d LRDIMM %dx rank multiply\n", - i, 1 << ((tmp >> 4) & 0x3)); - } - umc_debug_display_dimm_sizes(pvt, i); } } diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index b879b12971e7..17228d07de4c 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -256,15 +256,11 @@ #define UMCCH_ADDR_MASK 0x20 #define UMCCH_ADDR_MASK_SEC 0x28 #define UMCCH_ADDR_MASK_SEC_DDR5 0x30 -#define UMCCH_ADDR_CFG 0x30 -#define UMCCH_ADDR_CFG_DDR5 0x40 #define UMCCH_DIMM_CFG 0x80 #define UMCCH_DIMM_CFG_DDR5 0x90 #define UMCCH_UMC_CFG 0x100 #define UMCCH_SDP_CTRL 0x104 #define UMCCH_ECC_CTRL 0x14C -#define UMCCH_ECC_BAD_SYMBOL 0xD90 -#define UMCCH_UMC_CAP 0xDF0 #define UMCCH_UMC_CAP_HI 0xDF4 /* UMC CH bitfields */ From 5ac6293047cf5de6daca662347c19347e856c2a5 Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:55 -0500 Subject: [PATCH 04/15] EDAC/amd64: Check return value of amd_smn_read() Check the return value of amd_smn_read() before saving a value. This ensures invalid values aren't saved. The struct umc instance is initialized to 0 during memory allocation. Therefore, a bad read will keep the value as 0 providing the expected Read-as-Zero behavior. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-2-ffde21931c3f@amd.com --- drivers/edac/amd64_edac.c | 51 ++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index dfc4fb8f7a7a..ddfbdb66b794 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -1438,6 +1438,7 @@ static void umc_read_base_mask(struct amd64_pvt *pvt) u32 *base, *base_sec; u32 *mask, *mask_sec; int cs, umc; + u32 tmp; for_each_umc(umc) { umc_base_reg = get_umc_base(umc) + UMCCH_BASE_ADDR; @@ -1450,13 +1451,17 @@ static void umc_read_base_mask(struct amd64_pvt *pvt) base_reg = umc_base_reg + (cs * 4); base_reg_sec = umc_base_reg_sec + (cs * 4); - if (!amd_smn_read(pvt->mc_node_id, base_reg, base)) + if (!amd_smn_read(pvt->mc_node_id, base_reg, &tmp)) { + *base = tmp; edac_dbg(0, " DCSB%d[%d]=0x%08x reg: 0x%x\n", umc, cs, *base, base_reg); + } - if (!amd_smn_read(pvt->mc_node_id, base_reg_sec, base_sec)) + if (!amd_smn_read(pvt->mc_node_id, base_reg_sec, &tmp)) { + *base_sec = tmp; edac_dbg(0, " DCSB_SEC%d[%d]=0x%08x reg: 0x%x\n", umc, cs, *base_sec, base_reg_sec); + } } umc_mask_reg = get_umc_base(umc) + UMCCH_ADDR_MASK; @@ -1469,13 +1474,17 @@ static void umc_read_base_mask(struct amd64_pvt *pvt) mask_reg = umc_mask_reg + (cs * 4); mask_reg_sec = umc_mask_reg_sec + (cs * 4); - if (!amd_smn_read(pvt->mc_node_id, mask_reg, mask)) + if (!amd_smn_read(pvt->mc_node_id, mask_reg, &tmp)) { + *mask = tmp; edac_dbg(0, " DCSM%d[%d]=0x%08x reg: 0x%x\n", umc, cs, *mask, mask_reg); + } - if (!amd_smn_read(pvt->mc_node_id, mask_reg_sec, mask_sec)) + if (!amd_smn_read(pvt->mc_node_id, mask_reg_sec, &tmp)) { + *mask_sec = tmp; edac_dbg(0, " DCSM_SEC%d[%d]=0x%08x reg: 0x%x\n", umc, cs, *mask_sec, mask_reg_sec); + } } } } @@ -2894,7 +2903,7 @@ static void umc_read_mc_regs(struct amd64_pvt *pvt) { u8 nid = pvt->mc_node_id; struct amd64_umc *umc; - u32 i, umc_base; + u32 i, tmp, umc_base; /* Read registers from each UMC */ for_each_umc(i) { @@ -2902,11 +2911,20 @@ static void umc_read_mc_regs(struct amd64_pvt *pvt) umc_base = get_umc_base(i); umc = &pvt->umc[i]; - amd_smn_read(nid, umc_base + get_umc_reg(pvt, UMCCH_DIMM_CFG), &umc->dimm_cfg); - amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg); - amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl); - amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl); - amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &umc->umc_cap_hi); + if (!amd_smn_read(nid, umc_base + get_umc_reg(pvt, UMCCH_DIMM_CFG), &tmp)) + umc->dimm_cfg = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &tmp)) + umc->umc_cfg = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &tmp)) + umc->sdp_ctrl = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &tmp)) + umc->ecc_ctrl = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &tmp)) + umc->umc_cap_hi = tmp; } } @@ -3635,16 +3653,21 @@ static void gpu_read_mc_regs(struct amd64_pvt *pvt) { u8 nid = pvt->mc_node_id; struct amd64_umc *umc; - u32 i, umc_base; + u32 i, tmp, umc_base; /* Read registers from each UMC */ for_each_umc(i) { umc_base = gpu_get_umc_base(pvt, i, 0); umc = &pvt->umc[i]; - amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg); - amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl); - amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl); + if (!amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &tmp)) + umc->umc_cfg = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &tmp)) + umc->sdp_ctrl = tmp; + + if (!amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &tmp)) + umc->ecc_ctrl = tmp; } } From c2d79cc5455c891de6c93e1e0c73d806e299c54f Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:56 -0500 Subject: [PATCH 05/15] hwmon: (k10temp) Check return value of amd_smn_read() Check the return value of amd_smn_read() before saving a value. This ensures invalid values aren't saved or used. There are three cases here with slightly different behavior: 1) read_tempreg_nb_zen(): This is a function pointer which does not include a return code. In this case, set the register value to 0 on failure. This enforces Read-as-Zero behavior. 2) k10temp_read_temp(): This function does have return codes, so return the error code from the failed register read. Continued operation is not necessary, since there is no valid data from the register. Furthermore, if the register value was set to 0, then the following operation would underflow. 3) k10temp_get_ccd_support(): This function reads the same register from multiple CCD instances in a loop. And a bitmask is formed if a specific bit is set in each register instance. The loop should continue on a failed register read, skipping the bit check. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-3-ffde21931c3f@amd.com --- drivers/hwmon/k10temp.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index 8092312c0a87..6cad35e7f182 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -153,8 +153,9 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval) static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval) { - amd_smn_read(amd_pci_dev_to_node_id(pdev), - ZEN_REPORTED_TEMP_CTRL_BASE, regval); + if (amd_smn_read(amd_pci_dev_to_node_id(pdev), + ZEN_REPORTED_TEMP_CTRL_BASE, regval)) + *regval = 0; } static long get_raw_temp(struct k10temp_data *data) @@ -205,6 +206,7 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel, long *val) { struct k10temp_data *data = dev_get_drvdata(dev); + int ret = -EOPNOTSUPP; u32 regval; switch (attr) { @@ -221,13 +223,17 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel, *val = 0; break; case 2 ... 13: /* Tccd{1-12} */ - amd_smn_read(amd_pci_dev_to_node_id(data->pdev), - ZEN_CCD_TEMP(data->ccd_offset, channel - 2), - ®val); + ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev), + ZEN_CCD_TEMP(data->ccd_offset, channel - 2), + ®val); + + if (ret) + return ret; + *val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000; break; default: - return -EOPNOTSUPP; + return ret; } break; case hwmon_temp_max: @@ -243,7 +249,7 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel, - ((regval >> 24) & 0xf)) * 500 + 52000; break; default: - return -EOPNOTSUPP; + return ret; } return 0; } @@ -381,8 +387,20 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev, int i; for (i = 0; i < limit; i++) { - amd_smn_read(amd_pci_dev_to_node_id(pdev), - ZEN_CCD_TEMP(data->ccd_offset, i), ®val); + /* + * Ignore inaccessible CCDs. + * + * Some systems will return a register value of 0, and the TEMP_VALID + * bit check below will naturally fail. + * + * Other systems will return a PCI_ERROR_RESPONSE (0xFFFFFFFF) for + * the register value. And this will incorrectly pass the TEMP_VALID + * bit check. + */ + if (amd_smn_read(amd_pci_dev_to_node_id(pdev), + ZEN_CCD_TEMP(data->ccd_offset, i), ®val)) + continue; + if (regval & ZEN_CCD_TEMP_VALID) data->show_temp |= BIT(TCCD_BIT(i)); } From dc5243921be1b6a0b4259dbcec3dc95016ad8427 Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:57 -0500 Subject: [PATCH 06/15] x86/amd_nb: Enhance SMN access error checking AMD Zen-based systems use a System Management Network (SMN) that provides access to implementation-specific registers. SMN accesses are done indirectly through an index/data pair in PCI config space. The accesses can fail for a variety of reasons. Include code comments to describe some possible scenarios. Require error checking for callers of amd_smn_read() and amd_smn_write(). This is needed because many error conditions cannot be checked by these functions. [ bp: Touchup comment. ] Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-4-ffde21931c3f@amd.com --- arch/x86/include/asm/amd_nb.h | 4 ++-- arch/x86/kernel/amd_nb.c | 44 +++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h index 5c37944c8a5e..6f3b6aef47ba 100644 --- a/arch/x86/include/asm/amd_nb.h +++ b/arch/x86/include/asm/amd_nb.h @@ -21,8 +21,8 @@ extern int amd_numa_init(void); extern int amd_get_subcaches(int); extern int amd_set_subcaches(int, unsigned long); -extern int amd_smn_read(u16 node, u32 address, u32 *value); -extern int amd_smn_write(u16 node, u32 address, u32 value); +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; diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index 027a8c7a2c9e..059e5c16af05 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -180,6 +180,43 @@ static struct pci_dev *next_northbridge(struct pci_dev *dev, return dev; } +/* + * 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; @@ -202,9 +239,6 @@ static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write) err = (write ? pci_write_config_dword(root, 0x64, *value) : pci_read_config_dword(root, 0x64, value)); - if (err) - pr_warn("Error %s SMN address 0x%x.\n", - (write ? "writing to" : "reading from"), address); out_unlock: mutex_unlock(&smn_mutex); @@ -213,7 +247,7 @@ static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write) return err; } -int amd_smn_read(u16 node, u32 address, u32 *value) +int __must_check amd_smn_read(u16 node, u32 address, u32 *value) { int err = __amd_smn_rw(node, address, value, false); @@ -226,7 +260,7 @@ int amd_smn_read(u16 node, u32 address, u32 *value) } EXPORT_SYMBOL_GPL(amd_smn_read); -int amd_smn_write(u16 node, u32 address, u32 value) +int __must_check amd_smn_write(u16 node, u32 address, u32 value) { return __amd_smn_rw(node, address, &value, true); } From cc66126fd317a70d8612a8356ad512a1539abd75 Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:58 -0500 Subject: [PATCH 07/15] hwmon: (k10temp) Define a helper function to read CCD temperature The CCD temperature register is read in two places. These reads are done using an AMD SMN access, and a number of parameters are needed for the operation. Move the SMN access and parameter gathering into a helper function in order to simplify the code flow. This also has a benefit of centralizing the hardware register access in a single place in case fixes or special decoding is required. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-5-ffde21931c3f@amd.com --- drivers/hwmon/k10temp.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index 6cad35e7f182..315c52de6e54 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -158,6 +158,13 @@ static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval) *regval = 0; } +static int read_ccd_temp_reg(struct k10temp_data *data, int ccd, u32 *regval) +{ + u16 node_id = amd_pci_dev_to_node_id(data->pdev); + + return amd_smn_read(node_id, ZEN_CCD_TEMP(data->ccd_offset, ccd), regval); +} + static long get_raw_temp(struct k10temp_data *data) { u32 regval; @@ -223,9 +230,7 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel, *val = 0; break; case 2 ... 13: /* Tccd{1-12} */ - ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev), - ZEN_CCD_TEMP(data->ccd_offset, channel - 2), - ®val); + ret = read_ccd_temp_reg(data, channel - 2, ®val); if (ret) return ret; @@ -397,8 +402,7 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev, * the register value. And this will incorrectly pass the TEMP_VALID * bit check. */ - if (amd_smn_read(amd_pci_dev_to_node_id(pdev), - ZEN_CCD_TEMP(data->ccd_offset, i), ®val)) + if (read_ccd_temp_reg(data, i, ®val)) continue; if (regval & ZEN_CCD_TEMP_VALID) From a8bc4165d237f4a6bddbab55d2b6592b87341f0a Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:12:59 -0500 Subject: [PATCH 08/15] hwmon: (k10temp) Reduce k10temp_get_ccd_support() parameters Currently, k10temp_get_ccd_support() takes as input "pdev" and "data". However, "pdev" is already included in "data". Furthermore, the "pdev" parameter is no longer used in k10temp_get_ccd_support(), since its use was moved into read_ccd_temp_reg(). Drop the "pdev" input parameter as it is no longer needed. No functional change is intended. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Mario Limonciello Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-6-ffde21931c3f@amd.com --- drivers/hwmon/k10temp.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index 315c52de6e54..6deb272c7cef 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -385,8 +385,7 @@ static const struct hwmon_chip_info k10temp_chip_info = { .info = k10temp_info, }; -static void k10temp_get_ccd_support(struct pci_dev *pdev, - struct k10temp_data *data, int limit) +static void k10temp_get_ccd_support(struct k10temp_data *data, int limit) { u32 regval; int i; @@ -456,18 +455,18 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id) case 0x11: /* Zen APU */ case 0x18: /* Zen+ APU */ data->ccd_offset = 0x154; - k10temp_get_ccd_support(pdev, data, 4); + k10temp_get_ccd_support(data, 4); break; case 0x31: /* Zen2 Threadripper */ case 0x60: /* Renoir */ case 0x68: /* Lucienne */ case 0x71: /* Zen2 */ data->ccd_offset = 0x154; - k10temp_get_ccd_support(pdev, data, 8); + k10temp_get_ccd_support(data, 8); break; case 0xa0 ... 0xaf: data->ccd_offset = 0x300; - k10temp_get_ccd_support(pdev, data, 8); + k10temp_get_ccd_support(data, 8); break; } } else if (boot_cpu_data.x86 == 0x19) { @@ -481,21 +480,21 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id) case 0x21: /* Zen3 Ryzen Desktop */ case 0x50 ... 0x5f: /* Green Sardine */ data->ccd_offset = 0x154; - k10temp_get_ccd_support(pdev, data, 8); + k10temp_get_ccd_support(data, 8); break; case 0x40 ... 0x4f: /* Yellow Carp */ data->ccd_offset = 0x300; - k10temp_get_ccd_support(pdev, data, 8); + k10temp_get_ccd_support(data, 8); break; case 0x60 ... 0x6f: case 0x70 ... 0x7f: data->ccd_offset = 0x308; - k10temp_get_ccd_support(pdev, data, 8); + k10temp_get_ccd_support(data, 8); break; case 0x10 ... 0x1f: case 0xa0 ... 0xaf: data->ccd_offset = 0x300; - k10temp_get_ccd_support(pdev, data, 12); + k10temp_get_ccd_support(data, 12); break; } } else if (boot_cpu_data.x86 == 0x1a) { From 0e097f2b5928651de3b4f0100401c9e71fa73dba Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:13:00 -0500 Subject: [PATCH 09/15] hwmon: (k10temp) Remove unused HAVE_TDIE() macro ...to address the following warning: drivers/hwmon/k10temp.c:104:9: warning: macro is not used [-Wunused-macros] No functional change is intended. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-7-ffde21931c3f@amd.com --- drivers/hwmon/k10temp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index 6deb272c7cef..a2d203304533 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -101,7 +101,6 @@ struct k10temp_data { #define TCCD_BIT(x) ((x) + 2) #define HAVE_TEMP(d, channel) ((d)->show_temp & BIT(channel)) -#define HAVE_TDIE(d) HAVE_TEMP(d, TDIE_BIT) struct tctl_offset { u8 model; From efdf761a83cd390523223f58793755f5d60a4489 Mon Sep 17 00:00:00 2001 From: Yazen Ghannam Date: Thu, 6 Jun 2024 11:13:01 -0500 Subject: [PATCH 10/15] hwmon: (k10temp) Rename _data variable ...to address the following warning: drivers/hwmon/k10temp.c:273:47: warning: declaration shadows a variable in the global scope [-Wshadow] static umode_t k10temp_is_visible(const void *_data, ^ No functional change is intended. Signed-off-by: Yazen Ghannam Signed-off-by: Borislav Petkov (AMD) Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20240606-fix-smn-bad-read-v4-8-ffde21931c3f@amd.com --- drivers/hwmon/k10temp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c index a2d203304533..543526bac042 100644 --- a/drivers/hwmon/k10temp.c +++ b/drivers/hwmon/k10temp.c @@ -269,11 +269,11 @@ static int k10temp_read(struct device *dev, enum hwmon_sensor_types type, } } -static umode_t k10temp_is_visible(const void *_data, +static umode_t k10temp_is_visible(const void *drvdata, enum hwmon_sensor_types type, u32 attr, int channel) { - const struct k10temp_data *data = _data; + const struct k10temp_data *data = drvdata; struct pci_dev *pdev = data->pdev; u32 reg; From ec0b4c4d45cf7cf9a6c9626a494a89cb1ae7c645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 27 May 2024 15:55:35 +0300 Subject: [PATCH 11/15] x86/of: Return consistent error type from x86_of_pci_irq_enable() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x86_of_pci_irq_enable() returns PCIBIOS_* code received from pci_read_config_byte() directly and also -EINVAL which are not compatible error types. x86_of_pci_irq_enable() is used as (*pcibios_enable_irq) function which should not return PCIBIOS_* codes. Convert the PCIBIOS_* return code from pci_read_config_byte() into normal errno using pcibios_err_to_errno(). Fixes: 96e0a0797eba ("x86: dtb: Add support for PCI devices backed by dtb nodes") Signed-off-by: Ilpo Järvinen Signed-off-by: Borislav Petkov (AMD) Link: https://lore.kernel.org/r/20240527125538.13620-1-ilpo.jarvinen@linux.intel.com --- arch/x86/kernel/devicetree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c index 8e3c53b4d070..64280879c68c 100644 --- a/arch/x86/kernel/devicetree.c +++ b/arch/x86/kernel/devicetree.c @@ -83,7 +83,7 @@ static int x86_of_pci_irq_enable(struct pci_dev *dev) ret = pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin); if (ret) - return ret; + return pcibios_err_to_errno(ret); if (!pin) return 0; From 724852059e97c48557151b3aa4af424614819752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 27 May 2024 15:55:36 +0300 Subject: [PATCH 12/15] x86/pci/intel_mid_pci: Fix PCIBIOS_* return code handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit intel_mid_pci_irq_enable() uses pci_read_config_byte() that returns PCIBIOS_* codes. The error handling, however, assumes the codes are normal errnos because it checks for < 0. intel_mid_pci_irq_enable() also returns the PCIBIOS_* code back to the caller but the function is used as the (*pcibios_enable_irq) function which should return normal errnos. Convert the error check to plain non-zero check which works for PCIBIOS_* return codes and convert the PCIBIOS_* return code using pcibios_err_to_errno() into normal errno before returning it. Fixes: 5b395e2be6c4 ("x86/platform/intel-mid: Make IRQ allocation a bit more flexible") Signed-off-by: Ilpo Järvinen Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20240527125538.13620-2-ilpo.jarvinen@linux.intel.com --- arch/x86/pci/intel_mid_pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c index 8edd62206604..722a33be08a1 100644 --- a/arch/x86/pci/intel_mid_pci.c +++ b/arch/x86/pci/intel_mid_pci.c @@ -233,9 +233,9 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev) return 0; ret = pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &gsi); - if (ret < 0) { + if (ret) { dev_warn(&dev->dev, "Failed to read interrupt line: %d\n", ret); - return ret; + return pcibios_err_to_errno(ret); } id = x86_match_cpu(intel_mid_cpu_ids); From e9d7b435dfaec58432f4106aaa632bf39f52ce9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 27 May 2024 15:55:37 +0300 Subject: [PATCH 13/15] x86/pci/xen: Fix PCIBIOS_* return code handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xen_pcifront_enable_irq() uses pci_read_config_byte() that returns PCIBIOS_* codes. The error handling, however, assumes the codes are normal errnos because it checks for < 0. xen_pcifront_enable_irq() also returns the PCIBIOS_* code back to the caller but the function is used as the (*pcibios_enable_irq) function which should return normal errnos. Convert the error check to plain non-zero check which works for PCIBIOS_* return codes and convert the PCIBIOS_* return code using pcibios_err_to_errno() into normal errno before returning it. Fixes: 3f2a230caf21 ("xen: handled remapped IRQs when enabling a pcifront PCI device.") Signed-off-by: Ilpo Järvinen Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/20240527125538.13620-3-ilpo.jarvinen@linux.intel.com --- arch/x86/pci/xen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index 652cd53e77f6..0f2fe524f60d 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -38,10 +38,10 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev) u8 gsi; rc = pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &gsi); - if (rc < 0) { + if (rc) { dev_warn(&dev->dev, "Xen PCI: failed to read interrupt line: %d\n", rc); - return rc; + return pcibios_err_to_errno(rc); } /* In PV DomU the Xen PCI backend puts the PIRQ in the interrupt line.*/ pirq = gsi; From 7821fa101eab529521aa4b724bf708149d70820c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 27 May 2024 15:55:38 +0300 Subject: [PATCH 14/15] x86/platform/iosf_mbi: Convert PCIBIOS_* return codes to errnos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iosf_mbi_pci_{read,write}_mdr() use pci_{read,write}_config_dword() that return PCIBIOS_* codes but functions also return -ENODEV which are not compatible error codes. As neither of the functions are related to PCI read/write functions, they should return normal errnos. Convert PCIBIOS_* returns code using pcibios_err_to_errno() into normal errno before returning it. Fixes: 46184415368a ("arch: x86: New MailBox support driver for Intel SOC's") Signed-off-by: Ilpo Järvinen Signed-off-by: Borislav Petkov (AMD) Link: https://lore.kernel.org/r/20240527125538.13620-4-ilpo.jarvinen@linux.intel.com --- arch/x86/platform/intel/iosf_mbi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c index fdd49d70b437..c81cea208c2c 100644 --- a/arch/x86/platform/intel/iosf_mbi.c +++ b/arch/x86/platform/intel/iosf_mbi.c @@ -62,7 +62,7 @@ static int iosf_mbi_pci_read_mdr(u32 mcrx, u32 mcr, u32 *mdr) fail_read: dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result); - return result; + return pcibios_err_to_errno(result); } static int iosf_mbi_pci_write_mdr(u32 mcrx, u32 mcr, u32 mdr) @@ -91,7 +91,7 @@ static int iosf_mbi_pci_write_mdr(u32 mcrx, u32 mcr, u32 mdr) fail_write: dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result); - return result; + return pcibios_err_to_errno(result); } int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr) From bf6ab33d8487f5e2a0998ce75286eae65bb0a6d6 Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Thu, 23 May 2024 23:50:29 +0200 Subject: [PATCH 15/15] x86/kmsan: Fix hook for unaligned accesses When called with a 'from' that is not 4-byte-aligned, string_memcpy_fromio() calls the movs() macro to copy the first few bytes, so that 'from' becomes 4-byte-aligned before calling rep_movs(). This movs() macro modifies 'to', and the subsequent line modifies 'n'. As a result, on unaligned accesses, kmsan_unpoison_memory() uses the updated (aligned) values of 'to' and 'n'. Hence, it does not unpoison the entire region. Save the original values of 'to' and 'n', and pass those to kmsan_unpoison_memory(), so that the entire region is unpoisoned. Signed-off-by: Brian Johannesmeyer Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Alexander Potapenko Link: https://lore.kernel.org/r/20240523215029.4160518-1-bjohannesmeyer@gmail.com --- arch/x86/lib/iomem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/x86/lib/iomem.c b/arch/x86/lib/iomem.c index e0411a3774d4..5eecb45d05d5 100644 --- a/arch/x86/lib/iomem.c +++ b/arch/x86/lib/iomem.c @@ -25,6 +25,9 @@ static __always_inline void rep_movs(void *to, const void *from, size_t n) static void string_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n) { + const void *orig_to = to; + const size_t orig_n = n; + if (unlikely(!n)) return; @@ -39,7 +42,7 @@ static void string_memcpy_fromio(void *to, const volatile void __iomem *from, si } rep_movs(to, (const void *)from, n); /* KMSAN must treat values read from devices as initialized. */ - kmsan_unpoison_memory(to, n); + kmsan_unpoison_memory(orig_to, orig_n); } static void string_memcpy_toio(volatile void __iomem *to, const void *from, size_t n)