linux-um.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Brian Norris <briannorris@chromium.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	 Luis Chamberlain	 <mcgrof@kernel.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Daniel Gomez	 <da.gomez@kernel.org>
Cc: 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,
	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 1/4] PCI: Support FIXUP quirks in modules
Date: Mon, 15 Sep 2025 08:33:08 +0200	[thread overview]
Message-ID: <8e75d6cc3847899ba8d6a0cbd0ef3ac57eabf009.camel@sipsolutions.net> (raw)
In-Reply-To: <20250912230208.967129-2-briannorris@chromium.org> (sfid-20250913_010956_669404_FC913C9D)

On Fri, 2025-09-12 at 15:59 -0700, Brian Norris wrote:
> The PCI framework supports "quirks" for PCI devices via several
> DECLARE_PCI_FIXUP_*() macros. These macros allow arch or driver code to
> match device IDs to provide customizations or workarounds for broken
> devices.
> 
> This mechanism is generally used in code that can only be built into the
> kernel, but there are a few occasions where this mechanism is used in
> drivers that can be modules. For example, see commit 574f29036fce ("PCI:
> iproc: Apply quirk_paxc_bridge() for module as well as built-in").
> 
> The PCI fixup mechanism only works for built-in code, however, because
> pci_fixup_device() only scans the ".pci_fixup_*" linker sections found
> in the main kernel; it never touches modules.
> 
> Extend the fixup approach to modules.

This _feels_ a bit odd to me - what if you reload a module, should the
fixup be done twice? Right now this was not possible in a module, which
is a bit of a gotcha, but at least that's only one for developers, not
for users (unless someone messes up and puts it into modular code, as in
the example you gave.)

Although, come to think of it, you don't even apply the fixup when the
module is loaded, so what I just wrote isn't really true. That almost
seems like an oversight though, now the module has to be loaded before
the PCI device is enumerated, which is unlikely to happen in practice?
But then we get the next gotcha - the device is already enumerated, so
the fixups cannot be applied at the various enumeration stages, and
you're back to having to load the module before PCI enumeration, which
could be tricky, or somehow forcing re-enumeration of a given device
from userspace, but then you're firmly in "gotcha for the user"
territory again ...

I don't really have any skin in this game, but overall I'd probably
argue it's better to occasionally have to fix things such as in the
commit you point out but have a predictable system, than apply things
from modules.

Perhaps it'd be better to extend the section checking infrastructure to
catch and error out on these sections in modules instead, so we catch it
at build time, rather than finding things missing at runtime?

And yeah, now I've totally ignored the kunit angle, but ... not sure how
to combine the two requirements if they are, as I think, conflicting.

johannes


  reply	other threads:[~2025-09-15  6:33 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 [this message]
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
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=8e75d6cc3847899ba8d6a0cbd0ef3ac57eabf009.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=bhelgaas@google.com \
    --cc=brendan.higgins@linux.dev \
    --cc=briannorris@chromium.org \
    --cc=da.gomez@kernel.org \
    --cc=davidgow@google.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).