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

[Andrew Cooper <Andrew.Cooper3@citrix.com>]

On 14/11/2022 13:15, Roger Pau Monne wrote:
> 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.

And now it has a layer of obfuscation which makes the bug less obvious.

> 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.

It's not about whether a version of Windows supports x2APIC.  Its
whether windows turns x2APIC on.

Short of having an emulated IOMMU, or an absence of an IO-APIC (neither
of which we do), guests wont turn x2APIC on.

I know we have the magic hack for IO-APIC with >8 bit destinations, but
that only got enumerated in the Xen leaves (IIRC?), so only Linux can
pick it up.

The hint is still very relevant for any version of windows running in
xAPIC mode which, at a guess, is all of them under Xen.

~Andrew