On Tue, Aug 12, 2025 at 11:22 PM Binbin Wu binbin.wu@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
For now I can add a comment on the __packed and add a TODO to replace it with OFFSET. I think that making OFFSET work inside the kvm selftests will require more expertise in the Kbuild system which I don't have.
A side topic is when developers should provide comments for the uses of "__packed". Is it always preferred a comment for it or a developer can save the comment for obvious cases, e.g, the structure is defined according to some hardware layout?
- uint32_t cr0;
- uint32_t cr3;
- uint32_t cr4;
- struct td_boot_parameters_dtr gdtr;
- struct td_boot_parameters_dtr idtr;
- struct td_per_vcpu_parameters per_vcpu[];
+};
...
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S new file mode 100644 index 000000000000..c8cbe214bba9 --- /dev/null +++ b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S @@ -0,0 +1,100 @@ +/* SPDX-License-Identifier: GPL-2.0-only */
+#include "tdx/td_boot_asm.h"
+/* Offsets for reading struct td_boot_parameters. */ +#define TD_BOOT_PARAMETERS_CR0 0 +#define TD_BOOT_PARAMETERS_CR3 4 +#define TD_BOOT_PARAMETERS_CR4 8 +#define TD_BOOT_PARAMETERS_GDT 12 +#define TD_BOOT_PARAMETERS_IDT 18 +#define TD_BOOT_PARAMETERS_PER_VCPU 24
+/* Offsets for reading struct td_per_vcpu_parameters. */ +#define TD_PER_VCPU_PARAMETERS_ESP_GVA 0 +#define TD_PER_VCPU_PARAMETERS_LJMP_TARGET 4
+#define SIZEOF_TD_PER_VCPU_PARAMETERS 10
Please figure out how to replicate the functionality of the kernel's OFFSET() macro from include/linux/kbuild.h, I have zero desire to maintain open coded offset values.