Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Christian König]

On 8/12/26 01:13, Taimuraz Kaitmazov wrote:
> SYNC_BO carries an offset and a size, but amdxdna_flush_bo() honours them
> only on the vmap path. An imported BO is tested for first and flushes its
> whole scatterlist,

Absolutely clear NAK to that from a DMA-buf maintainer side.

Flushing on imported scatterlist of a DMA-buf is a really big NO-GO.

If DMA-buf imports are used with the device then the device needs to be able to coherently access the underlying memory.

In other words you *CAN'T* call drm_clflush_pages() on imported memory.

Regards,
Christian.

> so a sync costs what the BO is worth rather than what
> the caller asked to maintain: on npu4 an imported 64 MiB BO cost 1056 us
> to sync at every size from 4 KiB up. Patch 5 reorders the arms so the
> vmap path is tried first, and indexes the page-array fallback from the
> requested offset.
> 
> The four before it are the ground that has to be solid first. Patch 1
> refuses an I/O memory mapping, which the driver currently stores as if it
> were an ordinary kernel address. Patch 2 adds a probe that does not log,
> so patch 5 does not make an exporter without a vmap op print on every
> ioctl. Patches 3 and 4 fix two ways the ioctl mishandles its own range: a
> zero length reaching drm_clflush_virt_range(), and an offset and size
> added to the BO address without an overflow check, one level above a
> function that checks the same arithmetic. All four stand on their own and
> can be taken separately; only patch 5 depends on them.
> 
> v1 did not reach dri-devel, so this is the first version visible there.
> It is on lore via the other lists it was copied to:
> https://lore.kernel.org/lkml/20260811204556.875037-1-taimuraz@kaitmazov.com/
> 
> Changes in v2:
>  - patch 2: take the device from the GEM object rather than abo->client.
>    amdxdna_gem_obj_close() clears that pointer under abo->lock, which the
>    pre-split code held across the log and the split did not.
>  - new patch 3: return early from a zero-length flush.
>  - new patch 4: check the sync range for overflow on a device BO.
>  - patch 5: say why the persistent mapping adds no pin.
> 
> The measurements in patch 5 were taken with the equivalent change in
> AMD's out-of-tree xdna-driver, where this merged as #1541. That version
> and this one differ only in a page-array fallback mainline has no field
> for, reached when the mapping fails and the BO is neither imported nor
> shmem backed, and in the name of the mapping helper. The flush and the
> helper are otherwise identical. This version is compile-tested; it has
> not been booted.
> 
> Patch 1 is from inspection rather than a reproducer. The exporter I can
> test against is amdgpu, and amdgpu is the case that cannot reach it: it
> implements .pin, so a non peer to peer attachment like this driver's
> forces the buffer to GTT before anything maps it. Reproducing it needs a
> GPU whose exporter has no .pin, which I do not have paired with an NPU
> here.
> 
> Taimuraz Kaitmazov (5):
>   accel/amdxdna: refuse an I/O memory mapping of an imported BO
>   accel/amdxdna: add a quiet variant of amdxdna_gem_vmap()
>   accel/amdxdna: return early from a zero-length flush
>   accel/amdxdna: check the sync range for overflow on a device BO
>   accel/amdxdna: flush only the requested range in amdxdna_flush_bo
> 
>  drivers/accel/amdxdna/amdxdna_gem.c | 66 +++++++++++++++++++++--------
>  1 file changed, 49 insertions(+), 17 deletions(-)
> 
> --
> 2.55.0
> 

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Lizhi Hou]

Hi Christian,

Thanks for pointing this out.

Taimuraz, this is not introduced by your patch. And the current code 
violates dma-buf protocol. It look we need to unconditionally return 
-EOPNOTSUPP for imported BO at the beginning of amdxdna_flush_bo(). 
Could you help to modify your patch 1 for this if it makes sense?


Thanks,

Lizhi

