On Tue, 11 Nov 2025, Christian König wrote:
> On 11/11/25 12:08, Ilpo Järvinen wrote:
> > On Tue, 11 Nov 2025, Christian König wrote:
> >
> >> Sorry for the late reply I'm really busy at the moment.
> >>
> >> On 10/28/25 18:35, Ilpo Järvinen wrote:
> >>> PCI core handles releasing device's resources and their rollback in
> >>> case of failure of a BAR resizing operation. Releasing resource prior
> >>> to calling pci_resize_resource() prevents PCI core from restoring the
> >>> BARs as they were.
> >>
> >> I've intentionally didn't do it this way because at least on AMD HW we
> >> could only release the VRAM and doorbell BAR (both 64bit), but not the
> >> register BAR (32bit only).
> >>
> >> This patch set looks like the right thing in general, but which BARs are
> >> now released by pci_resize_resource()?
> >>
> >> If we avoid releasing the 32bit BAR as well then that should work,
> >> otherwise we will probably cause problems.
> >
> > After these changes, pci_resize_resource() releases BARs that share the
> > bridge window with the BAR to be resized. So the answer depends on the
> > upstream bridge.
> >
> > However, amdgpu_device_resize_fb_bar() also checks that root bus has a
> > resource with a 64-bit address. That won't tell what the nearest bridge
> > has though. Maybe that check should be converted to check the resources of
> > the nearest bus instead? It would make it impossible to have the
> > 32-bit resource share the bridge window with the 64-bit resources so the
> > resize would be safe.
>
> Mhm, I don't think that will work.
>
>
> I've added the check for the root bus to avoid a couple of issues during
> resize, but checking the nearest bridge would block a whole bunch of use
> cases and isn't even 100% save.
>
> See one use case of this is that all the BARs of the device start in the
> same 32bit bridge window (or a mixture of 64bit and 32bit window).
"32bit bridge window" is ambiguous. There are non-prefetchable and
prefetchable bridge windows, out of which the latter can be 64-bit as
well. Which one you're talking about?
If a 64-bit prefetchable window exists, pbus_size_mem() nor
__pci_assign_resource() would not have produced such a configuration where
they're put into the same bridge window, even before the commit
ae88d0b9c57f ("PCI: Use pbus_select_window_for_type() during mem window
sizing") (I think). Now pbus_size_mem() certainly doesn't.
> What we have is that BAR 0 and 2 are 64bit BARs which can (after some
> preparation) move around freely. But IIRC BAR 4 are the legacy I/O ports
> and BAR 5 is the 32bit MMIO registers (don't nail me on that, could be
> just the other way around).
>
> Especially that 32bit MMIO BAR *can't* move! Not only because it is
> 32bit, but also because the amdgpu driver as well as the HW itself
> through the VGA emulation, as well as the EFI/VESA/VBIOS code might
> reference its absolute address.
So if the 64-bit check is replaced with this:
+ /* Check if the parent bridge has a 64-bit (pref) memory resource */
+ res = pci_resource_n(adev->pdev, 0)->parent;
+ /* Trying to resize is pointless without a window above 4GB */
+ if (!(res->flags & IORESOURCE_MEM_64))
return 0;
...I don't think it's possible for 32-bit resource to share that window
under _any_ circumstance.
If you say that ->parent somehow points to a non-IORESOURCE_MEM_64 window
at this point, you're implying allocation for the 64-bit prefetchable
window was tried and failed, and __pci_assign_resource() then used one of
its fallbacks.
Are you saying that "some preparation" includes making room for that
64-bit prefetchable window that failed to assign earlier as I cannot see
how else it would ever get assigned so that the 64-bit BARs could be moved
there?
> Could we give pci_resize_resource() a mask of BARs which are save to
> release?
It is possible.
> Or maybe a flag to indicate that it can only free up 64bit BARs?
>
> Regards,
> Christian.
>
> >
> > Thanks a lot for checking this out!
> >
>
--
i.
