Hi Sagi,
On 8/14/25 10:20 PM, Sagi Shahar wrote:
> On Tue, Aug 12, 2025 at 11:22 PM Binbin Wu <binbin...@linux.intel.com> wrote:
>>
>>
>>
>> On 8/12/2025 4:13 AM, Sean Christopherson wrote:
>>> On Thu, Aug 07, 2025, Sagi Shahar wrote:
>> [...]
>>>> +
>>>> +/*
>>>> + * Boot parameters for the TD.
>>>> + *
>>>> + * Unlike a regular VM, KVM cannot set registers such as esp, eip, etc
>>>> + * before boot, so to run selftests, these registers' values have to be
>>>> + * initialized by the TD.
>>>> + *
>>>> + * This struct is loaded in TD private memory at TD_BOOT_PARAMETERS_GPA.
>>>> + *
>>>> + * The TD boot code will read off parameters from this struct and set up
>>>> the
>>>> + * vCPU for executing selftests.
>>>> + */
>>>> +struct __packed td_boot_parameters {
>>> None of these comments explain why these structures are __packed, and I
>>> suspect
>>> _that_ is the most interesting/relevant information for unfamiliar readers.
>> I guess because the fields defined in this structure are accessed by
>> hard-coded
>> offsets in boot code.
>> But as you suggested below, replicating the functionality of the kernel's
>> OFFSET() could get rid of "__packed".
>>
>
> I agree, I think the reason for using __packed is because of the hard
> coded offsets. I tried using OFFSET() as Sean suggested but couldn't
> make it work.
>
> I can't get the Kbuild scripts to work inside the kvm selftests
> Makefile. I tried adding the following rules based on a reference I
> found:
>
> +include/x86/tdx/td_boot_offsets.h: lib/x86/tdx/td_boot_offsets.s
> + $(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
> +
> +lib/x86/tdx/td_boot_offsets.s: lib/x86/tdx/td_boot_offsets.c
> + $(call if_changed_dep,cc_s_c)
>
> But I'm getting the following error when trying to generate the header:
>
> /bin/sh: -c: line 1: syntax error near unexpected token `;'
> /bin/sh: -c: line 1: `set -e; ; printf '# cannot find fixdep (%s)\n'
> > lib/x86/tdx/.td_boot_offsets.s.cmd; printf '# using basic dep
> data\n\n' >> lib/x86/tdx/.td_boot_offsets.s.cmd; cat
> lib/x86/tdx/.td_boot_offsets.s.d >>
> lib/x86/tdx/.td_boot_offsets.s.cmd; printf '\n%s\n'
> 'cmd_lib/x86/tdx/td_boot_offsets.s := ' >>
> lib/x86/tdx/.td_boot_offsets.s.cmd'
> make: *** [Makefile.kvm:44: lib/x86/tdx/td_boot_offsets.s] Error 2
>
I do not believe that the selftests can directly use the Kbuild
infrastructure. From what I understand, when similar things are needed they
are duplicated. Take for example tools/build/Build.include that is included
in Makefile.kvm and contains the needed things duplicated from
scripts/Kbuild.include.
I see two options here:
a) Selftests relies on the kernel to generate the header file. This
unfortunately
will create dependency on kernel being built first and I expect a no-go.
b) - Duplicate the filechk (that depends on tmp-target), filechk_offsets, and
sed-offsets defines in from scripts/Makefile.lib to
tools/build/Build.include
- Duplicate include/linux/kbuild.h in tools/include/linux
(b) will allow your rule duplicated below to work:
include/x86/tdx/td_boot_offsets.h: lib/x86/tdx/td_boot_offsets.s
$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
tools/build/Build.include already contains if_changed_dep and the command
it calls may just be locally defined to make the other rule you are trying work.
I played around with this a bit and found changes below to work. I certainly
admit that it is crude but "it works" as a proof-of-concept. I also look
forward to
learn from other folks on how to to this right.
If you think something like this is useful then please consider below only as a
proof-of-concept since it needs to be split appropriately, missing cleanups
(handling "make clean"), may not need to be TDX specific, etc.
---8<---
tools/build/Build.include | 56 +++++++++++++++++++
tools/include/linux/kbuild.h | 16 ++++++
tools/testing/selftests/kvm/Makefile.kvm | 11 ++++
.../selftests/kvm/lib/x86/asm-tdx-offsets.c | 28 ++++++++++
.../selftests/kvm/lib/x86/tdx/tdcall.S | 16 +-----
5 files changed, 113 insertions(+), 14 deletions(-)
create mode 100644 tools/include/linux/kbuild.h
create mode 100644 tools/testing/selftests/kvm/lib/x86/asm-tdx-offsets.c
diff --git a/tools/build/Build.include b/tools/build/Build.include
index e45b2eb0d24a..330892a1ba15 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -20,6 +20,10 @@ space := $(empty) $(empty)
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)
+###
+# Name of target with a '.tmp_' as filename prefix. foo/bar.o => foo/.tmp_bar.o
+tmp-target = $(dir $@).tmp_$(notdir $@)
+
###
# filename of target with directory and extension stripped
basetarget = $(basename $(notdir $@))
@@ -43,6 +47,58 @@ escsq = $(subst $(squote),'\$(squote)',$1)
echo-cmd = $(if $($(quiet)cmd_$(1)),\
echo ' $(call escsq,$($(quiet)cmd_$(1)))';)
+# Default sed regexp - multiline due to syntax constraints
+#
+# Use [:space:] because LLVM's integrated assembler inserts <tab> around
+# the .ascii directive whereas GCC keeps the <space> as-is.
+define sed-offsets
+ 's:^[[:space:]]*\.ascii[[:space:]]*"\(.*\)".*:\1:; \
+ /^->/{s:->#\(.*\):/* \1 */:; \
+ s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \
+ s:->::; p;}'
+endef
+
+# Use filechk to avoid rebuilds when a header changes, but the resulting file
+# does not
+define filechk_offsets
+ echo "#ifndef $2"; \
+ echo "#define $2"; \
+ echo "/*"; \
+ echo " * DO NOT MODIFY."; \
+ echo " *"; \
+ echo " * This file was generated by Kbuild"; \
+ echo " */"; \
+ echo ""; \
+ sed -ne $(sed-offsets) < $<; \
+ echo ""; \
+ echo "#endif"
+endef
+
+###
+# filechk is used to check if the content of a generated file is updated.
+# Sample usage:
+#
+# filechk_sample = echo $(KERNELRELEASE)
+# version.h: FORCE
+# $(call filechk,sample)
+#
+# The rule defined shall write to stdout the content of the new file.
+# The existing file will be compared with the new one.
+# - If no file exist it is created
+# - If the content differ the new file is used
+# - If they are equal no change, and no timestamp update
+define filechk
+ echo $(tmp-target); \
+ $(Q)set -e; \
+ mkdir -p $(dir $@); \
+ trap "rm -f $(tmp-target)" EXIT; \
+ { $(filechk_$(1)); } > $(tmp-target); \
+ if [ ! -r $@ ] || ! cmp -s $@ $(tmp-target); then \
+ $(echo-cmd) ' UPD $@'; \
+ mv -f $(tmp-target) $@; \
+ fi
+endef
+
###
# Replace >$< with >$$< to preserve $ when reloading the .cmd file
# (needed for make)
diff --git a/tools/include/linux/kbuild.h b/tools/include/linux/kbuild.h
new file mode 100644
index 000000000000..e7be517aaaf6
--- /dev/null
+++ b/tools/include/linux/kbuild.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_KBUILD_H
+#define __LINUX_KBUILD_H
+
+#define DEFINE(sym, val) \
+ asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
+
+#define BLANK() asm volatile("\n.ascii \"->\"" : : )
+
+#define OFFSET(sym, str, mem) \
+ DEFINE(sym, offsetof(struct str, mem))
+
+#define COMMENT(x) \
+ asm volatile("\n.ascii \"->#" x "\"")
+
+#endif
diff --git a/tools/testing/selftests/kvm/Makefile.kvm
b/tools/testing/selftests/kvm/Makefile.kvm
index ef2b1a47719c..e7bd835a56db 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -241,12 +241,14 @@ INSTALL_HDR_PATH = $(top_srcdir)/usr
LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
+LINUX_TOOL_OUTPUT_ARCH_INCLUDE = $(OUTPUT)/include/$(ARCH)
CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
-Wno-gnu-variable-sized-type-not-at-end -MD -MP -DCONFIG_64BIT \
-fno-builtin-memcmp -fno-builtin-memcpy \
-fno-builtin-memset -fno-builtin-strnlen \
-fno-stack-protector -fno-PIE -fno-strict-aliasing \
-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_TOOL_ARCH_INCLUDE) \
+ -I$(LINUX_TOOL_OUTPUT_ARCH_INCLUDE) \
-I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(ARCH) \
-I ../rseq -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
ifeq ($(ARCH),s390)
@@ -256,6 +258,15 @@ ifeq ($(ARCH),x86)
ifeq ($(shell echo "void foo(void) { }" | $(CC) -march=x86-64-v2 -x c - -c -o
/dev/null 2>/dev/null; echo "$$?"),0)
CFLAGS += -march=x86-64-v2
endif
+
+GEN_HDRS := $(OUTPUT)/include/$(ARCH)/generated/asm-tdx-offsets.h
+
+$(OUTPUT)/lib/$(ARCH)/asm-tdx-offsets.s: lib/$(ARCH)/asm-tdx-offsets.c
+ $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -S $< -o $@
+
+$(GEN_HDRS): $(OUTPUT)/lib/$(ARCH)/asm-tdx-offsets.s
+ $(call filechk,offsets,__ASM_KVM_SELFTESTS_OFFSETS_H__)
+
endif
ifeq ($(ARCH),arm64)
tools_dir := $(top_srcdir)/tools
diff --git a/tools/testing/selftests/kvm/lib/x86/asm-tdx-offsets.c
b/tools/testing/selftests/kvm/lib/x86/asm-tdx-offsets.c
new file mode 100644
index 000000000000..ad56d99b56be
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/asm-tdx-offsets.c
@@ -0,0 +1,28 @@
+#include <linux/kbuild.h>
+
+#include "tdx/tdcall.h"
+
+static inline void common(void)
+{
+ OFFSET(TDX_HYPERCALL_r10, tdx_hypercall_args, r10);
+ OFFSET(TDX_HYPERCALL_r11, tdx_hypercall_args, r11);
+ OFFSET(TDX_HYPERCALL_r12, tdx_hypercall_args, r12);
+ OFFSET(TDX_HYPERCALL_r13, tdx_hypercall_args, r13);
+ OFFSET(TDX_HYPERCALL_r14, tdx_hypercall_args, r14);
+ OFFSET(TDX_HYPERCALL_r15, tdx_hypercall_args, r15);
+
+ BLANK();
+ OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
+ OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
+ OFFSET(TDX_MODULE_r8, tdx_module_output, r8);
+ OFFSET(TDX_MODULE_r9, tdx_module_output, r9);
+ OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
+ OFFSET(TDX_MODULE_r11, tdx_module_output, r11);
+}
+
+int main(void)
+{
+ common();
+
+ return 0;
+}
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
index c393a0fb35be..e98aa5178db9 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
@@ -1,18 +1,13 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* Adapted from arch/x86/coco/tdx/tdcall.S */
+#include "generated/asm-tdx-offsets.h"
+
/*
* TDCALL is supported in Binutils >= 2.36, add it for older version.
*/
#define tdcall .byte 0x66,0x0f,0x01,0xcc
-#define TDX_HYPERCALL_r10 0 /* offsetof(struct tdx_hypercall_args, r10) */
-#define TDX_HYPERCALL_r11 8 /* offsetof(struct tdx_hypercall_args, r11) */
-#define TDX_HYPERCALL_r12 16 /* offsetof(struct tdx_hypercall_args, r12) */
-#define TDX_HYPERCALL_r13 24 /* offsetof(struct tdx_hypercall_args, r13) */
-#define TDX_HYPERCALL_r14 32 /* offsetof(struct tdx_hypercall_args, r14) */
-#define TDX_HYPERCALL_r15 40 /* offsetof(struct tdx_hypercall_args, r15) */
-
/*
* Bitmasks of exposed registers (with VMM).
*/
@@ -91,13 +86,6 @@ __tdx_hypercall:
pop %rbp
ret
-#define TDX_MODULE_rcx 0 /* offsetof(struct tdx_module_output, rcx) */
-#define TDX_MODULE_rdx 8 /* offsetof(struct tdx_module_output, rdx) */
-#define TDX_MODULE_r8 16 /* offsetof(struct tdx_module_output, r8) */
-#define TDX_MODULE_r9 24 /* offsetof(struct tdx_module_output, r9) */
-#define TDX_MODULE_r10 32 /* offsetof(struct tdx_module_output, r10) */
-#define TDX_MODULE_r11 40 /* offsetof(struct tdx_module_output, r11) */
-
.globl __tdx_module_call
.type __tdx_module_call, @function
__tdx_module_call:
---8<---