On 8/12/26 01:57, Christian König wrote:
> On 8/12/26 01:13, Taimuraz Kaitmazov wrote:
>> SYNC_BO carries an offset and a size, but amdxdna_flush_bo() honours them
>> only on the vmap path. An imported BO is tested for first and flushes its
>> whole scatterlist,
> Absolutely clear NAK to that from a DMA-buf maintainer side.
>
> Flushing on imported scatterlist of a DMA-buf is a really big NO-GO.
>
> If DMA-buf imports are used with the device then the device needs to be able to coherently access the underlying memory.
>
> In other words you *CAN'T* call drm_clflush_pages() on imported memory.
>
> Regards,
> Christian.
>
>> so a sync costs what the BO is worth rather than what
>> the caller asked to maintain: on npu4 an imported 64 MiB BO cost 1056 us
>> to sync at every size from 4 KiB up. Patch 5 reorders the arms so the
>> vmap path is tried first, and indexes the page-array fallback from the
>> requested offset.
>>
>> The four before it are the ground that has to be solid first. Patch 1
>> refuses an I/O memory mapping, which the driver currently stores as if it
>> were an ordinary kernel address. Patch 2 adds a probe that does not log,
>> so patch 5 does not make an exporter without a vmap op print on every
>> ioctl. Patches 3 and 4 fix two ways the ioctl mishandles its own range: a
>> zero length reaching drm_clflush_virt_range(), and an offset and size
>> added to the BO address without an overflow check, one level above a
>> function that checks the same arithmetic. All four stand on their own and
>> can be taken separately; only patch 5 depends on them.
>>
>> v1 did not reach dri-devel, so this is the first version visible there.
>> It is on lore via the other lists it was copied to:
>> https://lore.kernel.org/lkml/20260811204556.875037-1-taimuraz@kaitmazov.com/
>>
>> Changes in v2:
>>   - patch 2: take the device from the GEM object rather than abo->client.
>>     amdxdna_gem_obj_close() clears that pointer under abo->lock, which the
>>     pre-split code held across the log and the split did not.
>>   - new patch 3: return early from a zero-length flush.
>>   - new patch 4: check the sync range for overflow on a device BO.
>>   - patch 5: say why the persistent mapping adds no pin.
>>
>> The measurements in patch 5 were taken with the equivalent change in
>> AMD's out-of-tree xdna-driver, where this merged as #1541. That version
>> and this one differ only in a page-array fallback mainline has no field
>> for, reached when the mapping fails and the BO is neither imported nor
>> shmem backed, and in the name of the mapping helper. The flush and the
>> helper are otherwise identical. This version is compile-tested; it has
>> not been booted.
>>
>> Patch 1 is from inspection rather than a reproducer. The exporter I can
>> test against is amdgpu, and amdgpu is the case that cannot reach it: it
>> implements .pin, so a non peer to peer attachment like this driver's
>> forces the buffer to GTT before anything maps it. Reproducing it needs a
>> GPU whose exporter has no .pin, which I do not have paired with an NPU
>> here.
>>
>> Taimuraz Kaitmazov (5):
>>    accel/amdxdna: refuse an I/O memory mapping of an imported BO
>>    accel/amdxdna: add a quiet variant of amdxdna_gem_vmap()
>>    accel/amdxdna: return early from a zero-length flush
>>    accel/amdxdna: check the sync range for overflow on a device BO
>>    accel/amdxdna: flush only the requested range in amdxdna_flush_bo
>>
>>   drivers/accel/amdxdna/amdxdna_gem.c | 66 +++++++++++++++++++++--------
>>   1 file changed, 49 insertions(+), 17 deletions(-)
>>
>> --
>> 2.55.0
>>

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Christian König]

Hi Lizhi,

yeah that sounds reasonable.

An alternative would be to use DMA_BUF_IOCTL_SYNC from userspace, but that is usually only for the exporter to implement clflush or similar actions. As importer you need to be able to take the data as it is.

We have discussed before if that shouldn't be changed somehow, but so far didn't settled on an interface.

Question is why do you need clflush in the first place? The NPU is a PCIe device, isn't it? And so it should be using cache coherent memory accesses.

Regards,
Christian.

