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

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Jane Malalane <jane.malalane@citrix.com>, Wei Liu <wl@xen.org>,
	Anthony PERARD <anthony.perard@citrix.com>,
	Juergen Gross <jgross@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86
Date: Mon, 28 Feb 2022 11:59:15 +0100	[thread overview]
Message-ID: <YhyrAyxhNtNWi7NY@Air-de-Roger> (raw)
In-Reply-To: <0a31970c-c0f8-c995-ce04-67523a4a830a@suse.com>

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

Thanks, Roger.