Re: [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers

["Kinney, Michael D" <michael.d.kinney@intel.com>]

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.

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, February 20, 2018 9:00 AM
> To: Leif Lindholm <leif.lindholm@linaro.org>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: edk2-devel@lists.01.org; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; 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
> <ard.biesheuvel@linaro.org>
> >> ---
> >>
> 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 <lersek@redhat.com>
> 
> (
> 
> 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
> >>