[PATCH v2] s390/pci: Fix dev.dma_range_map missing sentinel element

[PATCH v2] s390/pci: Fix dev.dma_range_map missing sentinel element

[Niklas Schnelle]

The fixed commit sets up dev.dma_range_map but missed that this is
supposed to be an array of struct bus_dma_region with a sentinel element
with the size field set to 0 at the end. This would lead to overruns in
e.g. dma_range_map_min(). It could also result in wrong translations
instead of DMA_MAPPING_ERROR in translate_phys_to_dma() if the paddr
were to not fit in the aperture. Fix this by using the
dma_direct_set_offset() helper which creates a sentinel for us.

Fixes: d236843a6964 ("s390/pci: store DMA offset in bus_dma_region")
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Note: Based on iommu/next

v1 -> v2:
- Fixed typo, added trailers
- Link to v1: https://lore.kernel.org/r/20250306-fix_dma_map_alloc-v1-1-b4fa44304eac@linux.ibm.com
---
 arch/s390/pci/pci_bus.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
index 0e725039861f92925a38f7ff7cb6de6b0d965ac3..14310c3b48860a16de13536adf95ef99e6af21cc 100644
--- a/arch/s390/pci/pci_bus.c
+++ b/arch/s390/pci/pci_bus.c
@@ -287,23 +287,21 @@ static struct zpci_bus *zpci_bus_alloc(int topo, bool topo_is_tid)
 static void pci_dma_range_setup(struct pci_dev *pdev)
 {
 	struct zpci_dev *zdev = to_zpci(pdev);
-	struct bus_dma_region *map;
-	u64 aligned_end;
+	u64 aligned_end, size;
+	dma_addr_t dma_start;
+	int ret;
 
-	map = kzalloc(sizeof(*map), GFP_KERNEL);
-	if (!map)
-		return;
-
-	map->cpu_start = 0;
-	map->dma_start = PAGE_ALIGN(zdev->start_dma);
+	dma_start = PAGE_ALIGN(zdev->start_dma);
 	aligned_end = PAGE_ALIGN_DOWN(zdev->end_dma + 1);
-	if (aligned_end >= map->dma_start)
-		map->size = aligned_end - map->dma_start;
+	if (aligned_end >= dma_start)
+		size = aligned_end - dma_start;
 	else
-		map->size = 0;
-	WARN_ON_ONCE(map->size == 0);
+		size = 0;
+	WARN_ON_ONCE(size == 0);
 
-	pdev->dev.dma_range_map = map;
+	ret = dma_direct_set_offset(&pdev->dev, 0, dma_start, size);
+	if (ret)
+		pr_err("Failed to allocate DMA range map for %s\n", pci_name(pdev));
 }
 
 void pcibios_bus_add_device(struct pci_dev *pdev)

---
base-commit: e840414e5a73ac02ffba6299b46f535a0b7cba98
change-id: 20250306-fix_dma_map_alloc-b3b05903dcfb

Best regards,
-- 
Niklas Schnelle

Re: [PATCH v2] s390/pci: Fix dev.dma_range_map missing sentinel element

[Niklas Schnelle]

On Wed, 2025-03-12 at 11:32 +0100, Niklas Schnelle wrote:
> The fixed commit sets up dev.dma_range_map but missed that this is
> supposed to be an array of struct bus_dma_region with a sentinel element
> with the size field set to 0 at the end. This would lead to overruns in
> e.g. dma_range_map_min(). It could also result in wrong translations
> instead of DMA_MAPPING_ERROR in translate_phys_to_dma() if the paddr
> were to not fit in the aperture. Fix this by using the
> dma_direct_set_offset() helper which creates a sentinel for us.
> 
> Fixes: d236843a6964 ("s390/pci: store DMA offset in bus_dma_region")
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Note: Based on iommu/next
> 
> v1 -> v2:
> - Fixed typo, added trailers
> - Link to v1: https://lore.kernel.org/r/20250306-fix_dma_map_alloc-v1-1-b4fa44304eac@linux.ibm.com
> ---
>  arch/s390/pci/pci_bus.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 

With the IOMMU pull request for Linux v6.15 the fixed commit is now in
Linus tree (with the SHA as cited) but this fix isn't. Since this fix
only touches s390 specific code I think it could go through the s390
tree but that currently doesn't carry the fixed commit so would have to
wait until the s390 tree pulls in v6.15-rc1. 

