From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=45.249.212.190; helo=huawei.com; envelope-from=guoheyi@huawei.com; receiver=edk2-devel@lists.01.org Received: from huawei.com (szxga04-in.huawei.com [45.249.212.190]) (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 60832211E011F for ; Wed, 20 Mar 2019 05:16:22 -0700 (PDT) Received: from DGGEMS407-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 864B95456592A4ED20C6; Wed, 20 Mar 2019 20:16:19 +0800 (CST) Received: from [127.0.0.1] (10.177.31.55) by DGGEMS407-HUB.china.huawei.com (10.3.19.207) with Microsoft SMTP Server id 14.3.408.0; Wed, 20 Mar 2019 20:16:08 +0800 To: Laszlo Ersek , Ard Biesheuvel References: <1551341112-21645-1-git-send-email-guoheyi@huawei.com> <27394137-8c32-4f8a-c029-5fc883a2cce0@huawei.com> <69336094-c1c0-b978-7fe7-6a585faedda6@huawei.com> <8b2a9fcd-fb0d-6432-4481-e05d08816332@redhat.com> CC: Peter Maydell , Hao Wu , "edk2-devel@lists.01.org" , Julien Grall , wanghaibin 00208455 From: Heyi Guo Message-ID: <1e39908c-3f26-7b87-8bd3-8ae04221cf58@huawei.com> Date: Wed, 20 Mar 2019 20:16:07 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <8b2a9fcd-fb0d-6432-4481-e05d08816332@redhat.com> X-Originating-IP: [10.177.31.55] X-CFilter-Loop: Reflected 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 12:16:22 -0000 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit On 2019/3/20 17:55, Laszlo Ersek wrote: > 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.) Nothing at all, and thanks for your time in reading and replying the mails even when out of office :) > > 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. In my RFC, we need to use gDS which is from DxeServicesTableLib->EfiGetSystemConfigurationTable()->UefiLib, so that we need to flatten EfiGetSystemConfigurationTable(). And gDS->SetMemorySpaceAttributes() actually relies on gEfiCpuArchProtocolGuid or it will return EFI_NOT_AVAILABLE_YET. Thanks, Heyi > >> 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 > > . >