Hi Laszlo,
On 2015/11/27 1:44, Laszlo Ersek wrote:
On 11/26/15 18:39, Laszlo Ersek wrote:
Hi Star,
On 11/25/15 02:33, Star Zeng wrote:
1. Update fdf and dsc to use SerialDxe in MdeModulePkg.
2. Separate the code that gets SerialRegBase and SerialRegAccessType
by CbParseLib from CorebootPayloadPkg/Library/SerialPortLib to
PlatformHookLib, and then leverage BaseSerialPortLib16550 in
MdeModulePkg.
3. Remove CorebootPayloadPkg/SerialDxe and
CorebootPayloadPkg/Library/SerialPortLib.
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Maurice Ma <maurice...@intel.com>
Cc: Prince Agyeman <prince.agye...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.z...@intel.com>
Reviewed-by: Maurice Ma <maurice...@intel.com>
---
CorebootPayloadPkg/CorebootPayloadPkg.fdf | 4 +-
CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 11 +-
CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 11 +-
.../Library/PlatformBdsLib/BdsPlatform.h | 5 +-
.../Library/PlatformHookLib/PlatformHookLib.c | 56 +++
.../Library/PlatformHookLib/PlatformHookLib.inf | 38 ++
.../Library/SerialPortLib/SerialPortLib.c | 316 -----------------
.../Library/SerialPortLib/SerialPortLib.inf | 42 ---
CorebootPayloadPkg/SerialDxe/SerialDxe.inf | 55 ---
CorebootPayloadPkg/SerialDxe/SerialIo.c | 392 ---------------------
10 files changed, 114 insertions(+), 816 deletions(-)
create mode 100644
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c
create mode 100644
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
delete mode 100644 CorebootPayloadPkg/Library/SerialPortLib/SerialPortLib.c
delete mode 100644 CorebootPayloadPkg/Library/SerialPortLib/SerialPortLib.inf
delete mode 100644 CorebootPayloadPkg/SerialDxe/SerialDxe.inf
delete mode 100644 CorebootPayloadPkg/SerialDxe/SerialIo.c
I'm not completely sure if this patch caused the breakage, but I got a
CorebootPayloadPkg build failure report, and I sort of guess this patch could
be the culprit.
The build error is as follows:
In file included from <command-line>:0:0:
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c: In function
'PlatformHookSerialPortInitialize':
Build/CorebootPayloadPkgIA32/DEBUG_GCC49/IA32/CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib/DEBUG/AutoGen.h:31:102:
error: right-hand operand of comma expression has no effect
[-Werror=unused-value]
#define _PCD_SET_MODE_BOOL_S_PcdSerialUseMmio(Value)
((_gPcd_BinaryPatch_PcdSerialUseMmio = (Value)), RETURN_SUCCESS)
^
MdePkg/Include/Library/PcdLib.h:687:45: note: in expansion of macro
'_PCD_SET_MODE_BOOL_S_PcdSerialUseMmio'
#define PcdSetBoolS(TokenName, Value) _PCD_SET_MODE_BOOL_S_##TokenName
((Value))
^
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c:48:5: note: in
expansion of macro 'PcdSetBoolS'
PcdSetBoolS (PcdSerialUseMmio, TRUE);
^
Build/CorebootPayloadPkgIA32/DEBUG_GCC49/IA32/CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib/DEBUG/AutoGen.h:31:102:
error: right-hand operand of comma expression has no effect
[-Werror=unused-value]
#define _PCD_SET_MODE_BOOL_S_PcdSerialUseMmio(Value)
((_gPcd_BinaryPatch_PcdSerialUseMmio = (Value)), RETURN_SUCCESS)
^
MdePkg/Include/Library/PcdLib.h:687:45: note: in expansion of macro
'_PCD_SET_MODE_BOOL_S_PcdSerialUseMmio'
#define PcdSetBoolS(TokenName, Value) _PCD_SET_MODE_BOOL_S_##TokenName
((Value))
^
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c:50:5: note: in
expansion of macro 'PcdSetBoolS'
PcdSetBoolS (PcdSerialUseMmio, FALSE);
^
Build/CorebootPayloadPkgIA32/DEBUG_GCC49/IA32/CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib/DEBUG/AutoGen.h:39:110:
error: right-hand operand of comma expression has no effect
[-Werror=unused-value]
#define _PCD_SET_MODE_64_S_PcdSerialRegisterBase(Value)
((_gPcd_BinaryPatch_PcdSerialRegisterBase = (Value)), RETURN_SUCCESS)
^
MdePkg/Include/Library/PcdLib.h:647:45: note: in expansion of macro
'_PCD_SET_MODE_64_S_PcdSerialRegisterBase'
#define PcdSet64S(TokenName, Value) _PCD_SET_MODE_64_S_##TokenName
((Value))
^
CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.c:52:3: note: in
expansion of macro 'PcdSet64S'
PcdSet64S (PcdSerialRegisterBase, (UINT64) SerialRegBase);
^
cc1: all warnings being treated as errors
GNUmakefile:299: recipe for target
'Build/CorebootPayloadPkgIA32/DEBUG_GCC49/IA32/CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib/OUTPUT/PlatformHookLib.obj'
failed
make: ***
[Build/CorebootPayloadPkgIA32/DEBUG_GCC49/IA32/CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib/OUTPUT/PlatformHookLib.obj]
Error 1
Thinking a bit further, I believe this occurs because your patch adds a number
of PcdSetxxxS() calls, but the return status is never checked:
+RETURN_STATUS
+EFIAPI
+PlatformHookSerialPortInitialize (
+ VOID
+ )
+{
+ RETURN_STATUS Status;
+ UINT32 SerialRegBase;
+ UINT32 SerialRegAccessType;
+
+ Status = CbParseSerialInfo (&SerialRegBase, &SerialRegAccessType, NULL);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+
+ if (SerialRegAccessType == 2) { //MMIO
+ PcdSetBoolS (PcdSerialUseMmio, TRUE);
+ } else { //IO
+ PcdSetBoolS (PcdSerialUseMmio, FALSE);
+ }
+ PcdSet64S (PcdSerialRegisterBase, (UINT64) SerialRegBase);
+
+ return RETURN_SUCCESS;
+}
+
This makes PcdSetxxxS() kinda useless -- their purpose is *exactly* so the
programmer can check the return value.
So I'd propose to either check the return values, or to use the older
PcdSetxxxx() macros / functions (that don't return statuses).
Yes, I select to check the return status to fix this build failure.
And I have sent out patch with your Suggested-by.
Thanks for the report.
Star
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
