Re: [PATCH 13/23] PCI: Add pbus_mem_size_optional() to handle optional sizes

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 18583 bytes --]

On Thu, 9 Jul 2026, Eric Auger wrote:
> On 7/9/26 11:04 AM, Ilpo Järvinen wrote:
> > On Wed, 8 Jul 2026, Eric Auger wrote:
> >> On 7/8/26 4:05 PM, Ilpo Järvinen wrote:
> >>> On Wed, 8 Jul 2026, Eric Auger wrote:
> >>>> On 7/7/26 6:12 PM, Ilpo Järvinen wrote:
> >>>>> On Tue, 7 Jul 2026, Eric Auger wrote:
> >>>>>
> >>>>>> Hi Ilpo,
> >>>>>>
> >>>>>> On 12/19/25 6:40 PM, Ilpo Järvinen wrote:
> >>>>>>> The resource loop in pbus_size_mem() handles optional resources that
> >>>>>>> are either fully optional (SRIOV and disabled Expansion ROMs) or bridge
> >>>>>>> windows that may be optional only for a part. The logic is little
> >>>>>>> inconsistent when it comes to a bridge window that has only optional
> >>>>>>> children resources as it would be more natural to treat it similar to
> >>>>>>> any fully optional resource. As resource size should be zero in that
> >>>>>>> case, it shouldn't cause any bugs but it still seems useful to address
> >>>>>>> the inconsistency.
> >>>>>>>
> >>>>>>> Place the optional size related code of pbus_size_mem() into
> >>>>>>> pbus_mem_size_optional() and add check into pci_resource_is_optional()
> >>>>>>> for entirely optional bridge windows. Reorder the logic inside
> >>>>>>> pbus_mem_size_optional() such that fully optional resources are handled
> >>>>>>> the same irrespective to whether the resource is a bridge window or
> >>>>>>> not.
> >>>>>>
> >>>>>> This patch seems to introduce a regression when trying to hotplug a
> >>>>>> virtio-net-pci device behind a XIO3130 downstream port in the following
> >>>>>> hierarchy.
> >>>>>>
> >>>>>> this happens with a v7.2-rc2 guest kernel, on an arm64 guest with qemu vmm.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> 00:01.0 Host bridge: Red Hat, Inc. QEMU PCIe Expander bridge
> >>>>>> 	Subsystem: Red Hat, Inc. Device 1100
> >>>>>>
> >>>>>> |_ 0a:00.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
> >>>>>>    Subsystem: Red Hat, Inc. Device 0000
> >>>>>>    Kernel driver in use: pcieport
> >>>>>>
> >>>>>>    |_ 0b:00.0 PCI bridge: Texas Instruments XIO3130 PCI Express Switch
> >>>>>>
> >>>>>> 	(Upstream) (rev 02)
> >>>>>> 	Kernel driver in use: pcieport
> >>>>>>
> >>>>>> 	|_ 0c:02.0 PCI bridge: Texas Instruments XIO3130 PCI Express
> >>>>>> 		Switch (Downstream) (rev 01)
> >>>>>> 		Kernel driver in use: pcieport
> >>>>>
> >>>>> Hi Eric,
> >>>>>
> >>>>> Thanks for the report.
> >>>>>
> >>>>> I cannot get much done with the log snippets. The first line with 
> >>>>> difference is a symptom of something that originates from outside of the 
> >>>>> snippet.
> >>>>>
> >>>>> Could you please take a log with dyndbg="file drivers/pci/*.c +p" on the 
> >>>>> kernel command line and also include a /proc/iomem dump. Like 
> >>>>> you did now, preferrably take those from both working and failing case so 
> >>>>> I can easily diff them.
> >>>>>
> >>>>> --
> >>>>>  i.
> >>>>>
> >>>>>
> >>>>>> This produces the following trace
> >>>>>>
> >>>>>>
> >>>>>> [   28.557947] pcieport 0000:0c:02.0: pciehp: Slot(0-1): Card present
> >>>>>> [   28.557949] pcieport 0000:0c:02.0: pciehp: Slot(0-1): Link Up
> >>>>>> [   29.605376] pci 0000:0d:00.0: [1af4:1041] type 00 class 0x020000 PCIe
> >>>>>> Endpoint
> >>>>>> [   29.605724] pci 0000:0d:00.0: BAR 1 [mem 0x00000000-0x00000fff]
> >>>>>> [   29.605765] pci 0000:0d:00.0: BAR 4 [mem 0x00000000-0x00003fff 64bit
> >>>>>> pref]
> >>>>>> [   29.605816] pci 0000:0d:00.0: enabling Extended Tags
> >>>>>> [   29.606994] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   29.606998] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   29.607003] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: can't assign; no space
> >>>>>> [   29.607005] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: failed to assign
> >>>>>> [   29.607007] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: can't assign; no space
> >>>>>> [   29.607008] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: failed to assign
> >>>>>> [   29.607010] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   29.607012] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   29.607014] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000]: can't assign; no space
> >>>>>> [   29.607016] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000]: failed to assign
> >>>>>> [   29.607018] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref]: can't assign; no space
> >>>>>> [   29.607019] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref]: failed to assign
> >>>>>> [   29.607021] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   29.607022] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   29.607025] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> can't assign; no space
> >>>>>> [   29.607026] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> failed to assign
> >>>>>> [   29.607027] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: can't
> >>>>>> assign; no space
> >>>>>> [   29.607028] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: failed to
> >>>>>> assign
> >>>>>> [   29.607029] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> can't assign; no space
> >>>>>> [   29.607030] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> failed to assign
> >>>>>> [   29.607031] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: can't
> >>>>>> assign; no space
> >>>>>> [   29.607032] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: failed to
> >>>>>> assign
> >>>>>> [   29.607033] pcieport 0000:0c:02.0: PCI bridge to [bus 0d]
> >>>>>> [   29.611260] PCI: No. 2 try to assign unassigned res
> >>>>>> [   29.611269] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   29.611272] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   29.611276] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: can't assign; no space
> >>>>>> [   29.611277] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: failed to assign
> >>>>>> [   29.611278] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: can't assign; no space
> >>>>>> [   29.611279] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: failed to assign
> >>>>>> [   29.611280] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   29.611280] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   29.611281] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000]: can't assign; no space
> >>>>>> [   29.611282] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000]: failed to assign
> >>>>>> [   29.611283] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref]: can't assign; no space
> >>>>>> [   29.611283] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref]: failed to assign
> >>>>>> [   29.611284] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   29.611284] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   29.611286] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> can't assign; no space
> >>>>>> [   29.611286] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> failed to assign
> >>>>>> [   29.611287] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: can't
> >>>>>> assign; no space
> >>>>>> [   29.611288] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: failed to
> >>>>>> assign
> >>>>>> [   29.611288] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> can't assign; no space
> >>>>>> [   29.611289] pci 0000:0d:00.0: BAR 4 [mem size 0x00004000 64bit pref]:
> >>>>>> failed to assign
> >>>>>> [   29.611289] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: can't
> >>>>>> assign; no space
> >>>>>> [   29.611290] pci 0000:0d:00.0: BAR 1 [mem size 0x00001000]: failed to
> >>>>>> assign
> >>>>>> [   29.611291] pcieport 0000:0c:02.0: PCI bridge to [bus 0d]
> >>>>>> [   29.616141] ACPI: \_SB_.L0A2: Enabled at IRQ 37
> >>>>>> [   29.616378] virtio-pci 0000:0d:00.0: virtio_pci: leaving for legacy
> >>>>>> driver
> >>>>>>
> >>>>>> Previous to this commit hotplug was successful:
> >>>>>>
> >>>>>> vm-rhel10 login: [   38.385692] pcieport 0000:0c:02.0: pciehp:
> >>>>>> Slot(0-1): Button press: will power on in 5 sec
> >>>>>> [   38.385798] pcieport 0000:0c:02.0: pciehp: Slot(0-1): Card present
> >>>>>> [   38.385799] pcieport 0000:0c:02.0: pciehp: Slot(0-1): Link Up
> >>>>>> [   39.553709] pci 0000:0d:00.0: [1af4:1041] type 00 class 0x020000 PCIe
> >>>>>> Endpoint
> >>>>>> [   39.554018] pci 0000:0d:00.0: BAR 1 [mem 0x00000000-0x00000fff]
> >>>>>> [   39.554042] pci 0000:0d:00.0: BAR 4 [mem 0x00000000-0x00003fff 64bit
> >>>>>> pref]
> >>>>>> [   39.554099] pci 0000:0d:00.0: enabling Extended Tags
> >>>>>> [   39.555202] pcieport 0000:0c:02.0: bridge window [mem size 0x00100000
> >>>>>> 64bit pref] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   39.555206] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00100000] to [bus 0d] add_size 100000 add_align 100000
> >>>>>> [   39.555212] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: can't assign; no space
> >>>>>> [   39.555213] pcieport 0000:0c:02.0: bridge window [mem size
> >>>>>> 0x00200000]: failed to assign
> >>>>>> [   39.555215] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: can't assign; no space
> >>>>>> [   39.555216] pcieport 0000:0c:02.0: bridge window [mem size 0x00200000
> >>>>>> 64bit pref]: failed to assign
> >>>>>> [   39.555218] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   39.555219] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   39.555222] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10a00000-0x10afffff]: assigned
> >>>>>> [   39.555224] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10b00000-0x10bfffff 64bit pref]: assigned
> >>>>>> [   39.555225] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   39.555227] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   39.555228] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10a00000-0x10afffff]: failed to expand by 0x100000
> >>>>>> [   39.555230] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10a00000-0x10afffff]: failed to add optional 100000
> >>>>>> [   39.555232] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10b00000-0x10bfffff 64bit pref]: failed to expand by 0x100000
> >>>>>> [   39.555234] pcieport 0000:0c:02.0: bridge window [mem
> >>>>>> 0x10b00000-0x10bfffff 64bit pref]: failed to add optional 100000
> >>>>>> [   39.555236] pci 0000:0d:00.0: BAR 4 [mem 0x10b00000-0x10b03fff 64bit
> >>>>>> pref]: assigned
> >>>>>> [   39.555290] pci 0000:0d:00.0: BAR 1 [mem 0x10a00000-0x10a00fff]: assigned
> >>>>>> [   39.555305] pcieport 0000:0c:02.0: PCI bridge to [bus 0d]
> >>>>>> [   39.556804] pcieport 0000:0c:02.0:   bridge window [mem
> >>>>>> 0x10a00000-0x10afffff]
> >>>>>> [   39.557730] pcieport 0000:0c:02.0:   bridge window [mem
> >>>>>> 0x10b00000-0x10bfffff 64bit pref]
> >>>>>> [   39.559629] PCI: No. 2 try to assign unassigned res
> >>>>>> [   39.559636] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   39.559638] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   39.559640] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> can't assign; no space
> >>>>>> [   39.559642] pcieport 0000:0c:02.0: bridge window [io  size 0x1000]:
> >>>>>> failed to assign
> >>>>>> [   39.559644] pcieport 0000:0c:02.0: PCI bridge to [bus 0d]
> >>>>>> [   39.561078] pcieport 0000:0c:02.0:   bridge window [mem
> >>>>>> 0x10a00000-0x10afffff]
> >>>>>> [   39.562011] pcieport 0000:0c:02.0:   bridge window [mem
> >>>>>> 0x10b00000-0x10bfffff 64bit pref]
> >>>>>> [   39.564500] ACPI: \_SB_.L0A2: Enabled at IRQ 37
> >>>>>> [   39.564541] virtio-pci 0000:0d:00.0: enabling device (0000 -> 0002)
> >>>>>> [   39.572231] virtio_net virtio2 enp13s0: renamed from eth0
> >>>>>>
> >>>>>> Any clue?
> >>>>>>
> >>>>>> Thank you in advance
> >>>>>>
> >>>>>> Eric
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Additional motivation for this are the upcoming changes that add
> >>>>>>> complexity to the optional sizing logic due to Resizable BAR awareness.
> >>>>>>> The extra logic would exceed any reasonable indentation level if the
> >>>>>>> optional sizing code is kept within the loop body.
> >>>>>>>
> >>>>>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >>>>>>> ---
> >>>>>>>  drivers/pci/setup-bus.c | 77 +++++++++++++++++++++++++++++------------
> >>>>>>>  1 file changed, 54 insertions(+), 23 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> >>>>>>> index 3d1d3cefcdba..3fcc7641c374 100644
> >>>>>>> --- a/drivers/pci/setup-bus.c
> >>>>>>> +++ b/drivers/pci/setup-bus.c
> >>>>>>> @@ -125,15 +125,6 @@ static resource_size_t get_res_add_size(struct list_head *head,
> >>>>>>>  	return dev_res ? dev_res->add_size : 0;
> >>>>>>>  }
> >>>>>>>  
> >>>>>>> -static resource_size_t get_res_add_align(struct list_head *head,
> >>>>>>> -					 struct resource *res)
> >>>>>>> -{
> >>>>>>> -	struct pci_dev_resource *dev_res;
> >>>>>>> -
> >>>>>>> -	dev_res = res_to_dev_res(head, res);
> >>>>>>> -	return dev_res ? dev_res->min_align : 0;
> >>>>>>> -}
> >>>>>>> -
> >>>>>>>  static void restore_dev_resource(struct pci_dev_resource *dev_res)
> >>>>>>>  {
> >>>>>>>  	struct resource *res = dev_res->res;
> >>>>>>> @@ -386,6 +377,8 @@ bool pci_resource_is_optional(const struct pci_dev *dev, int resno)
> >>>>>>>  		return true;
> >>>>>>>  	if (resno == PCI_ROM_RESOURCE && !(res->flags & IORESOURCE_ROM_ENABLE))
> >>>>>>>  		return true;
> >>>>>>> +	if (pci_resource_is_bridge_win(resno) && !resource_size(res))
> >>>> adding
> >>>>
> >>>> && !dev->is_hotplug_bridge makes the hotplug successful.
> >>>>
> >>>>
> >>>> My understanding is that otherwise the hotplug bridge window is
> >>>> considered as optional and no memoy window is allocated to assign chid
> >>>> device's BARS
> >>>>
> >>>> Please let me know if that makes sense. In the positive I can send a patch.
> >>>
> >>> I don't think that is the correct solution.
> >>>
> >>>
> >>> These bridges, have both non-prefetchable and prefetchable windows, but 
> >>> the initial setup only sets the non-prefetchable one to 2M (and nothing 
> >>> more would fit to the associated root bus resource). There's only a single 
> >>> root bus resource that is 2M + a bit more (to fit that BAR0).
> >>>
> >>> Now, when sizing the bridge windows, the kernel sizing algorithm just uses 
> >>> the default 2M hotplug reservation to both of those windows as there are 
> >>> not yet any real devices underneath (the default hp reservation is 
> >>> controllable with hpmmiosize/hpmmioprefsize/hpmemsize cmdline parameters). 
> >>> Thus, to fit everything, the root bus resource should be 4M + a bit more 
> >>> for BAR0.
> >>>
> >>> However, it looks to me there's also a 4M add_size miscalculation (the 
> >>> downstream bridge window is only 2M per each type):
> >>>
> >>> [    0.048078] pci 0000:0a:00.0: bridge window [mem 0x00100000-0x000fffff 64bit pref] to [bus 0b-0d] add_size 400000 add_align 100000
> >>> [    0.048079] pci 0000:0a:00.0: bridge window [mem 0x00100000-0x000fffff] to [bus 0b-0d] add_size 400000 add_align 100000
> >>
> >> Hum even with the patch below I still get:
> >>
> >> [    0.047253] pci 0000:0a:00.0: bridge window [mem
> >> 0x00100000-0x000fffff 64bit pref] to [bus 0b-0d] add_size 400000
> >> add_align 100000
> >> [    0.047254] pci 0000:0a:00.0: bridge window [mem
> >> 0x00100000-0x000fffff] to [bus 0b-0d] add_size 400000 add_align 100000
> 
> If I comment the continue in the block hereafter in pbus_size_mem() it
> works.

Many changes may appear working, but that doesn't necessarily make them 
correct. The behavior with the older "working" kernel included that pulled 
mandatory 1M sizes out of thin air and just happened to succeed with those 
sizes to let your hotplug scenario to "work". It's still just numbers 
pulled out of thin air (not derived from downstream resource sizes).

> if (pbus_size_mem_optional(dev, i, align,
>                            realloc_head, &add_align,
>                            &children_add_size)) {
>                                 //continue;
> }
> 
> Given r_size == 0, this allows to set aligns[order] += align;
> 
> isn't it requested for the parent bridges (the Root Port and Switch
> Upstream port) to see the downstream alignment requirements and map them
> out on boot?

It needs to be flexible. When not everything fits some things are more 
important than others (aka. optional). On of such things is hotplug 
reservation. If you'd change that behavior of the algorithm, be my guest 
to fix all the brokenness that will ensue. ;-)

This here is the sizing algorithm. The actual assignments are done in a 
different phase, and they try first with optional parts included but if 
that fails, the algorithm falls back to first allocation all mandatory 
resources, and then as many optional ones as it can.

(It seems I need to figure out why my debug patch does appear to be doing 
anything tomorrow.)

-- 
 i.