回复: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY

Fri Dec 4 01:11:02 UTC 2020

Samer:
 Does all debug message output by PeiDxeDebugLibReportStatusCode? There is not debug message to print as UefiDebugLibStdErr or UefiDebugLibConOut. Right?

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+68301+4905953+8761045 at groups.io
> <bounce+27952+68301+4905953+8761045 at groups.io> 代表 Samer
> El-Haj-Mahmoud
> 发送时间: 2020年12月4日 8:05
> 收件人: devel at edk2.groups.io; zhichao.gao at intel.com; lersek at redhat.com
> 抄送: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Ard Biesheuvel
> <Ard.Biesheuvel at arm.com>; Andy Lutomirski <luto at kernel.org>; Samer
> El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>
> 主题: Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter: Change
> StdErr color to EFI_LIGHTGRAY
> 
> Zhichao,
> 
> I can understand the rationale if this is truly only for StdErr (although it would
> have been nice to allow platforms to customize the color with a PCD).
> However, I see the inconsistency in debug output with platforms I tested with.
> For example, on the RPi, with DEBUG build, and all ConOut/StdErr and
> DebugLiub using the same serial console. The serial debug starts with
> LIGHTGRAY (attached screenshot 1), until gEfiStandardErrorDeviceGuid is
> installed. At that point, the debug output switches to MAGENTA, and
> continues to do so until entering the UI or booting to UEFI Shell, where the
> color switches back to LIGHTGRAY (attached screenshot2). After that, all
> ConOut and Debug output is LIGHTGRAY . I do not really know of any actual
> StdErr output from the Shell.
> 
> So, there might be a bug somewhere that causes DEBUG output to switch to
> MAGENTA and back. I am not really sure. But this inconsistency is annoying.
> Can we simply avoid this by using a consistent color for all console output? Or
> at least allow platforms to decide?
> 
> 
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Gao,
> > Zhichao via groups.io
> > Sent: Wednesday, December 2, 2020 6:05 AM
> > To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>;
> > devel at edk2.groups.io; lersek at redhat.com
> > Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> > <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Ard Biesheuvel
> > <Ard.Biesheuvel at arm.com>; Andy Lutomirski <luto at kernel.org>
> > Subject: Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter:
> > Change StdErr color to EFI_LIGHTGRAY
> >
> >
> >
> > > -----Original Message-----
> > > From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>
> > > Sent: Tuesday, December 1, 2020 11:17 PM
> > > To: Gao, Zhichao <zhichao.gao at intel.com>; devel at edk2.groups.io;
> > > lersek at redhat.com
> > > Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> > > <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Ard Biesheuvel
> > > <Ard.Biesheuvel at arm.com>; Andy Lutomirski <luto at kernel.org>; Samer
> > > El-Haj-Mahmoud <Samer.El-Haj- Mahmoud at arm.com>
> > > Subject: RE: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter:
> > > Change StdErr color to EFI_LIGHTGRAY
> > >
> > > Why does StdErr have to be a different color from ConOut? If the
> > > system redirected both streams to the same console output then that is
> > their choice.
> > > Serial DEBUG output is not a different color even if the DEBUG is
> > > redirected to the same console as ConOut and StdErr. Also, from what I
> > > have seen, StdErr does not seem to always retain this MAGENTA color
> > > later (for example, after booting a UEFI Shell?).
> >
> > Can you share the use case of StdErr? Seems when using StdErr-
> > >OutputString, the output is not always MAGENTA color. If so, it is a bug of
> > console driver.
> >
> > I am thinking of one case. The platform only have the serial port without any
> > other display device. System boots to uefi shell and run a debug build
> > application. And the app would have both print output and debug print. If
> > the color are same, the info of normal print and debug print would be mixed
> > up. I am saying StdErr output not normal DebugLib.
> >
> > Thanks,
> > Zhichao
> >
> > >
> > > Do users really care (other than being annoyed by the inconsistency of
> > "some"
> > > text showing up in purple?). Using the same color for consoles/DEBUG
> > > output by default is consistent and clean. Applications/users can
> > > always change the colors later to whatever is the preference for that
> > particular UI/CLI.
> > >
> > > Thanks,
> > > --Samer
> > >
> > >
> > > > -----Original Message-----
> > > > From: Gao, Zhichao <zhichao.gao at intel.com>
> > > > Sent: Monday, November 30, 2020 8:00 PM
> > > > To: devel at edk2.groups.io; lersek at redhat.com; Samer El-Haj-Mahmoud
> > > > <Samer.El-Haj-Mahmoud at arm.com>
> > > > Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> > > > <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Ard Biesheuvel
> > > > <Ard.Biesheuvel at arm.com>; Andy Lutomirski <luto at kernel.org>
> > > > Subject: RE: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter:
> > > > Change StdErr color to EFI_LIGHTGRAY
> > > >
> > > > I agree the EFI_MAGENTA is not a good choose. But this may be a
> > > > different issue. Many platforms would set serial port as ConOut and
> > > > ErrOut. The different colors for them can differ the origin. I don't
> > > > think change them to the same color is a good idea.
> > > >
> > > > Thanks,
> > > > Zhichao
> > > >
> > > > > -----Original Message-----
> > > > > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > > > > Laszlo Ersek
> > > > > Sent: Wednesday, November 25, 2020 7:30 AM
> > > > > To: devel at edk2.groups.io; samer.el-haj-mahmoud at arm.com
> > > > > Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> > > > > <hao.a.wu at intel.com>; Gao, Zhichao <zhichao.gao at intel.com>; Ni,
> > > > > Ray <ray.ni at intel.com>; Ard Biesheuvel <Ard.Biesheuvel at arm.com>;
> > > > > Andy Lutomirski <luto at kernel.org>
> > > > > Subject: Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter:
> > > > > Change StdErr color to EFI_LIGHTGRAY
> > > > >
> > > > > On 11/24/20 20:15, Samer El-Haj-Mahmoud wrote:
> > > > > > ConSplitter was using EFI_LIGHTGRAY foreground color for ConOut
> > > > > > and EFI_MAGENTA for StdErr.
> > > > > >
> > > > > > This does not work all the time, and StdErr ends up showing
> > > > > > parts in MAGENTA and other parts in LIGHTGRAY. Changing StdErr
> > > > > > to LIGHTGRAY looks better and is more consistent.
> > > > > >
> > > > > > Cc: Jian J Wang <jian.j.wang at intel.com>
> > > > > > Cc: Hao A Wu <hao.a.wu at intel.com>
> > > > > > Cc: Zhichao Gao <zhichao.gao at intel.com>
> > > > > > Cc: Ray Ni <ray.ni at intel.com>
> > > > > > Cc: Ard Biesheuvel <Ard.Biesheuvel at arm.com>
> > > > > > Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-
> > > > Mahmoud at arm.com>
> > > > > > ---
> > > > > >  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c |
> 2
> > > > > > +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > > > > > b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > > > > > index b090de288517..e8cd4ce120a0 100644
> > > > > > ---
> > > > > > a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> > > > > > +++
> > b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.
> > > > > > +++ c
> > > > > > @@ -1476,7 +1476,7 @@ ConSplitterStdErrDriverBindingStart (
> > > > > >    // their MaxMode and QueryData should be the intersection of
> > both.
> > > > > >    //
> > > > > >    Status = ConSplitterTextOutAddDevice (&mStdErr, TextOut,
> > > > > > NULL, NULL);
> > > > > > -  ConSplitterTextOutSetAttribute (&mStdErr.TextOut,
> > > > > > EFI_TEXT_ATTR (EFI_MAGENTA, EFI_BLACK));
> > > > > > +  ConSplitterTextOutSetAttribute (&mStdErr.TextOut,
> > > > > > + EFI_TEXT_ATTR (EFI_LIGHTGRAY, EFI_BLACK));
> > > > > >
> > > > > >    return Status;
> > > > > >  }
> > > > > >
> > > > >
> > > > > I am very curious as to how this patch is going to fare, as Andy
> > > > > Lutomirski (CC'd) reported the same symptom in a Fedora bugzilla
> > > > > ticket
> > > > > 4+ years ago:
> > > > >
> > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1355913
> > > > >
> > > > > As you can see in that BZ, I found the same code location, I just
> > > > > didn't feel up to starting another crusade on edk2-devel -- about
> > > > > colors even!... So I'll be watching this one now. :)
> > > > >
> > > > > Thanks
> > > > > Laszlo
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > > IMPORTANT NOTICE: The contents of this email and any attachments are
> > > confidential and may also be privileged. If you are not the intended
> > > recipient, please notify the sender immediately and do not disclose
> > > the contents to any other person, use it for any purpose, or store or
> > > copy the information in any medium. Thank you.
> >
> >
> >
> >
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68304): https://edk2.groups.io/g/devel/message/68304
Mute This Topic: https://groups.io/mt/78700014/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-