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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.02.2022 16:36, Roger Pau Monné wrote:
> On Mon, Feb 28, 2022 at 02:11:04PM +0100, Jan Beulich wrote:
>> 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.
> 
> So you would rather prefer to expose this information in a synthetic
> CPUID leaf?

Depends on what you mean by "synthetic leaf". We already have a leaf.
What I'm suggesting to consider to the give that hypervisor leaf a
representation in the featureset.

Jan