Re: [PATCH 2/2] x86/xen: Allow per-domain usage of hardware virtualized APIC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.01.2022 17:05, Anthony PERARD wrote:
> On Thu, Jan 27, 2022 at 04:01:33PM +0000, Jane Malalane wrote:
>> --- a/tools/libs/light/libxl_x86.c
>> +++ b/tools/libs/light/libxl_x86.c
>> @@ -819,11 +825,44 @@ void 
>> libxl__arch_domain_create_info_setdefault(libxl__gc *gc,
>>  {
>>  }
>>  
>> -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>> -                                              libxl_domain_build_info 
>> *b_info)
>> +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>> +                                             libxl_domain_build_info 
>> *b_info,
>> +                                             const libxl_physinfo *physinfo)
>>  {
>> +    int rc;
>> +    bool assisted_xapic;
>> +    bool assisted_x2apic;
>> +
>>      libxl_defbool_setdefault(&b_info->acpi, true);
>>      libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false);
>> +
>> +    libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, false);
>> +    libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, false);
>> +
>> +    assisted_xapic = libxl_defbool_val(b_info->arch_x86.assisted_xapic);
>> +    assisted_x2apic = libxl_defbool_val(b_info->arch_x86.assisted_x2apic);
>> +
>> +    if ((assisted_xapic || assisted_x2apic) &&
>> +        b_info->type == LIBXL_DOMAIN_TYPE_PV)
>> +    {
>> +        LOG(ERROR, "Interrupt Controller Virtualization not supported for 
>> PV");
>> +        rc = ERROR_INVAL;
>> +        goto out;
>> +    }
>> +
>> +    if ((assisted_xapic && !physinfo->cap_assisted_xapic) ||
>> +         (assisted_x2apic && !physinfo->cap_assisted_x2apic))
>> +    {
>> +        LOG(ERROR, "x%sAPIC hardware supported emulation not available",
>> +            assisted_xapic && !physinfo->cap_assisted_xapic ? "" : "2");
>> +        rc =  ERROR_INVAL;
>> +        goto out;
>> +    }
> 
> Would it make sens to enable assisted_xapic and assisted_x2apic by
> default based on hardware support? That way users of libxl could benefit
> from the upgrade without having to enable it.

I think that's the only sensible way - disabling by default would result
in perceived performance regressions, I suppose.

Jan