Messages in this thread Patch in this message | | | Date | Thu, 15 Jun 2023 10:48:17 +0300 | From | Nikolay Borisov <> | Subject | Re: [PATCH v11 12/20] x86/virt/tdx: Allocate and set up PAMTs for TDMRs |
| |
On 4.06.23 г. 17:27 ч., Kai Huang wrote:
<snip>
> /* > * 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 > @@ -487,10 +684,13 @@ static int construct_tdmrs(struct list_head *tmb_list, > if (ret) > return ret; > > + ret = tdmrs_set_up_pamt_all(tdmr_list, tmb_list, > + sysinfo->pamt_entry_size); > + if (ret) > + return ret; > /* > * TODO: > * > - * - Allocate and set up PAMTs for each TDMR. > * - Designate reserved areas for each TDMR. > * > * Return -EINVAL until constructing TDMRs is done > @@ -547,6 +747,11 @@ static int init_tdx_module(void) > * Return error before all steps are done. > */ > ret = -EINVAL; > + if (ret) > + tdmrs_free_pamt_all(&tdx_tdmr_list); > + else > + pr_info("%lu KBs allocated for PAMT.\n", > + tdmrs_count_pamt_pages(&tdx_tdmr_list) * 4);
Why not put the pr_info right after the 'if (ret)' check following tdmrs_setup_pamt_all(). And make the tdmrs_free_pamt_all call unconditional.
It seems the main reason for having a bunch of conditionals in the exit reason is that you share the put_online_mems(); in both the success and failure cases. If you simply add :
put_online_mems(); return 0;
// failure labels follow
Then you can make do without the if (ret) checks and have straight line code doing the error handling.
> out_free_tdmrs: > if (ret) > free_tdmr_list(&tdx_tdmr_list); > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h > index c20848e76469..e8110e1a9980 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.h > +++ b/arch/x86/virt/vmx/tdx/tdx.h > @@ -133,6 +133,7 @@ struct tdx_memblock { > struct list_head list; > unsigned long start_pfn; > unsigned long end_pfn; > + int nid; > }; > > struct tdmr_info_list {
| |