Re: [PATCH for-4.17 v2] hvm/apic: repurpose the reporting of the APIC assist options

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

On Fri, Nov 11, 2022 at 05:47:02PM +0000, Andrew Cooper wrote:
> On 11/11/2022 17:35, Andrew Cooper wrote:
> > On 11/11/2022 07:45, Jan Beulich wrote:
> >> On 10.11.2022 23:47, Andrew Cooper wrote:
> >>> On 04/11/2022 16:18, Roger Pau Monne wrote:
> >>>> --- a/xen/arch/x86/hvm/viridian/viridian.c
> >>>> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> >>>> @@ -197,7 +197,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, 
> >>>> uint32_t leaf,
> >>>>          res->a = CPUID4A_RELAX_TIMER_INT;
> >>>>          if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush )
> >>>>              res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
> >>>> -        if ( !cpu_has_vmx_apic_reg_virt )
> >>>> +        if ( !has_assisted_xapic(d) )
> >>>>              res->a |= CPUID4A_MSR_BASED_APIC;
> >>> This check is broken before and after.  It needs to be keyed on
> >>> virtualised interrupt delivery, not register acceleration.
> >> To me this connection you suggest looks entirely unobvious, so would
> >> you mind expanding as to why you're thinking so? The hint to the guest
> >> here is related to how it would best access certain registers (aiui),
> >> which to me looks orthogonal to how interrupt delivery works.
> > I refer you again to the diagram.  (For everyone else on xen-devel, I
> > put this together when fixing XSA-412 because Intel's APIC acceleration
> > controls are entirely unintuitive.)
> >
> > It is "virtual interrupt delivery" which controls EOI/ICR acceleration,
> > and not "apic register virtualisation".  There's a decade worth of
> > hardware where this logic is an anti-optimsiation, by telling windows to
> > use a VMExit-ing mechanism even when microcode would have avoided the
> > VMExit.
> >
> > It is not by accident that "virtual interrupt delivery", introduced in
> > IvyBridge, is exactly the missing registers (on top of "use TPR Shadow"
> > which is even older) to make windows performance less bad.
> 
> Sorry, sent too early.
> 
> This also firmly highlights why the API/ABI is broken. 

I'm not seeing how you are making this connection: the context here is
strictly about a Viridian hint which Xen has been wrongly reporting,
but has nothing to do with the APIC assist API/ABI stuff.  It was
wrong before the introduction of APIC assist, and it's also wrong
after.

Also see my other reply - I'm dubious whether this hint is useful for
any Windows version that supports x2APIC (which seems to be >= Windows
Server 2008), because we expose x2APIC to guests regardless of whether
the underlying platform APIC supports such mode.

Thanks, Roger.