All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Stuart Hayes <stuart.w.hayes@gmail.com>
Cc: Austin Bolen <austin_bolen@dell.com>,
	keith.busch@intel.com, Alexandru Gagniuc <mr.nuke.me@gmail.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
	Sinan Kaya <okaya@kernel.org>,
	Oza Pawandeep <poza@codeaurora.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	lukas@wunner.de
Subject: Re: [PATCH] PCI: pciehp: Make sure pciehp_isr clears interrupt events
Date: Tue, 26 Nov 2019 17:37:35 -0600	[thread overview]
Message-ID: <20191126233735.GA215993@google.com> (raw)
In-Reply-To: <a7ac93e3-9a1f-2cd5-bf0b-30b562bd707d@gmail.com>

On Mon, Nov 25, 2019 at 03:03:23PM -0600, Stuart Hayes wrote:
> 
> On 11/12/19 3:59 PM, Stuart Hayes wrote:
> > The pciehp interrupt handler pciehp_isr() will read the slot status
> > register and then write back to it to clear just the bits that caused the
> > interrupt. If a different interrupt event bit gets set between the read and
> > the write, pciehp_isr() will exit without having cleared all of the
> > interrupt event bits, so we will never get another hotplug interrupt from
> > that device.
> > 
> > That is expected behavior according to the PCI Express spec (v.5.0, section
> > 6.7.3.4, "Software Notification of Hot-Plug Events").
> > 
> > Because the "presence detect changed" and "data link layer state changed"
> > event bits are both getting set at nearly the same time when a device is
> > added or removed, this is more likely to happen than it might seem. The
> > issue can be reproduced rather easily by connecting and disconnecting an
> > NVMe device on at least one system model.
> > 
> > This patch fixes the issue by modifying pciehp_isr() to loop back and
> > re-read the slot status register immediately after writing to it, until
> > it sees that all of the event status bits have been cleared.
> > 
> > Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> 
> Bjorn,
> 
> Do you have any comments or issues with this patch set?  Anything I can do?

Were you planning to address Lukas' comments?

https://lore.kernel.org/r/20191114025022.wz3gchr7w67fjtzn@wunner.de

  reply	other threads:[~2019-11-26 23:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-12 21:59 [PATCH] PCI: pciehp: Make sure pciehp_isr clears interrupt events Stuart Hayes
2019-11-13  4:59 ` Lukas Wunner
2019-11-13 21:58   ` Stuart Hayes
2019-11-14  2:50     ` Lukas Wunner
2019-11-13 12:13 ` kbuild test robot
2019-11-13 12:13   ` kbuild test robot
2019-11-25 21:03 ` Stuart Hayes
2019-11-26 23:37   ` Bjorn Helgaas [this message]
2019-11-27  0:06     ` Stuart Hayes

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=20191126233735.GA215993@google.com \
    --to=helgaas@kernel.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=austin_bolen@dell.com \
    --cc=gustavo@embeddedor.com \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mr.nuke.me@gmail.com \
    --cc=okaya@kernel.org \
    --cc=poza@codeaurora.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=stuart.w.hayes@gmail.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.