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

[Kirill A. Shutemov]

On Tue, Jan 16, 2024 at 10:28:22AM +0300, Kirill A. Shutemov wrote:
> > > @@ -809,12 +823,25 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> > >   static int tdx_enc_status_change_prepare(unsigned long vaddr, int numpages,
> > >   					 bool enc)
> > >   {
> > > +	atomic_inc(&conversions_in_progress);
> > > +
> > > +	/*
> > > +	 * Check after bumping conversions_in_progress to serialize
> > > +	 * against tdx_shutdown().

s/tdx_shutdown/tdx_kexec_stop_conversion/

> > > +	 */
> > > +	if (!conversion_allowed) {
> > > +		atomic_dec(&conversions_in_progress);
> > > +		return -EBUSY;
> > > +	}
> > 
> > nit: Can you make the inc of conversions_in_progress be done here, this
> > eliminated the atomic_dec in case they aren't. Somewhat simplifies the
> > logic.
> 
> Okay, fair enough. Will change.

Actually, no, it will breaks serialization.

Consider following scenario (includes change you proposed):

	    CPU0			      CPU1

tdx_enc_status_change_prepare()
  if (!conversion_allowed) // false
					tdx_kexec_stop_conversion()
					  conversion_allowed = false;
					  <...>
					  <return as no conversion in flight>
  <continue with conversion>

Incrementing conversion_in_progress before checking conversion_allowed
will make tdx_kexec_stop_conversion() to spin until CPU0 is done with
conversion.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov