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

[Jan Beulich <jbeulich@xxxxxxxx>]

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