On 10/31/23 06:56, Jan Beulich wrote:
> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct
>> xen_domctl_createdomain *config)
>> {
>> unsigned int max_vcpus;
>> unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
>> - unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu |
>> XEN_DOMCTL_CDF_vpmu);
>> + unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu |
>> XEN_DOMCTL_CDF_vpmu |
>> + XEN_DOMCTL_CDF_vpci);
>
> Is the flag (going to be, with the initial work) okay to have for Dom0
> on Arm?
Hm. Allowing/enabling vPCI for dom0 on ARM should follow or be part of the PCI
passthrough SMMU series [1]. I'll move this change to the next patch ("xen/arm:
enable vPCI for dom0") and add a note over there.
[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html
>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct
>> xen_domctl_createdomain *config)
>> return 0;
>> }
>>
>> -static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>> +static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
>> + uint32_t cdf)
>
> While apparently views differ, ./CODING_STYLE wants "unsigned int" to be
> used for the latter two arguments.
OK, I'll change both to unsigned int.
>
>> @@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d,
>> uint32_t emflags)
>> if ( is_hvm_domain(d) )
>> {
>> if ( is_hardware_domain(d) &&
>> - emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
>> + (!( cdf & XEN_DOMCTL_CDF_vpci ) ||
>
> Nit: Stray blanks inside the inner parentheses.
OK, I'll fix.
>
>> + emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
>> return false;
>> if ( !is_hardware_domain(d) &&
>> - emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
>> - emflags != X86_EMU_LAPIC )
>> + ((cdf & XEN_DOMCTL_CDF_vpci) ||
>> + (emflags != X86_EMU_ALL &&
>> + emflags != X86_EMU_LAPIC)) )
>> return false;
>> }
>> - else if ( emflags != 0 && emflags != X86_EMU_PIT )
>> + else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||
>
> Wouldn't this better be enforced in common code?
Yes, I will move it to xen/common/domain.c:sanitise_domain_config().
>
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const
>> module_t *image,
>> {
>> dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
>> ((hvm_hap_supported() && !opt_dom0_shadow) ?
>> - XEN_DOMCTL_CDF_hap : 0));
>> + XEN_DOMCTL_CDF_hap : 0) |
>> + XEN_DOMCTL_CDF_vpci);
>
> Less of a change and imo slightly neater as a result would be to simply
> put the addition on the same line where CDF_hvm already is. But as with
> many style aspects, views may differ here of course ...
I'll change it.
>
> Jan
