On 02.11.2023 20:59, Stewart Hildebrand wrote:
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -503,6 +503,15 @@ struct arch_domain
> #define has_vpit(d) (!!((d)->arch.emulation_flags & X86_EMU_PIT))
> #define has_pirq(d) (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
>
> +#define is_pvh_domain(d) ({ \
> + unsigned int _emflags = (d)->arch.emulation_flags; \
> + IS_ENABLED(CONFIG_HVM) && \
> + ((_emflags == X86_EMU_LAPIC) || \
> + (_emflags == (X86_EMU_LAPIC | X86_EMU_IOAPIC))); })
I'm not convinced we want to re-introduce such a predicate; it'll be at
risk of going stale when the boundary between HVM and PVH becomes more
"fuzzy".
> +/* PCI passthrough may be backed by qemu for non-PVH domains */
> +#define arch_needs_vpci(d) is_pvh_domain(d)
Wouldn't we want to check for exactly what the comment alludes to then,
i.e. whether the domain has any (specific?) device model attached?
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -51,8 +51,17 @@ void arch_get_domain_info(const struct domain *d,
>
> #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
>
> -#define has_vpci(d) (((d)->options & XEN_DOMCTL_CDF_vpci) && \
> - IS_ENABLED(CONFIG_HAS_VPCI))
> +#define has_vpci(d) ({
> \
> + const struct domain *_d = (d);
> \
> + bool _has_vpci = false;
> \
> + if ( (_d->options & XEN_DOMCTL_CDF_vpci) && IS_ENABLED(CONFIG_HAS_VPCI)
> ) \
> + {
> \
> + if ( is_hardware_domain(_d) )
> \
> + _has_vpci = true;
> \
> + else if ( IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )
> \
> + _has_vpci = true;
> \
> + }
> \
> + _has_vpci; })
This is a commonly executed check, and as such wants to remain as simple as
possible. Wouldn't it be better anyway to prevent XEN_DOMCTL_CDF_vpci getting
set for a domain which cannot possibly have vPCI?
Jan
