Re: [PATCH v2] sparc: Fix page alignment in dma mapping

On Wed, 2026-02-18 at 11:28 +0100, Marek Szyprowski wrote:
> On 17.02.2026 18:10, [email protected] wrote:
> > From: Stian Halseth <[email protected]>
> > 
> > 'phys' may include an offset within the page, while 'base_paddr' is
> > already
> > page-aligned. This caused incorrect DMA mapping in dma_4u_map_phys
> > and
> > dma_4v_map_phys.
> > 
> > Fix both functions by masking phys with IO_PAGE_MASK or subtracting
> > the
> > page offset, covering both generic SPARC code and sun4v.
> > 
> > Fixes: 38c0d0ebf520 ("sparc: Use physical address DMA mapping")
> > Reported-by: Stian Halseth <[email protected]>
> > Closes: https://protect2.fireeye.com/v1/url?k=4d8fbc00-1214850c-
> > 4d8e374f-000babff3563-ee1be24be576e072&q=1&e=0f8967ce-f558-4339-
> > bddb-
> > f324ec46c035&u=https%3A%2F%2Fgithub.com%2Fsparclinux%2Fissues%2Fiss
> > ues%2F75
> > Suggested-by: Marek Szyprowski <[email protected]>
> > Signed-off-by: Stian Halseth <[email protected]>
> > ---
> >   arch/sparc/kernel/iommu.c     | 2 ++
> >   arch/sparc/kernel/pci_sun4v.c | 2 +-
> >   2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
> > index 46ef88bc9c26..f3755a388ac7 100644
> > --- a/arch/sparc/kernel/iommu.c
> > +++ b/arch/sparc/kernel/iommu.c
> > @@ -312,6 +312,8 @@ static dma_addr_t dma_4u_map_phys(struct device
> > *dev, phys_addr_t phys,
> >    if (direction != DMA_TO_DEVICE)
> >    iopte_protection |= IOPTE_WRITE;
> >   
> > + phys -= offset_in_page(phys);
> > +
> 
> I've just noticed that IO_PAGE_SIZE might not equal to PAGE_SIZE (not
> sure if this is true in case of SPARCs), so it would be better to
> rely 
> on IO_PAGE_MASK or IO_PAGE_SIZE only. Just unify the fix for the both
> affected functions either by masking phys with IO_PAGE_SIZE where it
> is 
> used or by subtracting (phys & ~IO_PAGE_MASK) from it.
> 

Good point, and thanks for the review.

I failed to consider that the offset_in_page is based on PAGE_SIZE, not
IO_PAGE_SIZE.

Suggestion, subtract IO_PAGE_MASK in both functions.

arch/sparc/kernel/iommu.c:
 -phys -= offset_in_page(phys)
 +phys &= IO_PAGE_MASK;


arch/sparc/kernel/pci_sun4v.c (better to subtract before loop?):
 +phys &= IO_PAGE_MASK;

 for (i = 0; i < npages; i++, phys += IO_PAGE_SIZE) {
         -long err = iommu_batch_add(phys & IO_PAGE_MASK, mask);
         +long err = iommu_batch_add(phys, mask);
         if (unlikely(err < 0L))
                 goto iommu_map_fail;
 }

Is this acceptable, and in line with kernel coding style?

If yes, I can submit patch v3 with this proposed change.

And Patch v3, should be sent using the message ID of this review
response, right? Still learning :)



> >    for (i = 0; i < npages; i++, base++, phys += IO_PAGE_SIZE)
> >    iopte_val(*base) = iopte_protection | phys;
> >   
> > diff --git a/arch/sparc/kernel/pci_sun4v.c
> > b/arch/sparc/kernel/pci_sun4v.c
> > index 791f0a76665f..2f30eeac4861 100644
> > --- a/arch/sparc/kernel/pci_sun4v.c
> > +++ b/arch/sparc/kernel/pci_sun4v.c
> > @@ -411,7 +411,7 @@ static dma_addr_t dma_4v_map_phys(struct device
> > *dev, phys_addr_t phys,
> >    iommu_batch_start(dev, prot, entry);
> >   
> >    for (i = 0; i < npages; i++, phys += IO_PAGE_SIZE) {
> > - long err = iommu_batch_add(phys, mask);
> > + long err = iommu_batch_add(phys & IO_PAGE_MASK, mask);
> >    if (unlikely(err < 0L))
> >    goto iommu_map_fail;
> >    }
> 
> Best regards

-- 
Med vennlig hilsen
Stian Halseth
Mobile       +47 406 77 777
Office(dir) +47 236 80 400 (402)
E-mail          [email protected]
​​