mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kai Huang <kai.huang@intel.com>
To: dave.hansen@intel.com, kirill.shutemov@linux.intel.com,
	tglx@linutronix.de, bp@alien8.de, peterz@infradead.org,
	mingo@redhat.com, hpa@zytor.com, dan.j.williams@intel.com,
	seanjc@google.com, pbonzini@redhat.com
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, rick.p.edgecombe@intel.com,
	isaku.yamahata@intel.com, adrian.hunter@intel.com,
	nik.borisov@suse.com, kai.huang@intel.com
Subject: [PATCH v4 2/8] x86/virt/tdx: Rework TD_SYSINFO_MAP to support build-time verification
Date: Tue, 24 Sep 2024 23:28:29 +1200	[thread overview]
Message-ID: <608ba48cfdffcbbd06c8505e3ba8dc623edf5b10.1727173372.git.kai.huang@intel.com> (raw)
In-Reply-To: <cover.1727173372.git.kai.huang@intel.com>

Dan noticed [1] that read_sys_metadata_field16() has a runtime warning
to validate that the metadata field size matches the passed in buffer
size.  In turns out that all the information to perform that validation
is available at build time.  Rework TD_SYSINFO_MAP() to stop providing
runtime data to read_sys_metadata_field16() and instead just pass typed
fields to read_sys_metadata_field16() and let the compiler catch any
mismatches.  Rename TD_SYSINFO_MAP() to READ_SYS_INFO() since it no
longer stores the mapping of metadata field ID and the structure member
offset.

For now READ_SYS_INFO() is only used in get_tdx_sys_info_tdmr().  Future
changes will need to read other metadata fields that are organized in
different structures.  Do #undef READ_SYS_INFO() after use so the same
pattern can be used for reading other metadata fields.  To avoid needing
to duplicate build-time verification in each READ_SYS_INFO() in future
changes, add a wrapper macro to do build-time verification and call it
from READ_SYS_INFO().

The READ_SYS_INFO() has a couple quirks for readability.  It requires
the function that uses it to define a local variable @ret to carry the
error code and set the initial value to 0.  It also hard-codes the
variable name of the structure pointer used in the function, but it is
less code, build-time verifiable, and the same readability as the former
'struct field_mapping' approach.

Link: http://lore.kernel.org/66b16121c48f4_4fc729424@dwillia2-xfh.jf.intel.com.notmuch [1]
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---

v3 -> v4:
 - Rename TD_SYSINFO_MAP() to READ_SYS_INFO() - Ardian.
 - #undef READ_SYS_INFO() - Ardian.
 - Rewrite changelog based on text from Dan, with some clarification
   around using READ_SYS_INFO() and #undef it.
 - Move the BUILD_BUG_ON() out of read_sys_metadata_field16() - Dan.
 - Use permalink in changelog - Dan.

v2 -> v3:
 - Remove 'struct field_mapping' and reimplement TD_SYSINFO_MAP().

---
 arch/x86/virt/vmx/tdx/tdx.c | 58 ++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index e979bf442929..2f7e4abc1bb9 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -270,60 +270,44 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
 	return 0;
 }
 
-static int read_sys_metadata_field16(u64 field_id,
-				     int offset,
-				     struct tdx_sys_info_tdmr *ts)
+static int __read_sys_metadata_field16(u64 field_id, u16 *val)
 {
-	u16 *ts_member = ((void *)ts) + offset;
 	u64 tmp;
 	int ret;
 
-	if (WARN_ON_ONCE(MD_FIELD_ID_ELE_SIZE_CODE(field_id) !=
-			MD_FIELD_ID_ELE_SIZE_16BIT))
-		return -EINVAL;
-
 	ret = read_sys_metadata_field(field_id, &tmp);
 	if (ret)
 		return ret;
 
-	*ts_member = tmp;
+	*val = tmp;
 
 	return 0;
 }
 
