From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::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 5164021CF3B6F for ; Wed, 5 Jul 2017 08:32:55 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id m68so115964446ith.1 for ; Wed, 05 Jul 2017 08:34:34 -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=tbMDeAwQLOOnCsM6Ca8VFn0DnrAJL4HuhmwVhEG8lrQ=; b=gMNTjOTHRCOaLs35A896XR4nZ3QDGQpdj80DbuSz253W0Q3ihYoQKR1e9UQH1NqNk9 2BqZGIEritevBOoAtwalRyr23k1EwEvJTfxtVW2g0QEkJLJelaEyFeyo5vl7VhXT0ZCv nGRrPwfas4d3OdtJGJWwoNcsr+AHDd+WukxTI= 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=tbMDeAwQLOOnCsM6Ca8VFn0DnrAJL4HuhmwVhEG8lrQ=; b=gAxmFPlQ0JCZI/jNLO1VXT2zVVwF58aPEIKKhZ4qrOxoNQC758HumOglx0CjJDM1m6 UADBYqLnBv/fHuCH/L150W0feZg7+8381+IX+tqtrDdxekvSJLhWGhBv4TIrhNHer6+o PVtGl+bvmI6gCwYO/vYzlSNxHagtTgW8Zr5f3GIk7MZ+deIjnoTzT41OWgc/Nivw84Fc AQ6dJMDpBKFSi5Icq/c8MFNGPa0IfJDUqJjqkI+e0I1OgPfL+dorsnaE2yLakycvJ41y 3qXXI6q4IcYkLIAPXSDvVlOz+8p3I02Pfy1MQAINJtRQCFQeUmIssEom1HY1n5z4CTi6 ZPkA== X-Gm-Message-State: AKS2vOwu5CoaQuCEQVp2gEQf1FnPnyp92gPy0DO5BL4sEZaYrdc3dvQO Nb8nJAv2kyoq/OU7SiQt69lxly/6PMBgHwgBYg== X-Received: by 10.36.46.134 with SMTP id i128mr42422493ita.98.1499268874222; Wed, 05 Jul 2017 08:34:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.134.134 with HTTP; Wed, 5 Jul 2017 08:34:33 -0700 (PDT) In-Reply-To: References: <20170705130434.4525-1-ard.biesheuvel@linaro.org> <3c6585d6-fe8b-3805-7040-ca4decc9c004@redhat.com> <77192bd2-37d0-3dcf-fda3-889d5e91a54e@redhat.com> From: Ard Biesheuvel Date: Wed, 5 Jul 2017 16:34:33 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Marcin Wojtas 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 15:32:55 -0000 Content-Type: text/plain; charset="UTF-8" On 5 July 2017 at 16:07, Laszlo Ersek wrote: > On 07/05/17 17:01, Ard Biesheuvel wrote: >> On 5 July 2017 at 15:25, Laszlo Ersek wrote: >>> 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 for the explanation. I think one showcase is sufficient, indeed. >> > > In that case :) > > Reviewed-by: Laszlo Ersek > Thanks! Pushed as 59541d41633cf56e9b7c3ac0de112ab65d9331ca