From: sashiko-bot@kernel.org
To: "Alex Elder" <elder@riscstar.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org,
linux-pci@vger.kernel.org
Subject: Re: [PATCH 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags()
Date: Fri, 07 Aug 2026 19:56:03 +0000 [thread overview]
Message-ID: <20260807195604.277C31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260807194100.455599-3-elder@riscstar.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The function `of_pci_is_range_resource()` incorrectly evaluates `IORESOURCE_MEM_64` using the `resource_type()` macro. The `resource_type(res)` macro applies the `IORESOURCE_TYPE_BITS` mask (`0x1f00`). Because `IORESOURCE_MEM_64` is defined as `0x00100000`, it falls completely outside this mask. Consequently, `resource_type(res) & IORESOURCE_MEM_64` evaluates to a constant 0.
As a result, the condition `!(resource_type(res) & IORESOURCE_MEM_64)` is unconditionally `true`. This simplifies the entire filter condition to `!(resource_type(res) & IORESOURCE_MEM)`. Any resource that does not have the `IORESOURCE_MEM` bit set—such as `IORESOURCE_IO` (`0x100`)—will cause the condition to evaluate to true, making the function return `false`.
This logic flaw inadvertently filters out `IORESOURCE_IO` resources entirely, which prevents PCI host bridges from exposing IO space mappings via the dynamically generated device tree `ranges` property. Furthermore, if a 64-bit memory resource were defined with `IORESOURCE_MEM_64` but without `IORESOURCE_MEM`, it would also be incorrectly rejected.
--
--- Patch [2]: [PATCH 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags() ---
commit d6dba8ef5457a1efb23f2b68f270883ef856bbdb
Author: Alex Elder <elder@riscstar.com>
PCI: of: don't zero flags in of_pci_get_addr_flags()
The flags variable whose address is passed to of_pci_get_addr_flags()
is zeroed before assigning a value to it. Skip the zeroing and just
assign it instead.
> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
> index 505226b876c56..22fea1905a080 100644
> --- a/drivers/pci/of_property.c
> +++ b/drivers/pci/of_property.c
[ ... ]
> static bool of_pci_is_range_resource(const struct resource *res, u32 *flags)
> {
[ ... ]
> if (!(resource_type(res) & IORESOURCE_MEM) &&
> !(resource_type(res) & IORESOURCE_MEM_64))
> return false;
[Severity: High]
This is a pre-existing issue, but does this correctly evaluate
IORESOURCE_MEM_64?
The resource_type() macro applies the IORESOURCE_TYPE_BITS mask (0x1f00).
Because IORESOURCE_MEM_64 is defined as 0x00100000, it falls outside this
mask, causing resource_type(res) & IORESOURCE_MEM_64 to evaluate to a
constant 0.
This makes the condition !(resource_type(res) & IORESOURCE_MEM_64)
unconditionally true. This simplifies the logic to reject any resource
that does not have the IORESOURCE_MEM bit set, such as IORESOURCE_IO.
Could this inadvertently filter out IORESOURCE_IO resources entirely when
called from of_pci_host_bridge_prop_ranges()?
drivers/pci/of_property.c:of_pci_host_bridge_prop_ranges() {
...
resource_list_for_each_entry(window, &bridge->windows) {
res = window->res;
if (!of_pci_is_range_resource(res, &flags))
continue;
...
}
If IO windows are wrongly skipped here, will dynamically created PCI host
bridge device tree nodes lack IO mappings in their ranges property?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807194100.455599-1-elder@riscstar.com?part=2
next prev parent reply other threads:[~2026-08-07 19:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-07 19:40 [PATCH 0/4] PCI: of: warn on bogus device_type property Alex Elder
2026-08-07 19:40 ` [PATCH 1/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
2026-08-07 19:54 ` sashiko-bot
2026-08-07 20:58 ` Alex Elder
2026-08-07 19:40 ` [PATCH 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags() Alex Elder
2026-08-07 19:56 ` sashiko-bot [this message]
2026-08-07 20:58 ` Alex Elder
2026-08-07 19:40 ` [PATCH 3/4] PCI: of: make a flags argument optional Alex Elder
2026-08-07 19:51 ` sashiko-bot
2026-08-07 19:40 ` [PATCH 4/4] PCI: of: introduce of_pci_verify_node() Alex Elder
2026-08-07 19:49 ` sashiko-bot
2026-08-07 20:58 ` Alex Elder
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=20260807195604.277C31F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=elder@riscstar.com \
--cc=linux-pci@vger.kernel.org \
--cc=robh@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.