On 8/12/26 17:45, Lizhi Hou wrote:
> Hi Christian,
> 
> Thanks for pointing this out.
> 
> Taimuraz, this is not introduced by your patch. And the current code violates dma-buf protocol. It look we need to unconditionally return -EOPNOTSUPP for imported BO at the beginning of amdxdna_flush_bo(). Could you help to modify your patch 1 for this if it makes sense?
> 
> 
> Thanks,
> 
> Lizhi
> 
> On 8/12/26 01:57, Christian König wrote:
>> On 8/12/26 01:13, Taimuraz Kaitmazov wrote:
>>> SYNC_BO carries an offset and a size, but amdxdna_flush_bo() honours them
>>> only on the vmap path. An imported BO is tested for first and flushes its
>>> whole scatterlist,
>> Absolutely clear NAK to that from a DMA-buf maintainer side.
>>
>> Flushing on imported scatterlist of a DMA-buf is a really big NO-GO.
>>
>> If DMA-buf imports are used with the device then the device needs to be able to coherently access the underlying memory.
>>
>> In other words you *CAN'T* call drm_clflush_pages() on imported memory.
>>
>> Regards,
>> Christian.
>>
>>> so a sync costs what the BO is worth rather than what
>>> the caller asked to maintain: on npu4 an imported 64 MiB BO cost 1056 us
>>> to sync at every size from 4 KiB up. Patch 5 reorders the arms so the
>>> vmap path is tried first, and indexes the page-array fallback from the
>>> requested offset.
>>>
>>> The four before it are the ground that has to be solid first. Patch 1
>>> refuses an I/O memory mapping, which the driver currently stores as if it
>>> were an ordinary kernel address. Patch 2 adds a probe that does not log,
>>> so patch 5 does not make an exporter without a vmap op print on every
>>> ioctl. Patches 3 and 4 fix two ways the ioctl mishandles its own range: a
>>> zero length reaching drm_clflush_virt_range(), and an offset and size
>>> added to the BO address without an overflow check, one level above a
>>> function that checks the same arithmetic. All four stand on their own and
>>> can be taken separately; only patch 5 depends on them.
>>>
>>> v1 did not reach dri-devel, so this is the first version visible there.
>>> It is on lore via the other lists it was copied to:
>>> https://lore.kernel.org/lkml/20260811204556.875037-1-taimuraz@kaitmazov.com/
>>>
>>> Changes in v2:
>>>   - patch 2: take the device from the GEM object rather than abo->client.
>>>     amdxdna_gem_obj_close() clears that pointer under abo->lock, which the
>>>     pre-split code held across the log and the split did not.
>>>   - new patch 3: return early from a zero-length flush.
>>>   - new patch 4: check the sync range for overflow on a device BO.
>>>   - patch 5: say why the persistent mapping adds no pin.
>>>
>>> The measurements in patch 5 were taken with the equivalent change in
>>> AMD's out-of-tree xdna-driver, where this merged as #1541. That version
>>> and this one differ only in a page-array fallback mainline has no field
>>> for, reached when the mapping fails and the BO is neither imported nor
>>> shmem backed, and in the name of the mapping helper. The flush and the
>>> helper are otherwise identical. This version is compile-tested; it has
>>> not been booted.
>>>
>>> Patch 1 is from inspection rather than a reproducer. The exporter I can
>>> test against is amdgpu, and amdgpu is the case that cannot reach it: it
>>> implements .pin, so a non peer to peer attachment like this driver's
>>> forces the buffer to GTT before anything maps it. Reproducing it needs a
>>> GPU whose exporter has no .pin, which I do not have paired with an NPU
>>> here.
>>>
>>> Taimuraz Kaitmazov (5):
>>>    accel/amdxdna: refuse an I/O memory mapping of an imported BO
>>>    accel/amdxdna: add a quiet variant of amdxdna_gem_vmap()
>>>    accel/amdxdna: return early from a zero-length flush
>>>    accel/amdxdna: check the sync range for overflow on a device BO
>>>    accel/amdxdna: flush only the requested range in amdxdna_flush_bo
>>>
>>>   drivers/accel/amdxdna/amdxdna_gem.c | 66 +++++++++++++++++++++--------
>>>   1 file changed, 49 insertions(+), 17 deletions(-)
>>>
>>> -- 
>>> 2.55.0
>>>

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Lizhi Hou]

