Re: [PATCH 7/9] xen/x86: rename cache_flush_permitted() to has_arch_io_resources()

[Jan Beulich <jbeulich@xxxxxxxx>]

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