linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Sathyanarayanan Kuppuswamy Natarajan 
	<sathyanarayanan.nkuppuswamy@gmail.com>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Linux PCI <linux-pci@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	Keith Busch <kbusch@kernel.org>,
	knsathya@kernel.org, Sinan Kaya <okaya@kernel.org>
Subject: Re: [PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered
Date: Wed, 17 Mar 2021 20:09:52 +0100	[thread overview]
Message-ID: <20210317190952.GB27146@wunner.de> (raw)
In-Reply-To: <CAPcyv4gfBTuEj494aeg0opeL=PSbk_Cs16fX7A-cLvSV6EZg-Q@mail.gmail.com>

On Wed, Mar 17, 2021 at 10:45:21AM -0700, Dan Williams wrote:
> Ah, ok, we're missing a flush of the hotplug event handler after the
> link is up to make sure the hotplug handler does not see the Link Up.
> I'm not immediately seeing how the new proposal ensures that there is
> no Link Up event still in flight after DPC completes its work.
> Wouldn't it be required to throw away Link Up to Link Up transitions?

If you look at the new code added to pciehp_ist() by my patch...

      atomic_and(~PCI_EXP_SLTSTA_DLLSC, &ctrl->pending_events);
      if (pciehp_check_link_active(ctrl) > 0)
              events &= ~PCI_EXP_SLTSTA_DLLSC;

... the atomic_and() ignores the Link Up event which was picked
up by the hardirq handler pciehp_isr() while pciehp_ist() waited for
link recovery.  Afterwards, the Link Down event is only ignored if the
link is still up:  If the link has gone down again before the call to
pciehp_check_link_active(), that event is honored immediately (because
the DLLSC event is then not filtered).  If it goes down after the call,
that event will be picked up by pciehp_isr().  Thus, only the DLLSC
events caused by DPC are ignored, but no others.

A DLLSC event caused by surprise removal during DPC may be incorrectly
ignored, but the expectation is that the ensuing Presence Detect Changed
event will still cause bringdown of the slot after DPC has completed.
Hardware does exist which erroneously hardwires Presence Detect to zero,
but that's rare and DPC-capable systems are likely not affected.

I've realized today that I forgot to add a "synchronize_hardirq(irq);"
before the call to atomic_and().  Sorry, that will be fixed in the
next iteration.

Thanks,

Lukas

  parent reply	other threads:[~2021-03-17 19:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-13  3:32 [PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered sathyanarayanan.kuppuswamy
2021-03-13  3:35 ` Kuppuswamy, Sathyanarayanan
2021-03-17  4:13 ` Lukas Wunner
2021-03-17  5:08   ` Dan Williams
2021-03-17  5:31     ` Lukas Wunner
2021-03-17 16:31       ` Dan Williams
2021-03-17 17:19         ` Sathyanarayanan Kuppuswamy Natarajan
2021-03-17 17:45           ` Dan Williams
2021-03-17 17:54             ` Sathyanarayanan Kuppuswamy Natarajan
2021-03-17 19:01               ` Lukas Wunner
2021-03-17 20:02                 ` Kuppuswamy, Sathyanarayanan
2021-03-18 15:35                   ` Sinan Kaya
2021-03-28  9:53                   ` Lukas Wunner
2021-03-17 19:09             ` Lukas Wunner [this message]
2021-03-17 19:22               ` Raj, Ashok
2021-03-17 19:40                 ` Lukas Wunner
2021-03-28  5:49   ` Kuppuswamy, Sathyanarayanan
2021-03-28  9:07     ` Lukas Wunner

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=20210317190952.GB27146@wunner.de \
    --to=lukas@wunner.de \
    --cc=ashok.raj@intel.com \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=kbusch@kernel.org \
    --cc=knsathya@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=okaya@kernel.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=sathyanarayanan.nkuppuswamy@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 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).