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=-7.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 4CA37C433DB for ; Wed, 17 Mar 2021 19:25:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E7D7C64F53 for ; Wed, 17 Mar 2021 19:25:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233027AbhCQTZB (ORCPT ); Wed, 17 Mar 2021 15:25:01 -0400 Received: from mail.kernel.org ([198.145.29.99]:53356 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231378AbhCQTY2 (ORCPT ); Wed, 17 Mar 2021 15:24:28 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 938DD64E74; Wed, 17 Mar 2021 19:24:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1616009067; bh=KKLdsb/3DlDkU4aWs0Dgdf8TtDZwtJHl3u4fz67AniQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GZRTNw0CPL7kswGGvq7LaYQEyiyMeFE5x9bbrs/0yrOamTF4m1GdnaR/RopyRKCoV jAjzVtaTzi392HO5KVybX1T3uzbbOD+mqq8mpTxvM7ilTURG04TfRvyCdwfhpY90lu 5YseBIs6KQdENZi/ng+V8Rie6VcwA0K66GbH66cnm+DaTdAIa4lnBL9qyGnUUYWdOv OJlyQcuoWJsGA3Rnf3GyyPMkzmE8bf9NdCnwtqL7dxQVlV6zs44J/ewhVRYzOC1lAQ mIiojOTxXZHSoFpVnKuYgohf2Jjj8Rbs3kRllnZwH2ySv1x7ql/UglC14uCCIXxBdc W1aKq2IeCOWeA== Received: by pali.im (Postfix) id B26938A9; Wed, 17 Mar 2021 20:24:24 +0100 (CET) Date: Wed, 17 Mar 2021 20:24:24 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Alex Williamson Cc: Amey Narkhede , bhelgaas@google.com, raphael.norwitz@nutanix.com, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Subject: Re: [PATCH 4/4] PCI/sysfs: Allow userspace to query and set device reset mechanism Message-ID: <20210317192424.kpfybcrsen3ivr4f@pali> References: <20210312173452.3855-1-ameynarkhede03@gmail.com> <20210312173452.3855-5-ameynarkhede03@gmail.com> <20210314235545.girtrazsdxtrqo2q@pali> <20210315134323.llz2o7yhezwgealp@archlinux> <20210315135226.avwmnhkfsgof6ihw@pali> <20210315083409.08b1359b@x1.home.shazbot.org> <20210315145238.6sg5deblr2z2pupu@pali> <20210315090339.54546e91@x1.home.shazbot.org> <20210317190206.zrtzwgskxdogl7dz@pali> <20210317131536.38f398b0@omen.home.shazbot.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210317131536.38f398b0@omen.home.shazbot.org> User-Agent: NeoMutt/20180716 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 17 March 2021 13:15:36 Alex Williamson wrote: > On Wed, 17 Mar 2021 20:02:06 +0100 > Pali Rohár wrote: > > > On Monday 15 March 2021 09:03:39 Alex Williamson wrote: > > > On Mon, 15 Mar 2021 15:52:38 +0100 > > > Pali Rohár wrote: > > > > > > > On Monday 15 March 2021 08:34:09 Alex Williamson wrote: > > > > > On Mon, 15 Mar 2021 14:52:26 +0100 > > > > > Pali Rohár wrote: > > > > > > > > > > > On Monday 15 March 2021 19:13:23 Amey Narkhede wrote: > > > > > > > slot reset (pci_dev_reset_slot_function) and secondary bus > > > > > > > reset(pci_parent_bus_reset) which I think are hot reset and > > > > > > > warm reset respectively. > > > > > > > > > > > > No. PCI secondary bus reset = PCIe Hot Reset. Slot reset is just another > > > > > > type of reset, which is currently implemented only for PCIe hot plug > > > > > > bridges and for PowerPC PowerNV platform and it just call PCI secondary > > > > > > bus reset with some other hook. PCIe Warm Reset does not have API in > > > > > > kernel and therefore drivers do not export this type of reset via any > > > > > > kernel function (yet). > > > > > > > > > > Warm reset is beyond the scope of this series, but could be implemented > > > > > in a compatible way to fit within the pci_reset_fn_methods[] array > > > > > defined here. > > > > > > > > Ok! > > > > > > > > > Note that with this series the resets available through > > > > > pci_reset_function() and the per device reset attribute is sysfs remain > > > > > exactly the same as they are currently. The bus and slot reset > > > > > methods used here are limited to devices where only a single function is > > > > > affected by the reset, therefore it is not like the patch you proposed > > > > > which performed a reset irrespective of the downstream devices. This > > > > > series only enables selection of the existing methods. Thanks, > > > > > > > > > > Alex > > > > > > > > > > > > > But with this patch series, there is still an issue with PCI secondary > > > > bus reset mechanism as exported sysfs attribute does not do that > > > > remove-reset-rescan procedure. As discussed in other thread, this reset > > > > let device in unconfigured / broken state. > > > > > > No, there's not: > > > > > > int pci_reset_function(struct pci_dev *dev) > > > { > > > int rc; > > > > > > if (!dev->reset_fn) > > > return -ENOTTY; > > > > > > pci_dev_lock(dev); > > > >>> pci_dev_save_and_disable(dev); > > > > > > rc = __pci_reset_function_locked(dev); > > > > > > >>> pci_dev_restore(dev); > > > pci_dev_unlock(dev); > > > > > > return rc; > > > } > > > > > > The remove/re-scan was discussed primarily because your patch performed > > > a bus reset regardless of what devices were affected by that reset and > > > it's difficult to manage the scope where multiple devices are affected. > > > Here, the bus and slot reset functions will fail unless the scope is > > > limited to the single device triggering this reset. Thanks, > > > > > > Alex > > > > > > > I was thinking a bit more about it and I'm really sure how it would > > behave with hotplugging PCIe bridge. > > > > On aardvark PCIe controller I have already tested that secondary bus > > reset bit is triggering Hot Reset event and then also Link Down event. > > These events are not handled by aardvark driver yet (needs to > > implemented into kernel's emulated root bridge code). > > > > But I'm not sure how it would behave on real HW PCIe hotplugging bridge. > > Kernel has already code which removes PCIe device if it changes presence > > bit (and inform via interrupt). And Link Down event triggers this > > change. > > This is the difference between slot and bus resets, the slot reset is > implemented by the hotplug controller and disables presence detection > around the bus reset. Thanks, Yes, but I'm talking about bus reset, not about slot reset. I mean: to use bus reset via sysfs on hardware which supports slots and hotplugging. And if I'm reading code correctly, this combination is allowed, right? Via these new patches it is possible to disable slot reset and enable bus reset.