Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 15 Mar 2021 19:13:23 +0530
From:   Amey Narkhede <ameynarkhede03@gmail.com>
To:     Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
Cc:     bhelgaas@google.com, alex.williamson@redhat.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: <20210315134323.llz2o7yhezwgealp@archlinux>
References: <20210312173452.3855-1-ameynarkhede03@gmail.com>
 <20210312173452.3855-5-ameynarkhede03@gmail.com>
 <20210314235545.girtrazsdxtrqo2q@pali>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210314235545.girtrazsdxtrqo2q@pali>
Precedence: bulk

On 21/03/15 12:55AM, Pali Roh?r wrote:
> On Friday 12 March 2021 23:04:52 ameynarkhede03@gmail.com wrote:
> > From: Amey Narkhede <ameynarkhede03@gmail.com>
> >
> > Add reset_methods_enabled bitmap to struct pci_dev to
> > keep track of user preferred device reset mechanisms.
> > Add reset_method sysfs attribute to query and set
> > user preferred device reset mechanisms.
> >
> > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > ---
> > Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
> > Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> >
> >  Documentation/ABI/testing/sysfs-bus-pci | 15 ++++++
> >  drivers/pci/pci-sysfs.c                 | 66 +++++++++++++++++++++++--
> >  drivers/pci/pci.c                       |  3 +-
> >  include/linux/pci.h                     |  2 +
> >  4 files changed, 82 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> > index 25c9c3977..ae53ecd2e 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-pci
> > +++ b/Documentation/ABI/testing/sysfs-bus-pci
> > @@ -121,6 +121,21 @@ Description:
> >  		child buses, and re-discover devices removed earlier
> >  		from this part of the device tree.
> >
> > +What:		/sys/bus/pci/devices/.../reset_method
> > +Date:		March 2021
> > +Contact:	Amey Narkhede <ameynarkhede03@gmail.com>
> > +Description:
> > +		Some devices allow an individual function to be reset
> > +		without affecting other functions in the same slot.
> > +		For devices that have this support, a file named reset_method
> > +		will be present in sysfs. Reading this file will give names
> > +		of the device supported reset methods. Currently used methods
> > +		are enclosed in brackets. Writing the name of any of the device
> > +		supported reset method to this file will set the reset method to
> > +		be used when resetting the device. Writing "none" to this file
> > +		will disable ability to reset the device and writing "default"
> > +		will return to the original value.
> > +
>
> Hello Amey!
>
> I think that this API does not work for PCIe Hot Reset (=PCI secondary
> bus reset) and PCIe Warm Reset.
>
> First reset method is bound to the bus, not device and therefore kernel
> does not have to see any registered device. So there would be no
> "reset_method" sysfs file, and also no "reset" sysfs file. But PCIe Hot
> Reset is in most cases needed when buggy card is not registered on bus,
> to trigger this reset. And with this API this is not possible.
>
> PCIe Warm Reset is done by PERST# signal. When signal is asserted then
> device is in reset state and therefore is not registered. So again
> kernel does not have to see registered device.
>
> Moreover for mPCIe form factor cards, boards can share one PERST# signal
> with more PCIe cards and control this signal via GPIO. So asserting
> PERST# GPIO can trigger Warm reset for more PCIe cards, not just one. It
> depends on board or topology.
>
> So... I do not think that current approach with "reset_method" sysfs
> entry bound to the PCI device does not work for PCI secondary bus reset
> and also cannot be used for implementing PCIe Warm Reset.
>
> I would rather suggest to re-design and prepare a new API which would
> work also with PCIe Hot Reset and PCIe Warm Reset.
>
> This "reset" sysfs file can work only with PCI Function Level Reset or
> some PM or device specific reset. But not with reset types which are
> more like slot or bus orientated.
>
The scope of this patch was to expose current reset methods
to the userspace. Also reset methods are available
for only those devices that allow an individual function to be reset
without affecting other functions in the same device.
So if those conditions are satisfied by the device then it can
use 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.

Thanks,
Amey
[...]