linux-kernel - Re: [RFC PATCH v3 23/59] KVM: x86: Allow host-initiated WRMSR to set X2APIC regardless of CPUID

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]

Date:   Mon, 29 Nov 2021 21:21:10 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>, isaku.yamahata@...el.com,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, erdemaktas@...gle.com,
        Connor Kuehl <ckuehl@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, isaku.yamahata@...il.com,
        Sean Christopherson <sean.j.christopherson@...el.com>
Subject: Re: [RFC PATCH v3 23/59] KVM: x86: Allow host-initiated WRMSR to set
 X2APIC regardless of CPUID

On Fri, Nov 26, 2021, Paolo Bonzini wrote:
> On 11/25/21 20:41, Thomas Gleixner wrote:
> > On Wed, Nov 24 2021 at 16:20, isaku yamahata wrote:
> > > Let userspace, or in the case of TDX, KVM itself, enable X2APIC even if
                                            ^^^^^^^^^^

> > > X2APIC is not reported as supported in the guest's CPU model.  KVM
> > > generally does not force specific ordering between ioctls(), e.g. this
> > > forces userspace to configure CPUID before MSRs.  And for TDX, vCPUs
> > > will always run with X2APIC enabled, e.g. KVM will want/need to enable
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > X2APIC from time zero.
      ^^^^^^^^^^^^^^^^^^^^^
> > 
> > This is complete crap. Fix the broken user space and do not add
> > horrible hacks to the kernel.
> 
> tl;dr: I agree that it's a userspace issue but "configure CPUID before MSR"
> is not the issue (in fact QEMU calls KVM_SET_CPUID2 before any call to
> KVM_SET_MSRS).

Specifically for TDX, it's not a userspace issue.  To simplify other checks and
to report sane values for KVM_GET_MSRS, KVM forces X2APIC for TDX guests when the
vCPU is created, before its exposed to usersepace.  The bit about not forcing
specific ordering is justification for making the change independent of TDX,
i.e. to call out that APIC_BASE is different from every other MSR, and is even
inconsistent in its own behavior since illegal transitions are allowed when
userspace is stuffing the MSR.

IMO, this patch is valid irrespective of TDX.  It's included in the TDX series
because TDX support forces the issue.

That said, an alternative for TDX would be do handle this in kvm_lapic_reset()
now that the lAPIC RESET flows are consolidated.  Back when this patch was first
written, that wasn't really an option.