On 11/6/23 04:26, Jan Beulich wrote:
> 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".
OK, I'll drop it
>
>> +/* 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?
This patch is primarily dealing with Arm, so I'm considering simply making it
return false for now:
#define arch_needs_vpci(d) ({ (void)(d); false; })
If it needs to be changed in the future when we enable vPCI for PVH domUs, we
can deal with it at that time.
>
>> --- 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?
Yes, agreed, I'll leave has_vpci alone
>
> Jan
