linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Daniel Gomez <da.gomez@kernel.org>,
	linux-pci@vger.kernel.org, David Gow <davidgow@google.com>,
	Rae Moar <rmoar@google.com>,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-modules@vger.kernel.org,
	Johannes Berg <johannes@sipsolutions.net>,
	Sami Tolvanen <samitolvanen@google.com>,
	Richard Weinberger <richard@nod.at>, Wei Liu <wei.liu@kernel.org>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	kunit-dev@googlegroups.com,
	Anton Ivanov <anton.ivanov@cambridgegreys.com>,
	linux-um@lists.infradead.org
Subject: Re: [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules
Date: Mon, 15 Sep 2025 11:41:37 -0700	[thread overview]
Message-ID: <aMhd4REssOE-AlYw@google.com> (raw)
In-Reply-To: <aMgZJgU7p57KC0DL@infradead.org>

Hi Christoph,

On Mon, Sep 15, 2025 at 06:48:22AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 12, 2025 at 03:59:31PM -0700, Brian Norris wrote:
> > This series primarily adds support for DECLARE_PCI_FIXUP_*() in modules.
> > There are a few drivers that already use this, and so they are
> > presumably broken when built as modules.
> 
> That's a reall bad idea, because it allows random code to insert quirks
> not even bound to the hardware they support.

I see fixups in controller drivers here:

drivers/pci/controller/dwc/pci-imx6.c
drivers/pci/controller/dwc/pci-keystone.c
drivers/pci/controller/dwc/pcie-qcom.c
drivers/pci/controller/pci-loongson.c
drivers/pci/controller/pci-tegra.c
drivers/pci/controller/pcie-iproc-bcma.c
drivers/pci/controller/pcie-iproc.c

Are any of those somehow wrong?

And if they are not wrong, then is this a good reason to disallow making
these drivers modular? (Yes, few of them are currently modular; but I
don't see why that *must* be the case.)

I agree, as with many kernel features, there are plenty of ways to use
them incorrectly. But I'm just trying to patch over one rough edge about
how to use them incorrectly, and I don't really see why it's such a bad
idea.

> So no, modules should not allow quirks, but the kernel should probably
> be nice enough to fail compilation when someone is attemping that
> instead of silently ignoring the quirks.

Sure, if consensus says we should not support this, I'd definitely like
to make this failure mode more obvious -- likely a build error.

Thanks for your thoughts,
Brian

  reply	other threads:[~2025-09-15 18:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-12 22:59 [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules Brian Norris
2025-09-12 22:59 ` [PATCH 1/4] PCI: Support " Brian Norris
2025-09-15  6:33   ` Johannes Berg
2025-09-15 18:34     ` Brian Norris
2025-09-23 12:55   ` Petr Pavlu
2025-09-23 17:42     ` Brian Norris
2025-09-24  7:48       ` Petr Pavlu
2025-10-06 22:58         ` Brian Norris
2025-10-20 11:53           ` Petr Pavlu
2025-09-12 22:59 ` [PATCH 2/4] PCI: Add KUnit tests for FIXUP quirks Brian Norris
2025-09-15  8:06   ` Tzung-Bi Shih
2025-09-15 20:25     ` Brian Norris
2025-09-12 22:59 ` [PATCH 3/4] um: Select PCI_DOMAINS_GENERIC Brian Norris
2025-09-12 22:59 ` [PATCH 4/4] kunit: qemu_configs: Add PCI to arm, arm64 Brian Norris
2025-09-15 13:48 ` [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules Christoph Hellwig
2025-09-15 18:41   ` Brian Norris [this message]
2025-09-22 18:13     ` Christoph Hellwig
2025-09-22 18:48       ` Brian Norris
2025-09-29  8:56         ` Christoph Hellwig
2025-09-23 16:20       ` Manivannan Sadhasivam

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=aMhd4REssOE-AlYw@google.com \
    --to=briannorris@chromium.org \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=bhelgaas@google.com \
    --cc=brendan.higgins@linux.dev \
    --cc=da.gomez@kernel.org \
    --cc=davidgow@google.com \
    --cc=hch@infradead.org \
    --cc=johannes@sipsolutions.net \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=mcgrof@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=richard@nod.at \
    --cc=rmoar@google.com \
    --cc=samitolvanen@google.com \
    --cc=wei.liu@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 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).