From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web12.8275.1578416145727042718 for ; Tue, 07 Jan 2020 08:55:46 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=qgOilzgH; spf=pass (domain: linaro.org, ip: 209.85.221.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f68.google.com with SMTP id c14so164356wrn.7 for ; Tue, 07 Jan 2020 08:55:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=d9d7madNE883xup3LFs0h98Mun1SQQaOj2YjcGUxLek=; b=qgOilzgHT10YSvARVYs9YnGrrmPRfzWr+woAPAD2oJiB1JI4JmPp+5eY0Obkje9k8B WJZLFuLJ55NVJJBgQB/NH18uMZtwpyiAIp6K5M2rPgV/H76sGJpHQtUzmBjEbhOcQjbb VASX0mOxZDQF9xGfvlOmmi4wQQPmygJ3j2nBjqgTFKTvPMywYon4qSEuQmrm+qoj5Ia+ s2QVKFlv7TVq6sReFgXiHUK5RCooCUf659cboZMtorOPSOgC8TsQWZNVtaAqttafKMmr 3avIsGX0xBa2cpMBRMa6azCbZqqn4zaI2faXlFayYCZx5oHq2TdSr1yS7aVr11FVv94y aWFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=d9d7madNE883xup3LFs0h98Mun1SQQaOj2YjcGUxLek=; b=iMWPE2h5y2ESCu54ghsRLD9Z3DlzIQssklPzJVxdkVpKisWbPlTAeQXLXljMS+KKOA jAJ/+DFP+T3IQ31ipYBWRIYGD8OKGdIzAVM6TkC730zsoMsAv/iVjvhNj6KAMGoznsvm UAhe+5H6ErvI30zz+VdG9MAe5b2VvhjGDgNYcC5G9Nhq+KzTABOC5b3sCxW2epdIB+s3 SnoB5JlfPwYVLE2guPjjV/+SJMunrj368w5X/cxCQVrP9LWd55/XHAZOp/wLMC3btRvj I2hq0vtzm8mg87sK3kN7KwnRVG4vC8BmD5ihQc/Jlv48LXaloaysLfJfuyAMvPn0p/5D qLBg== X-Gm-Message-State: APjAAAWilMiZWIqiP1e3h/ZC4U9Aihq9dgHlnzXUx17gLDRf20wACLFT PqQJNpy92VKQiSUuM6WpAmOiggY+imVdKI7JgOefHkb6zQQWIg== X-Google-Smtp-Source: APXvYqwstEXTOj3OEeyGv0hR2jQRC5xyClwELYa3jIZ0ymj9p2K7yE93i2P/yHWcnUoQ/wYSG/f5Y0+VUuwFYMXy4A0= X-Received: by 2002:a5d:5345:: with SMTP id t5mr42244wrv.0.1578416143809; Tue, 07 Jan 2020 08:55:43 -0800 (PST) MIME-Version: 1.0 References: <20200107094800.4488-1-ard.biesheuvel@linaro.org> <20200107094800.4488-4-ard.biesheuvel@linaro.org> <913896f6-bd27-3942-1f79-20e347b48573@redhat.com> In-Reply-To: <913896f6-bd27-3942-1f79-20e347b48573@redhat.com> From: "Ard Biesheuvel" Date: Tue, 7 Jan 2020 17:55:32 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 3/4] ArmVirtPkg/PlatformPeiLib: implement Reset2 PPI based on PSCI To: edk2-devel-groups-io , Laszlo Ersek Content-Type: text/plain; charset="UTF-8" On Tue, 7 Jan 2020 at 17:50, Laszlo Ersek wrote: > > On 01/07/20 10:47, Ard Biesheuvel wrote: > > Extend the existing DT traversal routine in PlatformPeiLib with > > discovery of the PSCI method, and expose an implementation of the > > Reset2 PPI based on the method found. > > > > This satisfies a dependency of Tcg2Pei, which needs to reset the > > platform in some cases. Since there are no other uses for system > > reset in PEI on ArmVirtQemu, simply expose the PPI directly rather > > than using the generic ResetSystemPei and the associated plumbing. > > > > Signed-off-by: Ard Biesheuvel > > --- > > ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf | 3 + > > ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c | 123 ++++++++++++++++++++ > > 2 files changed, 126 insertions(+) > > Tcg2Pei uses ResetCold() from ResetSystemLib. > > ArmVirtPkg's existent ResetSystemLib instance > (ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf) is only > suitable for DXE_DRIVER and DXE_RUNTIME_DRIVER instances. It uses our > FDT Client protocol for looking up (I think) more or less the same > things that you parse here. > > As a PEI phase replacement, this patch produces gEfiPeiReset2PpiGuid, > and then in patch#4, we resolve ResetSystemLib, for PEIMs, to the > > MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf > > instance, which depends on the PPI installed here. > > I'm not too happy about installing the gEfiPeiReset2PpiGuid from > PlatformPeiLib. > > As replacement, it's not ResetSystemPei what I'd recommend (which > depends on a PEI-phase ResetSystemLib instance anyway, which we don't > have, in the first place). > > (1) Instead, I'd recommend implementing a PEI-phase ResetSystemLib for > ArmVirtQemu. (Under ArmVirtPkg/Library/ArmVirtPsciResetSystemLib -- new > INF file.) > > Would that be a large burden? I think we'd need a helper function in > that lib instance, for returning HVC versus SMC (from the FDT), and then > we'd have to call the proper interface for the actual reset. Not fast, > but resets don't need to be fast I think. > That is what I started out with. The problem is that I am not 100% convinced that it is safe to dereference the initial FDT base address at arbitrary times during PEI, and so it would be better to consume the FDT from the GUIDed HOB. That, however, creates another ordering issue, and so we should install a PPI that signals the availability of the FDT GUIDed HOB. At this point, I decided it would be better to just produce the PPI in the PlatformPeiLib, but I agree it is not the cleanest approach. > BTW I think the following modules are never meant to be used together: > > MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf > MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf > > because they seem to depend mutually on each other's abstract interface > (PPI vs. lib class). So I think a given platform should include *at most > one* of these, on top of the "other" facility that it already exposes. > In ArmVirtQemu's case, I'd suggest implementing the lib class for PEI, > and then we can include "ResetSystemPei" whenever the need arises. > The idea is that other PEIMs use the library, which is backed by the PEIM, so that any hooks and notifications that occur at reset time can be dispatched correctly. If every PEIM that needs to reset the system calls into a library directly, this no longer works.