mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Stian Halseth <stian@itx.no>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Andreas Larsson <andreas@gaisler.com>,
	Leon Romanovsky <leon@kernel.org>,
	sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sparc: Fix page alignment in dma mapping
Date: Wed, 18 Feb 2026 12:40:57 +0100	[thread overview]
Message-ID: <5e5a4514-228a-4631-ae91-aec1df8f67c7@samsung.com> (raw)
In-Reply-To: <37bd2be353f839c1cb13b0d18bc9e8903b31f9c0.camel@itx.no>

On 18.02.2026 12:09, Stian Halseth wrote:
> On Wed, 2026-02-18 at 11:28 +0100, Marek Szyprowski wrote:
>> On 17.02.2026 18:10, stian@itx.no wrote:
>>> From: Stian Halseth <stian@itx.no>
>>>
>>> '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 <stian@itx.no>
>>> 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 <m.szyprowski@samsung.com>
>>> Signed-off-by: Stian Halseth <stian@itx.no>
>>> ---
>>>    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?

Yes, that's fine.

> 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 :)

Usually the next version is being sent as a new thread. Just add a few 
lines of history and links to the previous attempt(s) to it, like for 
example here: 
https://lore.kernel.org/all/20260203102133.1478331-1-m.szyprowski@samsung.com/

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


      parent reply	other threads:[~2026-02-18 11:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-17 16:21 [PATCH] sparc: iommu/pci_sun4v: fix " stian
2026-02-17 16:38 ` John Paul Adrian Glaubitz
2026-02-17 16:48   ` Stian Halseth
2026-02-17 17:16     ` John Paul Adrian Glaubitz
2026-02-17 17:25       ` Leon Romanovsky
2026-02-17 17:33         ` Stian Halseth
2026-02-17 17:52           ` Leon Romanovsky
2026-02-17 19:41             ` Stian Halseth
2026-02-17 19:43               ` John Paul Adrian Glaubitz
2026-02-17 17:10   ` [PATCH v2] sparc: Fix " stian
2026-02-18  8:48     ` Leon Romanovsky
2026-02-18 10:28     ` Marek Szyprowski
2026-02-18 11:09       ` Stian Halseth
2026-02-18 11:14         ` Leon Romanovsky
2026-02-18 11:40         ` Marek Szyprowski [this message]

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=5e5a4514-228a-4631-ae91-aec1df8f67c7@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=andreas@gaisler.com \
    --cc=davem@davemloft.net \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=stian@itx.no \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®