public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "gaoliming" <gaoliming@byosoft.com.cn>
To: <devel@edk2.groups.io>, <samer.el-haj-mahmoud@arm.com>,
	<zhichao.gao@intel.com>, <lersek@redhat.com>
Cc: "'Wang, Jian J'" <jian.j.wang@intel.com>,
	"'Wu, Hao A'" <hao.a.wu@intel.com>,
	"'Ni, Ray'" <ray.ni@intel.com>,
	"'Ard Biesheuvel'" <Ard.Biesheuvel@arm.com>,
	"'Andy Lutomirski'" <luto@kernel.org>
Subject: 回复: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY
Date: Fri, 4 Dec 2020 09:11:02 +0800	[thread overview]
Message-ID: <009001d6c9da$55c9a460$015ced20$@byosoft.com.cn> (raw)
In-Reply-To: <DB7PR08MB326003C730DCE0DE55969B7790F10@DB7PR08MB3260.eurprd08.prod.outlook.com>

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@groups.io
> <bounce+27952+68301+4905953+8761045@groups.io> 代表 Samer
> El-Haj-Mahmoud
> 发送时间: 2020年12月4日 8:05
> 收件人: devel@edk2.groups.io; zhichao.gao@intel.com; lersek@redhat.com
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> <Ard.Biesheuvel@arm.com>; Andy Lutomirski <luto@kernel.org>; Samer
> El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@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@edk2.groups.io <devel@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@arm.com>;
> > devel@edk2.groups.io; lersek@redhat.com
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> > <Ard.Biesheuvel@arm.com>; Andy Lutomirski <luto@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@arm.com>
> > > Sent: Tuesday, December 1, 2020 11:17 PM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io;
> > > lersek@redhat.com
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> > > <Ard.Biesheuvel@arm.com>; Andy Lutomirski <luto@kernel.org>; Samer
> > > El-Haj-Mahmoud <Samer.El-Haj- Mahmoud@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@intel.com>
> > > > Sent: Monday, November 30, 2020 8:00 PM
> > > > To: devel@edk2.groups.io; lersek@redhat.com; Samer El-Haj-Mahmoud
> > > > <Samer.El-Haj-Mahmoud@arm.com>
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> > > > <Ard.Biesheuvel@arm.com>; Andy Lutomirski <luto@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Laszlo Ersek
> > > > > Sent: Wednesday, November 25, 2020 7:30 AM
> > > > > To: devel@edk2.groups.io; samer.el-haj-mahmoud@arm.com
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > > <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ni,
> > > > > Ray <ray.ni@intel.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>;
> > > > > Andy Lutomirski <luto@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@intel.com>
> > > > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > > Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>
> > > > > > Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-
> > > > Mahmoud@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.
> 
> 
> 
> 




  reply	other threads:[~2020-12-04  1:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-24 19:15 [PATCH v1 0/3] MdeModulePkg/GraphicsConsole : Console cleanup Samer El-Haj-Mahmoud
2020-11-24 19:15 ` [PATCH v1 1/3] MdeModulePkg/GraphicsConsoleDxe: Change default CursorVisible to FALSE Samer El-Haj-Mahmoud
2020-12-01  0:45   ` Gao, Zhichao
2020-11-24 19:15 ` [PATCH v1 2/3] MdeModulePkg/Graphics: Fix spelling mistakes Samer El-Haj-Mahmoud
2020-12-01  0:45   ` Gao, Zhichao
2020-11-24 19:15 ` [PATCH v1 3/3] MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY Samer El-Haj-Mahmoud
2020-11-24 23:30   ` [edk2-devel] " Laszlo Ersek
2020-12-01  0:59     ` Gao, Zhichao
2020-12-01 15:17       ` Samer El-Haj-Mahmoud
2020-12-02 11:04         ` Gao, Zhichao
2020-12-04  0:04           ` Samer El-Haj-Mahmoud
2020-12-04  1:11             ` gaoliming [this message]
2020-12-04  1:20               ` Samer El-Haj-Mahmoud
2020-12-04  5:23                 ` Gao, Zhichao
2020-12-04 12:42                   ` Samer El-Haj-Mahmoud
2020-12-04 14:13                     ` Ard Biesheuvel
2020-12-07 18:36                       ` Samer El-Haj-Mahmoud
2020-12-08  0:59                         ` Gao, Zhichao
2020-12-08 17:07                           ` Samer El-Haj-Mahmoud
2020-12-08  1:28                     ` Ni, Ray
2020-12-08 17:13                       ` Samer El-Haj-Mahmoud

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='009001d6c9da$55c9a460$015ced20$@byosoft.com.cn' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox