linux-kernel - Re: [RFC PATCH 4/6] x86: PCI: preserve IORESOURCE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240711184035.GA287904@bhelgaas>
Date: Thu, 11 Jul 2024 13:40:35 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Stewart Hildebrand <stewart.hildebrand@....com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 4/6] x86: PCI: preserve IORESOURCE_STARTALIGN
 alignment

On Wed, Jul 10, 2024 at 06:49:42PM -0400, Stewart Hildebrand wrote:
> On 7/10/24 17:24, Bjorn Helgaas wrote:
> > On Wed, Jul 10, 2024 at 12:16:24PM -0400, Stewart Hildebrand wrote:
> >> On 7/9/24 12:19, Bjorn Helgaas wrote:
> >>> On Tue, Jul 09, 2024 at 09:36:01AM -0400, Stewart Hildebrand wrote:
> >>>> Currently, it's not possible to use the IORESOURCE_STARTALIGN flag on
> >>>> x86 due to the alignment being overwritten in
> >>>> pcibios_allocate_dev_resources(). Make one small change in arch/x86 to
> >>>> make it work on x86.
> >>>
> >>> Is this a regression?  I didn't look up when IORESOURCE_STARTALIGN was
> >>> added, but likely it was for some situation on x86, so presumably it
> >>> worked at one time.  If something broke it in the meantime, it would
> >>> be nice to identify the commit that broke it.
> >>
> >> No, I don't have reason to believe it's a regression.
> >>
> >> IORESOURCE_STARTALIGN was introduced in commit 884525655d07 ("PCI: clean
> >> up resource alignment management").
> > 
> > Ah, OK.  IORESOURCE_STARTALIGN is used for bridge windows, which don't
> > need to be aligned on their size as BARs do.  It would be terrible if
> > that usage was broken, which is why I was alarmed by the idea of it
> > not working on x86> 
> > But this patch is only relevant for BARs.  I was a little confused
> > about IORESOURCE_STARTALIGN for a BAR, but I guess the point is to
> > force alignment on *more* than the BAR's size, e.g., to prevent
> > multiple BARs from being put in the same page.
> > 
> > Bottom line, this would need to be a little more specific so it
> > doesn't suggest that IORESOURCE_STARTALIGN for windows is broken.
> 
> I'll make the commit message clearer.
> 
> > IIUC, the main purpose of the series is to align all BARs to at least
> > 4K.  I don't think the series relies on IORESOURCE_STARTALIGN to do
> > that.
> 
> Yes, it does rely on IORESOURCE_STARTALIGN for BARs.

Oh, I missed that, sorry.  The only places I see that set
IORESOURCE_STARTALIGN are pci_request_resource_alignment(), which is
where I got the "pci=resource_alignment=..." connection, and
pbus_size_io(), pbus_size_mem(), and pci_bus_size_cardbus(), which are
for bridge windows, AFAICS.

Doesn't the >= 4K alignment in this series hinge on the
pcibios_default_alignment() change?  It looks like that would force at
least 4K alignment independent of IORESOURCE_STARTALIGN.

> > But there's an issue with "pci=resource_alignment=..." that you
> > noticed sort of incidentally, and this patch fixes that?
> 
> No, pci=resource_alignment= results in IORESOURCE_SIZEALIGN, which
> breaks pcitest. And we'd like pcitest to work properly for PCI
> passthrough validation with Xen, hence the need for
> IORESOURCE_STARTALIGN.