Received-SPF: pass (google.com: domain of linux-kernel+bounces-91254-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 4 Mar 2024 12:49:51 -0800
In-Reply-To: <CALMp9eRbOQpVsKkZ9N1VYTyOrKPWCmvi0B5WZCFXAPsSkShmEA@mail.gmail.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240301074423.643779-1-sandipan.das@amd.com> <CALMp9eRbOQpVsKkZ9N1VYTyOrKPWCmvi0B5WZCFXAPsSkShmEA@mail.gmail.com>
Message-ID: <ZeYz7zPGcIQSH_NI@google.com>
Subject: Re: [PATCH] KVM: x86: Do not mask LVTPC when handling a PMI on AMD platforms
From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Sandipan Das <sandipan.das@amd.com>, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, 
	pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, 
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, 
	mlevitsk@redhat.com, vkuznets@redhat.com, mizhang@google.com, 
	tao1.su@linux.intel.com, andriy.shevchenko@linux.intel.com, 
	ravi.bangoria@amd.com, ananth.narayan@amd.com, nikunj.dadhania@amd.com, 
	santosh.shukla@amd.com, manali.shukla@amd.com
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 01, 2024, Jim Mattson wrote:
> On Thu, Feb 29, 2024 at 11:44=E2=80=AFPM Sandipan Das <sandipan.das@amd.c=
om> wrote:
> >
> > On AMD and Hygon platforms, the local APIC does not automatically set
> > the mask bit of the LVTPC register when handling a PMI and there is
> > no need to clear it in the kernel's PMI handler.
>=20
> I don't know why it didn't occur to me that different x86 vendors
> wouldn't agree on this specification. :)

Because you're sane?  :-D

> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 3242f3da2457..0959a887c306 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -2768,7 +2768,7 @@ int kvm_apic_local_deliver(struct kvm_lapic *apic=
, int lvt_type)
> >                 trig_mode =3D reg & APIC_LVT_LEVEL_TRIGGER;
> >
> >                 r =3D __apic_accept_irq(apic, mode, vector, 1, trig_mod=
e, NULL);
> > -               if (r && lvt_type =3D=3D APIC_LVTPC)
> > +               if (r && lvt_type =3D=3D APIC_LVTPC && !guest_cpuid_is_=
amd_or_hygon(apic->vcpu))
>=20
> Perhaps we could use a positive predicate instead:
> guest_cpuid_is_intel(apic->vcpu)?

AFAICT, Zhaoxin follows intel behavior, so we'd theoretically have to allow=
 for
that too.  The many checks of guest_cpuid_is_intel() in KVM suggest that no=
 one
actually cares about about correctly virtualizing Zhaoxin CPUs, but it's an=
 easy
enough problem to solve.

I'm also very tempted to say KVM should cache the Intel vs. AMD vCPU model.=
  E.g.
if userspace does something weird with guest CPUID and puts CPUID.0x0 somew=
here
other than the zeroth entry, KVM's linear walk to find a CPUID entry will m=
ake
this a pretty slow lookup.

Then we could also encapsulate the gory details for Intel vs. Zhaoxin vs. C=
entaur,
and AMD vs. Hygon, e.g.

		if (r && lvt_type =3D=3D APIC_LVTPC &&
		    apic->vcpu->arch.is_model_intel_compatible)