[PATCH v15 19/23] x86/virt/tdx: Improve readability of module initialization error handling

Kai Huang
2023-11-09T11:58:15Z

With keeping TDMRs upon successful TDX module initialization, now only
put_online_mems() needs to be done even when module initialization is
successful. On the other hand, all other four "out_*" labels before
them explicitly check the return value and only clean up when module
initialization fails.

This isn't ideal. Make all other four "out_*" labels only reachable
when module initialization fails to improve the readability of error
handling. Rename them from "out_*" to "err_*" to reflect the fact.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---

v14 -> v15:
- Rebase due to TDH.SYS.RD patch (minor)

v13 -> v14:
- Fix spell typo (Rick)
- Add Kirill/Rick's tags

v12 -> v13:
- New patch to improve error handling. (Kirill, Nikolay)

---
arch/x86/virt/vmx/tdx/tdx.c | 69 +++++++++++++++++++------------------
1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 753e435a3040..e8cd91692ccf 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1081,22 +1081,22 @@ static int init_tdx_module(void)

ret = get_tdx_tdmr_sysinfo(&tdmr_sysinfo);
if (ret)
- goto out_free_tdxmem;
+ goto err_free_tdxmem;

/* Allocate enough space for constructing TDMRs */
ret = alloc_tdmr_list(&tdx_tdmr_list, &tdmr_sysinfo);
if (ret)
- goto out_free_tdxmem;
+ goto err_free_tdxmem;

/* Cover all TDX-usable memory regions in TDMRs */
ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, &tdmr_sysinfo);
if (ret)
- goto out_free_tdmrs;
+ goto err_free_tdmrs;

/* Pass the TDMRs and the global KeyID to the TDX module */
ret = config_tdx_module(&tdx_tdmr_list, tdx_global_keyid);
if (ret)
- goto out_free_pamts;
+ goto err_free_pamts;

/*
* Hardware doesn't guarantee cache coherency across different
@@ -1111,40 +1111,16 @@ static int init_tdx_module(void)
/* Config the key of global KeyID on all packages */
ret = config_global_keyid();
if (ret)
- goto out_reset_pamts;
+ goto err_reset_pamts;

/* Initialize TDMRs to complete the TDX module initialization */
ret = init_tdmrs(&tdx_tdmr_list);
-out_reset_pamts:
- if (ret) {
- /*
- * Part of PAMTs may already have been initialized by the
- * TDX module. Flush cache before returning PAMTs back
- * to the kernel.
- */
- wbinvd_on_all_cpus();
- /*
- * According to the TDX hardware spec, if the platform
- * doesn't have the "partial write machine check"
- * erratum, any kernel read/write will never cause #MC
- * in kernel space, thus it's OK to not convert PAMTs
- * back to normal. But do the conversion anyway here
- * as suggested by the TDX spec.
- */
- tdmrs_reset_pamt_all(&tdx_tdmr_list);
- }
-out_free_pamts:
if (ret)
- tdmrs_free_pamt_all(&tdx_tdmr_list);
- else
- pr_info("%lu KBs allocated for PAMT\n",
- tdmrs_count_pamt_kb(&tdx_tdmr_list));
-out_free_tdmrs:
- if (ret)
- free_tdmr_list(&tdx_tdmr_list);
-out_free_tdxmem:
- if (ret)
- free_tdx_memlist(&tdx_memlist);
+ goto err_reset_pamts;
+
+ pr_info("%lu KBs allocated for PAMT\n",
+ tdmrs_count_pamt_kb(&tdx_tdmr_list));
+
out_put_tdxmem:
/*
* @tdx_memlist is written here and read at memory hotplug time.
@@ -1152,6 +1128,31 @@ static int init_tdx_module(void)
*/
put_online_mems();
return ret;
+
+err_reset_pamts:
+ /*
+ * Part of PAMTs may already have been initialized by the
+ * TDX module. Flush cache before returning PAMTs back
+ * to the kernel.
+ */
+ wbinvd_on_all_cpus();
+ /*
+ * According to the TDX hardware spec, if the platform
+ * doesn't have the "partial write machine check"
+ * erratum, any kernel read/write will never cause #MC
+ * in kernel space, thus it's OK to not convert PAMTs
+ * back to normal. But do the conversion anyway here
+ * as suggested by the TDX spec.
+ */
+ tdmrs_reset_pamt_all(&tdx_tdmr_list);
+err_free_pamts:
+ tdmrs_free_pamt_all(&tdx_tdmr_list);
+err_free_tdmrs:
+ free_tdmr_list(&tdx_tdmr_list);
+err_free_tdxmem:
+ free_tdx_memlist(&tdx_memlist);
+ /* Do things irrelevant to module initialization result */
+ goto out_put_tdxmem;
}

static int __tdx_enable(void)
--
2.41.0