Port I/O instructions trigger #VE in the TDX environment. In response to
the exception, kernel emulates these instructions using hypercalls.
But during early boot, on the decompression stage, it is cumbersome to
deal with #VE. It is cleaner to go to hypercalls directly, bypassing #VE
handling.
Add a way to hook up alternative port I/O helpers in the boot stub with
a new pio_ops structure. For now, set the ops structure to just call
the normal I/O operation functions.
out*()/in*() macros redefined to use pio_ops callbacks. It eliminates
need in changing call sites. io_delay() changed to use port I/O helper
instead of inline assembly.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/boot/boot.h | 4 +--
arch/x86/boot/compressed/misc.c | 4 +++
arch/x86/boot/compressed/misc.h | 2 +-
arch/x86/boot/io.h | 50 +++++++++++++++++++++++++++++++++
arch/x86/boot/main.c | 4 +++
arch/x86/realmode/rm/wakemain.c | 4 +++
6 files changed, 65 insertions(+), 3 deletions(-)
create mode 100644 arch/x86/boot/io.h
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 22a474c5b3e8..b42b91606ca8 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -23,10 +23,10 @@
#include <linux/edd.h>
#include <asm/setup.h>
#include <asm/asm.h>
-#include <asm/shared/io.h>
#include "bitops.h"
#include "ctype.h"
#include "cpuflags.h"
+#include "io.h"
/* Useful macros */
#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
@@ -39,7 +39,7 @@ extern struct boot_params boot_params;
static inline void io_delay(void)
{
const u16 DELAY_PORT = 0x80;
- asm volatile("outb %%al,%0" : : "dN" (DELAY_PORT));
+ outb(0, DELAY_PORT);
}
/* These functions are used to reference data in other segments. */
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 2b1169869b96..9fdef6af20a1 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -47,6 +47,8 @@ void *memmove(void *dest, const void *src, size_t n);
*/
struct boot_params *boot_params;
+struct port_io_ops pio_ops;
+
memptr free_mem_ptr;
memptr free_mem_end_ptr;
@@ -370,6 +372,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
lines = boot_params->screen_info.orig_video_lines;
cols = boot_params->screen_info.orig_video_cols;
+ init_default_io_ops();
+
/*
* Detect TDX guest environment.
*
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 8a253e85f990..ea71cf3d64e1 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -26,7 +26,6 @@
#include <asm/boot.h>
#include <asm/bootparam.h>
#include <asm/desc_defs.h>
-#include <asm/shared/io.h>
#include "tdx.h"
@@ -35,6 +34,7 @@
#define BOOT_BOOT_H
#include "../ctype.h"
+#include "../io.h"
#ifdef CONFIG_X86_64
#define memptr long
diff --git a/arch/x86/boot/io.h b/arch/x86/boot/io.h
new file mode 100644
index 000000000000..5f0d99310f91
--- /dev/null
+++ b/arch/x86/boot/io.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef BOOT_IO_H
+#define BOOT_IO_H
+
+#include <asm/shared/io.h>
+
+#undef inb
+#undef inw
+#undef inl
+#undef outb
+#undef outw
+#undef outl
+
+struct port_io_ops {
+ u8 (*inb)(u16 port);
+ u16 (*inw)(u16 port);
+ u32 (*inl)(u16 port);
+ void (*outb)(u8 v, u16 port);
+ void (*outw)(u16 v, u16 port);
+ void (*outl)(u32 v, u16 port);
+};
+
+extern struct port_io_ops pio_ops;
+
+/*
+ * Use the normal I/O instructions by default.
+ * TDX guests override these to use hypercalls.
+ */
+static inline void init_default_io_ops(void)
+{
+ pio_ops.inb = __inb;
+ pio_ops.inw = __inw;
+ pio_ops.inl = __inl;
+ pio_ops.outb = __outb;
+ pio_ops.outw = __outw;
+ pio_ops.outl = __outl;
+}
+
+/*
+ * Redirect port I/O operations via pio_ops callbacks.
+ * TDX guests override these callbacks with TDX-specific helpers.
+ */
+#define inb pio_ops.inb
+#define inw pio_ops.inw
+#define inl pio_ops.inl
+#define outb pio_ops.outb
+#define outw pio_ops.outw
+#define outl pio_ops.outl
+
+#endif
diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index e3add857c2c9..1202d4f8a390 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -17,6 +17,8 @@
struct boot_params boot_params __attribute__((aligned(16)));
+struct port_io_ops pio_ops;
+
char *HEAP = _end;
char *heap_end = _end; /* Default end of heap = no heap */
@@ -133,6 +135,8 @@ static void init_heap(void)
void main(void)
{
+ init_default_io_ops();
+
/* First, copy the boot header into the "zeropage" */
copy_boot_params();
diff --git a/arch/x86/realmode/rm/wakemain.c b/arch/x86/realmode/rm/wakemain.c
index 1d6437e6d2ba..a6f4d8388ad8 100644
--- a/arch/x86/realmode/rm/wakemain.c
+++ b/arch/x86/realmode/rm/wakemain.c
@@ -62,8 +62,12 @@ static void send_morse(const char *pattern)
}
}
+struct port_io_ops pio_ops;
+
void main(void)
{
+ init_default_io_ops();
+
/* Kill machine if structures are wrong */
if (wakeup_header.real_magic != 0x12345678)
while (1)
--
2.34.1
On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> +#undef inb
> +#undef inw
> +#undef inl
> +#undef outb
> +#undef outw
> +#undef outl
> +
> +struct port_io_ops {
> + u8 (*inb)(u16 port);
> + u16 (*inw)(u16 port);
> + u32 (*inl)(u16 port);
> + void (*outb)(u8 v, u16 port);
> + void (*outw)(u16 v, u16 port);
> + void (*outl)(u32 v, u16 port);
u8 (*inb)(u16 port);
void (*outb)(u8 v, u16 port);
void (*outw)(u16 v, u16 port);
is all what's used AFAICT.
> +};
> +
> +extern struct port_io_ops pio_ops;
> +
> +/*
> + * Use the normal I/O instructions by default.
> + * TDX guests override these to use hypercalls.
> + */
> +static inline void init_default_io_ops(void)
> +{
> + pio_ops.inb = __inb;
> + pio_ops.inw = __inw;
> + pio_ops.inl = __inl;
> + pio_ops.outb = __outb;
> + pio_ops.outw = __outw;
> + pio_ops.outl = __outl;
> +}
#define DEFINE_PORT_IO_OPS() \
struct port_io_ops pio_ops = { \
.inb = __inb, \
.outb = __outb, \
.outw = __outw, }
Hmm?
Thanks,
tglx
On Thu, Mar 17, 2022 at 01:12:59PM +0100, Thomas Gleixner wrote:
> On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> > +#undef inb
> > +#undef inw
> > +#undef inl
> > +#undef outb
> > +#undef outw
> > +#undef outl
> > +
> > +struct port_io_ops {
> > + u8 (*inb)(u16 port);
> > + u16 (*inw)(u16 port);
> > + u32 (*inl)(u16 port);
> > + void (*outb)(u8 v, u16 port);
> > + void (*outw)(u16 v, u16 port);
> > + void (*outl)(u32 v, u16 port);
>
> u8 (*inb)(u16 port);
> void (*outb)(u8 v, u16 port);
> void (*outw)(u16 v, u16 port);
>
> is all what's used AFAICT.
Hm. Okay. I will drop the rest.
> > +};
> > +
> > +extern struct port_io_ops pio_ops;
> > +
> > +/*
> > + * Use the normal I/O instructions by default.
> > + * TDX guests override these to use hypercalls.
> > + */
> > +static inline void init_default_io_ops(void)
> > +{
> > + pio_ops.inb = __inb;
> > + pio_ops.inw = __inw;
> > + pio_ops.inl = __inl;
> > + pio_ops.outb = __outb;
> > + pio_ops.outw = __outw;
> > + pio_ops.outl = __outl;
> > +}
>
> #define DEFINE_PORT_IO_OPS() \
> struct port_io_ops pio_ops = { \
> .inb = __inb, \
> .outb = __outb, \
> .outw = __outw, }
>
> Hmm?
This kind of initializations are problematic. They generate run-time
relacations that kernel cannot handle in the boot stub. Linker complains
about this:
ld.lld: error: Unexpected run-time relocations (.rela) detected!
I will leave it as is, unless you have better ideas.
--
Kirill A. Shutemov
On 3/17/22 13:10, Kirill A. Shutemov wrote:
>> Hmm?
> This kind of initializations are problematic. They generate run-time
> relacations that kernel cannot handle in the boot stub. Linker complains
> about this:
>
> ld.lld: error: Unexpected run-time relocations (.rela) detected!
>
> I will leave it as is, unless you have better ideas.
Right now you've got:
#define inb pio_ops.inb
You keep the preprocessor away from things like
foo.inb = bar;
with:
#define inb(x) pio_ops.inb(x)
On 3/17/22 13:20, Dave Hansen wrote:
> You keep the preprocessor away from things like
>
> foo.inb = bar;
>
> with:
>
> #define inb(x) pio_ops.inb(x)
... and I spotted my nonsense just as I hit send.
You could do that ^^. But you'd need to rename the 'inb' op like:
struct port_io_ops {
u8 (*f_inb)(u16 port);
...
};
so that you do:
#define inb(x) pio_ops.f_inb(x)
But, remember when I said I hate playing #define tricks? ;) This is one
reason why.
On Thu, Mar 17, 2022 at 01:23:12PM -0700, Dave Hansen wrote:
> On 3/17/22 13:20, Dave Hansen wrote:
> > You keep the preprocessor away from things like
> >
> > foo.inb = bar;
> >
> > with:
> >
> > #define inb(x) pio_ops.inb(x)
>
> ... and I spotted my nonsense just as I hit send.
>
> You could do that ^^. But you'd need to rename the 'inb' op like:
>
> struct port_io_ops {
> u8 (*f_inb)(u16 port);
> ...
> };
>
> so that you do:
>
> #define inb(x) pio_ops.f_inb(x)
>
> But, remember when I said I hate playing #define tricks? ;) This is one
> reason why.
But the define tricks are unrelated to the linker issue. The issue pops up
after you get past the preprocessor tricks.
--
Kirill A. Shutemov
On Thu, Mar 17 2022 at 23:10, Kirill A. Shutemov wrote:
> On Thu, Mar 17, 2022 at 01:12:59PM +0100, Thomas Gleixner wrote:
>> #define DEFINE_PORT_IO_OPS() \
>> struct port_io_ops pio_ops = { \
>> .inb = __inb, \
>> .outb = __outb, \
>> .outw = __outw, }
>>
>> Hmm?
>
> This kind of initializations are problematic. They generate run-time
> relacations that kernel cannot handle in the boot stub. Linker complains
> about this:
>
> ld.lld: error: Unexpected run-time relocations (.rela) detected!
>
> I will leave it as is, unless you have better ideas.
Bah, did not think about that.
Thanks,
tglx