From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by mx.groups.io with SMTP id smtpd.web10.10441.1683325572470163017 for ; Fri, 05 May 2023 15:26:12 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=U1K+4HVZ; spf=pass (domain: gmail.com, ip: 209.85.218.45, mailfrom: vincent.zimmer@gmail.com) Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-957dbae98b4so364118466b.1 for ; Fri, 05 May 2023 15:26:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683325570; x=1685917570; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=yudEzgkyza7wBsSz8qnOJ1s06iqy032sHX0MPtnE25g=; b=U1K+4HVZz/hWwFqtrMYaotiJphcx34T+Q5755zWDWoRdUXbcP3+z2F3K9Zfxyc4kKy vycu5LL07uWfEnNkNik6fZM53C6lzSeJ28OVvDEmqKQ/EGPoTMKG6GfoJFOokeqPIJDV D8XZGjDn7Vj7QRqyFuAVfhbm6EnWrtJi0LV1Ez5LX0T91PBO0nx7rouJW1qU3V/1himI vAFdngFfMVt5cnuEDnT7tz1WObqokA6dkGQzsxcSXOL+yfRpqTbPPhRI7HUxaSzf/FOm +Qgb1W/4YxOQmUbbQe4oMCLeahw5eBmqEOXgJ5I6HscEV/0bWBM7etPRGaMus34RdN+X eQmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683325570; x=1685917570; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yudEzgkyza7wBsSz8qnOJ1s06iqy032sHX0MPtnE25g=; b=LsCKrq2lejpGpzxYo2ztb/16daFUsJY6H0J3091ASLf0dfmBI4k1ItkXEDeQbBtO4J PzupI5G/TVQYOUQFCAfDK6ZohS748KJu4qQW2OJsLjArzcOxpPFnzEilnNOe4a0fqr2p cBRuJaEN+c3Kk+KWoVnkL0sIJXJojmminYCuOppe4e73JR081OjeTEWT6m/4IBN59qv3 niH8xUc7q4ZTygwYXwF7bb+1OgoLRsOK2W5jDg9nGlp4MxdEC5v4fo8HepPLz4Xajp5U 3aHh3nkvQcOcTtlTB6QC/k4DyaesWG4mrrvY2m4Z0bLtpAkbVD2wS/2UoVcyQ2xtwiyM 04mQ== X-Gm-Message-State: AC+VfDxEuFYcU8+yIeoaZhVNTx3/nkYh1lBYb5JIGf3Od5X/KjWOyox4 pgPJHtPtVNxyfdUtx9UYgY7X+/cJwMOPrIE1lwW75k/CgzY= X-Google-Smtp-Source: ACHHUZ6RvsJdf4hwYP86XGNDDJIEryCx2/bgoBJVO6zEOm+e6hQbcgOLyqrfZPPTgS/JIkTIDtAU/i6qeCPPjbJuHTQ= X-Received: by 2002:a17:907:70d:b0:948:eed:b4e0 with SMTP id xb13-20020a170907070d00b009480eedb4e0mr2745113ejb.61.1683325570191; Fri, 05 May 2023 15:26:10 -0700 (PDT) MIME-Version: 1.0 References: <0cf211a5-ddb1-c2a7-1be4-1776c7068496@redhat.com> <607BADB3-C24D-419D-8ECA-AECD6225BBD2@apple.com> <4bccdf95-6700-e1f2-ba8c-a0e946e820b8@redhat.com> In-Reply-To: <4bccdf95-6700-e1f2-ba8c-a0e946e820b8@redhat.com> From: "vincent zimmer" Date: Fri, 5 May 2023 15:25:58 -0700 Message-ID: Subject: Re: [edk2-devel] purpose of EFI_LOCK To: devel@edk2.groups.io, lersek@redhat.com Cc: "Andrew (EFI) Fish" , Mike Kinney , Michael Brown , Gerd Hoffmann Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Random and tl;dr fun friday fact. You can see the efi locks replaced w/ spinlocks in https://github.com/tianocore/edk2-staging/tree/MpNetworkStack to support the use case described in https://uefi.org/sites/default/files/resources/7_Maciej%20Vincent_INTEL_net= work%20stack%20performance.pdf. These changes along w/ a bespoke threading protocol allow for running efi networking code across all cores and still be able to interact w/ an enlightened dxe core. We didn't push forward to standardize for various reasons, such as folks pursuing an alternate solution to the original problem statement (e.g., LinuxBoot for datacenter network provisioning) and the huge legacy app compat we'd face. So it's 'possible', just as you 'could' have i/o device interrupt service routines (isr) w/ their own 'device' tpl's between notify and high (see "firmware interrupts" https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#tpl-usage) if you were to expose the appropriate registration and dispatch interface from the core. I don't think we put anything in staging or elsewhere public w/ isr examples, though, and I expect trying to retrofit after 2+ decades would face similar app compat issues w/ folks who already curate their own proprietary device interrupt sol'n. Vincent On Fri, May 5, 2023 at 12:59=E2=80=AFPM Laszlo Ersek wr= ote: > > 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=E2=80=99m just comment= ing 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 =E2=80=9Cthreads of execution=E2=80=9D is n= ot threads=E2=80=A6. > > > > 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 =E2=80=9Cthreads of execution=E2=80=9D (even= ts) 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=E2=80=99s 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 =C2=B7 107 KB > > > > > > > > > > > > [1] > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#tpl-res= trictions > > > > 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 seem= s > >> 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 t= he > >> TPL we've just set as new. If the old TPL is strictly lower, then we'v= e > >> just "acquired the lock", otherwise we've "already been holding" the > >> lock. So, from this perspective, EfiAcquireLockOrFail() doesn't add mu= ch. > >> > >> - 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 i= n > >> "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 li= ke > >> 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 strict= ly > >> above the Tpl field. > >> > >> Please explain. :) > >> > >> Thanks! > >> Laszlo > >> > > > > > >=20 > >