All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: "Guilherme G. Piccoli" <gpiccoli@canonical.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	lukas@wunner.de, linux-pci@vger.kernel.org, kernelfans@gmail.com,
	andi@firstfloor.org, hpa@zytor.com, bhe@redhat.com,
	x86@kernel.org, okaya@kernel.org, mingo@redhat.com,
	jay.vosburgh@canonical.com, dyoung@redhat.com,
	gavin.guo@canonical.com, bp@alien8.de, bhelgaas@google.com,
	Guowen Shan <gshan@redhat.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	kernel@gpiccoli.net, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, ddstreet@canonical.com,
	vgoyal@redhat.com
Subject: Re: [PATCH 1/3] x86/quirks: Scan all busses for early PCI quirks
Date: Mon, 16 Nov 2020 18:19:07 -0600	[thread overview]
Message-ID: <20201117001907.GA1342260@bjorn-Precision-5520> (raw)
In-Reply-To: <c8238524-4197-c22d-7bae-df61133fcbfe@canonical.com>

On Mon, Nov 16, 2020 at 05:31:36PM -0300, Guilherme G. Piccoli wrote:
> First of all, thanks everybody for the great insights/discussion! This
> thread ended-up being a great learning for (at least) me.
> 
> Given the big amount of replies and intermixed comments, I wouldn't be
> able to respond inline to all, so I'll try another approach below.
> 
> 
> From Bjorn:
> "I think [0] proposes using early_quirks() to disable MSIs at boot-time.
> That doesn't seem like a robust solution because (a) the problem affects
> all arches but early_quirks() is x86-specific and (b) even on x86
> early_quirks() only works for PCI segment 0 because it relies on the
> 0xCF8/0xCFC I/O ports."
> 
> Ah. I wasn't aware of that limitation, I thought enhancing the
> early_quirks() search to go through all buses would fix that, thanks for
> the clarification! And again, worth to clarify that this is not a
> problem affecting all arches _in practice_ - PowerPC for example has the
> FW primitives allowing a powerful PCI controller (out-of-band) reset,
> preventing this kind of issue usually.
> 
> [0]
> https://lore.kernel.org/linux-pci/20181018183721.27467-1-gpiccoli@canonical.com
> 
> 
> From Bjorn:
> "A crash_device_shutdown() could do something at the host bridge level
> if that's possible, or reset/disable bus mastering/disable MSI/etc on
> individual PCI devices if necessary."
> 
> From Lukas:
> "Guilherme's original patches from 2018 iterate over all 256 PCI buses.
> That might impact boot time negatively.  The reason he has to do that is
> because the crashing kernel doesn't know which devices exist and which
> have interrupts enabled.  However the crashing kernel has that
> information.  It should either disable interrupts itself or pass the
> necessary information to the crashing kernel as setup_data or whatever.

I don't think passing the device information to the kdump kernel is
really practical.  The kdump kernel would use it to do PCI config
writes to disable MSIs before enabling IRQs, and it doesn't know how
to access config space that early.

We could invent special "early config access" things, but that gets
really complicated really fast.  Config access depends on ACPI MCFG
tables, firmware interfaces, and in many cases, on the native host
bridge drivers in drivers/pci/controllers/.

I think we need to disable MSIs in the crashing kernel before the
kexec.  It adds a little more code in the crash_kexec() path, but it
seems like a worthwhile tradeoff.

Bjorn

WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org>
To: "Guilherme G. Piccoli" <gpiccoli@canonical.com>
Cc: Guowen Shan <gshan@redhat.com>,
	linux-pci@vger.kernel.org, kernelfans@gmail.com,
	andi@firstfloor.org, hpa@zytor.com, bhe@redhat.com,
	x86@kernel.org, okaya@kernel.org, mingo@redhat.com,
	jay.vosburgh@canonical.com, dyoung@redhat.com,
	gavin.guo@canonical.com, bp@alien8.de, bhelgaas@google.com,
	Thomas Gleixner <tglx@linutronix.de>,
	kexec@lists.infradead.org, kernel@gpiccoli.net,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	linux-kernel@vger.kernel.org, vgoyal@redhat.com,
	ddstreet@canonical.com, lukas@wunner.de,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH 1/3] x86/quirks: Scan all busses for early PCI quirks
