Linux PCI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: linux-pci@vger.kernel.org
Subject: Re: [PATCH 1/1] PCI: Do not add hotplug reservation multiple times
Date: Mon, 20 Jul 2026 16:40:06 +0000	[thread overview]
Message-ID: <20260720164006.8E86C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260720162416.5771-1-ilpo.jarvinen@linux.intel.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fixes nested hotplug reservation accumulation for memory windows in `calculate_memsize()`, but misses the identical buggy logic in `calculate_iosize()`, leading to I/O space exhaustion.
--

--- Patch [1]: [PATCH 1/1] PCI: Do not add hotplug reservation multiple times ---
commit 3c4b4a62adcb92793a7c17214021e8b637be1016
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Do not add hotplug reservation multiple times

In nested topologies, hotplug reservations gets added multiple times
into the bridge window higher up in the hierarchy. Adding reservation
for intermediate level bridges does not seem very useful because the
hotplug is going to add device at a leaf.

Link: https://lore.kernel.org/linux-pci/f23946f2-06ac-4607-8f2c-3ffbc52b627a@redhat.com/

> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index c0a949f2c9956..e8c94aa1d3c12 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1021,7 +1021,7 @@ static resource_size_t calculate_memsize(resource_size_t size,
>  					 resource_size_t children_add_size,
>  					 resource_size_t align)
>  {
> -	size = max(size, min_size) + children_add_size;
> +	size = max(size + children_add_size, min_size);
>  	return ALIGN(size, align);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does calculate_iosize() need a
similar fix to avoid I/O space exhaustion in nested topologies?

Just above calculate_memsize(), calculate_iosize() retains the identical
logic for I/O windows:

drivers/pci/setup-bus.c:calculate_iosize() {
    ...
    size = max(size, add_size) + children_add_size;
    return ALIGN(max(size, old_size), align);
}

When allocating I/O windows for nested hotplug bridges, pbus_size_io() passes
both the accumulated children I/O sizes (children_add_size) and the hotplug
reservation (add_size) to calculate_iosize().

Because the logic is exactly the same as the unpatched calculate_memsize(),
this appears to still unconditionally add an extra 4KB (due to bridge window
alignment) at each nested bridge level. Can this linearly inflate the required
space and quickly exhaust the limited 64KB I/O address space on systems with
deeply nested hotplug PCIe bridges (like Thunderbolt docks)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720162416.5771-1-ilpo.jarvinen@linux.intel.com?part=1

  reply	other threads:[~2026-07-20 16:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 16:24 [PATCH 1/1] PCI: Do not add hotplug reservation multiple times Ilpo Järvinen
2026-07-20 16:40 ` sashiko-bot [this message]
2026-07-21  7:43   ` Ilpo Järvinen

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=20260720164006.8E86C1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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