From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=17.151.62.66; helo=nwk-aaemail-lapp01.apple.com; envelope-from=afish@apple.com; receiver=edk2-devel@lists.01.org Received: from nwk-aaemail-lapp01.apple.com (nwk-aaemail-lapp01.apple.com [17.151.62.66]) (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 AC362211DBE1D for ; Tue, 12 Mar 2019 10:28:18 -0700 (PDT) Received: from pps.filterd (nwk-aaemail-lapp01.apple.com [127.0.0.1]) by nwk-aaemail-lapp01.apple.com (8.16.0.27/8.16.0.27) with SMTP id x2CHLk4p051416; Tue, 12 Mar 2019 10:28:11 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=mime-version : content-type : sender : from : message-id : subject : date : in-reply-to : cc : to : references; s=20180706; bh=gLCODOnBY4YQJBxjX7YQOZnY2WBhVLZNKE58ebnJ5lQ=; b=WEbvUwFp92eySdtSrkn2L/SRMOrGXv/Qr6ftqY4SUZmyZSizXFbOUBmEfqmACNcz6FqY 3IsqkTixIkB3vHJloeQ3I2TqzsnZd+DqBqWp38eOrss9mFf99ZguOAuh4Fkbz51cb0sz SDqWqPLiYrdaa5L4Xb5ALR7aQGb7SSCxRE4eEetllLmlfBRdSIxcUrSzW8N0jcZDtyjR Wck1F0ZXbUBLhvBYm4QTZmUFjRPOQUnMLzY7fn0AjVs5OmhsBOCEx6eA+R9dT4E1j4pT E8pwjG2KwGIsTD6ZT/d1zlXB0m5khzlk0dGZideDg+/7CmUtLLbGQRzMJGAs4dUO3JE5 Ew== Received: from mr2-mtap-s03.rno.apple.com (mr2-mtap-s03.rno.apple.com [17.179.226.135]) by nwk-aaemail-lapp01.apple.com with ESMTP id 2r4da88rmq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 12 Mar 2019 10:28:10 -0700 MIME-version: 1.0 Received: from nwk-mmpp-sz12.apple.com (nwk-mmpp-sz12.apple.com [17.128.115.204]) by mr2-mtap-s03.rno.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) with ESMTPS id <0PO9008MTKIYD051@mr2-mtap-s03.rno.apple.com>; Tue, 12 Mar 2019 10:28:10 -0700 (PDT) Received: from process_milters-daemon.nwk-mmpp-sz12.apple.com by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) id <0PO900M00JN9ED00@nwk-mmpp-sz12.apple.com>; Tue, 12 Mar 2019 10:28:10 -0700 (PDT) X-Va-A: X-Va-T-CD: 13715775cfe6ed78bc954dbcb503dbb2 X-Va-E-CD: a431671bb237934a5bcc41bc072417b2 X-Va-R-CD: 79d1152b22247da7f6f5c537db83cf3d X-Va-CD: 0 X-Va-ID: 96bf3c26-f1fd-44a5-bf7e-a757147dc904 X-V-A: X-V-T-CD: 13715775cfe6ed78bc954dbcb503dbb2 X-V-E-CD: a431671bb237934a5bcc41bc072417b2 X-V-R-CD: 79d1152b22247da7f6f5c537db83cf3d X-V-CD: 0 X-V-ID: 6127d3bc-6539-4768-8eff-d25239b273a4 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-03-12_10:,, signatures=0 Received: from [17.226.41.73] (unknown [17.226.41.73]) by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.3.20181024 64bit (built Oct 24 2018)) with ESMTPSA id <0PO900LO8KIXUQA0@nwk-mmpp-sz12.apple.com>; Tue, 12 Mar 2019 10:28:09 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish Message-id: Date: Tue, 12 Mar 2019 10:28:07 -0700 In-reply-to: Cc: Heyi Guo , Ard Biesheuvel , Peter Maydell , Hao Wu , "edk2-devel@lists.01.org" , Julien Grall , wanghaibin 00208455 To: Laszlo Ersek References: <1551341112-21645-1-git-send-email-guoheyi@huawei.com> <27394137-8c32-4f8a-c029-5fc883a2cce0@huawei.com> X-Mailer: Apple Mail (2.3445.6.18) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-03-12_10:, , signatures=0 X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [RFC 0/3] Enable runtime serial debug for ArmVirtQemu 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: Tue, 12 Mar 2019 17:28:18 -0000 Content-Type: text/plain; CHARSET=US-ASCII Content-Transfer-Encoding: 7BIT > On Mar 12, 2019, at 10:05 AM, Laszlo Ersek wrote: > > Hello Heyi, > > On 03/12/19 07:56, Heyi Guo wrote: >> Hi Laszlo, >> >> I'm thinking about a proposal as below: >> >> 1. Reuse the framework of >> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf >> >> 2. The boot time behavior of this module is not changed >> 3. Switch to status code protocol for runtime debug >> 4. Use an overridden SerialPortLib instance for >> StatusCodeHandlerRuntimeDxe; the instance must support runtime access. >> >> Then we can have below benefits: >> 1. the boot time firmware log, and the OS log, went to one port; and >> there is no dependence for runtime drivers to output serial message at >> boot time. >> 2. By implementing different instances of SerialPortLib for >> StatusCodeHandlerRuntimeDxe, we can have enough flexibility to direct >> runtime serial message to platform specific serial port. > > This looks a bit over-engineered to me. Ultimately our goal is: > - for DEBUGs to reach "a" serial port at runtime -- namely one that > differs from the "normal" serial port. > > Given that you'd have to implement a "special" SerialPortLib instance > just for StatusCodeHandlerRuntimeDxe anyway, can we perhaps: > > (1) Undo -- dependent on a build flag? -- commit ebfca258f5d7; i.e., > stick universally with BaseDebugLibSerialPort, for DebugLib, > > (2) add a new implementation of "FdtPL011SerialPortLib.inf" with both > (a) runtime behavior and (b) handling of a special serial port? > > In other words, push down the "runtime?" distinction from DebugLib (see > commit 7f029e1b31ee) to SerialPortLib. The new SerialPortLib instance > would be used only with DXE_RUNTIME_DRIVERs. > > The lib instance would prepare everything in advance for accessing the > "special" serial port at runtime (which could require allocating runtime > memory in advance). Once ExitBootServices() is called, a callback should > switch over the behavior of the lib instance, without allocating further > memory. (The switched-over behavior would not have to be functional > until the virtual address map is set.) > Laszlo, Runtime does not quite work like that. As soon as the Set Virtual Address map fires it is only legal to call things EFI RT from the virtual mapping. The last stage of the Set Virtual Address Map call is to reapply the fixups in the Runtime Drivers for the virtual address space. That means any absolute address reference in the code now points to the virtual address. Also the Set Virtual Address event told the Runtime Driver to convert all its pointers to point to the new virtual address space. The blackout and the fact you need to hide the hardware from the OS make doing things at EFI Runtime Time hard. I agree with you that since our logging is really just a serial stream we don't require Report Status Code. Report Status Code exists so that the old PC POST Cards could still be supported, see: https://en.wikipedia.org/wiki/Power-on_self-test#Error_reporting. There is also a Report Status Code layer that allows redirecting the stream to multiple agents, so for example you could send data to port 80 (POST Card) and write out the same values in the serial stream. I find lookup up Report Status Code codes very tedious and not really helpful for debugging vs. DEBUG prints. Thanks, Andrew Fish > I've always seen status code reporting cumbersome in comparison to plain > DEBUG messages. My understanding is that status code reporting targets > devices that are even dumber than serial ports. But, we do target a > second serial port. So if we can keep the switchover internal to the > SerialPortLib class, at least for runtime DXE drivers, then I think we > should do that. > > Thanks, > Laszlo > >> On 2019/3/1 23:27, Laszlo Ersek wrote: >>> +Peter, for the last few paragraphs >>> >>> On 02/28/19 13:10, Ard Biesheuvel wrote: >>>> On Thu, 28 Feb 2019 at 09:06, Heyi Guo wrote: >>>>> Serial port output is useful when debugging UEFI runtime services in >>>>> OS runtime. >>>>> The patches are trying to provide a handy method to enable runtime >>>>> serial port >>>>> debug for ArmVirtQemu. >>>>> >>>>> Cc: Jian J Wang >>>>> Cc: Hao Wu >>>>> Cc: Laszlo Ersek >>>>> Cc: Ard Biesheuvel >>>>> Cc: Julien Grall >>>>> >>>>> Heyi Guo (3): >>>>> MdeModulePkg/StatusCode: Add PCD to enable runtime serial debug >>>>> ArmVirtPkg: add runtime instance of FdtPL011SerialPortLib >>>>> ArmVirtQemu: enable runtime debug by build flag >>>>> >>>> Hello Heyi, >>>> >>>> We have had this discussion before, and IIRC, the proposed solution >>>> was to use status codes. >>>> >>>> I'm not sure how that is supposed to work though - hopefully Laszlo or >>>> one of the MdeModulePkg maintainers can elaborate. >>> Here's the basic idea. >>> >>> Status Code reporting and routing are defined by the PI spec for OS >>> runtime as well, not just boot time. >>> >>> Recently we added status code *routing* to ArmVirtPkg, in commit >>> 5c574b222ec2, via the generic runtime driver >>> "ReportStatusCodeRouterRuntimeDxe". We also added the library resolution >>> >>> ReportStatusCodeLib --> >>> MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf >>> >>> (for some module types). As a result, when modules of those types report >>> status codes, they now reach the ReportStatusCodeRouterRuntimeDxe >>> driver, which "routes" the status codes. >>> >>> In the same series, we also modified ArmVirtPkg's PlatformBootManagerLib >>> (built into BdsDxe) to register a status code *handler* callback with >>> ReportStatusCodeRouterRuntimeDxe -- but only for boot time (not >>> runtime), and only for a very specific group of status codes. As a >>> result, when a module of suitable type reports a status code, >>> ReportStatusCodeRouterRuntimeDxe "routes it" to BdsDxe, and then BdsDxe >>> "handles it" (in our implementation, we simply format it to the UEFI >>> console), assuming the status code is the kind we are interested in. >>> >>> >>> Now we come to the current series. First, the series adds the following >>> DebugLib class resolution: >>> >>> DebugLib --> >>> MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf >>> >>> >>> This will cause modules to publish their DEBUG messages as status codes >>> via ReportStatusCodeLib, rather than log them via SerialPortLib. (I'm >>> too lazy to check the exact status code classes etc that >>> PeiDxeDebugLibReportStatusCode embeds the DEBUG messages into.) As a >>> result, DEBUG messages will reach ReportStatusCodeRouterRuntimeDxe for >>> "routing". As another result, until we reach a late enough stage in the >>> boot, those messages will not be printed by anything (because there's >>> not going to be any "handling" for them). >>> >>> (The present series also updates the ReportStatusCodeLib resolution so >>> it can be used at runtime too, but that's quite auxiliary to this >>> discussion.) >>> >>> Second, this series includes the generic Status Code *handling* driver >>> (also a runtime driver): "StatusCodeHandlerRuntimeDxe". Independently of >>> the particular handling that we already have in BdsDxe via the earlier >>> series, this generic status handler driver registers a handler callback >>> that simply prints status codes to the serial port (dependent on a PCD >>> setting), via SerialPortLib. >>> >>> With the modification from the first patch, this generic status code >>> *handler* driver is extended to runtime serial port operation. And, the >>> second patch in the series provides a SerialPortLib instance for it that >>> can work at runtime. >>> >>> All in all, when a runtime driver calls DEBUG, this happens: >>> >>> runtime driver calls DEBUG >>> -> PeiDxeDebugLibReportStatusCode >>> -> RuntimeDxeReportStatusCodeLib >>> [status code reporting] >>> >>> => ReportStatusCodeRouterRuntimeDxe >>> [status code routing] >>> >>> => StatusCodeHandlerRuntimeDxe >>> [status code handling, such as SerialPortWrite():] >>> -> FdtPL011SerialPortLibRuntime >>> >>> This is actually a good idea, but it would be nice if: >>> - QEMU had two PL011 ports, >>> - the boot time firmware log, and the OS log, went to one port >>> - the runtme firmware log went to the other port. >>> >>> Given that this series provides the SerialPortLib instance for >>> StatusCodeHandlerRuntimeDxe anyway, we could implement it so that it >>> locate a "special" PL011 in QEMU's DTB -- the port that we'd only use >>> for runtime firmware logging. >>> >>> I don't insist that this series implement all of that, but it should >>> either prevent a conflict on the one PL011 between the firmware and the >>> OS, or else be very explicit about the possible conflict in the commit >>> messages. >>> >>> Thanks >>> Laszlo >>> >>> . >>> >> >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel