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 X-Spam-Level: X-Spam-Status: No, score=-11.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0FEA0C4332B for ; Thu, 21 Jan 2021 15:32:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CD84923A1C for ; Thu, 21 Jan 2021 15:32:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732845AbhAUPcB (ORCPT ); Thu, 21 Jan 2021 10:32:01 -0500 Received: from mail.kernel.org ([198.145.29.99]:44822 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732996AbhAUPb1 (ORCPT ); Thu, 21 Jan 2021 10:31:27 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 6FB1623A1C; Thu, 21 Jan 2021 15:30:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1611243046; bh=46aIcbRTsz8tFFP2Hj5yj5BUbFIG/ba6L71tLXDA3dQ=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=aftAQewZujwvMZm2hiUsnbTGld1THjBzhheyQdZsXmm8Xw5JlSpK59Nt98wuI2b86 jL+J7o4Tq0Sa8/kZnmMtCZNzivHqms5eG+zMZtcmqk1PrCIKXuWiqi6eUVK9bob6/o p4fhcHpTD92D1vmmcPrpcL1XLGRH/MWAkBxq44XdCgI3clYNiDhmoHh1FJ8ArK42qm Qo6jZDtAgFeI0LVS+9dxOCQ3/icLKgaDHESG+6A0tXAVxWnLUPZdkaE8kvUAf5t67e NLxU3eqpzqsGtff/nMvOEQSPJzpD2phoCTgV5RJkewTM1O09RK2O/OYA9koHGFVBsr BTnP32SoIY4vw== Date: Thu, 21 Jan 2021 09:30:43 -0600 From: Bjorn Helgaas To: Chiqijun , Alex Williamson Cc: "bhelgaas@google.com" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Yinshi (Stone)" , "Wangxiaoyun (Cloud)" , zengweiliang zengweiliang , "Chenlizhong (IT Chip)" Subject: Re: [v3] PCI: Add pci reset quirk for Huawei Intelligent NIC virtual function Message-ID: <20210121153043.GA2654954@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7e0a6c6c-a12c-ee54-0468-69079b8edde4@huawei.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org [Alex is a reset expert, hoping he can chime in] On Thu, Jan 21, 2021 at 08:53:12PM +0800, Chiqijun wrote: > On 2021/1/9 6:25, Bjorn Helgaas wrote: > > On Fri, Dec 25, 2020 at 05:25:30PM +0800, Chiqijun wrote: > > > When multiple VFs do FLR at the same time, the firmware is > > > processed serially, resulting in some VF FLRs being delayed more > > > than 100ms, when the virtual machine restarts and the device > > > driver is loaded, the firmware is doing the corresponding VF > > > FLR, causing the driver to fail to load. > > > > > > To solve this problem, add host and firmware status synchronization > > > during FLR. > > > > > > Signed-off-by: Chiqijun > > > ... > > > + * Get and check firmware capabilities. > > > + */ > > > + val = readl(bar + HINIC_VF_FLR_TYPE); > > > + if (!(val & (1UL << HINIC_VF_FLR_CAP_BIT_SHIFT))) { > > > + pci_iounmap(pdev, bar); > > > + return -ENOTTY; > > > + } > > > + > > > + /* > > > + * Set the processing bit for the start of FLR, which will be cleared > > > + * by the firmware after FLR is completed. > > > + */ > > > + val = readl(bar + HINIC_VF_OP); > > > + val = val | (1UL << HINIC_VF_FLR_PROC_BIT_SHIFT); > > > + writel(val, bar + HINIC_VF_OP); > > > + > > > + /* Perform the actual device function reset */ > > > + pcie_flr(pdev); > > > + > > > + /* > > > + * The device must learn BDF after FLR in order to respond to BAR's > > > + * read request, therefore, we issue a configure write request to let > > > + * the device capture BDF. > > > + */ > > > + pci_read_config_word(pdev, PCI_COMMAND, &command); > > > + pci_write_config_word(pdev, PCI_COMMAND, command); > > > > I assume this is because of this requirement from PCIe r5.0, sec > > 2.2.9: > > > > Functions must capture the Bus and Device Numbers supplied with all > > Type 0 Configuration Write Requests completed by the Function, and > > supply these numbers in the Bus and Device Number fields of the > > Completer ID for all Completions generated by the Device/Function. > > > > I'm a little concerned because it seems like this requirement should > > apply to *all* resets, and I don't see where we do a similar write > > following other resets. Can you help me out? Do we need this in > > other cases? Do we do it? > > This depends on the hardware device. The HINIC device clears the BDF > information of the VF during FLR, so it relies on Configuration > Write Requests to capture BDF. If other devices do not clear the DBF > information during FLR, this operation is not required. If the spec says devices must keep the latched BDF during FLR, and the HINIC doesn't comply with that, then it makes sense to do a config write here in HINIC-specific code. But if devices are allowed to clear the BDF during FLR, the OS has to assume they all do, and the generic code for FLR (and probably other resets) should do a config write so devices can latch the BDF again. > In addition, I did not find other devices directly access the BAR register > after FLR in resets. I didn't catch your meaning here. If a device loses the BDF during FLR and we don't do something to allow it to latch the BDF again, any completions from the device will have the wrong information. We will likely do *some* config write to the device eventually, which will fix this, but we can't rely on some unknown future write to do this. If it's a problem, we need to explicitly do a write for this purpose. Bjorn