From patchwork Thu May 23 18:26:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yazen Ghannam X-Patchwork-Id: 13672161 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2077.outbound.protection.outlook.com [40.107.220.77]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 66C7584047; Thu, 23 May 2024 18:27:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=40.107.220.77 ARC-Seal: i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716488842; cv=fail; b=ekpvUGI3cG6WHlm+tmurZglGuIm3aQobiJF7eFIZ6huUTt7AhExHZn829nvzNJ51XqVQ3NVxwD8Fj/iz9T9aUjSDj5OpQDY8GjOfo0W80SXH35zNmd8UbbL3llOJwp6lso5mG7sNhufT9RDawEJUGpRkIfloItXdpV69pf/i0SA= ARC-Message-Signature: i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716488842; c=relaxed/simple; bh=dDTTCEV+d0iopyDJ4xM95DCs80v4QTmTcDGLuro42kc=; h=From:Date:Subject:MIME-Version:Content-Type:Message-ID:References: In-Reply-To:To:CC; b=OFGbBnFzXp9z5U2ePC0AXy2bRc1quZlMVHA0mvPfSKAU1WJOFOXSCa5BNfKFE9uKhZ3+1yQFkJ3YRAj6IXzeYZ3QKgzO5mV077YRSuU54UFQ7YvdTVftMokYuwGvRfeQ4SxDfzIlyw0jLZoLEERWPliBx+SmnrMykbhkFwYNfXE= ARC-Authentication-Results: i=2; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com; spf=fail smtp.mailfrom=amd.com; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b=uRKAIZzH; arc=fail smtp.client-ip=40.107.220.77 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=amd.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b="uRKAIZzH" ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Bi6BK3JeNxytLRiZPKQVOQa9cp7mZPhZRg0ebxz46spIVR8YkYnPfTuK4ZgBnffwlA7flrqKeA/eFpeerb6D8vIynvvXw+m1VYOekr/g0BdDo9lqnD7sFAyeZQ1ACNY95J3wWh5xWGwohP01cwGAeP/5RQFz1jl41RYETVann2jqQGt02hyv3KBajgkaW/CanS/tnOTQRNwetlK9T9KjOJGT1bOg3c4fEDslfKJbVX5JPe4ZabFdhBH9sDx4lg1pUL12GixnCDFfe2deqlngFmoLmS08VOnTAV4goWCH0uwgSFIUg2aJY6sh1kFabhvGutrOojYh+o/EZ/YMEJmi0A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=j/prkzvhX7aBUAyFTQ1Ogj1nExfxmcCLbWoU+rIQGK8=; b=jTfEYKvUUByjq+GXRWTV41QOwFy5GYuNgSLQEYU37XQqolipykIraVEt8DQdOvEQRomwxjaE7gFuEKSQOQWIrhCO4T3zFRoSiUxzBwF1Xr6jFJqshnX5SkRCeRCFrzh6S87isi8wiJ8LwUfIAp2GgS4tWNEutFzSe5UUFc2XCgv9JXlJxeVyWA6horetTsdnRGi4e1YTCkDqy4g7D2jyoRIcvCXgYVTwLQpp1jb3RgxyjsSyDFbfDxCFDnKMVqvOGAZrJ3ytKRkI/FA8Hp5Om/2cN/HjsD2OoWItP4VUHyAQso1AU4uyRhBHYFSdfj5gnFL5Bw1zZFd8H9tMB2CaEg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=kernel.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j/prkzvhX7aBUAyFTQ1Ogj1nExfxmcCLbWoU+rIQGK8=; b=uRKAIZzHo74xhQaZmTGOHQ+vedJL53GYmtuyC+14tjPGNgceokADGGfz9L1X9MLSOnUeOayEDg3u7OQ1jEJhXmv8P6iX4kIBwiJmnbjF2PrqXlvjWCPnlHVg415B7utOo46mfW1hsV4z0VTSAvots9txRUi8Ay6Zf9kMbBINImM= Received: from CH2PR19CA0002.namprd19.prod.outlook.com (2603:10b6:610:4d::12) by CY8PR12MB8214.namprd12.prod.outlook.com (2603:10b6:930:76::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7587.36; Thu, 23 May 2024 18:27:14 +0000 Received: from CH3PEPF00000012.namprd21.prod.outlook.com (2603:10b6:610:4d:cafe::15) by CH2PR19CA0002.outlook.office365.com (2603:10b6:610:4d::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7611.20 via Frontend Transport; Thu, 23 May 2024 18:27:13 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by CH3PEPF00000012.mail.protection.outlook.com (10.167.244.117) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.7633.0 via Frontend Transport; Thu, 23 May 2024 18:27:13 +0000 Received: from quartz-7b1chost.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 23 May 2024 13:27:10 -0500 From: Yazen Ghannam Date: Thu, 23 May 2024 13:26:55 -0500 Subject: [PATCH v3 4/8] x86/amd_nb: Enhance SMN access error checking Precedence: bulk X-Mailing-List: linux-hwmon@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-ID: <20240523-fix-smn-bad-read-v3-4-aa44c622de39@amd.com> References: <20240523-fix-smn-bad-read-v3-0-aa44c622de39@amd.com> In-Reply-To: <20240523-fix-smn-bad-read-v3-0-aa44c622de39@amd.com> To: Guenter Roeck , , Yazen Ghannam CC: Mario Limonciello , , , X-Mailer: b4 0.13.0 X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH3PEPF00000012:EE_|CY8PR12MB8214:EE_ X-MS-Office365-Filtering-Correlation-Id: 760bed3d-07e8-4154-5abd-08dc7b55f7ac X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230031|376005|36860700004|1800799015|82310400017; X-Microsoft-Antispam-Message-Info: =?utf-8?q?iqNAvlgAAQVW9n1z5fdw2yWfGzQi146?= =?utf-8?q?xD7LctBKgeB0V5aFhTC7l6wj3Pl5PuK/Ls+JlPS/iqdFl3WCPKOQ5ZDlWsHDLY6qs?= =?utf-8?q?JHf/35SAs4f+LH7haa585aYbVEhe46XSZdtgU/inUMbrCt01O2w0ey6ps/ENK7Zau?= =?utf-8?q?rRtQZXYmLqw6DxGm680cfc9nFOuiyIUhnsZJrghcknBNfB1G1/KTR1YksiEaLZUV2?= =?utf-8?q?KN52U5/ovuiQml1FbiwUMGtj6WTuaUfvax66bNFvsmcXI+8FMDuNZKswOF06NKLji?= =?utf-8?q?TiHGJxq0ZH2R8Ql33RRuU1GRosbEtdMRYYn0ba0AdqB+YxlTkLhVl8J5nnquAnFsS?= =?utf-8?q?VZ7FB5R0yXycfCW5JPGX89WVxhR0EKl454aktcCc+fUEoFx5BNTK4KA+ZlfFE/6n7?= =?utf-8?q?wHLsD1aV5uCIKzf4yQcMepG5uhyZV06oNXJMXoaKoWc0krKD+74hYsFU/YB7WYSLE?= =?utf-8?q?6QgVUucfrDTk2XT1JimRpLo0GYRNK+SY/sWpK8PdCF0JX6rrTAD9R7abwuQsXjJ83?= =?utf-8?q?nTC1SbqfRyD/zFThSdU2H1FmhB6SP6znR3Igow0tvjWj1PCL6xlMDH9wJD065PXtf?= =?utf-8?q?QnjCWFMaPrX3BJlRv6aNW02TBx79Mj78biVrYuRjzxX07en5DdAha67QRotEMk3L0?= =?utf-8?q?1CBL9kagfH5E7UE9jXma2RRtI+fN9s5nYiJmHCL3QWYw0I/v4VFM3FavOc/Xw7/5P?= =?utf-8?q?DGDjbIJaPhNbZcsTdfTJwJ5jWDD7yHMPKBWcz2pdzVZhCp91yi5BRN6ZdVIaX6TZt?= =?utf-8?q?ljuJDk4d9hqvM1+W9hfpC+tnZxOd+mbAAA4Cy28lkeMoFHEzUlPwfouEBEQ/Cg2fR?= =?utf-8?q?7wYGKLkl9lBqbdF0uLg/OdqcVEcEXCKP4j2lE6ruPoKK8AK/bAMCDuHSpuH9F9as8?= =?utf-8?q?M1LSWPDeSw6vfBIT3c5uPqbZCOKLvLNlsBfwg7CVqHqTBigsF02bdkY+3JbcA//UL?= =?utf-8?q?jSWxSiucW1JYGOhvAVt1x2JguDeU4+0HyWQpUZ5Co2VgBPal6I1gkXkJVeRhB000w?= =?utf-8?q?CtfJJsw6Fk+6tbTVvYTYvK3Go0eDCyZekkvV+RIOaNBc7rryN3QJqpxmQ4PHPsKN/?= =?utf-8?q?I4fm+T9uRizACB/RCpgknazb7rorrEJJghx214lK6b7hhG1PLkUu9OpSOtBUI4IdK?= =?utf-8?q?bo83c1PwkMrH/a6JmafUQ3ZfIQqFfm9qhs7yIjMTBK+fRcW+WSWeItc6fDCcZYXFo?= =?utf-8?q?G6wYh+mORwWmN/rr66DD4VEA7RWN9mNM4Rjvh0uE0minjIxyUzzRUmu8iL1xrjtyi?= =?utf-8?q?NeIOCoaTL4F0lW6GmqxFw0YPRdG77Sv4YcHFP8R4lkqC6x1KHQYMbTbWY1Q/0USeq?= =?utf-8?q?FwFPSF9lTuA8?= X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230031)(376005)(36860700004)(1800799015)(82310400017);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 18:27:13.6847 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 760bed3d-07e8-4154-5abd-08dc7b55f7ac X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: CH3PEPF00000012.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB8214 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 PCI config access may fail and return an error code. This would prevent the "read" value from being updated, and it would give an indication to the caller that the read or write operation failed. However for reads, the PCI config access may succeed, but the return value may be invalid. This is in similar fashion to PCI bad reads, i.e. return all bits set. Most systems will return 0 for SMN addresses that are not accessible. This is in line with AMD convention that unavailable registers are Read-as-Zero/Writes-Ignored. However, some systems will return a "PCI Error Response" instead. This value, along with an error code of 0 from the PCI config access, will confuse callers of the amd_smn_read() function. Check for this condition and set a proper error code for SMN reads. Furthermore, 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. Also, drop the extern keyword as it's not needed. And remove a warning that will not be trigger in many cases. Fixes: ddfe43cdc0da ("x86/amd_nb: Add SMN and Indirect Data Fabric access for AMD Fam17h") Signed-off-by: Yazen Ghannam Reviewed-by: Mario Limonciello Cc: stable@vger.kernel.org --- arch/x86/include/asm/amd_nb.h | 4 ++-- arch/x86/kernel/amd_nb.c | 46 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 8 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 3cf156f70859..d00c568e45b8 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -180,6 +180,38 @@ 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 smn_read() and smn_write(). Therefore, callers of these functions + * must do their own checking based on what behavior they expect. + * + * For SMN reads, the returned SMN 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" + * procedure. 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 +234,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,13 +242,18 @@ 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) { - return __amd_smn_rw(node, address, value, false); + int err = __amd_smn_rw(node, address, value, false); + + if (PCI_POSSIBLE_ERROR(*value)) + err = -ENODEV; + + return err; } 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); }