From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D49B9C001DF for ; Sun, 30 Jul 2023 19:15:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229682AbjG3TPI (ORCPT ); Sun, 30 Jul 2023 15:15:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40054 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229449AbjG3TPH (ORCPT ); Sun, 30 Jul 2023 15:15:07 -0400 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2065.outbound.protection.outlook.com [40.107.243.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 096ED10FC; Sun, 30 Jul 2023 12:15:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TxrdAYFrbC4pECP6qfOQXNUqG8/8+wwd7gY9t6lJnax2KASpuUJy2GW/RrAnVEGOKAL37VmKtfzLLc4lUaVlVnFtwAjK0ua/fe+2v5SQMHJGqRWMvTt2MSxzPutP2W7r5oob5vrKTOQKPKXPQ1i+thMYlu57PxJ+EVv24fa3HqgMHCo5gZyOpsA0Gv2Cdm3qaKB+5wu30+mn4f7FtIgu1VPvqbMUpNddhipcy1busRj5pFgkC04pkBqtfm7vL0bQMjsa5mgn4ARl9pbsuRU4B2GBiwBAP8R0E4AicZ7QRqyMDWM6mRABH8fgIgr8Kb6KPmK1DwO+47QikizLPv9sdQ== 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=NWl7L8gxTd8T6a2FIaGPCCuz23QmtrYGHxgFDL/7FFo=; b=FW5I0o5SQNjLRD9M3NMXxelOfbg6YXyTbyGTnCHyaNSDdMq7tt5GVg75wbhVAzSAFPsFJFPyHTImUI2HTOkpgCBrA4jLc4zWt9xCcAudikpdyy9XHK0cWEjWAJX7LOeApk+c0CukIoj0J64/UViF0H3WIqaAPXNVeYtRv6urFlKihV+OAzvAN7jCPiKtAzXspNYk8f3yaZwyHNtog72dihHVzcd67gsTR2kRon9Unt6Os8fp7ICytgQ+VjJuygLo6pHN7+r4XjhuUrz0RtCN2rHbs382DItaLhsHsXeAgvAQpIcl0HoIodwyi5H++8BM19DRxkuesQvXds1iAVxMQA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NWl7L8gxTd8T6a2FIaGPCCuz23QmtrYGHxgFDL/7FFo=; b=XH4DbtfC8D/5JAgnjY+BXF6gF09hy4TN1/jYxcsdHEM+SvsC2D7MDjk0dqtg9STfiIXvm+XeXCuvLyV/GZyxtiW717nAWhCGhTyJQbfN9eAXBhmewIcCq28w0zdbxRFKPxRgwnrxzzD6aPVuVSPfINl5zJbPNNBiZcwov295/9FfkYOkRkjSc6f0xhy1JjUWNdfqH3O6O3lrEXkmgBQtsAcLGpEkyUnVNCD7uDnB19Ak5sGMGQQI4cIRo2+ILxQjNmvkFFaAITG6mCuZVfSanYwD+9U94OrkGkX/Icf2jS6u2VlaliyjMXPxwUH6a0MiVh7OpsV2FBJ4kBNHOOji9g== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; Received: from BN8PR12MB2900.namprd12.prod.outlook.com (2603:10b6:408:69::18) by DM6PR12MB4861.namprd12.prod.outlook.com (2603:10b6:5:1bd::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6631.42; Sun, 30 Jul 2023 19:15:04 +0000 Received: from BN8PR12MB2900.namprd12.prod.outlook.com ([fe80::b1:f0a0:1b20:1bf5]) by BN8PR12MB2900.namprd12.prod.outlook.com ([fe80::b1:f0a0:1b20:1bf5%5]) with mapi id 15.20.6631.039; Sun, 30 Jul 2023 19:15:03 +0000 Message-ID: <6c9fef7e-2a78-b129-b176-41374861e20d@nvidia.com> Date: Mon, 31 Jul 2023 00:44:50 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH V2] PCI: pciehp: Disable ACS Source Validation during hot-remove To: Bjorn Helgaas Cc: bhelgaas@google.com, treding@nvidia.com, jonathanh@nvidia.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, vsethi@nvidia.com, kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com, Lukas Wunner , Alex Williamson References: <20230214233016.GA3095090@bhelgaas> Content-Language: en-US From: Vidya Sagar In-Reply-To: <20230214233016.GA3095090@bhelgaas> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: PN0PR01CA0021.INDPRD01.PROD.OUTLOOK.COM (2603:1096:c01:4e::6) To BN8PR12MB2900.namprd12.prod.outlook.com (2603:10b6:408:69::18) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN8PR12MB2900:EE_|DM6PR12MB4861:EE_ X-MS-Office365-Filtering-Correlation-Id: cb9dc4c4-8bea-41b2-0015-08db91314701 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: NLmVIch+t4h6Bj7Uxjg5u7Yv9jZu2oIbTx8/rwN6mrOL6NrZwlj3WuNSAOFwEygCD01VaL/01FuJJPv9zAamtEga2vJdOGCoXpKkLfUwqkdAgdQeaYvSZNmMpDFyb4xhcWeIzGoaE1/ZErz/5MJ0sCird8l06Tc1lO3AreGUX4FZfEvPtk4hj6xYD3TB6jqxJ0s3fbGwBofPAOl4UFzEYRofnnWpYTW4Dj5+TdSV30ZdW2bJ8F7+kOJYmII8OGNrVKagus2El/P2hmgZiNG6jvEIawsoHoPgia0pe43yb9QCZC+Li2e9Da5bz04xsEvrCA0Cno6GIuJdYzxpcfuvB0gL/YxaeRINXQ2lKfiqcybOgVn/pMZMAlMoujIhT24s93sD1RexdBvEW5zzy7nKrBtxLJpVeMZJJGWtgerb4j4DqC9RKrLSwL15Ux4ZiU+vlkOiCUxnSLF5dJpR9PzJJ1vY2zFHiG7Gk72Yx/Xcs5n9Cq6YZ4pA8MlRzymVQ2aDlbBScY8zWUz9QNymgSCVLjHcjm0wrYUH1YKQ7BHbkUpEo2OmUB8J5iPkwvUxwG0ITTzP9U4IrSIj/X0rE0gQR2VCHWKEwFIYJxV6Oo5wZ7SCOIKo47+l2V5sqWbPiu+hI76SoDQUnJLXnyDmgOwIhQ== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN8PR12MB2900.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(396003)(39850400004)(136003)(376002)(366004)(346002)(451199021)(38100700002)(31696002)(86362001)(36756003)(6512007)(478600001)(6666004)(6486002)(2616005)(53546011)(186003)(6506007)(26005)(8676002)(8936002)(5660300002)(4326008)(6916009)(66946007)(66556008)(66476007)(2906002)(54906003)(31686004)(41300700001)(316002)(83380400001)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Wm5HbTExWDBPMmlYdW9IbW9aUXlTNlVLRklkSFpna2cvVllmSGZhUUJySlVi?= =?utf-8?B?Q1N2UjUzZTNuUEFJcnVKWnJuNDVvVnJwNHY2N2JJVFNBcTdqcTdlWkI4WjAy?= =?utf-8?B?N1B0dFRRMkRrVXQvMDJCNXBBcVdOSElhVVlvV1NMMVlVazg2NmNjT1B1andi?= =?utf-8?B?VGk3OW9qWXBxY1VLVE1USGQrSStPa2Q1UzV6YkQ5NkF0RExoQnd6d0wzdElV?= =?utf-8?B?TERUZG51cHV3TXFLVjlkTjRoMFFtVldIS1Q0SEhOSU5Tb3ZKL2RWNUwzTGJ5?= =?utf-8?B?Wi9UanVocEVKbnc5bHVhNmFZYm90OVMxY2Q2ZW45MnFOUmtoRUJVWHlQb1Vk?= =?utf-8?B?aXowS3pxcVpTS1ZLbFIzRWZNdzNlMkxxMXdmVnBJN1JNOGQzZEtTZU1sOEdt?= =?utf-8?B?K0QyZ1VBNW5oelBoSUlDWmlreVlVTWw0Tmk0OS92YUlQYmhoeGdzSERQYUdl?= =?utf-8?B?L1g0d0hyVVEyejhPQVp2QkZMd1VBZzRLTWpKNmZmWTRGeUZpZlJoK0g0VnZq?= =?utf-8?B?dmdUZWhOV2xwOU1hNW1BT3FEMU0wVi9VdUxJamY5ZHk2TS9pUUphN0c2VXpj?= =?utf-8?B?Z3NRYnhxRENCNEtOK2drY1pRcUJJUVJlOHd4cGE0Ukw3TXlwaWlnUEp5elRs?= =?utf-8?B?bDdjbG9LYnFNSmR1MjIzcjV0L0Y1UE9kL3lEOVp1NWExY3g2VWc5MWpwak5w?= =?utf-8?B?RDVGMXJ4TUlBVUlMeUNtZk5vQWFqV3pkR0ZkUkI0RmI4dzhXajJ5djlWaGNS?= =?utf-8?B?US9rQXVndGkwRkM4TXNGYlhWTkJPQzNGdnpxbW5XZ3FhYWg2bjd4cDI1Szh1?= =?utf-8?B?SXFSZi9wN21qZzYwY2JsZkN6RFhuNHJKT3M2bkFhaFJQSWJyRTdKRWJMNGxx?= =?utf-8?B?SXJDYzRtRTRIQXVVa21QMzV2RDhGQzUwQVRnWnJ0UnFwMkdUR2U2TVNtbzFx?= =?utf-8?B?YmwvcjUyc2N4Z0k3WGlMVGt6Q3JTbDFJN1dQNVU3U3VzTExlZWQ4bTZKZ1Za?= =?utf-8?B?a0RQQ2thbGFMZGU1ZE5VUmlVaHlyVXBXWHNrZ3hsVDFUL09OZGVLUWJlZ01u?= =?utf-8?B?T1crVzZKUXR5c2l4ME5vbUk5ZmpqOXRIaDNOSEdhUTdKYk5XVWVhTmZQSXN6?= =?utf-8?B?WUN3Y1JrVk1wZUU1cEdLRm12OXpwSzhINXRoSllSVDB2MWV1dHNZdVBkQ3J4?= =?utf-8?B?RkNxaHdpbWtTYVp1YTRQWXU2SVpNeHRKVzhSVzRBNVpFR2E4bUZVMHlXRUdG?= =?utf-8?B?UHdsUXhreE1NdzZmVjJxY2dRZ0FjS21DMlltdDVJR01obk40WGwreEVEZ3VY?= =?utf-8?B?cGZVTkloLzRybE1yaUkvbWQyaHNpaE5NZXFYcWs4ODJ0TGJXZ3haWmJEQTNC?= =?utf-8?B?dXl0eWF1RlVORkZrSlJCR2F1WnJaRS9RTEVqeVFPdGpoRzJUNEI4TU9lang5?= =?utf-8?B?cTl2KzM2RFJXaEFMSyt2V3NyejRKK0ZSNTg4YmM3a1lObkkwK0NSMEZIUEtq?= =?utf-8?B?Ymo0NzVEMUNTUVNwS0ZaMDhDNm51UG5mN1FsK0c4T0E1MUlTY3JLTThIMmZj?= =?utf-8?B?VWV0VEFXT0h0ZEg1SUtkOG9aR1AyaUl5dU81Wk0xajhDRjZzaE5ObTdaOWRp?= =?utf-8?B?RXVpcitlOE41bSttalVjY21zTTZEbGRlZ0VHdnhVSXBHcDRQdWYxbENjbkhO?= =?utf-8?B?dC9QUWNRanNHVTBieHRkaEQvSi9uelpmeWJuMlFXZkhpN1BOL0dYWnhHWkww?= =?utf-8?B?Qk5MaWZyU015MXlyK0FZbWxPSHB4ak5YT283bm4raXhEcGRYcGdleW96dE15?= =?utf-8?B?SFlDZ1Y5SlRDMkhSenhSM2RrWjJVb1pYUElraWRPaXhjUkdFMms5LzFoTGM3?= =?utf-8?B?THFaejRyWHZtbURqRHJmTDZrcGRFMzJpTXlJMWVyd3RoNDd0TjU2MTJzUEQv?= =?utf-8?B?YkhJT05QV0F6STR6bjNtQWs3KzdMSy9LeDU3dXNwd1Urak1qL2cxK1JrTlht?= =?utf-8?B?Y2IrWTZETWlIZEZ1SldHQ1VwNGtzMVIwQzZ4MUVCQzdhenZ2WkxRQ0pvMGRI?= =?utf-8?B?MEppL1JYR0JEQS9SMnF1QkdlcWN4ZkdPdmE1eG9zNVRla3dsSEJUaXZndlNQ?= =?utf-8?Q?pcJ15vuBrXt9xy37L1a8tr+uY?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: cb9dc4c4-8bea-41b2-0015-08db91314701 X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB2900.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jul 2023 19:15:03.7033 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: vkXb5tEBVmGwcGCaGS5q/MUEFxkIZOgGNU+5RahLb8b0USIWzUYXZK7oFK2uGaRn8DzOU0XchE6rXlGTaP44cA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4861 Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On 2/15/2023 5:00 AM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > [+cc Lukas, Alex for pciehp and ACS comments] > > On Thu, Jan 12, 2023 at 12:35:33AM +0530, Vidya Sagar wrote: >> PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to >> send upstream messages before they have been assigned a bus number and >> such messages have a Requester ID with Bus number set to 00h. >> If the Downstrem port has ACS Source Validation enabled, these messages >> will be detected as ACS violation error. >> Hence, disable ACS Source Validation in the bridge device during >> hot-remove operation and re-enable it after enumeration of the >> downstream hierarchy but before binding the respective device drivers. > > s/Downstrem/Downstream/ > > Format as paragraphs (blank line between). > >> Signed-off-by: Vidya Sagar >> --- >> v2: >> * Fixed build issues >> >> drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++- >> drivers/pci/pci.c | 22 ++++++++++++++++++++++ >> include/linux/pci.h | 6 ++++++ >> 3 files changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c >> index d17f3bf36f70..ad90bcf3f621 100644 >> --- a/drivers/pci/hotplug/pciehp_pci.c >> +++ b/drivers/pci/hotplug/pciehp_pci.c >> @@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl) >> >> pci_assign_unassigned_bridge_resources(bridge); >> pcie_bus_configure_settings(parent); >> + pci_configure_acs_sv(bridge, true); >> pci_bus_add_devices(parent); >> >> out: >> @@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence) >> } >> pci_dev_put(dev); >> } >> - >> + /* >> + * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted >> + * to send upstream messages before they have been assigned a bus >> + * number and such messages have a Requester ID with Bus number >> + * set to 00h. If the Downstrem port has ACS Source Validation enabled, > > s/Downstrem/Downstream/ > >> + * these messages will be detected as ACS violation error. >> + * Hence, disable ACS Source Validation here and re-enable it after >> + * enumeration of the downstream hierarchy and before binding the >> + * respective device drivers in pciehp_configure_device(). >> + */ >> + pci_configure_acs_sv(ctrl->pcie->port, false); > > What if we have a slot that's empty at boot and we add a device later? > It looks like we still might see ACS errors there because the add > happens before a remove? Yes I'm afraid. But, looking at the implementation note in 6.22.1 and 6.12.1.1, I think it is expected and spec thought through this scenario. So, does it all come down to how the ASIC designers treat different messages (case-by-case) and see which of them need not have to be validated for source? For example, a DRS (Device Readiness Status) from an endpoint during enumeration (i.e. before the endpoint is being assigned with a valid bus number) can be ignored for ACS source validation? > >> pci_unlock_rescan_remove(); >> } >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index 95bc329e74c0..9cefaf814f49 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -991,6 +991,28 @@ static void pci_enable_acs(struct pci_dev *dev) >> pci_disable_acs_redir(dev); >> } >> >> +#ifdef CONFIG_HOTPLUG_PCI_PCIE >> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag) >> +{ >> + u16 cap; >> + u16 ctrl; >> + >> + if (!pci_acs_enable || !dev->acs_cap) >> + return; >> + >> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap); >> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl); >> + >> + if (flag) >> + ctrl |= (cap & PCI_ACS_SV); >> + else >> + ctrl &= ~(cap & PCI_ACS_SV); >> + >> + pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl); > > I guess we don't have a way to do this for the non-standard ACS-like > devices, i.e., pci_dev_specific_enable_acs(). Not the end of the > world, just unfortunate that we'll have different behavior there. I'll remove the exporting part in the next patch. As far as exposing it in the pci.h is concerned, since the actual usage of this function is in pciehp_pci.c file, I thought the best place to keep the declaration is pci.h file. Let me know if you have any other suggestion. > >> +} >> +EXPORT_SYMBOL_GPL(pci_configure_acs_sv); > > Doesn't seem like this needs to be exported or exposed via > linux/pci.h. pciehp cannot be built as a module. > >> +#endif >> + >> /** >> * pci_restore_bars - restore a device's BAR values (e.g. after wake-up) >> * @dev: PCI device to have its BARs restored >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 060af91bafcd..edf516e39764 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot); >> void pci_hp_remove_module_link(struct pci_slot *pci_slot); >> #endif >> >> +#ifdef CONFIG_HOTPLUG_PCI_PCIE >> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag); >> +#else >> +static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { } >> +#endif >> + >> /** >> * pci_pcie_cap - get the saved PCIe capability offset >> * @dev: PCI device >> -- >> 2.17.1 >>