All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Feng Tang <feng.tang@linux.alibaba.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Jonathan Cameron <Jonthan.Cameron@huawei.com>,
	ilpo.jarvinen@linux.intel.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] PCI: Disable PCIE hotplug interrupts early when msi is disabled
Date: Thu, 6 Feb 2025 07:21:47 +0100	[thread overview]
Message-ID: <Z6RU-681eXl7hcp6@wunner.de> (raw)
In-Reply-To: <Z6LhzGaYBW5S41MJ@U-2FWC9VHC-2323.local>

[to += Rafael, start of thread is here:
https://lore.kernel.org/all/Z6HcoUB3i51bzQDs@wunner.de/
]

Hi Rafael,

On Wed, Feb 05, 2025 at 11:58:04AM +0800, Feng Tang wrote:
> On Tue, Feb 04, 2025 at 10:23:45AM +0100, Lukas Wunner wrote:
> > On Tue, Feb 04, 2025 at 01:37:58PM +0800, Feng Tang wrote:
> > > There was a irq storm bug when testing "pci=nomsi" case, and the root
> > > cause is: 'nomsi' will disable MSI and let devices and root ports use
> > > legacy INTX inerrupt, and likely make several devices/ports share one
> > > interrupt. In the failure case, BIOS doesn't disable the PCIE hotplug
> > > interrupts, and  actually asserts the command-complete interrupt.
> > > As MSI is disabled, ACPI initialization code will not enumerate root
> > > port's PCIE hotplug capability, and pciehp service driver wont' be
> > > enabled for the root port to handle that interrupt, later on when it is
> > > shared and enabled by other device driver like NVME or NIC, the "nobody
> > > care irq storm" happens.
> >
> > Is there a section in the PCI Firmware Spec which says ACPI doesn't
> > enumerate the hotplug capability if MSI is disabled?
> 
> No, I didn't get it from spec, but found the logic by code reading
> during debugging the irq storm issue. The related code is about:
> 
> #define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
> 				| OSC_PCI_ASPM_SUPPORT \
> 				| OSC_PCI_CLOCK_PM_SUPPORT \
> 				| OSC_PCI_MSI_SUPPORT)

Commit 415e12b23792 ("PCI/ACPI: Request _OSC control once for each root
bridge (v3)") contains a change which doesn't seem to be explained in
the commit message:

If the user passes "pci=nomsi" on the command line, Linux doesn't
request hotplug control (or any other control) from the platform.
So ACPI always remains responsible for hotplug in the "pci=nomsi"
case.

The commit sought to fix a cpu hog issue:

https://bugzilla.kernel.org/show_bug.cgi?id=29722

It's unclear to me if that bug was fixed by requesting _OSC only once,
as the commit message suggests, or if the addition of OSC_MSI_SUPPORT
to ACPI_PCIE_REQ_SUPPORT fixed the issue.

Since the latter is not mentioned in the commit message,
it seems plausible to assume that the OSC_MSI_SUPPORT change
was unintentional.

In any case it doesn't seem to make sense to not request any
control in the "pci=nomsi" case.

It's also worth noting that the behavior is different on
Apple machines as they use a fixed _OSC set even for "pci=nomsi".

I'm wondering if OSC_PCI_MSI_SUPPORT should simply be removed
from ACPI_PCIE_REQ_SUPPORT, but I'm worried that it may cause
reappearance of the cpu hog issue.

Thoughts?

Thanks,

Lukas

  reply	other threads:[~2025-02-06  6:21 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-04  5:37 [PATCH 1/2] PCI/portdrv: Add necessary delay for disabling hotplug events Feng Tang
2025-02-04  5:37 ` [PATCH 2/2] PCI: Disable PCIE hotplug interrupts early when msi is disabled Feng Tang
2025-02-04  9:14   ` Lukas Wunner
2025-02-05  6:31     ` Feng Tang
2025-02-05 13:31       ` Feng Tang
2025-02-04  9:23   ` Lukas Wunner
2025-02-05  3:58     ` Feng Tang
2025-02-06  6:21       ` Lukas Wunner [this message]
2025-02-12 13:04         ` Feng Tang
2025-02-04  9:07 ` [PATCH 1/2] PCI/portdrv: Add necessary delay for disabling hotplug events Lukas Wunner
2025-02-05  2:46   ` Feng Tang
2025-02-05 17:48 ` Markus Elfring
2025-02-06  2:42   ` Feng Tang
2025-02-06 11:40     ` [1/2] " Markus Elfring
2025-02-07  1:40       ` Feng Tang
2025-02-05 18:26 ` [PATCH 1/2] " Sathyanarayanan Kuppuswamy
2025-02-06  3:18   ` Feng Tang
2025-02-07  4:26     ` Sathyanarayanan Kuppuswamy
2025-02-07  6:17       ` Feng Tang

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=Z6RU-681eXl7hcp6@wunner.de \
    --to=lukas@wunner.de \
    --cc=Jonthan.Cameron@huawei.com \
    --cc=bhelgaas@google.com \
    --cc=feng.tang@linux.alibaba.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rafael@kernel.org \
    /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.