Re: [PATCH v12 18/22] x86/virt/tdx: Keep TDMRs when module initialization is successful

[Huang, Kai]

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.