linux-kernel - Re: [PATCH v8 06/30] KVM: selftests: Add helper functions to create TDX VMs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c8d6d1c-d9e1-4ffd-bb26-a03fb87cde1f@linux.intel.com>
Date: Wed, 13 Aug 2025 12:22:39 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>, Sagi Shahar <sagis@...gle.com>
Cc: linux-kselftest@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
 Shuah Khan <shuah@...nel.org>, Ackerley Tng <ackerleytng@...gle.com>,
 Ryan Afranji <afranji@...gle.com>, Andrew Jones <ajones@...tanamicro.com>,
 Isaku Yamahata <isaku.yamahata@...el.com>,
 Erdem Aktas <erdemaktas@...gle.com>,
 Rick Edgecombe <rick.p.edgecombe@...el.com>,
 Roger Wang <runanwang@...gle.com>, Oliver Upton <oliver.upton@...ux.dev>,
 "Pratik R. Sampat" <pratikrajesh.sampat@....com>,
 Reinette Chatre <reinette.chatre@...el.com>, Ira Weiny
 <ira.weiny@...el.com>, linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v8 06/30] KVM: selftests: Add helper functions to create
 TDX VMs

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".

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.
>