Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Mario Limonciello <mario.limonciello@amd.com>
Cc: alexander.deucher@amd.com, christian.koenig@amd.com,
	airlied@gmail.com, simona@ffwll.ch, bhelgaas@google.com,
	"Reza Nikzad Chaleshori" <rezanikzadch@gmail.com>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-pci@vger.kernel.org, "Alex Williamson" <alex@shazbot.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Lukas Wunner" <lukas@wunner.de>,
	linux-acpi@vger.kernel.org
Subject: Re: [PATCH] drm/amdgpu: Gate VFCT bus fallback on pci=realloc
Date: Mon, 19 Oct 2026 17:19:22 -0500	[thread overview]
Message-ID: <20261019221922.GA2275522@bhelgaas> (raw)
In-Reply-To: <4284d6d4-244d-4d49-bbdb-f1bda89fc1e8@amd.com>

On Thu, Oct 15, 2026 at 03:25:17PM -0500, Mario Limonciello wrote:
> On 10/15/26 14:48, Bjorn Helgaas wrote:
> > [+cc Alex, Ilpo, Lukas, linux-acpi]
> > 
> > On Thu, Oct 15, 2026 at 07:48:42AM -0500, Mario Limonciello wrote:
> > > The soft filter system introduced by commit 11c141672045f ("drm/amdgpu:
> > > Fix VFCT bus number matching with soft filter") has a deficiency that
> > > it won't work when there are multiple GPUs of the exact same model.
> > > 
> > > As it was introduced specifically to help the case of pci=realloc, gate
> > > it to only run when that's in use.
> > > 
> > > Fixes: 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with soft filter")
> > > Reported-by: Reza Nikzad Chaleshori <rezanikzadch@gmail.com>
> > > Closes: https://lore.kernel.org/amd-gfx/20261014080523.186533-1-rezanikzadch@gmail.com/
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 15 ++++++++-------
> > >   drivers/pci/setup-bus.c                  |  5 +++++
> > >   include/linux/pci.h                      |  1 +
> > >   3 files changed, 14 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > > index 08ed1eb695586..75af9a59671ae 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > > @@ -384,12 +384,10 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
> > >    * @vhdr: VFCT image header to check
> > >    *
> > >    * VFCT entries contain the PCI bus number as recorded during BIOS POST.
> > > - * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
> > > - * resource conflicts), the runtime bus number may differ from the POST
> > > - * value.  Match by device identity (vendor + device + function) and use
> > > - * the bus number as a preference: exact bus match is preferred, but when
> > > - * the bus numbers disagree we accept the entry if the device identity
> > > - * matches.
> > > + * On systems where the user requested PCI resource reallocation, the
> > > + * runtime bus number may differ from the POST value. Match by device
> > > + * identity (vendor + device + function), but only treat the bus number as
> > > + * a preference when pci=realloc was explicitly requested.
> > 
> > This is a generic problem, isn't it?
> > 
> > I think any table from firmware that has PCI bus numbers in it could
> > trip over issues like this.  Seems like we need a more generic
> > solution.
> 
> Yeah; it's a generic issue which was triggered by pci=realloc which is why I
> did a soft filter in 11c141672045f.
> 
> But this causes problems when you have two (or more) of the exact same
> device.  You might match the wrong thing.
> 
> > >    *
> > >    * Returns: 0 on match, -ENODEV on no match
> > >    */
> > > @@ -409,7 +407,10 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
> > >   	if (vhdr->PCIBus == adev->pdev->bus->number)
> > >   		return 0;
> > > -	/* Bus mismatch but device identity matches (PCI renumbering case) */
> > > +	/* Allow bus renumbering only when the user requested pci=realloc. */
> > > +	if (!pci_realloc_enabled_by_user())
> > > +		return -ENODEV;
> > > +
> > >   	dev_notice(adev->dev,
> > >   		   "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
> > >   		   vhdr->PCIBus, adev->pdev->bus->number,
> > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > > index ed16ef7c26fa7..d9450585082c0 100644
> > > --- a/drivers/pci/setup-bus.c
> > > +++ b/drivers/pci/setup-bus.c
> > > @@ -1799,6 +1799,11 @@ static bool pci_realloc_enabled(enum enable_type enable)
> > >   	return enable >= user_enabled;
> > >   }
> > > +bool pci_realloc_enabled_by_user(void)
> > > +{
> > > +	return pci_realloc_enable == user_enabled;
> > > +}
> > 
> > I don't like this.  I think the same bus renumbering can happen in the
> > "auto_enabled" case, and I don't think we set pci_realloc_enable in
> > that case.
> > 
> > But more generally, I think ultimately the PCI core should be able to
> > reassign bus numbers arbitrarily to accommodate hotplugs (given the
> > appropriate driver support to tolerate that, of course).
> 
> Could we keep track of the original perhaps?  Then drivers can know
> what it was reassigned from?

I think the only thing that can change is the bus number, and saving
the original bus number in a struct pci_bus or similar might be
practical?

  reply	other threads:[~2026-10-19 22:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15 12:48 [PATCH] drm/amdgpu: Gate VFCT bus fallback on pci=realloc Mario Limonciello
2026-10-15 12:56 ` sashiko-bot
2026-10-15 12:58   ` Mario Limonciello
2026-10-15 19:48 ` Bjorn Helgaas
2026-10-15 20:25   ` Mario Limonciello
2026-10-19 22:19     ` Bjorn Helgaas [this message]
2026-10-19 20:45 ` kernel test robot

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=20261019221922.GA2275522@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=airlied@gmail.com \
    --cc=alex@shazbot.org \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=bhelgaas@google.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mario.limonciello@amd.com \
    --cc=rezanikzadch@gmail.com \
    --cc=simona@ffwll.ch \
    /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