[PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[Ard Biesheuvel]

The FVP models expose several emulated serial ports, and always start with
Xterm windows connected to at least two of them. So let's switch to the
second one for DEBUG output, leaving the original one for console output
via SerialDxe.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index 7094e57ee13a..7db1c675c3d9 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -125,7 +125,7 @@
   gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|24000000
 
   ## PL011 - Serial Terminal
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a0000
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
 
@@ -239,7 +239,10 @@
   MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
   MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
   MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
-  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
+    <PcdsFixedAtBuild>
+      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
+  }
 
   MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
 
-- 
2.17.1

Re: [PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[Philippe Mathieu-Daudé]

On 12/17/18 7:53 PM, Ard Biesheuvel wrote:
> The FVP models expose several emulated serial ports, and always start with
> Xterm windows connected to at least two of them. So let's switch to the
> second one for DEBUG output, leaving the original one for console output
> via SerialDxe.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> index 7094e57ee13a..7db1c675c3d9 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> @@ -125,7 +125,7 @@
>    gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|24000000
>  
>    ## PL011 - Serial Terminal
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a0000
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>  
> @@ -239,7 +239,10 @@
>    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
>    MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> +    <PcdsFixedAtBuild>
> +      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> +  }
>  
>    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  
> 

Re: [PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[Leif Lindholm]

On Mon, Dec 17, 2018 at 07:53:53PM +0100, Ard Biesheuvel wrote:
> The FVP models expose several emulated serial ports, and always start with
> Xterm windows connected to at least two of them. So let's switch to the
> second one for DEBUG output, leaving the original one for console output
> via SerialDxe.

Could you clarify whether this means _all_ non-DEBUG output will go to
the first UART, or whether there is a switchover point and some early
non-DEBUG messages will now appear on the second UART?

With that:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> index 7094e57ee13a..7db1c675c3d9 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> @@ -125,7 +125,7 @@

.inf diff format would have been slightly nicer here.

>    gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|24000000
>  
>    ## PL011 - Serial Terminal
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a0000
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>  
> @@ -239,7 +239,10 @@
>    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
>    MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> +    <PcdsFixedAtBuild>
> +      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> +  }
>  
>    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  
> -- 
> 2.17.1
> 

Re: [PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[Ard Biesheuvel]

On Thu, 20 Dec 2018 at 16:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Mon, Dec 17, 2018 at 07:53:53PM +0100, Ard Biesheuvel wrote:
> > The FVP models expose several emulated serial ports, and always start with
> > Xterm windows connected to at least two of them. So let's switch to the
> > second one for DEBUG output, leaving the original one for console output
> > via SerialDxe.
>
> Could you clarify whether this means _all_ non-DEBUG output will go to
> the first UART, or whether there is a switchover point and some early
> non-DEBUG messages will now appear on the second UART?
>

Ah yes, good point.

So any explicit calls to SerialPorWrite () from modules other than
SerialDxe will get directed to the second UART in this case.

There is such a call in PrePi:

CharCount = AsciiSPrint (Buffer,sizeof (Buffer),
    "UEFI firmware (version %s built at %a on %a)\n\r",
    (CHAR16*)PcdGetPtr(PcdFirmwareVersionString), __TIME__, __DATE__);
SerialPortWrite ((UINT8 *) Buffer, CharCount)

and in one or two other places. Also note that DEBUG() directives in
SerialDxe itself will be sent to the non-DEBUG uart.

I think it makes sense for the DefaultExceptionHandlerLib to write to
the console instead of doing a SerialPortWrite () on RELEASE builds,
so I'll look into that next.


> With that:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > index 7094e57ee13a..7db1c675c3d9 100644
> > --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > @@ -125,7 +125,7 @@
>
> .inf diff format would have been slightly nicer here.
>
> >    gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|24000000
> >
> >    ## PL011 - Serial Terminal
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a0000
> >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> >
> > @@ -239,7 +239,10 @@
> >    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> >    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> >    MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> > -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> > +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> > +    <PcdsFixedAtBuild>
> > +      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> > +  }
> >
> >    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> >
> > --
> > 2.17.1
> >

Re: [PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

[Ard Biesheuvel]

On Thu, 20 Dec 2018 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 20 Dec 2018 at 16:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Mon, Dec 17, 2018 at 07:53:53PM +0100, Ard Biesheuvel wrote:
> > > The FVP models expose several emulated serial ports, and always start with
> > > Xterm windows connected to at least two of them. So let's switch to the
> > > second one for DEBUG output, leaving the original one for console output
> > > via SerialDxe.
> >
> > Could you clarify whether this means _all_ non-DEBUG output will go to
> > the first UART, or whether there is a switchover point and some early
> > non-DEBUG messages will now appear on the second UART?
> >
>
> Ah yes, good point.
>
> So any explicit calls to SerialPorWrite () from modules other than
> SerialDxe will get directed to the second UART in this case.
>
> There is such a call in PrePi:
>
> CharCount = AsciiSPrint (Buffer,sizeof (Buffer),
>     "UEFI firmware (version %s built at %a on %a)\n\r",
>     (CHAR16*)PcdGetPtr(PcdFirmwareVersionString), __TIME__, __DATE__);
> SerialPortWrite ((UINT8 *) Buffer, CharCount)
>
> and in one or two other places. Also note that DEBUG() directives in
> SerialDxe itself will be sent to the non-DEBUG uart.
>
> I think it makes sense for the DefaultExceptionHandlerLib to write to
> the console instead of doing a SerialPortWrite () on RELEASE builds,
> so I'll look into that next.
>
>
> > With that:
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >

Pushed as 88e5de1a2346..861c200cda14, with the following paragraph
added to the commit log:

    Note that explicit SerialPortWrite() calls made from other modules will
    now also end up on the DEBUG console. Conversely, DEBUG output generated
    in the context of SerialDxe will end up on the primary serial console.
    This is a fundamental consequence of the way that DEBUG() is built on top
    of SerialPortWrite(), which can only be implemented in one way in each
    module (but can differ between modules, hence this patch)


> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > > index 7094e57ee13a..7db1c675c3d9 100644
> > > --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > > +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > > @@ -125,7 +125,7 @@
> >
> > .inf diff format would have been slightly nicer here.
> >
> > >    gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|24000000
> > >
> > >    ## PL011 - Serial Terminal
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a0000
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> > >    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> > >
> > > @@ -239,7 +239,10 @@
> > >    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> > >    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
> > >    MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> > > -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> > > +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> > > +    <PcdsFixedAtBuild>
> > > +      gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c090000
> > > +  }
> > >
> > >    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> > >
> > > --
> > > 2.17.1
> > >