[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220117143920.3umnnlx7dl27cm5z@box.shutemov.name>
Date: Mon, 17 Jan 2022 17:39:20 +0300
From: "Kirill A. Shutemov" <kirill@...temov.name>
To: Borislav Petkov <bp@...en8.de>
Cc: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
tglx@...utronix.de, mingo@...hat.com, dave.hansen@...el.com,
luto@...nel.org, peterz@...radead.org,
sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/26] x86/tdx: Support TDX guest port I/O at
decompression time
On Sat, Jan 15, 2022 at 01:16:00PM +0100, Borislav Petkov wrote:
> On Sat, Jan 15, 2022 at 04:01:55AM +0300, Kirill A. Shutemov wrote:
> > Do you see it differently? Do you want to switch to #VE here?
>
> I'm just comparing to what SEV does and wondering why you guys do it
> differently. But if you think hypercalls is easier, fine by me.
>
> The thing I don't like about that patch is you mixing up kernel proper
> io helpers with the decompressor code instead of modifying the ones in
> arch/x86/boot/boot.h.
arch/x86/boot and arch/x86/boot/compressed are separate linking domains.
boot/ uses own implementation while boot/compressed uses implementation
from <asm/io.h>. Decopliing boot/compressed from <asm/io.h> requires hack.
See #define _ACPI_IO_H_ below.
And even after that we cannot directly use implementation in boot/ since
we would need to make aware about TDX. That's not needed beyond
boot/comressed.
> We need to hammer out how the code sharing between kernel proper and the
> decompressor should be done but that ain't it, especially if there are
> already special io helpers in the decompressor.
What about the patch below?
I've added another (yes, third) implementation of outb()/inb() for
boot/compressed (we don't need the rest io helpers there).
Looks cleaner to me.
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 22a2a6cc2ab4..1bfe30ebadbe 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -99,6 +99,7 @@ endif
vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdx.o
+vmlinux-objs-$(CONFIG_INTEL_TDX_GUEST) += $(obj)/tdcall.o
vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
efi-obj-$(CONFIG_EFI_STUB) = $(objtree)/drivers/firmware/efi/libstub/lib.a
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 261e81fb9582..1b842d04e687 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,4 +1,5 @@
#include "misc.h"
+#include "io.h"
int early_serial_base;
diff --git a/arch/x86/boot/compressed/io.h b/arch/x86/boot/compressed/io.h
new file mode 100644
index 000000000000..5e9de1e781d7
--- /dev/null
+++ b/arch/x86/boot/compressed/io.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef BOOT_COMPRESSED_IO_H
+#define BOOT_COMPRESSED_IO_H
+
+#include "tdx.h"
+
+static inline void outb(u8 v, u16 port)
+{
+ if (early_is_tdx_guest())
+ tdx_io_out(1, port, v);
+ else
+ asm volatile("outb %0,%1" : : "a" (v), "dN" (port));
+}
+
+static inline u8 inb(u16 port)
+{
+ u8 v;
+ if (early_is_tdx_guest())
+ v = tdx_io_in(1, port);
+ else
+ asm volatile("inb %1,%0" : "=a" (v) : "dN" (port));
+ return v;
+}
+
+#endif
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index d8373d766672..dd97d9ca73db 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -15,6 +15,8 @@
#include "misc.h"
#include "error.h"
#include "pgtable.h"
+#include "tdx.h"
+#include "io.h"
#include "../string.h"
#include "../voffset.h"
#include <asm/bootparam_utils.h>
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 0d8e275a9d96..f3c10ae33c45 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -22,13 +22,13 @@
#include <linux/linkage.h>
#include <linux/screen_info.h>
#include <linux/elf.h>
-#include <linux/io.h>
#include <asm/page.h>
#include <asm/boot.h>
#include <asm/bootparam.h>
#include <asm/desc_defs.h>
-#include "tdx.h"
+/* Avoid pulling outb()/inb() from <asm/io.h> */
+#define _ACPI_IO_H_
#define BOOT_CTYPE_H
#include <linux/acpi.h>
diff --git a/arch/x86/boot/compressed/tdcall.S b/arch/x86/boot/compressed/tdcall.S
new file mode 100644
index 000000000000..aafadc136c88
--- /dev/null
+++ b/arch/x86/boot/compressed/tdcall.S
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include "../../kernel/tdcall.S"
diff --git a/arch/x86/boot/compressed/tdx.h b/arch/x86/boot/compressed/tdx.h
index 18970c09512e..6d6799c1daec 100644
--- a/arch/x86/boot/compressed/tdx.h
+++ b/arch/x86/boot/compressed/tdx.h
@@ -6,11 +6,37 @@
#include <linux/types.h>
#ifdef CONFIG_INTEL_TDX_GUEST
+
+#include <vdso/limits.h>
+#include <uapi/asm/vmx.h>
+#include <asm/tdx.h>
+
void early_tdx_detect(void);
bool early_is_tdx_guest(void);
+
+static inline unsigned int tdx_io_in(int size, int port)
+{
+ struct tdx_hypercall_output out;
+
+ __tdx_hypercall(TDX_HYPERCALL_STANDARD, EXIT_REASON_IO_INSTRUCTION,
+ size, 0, port, 0, &out);
+
+ return out.r10 ? UINT_MAX : out.r11;
+}
+
+static inline void tdx_io_out(int size, int port, u64 value)
+{
+ struct tdx_hypercall_output out;
+
+ __tdx_hypercall(TDX_HYPERCALL_STANDARD, EXIT_REASON_IO_INSTRUCTION,
+ size, 1, port, value, &out);
+}
+
#else
static inline void early_tdx_detect(void) { };
static inline bool early_is_tdx_guest(void) { return false; }
+static inline unsigned int tdx_io_in(int size, int port) { return 0; }
+static inline void tdx_io_out(int size, int port, u64 value) { }
#endif
#endif
--
Kirill A. Shutemov
Powered by blists - more mailing lists