Re: Nested AVIC design (was:Re: [RFC PATCH v3 04/19] KVM: x86: mmu: allow to enable write tracking externally)

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2022-09-29 at 22:38 +0000, Sean Christopherson wrote:
> On Mon, Aug 08, 2022, Maxim Levitsky wrote:
> > Hi Sean, Paolo, and everyone else who wants to review my nested AVIC work.
> 
> Before we dive deep into design details, I think we should first decide whether
> or not nested AVIC is worth pursing/supporting.
> 
>   - Rome has a ucode/silicon bug with no known workaround and no anticipated fix[*];
>     AMD's recommended "workaround" is to disable AVIC.
>   - AVIC is not available in Milan, which may or may not be related to the
>     aforementioned bug.
>   - AVIC is making a comeback on Zen4, but Zen4 comes with x2AVIC.
>   - x2APIC is likely going to become ubiquitous, e.g. Intel is effectively
>     requiring x2APIC to fudge around xAPIC bugs.
>   - It's actually quite realistic to effectively force the guest to use x2APIC,
>     at least if it's a Linux guest.  E.g. turn x2APIC on in BIOS, which is often
>     (always?) controlled by the host, and Linux will use x2APIC.
> 
> In other words, given that AVIC is well on its way to becoming a "legacy" feature,
> IMO there needs to be a fairly strong use case to justify taking on this much code
> and complexity.  ~1500 lines of code to support a feature that has historically
> been buggy _without_ nested support is going to require a non-trivial amount of
> effort to review, stabilize, and maintain.
> 
> [*] 1235 "Guest With AVIC (Advanced Virtual Interrupt Controller) Enabled May Fail
>     to Process IPI (Inter-Processor Interrupt) Until Guest Is Re-Scheduled" in
>     https://www.amd.com/system/files/TechDocs/56323-PUB_1.00.pdf
> 

I am afraid that you mixed things up:

You mistake is that x2avic is just a minor addition to AVIC. It is still for
all practical purposes the same feature.

 
1. The AVIC is indeed kind of broken on Zen2 (but AFAIK for all practical purposes,
   including nested it works fine, the errata only shows up in a unit test and/or
   under very specific workloads (most of the time a delayed wakeup doesn't cause a hang).
   Yet, I agree that for production Zen2 should not have AVIC enabled.
 

2. Zen3 does indeed have AVIC soft disabled in CPUID. AFAIK it works just fine,
   but I understand that customers won't use it against AMD's guidance.
 
 
3. On Zen4, AVIC is fully enabled and also extended to support x2apic mode.
   The fact that AVIC was extended to support X2apic mode also shows that AMD
   is committed to supporting it.
 
 
My nested AVIC code technically doesn't expose x2avic to the guest, but it
is pretty much trivial to add (I am only waiting to get my hands on Zen4 machine
to do it), and also even in its current form it would work just fine if the host
uses normal AVIC .
 
(or even doesn't use AVIC at all - the nested AVIC code works just fine
even if the host has its AVIC inhibited for some reason).
 
Adding nested x2avic support is literally about not passing through that MMIO address,
Enabling the x2avic bit in int_ctl, and opening up the access to x2apic msrs.
Plus I need to do some minor changes in unaccelerated IPI handler, dealing
With read-only logical ID and such.
 
Physid tables, apic backing pages, doorbell emulation, 
everything is pretty much unchanged.
 
So AVIC is nothing but a legacy feature, and my nested AVIC code will support
both nested AVIC and nested X2AVIC.
 
Best regards,
	Maxim Levitsky