All of lore.kernel.org
 help / color / mirror / Atom feed
From: Farhan Ali <alifm@linux.ibm.com>
To: Benjamin Block <bblock@linux.ibm.com>,
	Niklas Schnelle <schnelle@linux.ibm.com>
Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	alex.williamson@redhat.com, helgaas@kernel.org, clg@redhat.com,
	mjrosato@linux.ibm.com
Subject: Re: [PATCH v4 04/10] s390/pci: Add architecture specific resource/bus address translation
Date: Wed, 1 Oct 2025 11:01:47 -0700	[thread overview]
Message-ID: <a4c448b2-6909-4efb-b41d-396ea8ececd3@linux.ibm.com> (raw)
In-Reply-To: <20251001160415.GC408411@p1gen4-pw042f0m>


On 10/1/2025 9:04 AM, Benjamin Block wrote:
> 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?

I had thought about doing the range check, similar to 
resource_contains/region_contains rather than doing exact checks. But I 
think the way we expose the topology of the devices, the offset in our 
(s390x) case is zero. So I thought it should be safe to just doing exact 
match and might help us catch any issues if its not exact.

Thanks

Farhan


  reply	other threads:[~2025-10-01 18:01 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-24 17:16 [PATCH v4 00/10] Error recovery for vfio-pci devices on s390x Farhan Ali
2025-09-24 17:16 ` [PATCH v4 01/10] PCI: Avoid saving error values for config space Farhan Ali
2025-10-01 15:15   ` Benjamin Block
2025-10-01 17:12     ` Farhan Ali
2025-10-02  9:16       ` Benjamin Block
2025-10-04 14:54       ` Lukas Wunner
2025-10-06 17:54         ` Farhan Ali
2025-10-06 19:26           ` Lukas Wunner
2025-10-06 21:35             ` Farhan Ali
2025-10-08 13:34               ` Lukas Wunner
2025-10-08 17:56                 ` Farhan Ali
2025-10-08 18:14                   ` Lukas Wunner
2025-10-08 21:55                     ` Farhan Ali
2025-10-09  4:52                       ` Lukas Wunner
2025-10-09 17:02                         ` Farhan Ali
2025-10-12  6:43                           ` Lukas Wunner
2025-10-09  9:12                     ` Niklas Schnelle
2025-10-12  6:34                       ` Lukas Wunner
2025-10-14 12:07                         ` Niklas Schnelle
2025-10-16 21:00                           ` Farhan Ali
2025-10-19 14:34                           ` Lukas Wunner
2025-10-20  8:59                             ` Niklas Schnelle
2025-11-22 10:58                               ` Lukas Wunner
2025-09-24 17:16 ` [PATCH v4 02/10] PCI: Add additional checks for flr reset Farhan Ali
2025-09-30 10:03   ` Benjamin Block
2025-09-30 17:04     ` Farhan Ali
2025-10-01  8:33       ` Benjamin Block
2025-10-01 14:37   ` Benjamin Block
2025-09-24 17:16 ` [PATCH v4 03/10] PCI: Allow per function PCI slots Farhan Ali
2025-10-01 14:34   ` Benjamin Block
2025-09-24 17:16 ` [PATCH v4 04/10] s390/pci: Add architecture specific resource/bus address translation Farhan Ali
2025-09-25 10:54   ` Niklas Schnelle
2025-10-01 16:04     ` Benjamin Block
2025-10-01 18:01       ` Farhan Ali [this message]
2025-10-02 12:58   ` Niklas Schnelle
2025-10-02 17:00     ` Bjorn Helgaas
2025-10-02 17:16       ` Ilpo Järvinen
2025-10-02 18:14       ` Niklas Schnelle
2025-09-24 17:16 ` [PATCH v4 05/10] s390/pci: Restore IRQ unconditionally for the zPCI device Farhan Ali
2025-09-24 17:16 ` [PATCH v4 06/10] s390/pci: Update the logic for detecting passthrough device Farhan Ali
2025-09-24 17:16 ` [PATCH v4 07/10] s390/pci: Store PCI error information for passthrough devices Farhan Ali
2025-09-25 14:28   ` Niklas Schnelle
2025-09-25 16:29     ` Farhan Ali
2025-09-24 17:16 ` [PATCH v4 08/10] vfio-pci/zdev: Add a device feature for error information Farhan Ali
2025-09-25  8:04   ` kernel test robot
2025-09-24 17:16 ` [PATCH v4 09/10] vfio: Add a reset_done callback for vfio-pci driver Farhan Ali
2025-09-24 17:16 ` [PATCH v4 10/10] vfio: Remove the pcie check for VFIO_PCI_ERR_IRQ_INDEX Farhan Ali

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a4c448b2-6909-4efb-b41d-396ea8ececd3@linux.ibm.com \
    --to=alifm@linux.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=bblock@linux.ibm.com \
    --cc=clg@redhat.com \
    --cc=helgaas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=schnelle@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.