On 11.11.2022 18: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.
Okay, I agree that I was mislead by the name of the control. Together with Paul (re)clarifying what (virtual) MSRs the CPUID hint is about I agree that it wants to be "virtual interrupt delivery" alone which is checked here. Which in turn makes this a change orthogonal to the other APIC assist work. Jan
