| Message ID | 7d06fe5fda0e330895c1c9043b881f3c2a2d4f3f.1687784645.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX host kernel support | expand |
On 26.06.23 г. 17:12 ч., Kai Huang wrote: > On the platforms with the "partial write machine check" erratum, the > kexec() needs to convert all TDX private pages back to normal before > booting to the new kernel. Otherwise, the new kernel may get unexpected > machine check. > > There's no existing infrastructure to track TDX private pages. Change > to keep TDMRs when module initialization is successful so that they can > be used to find PAMTs. > > With this change, only put_online_mems() and freeing the buffer of the > TDSYSINFO_STRUCT and CMR array still need to be done even when module > initialization is successful. Adjust the error handling to explicitly > do them when module initialization is successful and unconditionally > clean up the rest when initialization fails. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > --- > > v11 -> v12 (new patch): > - Defer keeping TDMRs logic to this patch for better review > - Improved error handling logic (Nikolay/Kirill in patch 15) > > --- > arch/x86/virt/vmx/tdx/tdx.c | 84 ++++++++++++++++++------------------- > 1 file changed, 42 insertions(+), 42 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 52b7267ea226..85b24b2e9417 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -49,6 +49,8 @@ static DEFINE_MUTEX(tdx_module_lock); > /* All TDX-usable memory regions. Protected by mem_hotplug_lock. */ > static LIST_HEAD(tdx_memlist); > > +static struct tdmr_info_list tdx_tdmr_list; > + > /* > * Wrapper of __seamcall() to convert SEAMCALL leaf function error code > * to kernel error code. @seamcall_ret and @out contain the SEAMCALL > @@ -1047,7 +1049,6 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) > static int init_tdx_module(void) > { > struct tdsysinfo_struct *sysinfo; > - struct tdmr_info_list tdmr_list; > struct cmr_info *cmr_array; > int ret; > > @@ -1088,17 +1089,17 @@ static int init_tdx_module(void) > goto out_put_tdxmem; > > /* Allocate enough space for constructing TDMRs */ > - ret = alloc_tdmr_list(&tdmr_list, sysinfo); > + ret = alloc_tdmr_list(&tdx_tdmr_list, sysinfo); > if (ret) > goto out_free_tdxmem; > > /* Cover all TDX-usable memory regions in TDMRs */ > - ret = construct_tdmrs(&tdx_memlist, &tdmr_list, sysinfo); > + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, sysinfo); nit: Does it make sense to keep passing those global variables are function parameters? Since those functions are static it's unlikely that they are going to be used with any other parameter so might as well use the parameter directly. It makes the code somewhat easier to follow. <snip>
On Tue, Jun 27, 2023 at 02:12:48AM +1200, Kai Huang wrote: > On the platforms with the "partial write machine check" erratum, the > kexec() needs to convert all TDX private pages back to normal before > booting to the new kernel. Otherwise, the new kernel may get unexpected > machine check. > > There's no existing infrastructure to track TDX private pages. Change > to keep TDMRs when module initialization is successful so that they can > be used to find PAMTs. > > With this change, only put_online_mems() and freeing the buffer of the > TDSYSINFO_STRUCT and CMR array still need to be done even when module > initialization is successful. Adjust the error handling to explicitly > do them when module initialization is successful and unconditionally > clean up the rest when initialization fails. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > --- > > v11 -> v12 (new patch): > - Defer keeping TDMRs logic to this patch for better review > - Improved error handling logic (Nikolay/Kirill in patch 15) > > --- > arch/x86/virt/vmx/tdx/tdx.c | 84 ++++++++++++++++++------------------- > 1 file changed, 42 insertions(+), 42 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 52b7267ea226..85b24b2e9417 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -49,6 +49,8 @@ static DEFINE_MUTEX(tdx_module_lock); > /* All TDX-usable memory regions. Protected by mem_hotplug_lock. */ > static LIST_HEAD(tdx_memlist); > > +static struct tdmr_info_list tdx_tdmr_list; > + > /* > * Wrapper of __seamcall() to convert SEAMCALL leaf function error code > * to kernel error code. @seamcall_ret and @out contain the SEAMCALL > @@ -1047,7 +1049,6 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) > static int init_tdx_module(void) > { > struct tdsysinfo_struct *sysinfo; > - struct tdmr_info_list tdmr_list; > struct cmr_info *cmr_array; > int ret; > > @@ -1088,17 +1089,17 @@ static int init_tdx_module(void) > goto out_put_tdxmem; > > /* Allocate enough space for constructing TDMRs */ > - ret = alloc_tdmr_list(&tdmr_list, sysinfo); > + ret = alloc_tdmr_list(&tdx_tdmr_list, sysinfo); > if (ret) > goto out_free_tdxmem; > > /* Cover all TDX-usable memory regions in TDMRs */ > - ret = construct_tdmrs(&tdx_memlist, &tdmr_list, sysinfo); > + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, sysinfo); > if (ret) > goto out_free_tdmrs; > > /* Pass the TDMRs and the global KeyID to the TDX module */ > - ret = config_tdx_module(&tdmr_list, tdx_global_keyid); > + ret = config_tdx_module(&tdx_tdmr_list, tdx_global_keyid); > if (ret) > goto out_free_pamts; > > @@ -1118,51 +1119,50 @@ static int init_tdx_module(void) > goto out_reset_pamts; > > /* Initialize TDMRs to complete the TDX module initialization */ > - ret = init_tdmrs(&tdmr_list); > + ret = init_tdmrs(&tdx_tdmr_list); > + if (ret) > + goto out_reset_pamts; > + > + pr_info("%lu KBs allocated for PAMT.\n", > + tdmrs_count_pamt_kb(&tdx_tdmr_list)); > + > + /* > + * @tdx_memlist is written here and read at memory hotplug time. > + * Lock out memory hotplug code while building it. > + */ > + put_online_mems(); > + /* > + * For now both @sysinfo and @cmr_array are only used during > + * module initialization, so always free them. > + */ > + free_page((unsigned long)sysinfo); > + > + return 0; > 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(&tdmr_list); > - } > + /* > + * 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(&tdmr_list); > - else > - pr_info("%lu KBs allocated for PAMT.\n", > - tdmrs_count_pamt_kb(&tdmr_list)); > + tdmrs_free_pamt_all(&tdx_tdmr_list); > out_free_tdmrs: > - /* > - * Always free the buffer of TDMRs as they are only used during > - * module initialization. > - */ > - free_tdmr_list(&tdmr_list); > + free_tdmr_list(&tdx_tdmr_list); > out_free_tdxmem: > - if (ret) > - free_tdx_memlist(&tdx_memlist); > + free_tdx_memlist(&tdx_memlist); > out_put_tdxmem: > - /* > - * @tdx_memlist is written here and read at memory hotplug time. > - * Lock out memory hotplug code while building it. > - */ > put_online_mems(); > out: > - /* > - * For now both @sysinfo and @cmr_array are only used during > - * module initialization, so always free them. > - */ > free_page((unsigned long)sysinfo); > return ret; > } This diff is extremely hard to follow, but I think the change to error handling Nikolay proposed has to be applied to the function from the beginning, not changed drastically in this patch.
On 28.06.23 г. 15:23 ч., kirill.shutemov@linux.intel.com wrote: > On Tue, Jun 27, 2023 at 02:12:48AM +1200, Kai Huang wrote: >> On the platforms with the "partial write machine check" erratum, the >> kexec() needs to convert all TDX private pages back to normal before >> booting to the new kernel. Otherwise, the new kernel may get unexpected >> machine check. >> >> There's no existing infrastructure to track TDX private pages. Change >> to keep TDMRs when module initialization is successful so that they can >> be used to find PAMTs. >> >> With this change, only put_online_mems() and freeing the buffer of the >> TDSYSINFO_STRUCT and CMR array still need to be done even when module >> initialization is successful. Adjust the error handling to explicitly >> do them when module initialization is successful and unconditionally >> clean up the rest when initialization fails. >> >> Signed-off-by: Kai Huang <kai.huang@intel.com> >> --- >> >> v11 -> v12 (new patch): >> - Defer keeping TDMRs logic to this patch for better review >> - Improved error handling logic (Nikolay/Kirill in patch 15) >> >> --- >> arch/x86/virt/vmx/tdx/tdx.c | 84 ++++++++++++++++++------------------- >> 1 file changed, 42 insertions(+), 42 deletions(-) >> >> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c >> index 52b7267ea226..85b24b2e9417 100644 >> --- a/arch/x86/virt/vmx/tdx/tdx.c >> +++ b/arch/x86/virt/vmx/tdx/tdx.c >> @@ -49,6 +49,8 @@ static DEFINE_MUTEX(tdx_module_lock); >> /* All TDX-usable memory regions. Protected by mem_hotplug_lock. */ >> static LIST_HEAD(tdx_memlist); >> >> +static struct tdmr_info_list tdx_tdmr_list; >> + >> /* >> * Wrapper of __seamcall() to convert SEAMCALL leaf function error code >> * to kernel error code. @seamcall_ret and @out contain the SEAMCALL >> @@ -1047,7 +1049,6 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) >> static int init_tdx_module(void) >> { >> struct tdsysinfo_struct *sysinfo; >> - struct tdmr_info_list tdmr_list; >> struct cmr_info *cmr_array; >> int ret; >> >> @@ -1088,17 +1089,17 @@ static int init_tdx_module(void) >> goto out_put_tdxmem; >> >> /* Allocate enough space for constructing TDMRs */ >> - ret = alloc_tdmr_list(&tdmr_list, sysinfo); >> + ret = alloc_tdmr_list(&tdx_tdmr_list, sysinfo); >> if (ret) >> goto out_free_tdxmem; >> >> /* Cover all TDX-usable memory regions in TDMRs */ >> - ret = construct_tdmrs(&tdx_memlist, &tdmr_list, sysinfo); >> + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, sysinfo); >> if (ret) >> goto out_free_tdmrs; >> >> /* Pass the TDMRs and the global KeyID to the TDX module */ >> - ret = config_tdx_module(&tdmr_list, tdx_global_keyid); >> + ret = config_tdx_module(&tdx_tdmr_list, tdx_global_keyid); >> if (ret) >> goto out_free_pamts; >> >> @@ -1118,51 +1119,50 @@ static int init_tdx_module(void) >> goto out_reset_pamts; >> >> /* Initialize TDMRs to complete the TDX module initialization */ >> - ret = init_tdmrs(&tdmr_list); >> + ret = init_tdmrs(&tdx_tdmr_list); >> + if (ret) >> + goto out_reset_pamts; >> + >> + pr_info("%lu KBs allocated for PAMT.\n", >> + tdmrs_count_pamt_kb(&tdx_tdmr_list)); >> + >> + /* >> + * @tdx_memlist is written here and read at memory hotplug time. >> + * Lock out memory hotplug code while building it. >> + */ >> + put_online_mems(); >> + /* >> + * For now both @sysinfo and @cmr_array are only used during >> + * module initialization, so always free them. >> + */ >> + free_page((unsigned long)sysinfo); >> + >> + return 0; >> 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(&tdmr_list); >> - } >> + /* >> + * 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(&tdmr_list); >> - else >> - pr_info("%lu KBs allocated for PAMT.\n", >> - tdmrs_count_pamt_kb(&tdmr_list)); >> + tdmrs_free_pamt_all(&tdx_tdmr_list); >> out_free_tdmrs: >> - /* >> - * Always free the buffer of TDMRs as they are only used during >> - * module initialization. >> - */ >> - free_tdmr_list(&tdmr_list); >> + free_tdmr_list(&tdx_tdmr_list); >> out_free_tdxmem: >> - if (ret) >> - free_tdx_memlist(&tdx_memlist); >> + free_tdx_memlist(&tdx_memlist); >> out_put_tdxmem: >> - /* >> - * @tdx_memlist is written here and read at memory hotplug time. >> - * Lock out memory hotplug code while building it. >> - */ >> put_online_mems(); >> out: >> - /* >> - * For now both @sysinfo and @cmr_array are only used during >> - * module initialization, so always free them. >> - */ >> free_page((unsigned long)sysinfo); >> return ret; >> } > > This diff is extremely hard to follow, but I think the change to error > handling Nikolay proposed has to be applied to the function from the > beginning, not changed drastically in this patch. > I agree. That change should be broken across the various patches introducing each piece of error handling.
On Wed, 2023-06-28 at 15:48 +0300, Nikolay Borisov wrote: > > This diff is extremely hard to follow, but I think the change to error > > handling Nikolay proposed has to be applied to the function from the > > beginning, not changed drastically in this patch. > > > > > I agree. That change should be broken across the various patches > introducing each piece of error handling. No I don't want to do this. The TDMRs are only needed to be saved if we want to do the next patch (reset TDX memory). They are always freed in previous patch. We can add justification to keep in previous patch but I now want to avoid such pattern because I now believe it's not the right way to organize patches: Obviously such justification depends on the later patch. In case the later patch has something wrong and needs to be updated, the justification can be invalid, and we need to adjust the previous patches accordingly. This could result in code review frustration. Specifically for this issue, if we always free TDMRs in previous patches, then it's just not right to do what you suggested there. Also, now with dynamic allocation of TDSYSINFO_STRUCT and CMR array, we need to do 3 things when module initialization is successful: put_online_mems(); kfree(sysinfo); kfree(cmr_array); return 0; out_xxx: .... put_online_mems(); kfree(sysinfo); kfree(cmr_array); return ret; I can hardly say which is better. I am willing to do the above pattern if you guys prefer but I certainly don't want to mix this logic to previous patches.
On Wed, 2023-06-28 at 12:04 +0300, Nikolay Borisov wrote: > > On 26.06.23 г. 17:12 ч., Kai Huang wrote: > > On the platforms with the "partial write machine check" erratum, the > > kexec() needs to convert all TDX private pages back to normal before > > booting to the new kernel. Otherwise, the new kernel may get unexpected > > machine check. > > > > There's no existing infrastructure to track TDX private pages. Change > > to keep TDMRs when module initialization is successful so that they can > > be used to find PAMTs. > > > > With this change, only put_online_mems() and freeing the buffer of the > > TDSYSINFO_STRUCT and CMR array still need to be done even when module > > initialization is successful. Adjust the error handling to explicitly > > do them when module initialization is successful and unconditionally > > clean up the rest when initialization fails. > > > > Signed-off-by: Kai Huang <kai.huang@intel.com> > > --- > > > > v11 -> v12 (new patch): > > - Defer keeping TDMRs logic to this patch for better review > > - Improved error handling logic (Nikolay/Kirill in patch 15) > > > > --- > > arch/x86/virt/vmx/tdx/tdx.c | 84 ++++++++++++++++++------------------- > > 1 file changed, 42 insertions(+), 42 deletions(-) > > > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > > index 52b7267ea226..85b24b2e9417 100644 > > --- a/arch/x86/virt/vmx/tdx/tdx.c > > +++ b/arch/x86/virt/vmx/tdx/tdx.c > > @@ -49,6 +49,8 @@ static DEFINE_MUTEX(tdx_module_lock); > > /* All TDX-usable memory regions. Protected by mem_hotplug_lock. */ > > static LIST_HEAD(tdx_memlist); > > > > +static struct tdmr_info_list tdx_tdmr_list; > > + > > /* > > * Wrapper of __seamcall() to convert SEAMCALL leaf function error code > > * to kernel error code. @seamcall_ret and @out contain the SEAMCALL > > @@ -1047,7 +1049,6 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) > > static int init_tdx_module(void) > > { > > struct tdsysinfo_struct *sysinfo; > > - struct tdmr_info_list tdmr_list; > > struct cmr_info *cmr_array; > > int ret; > > > > @@ -1088,17 +1089,17 @@ static int init_tdx_module(void) > > goto out_put_tdxmem; > > > > /* Allocate enough space for constructing TDMRs */ > > - ret = alloc_tdmr_list(&tdmr_list, sysinfo); > > + ret = alloc_tdmr_list(&tdx_tdmr_list, sysinfo); > > if (ret) > > goto out_free_tdxmem; > > > > /* Cover all TDX-usable memory regions in TDMRs */ > > - ret = construct_tdmrs(&tdx_memlist, &tdmr_list, sysinfo); > > + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, sysinfo); > > nit: Does it make sense to keep passing those global variables are > function parameters? Since those functions are static it's unlikely that > they are going to be used with any other parameter so might as well use > the parameter directly. It makes the code somewhat easier to follow. > I disagree. To me passing 'struct tdx_tdmr_info *tdmr_list' to construct_tdmrs() as parameter makes this function clearer: It takes all TDX memory blocks and sysinfo, generates the TDMRs, and stores them to the buffer specified in the tdmr_list. The internal logic doesn't need to care whether any of of those parameters are static or not.
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index 52b7267ea226..85b24b2e9417 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -49,6 +49,8 @@ static DEFINE_MUTEX(tdx_module_lock); /* All TDX-usable memory regions. Protected by mem_hotplug_lock. */ static LIST_HEAD(tdx_memlist); +static struct tdmr_info_list tdx_tdmr_list; + /* * Wrapper of __seamcall() to convert SEAMCALL leaf function error code * to kernel error code. @seamcall_ret and @out contain the SEAMCALL @@ -1047,7 +1049,6 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list) static int init_tdx_module(void) { struct tdsysinfo_struct *sysinfo; - struct tdmr_info_list tdmr_list; struct cmr_info *cmr_array; int ret; @@ -1088,17 +1089,17 @@ static int init_tdx_module(void) goto out_put_tdxmem; /* Allocate enough space for constructing TDMRs */ - ret = alloc_tdmr_list(&tdmr_list, sysinfo); + ret = alloc_tdmr_list(&tdx_tdmr_list, sysinfo); if (ret) goto out_free_tdxmem; /* Cover all TDX-usable memory regions in TDMRs */ - ret = construct_tdmrs(&tdx_memlist, &tdmr_list, sysinfo); + ret = construct_tdmrs(&tdx_memlist, &tdx_tdmr_list, sysinfo); if (ret) goto out_free_tdmrs; /* Pass the TDMRs and the global KeyID to the TDX module */ - ret = config_tdx_module(&tdmr_list, tdx_global_keyid); + ret = config_tdx_module(&tdx_tdmr_list, tdx_global_keyid); if (ret) goto out_free_pamts; @@ -1118,51 +1119,50 @@ static int init_tdx_module(void) goto out_reset_pamts; /* Initialize TDMRs to complete the TDX module initialization */ - ret = init_tdmrs(&tdmr_list); + ret = init_tdmrs(&tdx_tdmr_list); + if (ret) + goto out_reset_pamts; + + pr_info("%lu KBs allocated for PAMT.\n", + tdmrs_count_pamt_kb(&tdx_tdmr_list)); + + /* + * @tdx_memlist is written here and read at memory hotplug time. + * Lock out memory hotplug code while building it. + */ + put_online_mems(); + /* + * For now both @sysinfo and @cmr_array are only used during + * module initialization, so always free them. + */ + free_page((unsigned long)sysinfo); + + return 0; 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(&tdmr_list); - } + /* + * 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(&tdmr_list); - else - pr_info("%lu KBs allocated for PAMT.\n", - tdmrs_count_pamt_kb(&tdmr_list)); + tdmrs_free_pamt_all(&tdx_tdmr_list); out_free_tdmrs: - /* - * Always free the buffer of TDMRs as they are only used during - * module initialization. - */ - free_tdmr_list(&tdmr_list); + free_tdmr_list(&tdx_tdmr_list); out_free_tdxmem: - if (ret) - free_tdx_memlist(&tdx_memlist); + free_tdx_memlist(&tdx_memlist); out_put_tdxmem: - /* - * @tdx_memlist is written here and read at memory hotplug time. - * Lock out memory hotplug code while building it. - */ put_online_mems(); out: - /* - * For now both @sysinfo and @cmr_array are only used during - * module initialization, so always free them. - */ free_page((unsigned long)sysinfo); return ret; }
On the platforms with the "partial write machine check" erratum, the kexec() needs to convert all TDX private pages back to normal before booting to the new kernel. Otherwise, the new kernel may get unexpected machine check. There's no existing infrastructure to track TDX private pages. Change to keep TDMRs when module initialization is successful so that they can be used to find PAMTs. With this change, only put_online_mems() and freeing the buffer of the TDSYSINFO_STRUCT and CMR array still need to be done even when module initialization is successful. Adjust the error handling to explicitly do them when module initialization is successful and unconditionally clean up the rest when initialization fails. Signed-off-by: Kai Huang <kai.huang@intel.com> --- v11 -> v12 (new patch): - Defer keeping TDMRs logic to this patch for better review - Improved error handling logic (Nikolay/Kirill in patch 15) --- arch/x86/virt/vmx/tdx/tdx.c | 84 ++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 42 deletions(-)