[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180304085159.GB7931@amd>
Date: Sun, 4 Mar 2018 09:51:59 +0100
From: Pavel Machek <pavel@....cz>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, r.marek@...embler.cz,
ricardo.neri-calderon@...ux.intel.com, rkrcmar@...hat.com,
Janakarajan.Natarajan@....com, bp@...e.de, x86@...nel.org,
hpa@...or.com, mingo@...hat.com,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] clarify how insecure CPU is
Hi!
> > > > > >
> > > > > > First, what is going on with X86_BUG_AMD_E400 and X86_BUG_AMD_APIC_C1E
> > > > > > ? They seem to refer to the same bug, perhaps comment should mention
> > > > > > that? (Do we need two flags for one bug?)
> > > > > >
> > > > > > Next, maybe X86_BUG_CPU_INSECURE is a bit too generic? This seems to
> > > > > > address "Meltdown" problem, but not "Spectre". Should it be limited to
> > > > > > PPro and newer Intel CPUs?
> > > > > >
> > > > > > Should another erratum be added for "Spectre"? This is present even on
> > > > > > AMD CPUs, but should not be present in 486, maybe Pentium, and some
> > > > > > Atom chips?
> > > > > >
> > > > > > Plus... is this reasonable interface?
> > > > > >
> > > > > > bugs : cpu_insecure
> > > > >
> > > > > We've renamed it to meltdown already and added spectre_v1/v2 bits for the
> > > > > rest of the mess.
> > > >
> > > > Could you explain (best with code comment) what is going on with
> > > > X86_BUG_AMD_E400 and X86_BUG_AMD_APIC_C1E ? They seem to refer to the
> > > > same bug.
> > >
> > > Sorry, that;s really not the time for this.
> >
> > Ok, is there better time now? The code is a bit confusing...
>
> What's confusing? Here are the relevant code snippets in invocation order.
>
> /*
> * Check whether the machine is affected by erratum 400. This is
> * used to select the proper idle routine and to enable the check
> * whether the machine is affected in arch_post_acpi_init(), which
> * sets the X86_BUG_AMD_APIC_C1E bug depending on the MSR check.
> */
> if (cpu_has_amd_erratum(c, amd_erratum_400))
> set_cpu_bug(c, X86_BUG_AMD_E400);
>
> This sets the errate 400 bug bit to tell subsequent code that the CPU might
> be affected by that erratum.
>
> void select_idle_routine(const struct cpuinfo_x86 *c)
> {
>
> if (boot_cpu_has_bug(X86_BUG_AMD_E400)) {
> pr_info("using AMD E400 aware idle routine\n");
> x86_idle = amd_e400_idle;
>
> Selects the idle routine depending on the bug flag
>
> void __init arch_post_acpi_subsys_init(void)
> {
> u32 lo, hi;
>
> if (!boot_cpu_has_bug(X86_BUG_AMD_E400))
> return;
>
> /*
> * AMD E400 detection needs to happen after ACPI has been enabled. If
> * the machine is affected K8_INTP_C1E_ACTIVE_MASK bits are set in
> * MSR_K8_INT_PENDING_MSG.
> */
> rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
> if (!(lo & K8_INTP_C1E_ACTIVE_MASK))
> return;
>
> Late detection whether C1E which halts TSC and APIC is enabled. This needs
> to be done after ACPI is initialized.
>
> /*
> * AMD Erratum 400 aware idle routine. We handle it the same way as C3 power
> * states (local apic timer and TSC stop).
> */
> static void amd_e400_idle(void)
> {
> /*
> * We cannot use static_cpu_has_bug() here because X86_BUG_AMD_APIC_C1E
> * gets set after static_cpu_has() places have been converted via
> * alternatives.
> */
> if (!boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E)) {
> default_idle();
> return;
> }
>
> The actual idle routine. If the C1E bug flag is not set, CPU is not
> affected, use default idle, otherwise handle it like other C-States which
> stop TSC and APIC.
>
>
> The comments are clear enough, but Feel free to send patches which enhance
> them if you think thats necessary.
Thanks for explanation. Might this be good idea?
Signed-off-by: Pavel Machek <pavel@....cz>
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index f41079d..4901742 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -341,7 +341,7 @@
#define X86_BUG_FDIV X86_BUG(1) /* FPU FDIV */
#define X86_BUG_COMA X86_BUG(2) /* Cyrix 6x86 coma */
#define X86_BUG_AMD_TLB_MMATCH X86_BUG(3) /* "tlb_mmatch" AMD Erratum 383 */
-#define X86_BUG_AMD_APIC_C1E X86_BUG(4) /* "apic_c1e" AMD Erratum 400 */
+#define X86_BUG_AMD_APIC_C1E X86_BUG(4) /* System is affected AMD Erratum 400, special idle routine is needed */
#define X86_BUG_11AP X86_BUG(5) /* Bad local APIC aka 11AP */
#define X86_BUG_FXSAVE_LEAK X86_BUG(6) /* FXSAVE leaks FOP/FIP/FOP */
#define X86_BUG_CLFLUSH_MONITOR X86_BUG(7) /* AAI65, CLFLUSH required before MONITOR */
@@ -356,7 +356,7 @@
#define X86_BUG_NULL_SEG X86_BUG(10) /* Nulling a selector preserves the base */
#define X86_BUG_SWAPGS_FENCE X86_BUG(11) /* SWAPGS without input dep on GS */
#define X86_BUG_MONITOR X86_BUG(12) /* IPI required to wake up remote CPU */
-#define X86_BUG_AMD_E400 X86_BUG(13) /* CPU is among the affected by Erratum 400 */
+#define X86_BUG_AMD_E400 X86_BUG(13) /* System may be affected by Erratum 400, X86_BUG_AMD_APIC_C1E might be needed */
#define X86_BUG_CPU_MELTDOWN X86_BUG(14) /* CPU is affected by meltdown attack and needs kernel page table isolation */
#define X86_BUG_SPECTRE_V1 X86_BUG(15) /* CPU is affected by Spectre variant 1 attack with conditional branches */
#define X86_BUG_SPECTRE_V2 X86_BUG(16) /* CPU is affected by Spectre variant 2 attack with indirect branches */
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)
Powered by blists - more mailing lists