From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org 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 31A1021962301 for ; Wed, 20 Mar 2019 02:55:39 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C643C20261; Wed, 20 Mar 2019 09:55:38 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-91.rdu2.redhat.com [10.10.120.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id EC8762E030; Wed, 20 Mar 2019 09:55:36 +0000 (UTC) To: Heyi Guo , Ard Biesheuvel Cc: Peter Maydell , Hao Wu , "edk2-devel@lists.01.org" , Julien Grall , wanghaibin 00208455 References: <1551341112-21645-1-git-send-email-guoheyi@huawei.com> <27394137-8c32-4f8a-c029-5fc883a2cce0@huawei.com> <69336094-c1c0-b978-7fe7-6a585faedda6@huawei.com> From: Laszlo Ersek Message-ID: <8b2a9fcd-fb0d-6432-4481-e05d08816332@redhat.com> Date: Wed, 20 Mar 2019 10:55:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <69336094-c1c0-b978-7fe7-6a585faedda6@huawei.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 20 Mar 2019 09:55:38 +0000 (UTC) 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: Wed, 20 Mar 2019 09:55:40 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 03/16/19 10:41, Heyi Guo wrote: > On 2019/3/13 1:05, Laszlo Ersek wrote: >> 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.) > My first idea was also simply implement a runtime serial port instance, > but the problem is for the initialization. > > Since serial port lib is only a library, it seems we can only initialize > everything in SerialPortInitialize() or library constructor, but either > of which could no work in current EDK2 code framework. Please see my > explanation in earlier mail as below: > > "The long story is: at first I proposed to still use > BaseSerialPortDebugLib, so I added RuntimePrepare() function into the > constructor directly, but the builder complained about looped > constructors, for RuntimePrepare() invokes gBS and some RunTime > libraries. Then I tried disabling the constructor and moved > RuntimePrepare() into SerialPortInitialize() which could complete the > build, but still couldn't guarantee the calling order of these > constructors, for BaseSerialPortDebugLib has a constructor to call > SerialPortInitialize()." (Sorry about the late response (I've been away for a few days), and also for missing that construction loop that you apparently mentioned earlier.) You mention that "RuntimePrepare() invokes gBS and some RunTime libraries". How important is it to rely on those libraries? For example, "gBS" is simply a convenience; you can get at the same thing from the parameter list of the constructor function too (assuming you use a MODULE_TYPE of (minimally) DXE_DRIVER, in the lib instance's INF file). If we don't have to flatten a ridiculous amount of other library code into the RuntimePrepare() function, I think such flattening would be a viable approach. We've run into this constructor loop several times before, and -- if I remember correctly anyway! -- one approach has been to declare SerialPortLib the "lowest level abstraction", and make the affected lib instance self-contained. > Can we use event notify or protocol notify to do the runtime > intialization? Like EndOfDxe event group. Any other advice? I guess this could work too. I'm not really a fan of callbacks as long as we can manage otherwise (for example, we can't propagate errors out of callbacks), but if the lib flattening thing gets too complex, we could do this as well. Regarding EndOfDxe -- I think EndOfDxe is considered a trust barrier (namely between platform fw modules and 3rd party modules), so I wouldn't tie our initialization to that, for clarity's sake. ReadyToBoot seems a tiny bit less "forced" -- it is sufficiently late, you can still allocate resources, and we can claim it is "on the path towards OS runtime". Just be aware that ReadyToBoot can be signaled multiple times, e.g. if some boot options fail (so uninstall the handler / close the event when serving the first callback I guess). Thanks Laszlo