From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 00473208F7AB3 for ; Wed, 5 Jul 2017 07:24:02 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD8D48008D; Wed, 5 Jul 2017 14:25:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DD8D48008D Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com DD8D48008D Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-25.phx2.redhat.com [10.3.116.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id B9D4E5D962; Wed, 5 Jul 2017 14:25:40 +0000 (UTC) To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Marcin Wojtas References: <20170705130434.4525-1-ard.biesheuvel@linaro.org> <3c6585d6-fe8b-3805-7040-ca4decc9c004@redhat.com> From: Laszlo Ersek Message-ID: <77192bd2-37d0-3dcf-fda3-889d5e91a54e@redhat.com> Date: Wed, 5 Jul 2017 16:25:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 05 Jul 2017 14:25:42 +0000 (UTC) Subject: Re: [PATCH] ArmVirtPkg: remove status code support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Jul 2017 14:24:03 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/05/17 15:46, Ard Biesheuvel wrote: > On 5 July 2017 at 14:46, Ard Biesheuvel wrote: >> On 5 July 2017 at 14:43, Laszlo Ersek wrote: >>> On 07/05/17 15:04, Ard Biesheuvel wrote: >>>> Commit 7b1dc6c569a 'ArmVirtPkg: switch to generic ResetSystemRuntimeDxe' >>>> replaced all references in ArmVirtPkg to the deprecated ResetRuntimeDxe >>>> from EmbeddedPkg with the well maintained generic alternative that lives >>>> in MdeModulePkg. >>>> >>>> However, as it turns out, the generic driver has a dependency on the >>>> library class ReportStatusCodeLib, whose default resolution is an >>>> implementation that is not safe for use at runtime, resulting in crashes >>>> when trying to invoke it from the OS. >>>> >>>> Since we have no use for status codes in any of the ArmVirtPkg >>>> platforms, let's replace all resolutions with a common one to the NULL >>>> implementation. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Ard Biesheuvel >>>> --- >>>> ArmVirtPkg/ArmVirt.dsc.inc | 11 ++--------- >>>> 1 file changed, 2 insertions(+), 9 deletions(-) >>> >>> Alternative approach (if we wish to follow what OVMF does): >>> >>> (1) Copy the library resolutions (as appropriate) from OvmfPkg: >>> >>> - SEC, PEI_CORE, PEIM: >>> MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf >>> >>> - DXE_CORE, DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION: >>> MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf >>> >>> - DXE_RUNTIME_DRIVER: >>> MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf >>> >>> (2) Port commit a6d594c5fabd ("OvmfPkg: use StatusCode Router and Handler from MdeModulePkg", 2016-08-03) to ArmVirtPkg. >>> >>> This should result in status code reporting and handling that is functional at runtime as well. >>> >>> What do you prefer? >>> > > > Depends on what status codes actually buy us. I'm clueless > I was similarly clueless :) which is why I asked Cinnamon back then [*] to write a detailed commit message for what would become commit a6d594c5fabd. [*] https://lists.01.org/pipermail/edk2-devel/2016-August/000199.html So, if you read that message, you'll see that roughly, it buys us the following in practice: when a module (boot time or runtime) calls one of the REPORT_STATUS_CODE*() macros, a status code message will be printed to the serial port. That's it :) I don't think it's extremely useful in practice for in-tree modules, given that we -- ArmVirtPkg and OvmfPkg users -- prefer building all modules with high DEBUG levels, add DEBUGs liberally to our own platform code, and always capture the DEBUG output (serial port or QEMU debug port) during boot. For working with out-of-tree modules that rely heavily on status code reporting, or else just to showcase/test the related PPIs and protocols (which is arguably one of the goals of the in-tree virt platforms), enabling the status code libs and drivers can be considered useful. Given that OvmfPkg is already serving this showcase/test purpose, I'm fine with disabling status code stuff in ArmVirtPkg for good; I just wanted you to consider the alternative. So, what say you? :) Thanks Laszlo