linux-kernel - Re: [PATCH v4 04/10] s390/pci: Add architecture specific resource/bus address translation

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: <20251001160415.GC408411@p1gen4-pw042f0m>
Date: Wed, 1 Oct 2025 18:04:15 +0200
From: Benjamin Block <bblock@...ux.ibm.com>
To: Niklas Schnelle <schnelle@...ux.ibm.com>
Cc: Farhan Ali <alifm@...ux.ibm.com>, linux-s390@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, alex.williamson@...hat.com,
        helgaas@...nel.org, clg@...hat.com, mjrosato@...ux.ibm.com
Subject: Re: [PATCH v4 04/10] s390/pci: Add architecture specific
 resource/bus address translation

On Thu, Sep 25, 2025 at 12:54:07PM +0200, Niklas Schnelle wrote:
> On Wed, 2025-09-24 at 10:16 -0700, Farhan Ali wrote:
> > +void pcibios_resource_to_bus(struct pci_bus *bus, struct pci_bus_region *region,
> > +			     struct resource *res)
> > +{
> > +	struct zpci_bus *zbus = bus->sysdata;
> > +	struct zpci_bar_struct *zbar;
> > +	struct zpci_dev *zdev;
> > +
> > +	region->start = res->start;
> > +	region->end = res->end;
> 
> When we don't find a BAR matching the resource this would become the
> region used. I'm not sure what a better value would be if we don't find
> a match though and that should hopefully not happen in sensible uses.
> Also this would keep the existing behavior so seems fine.

I was wondering the same things, but I guess it matches what happens elsewhere
as well, if no match is found

	void __weak pcibios_resource_to_bus(struct pci_bus *bus, struct pci_bus_region *region,
				     struct resource *res)
	{
		...
		resource_size_t offset = 0;

		resource_list_for_each_entry(window, &bridge->windows) {
			if (resource_contains(window->res, res)) {
				offset = window->offset;
				break;
			}
		}

		region->start = res->start - offset;
		region->end = res->end - offset;
	}

So I guess that is fine.

The thing I'm also unclear on is whether it is OK to "throw out" this whole
logic about `resource_contains(window->res, res)` here and
`region_contains(&bus_region, region)` in the other original function?
I mean, the original function don't search for perfect matches, but also
matches where are contained in a given resource/region, which is different
from what we do here. Are we OK with not doing that at all?

> > +
> > +	for (int i = 0; i < ZPCI_FUNCTIONS_PER_BUS; i++) {
> > +		int j = 0;
> > +
> > +		zbar = NULL;
> > +		zdev = zbus->function[i];
> > +		if (!zdev)
> > +			continue;
> > +
> > +		for (j = 0; j < PCI_STD_NUM_BARS; j++) {
> > +			if (zdev->bars[j].res->start == res->start &&
> > +			    zdev->bars[j].res->end == res->end &&
> > +			    res->flags & IORESOURCE_MEM) {
> > +				zbar = &zdev->bars[j];
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (zbar) {
> > +			/* only MMIO is supported */
> > +			region->start = zbar->val & PCI_BASE_ADDRESS_MEM_MASK;
> > +			if (zbar->val & PCI_BASE_ADDRESS_MEM_TYPE_64)
> > +				region->start |= (u64)zdev->bars[j + 1].val << 32;
> > +
> > +			region->end = region->start + (1UL << zbar->size) - 1;
> > +			return;
> > +		}
> > +	}
> > +}

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Böblingen     /    Registergericht: AmtsG Stuttgart, HRB 243294