Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/amdgpu: amdgpu_acpi_vfct_match() uses post-renumbering initial_bus_number --
commit 1c0de5f9175b6acb9431f78243e7c9f86e945b98 Author: Mario Limonciello <[email protected]> drm/amdgpu: Match VFCT using initial PCI bus number This commit updates amdgpu_acpi_vfct_match() to match the VFCT header's PCIBus field against adev->pdev->initial_bus_number instead of accepting bus mismatches based on device identity alone. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > index f9315e0472fe9..7b66dc2414250 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > @@ -436,10 +436,8 @@ static bool amdgpu_read_disabled_bios(struct > amdgpu_device *adev) > * 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. > + * value. Match using the bus number captured when the device was > + * discovered, before any bus renumbering. > * > * Returns: 0 on match, -ENODEV on no match > */ > @@ -455,16 +453,7 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device > *adev, > vhdr->PCIFunction != PCI_FUNC(adev->pdev->devfn)) > return -ENODEV; > > - /* Exact bus number match - preferred */ > - if (vhdr->PCIBus == adev->pdev->bus->number) > - return 0; > - > - /* Bus mismatch but device identity matches (PCI renumbering case) */ > - 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, > - adev->pdev->vendor, adev->pdev->device); > - return 0; > + return vhdr->PCIBus == adev->pdev->initial_bus_number ? 0 : -ENODEV; > } [Severity: High] Does adev->pdev->initial_bus_number actually hold the BIOS POST bus number when the PCI core renumbers a bridge? In drivers/pci/probe.c, pci_alloc_dev() initializes initial_bus_number when the pci_dev structure is allocated during pci_scan_device(): drivers/pci/probe.c:pci_alloc_dev() { ... dev->bus = pci_bus_get(bus); dev->initial_bus_number = bus->number; ... } However, when pci_scan_bridge_extend() renumbers a bridge (for example, when pcibios_assign_all_busses() is true or when the firmware bridge configuration is broken/conflicting), it skips scanning behind the bridge during pass 0 and clears the firmware bus numbers in the bridge's PCI_PRIMARY_BUS register: drivers/pci/probe.c:pci_scan_bridge_extend() { ... if (!pass) { if (pcibios_assign_all_busses() || broken) pci_write_config_dword(dev, PCI_PRIMARY_BUS, buses & PCI_SEC_LATENCY_TIMER_MASK); goto out; } ... child = pci_find_bus(pci_domain_nr(bus), next_busnr); if (!child) { child = pci_add_new_bus(bus, dev, next_busnr); ... } ... max = pci_scan_child_bus_extend(child, available_buses); } The child pci_bus and its devices are only allocated and scanned in pass 1, after child->number has already been set to the newly assigned next_busnr in pci_add_new_bus() -> pci_alloc_child_bus(). Because bus->number is not changed after child bus creation, pdev->initial_bus_number is always equal to the post-renumbering runtime pdev->bus->number. When the kernel renumbers the PCI bus hierarchy leading to the GPU during amdgpu_device_init() -> amdgpu_get_bios() -> amdgpu_acpi_vfct_bios() -> amdgpu_acpi_vfct_match(), won't vhdr->PCIBus == adev->pdev->initial_bus_number evaluate to false and return -ENODEV, causing VFCT VBIOS lookup to fail just as it did before commit 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with soft filter")? > > static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