On 8/13/26 00:44, Christian König wrote:
> Hi Lizhi,
>
> yeah that sounds reasonable.
Thanks. :)
>
> An alternative would be to use DMA_BUF_IOCTL_SYNC from userspace, but that is usually only for the exporter to implement clflush or similar actions. As importer you need to be able to take the data as it is.
>
> We have discussed before if that shouldn't be changed somehow, but so far didn't settled on an interface.
>
> Question is why do you need clflush in the first place? The NPU is a PCIe device, isn't it? And so it should be using cache coherent memory accesses.

I do not know the hardware detail. The legacy NPU device is not cache 
coherent. And the next generation (aie4) devices will be cache coherent.


Lizhi

>
> Regards,
> Christian.
>
> On 8/12/26 17:45, Lizhi Hou wrote:
>> Hi Christian,
>>
>> Thanks for pointing this out.
>>
>> Taimuraz, this is not introduced by your patch. And the current code violates dma-buf protocol. It look we need to unconditionally return -EOPNOTSUPP for imported BO at the beginning of amdxdna_flush_bo(). Could you help to modify your patch 1 for this if it makes sense?
>>
>>
>> Thanks,
>>
>> Lizhi
>>
>> On 8/12/26 01:57, Christian König wrote:
>>> On 8/12/26 01:13, Taimuraz Kaitmazov wrote:
>>>> SYNC_BO carries an offset and a size, but amdxdna_flush_bo() honours them
>>>> only on the vmap path. An imported BO is tested for first and flushes its
>>>> whole scatterlist,
>>> Absolutely clear NAK to that from a DMA-buf maintainer side.
>>>
>>> Flushing on imported scatterlist of a DMA-buf is a really big NO-GO.
>>>
>>> If DMA-buf imports are used with the device then the device needs to be able to coherently access the underlying memory.
>>>
>>> In other words you *CAN'T* call drm_clflush_pages() on imported memory.
>>>
>>> Regards,
>>> Christian.
>>>
>>>> so a sync costs what the BO is worth rather than what
>>>> the caller asked to maintain: on npu4 an imported 64 MiB BO cost 1056 us
>>>> to sync at every size from 4 KiB up. Patch 5 reorders the arms so the
>>>> vmap path is tried first, and indexes the page-array fallback from the
>>>> requested offset.
>>>>
>>>> The four before it are the ground that has to be solid first. Patch 1
>>>> refuses an I/O memory mapping, which the driver currently stores as if it
>>>> were an ordinary kernel address. Patch 2 adds a probe that does not log,
>>>> so patch 5 does not make an exporter without a vmap op print on every
>>>> ioctl. Patches 3 and 4 fix two ways the ioctl mishandles its own range: a
>>>> zero length reaching drm_clflush_virt_range(), and an offset and size
>>>> added to the BO address without an overflow check, one level above a
>>>> function that checks the same arithmetic. All four stand on their own and
>>>> can be taken separately; only patch 5 depends on them.
>>>>
>>>> v1 did not reach dri-devel, so this is the first version visible there.
>>>> It is on lore via the other lists it was copied to:
>>>> https://lore.kernel.org/lkml/20260811204556.875037-1-taimuraz@kaitmazov.com/
>>>>
>>>> Changes in v2:
>>>>    - patch 2: take the device from the GEM object rather than abo->client.
>>>>      amdxdna_gem_obj_close() clears that pointer under abo->lock, which the
>>>>      pre-split code held across the log and the split did not.
>>>>    - new patch 3: return early from a zero-length flush.
>>>>    - new patch 4: check the sync range for overflow on a device BO.
>>>>    - patch 5: say why the persistent mapping adds no pin.
>>>>
>>>> The measurements in patch 5 were taken with the equivalent change in
>>>> AMD's out-of-tree xdna-driver, where this merged as #1541. That version
>>>> and this one differ only in a page-array fallback mainline has no field
>>>> for, reached when the mapping fails and the BO is neither imported nor
>>>> shmem backed, and in the name of the mapping helper. The flush and the
>>>> helper are otherwise identical. This version is compile-tested; it has
>>>> not been booted.
>>>>
>>>> Patch 1 is from inspection rather than a reproducer. The exporter I can
>>>> test against is amdgpu, and amdgpu is the case that cannot reach it: it
>>>> implements .pin, so a non peer to peer attachment like this driver's
>>>> forces the buffer to GTT before anything maps it. Reproducing it needs a
>>>> GPU whose exporter has no .pin, which I do not have paired with an NPU
>>>> here.
>>>>
>>>> Taimuraz Kaitmazov (5):
>>>>     accel/amdxdna: refuse an I/O memory mapping of an imported BO
>>>>     accel/amdxdna: add a quiet variant of amdxdna_gem_vmap()
>>>>     accel/amdxdna: return early from a zero-length flush
>>>>     accel/amdxdna: check the sync range for overflow on a device BO
>>>>     accel/amdxdna: flush only the requested range in amdxdna_flush_bo
>>>>
>>>>    drivers/accel/amdxdna/amdxdna_gem.c | 66 +++++++++++++++++++++--------
>>>>    1 file changed, 49 insertions(+), 17 deletions(-)
>>>>
>>>> -- 
>>>> 2.55.0
>>>>

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Alex Deucher]