Date: Mon, 16 Nov 2020 18:19:07 -0600	[thread overview]
Message-ID: <20201117001907.GA1342260@bjorn-Precision-5520> (raw)
In-Reply-To: <c8238524-4197-c22d-7bae-df61133fcbfe@canonical.com>

On Mon, Nov 16, 2020 at 05:31:36PM -0300, Guilherme G. Piccoli wrote:
> First of all, thanks everybody for the great insights/discussion! This
> thread ended-up being a great learning for (at least) me.
> 
> Given the big amount of replies and intermixed comments, I wouldn't be
> able to respond inline to all, so I'll try another approach below.
> 
> 
> From Bjorn:
> "I think [0] proposes using early_quirks() to disable MSIs at boot-time.
> That doesn't seem like a robust solution because (a) the problem affects
> all arches but early_quirks() is x86-specific and (b) even on x86
> early_quirks() only works for PCI segment 0 because it relies on the
> 0xCF8/0xCFC I/O ports."
> 
> Ah. I wasn't aware of that limitation, I thought enhancing the
> early_quirks() search to go through all buses would fix that, thanks for
> the clarification! And again, worth to clarify that this is not a
> problem affecting all arches _in practice_ - PowerPC for example has the
> FW primitives allowing a powerful PCI controller (out-of-band) reset,
> preventing this kind of issue usually.
> 
> [0]
> https://lore.kernel.org/linux-pci/20181018183721.27467-1-gpiccoli@canonical.com
> 
> 
> From Bjorn:
> "A crash_device_shutdown() could do something at the host bridge level
> if that's possible, or reset/disable bus mastering/disable MSI/etc on
> individual PCI devices if necessary."
> 
> From Lukas:
> "Guilherme's original patches from 2018 iterate over all 256 PCI buses.
> That might impact boot time negatively.  The reason he has to do that is
> because the crashing kernel doesn't know which devices exist and which
> have interrupts enabled.  However the crashing kernel has that
> information.  It should either disable interrupts itself or pass the
> necessary information to the crashing kernel as setup_data or whatever.

I don't think passing the device information to the kdump kernel is
really practical.  The kdump kernel would use it to do PCI config
writes to disable MSIs before enabling IRQs, and it doesn't know how
to access config space that early.

We could invent special "early config access" things, but that gets
really complicated really fast.  Config access depends on ACPI MCFG
tables, firmware interfaces, and in many cases, on the native host
bridge drivers in drivers/pci/controllers/.

I think we need to disable MSIs in the crashing kernel before the
kexec.  It adds a little more code in the crash_kexec() path, but it
seems like a worthwhile tradeoff.

