On Wed, 2023-06-28 at 17:32 -0700, Dave Hansen wrote: > On 6/28/23 02:20, Nikolay Borisov wrote: > > > > > > + /* > > > + * Starting from this point the system may have TDX private > > > + * memory. Make it globally visible so tdx_reset_memory() only > > > + * reads TDMRs/PAMTs when they are stable. > > > + * > > > + * Note using atomic_inc_return() to provide the explicit memory > > > + * ordering isn't mandatory here as the WBINVD above already > > > + * does that. Compiler barrier isn't needed here either. > > > + */ > > > > If it's not needed, then why use it? Simply do atomic_inc() and instead > > rephrase the comment to state what are the ordering guarantees and how > > they are achieved (i.e by using wbinvd above). > > Even better, explain why the barrier needs to be there and *IGNORE* the > WBVIND. > > If the WBINVD gets moved -- or if the gods ever bless us with a halfway > reasonable way to flush the caches that's not full serializing -- this > code is screwed. > > There is _zero_ reason to try and "optimize" this junk by trying to get > rid of a memory barrier at the risk of screwing it over later. > > I use "optimize" in quotes because that's a highly charitable way of > describing this activity. > Agreed. I'll try to explain this well and come back. Thanks!
