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:[email protected]] >> Sent: Tuesday, February 20, 2018 9:00 AM >> To: Leif Lindholm <[email protected]>; Ard >> Biesheuvel <[email protected]> >> Cc: [email protected]; Ni, Ruiyu >> <[email protected]>; Kinney, Michael D >> <[email protected]>; Gao, Liming >> <[email protected]>; Zeng, Star >> <[email protected]>; [email protected] >> 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 >> <[email protected]> >>>> --- >>>> >> 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 <[email protected]> >> >> ( >> >> 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 >>>> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
