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 <ler...@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 <ler...@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.fr...@linaro.org>
>> Reviewed-by: Ard Biesheuvel <ard.biesheu...@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.
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
