Re: [PATCH v2 3/7] PCI: Honor alignment overrides

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: pci_init_pci_resreq_data() confuses default alignment with user override
--

commit 9d8429783cc0cbfe47ed3509438a7591dc940ce5
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Honor alignment overrides

This commit introduces struct pci_resreq_data to hold device and user-given
alignment information so that pci_align_resource() can honor the
pci=resource_alignment argument. When a user-given alignment is found, any
remainder movement is skipped to avoid violating the requested alignment.

Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/

[ ... ]

> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 376f09630a4ab..441a62807719d 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -265,17 +265,21 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
>   * before res->start if there's enough free space there. This enables
>   * tighter packing for resources.
>   */
> -resource_size_t pci_align_resource(struct pci_dev *dev,
> +resource_size_t pci_align_resource(struct pci_resreq_data *rr,
>  				   const struct resource *res,
>  				   const struct resource *empty_res,
>  				   resource_size_t size,
>  				   resource_size_t align)
>  {
> +	struct pci_dev *dev = rr->dev;
>  	resource_size_t remainder, start_addr;
>  
>  	if (!(res->flags & IORESOURCE_MEM))
>  		return res->start;
>  
> +	if (rr->user_align)
> +		return res->start;
> +
[Severity: Medium]
Does this inadvertently skip remainder movement for all devices on platforms
with non-zero default alignments?

On platforms where pcibios_default_alignment() returns a non-zero value
(such as PowerPC), rr->user_align might be populated with that default
rather than specifically a user-requested override. If this condition
evaluates as true for those defaults, it appears it would entirely disable
the remainder movement logic and limit PCI resource packing efficiency across
the platform.

Note that follow-up modifications to this condition (such as checking if the
index is 1 or 3) would still incorrectly trigger based on a default alignment
if the distinction between user overrides and defaults is lost.

[ ... ]

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..fe0233f0d9e6e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1217,12 +1217,34 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
>  /* Architecture-specific versions may override this (weak) */
>  char *pcibios_setup(char *str);
>  
> -/* Used only when drivers/pci/setup.c is used */
> +/* Resource assignment request handling */
> +/**
> + * struct pci_resreq_data - Data passed to pcibios_align_resource()
> + * @dev: PCI device
> + * @user_align: User provided alignment override (pci=resource_alignment param)
> + */
> +struct pci_resreq_data {
> +	struct pci_dev *dev;
> +	resource_size_t user_align;
> +};
> +
> +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> +						 bool *resize);
> +static inline void pci_init_pci_resreq_data(struct pci_resreq_data *rr,
> +					    struct pci_dev *dev)
> +{
> +	bool user_aligned;
> +
> +	memset(rr, 0, sizeof(*rr));
> +	rr->dev = dev;
> +	rr->user_align = pci_specified_resource_alignment(dev, &user_aligned);
> +}
> +
[Severity: Medium]
Does this initialize rr->user_align with the returned alignment size while
ignoring the boolean flag that indicates if it was actually requested by
the user?

Looking at pci_specified_resource_alignment() in drivers/pci/pci.c, it
falls back to pcibios_default_alignment() if no user override matches. The
user_aligned flag is set to true only when a specific override is found.
Because the return value is assigned directly to rr->user_align, it seems
struct pci_resreq_data cannot differentiate between a default alignment and an
actual user override.

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