On Thu, Aug 13, 2026 at 2:45 PM Lizhi Hou <lizhi.hou@amd.com> wrote:
>
>
> On 8/13/26 00:44, Christian König wrote:
> > Hi Lizhi,
> >
> > yeah that sounds reasonable.
> Thanks. :)
> >
> > An alternative would be to use DMA_BUF_IOCTL_SYNC from userspace, but that is usually only for the exporter to implement clflush or similar actions. As importer you need to be able to take the data as it is.
> >
> > We have discussed before if that shouldn't be changed somehow, but so far didn't settled on an interface.
> >
> > Question is why do you need clflush in the first place? The NPU is a PCIe device, isn't it? And so it should be using cache coherent memory accesses.
>
> I do not know the hardware detail. The legacy NPU device is not cache
> coherent. And the next generation (aie4) devices will be cache coherent.
>

The CPU wouldn't be coherent with device caches, but the device should
be coherent with the CPU's caches.  I.e., PCIe transactions from the
device should snoop the CPU's cache.  Otherwise, you couldn't use CPU
cached memory for DMA.

Alex

>
> Lizhi
>
> >
> > Regards,
> > Christian.
> >
> > On 8/12/26 17:45, Lizhi Hou wrote:
> >> Hi Christian,
> >>
> >> Thanks for pointing this out.
> >>
> >> Taimuraz, this is not introduced by your patch. And the current code violates dma-buf protocol. It look we need to unconditionally return -EOPNOTSUPP for imported BO at the beginning of amdxdna_flush_bo(). Could you help to modify your patch 1 for this if it makes sense?
> >>
> >>
> >> Thanks,
> >>
> >> Lizhi
> >>
> >> On 8/12/26 01:57, Christian König wrote:
> >>> On 8/12/26 01:13, Taimuraz Kaitmazov wrote:
> >>>> SYNC_BO carries an offset and a size, but amdxdna_flush_bo() honours them
> >>>> only on the vmap path. An imported BO is tested for first and flushes its
> >>>> whole scatterlist,
> >>> Absolutely clear NAK to that from a DMA-buf maintainer side.
> >>>
> >>> Flushing on imported scatterlist of a DMA-buf is a really big NO-GO.
> >>>
> >>> If DMA-buf imports are used with the device then the device needs to be able to coherently access the underlying memory.
> >>>
> >>> In other words you *CAN'T* call drm_clflush_pages() on imported memory.
> >>>
> >>> Regards,
> >>> Christian.
> >>>
> >>>> so a sync costs what the BO is worth rather than what
> >>>> the caller asked to maintain: on npu4 an imported 64 MiB BO cost 1056 us
> >>>> to sync at every size from 4 KiB up. Patch 5 reorders the arms so the
> >>>> vmap path is tried first, and indexes the page-array fallback from the
> >>>> requested offset.
> >>>>
> >>>> The four before it are the ground that has to be solid first. Patch 1
> >>>> refuses an I/O memory mapping, which the driver currently stores as if it
> >>>> were an ordinary kernel address. Patch 2 adds a probe that does not log,
> >>>> so patch 5 does not make an exporter without a vmap op print on every
> >>>> ioctl. Patches 3 and 4 fix two ways the ioctl mishandles its own range: a
> >>>> zero length reaching drm_clflush_virt_range(), and an offset and size
> >>>> added to the BO address without an overflow check, one level above a
> >>>> function that checks the same arithmetic. All four stand on their own and
> >>>> can be taken separately; only patch 5 depends on them.
> >>>>
> >>>> v1 did not reach dri-devel, so this is the first version visible there.
> >>>> It is on lore via the other lists it was copied to:
> >>>> https://lore.kernel.org/lkml/20260811204556.875037-1-taimuraz@kaitmazov.com/
> >>>>
> >>>> Changes in v2:
> >>>>    - patch 2: take the device from the GEM object rather than abo->client.
> >>>>      amdxdna_gem_obj_close() clears that pointer under abo->lock, which the
> >>>>      pre-split code held across the log and the split did not.
> >>>>    - new patch 3: return early from a zero-length flush.
> >>>>    - new patch 4: check the sync range for overflow on a device BO.
> >>>>    - patch 5: say why the persistent mapping adds no pin.
> >>>>
> >>>> The measurements in patch 5 were taken with the equivalent change in
> >>>> AMD's out-of-tree xdna-driver, where this merged as #1541. That version
> >>>> and this one differ only in a page-array fallback mainline has no field
> >>>> for, reached when the mapping fails and the BO is neither imported nor
> >>>> shmem backed, and in the name of the mapping helper. The flush and the
> >>>> helper are otherwise identical. This version is compile-tested; it has
> >>>> not been booted.
> >>>>
> >>>> Patch 1 is from inspection rather than a reproducer. The exporter I can
> >>>> test against is amdgpu, and amdgpu is the case that cannot reach it: it
> >>>> implements .pin, so a non peer to peer attachment like this driver's
> >>>> forces the buffer to GTT before anything maps it. Reproducing it needs a
> >>>> GPU whose exporter has no .pin, which I do not have paired with an NPU
> >>>> here.
> >>>>
> >>>> Taimuraz Kaitmazov (5):
> >>>>     accel/amdxdna: refuse an I/O memory mapping of an imported BO
> >>>>     accel/amdxdna: add a quiet variant of amdxdna_gem_vmap()
> >>>>     accel/amdxdna: return early from a zero-length flush
> >>>>     accel/amdxdna: check the sync range for overflow on a device BO
> >>>>     accel/amdxdna: flush only the requested range in amdxdna_flush_bo
> >>>>
> >>>>    drivers/accel/amdxdna/amdxdna_gem.c | 66 +++++++++++++++++++++--------
> >>>>    1 file changed, 49 insertions(+), 17 deletions(-)
> >>>>
> >>>> --
> >>>> 2.55.0
> >>>>

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Alex Deucher]

