linux-kernel - Re: [PATCH] KVM: x86: Do not mask LVTPC when handling a PMI on AMD platforms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZeYz7zPGcIQSH_NI@google.com>
Date: Mon, 4 Mar 2024 12:49:51 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Jim Mattson <jmattson@...gle.com>
Cc: Sandipan Das <sandipan.das@....com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	mlevitsk@...hat.com, vkuznets@...hat.com, mizhang@...gle.com, 
	tao1.su@...ux.intel.com, andriy.shevchenko@...ux.intel.com, 
	ravi.bangoria@....com, ananth.narayan@....com, nikunj.dadhania@....com, 
	santosh.shukla@....com, manali.shukla@....com
Subject: Re: [PATCH] KVM: x86: Do not mask LVTPC when handling a PMI on AMD platforms

On Fri, Mar 01, 2024, Jim Mattson wrote:
> On Thu, Feb 29, 2024 at 11:44 PM Sandipan Das <sandipan.das@....com> 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.
> 
> 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 = reg & APIC_LVT_LEVEL_TRIGGER;
> >
> >                 r = __apic_accept_irq(apic, mode, vector, 1, trig_mode, NULL);
> > -               if (r && lvt_type == APIC_LVTPC)
> > +               if (r && lvt_type == APIC_LVTPC && !guest_cpuid_is_amd_or_hygon(apic->vcpu))
> 
> 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 somewhere
other than the zeroth entry, KVM's linear walk to find a CPUID entry will make
this a pretty slow lookup.

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

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