From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::22b; helo=mail-io0-x22b.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x22b.google.com (mail-io0-x22b.google.com [IPv6:2607:f8b0:4001:c06::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 98325226EAC75 for ; Thu, 12 Apr 2018 03:06:47 -0700 (PDT) Received: by mail-io0-x22b.google.com with SMTP id d6so5687951iog.1 for ; Thu, 12 Apr 2018 03:06:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=lYI2SjHvQMJP/YRLHx5LrsrmR8LauEOunJett1XJUVo=; b=US0Eu2/kLBoQhvV3g6bVQIeR9++9pbO4y/d7EO/UcXvoFLfduxyiV0AKMlyH/xUdV3 jQAfsBDA2EKK3rgn9sA9fdYg+O1of6pc1VX0MwOPY1YDsfqxeS2/oBCIM0d44PriKMsV 2mH//0gh63ACRMfVpGmXHBwF4lx4ymZONOKXA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=lYI2SjHvQMJP/YRLHx5LrsrmR8LauEOunJett1XJUVo=; b=Fy4i7UPtAXW1tpUcPFZKmJddQl7w5zc8nS5wVAp7QKJi8YqxvMyoBV5y4JpbEVqbgc iSy2pKJJloky0bcgnQ3X4seASPvZEsBY+zpzTnwFcG1yeiiqu1GiDkaVuxPfzr3Djowc ihjZrQnpplx0ehM31pbvbE6k99f8esb+IiwhpFPeh7JrI1W6m22B97X50LuzZpEC4bnz jq+xRQDOz2ia4tOp/uRCM8HkfILs1CZbEeUUpBQTT70PCyA0v07EYBGfLLscF9dt+HZ2 IfWBFQepkE0yUUMh/EltWEMtpWbvqRqrWOyHaWWrG2A3aneUqNd8xxoNjjww0vFAlEQD T7tQ== X-Gm-Message-State: ALQs6tDVeN+8PpJwS5Etm2aM2n/UauukcqJv9+LRA81jD4/Slt/wDqO7 JvmtDx0K0kG7chWcMoZI/ifKdeBVm3z8DuZoZrVyFA== X-Google-Smtp-Source: AIpwx49O5wRVMACkZteSrIxLwOzSokwf5AyfsYXWTrgHUTd8iXxNT8m1r0wABO06K7cJWWrQ1MyvyNkUs+j73EODuKw= X-Received: by 10.107.161.199 with SMTP id k190mr2404694ioe.60.1523527606491; Thu, 12 Apr 2018 03:06:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.187.67 with HTTP; Thu, 12 Apr 2018 03:06:46 -0700 (PDT) In-Reply-To: References: <20180412005540.26651-1-lersek@redhat.com> <20180412005540.26651-10-lersek@redhat.com> From: Ard Biesheuvel Date: Thu, 12 Apr 2018 12:06:46 +0200 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Steve Capper , Supreeth Venkatesh Subject: Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Apr 2018 10:06:47 -0000 Content-Type: text/plain; charset="UTF-8" On 12 April 2018 at 11:05, Laszlo Ersek wrote: > On 04/12/18 08:28, Ard Biesheuvel wrote: >> On 12 April 2018 at 02:55, Laszlo Ersek wrote: >>> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called >>> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call >>> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe >>> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX >>> so that we not attempt the call before the PCD driver can successfully >>> read non-volatile variables. >>> >> >> OK, so does this mean that UefiRuntimeServicesTableLib deliberately >> does not depex on gEfiVariableArchProtocolGuid, > > Yes, I do think so; the runtime services are expected to become > available gradually. For example, "MdePkg/Include/Protocol/Variable.h" > has a great long comment on this. > >> and it will return >> EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I >> would assume it is the responsibility of the PcdLib implementation to >> deal with that, not the PcdLib clients. > > Yes, this idea occurred to me as well, but I think it is not correct. > The PcdLib instance for the DXE phase is responsible for talking to the > PCD protocol, and indeed it has a depex on that protocol > (MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses > dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by > non-volatile UEFI variables), then IMO neither the PcdLib instance, nor > the PCD DXE driver, should depend on the variable read service > (gEfiVariableArchProtocolGuid). > > (Well the PCD DXE driver is shared / used by all of the DXE modules, so > it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.) > > Ultimately, in the optimal case, the PcdLib instance for DXE should > "know" if any PCDs used by the main driver module are DynamicHII, and in > that case, the PcdLib instance should create a dependency on the > variable read (and possibly write) services. I think edk2 simply doesn't > support this use case at this moment (it is ultimately a DSC decision > what type we give to a PCD, choosing from the possibilities listed in > the DEC), so we have to take care of it ourselves. I do see that this > limits the reusability of individual drivers across different DSC files > -- if another DSC makes the same PCD fixed, for example, then the > gEfiVariableArchProtocolGuid dependency becomes useless / needlessly > restrictive in the driver. > > We could factor out just the gEfiVariableArchProtocolGuid depex into > another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the > DSC file, because in the DSC we do know that the PCD is DynamicHII. If > you feel strongly about it, it's doable, but I certainly think it's > overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty > unlikely to be reused in any other DSC. (Even ArmVirtXen has a different > implementation, which does not use the PCD.) I feel the proposed patch > should be fine -- what's your take? > I missed the bit where gRT->GetVariable() is only called for HII style dynamic PCDs, so I agree that it makes sense to deal with things as you are doing here.