On Thu, Aug 13, 2026 at 3:51 PM Taimuraz Kaitmazov
<taimuraz@kaitmazov.com> wrote:
>
> (Resending to the list -- my earlier copy went to Alex alone.)
>
> On 8/13/26 21:54, Alex Deucher wrote:
> > PCIe transactions from the device should snoop the CPU's cache.
>
> That is what I assumed too, but it is not what I measure on npu4. Leaving a new input dirty in the CPU and running, the array computes on the previous contents, each out of 20 runs. Flushing that same input and running again, it sees it - so the data does reach the buffer, and something is not picking up the dirty line.
>
> What I cannot square is that ubuf maps with dma_map_sgtable() and nothing else, and on x86 the DMA API no-ops the syncs, so that path looks correct only because SYNC_BO clflushes on top of it.
>
> Is the array expected to snoop on this part? Happy to run whatever would settle it, or to be told what I am measuring wrong.

The snoop should be part of the DMA from the device.  By default PCI
devices are supposed to snoop CPU caches, and there is the option of
doing non-snooped transactions if the platform supports it.  It
certainly sounds like these are non-snooped from what you've said.  I
wonder if there is some device config option to enable snooped vs.
non-snooped DMAs?  If not, then either you'd need to allocate
non-cached memory or you'll need to flush the caches appropriately.

