Needed by the eDMA v3 DMA engine found in iommu-less SoCs such as iMX8QXP
to be able to perform DMA operations as a Xen Hardware Domain, which needs
to be able to do DMA in MMIO space.
The callback implementation is basically the same as the one for direct
mapping of resources, except this also takes into account Xen page
mappings.
There is nothing to do for unmap, just like with direct, so the unmap
callback is not needed.
Signed-off-by: John Ernberg <john.ernberg@actia.se>
---
I originally exported dma_direct_map_resource() and used that which
appeared to work, but it felt like not checking Xen page mappings wasn't
fully correct and I went with this. But if dma_direct_map_resource() is
the correct approach here then I can submit that isntead.
---
drivers/xen/swiotlb-xen.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index ef56a2500ed6..0f02fdd9128d 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -285,6 +285,20 @@ static void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
attrs, pool);
}
+static dma_addr_t xen_swiotlb_map_resource(struct device *dev, phys_addr_t phys,
+ size_t size, enum dma_data_direction dir,
+ unsigned long attrs)
+{
+ dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
+
+ BUG_ON(dir == DMA_NONE);
+
+ if (!dma_capable(dev, dev_addr, size, false))
+ return DMA_MAPPING_ERROR;
+
+ return dev_addr;
+}
+
static void
xen_swiotlb_sync_single_for_cpu(struct device *dev, dma_addr_t dma_addr,
size_t size, enum dma_data_direction dir)
@@ -426,4 +440,5 @@ const struct dma_map_ops xen_swiotlb_dma_ops = {
.alloc_pages_op = dma_common_alloc_pages,
.free_pages = dma_common_free_pages,
.max_mapping_size = swiotlb_max_mapping_size,
+ .map_resource = xen_swiotlb_map_resource,
};
--
2.49.0+Christoph
On Fri, 2 May 2025, John Ernberg wrote:
> Needed by the eDMA v3 DMA engine found in iommu-less SoCs such as iMX8QXP
> to be able to perform DMA operations as a Xen Hardware Domain, which needs
> to be able to do DMA in MMIO space.
>
> The callback implementation is basically the same as the one for direct
> mapping of resources, except this also takes into account Xen page
> mappings.
>
> There is nothing to do for unmap, just like with direct, so the unmap
> callback is not needed.
>
> Signed-off-by: John Ernberg <john.ernberg@actia.se>
>
> ---
>
> I originally exported dma_direct_map_resource() and used that which
> appeared to work, but it felt like not checking Xen page mappings wasn't
> fully correct and I went with this. But if dma_direct_map_resource() is
> the correct approach here then I can submit that isntead.
> ---
> drivers/xen/swiotlb-xen.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index ef56a2500ed6..0f02fdd9128d 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -285,6 +285,20 @@ static void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> attrs, pool);
> }
>
> +static dma_addr_t xen_swiotlb_map_resource(struct device *dev, phys_addr_t phys,
> + size_t size, enum dma_data_direction dir,
> + unsigned long attrs)
> +{
> + dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
Yes, we need the xen_phys_to_dma call here. This is one of the reasons I
don't think we can use dma_direct_map_resource() to implement
map_resource
> + BUG_ON(dir == DMA_NONE);
> +
> + if (!dma_capable(dev, dev_addr, size, false))
> + return DMA_MAPPING_ERROR;
But here you also need to check that phys+size doesn't cross a page
boundary. You need to call range_straddles_page_boundary, like we do at
the beginning of xen_swiotlb_map_page.
If it is possible to cross a page boundary, then we need to basically to
do the same thing here as we do in xen_swiotlb_map_page where we check
for:
if (dma_capable(dev, dev_addr, size, true) &&
!range_straddles_page_boundary(phys, size) &&
!xen_arch_need_swiotlb(dev, phys, dev_addr) &&
!is_swiotlb_force_bounce(dev))
goto done;
if all is well, we go with the native path, otherwise we bounce on
swiotlb-xen.
> + return dev_addr;
> +}
> +
> static void
> xen_swiotlb_sync_single_for_cpu(struct device *dev, dma_addr_t dma_addr,
> size_t size, enum dma_data_direction dir)
> @@ -426,4 +440,5 @@ const struct dma_map_ops xen_swiotlb_dma_ops = {
> .alloc_pages_op = dma_common_alloc_pages,
> .free_pages = dma_common_free_pages,
> .max_mapping_size = swiotlb_max_mapping_size,
> + .map_resource = xen_swiotlb_map_resource,
> };
> --
> 2.49.0
>
Hi Stefano,
On 5/2/25 7:20 PM, Stefano Stabellini wrote:
> +Christoph
>
> On Fri, 2 May 2025, John Ernberg wrote:
>> Needed by the eDMA v3 DMA engine found in iommu-less SoCs such as iMX8QXP
>> to be able to perform DMA operations as a Xen Hardware Domain, which needs
>> to be able to do DMA in MMIO space.
Self-note: The above part of the commit message is a disaster and should
read something like.
Needed by SoCs without an iommu (such as the iMX8QXP and it's eDMA v3
engine) that need to be able to perform DMA operations in MMIO space.
>>
>> The callback implementation is basically the same as the one for direct
>> mapping of resources, except this also takes into account Xen page
>> mappings.
>>
>> There is nothing to do for unmap, just like with direct, so the unmap
>> callback is not needed.
>>
>> Signed-off-by: John Ernberg <john.ernberg@actia.se>
>>
>> ---
>>
>> I originally exported dma_direct_map_resource() and used that which
>> appeared to work, but it felt like not checking Xen page mappings wasn't
>> fully correct and I went with this. But if dma_direct_map_resource() is
>> the correct approach here then I can submit that isntead.
>> ---
>> drivers/xen/swiotlb-xen.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
>> index ef56a2500ed6..0f02fdd9128d 100644
>> --- a/drivers/xen/swiotlb-xen.c
>> +++ b/drivers/xen/swiotlb-xen.c
>> @@ -285,6 +285,20 @@ static void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
>> attrs, pool);
>> }
>>
>> +static dma_addr_t xen_swiotlb_map_resource(struct device *dev, phys_addr_t phys,
>> + size_t size, enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
>
> Yes, we need the xen_phys_to_dma call here. This is one of the reasons I
> don't think we can use dma_direct_map_resource() to implement
> map_resource
>
>
>> + BUG_ON(dir == DMA_NONE);
>> +
>> + if (!dma_capable(dev, dev_addr, size, false))
>> + return DMA_MAPPING_ERROR;
>
> But here you also need to check that phys+size doesn't cross a page
> boundary. You need to call range_straddles_page_boundary, like we do at
> the beginning of xen_swiotlb_map_page.
>
> If it is possible to cross a page boundary, then we need to basically to
> do the same thing here as we do in xen_swiotlb_map_page where we check
> for:
>
> if (dma_capable(dev, dev_addr, size, true) &&
> !range_straddles_page_boundary(phys, size) &&
> !xen_arch_need_swiotlb(dev, phys, dev_addr) &&
> !is_swiotlb_force_bounce(dev))
> goto done;
>
> if all is well, we go with the native path, otherwise we bounce on
> swiotlb-xen.
>
I'll preface this with that it's my first deep dive in DMA, so the
following may entirely be me being stupid:
I'm not sure a straddles page boundary path makes sense.
This mapping is not for a RAM backed address. In the eDMA case for the
iMX8QXP the `phys` coming in here is the address of a register. I cannot
see how a swiotlb bounce would fix anything if you end up straddling a
page boundary. At most I can see a WARN_ON with a DMA_MAPPING_ERROR
return if such a check would yield true.
Let's say you want to do a DEV_TO_MEM and a check decides you need to
bounce it you'd bounce an RX register address. I cannot see how that DMA
would ever end up doing the expected thing.
The eDMA engine will manage the RX/TX registers for an IP block and move
the data between them and RAM, where the RAM memory is mapped separately
by dma_map_page (and family). And these can be swiotlb bounced via the
map page callback with no problem.
Best regards // John Ernberg
On Tue, 6 May 2025, John Ernberg wrote:
> Hi Stefano,
>
> On 5/2/25 7:20 PM, Stefano Stabellini wrote:
> > +Christoph
> >
> > On Fri, 2 May 2025, John Ernberg wrote:
> >> Needed by the eDMA v3 DMA engine found in iommu-less SoCs such as iMX8QXP
> >> to be able to perform DMA operations as a Xen Hardware Domain, which needs
> >> to be able to do DMA in MMIO space.
>
> Self-note: The above part of the commit message is a disaster and should
> read something like.
>
> Needed by SoCs without an iommu (such as the iMX8QXP and it's eDMA v3
> engine) that need to be able to perform DMA operations in MMIO space.
>
> >>
> >> The callback implementation is basically the same as the one for direct
> >> mapping of resources, except this also takes into account Xen page
> >> mappings.
> >>
> >> There is nothing to do for unmap, just like with direct, so the unmap
> >> callback is not needed.
> >>
> >> Signed-off-by: John Ernberg <john.ernberg@actia.se>
> >>
> >> ---
> >>
> >> I originally exported dma_direct_map_resource() and used that which
> >> appeared to work, but it felt like not checking Xen page mappings wasn't
> >> fully correct and I went with this. But if dma_direct_map_resource() is
> >> the correct approach here then I can submit that isntead.
> >> ---
> >> drivers/xen/swiotlb-xen.c | 15 +++++++++++++++
> >> 1 file changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> >> index ef56a2500ed6..0f02fdd9128d 100644
> >> --- a/drivers/xen/swiotlb-xen.c
> >> +++ b/drivers/xen/swiotlb-xen.c
> >> @@ -285,6 +285,20 @@ static void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> >> attrs, pool);
> >> }
> >>
> >> +static dma_addr_t xen_swiotlb_map_resource(struct device *dev, phys_addr_t phys,
> >> + size_t size, enum dma_data_direction dir,
> >> + unsigned long attrs)
> >> +{
> >> + dma_addr_t dev_addr = xen_phys_to_dma(dev, phys);
> >
> > Yes, we need the xen_phys_to_dma call here. This is one of the reasons I
> > don't think we can use dma_direct_map_resource() to implement
> > map_resource
> >
> >
> >> + BUG_ON(dir == DMA_NONE);
> >> +
> >> + if (!dma_capable(dev, dev_addr, size, false))
> >> + return DMA_MAPPING_ERROR;
> >
> > But here you also need to check that phys+size doesn't cross a page
> > boundary. You need to call range_straddles_page_boundary, like we do at
> > the beginning of xen_swiotlb_map_page.
> >
> > If it is possible to cross a page boundary, then we need to basically to
> > do the same thing here as we do in xen_swiotlb_map_page where we check
> > for:
> >
> > if (dma_capable(dev, dev_addr, size, true) &&
> > !range_straddles_page_boundary(phys, size) &&
> > !xen_arch_need_swiotlb(dev, phys, dev_addr) &&
> > !is_swiotlb_force_bounce(dev))
> > goto done;
> >
> > if all is well, we go with the native path, otherwise we bounce on
> > swiotlb-xen.
> >
>
> I'll preface this with that it's my first deep dive in DMA, so the
> following may entirely be me being stupid:
>
> I'm not sure a straddles page boundary path makes sense.
>
> This mapping is not for a RAM backed address. In the eDMA case for the
> iMX8QXP the `phys` coming in here is the address of a register.
Ok, this information is important :-)
I am not certain whether the map_resource interface can only be called
for MMIO addresses or if it can also be called for RAM-backed addresses
with a size > PAGE_SIZE. In the latter case, we could run into the issue
I was describing.
> I cannot see how a swiotlb bounce would fix anything if you end up
> straddling a page boundary. At most I can see a WARN_ON with a
> DMA_MAPPING_ERROR return if such a check would yield true.
>
> Let's say you want to do a DEV_TO_MEM and a check decides you need to
> bounce it you'd bounce an RX register address. I cannot see how that DMA
> would ever end up doing the expected thing.
>
> The eDMA engine will manage the RX/TX registers for an IP block and move
> the data between them and RAM, where the RAM memory is mapped separately
> by dma_map_page (and family). And these can be swiotlb bounced via the
> map page callback with no problem.
OK thanks for the explanation. Like I wrote above, if we are guaranteed
that map_resource cannot be called for RAM-backed addresses or size is
less than PAGE_SIZE, then your patch is good as-is. If there are no such
guarantees, then we'll have to think a bit more about it.
On Wed, May 07, 2025 at 04:09:15PM -0700, Stefano Stabellini wrote: > > This mapping is not for a RAM backed address. In the eDMA case for the > > iMX8QXP the `phys` coming in here is the address of a register. > > Ok, this information is important :-) > > I am not certain whether the map_resource interface can only be called > for MMIO addresses or if it can also be called for RAM-backed addresses > with a size > PAGE_SIZE. In the latter case, we could run into the issue > I was describing. map_resource is intended for MMIO regions, although those could be > PAGE_SIZE. It must not be called on RAM.
On Wed, 7 May 2025, Christoph Hellwig wrote: > On Wed, May 07, 2025 at 04:09:15PM -0700, Stefano Stabellini wrote: > > > This mapping is not for a RAM backed address. In the eDMA case for the > > > iMX8QXP the `phys` coming in here is the address of a register. > > > > Ok, this information is important :-) > > > > I am not certain whether the map_resource interface can only be called > > for MMIO addresses or if it can also be called for RAM-backed addresses > > with a size > PAGE_SIZE. In the latter case, we could run into the issue > > I was describing. > > map_resource is intended for MMIO regions, although those could be > > PAGE_SIZE. It must not be called on RAM. In that case, John, you can just use dma_direct_map_resource(). That's because MMIO regions: - are 1:1 mapped on ARM - are 1:1 mapped on x86 for PV Dom0 - might not be 1:1 mapped on x86 for PVH Dom0, but in this case we rely on the IOMMU to do address translation In none of these cases xen_phys_to_dma would give us any interesting results. It would be the same as calling phys_to_dma.
Hi Stefano, On 5/9/25 1:14 AM, Stefano Stabellini wrote: > On Wed, 7 May 2025, Christoph Hellwig wrote: >> On Wed, May 07, 2025 at 04:09:15PM -0700, Stefano Stabellini wrote: >>>> This mapping is not for a RAM backed address. In the eDMA case for the >>>> iMX8QXP the `phys` coming in here is the address of a register. >>> >>> Ok, this information is important :-) >>> >>> I am not certain whether the map_resource interface can only be called >>> for MMIO addresses or if it can also be called for RAM-backed addresses >>> with a size > PAGE_SIZE. In the latter case, we could run into the issue >>> I was describing. >> >> map_resource is intended for MMIO regions, although those could be > >> PAGE_SIZE. It must not be called on RAM. > > In that case, John, you can just use dma_direct_map_resource(). > > That's because MMIO regions: > - are 1:1 mapped on ARM > - are 1:1 mapped on x86 for PV Dom0 > - might not be 1:1 mapped on x86 for PVH Dom0, but in this case we rely > on the IOMMU to do address translation > > In none of these cases xen_phys_to_dma would give us any interesting > results. It would be the same as calling phys_to_dma. Thank you for explaining. I will spin a V2 incorporating this. Thanks! // John Ernberg
© 2016 - 2025 Red Hat, Inc.