Re: [PATCH RFC] xen/swiotlb: avoid arch_sync_dma_* on per-device DMA memory

[Peng Fan <peng.fan@xxxxxxxxxxx>]

Hi Jason,

On Mon, Apr 20, 2026 at 08:01:37PM -0300, Jason Gunthorpe wrote:
>On Wed, Apr 15, 2026 at 11:08:36PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@xxxxxxx>
>> 
>> On ARM64, arch_sync_dma_for_{cpu,device}() assumes that the
>> physical address passed in refers to normal RAM that is part of the
>> kernel linear(direct) mapping, as it unconditionally derives a CPU
>> virtual address via phys_to_virt().
>> 
>> With Xen swiotlb, devices may use per-device coherent DMA memory,
>> such as reserved-memory regions described by 'shared-dma-pool',
>> which are assigned to dev->dma_mem. These regions may be marked
>> no-map in DT and therefore are not part of the kernel linear map.
>> In such cases, pfn_valid() still returns true, but phys_to_virt()
>> is not valid and cache maintenance via arch_sync_dma_* will fault.
>> 
>> Prevent this by excluding devices with a private DMA memory pool
>> (dev->dma_mem) from the arch_sync_dma_* fast path, and always
>> fall back to xen_dma_sync_* for those devices to avoid invalid
>> phys_to_virt() conversions for no-map DMA memory while preserving the
>> existing fast path for normal, linear-mapped RAM.
>
>I think this is the same sort of weirdness the other two CC threads are
>dealing with.. We already have two different flags indicating the
>cache flush should be skipped, it would make more sense to have the
>swiotlb mangle the flags, just like for cc.
>
>https://lore.kernel.org/r/20260420061415.3650870-1-aneesh.kumar@xxxxxxxxxx
>
>Then you know that the swiotlb was used and it should flow down to
>here.

Xen fully implements dev->dma_ops and does not leak hypervisor-specific
semantics outside of it.

The issue is that the existing DMA attribute model only distinguishes
between "CPU sync required" and "no sync required at all". Xen needs
a third case: CPU cache sync must be skipped, but platform-level DMA
synchronization remains mandatory.

This is not a generic DMA extension but a constraint of Xen's DMA
model when mapping private or foreign memory that is not CPU-mapped.

>
>>   * physical address to use is returned.
>> @@ -262,7 +267,8 @@ static dma_addr_t xen_swiotlb_map_phys(struct device 
>> *dev, phys_addr_t phys,
>>  
>>  done:
>>      if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
>> -            if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dev_addr)))) {
>> +            if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dev_addr))) &&
>> +                !dev_has_private_dma_pool(dev)) {
>
>Also this pfn_valid() is totally bogus. Unless DMA_ATTR_MMIO the phys
>must have a struct page, be pfn_valid, etc.
>
>This is why you are getting into trouble here, beacuse swiotlb created
>a non-struct page address and passed it to lower layers without
>setting something like DMA_ATTR_MMIO..

See above.
xen swiotlb maybe a bit misleading, it is not kind of linux swiotlb.

Juergen, Stefano,

Please help correct if I am wrong or I may miss something.

Thanks,
Peng

>
>Jason
>