From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.6451.1683316794671892366 for ; Fri, 05 May 2023 12:59:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=KQ13Ag2h; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683316793; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qk9rlBlJBwvgHHHxaeEZTRyS9rwyFWgjmajhtrcQqt4=; b=KQ13Ag2hvtFyWzLUXDqOg0Zv+LoIYqIZJvoZN6ekGUFvWuEL5a3yw/6RGZl/9V4TMzlUQw dvUvYcLNfPS2f5ZzTa9RnsOdERMqhRWxBrtFt6LehdfiP9ApkXoCzh/FfuC6Uski5BXook frnoE06Y3gm1LEUJRenJQl/4O/QBLgQ= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-378-J2cl7qa6NkGohjAxrsbGwA-1; Fri, 05 May 2023 15:59:52 -0400 X-MC-Unique: J2cl7qa6NkGohjAxrsbGwA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 464202814259; Fri, 5 May 2023 19:59:52 +0000 (UTC) Received: from [10.39.193.58] (unknown [10.39.193.58]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3DCE540C6F41; Fri, 5 May 2023 19:59:51 +0000 (UTC) Message-ID: <4bccdf95-6700-e1f2-ba8c-a0e946e820b8@redhat.com> Date: Fri, 5 May 2023 21:59:49 +0200 MIME-Version: 1.0 Subject: Re: purpose of EFI_LOCK To: "Andrew (EFI) Fish" Cc: edk2-devel-groups-io , Mike Kinney , Michael Brown , Gerd Hoffmann References: <0cf211a5-ddb1-c2a7-1be4-1776c7068496@redhat.com> <607BADB3-C24D-419D-8ECA-AECD6225BBD2@apple.com> From: "Laszlo Ersek" In-Reply-To: <607BADB3-C24D-419D-8ECA-AECD6225BBD2@apple.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Andrew, thank you! The background / paragraph on the naming helps! Cheers, Laszlo On 5/4/23 20:01, Andrew (EFI) Fish wrote: > Laszlo, > > Hope you are doing well! Sorry to top post but I’m just commenting on > the big picture, not answering your specific questions yet. > > The Wiki definition of lock  is something like: In computer science > locks are a mechanism that enforces limits on access to a resource when > there is the possibility for contention between threads of execution. I > think the key point here is “threads of execution” is not threads…. > > UEFI is single threaded with a cooperative event model. There is no > scheduler and an event blocks all forward progress of any event at the > same or lower TPL. The protocol services have defined TPL Restrictions > [1] so that is possible to implement locking. In the context of EFI > raising the TPL blocks any “threads of execution” (events) that could > preempt the running code from contending with a critical section.  > > Lets thing about what would happen if you use an atomic primitive as a > lock in EFI. Let’s say the app is installing a protocol so the DXE Core > has the protocol database locked. Any event that fired in that window > would not be able to call any UEFI Boot Service that was related to > protocols and expect it to succeed. If the event blocked on the lock, > the system is dead locked. If the the lock was tested and failed that > basically means it would be normal for any UEFI service to fail in > events and event code needed to coded to deal with that. Basically the > even could we need to defer to a future time the event gets signaled. I > think think this quickly devolves in the event code having to implement > a simple scheduler for its set. Thus making locks raise the TPL is just > better for everyone.  > > Not that we have always been good at naming things, but in the context > of EFI a Lock is best implemented as raising TPL so we made an up level > look API to make that clear to people, and to help educate people how > locks should be implemented in EFI. > > This is old, but it is a good sumarry of why we did not want BIOS > programmers dealing with threads 20+ years ago when we designed EFI.  > > > preview.png > threads > PDF Document · 107 KB > > > > > > [1] > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#tpl-restrictions > > Thanks, > > Andrew Fish > >> On May 4, 2023, at 1:45 AM, Laszlo Ersek wrote: >> >> Hi, >> >> what benefit does EFI_LOCK add, over direct gBS->RaiseTPL() and >> gBS->RestoreTPL() calls? >> >> Considering just the two APIs EfiAcquireLock() and EfiReleaseLock(): >> >> - The "Lock" field (effectively, lock status field) is useless; it is >> only written to, never read (outside of ASSERT()s) >> >> - The "OwnerTpl" and "Tpl" fields are just convenience storage for the >> new TPL we raise to, and the old TPL we restore. >> >> Considering the EfiAcquireLockOrFail() API as well: >> >> - This does read the "Lock" (lock status) field, and if it is >> EfiLockAcquired, the RaiseTPL() call is refused. So the idea here seems >> to be to ensure a *strict* rise in the TPL. Namely, the RaiseTPL() in >> EfiAcquireLockOrFail() would still succeed after the RaiseTPL() in >> EfiAcquireLock() -- it is valid to "raise" the TPL to the current TPL >> --, but the lock status check prevents that. >> >> - However (#1), this same "strict" raise would be possible by just >> calling RaiseTPL() again, and comparing the returned old TPL against the >> TPL we've just set as new. If the old TPL is strictly lower, then we've >> just "acquired the lock", otherwise we've "already been holding" the >> lock. So, from this perspective, EfiAcquireLockOrFail() doesn't add much. >> >> - Furthermore (#2), if another agent raised the TPL already, but didn't >> use our particlar EFI_LOCK object to do that, then the status stored in >> "EFI_LOCK.Lock" will not be able to track anything. >> >> So what is EFI_LOCK good for? >> >> Effectively I'm disturbed by the *name* "EFI_LOCK". A lock (or mutex) is >> supposed to protect some shared resource. If two agents access the >> resource without first acquiring the lock, there's trouble. Therefore >> the resource itself becomes qualified as "requiring protection by the >> lock, at all times". However, the "current TPL" is *not* a resource like >> that. It's a UEFI spec level concept; the RaiseTPL and RestoreTPL boot >> services are available to any agents; those agents are *not* required to >> go through any kind of extra mutual exclusion. >> >> I've also considered that maybe EFI_LOCK could provide some protection >> against inadvertently *lowering* the TPL via RaiseTPL() (which is >> undefined behavior, per spec). But I don't see how -- >> EfiAcquireLockOrFail() will happily lower the TPL if the Lock field is >> EfiLockReleased, but another agent has meanwhile raised the TPL strictly >> above the Tpl field. >> >> Please explain. :) >> >> Thanks! >> Laszlo >> >