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

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

On Thu, Apr 07, 2022 at 10:45:10AM +0200, Jan Beulich wrote:
> On 07.04.2022 10:28, Roger Pau Monné wrote:
> > On Fri, Apr 01, 2022 at 11:47:12AM +0100, Jane Malalane wrote:
> >> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> >> @@ -2145,6 +2145,12 @@ int __init vmx_vmcs_init(void)
> >>  
> >>      ret = _vmx_cpu_up(true);
> >>  
> >> +    /* Check whether hardware supports accelerated xapic and x2apic. */
> >> +    assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses;
> >> +    assisted_x2apic_available = cpu_has_vmx_virtualize_x2apic_mode &&
> >> +                                (cpu_has_vmx_apic_reg_virt ||
> >> +                                 cpu_has_vmx_virtual_intr_delivery);
> > 
> > Setting assisted_x{2}apic_available should only be done !ret, or else
> > we might be reporting those capabilities when VMX is not usable, which
> > would be misleading IMO.
> 
> Hmm, while I agree with the observation, wouldn't it be better if all
> the feature masks were cleared in case of failure, such that other
> code using the predicates wouldn't be mislead either? (That would
> likely want to be a separate, prereq change.)

Possibly, yes.

vmx_vmcs_init failing will lead to start_vmx failing and thus the
hvm_function table won't get setup, so I think we got away without
doing the cleaning because there where no code paths using it
anymore as HVM was disabled.

To not delay this series anymore it might be easier to just set
assisted_x{2}apic_available inside the !ret if where the keyhandler
also gets set.

Thanks, Roger.