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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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?

I assume the duplication of information will depend on what we end up
exposing with the sysctl interface, whether it's just support for
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES and
SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE or there's more to it.

Thanks, Roger.