Re: [PATCH v3 2/3] s390/pci: store DMA offset in bus_dma_region

Niklas Schnelle
2025-01-29T10:20:29Z

On Fri, 2025-01-24 at 15:17 -0500, Matthew Rosato wrote:
> PCI devices on s390 have a DMA offset that is reported via CLP. In
> preparation for allowing identity domains, setup the bus_dma_region
> for all PCI devices using the reported CLP value.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
> arch/s390/pci/pci_bus.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
> index d5ace00d10f0..51fa993b64fc 100644
> --- a/arch/s390/pci/pci_bus.c
> +++ b/arch/s390/pci/pci_bus.c
> @@ -19,6 +19,7 @@
> #include <linux/jump_label.h>
> #include <linux/pci.h>
> #include <linux/printk.h>
> +#include <linux/dma-direct.h>
>
> #include <asm/pci_clp.h>
> #include <asm/pci_dma.h>
> @@ -284,10 +285,27 @@ static struct zpci_bus *zpci_bus_alloc(int topo, bool topo_is_tid)
> return zbus;
> }
>
> +static void pci_dma_range_setup(struct pci_dev *pdev)
> +{
> + struct zpci_dev *zdev = to_zpci(pdev);
> + struct bus_dma_region *map;
> +
> + map = kzalloc(sizeof(*map), GFP_KERNEL);
> + if (!map)
> + return;
> +
> + map->cpu_start = 0;
> + map->dma_start = PAGE_ALIGN(zdev->start_dma);
> + map->size = zdev->end_dma - zdev->start_dma + 1;

Shouldn't the size calculation also use PAGE_ALIGN(zdev->start_dma) in
case that actually aligns? And for zdev->end_dma I think we want
PAGE_ALIGN_DOWN(zdev->end_dma + 1). There is also the, hopefully
theoretical, case that zdev->end_dma could be less than PAGE_SIZE
larger than zdev->start_dma. I guess in that case we should just ensure
that size is 0, maybe using max(…, 0) around the expression.

> + pdev->dev.dma_range_map = map;
> +}
> +
> void pcibios_bus_add_device(struct pci_dev *pdev)
> {
> struct zpci_dev *zdev = to_zpci(pdev);
>
> + pci_dma_range_setup(pdev);
> +
> /*
> * With pdev->no_vf_scan the common PCI probing code does not
> * perform PF/VF linking.

I was initially wondering where the map is freed, then found the
kfree()s in common code in device_unbind_cleanup() and
device_release().

Despite the thoughts above I already tested this on my setup and it
worked great, so you can already add my:

Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>