From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d34; helo=mail-io1-xd34.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd34.google.com (mail-io1-xd34.google.com [IPv6:2607:f8b0:4864:20::d34]) (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 6227F2119BBC8 for ; Thu, 3 Jan 2019 01:52:46 -0800 (PST) Received: by mail-io1-xd34.google.com with SMTP id v10so26590033ios.13 for ; Thu, 03 Jan 2019 01:52:46 -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 :cc:content-transfer-encoding; bh=7V0H7nx5w1YMnfSJNIzaWvZsy1KNPafz1mkPu9JJTy4=; b=ZldJQj1Np8Vo3jSSdPTWA6alzelLCW11xWBfrEmK8yysVPFmGJSpTxFusvHu8JA/8A qx01KVvSTLKYTN2t1UshEVbmn1kgJKsUZm1GAJYsSwKYhwahPeDWcknnqLSHd/DGt7JR EbAaxWvKtJmWJrWLSFsdLyWNGb5/jeksB9a7o= 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:cc:content-transfer-encoding; bh=7V0H7nx5w1YMnfSJNIzaWvZsy1KNPafz1mkPu9JJTy4=; b=ifNGujjzaxJumViqeKvDbXvokR5AMlBhsI2Y2SvVVg5OTq/Se1eV2ELdrgFJHhWRXi cGWAK8ArThN5SWsRd9gASQtTlpWHdE0itoCEYIvC2Vn1kEatomh+h4/uESaPusCddq9D KQ7Ky+qgzGQa3c7ETUlL90mgm4M6I47lwCwfcYfETGkuhiqZK2ofaKOPB1csmwigr9Aq MICm18Qy7L03461t29XQSlLj6PXnLhyr3OTA8j9GtU+zaUDihIqlDDgcpwC7UFrsi1rN 3Wt49jmfL9se7H3e7mhXDXa0T0tm9+id58DY3Z6QuGc+Ai62zy8nTpBpYcacyGSBcx7P zaDA== X-Gm-Message-State: AJcUukcHIwLifd3iM0uG1/BRGOO77PHwuY8ezbLQ9/VHe1spaXIksRNE +Vnh9qceMdQbTI7BgrCAdgOmKzRcyUt9Px/tgEIYHg== X-Google-Smtp-Source: ALg8bN5c1xC+ZdSDzF6YQYvZY1FYMG6O1kTVweoDgdOzIDg9lEZj0DdrZfUm7No8MhbbtvLg8yTwDPcs/wFb6g7u9e8= X-Received: by 2002:a5e:c206:: with SMTP id v6mr16397965iop.60.1546509165271; Thu, 03 Jan 2019 01:52:45 -0800 (PST) MIME-Version: 1.0 References: <1544789607-11316-1-git-send-email-jagadeesh.ujja@arm.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E38D3C3@SHSMSX104.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E38DE09@SHSMSX104.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E38F557@SHSMSX104.ccr.corp.intel.com> In-Reply-To: From: Ard Biesheuvel Date: Thu, 3 Jan 2019 10:52:34 +0100 Message-ID: To: Jagadeesh Ujja Cc: "Gao, Liming" , "edk2-devel@lists.01.org" , "Zhang, Chao B" Subject: Re: [PATCH 00/13] Extend secure variable service to be usable from Standalone MM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Jan 2019 09:52:46 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 3 Jan 2019 at 08:43, Jagadeesh Ujja wrote: > > hi Ard > > On Wed, Jan 2, 2019 at 10:45 PM Ard Biesheuvel > wrote: > > > > On Thu, 20 Dec 2018 at 15:23, Gao, Liming wrote: > > > > > > Jagadeesh: > > > MdeModulePkg Variable service/Fault tolerant/Nor Flash driver depen= ds on StandaloneMmServicesTableLib library class header file. This header f= ile is added into MdePkg. It has two interfaces. One is global gMmst, anoth= er is function InMm(). So, there is no dependency issue here. > > > And, MdePkg adds one StandaloneMmServicesTableLib library INF with em= pty implementation, this library is just for build. It sets gMmst=3DNULL, a= nd always return FASLE in InMm(). This library can be used in MdeModulePkg.= dsc to make Variable driver pass build. There is also no dependency issue h= ere. Last, Platform DSC file will refer to the real StandaloneMmServicesTab= leLib library INF from StandaloneMmPkg. > > > > > > > I think we should avoid the need for InMm() altogether for standalone > > MM. It will always return TRUE for standalone MM modules, and it will > > always return FALSE for other modules, so the distinction should be > > made at build time. > > > > This means that we need to refactor the SMM 'server' modules and/or > > libraries so that any code they cannot share (like boot services > > invocations) are only included in the classic SMM versions. > > > > I have pushed my own prototype code here: > > https://github.com/ardbiesheuvel/edk2/commits/standalone-mm > > > > There is some overlap with Jagadeesh's work. I will work with him > > directly to resolve this before posting any new revisions. > > > InMm()=E2=80=9D and =E2=80=9CPcdStandaloneMmVariableEnabled=E2=80=9D are= defined to reuse the > existing code as much as possible. > Initially I have done separate copy of the file to avoid =E2=80=9Cif..els= e=E2=80=9D > but had a comment about =E2=80=9Cduplicating code primarily due to the > maintenance overhead=E2=80=9D > We shouldn't rely on runtime functions and PCDs to make build time decision= . Lots of the SMM code can be refactored. As Jian suggested, we could introduce a helper library with implementations for the MM protocol handling and memory allocation routines exposed via the system table, so that the users can invoke the abstract library. As for the various boot services calls: these just have to be factored out or replaced. - gBS->CalculcateCrc32 () in the FTW driver can just be replaced with the version from BaseLib (but only for the standalone MM version) - MorLockInitAtEndOfDxe() in the variable driver attempts to locate some TCG protocols to infer whether the variable may have been set by platform firmware, this code just needs to be dropped in the standalone MM version - the ArmPkg NOR flash driver needs some refactoring in any case, since all the block I/O and disk I/O routines can be dropped for standalone MM. So of course, we have to take the maintenance burden into account, and so we have to refactor carefully so that we share as much code as possible. But relying on InMm() and PCDs is not acceptable. > So we are using =E2=80=9CInMm()=E2=80=9D and =E2=80=9CPcdStandaloneMmVari= ableEnabled=E2=80=9D PCD flag > and trying to use the same code as much as possible. > > The patchset =E2=80=9CExtend secure variable service to be usable from > Standalone MM=E2=80=9D as POC was submitted as RFC patches on =E2=80=9COc= tober 31, > 2018=E2=80=9D. > Subsequent comments are fixed and we had 7 version of the patch set > under review. > I'm not sure why you are bringing this up, but it is irrelevant. This is important stuff that touches a lot of different packages, so if it takes 20 revisions, so be it.