linux-kernel - Re: [PATCH] x86/apic: Don't disable x2APIC if locked

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:   Wed, 10 Aug 2022 11:30:13 -0700
From:   Daniel Sneddon <daniel.sneddon@...ux.intel.com>
To:     Dave Hansen <dave.hansen@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "Shutemov, Kirill" <kirill.shutemov@...el.com>
Cc:     "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/apic: Don't disable x2APIC if locked

On 8/10/22 11:01, Dave Hansen wrote:
> On 8/9/22 16:40, Daniel Sneddon wrote:
>> The APIC supports two modes, legacy APIC (or xAPIC), and Extended APIC
>> (or x2APIC).  X2APIC mode is mostly compatible with legacy APIC, but
>> it disables the memory-mapped APIC interface in favor of one that uses
>> MSRs.  The APIC mode is controlled by the EXT bit in the APIC MSR.
>>
>> Introduce support for a new feature that will allow the BIOS to lock
>> the APIC in x2APIC mode.  If the APIC is locked in x2APIC mode and the
>> kernel tries to disable the APIC or revert to legacy APIC mode a GP
>> fault will occur.
>>
>> Introduce support for a new MSR (IA32_XAPIC_DISABLE_STATUS) and handle the
>> new locked mode when the LEGACY_XAPIC_DISABLED bit is set.
>>
>> If the LEGACY_XAPIC_DISABLED is set, prevent the kernel
>> from trying to disable it.
> 
> Let's also not obscure the fact that the MMIO/xAPIC interface is
> troublesome and there are real-world, practical reasons a piece of
> hardware might not want to implement it.  First on the list is:
> 
> 	https://aepicleak.com/
> 
Great point.  Since the problem with that interface wasn't public when I wrote
this patch I didn't have anything like that to link against.  Would you like me
to add the above link to the commit message?  I can certainly add more about the
SGX leak associated with APIC.

> Second on the list is TDX.  The TDX module spec currently just dictates
> that TDX guests must use x2APIC.  If this (IA32_XAPIC_DISABLE_STATUS)
> mechanism was enumerated to TDX guests, they could use this instead of
> crashing in burning in whatever horrid way they are now if someone
> disables x2APIC on the command line.
> 
> It would also be nice to know roughly when this feature is showing up.
> If it's going to show up as a part of a microcode update for my laptop
> next week or next month, this might warrant a backport.  Intel would
> presumably *want* a backport to happen there, too.
> 
I've been told that this will only be on Sapphire Rapids and newer.

> If it's going to be on one server CPU that's coming out in ten years,
> then we can hold off.
SPR will certainly be sooner than 10 years, and with distros running on LTS
kernels, it is probably worth backporting.  Since this isn't a bug fix (not a sw
bug anyway), is this something I can just CC stable or is there a better way to
say "Yes, this is a new feature, BUT, you really want it anyway"?

> 
> It might also help to link to the documentation, even if it's below a
> "--" in the changelog since these URLs aren't very stable.
> 
>> https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/cpuid-enumeration-and-architectural-msrs.html
> 
> or at least mention what the status of the documentation is.
> 
> The code looks OK to me, but I'm far from impartial because this isn't
> my first look at it.  In any case:
> 
> Acked-by: Dave Hansen <dave.hansen@...ux.intel.com>

Thanks!