LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Amey Narkhede <ameynarkhede03@gmail.com>
Cc: alex.williamson@redhat.com,
	Raphael Norwitz <raphael.norwitz@nutanix.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kw@linux.com, Shanker Donthineni <sdonthineni@nvidia.com>,
	Sinan Kaya <okaya@kernel.org>, Len Brown <lenb@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v10 4/8] PCI/sysfs: Allow userspace to query and set device reset mechanism
Date: Wed, 28 Jul 2021 15:18:30 -0500	[thread overview]
Message-ID: <20210728201830.GA845737@bjorn-Precision-5520> (raw)
In-Reply-To: <20210728185831.isdqa5t5oxxjfhnv@archlinux>

On Thu, Jul 29, 2021 at 12:28:31AM +0530, Amey Narkhede wrote:
> On 21/07/28 01:13PM, Bjorn Helgaas wrote:
> > On Wed, Jul 28, 2021 at 11:29:50PM +0530, Amey Narkhede wrote:
> > > On 21/07/27 06:28PM, Bjorn Helgaas wrote:
> > > > On Fri, Jul 09, 2021 at 06:08:09PM +0530, Amey Narkhede wrote:
> > > > > Add reset_method sysfs attribute to enable user to query and set user
> > > > > preferred device reset methods and their ordering.
> > > > >
> > > > > Co-developed-by: Alex Williamson <alex.williamson@redhat.com>
> > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > > > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > > > > ---
> > > > >  Documentation/ABI/testing/sysfs-bus-pci |  19 +++++
> > > > >  drivers/pci/pci-sysfs.c                 | 103 ++++++++++++++++++++++++
> > > > >  2 files changed, 122 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> > > > > index ef00fada2..43f4e33c7 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-bus-pci
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-pci
> > > > > @@ -121,6 +121,25 @@ 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. Initially reading
> > > > > +		this file will give names of the device supported reset
> > > > > +		methods and their ordering. After write, this file will
> > > > > +		give names and ordering of currently enabled reset methods.
> > > > > +		Writing the name or comma separated list of names of any of
> > > > > +		the device supported reset methods to this file will set
> > > > > +		the reset methods and their ordering to be used when
> > > > > +		resetting the device. Writing empty string to this file
> > > > > +		will disable ability to reset the device and writing
> > > > > +		"default" will return to the original value.
> > > > > +
> > > > >  What:		/sys/bus/pci/devices/.../reset
> > > > >  Date:		July 2009
> > > > >  Contact:	Michael S. Tsirkin <mst@redhat.com>
> > > >
> > > [...]
> > >
> > > > > +		int i;
> > > > > +
> > > > > +		if (sysfs_streq(name, ""))
> > > > > +			continue;
> > > > > +
> > > > > +		name = strim(name);
> > > > > +
> > > > > +		for (i = 1; i < PCI_NUM_RESET_METHODS; i++) {
> > > > > +			if (sysfs_streq(name, pci_reset_fn_methods[i].name) &&
> > > > > +			    !pci_reset_fn_methods[i].reset_fn(pdev, 1)) {
> > > > > +				reset_methods[n++] = i;
> > > >
> > > > Can we build this directly in pdev->reset_methods[] so we don't need
> > > > the memcpy() below?
> > > >
> > > This is to avoid writing partial values directly to dev->reset_methods.
> > > So for example if user writes flr,unsupported_value then
> > > dev->reset_methods should not be modified even though flr is valid reset
> > > method in this example to avoid partial writes. Either operation(in this
> > > case writing supported reset methods to reset_method attr) succeeds
> > > completely or it fails othewise.
> >
> > I guess the question is, if somebody writes
> >
> >   flr junk bus
> >
> > and we set the supported methods to "flr bus", is that OK?  It seems
> > OK to me; obviously we have to protect ourselves from attack, but
> > over-zealous checking can make things unnecessarily complicated.
>
> The problem is once we encounter "junk" we return -EINVAL so in your
> example we only set flr.

We're still talking about whether we need the memcpy().  We can decide
whether it's the right thing to return -EINVAL if we encounter "junk".

Maybe it's OK if we set it to "flr" and ignore everything after
"junk"?  Or maybe it's OK to ignore unsupported values and set it to
"flr bus"?

  reply	other threads:[~2021-07-28 20:18 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 12:38 [PATCH v10 0/8] Expose and manage PCI device reset Amey Narkhede
2021-07-09 12:38 ` [PATCH v10 1/8] PCI: Add pcie_reset_flr to follow calling convention of other reset methods Amey Narkhede
2021-07-12 22:07   ` Alex Williamson
2021-07-27 22:12   ` Bjorn Helgaas
2021-07-28 18:54     ` Shanker R Donthineni
2021-07-28 20:23       ` Bjorn Helgaas
2021-07-28 21:58         ` Shanker R Donthineni
2021-07-28 22:04           ` Shanker R Donthineni
2021-07-28 22:16           ` Bjorn Helgaas
2021-07-09 12:38 ` [PATCH v10 2/8] PCI: Add new array for keeping track of ordering of " Amey Narkhede
2021-07-27 22:59   ` Bjorn Helgaas
2021-07-28 17:45     ` Amey Narkhede
2021-07-28 17:59       ` Bjorn Helgaas
2021-07-28 18:17         ` Shanker R Donthineni
2021-07-28 18:08       ` Shanker R Donthineni
2021-07-28 18:31     ` Shanker R Donthineni
2021-07-28 20:25       ` Bjorn Helgaas
2021-07-28 22:01         ` Shanker R Donthineni
2021-07-09 12:38 ` [PATCH v10 3/8] PCI: Remove reset_fn field from pci_dev Amey Narkhede
2021-07-09 12:38 ` [PATCH v10 4/8] PCI/sysfs: Allow userspace to query and set device reset mechanism Amey Narkhede
2021-07-27 23:28   ` Bjorn Helgaas
2021-07-28  1:27     ` Krzysztof Wilczyński
2021-07-28 15:36       ` Bjorn Helgaas
2021-07-28 17:59     ` Amey Narkhede
2021-07-28 18:13       ` Bjorn Helgaas
2021-07-28 18:58         ` Amey Narkhede
2021-07-28 20:18           ` Bjorn Helgaas [this message]
2021-07-31 19:15     ` Amey Narkhede
2021-07-28 17:09   ` Bjorn Helgaas
2021-07-09 12:38 ` [PATCH v10 5/8] PCI: Define a function to set ACPI_COMPANION in pci_dev Amey Narkhede
2021-07-12 22:29   ` Alex Williamson
2021-07-09 12:38 ` [PATCH v10 6/8] PCI: Setup ACPI fwnode early and at the same time with OF Amey Narkhede
2021-07-12 23:09   ` Alex Williamson
2021-07-27 23:30   ` Bjorn Helgaas
2021-07-27 23:50     ` Shanker R Donthineni
2021-07-09 12:38 ` [PATCH v10 7/8] PCI: Add support for ACPI _RST reset method Amey Narkhede
2021-07-12 23:09   ` Alex Williamson
2021-07-13  0:51     ` Shanker R Donthineni
2021-07-14 22:56       ` Alex Williamson
2021-07-09 12:38 ` [PATCH v10 8/8] PCI: Change the type of probe argument in reset functions Amey Narkhede
2021-07-12 22:24   ` Alex Williamson
2021-07-27 22:22   ` Bjorn Helgaas
2021-07-28 17:35     ` Amey Narkhede
2021-07-28 17:55       ` Bjorn Helgaas
2021-07-09 12:46 ` [PATCH v10 0/8] Expose and manage PCI device reset Amey Narkhede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210728201830.GA845737@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=alex.williamson@redhat.com \
    --cc=ameynarkhede03@gmail.com \
    --cc=kw@linux.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=okaya@kernel.org \
    --cc=raphael.norwitz@nutanix.com \
    --cc=rjw@rjwysocki.net \
    --cc=sdonthineni@nvidia.com \
    --subject='Re: [PATCH v10 4/8] PCI/sysfs: Allow userspace to query and set device reset mechanism' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).