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.
