All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>,
	linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org,
	ira.weiny@intel.com, vishal.l.verma@intel.com,
	alison.schofield@intel.com, Jonathan.Cameron@huawei.com,
	dave@stgolabs.net, bhelgaas@google.com
Subject: Re: [PATCH v4 3/4] PCI: Create new reset method to force SBR for CXL
Date: Sat, 27 Apr 2024 08:19:56 +0200	[thread overview]
Message-ID: <ZiyZDKSxtfeYi0N4@wunner.de> (raw)
In-Reply-To: <662c04a5d8f5f_b6e02945f@dwillia2-mobl3.amr.corp.intel.com.notmuch>

On Fri, Apr 26, 2024 at 12:46:45PM -0700, Dan Williams wrote:
> This also highlights that the pci_dev_lock() performed by
> pci_reset_function() has long been insufficient for the
> pci_reset_bus_function() method case that could race userspace when
> pci_reset_secondary_bus() is manipulating the bridge control register.
> 
> So, if the goal of the lock is to prevent userspace from clobbering the
> kernel's read-modify-write cycles of @dev's parent bridge, then the lock
> needs to be held over both the cxl_reset_bus_function() and the
> pci_reset_bus_function() cases, and it needs to be taken in
> upstream-bridge => endpoint order.

No, the device lock is taken to prevent the driver from unbinding.
It has nothing to do with protecting RMW of parent bridge registers.

Here's Christoph Hellwig's explanation why he introduced acquisition
of the device lock in the PCI reset code paths:

https://lore.kernel.org/all/20200325104018.GA30853@lst.de/

TL;DR:  The PCI core calls the driver's ->reset_prepare and ->reset_done
callbacks and the driver needs to be held in place for that.

Thanks,

Lukas

  reply	other threads:[~2024-04-27  6:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-09 16:01 [PATCH 0/4 v4] PCI: Add Secondary Bus Reset (SBR) support for CXL Dave Jiang
2024-04-09 16:01 ` [PATCH v4 1/4] PCI/cxl: Move PCI CXL vendor Id to a common location from CXL subsystem Dave Jiang
2024-04-09 21:28   ` Kuppuswamy Sathyanarayanan
2024-04-09 22:51   ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 2/4] PCI: Add check for CXL Secondary Bus Reset Dave Jiang
2024-04-09 21:39   ` Kuppuswamy Sathyanarayanan
2024-04-09 21:56     ` Dave Jiang
2024-04-11  2:33       ` Kuppuswamy Sathyanarayanan
2024-04-09 22:56   ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 3/4] PCI: Create new reset method to force SBR for CXL Dave Jiang
2024-04-26 19:46   ` Dan Williams
2024-04-27  6:19     ` Lukas Wunner [this message]
2024-04-27 17:07       ` Dan Williams
2024-04-09 16:01 ` [PATCH v4 4/4] cxl: Add post reset warning if reset results in loss of previously committed HDM decoders Dave Jiang
2024-04-26 20:03   ` Dan Williams

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=ZiyZDKSxtfeYi0N4@wunner.de \
    --to=lukas@wunner.de \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=vishal.l.verma@intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.