public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Adam Dunlap <acdunlap@google.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Nathan Chancellor <nathan@kernel.org>,
	 Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	 Justin Stitt <justinstitt@google.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	 Arjan van de Ven <arjan@linux.intel.com>,
	Wei Liu <wei.liu@kernel.org>,
	linux-kernel@vger.kernel.org,  llvm@lists.linux.dev,
	Jacob Xu <jacobhxu@google.com>, Alper Gun <alpergun@google.com>,
	 Kevin Loughlin <kevinloughlin@google.com>,
	Peter Gonda <pgonda@google.com>,
	 Ard Biesheuvel <ardb@kernel.org>
Cc: Adam Dunlap <acdunlap@google.com>
Subject: [PATCH v3] x86/asm: Force native_apic_mem_read to use mov
Date: Tue,  6 Feb 2024 14:36:20 -0800	[thread overview]
Message-ID: <20240206223620.1833276-1-acdunlap@google.com> (raw)

When done from a virtual machine, instructions that touch APIC memory
must be emulated. By convention, MMIO access are typically performed via
io.h helpers such as 'readl()' or 'writeq()' to simplify instruction
emulation/decoding (ex: in KVM hosts and SEV guests) [0].

Currently, native_apic_mem_read does not follow this convention,
allowing the compiler to emit instructions other than the mov generated
by readl(). In particular, when compiled with clang and run as a SEV-ES
or SEV-SNP guest, the compiler would emit a testl instruction which is
not supported by the SEV-ES emulator, causing a boot failure in that
environment. It is likely the same problem would happen in a TDX guest
as that uses the same instruction emulator as SEV-ES.

To make sure all emulators can emulate APIC memory reads via mov, use
the readl function in native_apic_mem_read. It is expected that any
emulator would support mov in any addressing mode it is the most generic
and is what is ususally emitted currently.

The testl instruction is emitted when native_apic_mem_read
is inlined into __xapic_wait_icr_idle. The emulator comes from
insn_decode_mmio in arch/x86/lib/insn-eval.c. It would not be worth it
to extend insn_decode_mmio to support more instructions since, in
theory, the compiler could choose to output nearly any instruction for
such reads which would bloat the emulator beyond reason.

An alterative to this approach would be to use inline assembly instead
of the readl helper, as that is what native_apic_mem_write does. I
consider using readl to be cleaner since it is documented to be a simple
wrapper and inline assembly is less readable. native_apic_mem_write
cannot be trivially updated to use writel since it appears to use custom
asm to workaround for a processor-specific bug.

[0] https://lore.kernel.org/all/20220405232939.73860-12-kirill.shutemov@linux.intel.com/

Signed-off-by: Adam Dunlap <acdunlap@google.com>
Tested-by: Kevin Loughlin <kevinloughlin@google.com>
---

Patch changelog:
V1 -> V2: Replaced asm with readl function which does the same thing
V2 -> V3: Updated commit message to show more motivation and
justification

Link to v2 discussion: https://lore.kernel.org/all/20220908170456.3177635-1-acdunlap@google.com/

 arch/x86/include/asm/apic.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 9d159b771dc8..dddd3fc195ef 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -13,6 +13,7 @@
 #include <asm/mpspec.h>
 #include <asm/msr.h>
 #include <asm/hardirq.h>
+#include <asm/io.h>
 
 #define ARCH_APICTIMER_STOPS_ON_C3	1
 
@@ -96,7 +97,7 @@ static inline void native_apic_mem_write(u32 reg, u32 v)
 
 static inline u32 native_apic_mem_read(u32 reg)
 {
-	return *((volatile u32 *)(APIC_BASE + reg));
+	return readl((void __iomem *)(APIC_BASE + reg));
 }
 
 static inline void native_apic_mem_eoi(void)
-- 
2.43.0.594.gd9cf4e227d-goog


             reply	other threads:[~2024-02-06 22:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06 22:36 Adam Dunlap [this message]
2024-02-08 16:47 ` [PATCH v3] x86/asm: Force native_apic_mem_read to use mov Dave Hansen
2024-02-09 15:22   ` Ard Biesheuvel
2024-02-09 18:20     ` Adam Dunlap
2024-03-14 15:57       ` Kevin Loughlin
2024-03-15 23:44 ` Thomas Gleixner
2024-03-18 23:09   ` [PATCH v4] x86/asm: Force native_apic_mem_read() to use the MOV instruction Adam Dunlap
2024-03-19 11:27     ` Ard Biesheuvel
2024-04-08 13:43     ` [tip: x86/urgent] x86/apic: " tip-bot2 for Adam Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240206223620.1833276-1-acdunlap@google.com \
    --to=acdunlap@google.com \
    --cc=alpergun@google.com \
    --cc=ardb@kernel.org \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jacobhxu@google.com \
    --cc=justinstitt@google.com \
    --cc=kevinloughlin@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mingo@redhat.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=pgonda@google.com \
    --cc=tglx@linutronix.de \
    --cc=wei.liu@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox