On Mon, Apr 06, 2020 at 09:15:57AM -0700, Sean Christopherson wrote:
> > encl->ssaframesize = secs->ssa_frame_size;
> > + encl->mm_list_version = 1;
>
> This is unnecessary. A mm_list_version of '0' means the list walk started
> when there were no mm structs associated with the enclave, i.e. skipping
> everything related to walking the list is ok. It's subtle, and I dislike
> relying on that behavior, but IMO it's preferable to incorrectly implying
> that a list version of '0' is somehow bad.
'0' means whatever code requires to mean. There is no absolute meaning.
> > + for ( ; ; ) {
> > + next = encl->mm_list_version;
> >
> > - down_read(&encl_mm->mm->mmap_sem);
> > + if (version == next)
> > + break;
>
> Functionally this works, but I personally find it the logic kludgy, and it
> generates worse code. Not that we're at the point where counting uops is a
> to priority, but I don't think it makes sense to go out of our way to make
> the resulting code worse.
I'll pick v2 then.
/Jarkko
