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.coop...@citrix.com> > >>> Signed-off-by: Jane Malalane <jane.malal...@citrix.com> > >>> --- > >>> 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.