Re: [PATCH v4 2/4] s390/pci: store DMA offset in bus_dma_region

[Niklas Schnelle]

On Wed, 2025-02-12 at 10:23 -0500, Matthew Rosato wrote:
> > > +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 = max(PAGE_ALIGN_DOWN(zdev->end_dma + 1) - map->dma_start, 0);
> > 
> > Ugh, this is my fault as I suggested it, but this max() doesn't work
> > here. The zdev->end_dma is unsigned and so is map->dma_start so if the
> > former is smaller underflow will occur and the max() won't save us.
> > It's largely a theoretical issue since zdev->end_dma + 1 should always
> > be larger than zdev->start_dma, but now the max() looks like we thought
> > of that, but then it doesn't work.
> > 
> > If we handle it maybe just go with:
> > 
> > aligned_end = PAGE_ALIGN_DOWN(zdev->end_dma + 1);
> > if (aligned_end >= map->dma_start)
> >      map->size = aligned_end - map->dma_start;
> > else
> >      map->size = 0;
> > 
> 
> 
> Given that it's not really something that's supposed to happen, would it make sense then to add a 
> 
> WARN_ON_ONCE(map->size == 0);
> 
> At the end of this?


Yes that makes sense to me