| 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
| ||
|
Message-ID: <1b810874-2734-4ca8-933d-ebe9500a8ddc@intel.com>
Date: Tue, 27 Aug 2024 16:10:40 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Kai Huang <kai.huang@...el.com>, dave.hansen@...el.com,
kirill.shutemov@...ux.intel.com, tglx@...utronix.de, bp@...en8.de,
peterz@...radead.org, mingo@...hat.com, hpa@...or.com,
dan.j.williams@...el.com, seanjc@...gle.com, pbonzini@...hat.com
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
rick.p.edgecombe@...el.com, isaku.yamahata@...el.com, chao.gao@...el.com,
binbin.wu@...ux.intel.com
Subject: Re: [PATCH v3 1/8] x86/virt/tdx: Rename 'struct tdx_tdmr_sysinfo' to
reflect the spec better
On 27/08/24 10:14, Kai Huang wrote:
"to reflect the spec better" is a bit vague. How about:
x86/virt/tdx: Rename 'struct tdx_tdmr_sysinfo' to 'struct tdx_sys_info_tdmr'
Rename 'struct tdx_tdmr_sysinfo' to 'struct tdx_sys_info_tdmr' to
prepare for adding similar structures that will all be prefixed by
'tdx_sys_info_'.
> The TDX module provides a set of "global metadata fields". They report
Since it is a name of something, could capitalize "Global Metadata Fields"
> things like TDX module version, supported features, and fields related
> to create/run TDX guests and so on.
>
> TDX organizes those metadata fields by "Class"es based on the meaning of
by "Class"es -> into "Classes"
> those fields. E.g., for now the kernel only reads "TD Memory Region"
> (TDMR) related fields for module initialization. Those fields are
> defined under class "TDMR Info".
>
> There are both immediate needs to read more metadata fields for module
> initialization and near-future needs for other kernel components like
> KVM to run TDX guests. To meet all those requirements, the idea is the
> TDX host core-kernel to provide a centralized, canonical, and read-only
> structure for the global metadata that comes out from the TDX module for
> all kernel components to use.
>
> More specifically, the target is to end up with something like:
>
> struct tdx_sys_info {
> struct tdx_sys_info_classA a;
> struct tdx_sys_info_classB b;
> ...
> };
>
> Currently the kernel organizes all fields under "TDMR Info" class in
> 'struct tdx_tdmr_sysinfo'. To prepare for the above target, rename the
> structure to 'struct tdx_sys_info_tdmr' to follow the class name better.
>
> No functional change intended.
>
> Signed-off-by: Kai Huang <kai.huang@...el.com>
Reviewed-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
> v2 -> v3:
> - Split out as a separate patch and place it at beginning:
>
> https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@intel.com/T/#m8fec7c429242d640cf5e756eb68e3b822e6dff8b
>
> - Rename to 'struct tdx_sys_info_tdmr':
>
> https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@intel.com/T/#md73dd9b02a492acf4a6facae63e8d030e320967d
> https://lore.kernel.org/kvm/cover.1721186590.git.kai.huang@intel.com/T/#m8fec7c429242d640cf5e756eb68e3b822e6dff8b
>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 36 ++++++++++++++++++------------------
> arch/x86/virt/vmx/tdx/tdx.h | 2 +-
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 4e2b2e2ac9f9..e979bf442929 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -272,7 +272,7 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
>
> static int read_sys_metadata_field16(u64 field_id,
> int offset,
> - struct tdx_tdmr_sysinfo *ts)
> + struct tdx_sys_info_tdmr *ts)
> {
> u16 *ts_member = ((void *)ts) + offset;
> u64 tmp;
> @@ -298,9 +298,9 @@ struct field_mapping {
>
> #define TD_SYSINFO_MAP(_field_id, _offset) \
> { .field_id = MD_FIELD_ID_##_field_id, \
> - .offset = offsetof(struct tdx_tdmr_sysinfo, _offset) }
> + .offset = offsetof(struct tdx_sys_info_tdmr, _offset) }
>
> -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */
> +/* Map TD_SYSINFO fields into 'struct tdx_sys_info_tdmr': */
> static const struct field_mapping fields[] = {
> TD_SYSINFO_MAP(MAX_TDMRS, max_tdmrs),
> TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr),
> @@ -309,16 +309,16 @@ static const struct field_mapping fields[] = {
> TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]),
> };
>
> -static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo)
> +static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> {
> int ret;
> int i;
>
> - /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */
> + /* Populate 'sysinfo_tdmr' fields using the mapping structure above: */
> for (i = 0; i < ARRAY_SIZE(fields); i++) {
> ret = read_sys_metadata_field16(fields[i].field_id,
> fields[i].offset,
> - tdmr_sysinfo);
> + sysinfo_tdmr);
> if (ret)
> return ret;
> }
> @@ -342,13 +342,13 @@ static int tdmr_size_single(u16 max_reserved_per_tdmr)
> }
>
> static int alloc_tdmr_list(struct tdmr_info_list *tdmr_list,
> - struct tdx_tdmr_sysinfo *tdmr_sysinfo)
> + struct tdx_sys_info_tdmr *sysinfo_tdmr)
> {
> size_t tdmr_sz, tdmr_array_sz;
> void *tdmr_array;
>
> - tdmr_sz = tdmr_size_single(tdmr_sysinfo->max_reserved_per_tdmr);
> - tdmr_array_sz = tdmr_sz * tdmr_sysinfo->max_tdmrs;
> + tdmr_sz = tdmr_size_single(sysinfo_tdmr->max_reserved_per_tdmr);
> + tdmr_array_sz = tdmr_sz * sysinfo_tdmr->max_tdmrs;
>
> /*
> * To keep things simple, allocate all TDMRs together.
> @@ -367,7 +367,7 @@ static int alloc_tdmr_list(struct tdmr_info_list *tdmr_list,
> * at a given index in the TDMR list.
> */
> tdmr_list->tdmr_sz = tdmr_sz;
> - tdmr_list->max_tdmrs = tdmr_sysinfo->max_tdmrs;
> + tdmr_list->max_tdmrs = sysinfo_tdmr->max_tdmrs;
> tdmr_list->nr_consumed_tdmrs = 0;
>
> return 0;
> @@ -921,11 +921,11 @@ static int tdmrs_populate_rsvd_areas_all(struct tdmr_info_list *tdmr_list,
> /*
> * Construct a list of TDMRs on the preallocated space in @tdmr_list
> * to cover all TDX memory regions in @tmb_list based on the TDX module
> - * TDMR global information in @tdmr_sysinfo.
> + * TDMR global information in @sysinfo_tdmr.
> */
> static int construct_tdmrs(struct list_head *tmb_list,
> struct tdmr_info_list *tdmr_list,
> - struct tdx_tdmr_sysinfo *tdmr_sysinfo)
> + struct tdx_sys_info_tdmr *sysinfo_tdmr)
> {
> int ret;
>
> @@ -934,12 +934,12 @@ static int construct_tdmrs(struct list_head *tmb_list,
> return ret;
>
> ret = tdmrs_set_up_pamt_all(tdmr_list, tmb_list,
> - tdmr_sysinfo->pamt_entry_size);
> + sysinfo_tdmr->pamt_entry_size);
> if (ret)
> return ret;
>
> ret = tdmrs_populate_rsvd_areas_all(tdmr_list, tmb_list,
> - tdmr_sysinfo->max_reserved_per_tdmr);
> + sysinfo_tdmr->max_reserved_per_tdmr);
> if (ret)
> tdmrs_free_pamt_all(tdmr_list);
>
> @@ -1098,7 +1098,7 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list)
>
> static int init_tdx_module(void)
> {
> - struct tdx_tdmr_sysinfo tdmr_sysinfo;
> + struct tdx_sys_info_tdmr sysinfo_tdmr;
> int ret;
>
> /*
> @@ -1117,17 +1117,17 @@ static int init_tdx_module(void)
> if (ret)
> goto out_put_tdxmem;
>
> - ret = get_tdx_tdmr_sysinfo(&tdmr_sysinfo);
> + ret = get_tdx_sys_info_tdmr(&sysinfo_tdmr);
> if (ret)
> goto err_free_tdxmem;
>
> /* Allocate enough space for constructing TDMRs */
> - ret = alloc_tdmr_list(&tdx_tdmr_list, &tdmr_sysinfo);
> + ret = alloc_tdmr_list(&tdx_tdmr_list, &sysinfo_tdmr);
> if (ret)
> goto err_free_tdxmem;
>
> /* Cover all TDX-usable memory regions in TDMRs */
> - ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, &tdmr_sysinfo);
> + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, &sysinfo_tdmr);
> if (ret)
> goto err_free_tdmrs;
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index b701f69485d3..148f9b4d1140 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -100,7 +100,7 @@ struct tdx_memblock {
> };
>
> /* "TDMR info" part of "Global Scope Metadata" for constructing TDMRs */
> -struct tdx_tdmr_sysinfo {
> +struct tdx_sys_info_tdmr {
> u16 max_tdmrs;
> u16 max_reserved_per_tdmr;
> u16 pamt_entry_size[TDX_PS_NR];
Powered by blists - more mailing lists