Re: [PATCH v3 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.02.2022 11:59, Roger Pau Monné wrote:
> On Thu, Feb 24, 2022 at 03:08:41PM +0100, Jan Beulich wrote:
>> On 18.02.2022 18:29, Jane Malalane wrote:
>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>> and x2apic, on x86 hardware.
>>> No such features are currently implemented on AMD hardware.
>>>
>>> For that purpose, also add an arch-specific "capabilities" parameter
>>> to struct xen_sysctl_physinfo.
>>>
>>> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
>>> ---
>>> v3:
>>>  * Define XEN_SYSCTL_PHYSCAP_ARCH_MAX for ABI checking and actually
>>>    set arch_capbilities, via a call to c_bitmap_to_ocaml_list()
>>>  * Have assisted_x2apic_available only depend on
>>>    cpu_has_vmx_virtualize_x2apic_mode
>>
>> I understand this was the result from previous discussion, but this
>> needs justifying in the description. Not the least because it differs
>> from when XEN_HVM_CPUID_X2APIC_VIRT would be set as well as from what
>> vmx_vlapic_msr_changed() does. The difference between those two is
>> probably intended (judging from a comment there), but the further
>> difference to what you add isn't obvious.
>>
>> Which raises another thought: If that hypervisor leaf was part of the
>> HVM feature set, the tool stack could be able to obtain the wanted
>> information without altering sysctl (assuming the conditions to set
>> the respective bits were the same). And I would view it as generally
>> reasonable for there to be a way for tool stacks to know what
>> hypervisor leaves guests are going to get to see (at the maximum and
>> by default).
> 
> I'm not sure using CPUID would be appropriate for this. Those fields
> are supposed to be used by a guest to decide whether it should prefer
> the x{2}APIC over PV alternatives for certain operations (ie: IPIs for
> example), but the level of control we can provide with the sysctl is
> more fine grained.
> 
> The current proposal is limited to the exposure and control of the
> usage of APIC virtualization, but we could also expose availability
> and per-domain enablement of APIC register virtualization and posted
> interrupts.

But then I would still like to avoid duplication of information
exposure and expose through the featureset what can be exposed there
and limit sysctl to what cannot be expressed otherwise.

Jan