I don't think any breakage from the missing sentinel is visible outside
of guests using the new IOMMU passthrough mode so I guess waiting until
after v6.15-rc1 would be okay.

Heiko, Vasily, Alexander what do you think?

Thanks,
Niklas

Re: [PATCH v2] s390/pci: Fix dev.dma_range_map missing sentinel element

[Vasily Gorbik]

On Thu, Mar 27, 2025 at 10:45:40AM +0100, Niklas Schnelle wrote:
> On Wed, 2025-03-12 at 11:32 +0100, Niklas Schnelle wrote:
> > The fixed commit sets up dev.dma_range_map but missed that this is
> > supposed to be an array of struct bus_dma_region with a sentinel element
> > with the size field set to 0 at the end. This would lead to overruns in
> > e.g. dma_range_map_min(). It could also result in wrong translations
> > instead of DMA_MAPPING_ERROR in translate_phys_to_dma() if the paddr
> > were to not fit in the aperture. Fix this by using the
> > dma_direct_set_offset() helper which creates a sentinel for us.
> > 
> > Fixes: d236843a6964 ("s390/pci: store DMA offset in bus_dma_region")
> > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> > Note: Based on iommu/next
> > 
> > v1 -> v2:
> > - Fixed typo, added trailers
> > - Link to v1: https://lore.kernel.org/r/20250306-fix_dma_map_alloc-v1-1-b4fa44304eac@linux.ibm.com
> > ---
> >  arch/s390/pci/pci_bus.c | 24 +++++++++++-------------
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> > 
> 
> With the IOMMU pull request for Linux v6.15 the fixed commit is now in
> Linus tree (with the SHA as cited) but this fix isn't. Since this fix
> only touches s390 specific code I think it could go through the s390
> tree but that currently doesn't carry the fixed commit so would have to
> wait until the s390 tree pulls in v6.15-rc1. 
> 
> I don't think any breakage from the missing sentinel is visible outside
> of guests using the new IOMMU passthrough mode so I guess waiting until
> after v6.15-rc1 would be okay.
> 
> Heiko, Vasily, Alexander what do you think?

Sure, I'll pick it up in the s390 tree for rc1 or rc2, depending on
whether there will be a second s390 tree pull request during the merge
window.

Re: [PATCH v2] s390/pci: Fix dev.dma_range_map missing sentinel element

[Vasily Gorbik]

On Thu, Mar 27, 2025 at 03:51:50PM +0100, Vasily Gorbik wrote:
> On Thu, Mar 27, 2025 at 10:45:40AM +0100, Niklas Schnelle wrote:
> > On Wed, 2025-03-12 at 11:32 +0100, Niklas Schnelle wrote:
> > > The fixed commit sets up dev.dma_range_map but missed that this is
> > > supposed to be an array of struct bus_dma_region with a sentinel element
> > > with the size field set to 0 at the end. This would lead to overruns in
> > > e.g. dma_range_map_min(). It could also result in wrong translations
> > > instead of DMA_MAPPING_ERROR in translate_phys_to_dma() if the paddr
> > > were to not fit in the aperture. Fix this by using the
> > > dma_direct_set_offset() helper which creates a sentinel for us.
> > > 
> > > Fixes: d236843a6964 ("s390/pci: store DMA offset in bus_dma_region")
> > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > > Note: Based on iommu/next
> > > 
> > > v1 -> v2:
> > > - Fixed typo, added trailers
> > > - Link to v1: https://lore.kernel.org/r/20250306-fix_dma_map_alloc-v1-1-b4fa44304eac@linux.ibm.com
> > > ---
> > >  arch/s390/pci/pci_bus.c | 24 +++++++++++-------------
> > >  1 file changed, 11 insertions(+), 13 deletions(-)
> > > 
> > 
> > With the IOMMU pull request for Linux v6.15 the fixed commit is now in
> > Linus tree (with the SHA as cited) but this fix isn't. Since this fix
> > only touches s390 specific code I think it could go through the s390
> > tree but that currently doesn't carry the fixed commit so would have to
> > wait until the s390 tree pulls in v6.15-rc1. 
> > 
> > I don't think any breakage from the missing sentinel is visible outside
> > of guests using the new IOMMU passthrough mode so I guess waiting until
> > after v6.15-rc1 would be okay.
> > 
> > Heiko, Vasily, Alexander what do you think?
> 
> Sure, I'll pick it up in the s390 tree for rc1 or rc2, depending on
> whether there will be a second s390 tree pull request during the merge
> window.

Applied, thank you!