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

[Benjamin Block <bblock@linux.ibm.com>]

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