diff mbox

[edk2,V3,4/5] Add PCD for selecting terminal type at build time

Message ID 1436316258-19655-5-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz July 8, 2015, 12:44 a.m. UTC
From: Laszlo Ersek <lersek@redhat.com>

Add a fixed pointer PCD to allow build-time selection of VT100 or TTY terminal
type.  The default remains VT100 emulation.
Add support for building the ARM QEMU platforms with the TTY terminal
with the "-D TTY_TERMINAL" build option.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
[Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL to TTY_TERMINAL]
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Roy Franz <roy.franz@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc                                     |  6 ++++++
 ArmVirtPkg/ArmVirtPkg.dec                                      |  7 +++++++
 ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c      | 12 ++++++++----
 ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf |  4 ++++
 4 files changed, 25 insertions(+), 4 deletions(-)

Comments

gary guo July 8, 2015, 2:46 a.m. UTC | #1
Hi Roy,

I have one question; please see my comments below.

On 07/08/2015 08:44 AM, Roy Franz wrote:
> From: Laszlo Ersek <lersek@redhat.com>
>
> Add a fixed pointer PCD to allow build-time selection of VT100 or TTY terminal
> type.  The default remains VT100 emulation.
> Add support for building the ARM QEMU platforms with the TTY terminal
> with the "-D TTY_TERMINAL" build option.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> [Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL to TTY_TERMINAL]
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>   ArmVirtPkg/ArmVirt.dsc.inc                                     |  6 ++++++
>   ArmVirtPkg/ArmVirtPkg.dec                                      |  7 +++++++
>   ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c      | 12 ++++++++----
>   ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf |  4 ++++
>   4 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 7ec0de4..2feebd3 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -15,6 +15,7 @@
>   
>   [Defines]
>     DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
> +  DEFINE TTY_TERMINAL            = FALSE
I see in ArmVirtQemu.dsc file (patch 5#), we are using #ifdef 
TTY_TERMINAL. In my opinion the above statement will cause #ifdef 
TTY_TERMINAL to be always true.
>   
>   [LibraryClasses.common]
>   !if $(TARGET) == RELEASE
> @@ -359,6 +360,11 @@
>     gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
>   !endif
>   
> +!if $(TTY_TERMINAL) == TRUE
And I don't understand why we use "!if" here and "!ifdef" in dsc file.

I should have tested the patch directly, but I couldn't apply the mail 
patches with "git am" right now (git 2.1.4 on Debian; really appreciate 
if someone could tell me the reason :-) ).

Heyi Guo

