Re: [PATCHv7 10/16] x86/tdx: Convert shared memory back to private on kexec

[Dave Hansen]

On 2/25/24 06:58, Kirill A. Shutemov wrote:
> On Fri, Feb 23, 2024 at 11:39:07AM -0800, Dave Hansen wrote:
..
>> I'd really prefer we find a way to do this with actual locks, especially
>> 'conversion_allowed'.
>>
>> This is _awfully_ close to being able to be handled by a rwsem where the
>> readers are the converters and tdx_kexec_stop_conversion() takes a write.
> 
> Okay, here's what I come up with. It needs more testing.
> 
> Any comments?

Looks a heck of a lot more straightforward to me.

> +static void tdx_kexec_stop_conversion(bool crash)
> +{
> +	/* Stop new private<->shared conversions */
> +	if (!stop_memory_enc_conversion(!crash))
> +		pr_warn("Failed to finish shared<->private conversions\n");
> +}

FWIW, this is one of those places that could use a temporary variable to
help explain what is going on:

	bool wait_for_lock = !crash;

	/*
	 * ... explain why it doesn't or shouldn't take the lock here
	 */
	if (!stop_memory_enc_conversion(wait_for_lock))
		...

This makes it understandable without looking at the called function.

> +bool stop_memory_enc_conversion(bool wait)
> +{
> +	if (!wait)
> +		return down_write_trylock(&mem_enc_lock);
> +
> +	down_write(&mem_enc_lock);
> +
> +	return true;
> +}

This also needs a comment about why the lock isn't being released.