* [PATCH v2 3/7] PCI: Honor alignment overrides [not found] <20261002113319.6652-1-ilpo.jarvinen@linux.intel.com> @ 2026-10-02 11:33 ` Ilpo Järvinen 2026-10-02 11:46 ` Jani Nikula 2026-10-02 11:49 ` sashiko-bot 0 siblings, 2 replies; 3+ messages in thread From: Ilpo Järvinen @ 2026-10-02 11:33 UTC (permalink / raw) To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless, Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Richard Henderson, Matt Turner, Magnus Lindholm, Russell King, Geert Uytterhoeven, Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller, Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP), Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Chris Zankel, Max Filippov, David Airlie, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Simona Vetter, Ilpo Järvinen, linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-sh, dri-devel, intel-gfx Cc: Bradley Morgan, Sashiko pci=resource_alignment argument can override the default alignment for the resource. The remainder code introduced in the commit 9036bd0efcb6 ("PCI: Align head space better") can move remainder space (non-aligning part of the size) before the aligning left edge which results in violating the requested alignment. Introduce struct pci_resreq_data to hold device and user-given alignment to be able to honor it in pci_align_resource(). When user-given alignment is found, any remainder movement is skipped. Fixes: 9036bd0efcb6 ("PCI: Align head space better") Reported-by: Sashiko <sashiko-bot@kernel.org> Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/ Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- arch/alpha/kernel/pci.c | 3 ++- arch/arm/kernel/bios32.c | 5 +++-- arch/m68k/kernel/pcibios.c | 4 ++-- arch/mips/pci/pci-generic.c | 5 +++-- arch/mips/pci/pci-legacy.c | 5 +++-- arch/parisc/kernel/pci.c | 5 +++-- arch/powerpc/kernel/pci-common.c | 5 +++-- arch/sh/drivers/pci/pci.c | 5 +++-- arch/x86/pci/i386.c | 5 +++-- arch/xtensa/kernel/pci.c | 5 +++-- drivers/char/agp/intel-gtt.c | 5 ++++- drivers/gpu/drm/i915/i915_gmch.c | 5 +++-- drivers/pci/pci.c | 5 +++-- drivers/pci/setup-res.c | 19 +++++++++++++------ include/linux/pci.h | 26 ++++++++++++++++++++++++-- 15 files changed, 75 insertions(+), 32 deletions(-) diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c index 11df411b1d18..c5d725a9781f 100644 --- a/arch/alpha/kernel/pci.c +++ b/arch/alpha/kernel/pci.c @@ -128,7 +128,8 @@ pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; struct pci_controller *hose = dev->sysdata; unsigned long alignto; resource_size_t start = res->start; diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index ac0e890510da..3e61826f07fb 100644 --- a/arch/arm/kernel/bios32.c +++ b/arch/arm/kernel/bios32.c @@ -564,7 +564,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t start = res->start; struct pci_host_bridge *host_bridge; @@ -578,7 +579,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, start, size, align); if (res->flags & IORESOURCE_MEM) - return pci_align_resource(dev, res, empty_res, size, align); + return pci_align_resource(rr, res, empty_res, size, align); return start; } diff --git a/arch/m68k/kernel/pcibios.c b/arch/m68k/kernel/pcibios.c index 7a9e60df79c5..3024408412bc 100644 --- a/arch/m68k/kernel/pcibios.c +++ b/arch/m68k/kernel/pcibios.c @@ -31,14 +31,14 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; resource_size_t start = res->start; if ((res->flags & IORESOURCE_IO) && (start & 0x300)) start = (start + 0x3ff) & ~0x3ff; if (res->flags & IORESOURCE_MEM) - return pci_align_resource(dev, res, empty_res, size, align); + return pci_align_resource(rr, res, empty_res, size, align); return start; } diff --git a/arch/mips/pci/pci-generic.c b/arch/mips/pci/pci-generic.c index c2e23d0c1d77..7f3ecb91b85b 100644 --- a/arch/mips/pci/pci-generic.c +++ b/arch/mips/pci/pci-generic.c @@ -25,7 +25,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t start = res->start; struct pci_host_bridge *host_bridge; @@ -39,7 +40,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, start, size, align); if (res->flags & IORESOURCE_MEM) - return pci_align_resource(dev, res, empty_res, size, align); + return pci_align_resource(rr, res, empty_res, size, align); return start; } diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c index dae6dafdd6e0..82d4b01db64e 100644 --- a/arch/mips/pci/pci-legacy.c +++ b/arch/mips/pci/pci-legacy.c @@ -55,7 +55,8 @@ pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; struct pci_controller *hose = dev->sysdata; resource_size_t start = res->start; @@ -70,7 +71,7 @@ pcibios_align_resource(void *data, const struct resource *res, if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; } else if (res->flags & IORESOURCE_MEM) { - start = pci_align_resource(dev, res, empty_res, size, align); + start = pci_align_resource(rr, res, empty_res, size, align); /* Make sure we start at our min on all hoses */ if (start < PCIBIOS_MIN_MEM + hose->mem_resource->start) diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c index 518f532ccddc..d582be996051 100644 --- a/arch/parisc/kernel/pci.c +++ b/arch/parisc/kernel/pci.c @@ -193,7 +193,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t alignment) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t align, start = res->start; pci_dbg(dev, "%pR: pcibios_align_resource(%lx, 0x%llx, 0x%llx), parent %pR\n", @@ -205,7 +206,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, if (align > alignment) start = ALIGN(start, align); else - start = pci_align_resource(dev, res, empty_res, size, alignment); + start = pci_align_resource(rr, res, empty_res, size, alignment); return start; } diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 4fc52c21fe5d..23594759cfe2 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1131,7 +1131,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t start = res->start; if (res->flags & IORESOURCE_IO) { @@ -1140,7 +1141,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; } else if (res->flags & IORESOURCE_MEM) { - start = pci_align_resource(dev, res, empty_res, size, align); + start = pci_align_resource(rr, res, empty_res, size, align); } return start; diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c index 878a27a1acfb..279f2770ff4b 100644 --- a/arch/sh/drivers/pci/pci.c +++ b/arch/sh/drivers/pci/pci.c @@ -172,7 +172,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; struct pci_channel *hose = dev->sysdata; resource_size_t start = res->start; @@ -186,7 +187,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; } else if (res->flags & IORESOURCE_MEM) { - start = pci_align_resource(dev, res, empty_res, size, align); + start = pci_align_resource(rr, res, empty_res, size, align); } return start; diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c index e2de26b82940..4178ae4380c2 100644 --- a/arch/x86/pci/i386.c +++ b/arch/x86/pci/i386.c @@ -156,7 +156,8 @@ pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t start = res->start; if (res->flags & IORESOURCE_IO) { @@ -165,7 +166,7 @@ pcibios_align_resource(void *data, const struct resource *res, if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; } else if (res->flags & IORESOURCE_MEM) { - start = pci_align_resource(dev, res, empty_res, size, align); + start = pci_align_resource(rr, res, empty_res, size, align); /* The low 1MB range is reserved for ISA cards */ if (start < BIOS_END) diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c index 305031551136..4e8ad4a3c9fe 100644 --- a/arch/xtensa/kernel/pci.c +++ b/arch/xtensa/kernel/pci.c @@ -42,7 +42,8 @@ pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; + struct pci_dev *dev = rr->dev; resource_size_t start = res->start; if (res->flags & IORESOURCE_IO) { @@ -55,7 +56,7 @@ pcibios_align_resource(void *data, const struct resource *res, if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; } else if (res->flags & IORESOURCE_MEM) { - start = pci_align_resource(dev, res, empty_res, size, align); + start = pci_align_resource(rr, res, empty_res, size, align); } return start; diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index bcc26785175d..2214347906a6 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -1038,10 +1038,13 @@ static struct agp_memory *intel_fake_agp_alloc_by_type(size_t pg_count, static int intel_alloc_chipset_flush_resource(void) { + struct pci_resreq_data rr; int ret; + + pci_init_pci_resreq_data(&rr, intel_private.bridge_dev); ret = pci_bus_alloc_resource(intel_private.bridge_dev->bus, &intel_private.ifp_resource, PAGE_SIZE, PAGE_SIZE, PCIBIOS_MIN_MEM, 0, - pcibios_align_resource, intel_private.bridge_dev); + pcibios_align_resource, &rr); return ret; } diff --git a/drivers/gpu/drm/i915/i915_gmch.c b/drivers/gpu/drm/i915/i915_gmch.c index b0ef6ef577a3..d35097bebc0f 100644 --- a/drivers/gpu/drm/i915/i915_gmch.c +++ b/drivers/gpu/drm/i915/i915_gmch.c @@ -38,6 +38,7 @@ static int mchbar_reg(struct drm_i915_private *i915) static int intel_alloc_mchbar_resource(struct drm_i915_private *i915) { + struct pci_resreq_data rr; u32 temp_lo, temp_hi = 0; u64 mchbar_addr; int ret; @@ -55,12 +56,12 @@ intel_alloc_mchbar_resource(struct drm_i915_private *i915) /* Get some space for it */ i915->gmch.mch_res.name = "i915 MCHBAR"; i915->gmch.mch_res.flags = IORESOURCE_MEM; + pci_init_pci_resreq_data(&rr, i915->gmch.pdev); ret = pci_bus_alloc_resource(i915->gmch.pdev->bus, &i915->gmch.mch_res, MCHBAR_SIZE, MCHBAR_SIZE, PCIBIOS_MIN_MEM, - 0, pcibios_align_resource, - i915->gmch.pdev); + 0, pcibios_align_resource, &rr); if (ret) { drm_dbg(&i915->drm, "failed bus alloc: %d\n", ret); i915->gmch.mch_res.start = 0; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b2879a6be5f8..1b7a4469c6ca 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6444,8 +6444,8 @@ static DEFINE_SPINLOCK(resource_alignment_lock); * RETURNS: Resource alignment if it is specified. * Zero if it is not specified. */ -static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, - bool *resize) +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, + bool *resize) { int align_order, count; resource_size_t align = pcibios_default_alignment(); @@ -6497,6 +6497,7 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, spin_unlock(&resource_alignment_lock); return align; } +EXPORT_SYMBOL_GPL(pci_specified_resource_alignment); static void pci_request_resource_alignment(struct pci_dev *dev, int bar, resource_size_t align, bool resize) diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c index 376f09630a4a..441a62807719 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; + if (IS_ALIGNED(size, align)) return res->start; @@ -306,18 +310,21 @@ resource_size_t __weak pcibios_align_resource(void *data, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; + struct pci_resreq_data *rr = data; - return pci_align_resource(dev, res, empty_res, size, align); + return pci_align_resource(rr, res, empty_res, size, align); } static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, int resno, resource_size_t size, resource_size_t align) { struct resource *res = pci_resource_n(dev, resno); + struct pci_resreq_data rr; resource_size_t min; int ret; + pci_init_pci_resreq_data(&rr, dev); + min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM; /* @@ -329,7 +336,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, */ ret = pci_bus_alloc_resource(bus, res, size, align, min, IORESOURCE_PREFETCH | IORESOURCE_MEM_64, - pcibios_align_resource, dev); + pcibios_align_resource, &rr); if (ret == 0) return 0; @@ -341,7 +348,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) { ret = pci_bus_alloc_resource(bus, res, size, align, min, IORESOURCE_PREFETCH, - pcibios_align_resource, dev); + pcibios_align_resource, &rr); if (ret == 0) return 0; } @@ -354,7 +361,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, */ if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) ret = pci_bus_alloc_resource(bus, res, size, align, min, 0, - pcibios_align_resource, dev); + pcibios_align_resource, &rr); return ret; } diff --git a/include/linux/pci.h b/include/linux/pci.h index d31a8d107b1e..fe0233f0d9e6 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); +} + resource_size_t pcibios_align_resource(void *data, const struct resource *res, const struct resource *empty_res, resource_size_t size, resource_size_t align); -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, -- 2.47.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 3/7] PCI: Honor alignment overrides 2026-10-02 11:33 ` [PATCH v2 3/7] PCI: Honor alignment overrides Ilpo Järvinen @ 2026-10-02 11:46 ` Jani Nikula 2026-10-02 11:49 ` sashiko-bot 1 sibling, 0 replies; 3+ messages in thread From: Jani Nikula @ 2026-10-02 11:46 UTC (permalink / raw) To: Ilpo Järvinen, Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless, Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Richard Henderson, Matt Turner, Magnus Lindholm, Russell King, Geert Uytterhoeven, Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller, Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP), Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Chris Zankel, Max Filippov, David Airlie, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Simona Vetter, Ilpo Järvinen, linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-sh, dri-devel, intel-gfx Cc: Bradley Morgan, Sashiko On Fri, 02 Oct 2026, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > pci=resource_alignment argument can override the default alignment for > the resource. The remainder code introduced in the commit 9036bd0efcb6 > ("PCI: Align head space better") can move remainder space (non-aligning > part of the size) before the aligning left edge which results in > violating the requested alignment. > > Introduce struct pci_resreq_data to hold device and user-given alignment > to be able to honor it in pci_align_resource(). When user-given alignment > is found, any remainder movement is skipped. > > Fixes: 9036bd0efcb6 ("PCI: Align head space better") > Reported-by: Sashiko <sashiko-bot@kernel.org> > Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/ > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> So I don't have the time to figure out what's going on here, but assuming you get proper review, the i915 part is Acked-by: Jani Nikula <jani.nikula@intel.com> for merging via whichever tree makes sense. > --- > arch/alpha/kernel/pci.c | 3 ++- > arch/arm/kernel/bios32.c | 5 +++-- > arch/m68k/kernel/pcibios.c | 4 ++-- > arch/mips/pci/pci-generic.c | 5 +++-- > arch/mips/pci/pci-legacy.c | 5 +++-- > arch/parisc/kernel/pci.c | 5 +++-- > arch/powerpc/kernel/pci-common.c | 5 +++-- > arch/sh/drivers/pci/pci.c | 5 +++-- > arch/x86/pci/i386.c | 5 +++-- > arch/xtensa/kernel/pci.c | 5 +++-- > drivers/char/agp/intel-gtt.c | 5 ++++- > drivers/gpu/drm/i915/i915_gmch.c | 5 +++-- > drivers/pci/pci.c | 5 +++-- > drivers/pci/setup-res.c | 19 +++++++++++++------ > include/linux/pci.h | 26 ++++++++++++++++++++++++-- > 15 files changed, 75 insertions(+), 32 deletions(-) > > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c > index 11df411b1d18..c5d725a9781f 100644 > --- a/arch/alpha/kernel/pci.c > +++ b/arch/alpha/kernel/pci.c > @@ -128,7 +128,8 @@ pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > struct pci_controller *hose = dev->sysdata; > unsigned long alignto; > resource_size_t start = res->start; > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index ac0e890510da..3e61826f07fb 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -564,7 +564,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, > resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t start = res->start; > struct pci_host_bridge *host_bridge; > > @@ -578,7 +579,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > start, size, align); > > if (res->flags & IORESOURCE_MEM) > - return pci_align_resource(dev, res, empty_res, size, align); > + return pci_align_resource(rr, res, empty_res, size, align); > > return start; > } > diff --git a/arch/m68k/kernel/pcibios.c b/arch/m68k/kernel/pcibios.c > index 7a9e60df79c5..3024408412bc 100644 > --- a/arch/m68k/kernel/pcibios.c > +++ b/arch/m68k/kernel/pcibios.c > @@ -31,14 +31,14 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, > resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > resource_size_t start = res->start; > > if ((res->flags & IORESOURCE_IO) && (start & 0x300)) > start = (start + 0x3ff) & ~0x3ff; > > if (res->flags & IORESOURCE_MEM) > - return pci_align_resource(dev, res, empty_res, size, align); > + return pci_align_resource(rr, res, empty_res, size, align); > > return start; > } > diff --git a/arch/mips/pci/pci-generic.c b/arch/mips/pci/pci-generic.c > index c2e23d0c1d77..7f3ecb91b85b 100644 > --- a/arch/mips/pci/pci-generic.c > +++ b/arch/mips/pci/pci-generic.c > @@ -25,7 +25,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t start = res->start; > struct pci_host_bridge *host_bridge; > > @@ -39,7 +40,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > start, size, align); > > if (res->flags & IORESOURCE_MEM) > - return pci_align_resource(dev, res, empty_res, size, align); > + return pci_align_resource(rr, res, empty_res, size, align); > > return start; > } > diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c > index dae6dafdd6e0..82d4b01db64e 100644 > --- a/arch/mips/pci/pci-legacy.c > +++ b/arch/mips/pci/pci-legacy.c > @@ -55,7 +55,8 @@ pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > struct pci_controller *hose = dev->sysdata; > resource_size_t start = res->start; > > @@ -70,7 +71,7 @@ pcibios_align_resource(void *data, const struct resource *res, > if (start & 0x300) > start = (start + 0x3ff) & ~0x3ff; > } else if (res->flags & IORESOURCE_MEM) { > - start = pci_align_resource(dev, res, empty_res, size, align); > + start = pci_align_resource(rr, res, empty_res, size, align); > > /* Make sure we start at our min on all hoses */ > if (start < PCIBIOS_MIN_MEM + hose->mem_resource->start) > diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c > index 518f532ccddc..d582be996051 100644 > --- a/arch/parisc/kernel/pci.c > +++ b/arch/parisc/kernel/pci.c > @@ -193,7 +193,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, > resource_size_t alignment) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t align, start = res->start; > > pci_dbg(dev, "%pR: pcibios_align_resource(%lx, 0x%llx, 0x%llx), parent %pR\n", > @@ -205,7 +206,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > if (align > alignment) > start = ALIGN(start, align); > else > - start = pci_align_resource(dev, res, empty_res, size, alignment); > + start = pci_align_resource(rr, res, empty_res, size, alignment); > > return start; > } > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index 4fc52c21fe5d..23594759cfe2 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -1131,7 +1131,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, > resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t start = res->start; > > if (res->flags & IORESOURCE_IO) { > @@ -1140,7 +1141,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > if (start & 0x300) > start = (start + 0x3ff) & ~0x3ff; > } else if (res->flags & IORESOURCE_MEM) { > - start = pci_align_resource(dev, res, empty_res, size, align); > + start = pci_align_resource(rr, res, empty_res, size, align); > } > > return start; > diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c > index 878a27a1acfb..279f2770ff4b 100644 > --- a/arch/sh/drivers/pci/pci.c > +++ b/arch/sh/drivers/pci/pci.c > @@ -172,7 +172,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, > resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > struct pci_channel *hose = dev->sysdata; > resource_size_t start = res->start; > > @@ -186,7 +187,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, > if (start & 0x300) > start = (start + 0x3ff) & ~0x3ff; > } else if (res->flags & IORESOURCE_MEM) { > - start = pci_align_resource(dev, res, empty_res, size, align); > + start = pci_align_resource(rr, res, empty_res, size, align); > } > > return start; > diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c > index e2de26b82940..4178ae4380c2 100644 > --- a/arch/x86/pci/i386.c > +++ b/arch/x86/pci/i386.c > @@ -156,7 +156,8 @@ pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t start = res->start; > > if (res->flags & IORESOURCE_IO) { > @@ -165,7 +166,7 @@ pcibios_align_resource(void *data, const struct resource *res, > if (start & 0x300) > start = (start + 0x3ff) & ~0x3ff; > } else if (res->flags & IORESOURCE_MEM) { > - start = pci_align_resource(dev, res, empty_res, size, align); > + start = pci_align_resource(rr, res, empty_res, size, align); > > /* The low 1MB range is reserved for ISA cards */ > if (start < BIOS_END) > diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c > index 305031551136..4e8ad4a3c9fe 100644 > --- a/arch/xtensa/kernel/pci.c > +++ b/arch/xtensa/kernel/pci.c > @@ -42,7 +42,8 @@ pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > + struct pci_dev *dev = rr->dev; > resource_size_t start = res->start; > > if (res->flags & IORESOURCE_IO) { > @@ -55,7 +56,7 @@ pcibios_align_resource(void *data, const struct resource *res, > if (start & 0x300) > start = (start + 0x3ff) & ~0x3ff; > } else if (res->flags & IORESOURCE_MEM) { > - start = pci_align_resource(dev, res, empty_res, size, align); > + start = pci_align_resource(rr, res, empty_res, size, align); > } > > return start; > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index bcc26785175d..2214347906a6 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -1038,10 +1038,13 @@ static struct agp_memory *intel_fake_agp_alloc_by_type(size_t pg_count, > > static int intel_alloc_chipset_flush_resource(void) > { > + struct pci_resreq_data rr; > int ret; > + > + pci_init_pci_resreq_data(&rr, intel_private.bridge_dev); > ret = pci_bus_alloc_resource(intel_private.bridge_dev->bus, &intel_private.ifp_resource, PAGE_SIZE, > PAGE_SIZE, PCIBIOS_MIN_MEM, 0, > - pcibios_align_resource, intel_private.bridge_dev); > + pcibios_align_resource, &rr); > > return ret; > } > diff --git a/drivers/gpu/drm/i915/i915_gmch.c b/drivers/gpu/drm/i915/i915_gmch.c > index b0ef6ef577a3..d35097bebc0f 100644 > --- a/drivers/gpu/drm/i915/i915_gmch.c > +++ b/drivers/gpu/drm/i915/i915_gmch.c > @@ -38,6 +38,7 @@ static int mchbar_reg(struct drm_i915_private *i915) > static int > intel_alloc_mchbar_resource(struct drm_i915_private *i915) > { > + struct pci_resreq_data rr; > u32 temp_lo, temp_hi = 0; > u64 mchbar_addr; > int ret; > @@ -55,12 +56,12 @@ intel_alloc_mchbar_resource(struct drm_i915_private *i915) > /* Get some space for it */ > i915->gmch.mch_res.name = "i915 MCHBAR"; > i915->gmch.mch_res.flags = IORESOURCE_MEM; > + pci_init_pci_resreq_data(&rr, i915->gmch.pdev); > ret = pci_bus_alloc_resource(i915->gmch.pdev->bus, > &i915->gmch.mch_res, > MCHBAR_SIZE, MCHBAR_SIZE, > PCIBIOS_MIN_MEM, > - 0, pcibios_align_resource, > - i915->gmch.pdev); > + 0, pcibios_align_resource, &rr); > if (ret) { > drm_dbg(&i915->drm, "failed bus alloc: %d\n", ret); > i915->gmch.mch_res.start = 0; > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b2879a6be5f8..1b7a4469c6ca 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -6444,8 +6444,8 @@ static DEFINE_SPINLOCK(resource_alignment_lock); > * RETURNS: Resource alignment if it is specified. > * Zero if it is not specified. > */ > -static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, > - bool *resize) > +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, > + bool *resize) > { > int align_order, count; > resource_size_t align = pcibios_default_alignment(); > @@ -6497,6 +6497,7 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev, > spin_unlock(&resource_alignment_lock); > return align; > } > +EXPORT_SYMBOL_GPL(pci_specified_resource_alignment); > > static void pci_request_resource_alignment(struct pci_dev *dev, int bar, > resource_size_t align, bool resize) > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > index 376f09630a4a..441a62807719 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; > + > if (IS_ALIGNED(size, align)) > return res->start; > > @@ -306,18 +310,21 @@ resource_size_t __weak pcibios_align_resource(void *data, > resource_size_t size, > resource_size_t align) > { > - struct pci_dev *dev = data; > + struct pci_resreq_data *rr = data; > > - return pci_align_resource(dev, res, empty_res, size, align); > + return pci_align_resource(rr, res, empty_res, size, align); > } > > static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > int resno, resource_size_t size, resource_size_t align) > { > struct resource *res = pci_resource_n(dev, resno); > + struct pci_resreq_data rr; > resource_size_t min; > int ret; > > + pci_init_pci_resreq_data(&rr, dev); > + > min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM; > > /* > @@ -329,7 +336,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > */ > ret = pci_bus_alloc_resource(bus, res, size, align, min, > IORESOURCE_PREFETCH | IORESOURCE_MEM_64, > - pcibios_align_resource, dev); > + pcibios_align_resource, &rr); > if (ret == 0) > return 0; > > @@ -341,7 +348,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) { > ret = pci_bus_alloc_resource(bus, res, size, align, min, > IORESOURCE_PREFETCH, > - pcibios_align_resource, dev); > + pcibios_align_resource, &rr); > if (ret == 0) > return 0; > } > @@ -354,7 +361,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev, > */ > if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) > ret = pci_bus_alloc_resource(bus, res, size, align, min, 0, > - pcibios_align_resource, dev); > + pcibios_align_resource, &rr); > > return ret; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d31a8d107b1e..fe0233f0d9e6 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); > +} > + > resource_size_t pcibios_align_resource(void *data, const struct resource *res, > const struct resource *empty_res, > resource_size_t size, > resource_size_t align); > -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, -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 3/7] PCI: Honor alignment overrides 2026-10-02 11:33 ` [PATCH v2 3/7] PCI: Honor alignment overrides Ilpo Järvinen 2026-10-02 11:46 ` Jani Nikula @ 2026-10-02 11:49 ` sashiko-bot 1 sibling, 0 replies; 3+ messages in thread From: sashiko-bot @ 2026-10-02 11:49 UTC (permalink / raw) To: Ilpo Järvinen; +Cc: intel-gfx, linux-pci, dri-devel 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-02 11:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261002113319.6652-1-ilpo.jarvinen@linux.intel.com>
2026-10-02 11:33 ` [PATCH v2 3/7] PCI: Honor alignment overrides Ilpo Järvinen
2026-10-02 11:46 ` Jani Nikula
2026-10-02 11:49 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox