On 06.05.2025 10:31, Roger Pau Monne wrote: > To better describe the underlying implementation. Define > cache_flush_permitted() as an alias of has_arch_io_resources(), so that > current users of cache_flush_permitted() are not effectively modified. > > With the introduction of the new handler, change some of the call sites of > cache_flush_permitted() to instead use has_arch_io_resources() as such > callers are not after whether cache flush is enabled, but rather whether > the domain has any IO resources assigned. > > Take the opportunity to adjust l1_disallow_mask() to use the newly > introduced has_arch_io_resources() macro.
While I'm happy with everything else here, to me it's at least on the edge whether cache_flush_permitted() wouldn't be the better predicate to use there, for this being about ... > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -172,8 +172,7 @@ static DEFINE_SPINLOCK(subpage_ro_lock); > > #define l1_disallow_mask(d) \ > (((d) != dom_io) && \ > - (rangeset_is_empty((d)->iomem_caps) && \ > - rangeset_is_empty((d)->arch.ioport_caps) && \ > + (!has_arch_io_resources(d) && \ > !has_arch_pdevs(d) && \ > is_pv_domain(d)) ? \ > L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS)) ... cachability, which goes hand in hand with the ability to also flush cache contents. Tangentially - is it plausible for has_arch_io_resources() to return false when has_arch_pdevs() returns true? Perhaps there are exotic PCI devices (but non-bridges) which work with no BARs at all ... Jan