-struct field_mapping {
-	u64 field_id;
-	int offset;
-};
-
-#define TD_SYSINFO_MAP(_field_id, _offset) \
-	{ .field_id = MD_FIELD_ID_##_field_id,	   \
-	  .offset   = offsetof(struct tdx_sys_info_tdmr, _offset) }
-
-/* 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),
-	TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE,    pamt_entry_size[TDX_PS_4K]),
-	TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE,    pamt_entry_size[TDX_PS_2M]),
-	TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE,    pamt_entry_size[TDX_PS_1G]),
-};
+#define read_sys_metadata_field16(_field_id, _val)		\
+({								\
+	BUILD_BUG_ON(MD_FIELD_ID_ELE_SIZE_CODE(_field_id) !=	\
+			MD_FIELD_ID_ELE_SIZE_16BIT);		\
+	__read_sys_metadata_field16(_field_id, _val);		\
+})
 
 static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
 {
-	int ret;
-	int i;
+	int ret = 0;
 
-	/* 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,
-						sysinfo_tdmr);
-		if (ret)
-			return ret;
-	}
+#define READ_SYS_INFO(_field_id, _member)				\
+	ret = ret ?: read_sys_metadata_field16(MD_FIELD_ID_##_field_id,	\
+					&sysinfo_tdmr->_member)
 
-	return 0;
+	READ_SYS_INFO(MAX_TDMRS,	     max_tdmrs);
+	READ_SYS_INFO(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr);
+	READ_SYS_INFO(PAMT_4K_ENTRY_SIZE,    pamt_entry_size[TDX_PS_4K]);
+	READ_SYS_INFO(PAMT_2M_ENTRY_SIZE,    pamt_entry_size[TDX_PS_2M]);
+	READ_SYS_INFO(PAMT_1G_ENTRY_SIZE,    pamt_entry_size[TDX_PS_1G]);
+
+#undef READ_SYS_INFO
+
+	return ret;
 }
 
 /* Calculate the actual TDMR size */
-- 
2.46.0


  parent reply	other threads:[~2024-09-24 11:29 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-24 11:28 [PATCH v4 0/8] TDX host: metadata reading tweaks, bug fix and info dump Kai Huang
2024-09-24 11:28 ` [PATCH v4 1/8] x86/virt/tdx: Rename 'struct tdx_tdmr_sysinfo' to reflect the spec better Kai Huang
2024-09-24 11:28 ` Kai Huang [this message]
2024-09-24 11:28 ` [PATCH v4 3/8] x86/virt/tdx: Prepare to support reading other global metadata fields Kai Huang
2024-09-26  6:27   ` Nikolay Borisov
2024-09-26 12:13     ` Huang, Kai
2024-09-26 15:47   ` Dave Hansen
2024-09-26 22:22     ` Huang, Kai
2024-09-26 22:26       ` Dave Hansen
2024-09-26 23:05         ` Huang, Kai
2024-10-01  7:56           ` Dan Williams
2024-10-01 10:44             ` Huang, Kai
2024-10-01 15:19               ` Dave Hansen
2024-10-01 21:40                 ` Huang, Kai
2024-10-07  3:07                   ` Huang, Kai
2024-10-07  3:44                     ` Dave Hansen
2024-10-07  6:53                       ` Huang, Kai
2024-10-09 11:10                         ` Huang, Kai
2024-09-24 11:28 ` [PATCH v4 4/8] x86/virt/tdx: Refine a comment to reflect the latest TDX spec Kai Huang
2024-09-24 11:28 ` [PATCH v4 5/8] x86/virt/tdx: Start to track all global metadata in one structure Kai Huang
2024-09-26  6:21   ` Nikolay Borisov
2024-09-24 11:28 ` [PATCH v4 6/8] x86/virt/tdx: Print TDX module version Kai Huang
2024-09-24 11:28 ` [PATCH v4 7/8] x86/virt/tdx: Require the module to assert it has the NO_RBP_MOD mitigation Kai Huang
2024-09-24 11:28 ` [PATCH v4 8/8] x86/virt/tdx: Reduce TDMR's reserved areas by using CMRs to find memory holes Kai Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=608ba48cfdffcbbd06c8505e3ba8dc623edf5b10.1727173372.git.kai.huang@intel.com \
    --to=kai.huang@intel.com \
    --cc=adrian.hunter@intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=isaku.yamahata@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nik.borisov@suse.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®