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
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