Alex

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Lizhi Hou]

On 8/13/26 13:14, Taimuraz Kaitmazov wrote:
> On 8/13/26 23:06, Alex Deucher wrote:
>> I wonder if there is some device config option to enable snooped vs.
>> non-snooped DMAs?
> I tried the PCIe one. Clearing Enable No Snoop in DevCtl, with lspci then showing NoSnoop-, changes nothing: same 20 out of 20 stale both ways. So the array either isn't carrying the attribute or isn't looking at it.

Yes. It is known that the legacy hardware has problem to support snoop. 
In your case, the XRT's buffer::sync() could be used.

Lizhi

>
> Taimuraz
>
>

Re: [PATCH v2 0/5] accel/amdxdna: honour the SYNC_BO range

[Lizhi Hou]

On 8/13/26 11:29, Taimuraz Kaitmazov wrote:
> Resending: my earlier reply does not appear on the lists, so I assume it
> did not reach you either.
>
> On 8/13/26 20:08, Lizhi Hou wrote:
>> The legacy NPU device is not cache coherent. And the next generation
>> (aie4) devices will be cache coherent.
> Confirming that with numbers, since I had measured it before your reply
> landed. On npu4, without a flush the CPU reads what the buffer held
> before the NPU wrote it, and the NPU reads what DRAM held before the CPU
> wrote it. Both reproduce on all 20 runs, and the stale read is most of
> the buffer, not a stray line: 3931 of 4096 values on average.
>
> Good to know aie4 is coherent -- that makes anything we do here a
> legacy-only concern.
>
> On 8/13/26 09:44, Christian König wrote:
>> An alternative would be to use DMA_BUF_IOCTL_SYNC from userspace
> Tried it against amdgpu, imported into amdxdna: stale on all 20 runs, no
> better than no sync at all. SYNC_BO on the same buffer is clean on all
> 20.
>
> Which leaves me no legal way to import a buffer the CPU also reads. Is
> there one I'm missing, or should a device like this just not import?
>
> On 8/12/26 17:45, Lizhi Hou wrote:
>> we need to unconditionally return -EOPNOTSUPP for imported BO
> is_import_bo() also covers ubuf and cbuf, so that stops maintaining our
> own userptr and carve-out BOs too. They take that arm today: on a 64 MiB
> userptr BO a 4 KiB sync and a full sync both cost 659 us, so the range
> is already being ignored there.

I am working on removing the dma-buf part for ubuf BO because that is 
also not a good usage of dma-buf. So the ubuf will be a object soon. 
cbuf is mainly for debug and is disabled by default.


Lizhi

>
> Keying on dma_buf->ops instead would confine it to foreign buffers.
> Either is fine by me, tell me which you want.
>
> Separately: XRT's buffer::sync() clflushes in userspace unless
> Debug.force_driver_sync is set, so the stack does this to foreign
> dma-bufs whatever the driver does. And when the ioctl is used, a
> FROM_DEVICE sync returns -EINVAL after the flush has already run, out of
> amdxdna_hwctx_sync_debug_bo() when the BO has no assigned hwctx. Happy
> to send that as its own patch; the helper has one caller, so returning 0
> there is the obvious shape unless you want it done elsewhere.
>
> v3 is sent: patches 1, 3 and 4 only. Patch 5 is dropped, and 2 with it
> since it only serves 5. I have not tested 5 on a matching tree and its
> numbers came from the foreign import case.
>
> Taimuraz
>
>