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.
