From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 C463A223C1762 for ; Wed, 21 Feb 2018 02:21:12 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A5D0840201A0; Wed, 21 Feb 2018 10:27:10 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-45.rdu2.redhat.com [10.10.120.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0F663213AEE2; Wed, 21 Feb 2018 10:27:08 +0000 (UTC) To: "Kinney, Michael D" , Leif Lindholm , Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , "Ni, Ruiyu" , "Gao, Liming" , "Zeng, Star" , "afish@apple.com" References: <20180220110524.9050-1-ard.biesheuvel@linaro.org> <20180220110524.9050-4-ard.biesheuvel@linaro.org> <20180220142244.i6sbbeph2mbycayz@bivouac.eciton.net> <4f855121-1417-26fa-c3a4-05a3e0a3de1d@redhat.com> From: Laszlo Ersek Message-ID: Date: Wed, 21 Feb 2018 11:27:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 21 Feb 2018 10:27:10 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 21 Feb 2018 10:27:10 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Feb 2018 10:21:13 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 02/20/18 19:02, Kinney, Michael D wrote: > I do not agree with this change. > > If the PCDs that describe the UART are for a UART > that is owned by the FW and hidden from the OS, then > this lib can work well at runtime. Does that imply that we should do the runtime disablement in the SerialPortLib instance that underlies BaseDebugLibSerialPort? I think it is an integral part of the feature (or, well, fix) that the runtime incompatibility be caught at edk2 platform build time. In other words, *some* library instance in edk2 should blacklist DXE_RUNTIME_DRIVER modules (and the counterpart library instance should be appropriate for DXE_RUNTIME_DRIVER modules only, and handle EBS, to dynamically disable use of the serial port). Are you suggesting that we implement this at the PL011 SerialPortLib instance level? Thanks! Laszlo >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Tuesday, February 20, 2018 9:00 AM >> To: Leif Lindholm ; Ard >> Biesheuvel >> Cc: edk2-devel@lists.01.org; Ni, Ruiyu >> ; Kinney, Michael D >> ; Gao, Liming >> ; Zeng, Star >> ; afish@apple.com >> Subject: Re: [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: >> blacklist for use by DXE runtime drivers >> >> On 02/20/18 15:22, Leif Lindholm wrote: >>> On Tue, Feb 20, 2018 at 11:05:24AM +0000, Ard >> Biesheuvel wrote: >>>> BaseDebugLibSerialPort is not suitable for use by >> DXE_RUNTIME_DRIVER >>>> modules, so blacklist it for use by such modules. >>>> >>>> Contributed-under: TianoCore Contribution Agreement >> 1.1 >>>> Signed-off-by: Ard Biesheuvel >> >>>> --- >>>> >> MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerial >> Port.inf | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git >> a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri >> alPort.inf >> b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri >> alPort.inf >>>> index 823511b22f6b..25da1fb9363a 100644 >>>> --- >> a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri >> alPort.inf >>>> +++ >> b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri >> alPort.inf >>>> @@ -21,7 +21,7 @@ [Defines] >>>> FILE_GUID = BB83F95F-EDBC- >> 4884-A520-CD42AF388FAE >>>> MODULE_TYPE = BASE >>>> VERSION_STRING = 1.0 >>>> - LIBRARY_CLASS = DebugLib >>>> + LIBRARY_CLASS = DebugLib|SEC >> PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_SMM_DRIVER >> DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION SMM_CORE >> MM_STANDALONE MM_CORE_STANDALONE >>> >>> Not a comment on this patch as such (which looks >> sensible to me), but >>> what you're doing here isn't blacklisting >> DXE_RUNTIME_DRIVER, but >>> rather whitelisting every other module type. >>> >>> Could we use a ! operator added to the syntax? >> >> No, I don't think so, based on the INF file spec. >> >> Reviewed-by: Laszlo Ersek >> >> ( >> >> A future customization would be to give a similar >> treatment to SMM (or >> "MM") drivers. While MM has its own set of (identity >> mapped) page >> tables, and therefore I'd expect an MMIO serial port to >> "just work", we >> still shouldn't mess with the serial port once the OS >> owns it, >> regardless of the fact that we're in MM. So, for that, >> the lib instance >> meant for MM drivers would have to catch EBS too. >> >> Of course, that doesn't work the same way -- the >> SMM_CORE catches the >> normal EBS notification, and "forwards" it into the MM >> protocol database >> (see SmmExitBootServicesHandler() in >> "MdeModulePkg/Core/PiSmmCore/PiSmmCore.c"). So the MM >> lib instance would >> have to register an MM protocol notify for >> "gEdkiiSmmExitBootServicesProtocolGuid". >> >> But, that's for the future at best. >> >> *This* lib instance should remain correct for the >> SMM_CORE itself, however. >> >> ) >> >> Thanks, >> Laszlo >> >> >> >>> / >>> Leif >>> >>>> CONSTRUCTOR = >> BaseDebugLibSerialPortConstructor >>>> >>>> # >>>> -- >>>> 2.11.0 >>>> >