Bjorn

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  parent reply	other threads:[~2020-11-17  0:19 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-18 18:37 [PATCH 1/3] x86/quirks: Scan all busses for early PCI quirks Guilherme G. Piccoli
2018-10-18 18:37 ` Guilherme G. Piccoli
2018-10-18 18:37 ` [PATCH 2/3] x86/PCI: Export find_cap() to be used in early PCI code Guilherme G. Piccoli
2018-10-18 18:37   ` Guilherme G. Piccoli
2018-10-18 18:37 ` [PATCH 3/3] x86/quirks: Add parameter to clear MSIs early on boot Guilherme G. Piccoli
2018-10-18 18:37   ` Guilherme G. Piccoli
2018-10-18 20:08   ` Sinan Kaya
2018-10-18 20:08     ` Sinan Kaya
2018-10-18 20:13     ` Guilherme G. Piccoli
2018-10-18 20:13       ` Guilherme G. Piccoli
2018-10-18 20:30       ` Sinan Kaya
2018-10-18 20:30         ` Sinan Kaya
2018-10-22 19:44         ` Guilherme G. Piccoli
2018-10-22 19:44           ` Guilherme G. Piccoli
2018-10-18 22:15 ` [PATCH 1/3] x86/quirks: Scan all busses for early PCI quirks Bjorn Helgaas
2018-10-18 22:15   ` Bjorn Helgaas
2018-10-22 20:35   ` Guilherme G. Piccoli
2018-10-22 20:35     ` Guilherme G. Piccoli
2018-10-23 17:03     ` Bjorn Helgaas
2018-10-23 17:03       ` Bjorn Helgaas
2020-11-06 13:14       ` Guilherme G. Piccoli
2020-11-06 13:14         ` Guilherme G. Piccoli
2020-11-13 16:46         ` Bjorn Helgaas
2020-11-13 16:46           ` Bjorn Helgaas
2020-11-13 23:31           ` Thomas Gleixner
2020-11-13 23:31             ` Thomas Gleixner
2020-11-13 23:40             ` Thomas Gleixner
2020-11-13 23:40               ` Thomas Gleixner
2020-11-14 20:39               ` Bjorn Helgaas
2020-11-14 20:39                 ` Bjorn Helgaas
2020-11-14 20:58                 ` Thomas Gleixner
2020-11-14 20:58                   ` Thomas Gleixner
2020-11-14 21:22                   ` Bjorn Helgaas
2020-11-14 21:22                     ` Bjorn Helgaas
2020-11-15 14:05                     ` Eric W. Biederman
2020-11-15 14:05                       ` Eric W. Biederman
2020-11-15 14:29                       ` Eric W. Biederman
2020-11-15 14:29                         ` Eric W. Biederman
2020-11-15 15:11                         ` Thomas Gleixner
2020-11-15 15:11                           ` Thomas Gleixner
2020-11-15 17:01                           ` Lukas Wunner
2020-11-15 19:18                             ` Thomas Gleixner
2020-11-15 19:18                               ` Thomas Gleixner
2020-11-15 20:46                           ` Eric W. Biederman
2020-11-15 20:46                             ` Eric W. Biederman
2020-11-16 20:31                             ` Guilherme G. Piccoli
2020-11-16 20:31                               ` Guilherme G. Piccoli
2020-11-16 21:45                               ` Eric W. Biederman
2020-11-16 21:45                                 ` Eric W. Biederman
2020-11-16 21:49                                 ` Guilherme Piccoli
2020-11-16 21:49                                   ` Guilherme Piccoli
2020-11-17  0:19                               ` Bjorn Helgaas [this message]
2020-11-17  0:19                                 ` Bjorn Helgaas
2020-11-17  1:06                                 ` Eric W. Biederman
2020-11-17  1:06                                   ` Eric W. Biederman
2020-11-17  9:53                                   ` Thomas Gleixner
2020-11-17  9:53                                     ` Thomas Gleixner
2020-11-17 12:19                                     ` David Woodhouse
2020-11-17 12:19                                       ` David Woodhouse
2020-11-17 19:34                                       ` Thomas Gleixner
2020-11-17 19:34                                         ` Thomas Gleixner
2020-11-17 22:25                                         ` Eric W. Biederman
2020-11-17 22:25                                           ` Eric W. Biederman
2020-11-17 12:04                                   ` Guilherme Piccoli
2020-11-17 12:04                                     ` Guilherme Piccoli
2020-11-18 21:05                                     ` Bjorn Helgaas
2020-11-18 21:05                                       ` Bjorn Helgaas
2020-11-18 22:36                                       ` Guilherme Piccoli
2020-11-18 22:36                                         ` Guilherme Piccoli
2020-11-30 20:20                                         ` Bjorn Helgaas
2020-11-30 20:20                                           ` Bjorn Helgaas
2020-12-14 18:32                                           ` Guilherme Piccoli
2020-12-14 18:32                                             ` Guilherme Piccoli

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=20201117001907.GA1342260@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=bhe@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=bp@alien8.de \
    --cc=ddstreet@canonical.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=gavin.guo@canonical.com \
    --cc=gpiccoli@canonical.com \
    --cc=gshan@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jay.vosburgh@canonical.com \
    --cc=kernel@gpiccoli.net \
    --cc=kernelfans@gmail.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mingo@redhat.com \
    --cc=okaya@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --cc=x86@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.