From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id E60F574003C for ; Fri, 20 Oct 2023 12:28:12 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=31DfAoB0h6QF1N7ZcZZG0op4gY33Sjx3urccC4LaN2o=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1697804891; v=1; b=B4SsxfMjgnvKblOSn8pyzlZfp5BL8fhpAAykYmduL0v3S4hq49TdmB9LPJgAkybZP95+wBul sGr+m9tLX6vPVM+URm4kA0Sls8H40k8XFruXfbBTWKP97O7j7xO0e6Vrfb3jGihyxNA9Whdf3uv jd/MlhGFJtWyAnfKPAYlYqwE= X-Received: by 127.0.0.2 with SMTP id FXyfYY7687511xFKFgLlKf9a; Fri, 20 Oct 2023 05:28:11 -0700 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.53110.1697804890984567487 for ; Fri, 20 Oct 2023 05:28:11 -0700 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 6297F6212E for ; Fri, 20 Oct 2023 12:28:10 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 13200C433C7 for ; Fri, 20 Oct 2023 12:28:10 +0000 (UTC) X-Received: by mail-lj1-f174.google.com with SMTP id 38308e7fff4ca-2c515527310so10875651fa.2 for ; Fri, 20 Oct 2023 05:28:09 -0700 (PDT) X-Gm-Message-State: Bq7A0rPDSvmjgzZjDv8O9hg0x7686176AA= X-Google-Smtp-Source: AGHT+IFoO63DlDYjmEnfhRI8VQ5vTi86elXEyATKyM6b7VFbXiA4WwNVN0AdqegL/p5tLhrJYl3w8kdOK2so775j1d0= X-Received: by 2002:a2e:8656:0:b0:2c5:1c9d:7f81 with SMTP id i22-20020a2e8656000000b002c51c9d7f81mr1433923ljj.32.1697804888258; Fri, 20 Oct 2023 05:28:08 -0700 (PDT) MIME-Version: 1.0 References: <20231020121748.44862-1-lersek@redhat.com> In-Reply-To: <20231020121748.44862-1-lersek@redhat.com> From: "Ard Biesheuvel" Date: Fri, 20 Oct 2023 14:27:56 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] ArmPlatformPkg/PL031RealTimeClockLib: remove superfluous instance init steps To: Laszlo Ersek Cc: devel@edk2.groups.io, Ard Biesheuvel , Leif Lindholm Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=B4SsxfMj; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Fri, 20 Oct 2023 at 14:17, Laszlo Ersek wrote: > > RealTimeClockLib instances are consumed by edk2's > EmbeddedPkg/RealTimeClockRuntimeDxe driver. In its entry point function > InitializeRealTimeClock(), the driver: > > (1) calls LibRtcInitialize(), > > (2) sets the GetTime(), SetTime(), GetWakeupTime() and SetWakeupTime() > runtime services to its own similarly-named functions -- where those > functions wrap the corresponding RealTimeClockLib APIs, > > (3) installs EFI_REAL_TIME_CLOCK_ARCH_PROTOCOL with a NULL protocol > interface. > > Steps (2) and (3) conform to PI v1.8 sections II-9.7.2.4 through > II-9.7.2.7. > > However, this means that LibRtcInitialize() (of any RealTimeClockLib > instance) should not itself (a) set the GetTime(), SetTime(), > GetWakeupTime() and SetWakeupTime() runtime services, nor (b) install > EFI_REAL_TIME_CLOCK_ARCH_PROTOCOL. The runtime service pointers will be > overwritten in step (2) anyway, and step (3) will uselessly install a > second (NULL-interface) EFI_REAL_TIME_CLOCK_ARCH_PROTOCOL instance in the > protocol database. (The protocol only serves to notify the DXE Foundation > about said runtime services being available.) > > Clean up ArmPlatformPkg/PL031RealTimeClockLib accordingly (it only has > code that's redundant for step (3); it does not try to set "gRT" fields). > > (Note that the lib instance INF file already does not list > gEfiRealTimeClockArchProtocolGuid.) > > Tested with ArmVirtQemu. > > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4565 > Signed-off-by: Laszlo Ersek > --- > > Notes: > context:-W > > ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 13 ------------- > 1 file changed, 13 deletions(-) > Acked-by: Ard Biesheuvel Thanks a lot for cleaning up this mess. > diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > index 9e852696d2fd..1896f9d16d3b 100644 > --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c > @@ -1,34 +1,32 @@ > /** @file > Implement EFI RealTimeClock runtime services via RTC Lib. > > Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
> Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.
> Copyright (c) 2019, Linaro Ltd. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > #include > > #include > #include > > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > > -#include > - > #include "PL031RealTimeClock.h" > > STATIC BOOLEAN mPL031Initialized = FALSE; > @@ -307,52 +305,41 @@ EFIAPI > LibRtcInitialize ( > IN EFI_HANDLE ImageHandle, > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > EFI_STATUS Status; > - EFI_HANDLE Handle; > > // Initialize RTC Base Address > mPL031RtcBase = PcdGet32 (PcdPL031RtcBase); > > // Declare the controller as EFI_MEMORY_RUNTIME > Status = gDS->AddMemorySpace ( > EfiGcdMemoryTypeMemoryMappedIo, > mPL031RtcBase, > SIZE_4KB, > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > ); > if (EFI_ERROR (Status)) { > return Status; > } > > Status = gDS->SetMemorySpaceAttributes (mPL031RtcBase, SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > if (EFI_ERROR (Status)) { > return Status; > } > > - // Install the protocol > - Handle = NULL; > - Status = gBS->InstallMultipleProtocolInterfaces ( > - &Handle, > - &gEfiRealTimeClockArchProtocolGuid, > - NULL, > - NULL > - ); > - ASSERT_EFI_ERROR (Status); > - > // > // Register for the virtual address change event > // > Status = gBS->CreateEventEx ( > EVT_NOTIFY_SIGNAL, > TPL_NOTIFY, > VirtualNotifyEvent, > NULL, > &gEfiEventVirtualAddressChangeGuid, > &mRtcVirtualAddrChangeEvent > ); > ASSERT_EFI_ERROR (Status); > > return Status; > } -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109848): https://edk2.groups.io/g/devel/message/109848 Mute This Topic: https://groups.io/mt/102079629/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-