We don't need to invalidate caches here; all we're after is that earlier
writes have made it to main memory (and aiui even that just in case).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This, aiui, being an analogue to uses of iommu_sync_cache() (just not
range restricted), I wonder whether it shouldn't be conditional upon
iommu_non_coherent. Then again I'm vaguely under the impression that
we had been here before, possibly even as far as questioning the need
for this call altogether.
---
v2: FLUSH_WRITEBACK -> FLUSH_CACHE_WRITEBACK.

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -693,7 +693,7 @@ static int __must_check iommu_flush_all(
     bool_t flush_dev_iotlb;
     int rc = 0;
 
-    flush_local(FLUSH_CACHE);
+    flush_local(FLUSH_CACHE_WRITEBACK);
 
     for_each_drhd_unit ( drhd )
     {

On Wed, May 03, 2023 at 11:46:11AM +0200, Jan Beulich wrote:
> We don't need to invalidate caches here; all we're after is that earlier
> writes have made it to main memory (and aiui even that just in case).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I wouldn't mind if you gated the flush to iommu_non_coherent, but
that's not very relevant if we plan to remove the call anyway.

Thanks, Roger.

On 14.05.2025 16:44, Roger Pau Monné wrote:
> On Wed, May 03, 2023 at 11:46:11AM +0200, Jan Beulich wrote:
>> We don't need to invalidate caches here; all we're after is that earlier
>> writes have made it to main memory (and aiui even that just in case).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> I wouldn't mind if you gated the flush to iommu_non_coherent, but
> that's not very relevant if we plan to remove the call anyway.

Actually I think I'll do that. Then for a fair number of systems it
won't matter anymore whether we remove this call sooner vs later.

Jan

On Wed, May 03, 2023 at 11:46:11AM +0200, Jan Beulich wrote:
> We don't need to invalidate caches here; all we're after is that earlier
> writes have made it to main memory (and aiui even that just in case).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This, aiui, being an analogue to uses of iommu_sync_cache() (just not
> range restricted), I wonder whether it shouldn't be conditional upon
> iommu_non_coherent. Then again I'm vaguely under the impression that
> we had been here before, possibly even as far as questioning the need
> for this call altogether.

I think yes, it would better be only done for iommu_non_coherent.  Yet
in that case I wonder why we need this wide flush.  In principle all
accesses should already have their own write-back calls if the IOMMU
is non-coherent?

There's maybe the call from vtd_crash_shutdown() which I guess could
trigger in the middle of some interaction with the IOMMU, but at that
point do we really care to flush anyway if Xen is going to crash?

Otherwise it seems fine to switch to write-back.

Thanks, Roger.

On 14.05.2025 13:40, Roger Pau Monné wrote:
> On Wed, May 03, 2023 at 11:46:11AM +0200, Jan Beulich wrote:
>> We don't need to invalidate caches here; all we're after is that earlier
>> writes have made it to main memory (and aiui even that just in case).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> This, aiui, being an analogue to uses of iommu_sync_cache() (just not
>> range restricted), I wonder whether it shouldn't be conditional upon
>> iommu_non_coherent. Then again I'm vaguely under the impression that
>> we had been here before, possibly even as far as questioning the need
>> for this call altogether.
> 
> I think yes, it would better be only done for iommu_non_coherent.  Yet
> in that case I wonder why we need this wide flush.  In principle all
> accesses should already have their own write-back calls if the IOMMU
> is non-coherent?
> 
> There's maybe the call from vtd_crash_shutdown() which I guess could
> trigger in the middle of some interaction with the IOMMU, but at that
> point do we really care to flush anyway if Xen is going to crash?
> 
> Otherwise it seems fine to switch to write-back.

In which case why don't we do it in two steps: This patch, and then one
removing the call altogether. Just in case the latter one turns out wrong
for whatever reason.

Jan