On 19/04/2023 11:44 am, Jan Beulich wrote: > --- a/xen/arch/x86/flushtlb.c > +++ b/xen/arch/x86/flushtlb.c > @@ -232,7 +232,7 @@ unsigned int flush_area_local(const void > if ( flags & FLUSH_HVM_ASID_CORE ) > hvm_flush_guest_tlbs(); > > - if ( flags & FLUSH_CACHE ) > + if ( flags & (FLUSH_CACHE | FLUSH_WRITEBACK) )
Given that we already have FLUSH_CACHE, then adding writeback also seems fine, but we need to get the naming corrected first. We've got a file called flushtlb.c which flushes more than the TLBs now, and various APIs in it. We have a bunch of ARM specific APIs which AFAICT exist purely to prop up the ARM-specific gnttab_cache_flush(). That needs to go and live behind an ifdef and stop polluting other architectures with an incredibly short sighted hypercall interface decision. The "area" in the low level calls isn't good. Range might be better, but I'm not sure. The "mask" part of the name would be better as "some" or perhaps "others", to be a better counterpoint to "local". Some of the wrappers too really ought to be dropped - there are lots of them, and too few users to justify. But on to the main thing which caught my eye... The FLUSH in FLUSH_CACHE means the flush infrastructure, not "cache flushing", and FLUSH_WRITEBACK is nonsensical next to this. All other things we flush have a qualification that makes them clear in context. (other than the assist one which I'm going to time out objections to and revert back to name which made more sense.) At an absolutely minimum, FLUSH_CACHE first needs renaming to FLUSH_CACHE_EVICT and then this new one you're adding needs to be FLUSH_CACHE_WRITEBACK. Except... Is there any credible reason to have EVICT as an option by the end of this cleanup? CLDEMOTE does exist for a reason (reducing coherency traffic overhead when you know the consumer is on a different CPU), but it would be totally bogus to use this in an all or mask form, and you wouldn't want to use it in local form either, simply from an overhead point of view. We have evict behaviour simply because `clflush` was the only game in town for decades, not because evicting the cacheline is what you want actually want to do. ~Andrew
