[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <238ea612-5a25-9323-b31f-0a14493db2f7@linux.intel.com>
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!
Powered by blists - more mailing lists