> +  # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
> +!endif
> +
>   [Components.common]
>     #
>     # Networking stack
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index 7bbd9ff..9833c5a 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -49,6 +49,13 @@
>     #
>     gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
>   
> +  #
> +  # Binary representation of the GUID that determines the terminal type. The
> +  # size must be exactly 16 bytes. The default value corresponds to
> +  # EFI_VT_100_GUID.
> +  #
> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
> +
>   [PcdsDynamic, PcdsFixedAtBuild]
>     #
>     # ARM PSCI function invocations can be done either through hypervisor
> diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> index 13830cb..b242a29 100644
> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> @@ -35,7 +35,7 @@
>   typedef struct {
>     VENDOR_DEVICE_PATH         SerialDxe;
>     UART_DEVICE_PATH           Uart;
> -  VENDOR_DEFINED_DEVICE_PATH Vt100;
> +  VENDOR_DEFINED_DEVICE_PATH TermType;
>     EFI_DEVICE_PATH_PROTOCOL   End;
>   } PLATFORM_SERIAL_CONSOLE;
>   #pragma pack ()
> @@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
>     },
>   
>     //
> -  // VENDOR_DEFINED_DEVICE_PATH Vt100
> +  // VENDOR_DEFINED_DEVICE_PATH TermType
>     //
>     {
>       {
>         MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
>         DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
> -    },
> -    EFI_VT_100_GUID
> +    }
> +    //
> +    // Guid to be filled in dynamically
> +    //
>     },
>   
>     //
> @@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
>     //
>     // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
>     //
> +  CopyGuid (&mSerialConsole.TermType.Guid,
> +    PcdGetPtr (PcdTerminalTypeGuidBuffer));
>     BdsLibUpdateConsoleVariable (L"ConIn",
>       (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
>     BdsLibUpdateConsoleVariable (L"ConOut",
> diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> index d998216..9a3cfcd 100644
> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> @@ -39,6 +39,7 @@
>     MdeModulePkg/MdeModulePkg.dec
>     MdePkg/MdePkg.dec
>     OvmfPkg/OvmfPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
>   
>   [LibraryClasses]
>     BaseLib
> @@ -61,6 +62,9 @@
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
>   
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
> +
>   [Guids]
>     gEfiFileInfoGuid
>     gEfiFileSystemInfoGuid


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
Roy Franz July 8, 2015, 6:49 p.m. UTC | #2
On Wed, Jul 8, 2015 at 2:46 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/08/15 04:46, Heyi Guo wrote:
>> Hi Roy,
>>
>> I have one question; please see my comments below.
>
> Indeed:
>
>>
>> On 07/08/2015 08:44 AM, Roy Franz wrote:
>>> From: Laszlo Ersek <lersek@redhat.com>
>>>
>>> Add a fixed pointer PCD to allow build-time selection of VT100 or TTY
>>> terminal
>>> type.  The default remains VT100 emulation.
>>> Add support for building the ARM QEMU platforms with the TTY terminal
>>> with the "-D TTY_TERMINAL" build option.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> [Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL
>>> to TTY_TERMINAL]
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>   ArmVirtPkg/ArmVirt.dsc.inc                                     |  6
>>> ++++++
>>>   ArmVirtPkg/ArmVirtPkg.dec                                      |  7
>>> +++++++
>>>   ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c      | 12
>>> ++++++++----
>>>   ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf |  4
>>> ++++
>>>   4 files changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>>> index 7ec0de4..2feebd3 100644
>>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>>> @@ -15,6 +15,7 @@
>>>     [Defines]
>>>     DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
>>> +  DEFINE TTY_TERMINAL            = FALSE
>
>> I see in ArmVirtQemu.dsc file (patch 5#), we are using #ifdef
>> TTY_TERMINAL. In my opinion the above statement will cause #ifdef
>> TTY_TERMINAL to be always true.
>
> I agree. That's my only comment for patch #5 too. Patch #5 should be
> consistent with this one here, and use
>
> !if $(TTY_TERMINAL) == TRUE
>
>>>     [LibraryClasses.common]
>>>   !if $(TARGET) == RELEASE
>>> @@ -359,6 +360,11 @@
>>>
>>> gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
>>>
>>>   !endif
>>>   +!if $(TTY_TERMINAL) == TRUE
>> And I don't understand why we use "!if" here and "!ifdef" in dsc file.
>
> BTW, we have a good reason for this kind of conditional. We use
>
>   $(WHATEVER_ENABLE) == TRUE
>
> because that allows
>
>   -D WHATEVER_ENABLE=FALSE
>
> on a command line to *override* an earlier
>
>   -D WHATEVER_ENABLE[=TRUE]
>
> on the same command line. The "build" utility has no -U option (for
> undefining a macro).
>
> Assume you have a build script that hardcodes
>
>   -D WHATEVER_ENABLE
>
> because that's how you build the tree most of the time. However,
> occasionally you'd like to build without that feature, and you'd like to
> override the macro. If the script just appends "$@" to the build command
> line, *and* the DSC files use the above kind of conditional, then you
> can simply pass -D WHATEVER_ENABLE=FALSE to override the macro (but
> still use the rest of your build script).
>
> ... Yes, we should have done the same with INTEL_BDS too, and we didn't.
>
> I guess I didn't catch that because I *never* build without INTEL_BDS.
And I followed the INTEL_BDS pattern in that file :)

I have fixed this (and the INTEL_BDS define as well), and I'll send
this off shortly.

I think I have addressed all comments on the other patches.

Thanks,
Roy

>
> ... Well, that, and because the patch that added INTEL_BDS (SVN r16208)
> was never reviewed on the list.
>
> Anyway, I digress.
>
>> I should have tested the patch directly, but I couldn't apply the mail
>> patches with "git am" right now (git 2.1.4 on Debian; really appreciate
>> if someone could tell me the reason :-) ).
>
> Sure. The edk2 project uses CRLF line endings. For that to work in the
> first place (without git yelling at you all the time), you need to add
> the following git config settings:
>
> [core]
>         whitespace = cr-at-eol
>
> Then, to make git-am "compatible", you further need:
>
> [am]
>         keepcr = true
>
> This will *almost* make things work, but it will still break down when
> you have a patch that adds or removes files *and* has crossed MTA
> boundaries. In this case, you'll have
>
> +++ /dev/null\r
>
> or
>
> --- /dev/null\r
>
> hunk headers in the patch. The am.keepcr setting (which is otherwise
> necessary for the *source code* CRLFs) will prevent git-am from
> stripping the \r even from the /dev/null hunk headers, and that will
> trip up git-am. So you need to pre-process the /dev/null hunk headers
> manually, stripping *only those* \r characters, *if* you have a patch
> that adds or removes files *and* has crossed MTA boundaries. (If the
> patch doesn't cross an MTA, ie. it is straight out of git format-patch,
> then /dev/null will have no \r.)
>
> Summary (I'm sure you'll appreciate a summary): if you'd like to apply
> edk2 patches from the mailing list, with git-am, do the following:
> (1) set "core.whitespace" to "cr-at-eol",
> (2) set "am.keepcr" to "true",
> (3) save the patch emails *intact* into local files (eg. *.eml)
> (4) preprocess those local files with:
>
>     sed -r -i 's,^((---|\+\+\+) /dev/null)\r$,\1,'   *.eml
>
> (5) apply them with git-am.
>
> HTH
> Laszlo
>
>>
>> Heyi Guo
>>
>>> +  # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
>>> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91,
>>> 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51,
>>> 0xef, 0x94}
>>> +!endif
>>> +
>>>   [Components.common]
>>>     #
>>>     # Networking stack
>>> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
>>> index 7bbd9ff..9833c5a 100644
>>> --- a/ArmVirtPkg/ArmVirtPkg.dec
>>> +++ b/ArmVirtPkg/ArmVirtPkg.dec
>>> @@ -49,6 +49,13 @@
>>>     #
>>>
>>> gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
>>>
>>>   +  #
>>> +  # Binary representation of the GUID that determines the terminal
>>> type. The
>>> +  # size must be exactly 16 bytes. The default value corresponds to
>>> +  # EFI_VT_100_GUID.
>>> +  #
>>> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6,
>>> 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F,
>>> 0xC1, 0x4D}|VOID*|0x00000007
>>> +
>>>   [PcdsDynamic, PcdsFixedAtBuild]
>>>     #
>>>     # ARM PSCI function invocations can be done either through hypervisor
>>> diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>> b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>> index 13830cb..b242a29 100644
>>> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
>>> @@ -35,7 +35,7 @@
>>>   typedef struct {
>>>     VENDOR_DEVICE_PATH         SerialDxe;
>>>     UART_DEVICE_PATH           Uart;
>>> -  VENDOR_DEFINED_DEVICE_PATH Vt100;
>>> +  VENDOR_DEFINED_DEVICE_PATH TermType;
>>>     EFI_DEVICE_PATH_PROTOCOL   End;
>>>   } PLATFORM_SERIAL_CONSOLE;
>>>   #pragma pack ()
>>> @@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
>>>     },
>>>       //
>>> -  // VENDOR_DEFINED_DEVICE_PATH Vt100
>>> +  // VENDOR_DEFINED_DEVICE_PATH TermType
>>>     //
>>>     {
>>>       {
>>>         MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
>>>         DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
>>> -    },
>>> -    EFI_VT_100_GUID
>>> +    }
>>> +    //
>>> +    // Guid to be filled in dynamically
>>> +    //
>>>     },
>>>       //
>>> @@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
>>>     //
>>>     // Add the hardcoded serial console device path to ConIn, ConOut,
>>> ErrOut.
>>>     //
>>> +  CopyGuid (&mSerialConsole.TermType.Guid,
>>> +    PcdGetPtr (PcdTerminalTypeGuidBuffer));
>>>     BdsLibUpdateConsoleVariable (L"ConIn",
>>>       (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
>>>     BdsLibUpdateConsoleVariable (L"ConOut",
>>> diff --git
>>> a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>> b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>> index d998216..9a3cfcd 100644
>>> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
>>> @@ -39,6 +39,7 @@
>>>     MdeModulePkg/MdeModulePkg.dec
>>>     MdePkg/MdePkg.dec
>>>     OvmfPkg/OvmfPkg.dec
>>> +  ArmVirtPkg/ArmVirtPkg.dec
>>>     [LibraryClasses]
>>>     BaseLib
>>> @@ -61,6 +62,9 @@
>>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
>>>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
>>>   +[Pcd]
>>> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
>>> +
>>>   [Guids]
>>>     gEfiFileInfoGuid
>>>     gEfiFileSystemInfoGuid
>>
>

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
Ryan Harkin July 8, 2015, 8:36 p.m. UTC | #3
Hi Laszlo,

On 8 July 2015 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:

> On 07/08/15 04:46, Heyi Guo wrote:
> > Hi Roy,
> >
> > I have one question; please see my comments below.
>
> Indeed:
>
> >
> > On 07/08/2015 08:44 AM, Roy Franz wrote:
> >> From: Laszlo Ersek <lersek@redhat.com>
> >>
> >> Add a fixed pointer PCD to allow build-time selection of VT100 or TTY
> >> terminal
> >> type.  The default remains VT100 emulation.
> >> Add support for building the ARM QEMU platforms with the TTY terminal
> >> with the "-D TTY_TERMINAL" build option.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> [Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL
> >> to TTY_TERMINAL]
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>   ArmVirtPkg/ArmVirt.dsc.inc                                     |  6
> >> ++++++
> >>   ArmVirtPkg/ArmVirtPkg.dec                                      |  7
> >> +++++++
> >>   ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c      | 12
> >> ++++++++----
> >>   ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf |  4
> >> ++++
> >>   4 files changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> >> index 7ec0de4..2feebd3 100644
> >> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> >> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> >> @@ -15,6 +15,7 @@
> >>     [Defines]
> >>     DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
> >> +  DEFINE TTY_TERMINAL            = FALSE
>
> > I see in ArmVirtQemu.dsc file (patch 5#), we are using #ifdef
> > TTY_TERMINAL. In my opinion the above statement will cause #ifdef
> > TTY_TERMINAL to be always true.
>
> I agree. That's my only comment for patch #5 too. Patch #5 should be
> consistent with this one here, and use
>
> !if $(TTY_TERMINAL) == TRUE
>
> >>     [LibraryClasses.common]
> >>   !if $(TARGET) == RELEASE
> >> @@ -359,6 +360,11 @@
> >>
> >>
> gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
> >>
> >>   !endif
> >>   +!if $(TTY_TERMINAL) == TRUE
> > And I don't understand why we use "!if" here and "!ifdef" in dsc file.
>
> BTW, we have a good reason for this kind of conditional. We use
>
>   $(WHATEVER_ENABLE) == TRUE
>
> because that allows
>
>   -D WHATEVER_ENABLE=FALSE
>
> on a command line to *override* an earlier
>
>   -D WHATEVER_ENABLE[=TRUE]
>
> on the same command line. The "build" utility has no -U option (for
> undefining a macro).
>
> Assume you have a build script that hardcodes
>
>   -D WHATEVER_ENABLE
>
> because that's how you build the tree most of the time. However,
> occasionally you'd like to build without that feature, and you'd like to
> override the macro. If the script just appends "$@" to the build command
> line, *and* the DSC files use the above kind of conditional, then you
> can simply pass -D WHATEVER_ENABLE=FALSE to override the macro (but
> still use the rest of your build script).
>
> ... Yes, we should have done the same with INTEL_BDS too, and we didn't.
>
> I guess I didn't catch that because I *never* build without INTEL_BDS.
>
> ... Well, that, and because the patch that added INTEL_BDS (SVN r16208)
> was never reviewed on the list.
>
> Anyway, I digress.
>
> > I should have tested the patch directly, but I couldn't apply the mail
> > patches with "git am" right now (git 2.1.4 on Debian; really appreciate
> > if someone could tell me the reason :-) ).
>
> Sure. The edk2 project uses CRLF line endings. For that to work in the
> first place (without git yelling at you all the time), you need to add
> the following git config settings:
>
> [core]
>         whitespace = cr-at-eol
>
> Then, to make git-am "compatible", you further need:
>
> [am]
>         keepcr = true
>
> This will *almost* make things work, but it will still break down when
> you have a patch that adds or removes files *and* has crossed MTA
> boundaries. In this case, you'll have
>
> +++ /dev/null\r
>
> or
>
> --- /dev/null\r
>
> hunk headers in the patch. The am.keepcr setting (which is otherwise
> necessary for the *source code* CRLFs) will prevent git-am from
> stripping the \r even from the /dev/null hunk headers, and that will
> trip up git-am. So you need to pre-process the /dev/null hunk headers
> manually, stripping *only those* \r characters, *if* you have a patch
> that adds or removes files *and* has crossed MTA boundaries. (If the
> patch doesn't cross an MTA, ie. it is straight out of git format-patch,
> then /dev/null will have no \r.)
>
> Summary (I'm sure you'll appreciate a summary): if you'd like to apply
> edk2 patches from the mailing list, with git-am, do the following:
> (1) set "core.whitespace" to "cr-at-eol",
> (2) set "am.keepcr" to "true",
> (3) save the patch emails *intact* into local files (eg. *.eml)
> (4) preprocess those local files with:
>
>     sed -r -i 's,^((---|\+\+\+) /dev/null)\r$,\1,'   *.eml
>
> (5) apply them with git-am.
>

I have an alias in my ~/.gitconfig file:

[alias]
    amm=am --ignore-whitespace --ignore-space-change


Then I use "git amm" to apply patches and that seems to work most of the
time.

But I suspect your method is doing something smarter and more complex
whereas mine is more brute force.



> HTH
> Laszlo
>
> >
> > Heyi Guo
> >
> >> +  # Set terminal type to TtyTerm, the value encoded is
> EFI_TTY_TERM_GUID
> >> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91,
> >> 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51,
> >> 0xef, 0x94}
> >> +!endif
> >> +
> >>   [Components.common]
> >>     #
> >>     # Networking stack
> >> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> >> index 7bbd9ff..9833c5a 100644
> >> --- a/ArmVirtPkg/ArmVirtPkg.dec
> >> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> >> @@ -49,6 +49,13 @@
> >>     #
> >>
> >>
> gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
> >>
> >>   +  #
> >> +  # Binary representation of the GUID that determines the terminal
> >> type. The
> >> +  # size must be exactly 16 bytes. The default value corresponds to
> >> +  # EFI_VT_100_GUID.
> >> +  #
> >> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6,
> >> 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F,
> >> 0xC1, 0x4D}|VOID*|0x00000007
> >> +
> >>   [PcdsDynamic, PcdsFixedAtBuild]
> >>     #
> >>     # ARM PSCI function invocations can be done either through
> hypervisor
> >> diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> >> b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> >> index 13830cb..b242a29 100644
> >> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> >> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> >> @@ -35,7 +35,7 @@
> >>   typedef struct {
> >>     VENDOR_DEVICE_PATH         SerialDxe;
> >>     UART_DEVICE_PATH           Uart;
> >> -  VENDOR_DEFINED_DEVICE_PATH Vt100;
> >> +  VENDOR_DEFINED_DEVICE_PATH TermType;
> >>     EFI_DEVICE_PATH_PROTOCOL   End;
> >>   } PLATFORM_SERIAL_CONSOLE;
> >>   #pragma pack ()
> >> @@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
> >>     },
> >>       //
> >> -  // VENDOR_DEFINED_DEVICE_PATH Vt100
> >> +  // VENDOR_DEFINED_DEVICE_PATH TermType
> >>     //
> >>     {
> >>       {
> >>         MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
> >>         DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
> >> -    },
> >> -    EFI_VT_100_GUID
> >> +    }
> >> +    //
> >> +    // Guid to be filled in dynamically
> >> +    //
> >>     },
> >>       //
> >> @@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
> >>     //
> >>     // Add the hardcoded serial console device path to ConIn, ConOut,
> >> ErrOut.
> >>     //
> >> +  CopyGuid (&mSerialConsole.TermType.Guid,
> >> +    PcdGetPtr (PcdTerminalTypeGuidBuffer));
> >>     BdsLibUpdateConsoleVariable (L"ConIn",
> >>       (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> >>     BdsLibUpdateConsoleVariable (L"ConOut",
> >> diff --git
> >> a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> >> b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> >> index d998216..9a3cfcd 100644
> >> --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> >> +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> >> @@ -39,6 +39,7 @@
> >>     MdeModulePkg/MdeModulePkg.dec
> >>     MdePkg/MdePkg.dec
> >>     OvmfPkg/OvmfPkg.dec
> >> +  ArmVirtPkg/ArmVirtPkg.dec
> >>     [LibraryClasses]
> >>     BaseLib
> >> @@ -61,6 +62,9 @@
> >>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> >>     gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> >>   +[Pcd]
> >> +  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
> >> +
> >>   [Guids]
> >>     gEfiFileInfoGuid
> >>     gEfiFileSystemInfoGuid
> >
>
>
>
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
diff mbox

Patch

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 7ec0de4..2feebd3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -15,6 +15,7 @@ 
 
 [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+  DEFINE TTY_TERMINAL            = FALSE
 
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
@@ -359,6 +360,11 @@ 
   gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
 !endif
 
+!if $(TTY_TERMINAL) == TRUE
+  # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
+  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
+!endif
+
 [Components.common]
   #
   # Networking stack
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 7bbd9ff..9833c5a 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -49,6 +49,13 @@ 
   #
   gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
 
+  #
+  # Binary representation of the GUID that determines the terminal type. The
+  # size must be exactly 16 bytes. The default value corresponds to
+  # EFI_VT_100_GUID.
+  #
+  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
+
 [PcdsDynamic, PcdsFixedAtBuild]
   #
   # ARM PSCI function invocations can be done either through hypervisor
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
index 13830cb..b242a29 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
@@ -35,7 +35,7 @@ 
 typedef struct {
   VENDOR_DEVICE_PATH         SerialDxe;
   UART_DEVICE_PATH           Uart;
-  VENDOR_DEFINED_DEVICE_PATH Vt100;
+  VENDOR_DEFINED_DEVICE_PATH TermType;
   EFI_DEVICE_PATH_PROTOCOL   End;
 } PLATFORM_SERIAL_CONSOLE;
 #pragma pack ()
@@ -67,14 +67,16 @@  STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
   },
 
   //
-  // VENDOR_DEFINED_DEVICE_PATH Vt100
+  // VENDOR_DEFINED_DEVICE_PATH TermType
   //
   {
     {
       MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
       DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
-    },
-    EFI_VT_100_GUID
+    }
+    //
+    // Guid to be filled in dynamically
+    //
   },
 
   //
@@ -421,6 +423,8 @@  PlatformBdsPolicyBehavior (
   //
   // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
   //
+  CopyGuid (&mSerialConsole.TermType.Guid,
+    PcdGetPtr (PcdTerminalTypeGuidBuffer));
   BdsLibUpdateConsoleVariable (L"ConIn",
     (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
   BdsLibUpdateConsoleVariable (L"ConOut",
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
index d998216..9a3cfcd 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
@@ -39,6 +39,7 @@ 
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -61,6 +62,9 @@ 
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
 
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
+
 [Guids]
   gEfiFileInfoGuid
   gEfiFileSystemInfoGuid