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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Feb 11, 2022 at 10:06:48AM +0000, Jane Malalane wrote:
> On 10/02/2022 10:03, Roger Pau Monné wrote:
> > On Mon, Feb 07, 2022 at 06:21:00PM +0000, Jane Malalane wrote:
> >> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> >> index 7ab15e07a0..4060aef1bd 100644
> >> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> >> @@ -343,6 +343,15 @@ static int vmx_init_vmcs_config(bool bsp)
> >>               MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);
> >>       }
> >>   
> >> +    /* Check whether hardware supports accelerated xapic and x2apic. */
> >> +    if ( bsp )
> >> +    {
> >> +        assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses;
> >> +        assisted_x2apic_available = (cpu_has_vmx_apic_reg_virt ||
> >> +                                     cpu_has_vmx_virtual_intr_delivery) &&
> >> +                                    cpu_has_vmx_virtualize_x2apic_mode;
> > 
> > I've been think about this, and it seems kind of asymmetric that for
> > xAPIC mode we report hw assisted support only with
> > virtualize_apic_accesses available, while for x2APIC we require
> > virtualize_x2apic_mode plus either apic_reg_virt or
> > virtual_intr_delivery.
> > 
> > I think we likely need to be more consistent here, and report hw
> > assisted x2APIC support as long as virtualize_x2apic_mode is
> > available.
> > 
> > This will likely have some effect on patch 2 also, as you will have to
> > adjust vmx_vlapic_msr_changed.
> > 
> > Thanks, Roger.
> 
> Any other thoughts on this? As on one hand it is asymmetric but also 
> there isn't much assistance with only virtualize_x2apic_mode set as, in 
> this case, a VM exit will be avoided only when trying to access the TPR 
> register.

I've been thinking about this, and reporting hardware assisted
x{2}APIC virtualization with just
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES or
SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE doesn't seem very helpful. While
those provide some assistance to the VMM in order to handle APIC
accesses, it will still require a trap into the hypervisor to handle
most of the accesses.

So maybe we should only report hardware assisted support when the
mentioned features are present together with
SECONDARY_EXEC_APIC_REGISTER_VIRT?

Thanks, Roger.