* [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:47 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning Sami Mujawar
` (17 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the Generator modules. VS2017 compiler also
reports similar warnings for the DXE entry points.
Remove the CONST qualifier for the SystemTable pointer (the
second parameter to the constructor/destructor/DXE Entry
point) to make it compatible with the formal declaration.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c | 6 +++---
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 4 ++--
DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 8 ++++----
11 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
index d87ea7809eae09cdb5ff06290cff8c2d7e7fa904..d1432348f05143bed8588cea54d7b9b6722b9b6d 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
@@ -1,7 +1,7 @@
/** @file
Dynamic Table Factory Dxe
- Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -60,8 +60,8 @@ EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL DynamicTableFactoryProtocol = {
EFI_STATUS
EFIAPI
DynamicTableFactoryDxeInitialize (
- IN EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index 39b75996d9922d2daf953bb28809aa285f7d214c..e27dcaf37470d811c4d6218d4f40d2260f550c02 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -655,8 +655,8 @@ ProcessAcpiTables (
EFI_STATUS
EFIAPI
DynamicTableManagerDxeInitialize (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
index 0b45e15d2d1c2c1e35e801ca719d68d9ff88a65e..346ab5b22f5402bf87c385558f68f080d1b454ed 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
@@ -418,8 +418,8 @@ ACPI_TABLE_GENERATOR Dbg2Generator = {
EFI_STATUS
EFIAPI
AcpiDbg2LibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -442,8 +442,8 @@ AcpiDbg2LibConstructor (
EFI_STATUS
EFIAPI
AcpiDbg2LibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
index e9e4991dc127abb97f859436df0c4ceafdb3f5ec..6cb60e898f697ce4347ad3adeb5bca1cc1f30a0c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
@@ -648,8 +648,8 @@ ACPI_TABLE_GENERATOR FadtGenerator = {
EFI_STATUS
EFIAPI
AcpiFadtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -671,8 +671,8 @@ AcpiFadtLibConstructor (
EFI_STATUS
EFIAPI
AcpiFadtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index a30870806d7f223fde7311e3609cca92f47731cc..7e86c30649bd36ecebac75c7e3a86a0c25cd590b 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -738,8 +738,8 @@ ACPI_TABLE_GENERATOR GtdtGenerator = {
EFI_STATUS
EFIAPI
AcpiGtdtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -761,8 +761,8 @@ AcpiGtdtLibConstructor (
EFI_STATUS
EFIAPI
AcpiGtdtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index 05e360c899c7563aed04c071921865929524fdc4..f303e9f29cc700634d236c08505cab91b9d62cb8 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -2040,8 +2040,8 @@ ACPI_IORT_GENERATOR IortGenerator = {
EFI_STATUS
EFIAPI
AcpiIortLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -2063,8 +2063,8 @@ AcpiIortLibConstructor (
EFI_STATUS
EFIAPI
AcpiIortLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
index dc5238078376746847e7ae14258bbe35ca07ca2c..2651ca15414694c494678bcec655fc882ee2cc0c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
@@ -795,8 +795,8 @@ ACPI_TABLE_GENERATOR MadtGenerator = {
EFI_STATUS
EFIAPI
AcpiMadtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -818,8 +818,8 @@ AcpiMadtLibConstructor (
EFI_STATUS
EFIAPI
AcpiMadtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
index d219e2f04ad6602b8157b048ae840a3094492680..1293ac8e43c8e0ed641b20ba42519172bc541b54 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
@@ -329,8 +329,8 @@ ACPI_TABLE_GENERATOR McfgGenerator = {
EFI_STATUS
EFIAPI
AcpiMcfgLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -352,8 +352,8 @@ AcpiMcfgLibConstructor (
EFI_STATUS
EFIAPI
AcpiMcfgLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index c8713dec625cc18b0806659ffcdc3d9e70411e11..9e42eee9b75bb330833b0f56c98947563d9eb821 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1492,8 +1492,8 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
EFI_STATUS
EFIAPI
AcpiPpttLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -1516,8 +1516,8 @@ AcpiPpttLibConstructor (
EFI_STATUS
EFIAPI
AcpiPpttLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
index 544ea06f0bcb3442b78cf6409846f70ae5360664..2facfaa048115575829c9787084876efbe60834a 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
@@ -109,8 +109,8 @@ ACPI_TABLE_GENERATOR RawGenerator = {
EFI_STATUS
EFIAPI
AcpiRawLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -132,8 +132,8 @@ AcpiRawLibConstructor (
EFI_STATUS
EFIAPI
AcpiRawLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 7b70ba2c2c55a9a15a569680d1a57b359db4e73d..1404279f828d5c06bb7605cb2fe6d864ef7a080e 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -308,8 +308,8 @@ ACPI_TABLE_GENERATOR SpcrGenerator = {
EFI_STATUS
EFIAPI
AcpiSpcrLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -331,8 +331,8 @@ AcpiSpcrLibConstructor (
EFI_STATUS
EFIAPI
AcpiSpcrLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition
2019-08-23 10:55 ` [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition Sami Mujawar
@ 2019-08-23 11:47 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:47 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
Cc: leif.lindholm@linaro.org, Matteo Carlini, nd
[-- Attachment #1: Type: text/plain, Size: 13103 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition
VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the Generator modules. VS2017 compiler also
reports similar warnings for the DXE entry points.
Remove the CONST qualifier for the SystemTable pointer (the
second parameter to the constructor/destructor/DXE Entry
point) to make it compatible with the formal declaration.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c | 6 +++---
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c | 4 ++--
DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c | 8 ++++----
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 8 ++++----
11 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
index d87ea7809eae09cdb5ff06290cff8c2d7e7fa904..d1432348f05143bed8588cea54d7b9b6722b9b6d 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.c
@@ -1,7 +1,7 @@
/** @file
Dynamic Table Factory Dxe
- Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -60,8 +60,8 @@ EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL DynamicTableFactoryProtocol = {
EFI_STATUS
EFIAPI
DynamicTableFactoryDxeInitialize (
- IN EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index 39b75996d9922d2daf953bb28809aa285f7d214c..e27dcaf37470d811c4d6218d4f40d2260f550c02 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -655,8 +655,8 @@ ProcessAcpiTables (
EFI_STATUS
EFIAPI
DynamicTableManagerDxeInitialize (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
index 0b45e15d2d1c2c1e35e801ca719d68d9ff88a65e..346ab5b22f5402bf87c385558f68f080d1b454ed 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
@@ -418,8 +418,8 @@ ACPI_TABLE_GENERATOR Dbg2Generator = {
EFI_STATUS
EFIAPI
AcpiDbg2LibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -442,8 +442,8 @@ AcpiDbg2LibConstructor (
EFI_STATUS
EFIAPI
AcpiDbg2LibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
index e9e4991dc127abb97f859436df0c4ceafdb3f5ec..6cb60e898f697ce4347ad3adeb5bca1cc1f30a0c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c
@@ -648,8 +648,8 @@ ACPI_TABLE_GENERATOR FadtGenerator = {
EFI_STATUS
EFIAPI
AcpiFadtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -671,8 +671,8 @@ AcpiFadtLibConstructor (
EFI_STATUS
EFIAPI
AcpiFadtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index a30870806d7f223fde7311e3609cca92f47731cc..7e86c30649bd36ecebac75c7e3a86a0c25cd590b 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -738,8 +738,8 @@ ACPI_TABLE_GENERATOR GtdtGenerator = {
EFI_STATUS
EFIAPI
AcpiGtdtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -761,8 +761,8 @@ AcpiGtdtLibConstructor (
EFI_STATUS
EFIAPI
AcpiGtdtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index 05e360c899c7563aed04c071921865929524fdc4..f303e9f29cc700634d236c08505cab91b9d62cb8 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -2040,8 +2040,8 @@ ACPI_IORT_GENERATOR IortGenerator = {
EFI_STATUS
EFIAPI
AcpiIortLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -2063,8 +2063,8 @@ AcpiIortLibConstructor (
EFI_STATUS
EFIAPI
AcpiIortLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
index dc5238078376746847e7ae14258bbe35ca07ca2c..2651ca15414694c494678bcec655fc882ee2cc0c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
@@ -795,8 +795,8 @@ ACPI_TABLE_GENERATOR MadtGenerator = {
EFI_STATUS
EFIAPI
AcpiMadtLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -818,8 +818,8 @@ AcpiMadtLibConstructor (
EFI_STATUS
EFIAPI
AcpiMadtLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
index d219e2f04ad6602b8157b048ae840a3094492680..1293ac8e43c8e0ed641b20ba42519172bc541b54 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c
@@ -329,8 +329,8 @@ ACPI_TABLE_GENERATOR McfgGenerator = {
EFI_STATUS
EFIAPI
AcpiMcfgLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -352,8 +352,8 @@ AcpiMcfgLibConstructor (
EFI_STATUS
EFIAPI
AcpiMcfgLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index c8713dec625cc18b0806659ffcdc3d9e70411e11..9e42eee9b75bb330833b0f56c98947563d9eb821 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1492,8 +1492,8 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
EFI_STATUS
EFIAPI
AcpiPpttLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -1516,8 +1516,8 @@ AcpiPpttLibConstructor (
EFI_STATUS
EFIAPI
AcpiPpttLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
index 544ea06f0bcb3442b78cf6409846f70ae5360664..2facfaa048115575829c9787084876efbe60834a 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiRawLibArm/RawGenerator.c
@@ -109,8 +109,8 @@ ACPI_TABLE_GENERATOR RawGenerator = {
EFI_STATUS
EFIAPI
AcpiRawLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -132,8 +132,8 @@ AcpiRawLibConstructor (
EFI_STATUS
EFIAPI
AcpiRawLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 7b70ba2c2c55a9a15a569680d1a57b359db4e73d..1404279f828d5c06bb7605cb2fe6d864ef7a080e 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -308,8 +308,8 @@ ACPI_TABLE_GENERATOR SpcrGenerator = {
EFI_STATUS
EFIAPI
AcpiSpcrLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -331,8 +331,8 @@ AcpiSpcrLibConstructor (
EFI_STATUS
EFIAPI
AcpiSpcrLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
[-- Attachment #2: Type: text/html, Size: 21546 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
2019-08-23 10:55 ` [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:51 ` [edk2-devel] " Alexei Fedorov
2019-11-21 15:22 ` Philippe Mathieu-Daudé
2019-08-23 10:55 ` [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO Sami Mujawar
` (16 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
The edk2 BaseTools report a warning if a local header file
is not listed under the [Sources] section in the INF file.
Add header files to the [Sources] section in the respective
INF files to fix the warnings.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf | 1 +
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf | 3 ++-
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
index b47a690dd72702a2d7157cbb21249ba1d01d08cd..56b69591ac52ca3ba51250dc01a0846b22442ce9 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
@@ -25,6 +25,7 @@ [Sources]
DeviceTreeTableFactory/DeviceTreeTableFactory.c
DynamicTableFactoryDxe.c
SmbiosTableFactory/SmbiosTableFactory.c
+ DynamicTableFactory.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
index 3bbd0e7818c6172f9178b12a3d0960e2d298bca1..2f987e3b6d339d4b5f342cb52b8dc712cf4e585c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
@@ -1,7 +1,7 @@
## @file
# IORT Table Generator
#
-# Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
@@ -18,6 +18,7 @@ [Defines]
[Sources]
IortGenerator.c
+ IortGenerator.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
index 3cb13d7d8fd3df04f3e16cc6161d9736f7579a80..2c7d19513d6ceea29fd9f271f983a6cf6d2c1046 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
@@ -18,6 +18,7 @@ [Defines]
[Sources]
PpttGenerator.c
+ PpttGenerator.h
[Packages]
EmbeddedPkg/EmbeddedPkg.dec
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning
2019-08-23 10:55 ` [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning Sami Mujawar
@ 2019-08-23 11:51 ` Alexei Fedorov
2019-11-21 15:22 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:51 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami Mujawar
[-- Attachment #1: Type: text/plain, Size: 3560 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sami Mujawar via Groups.Io <sami.mujawar=arm.com@groups.io>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning
The edk2 BaseTools report a warning if a local header file
is not listed under the [Sources] section in the INF file.
Add header files to the [Sources] section in the respective
INF files to fix the warnings.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf | 1 +
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf | 3 ++-
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
index b47a690dd72702a2d7157cbb21249ba1d01d08cd..56b69591ac52ca3ba51250dc01a0846b22442ce9 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
+++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
@@ -25,6 +25,7 @@ [Sources]
DeviceTreeTableFactory/DeviceTreeTableFactory.c
DynamicTableFactoryDxe.c
SmbiosTableFactory/SmbiosTableFactory.c
+ DynamicTableFactory.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
index 3bbd0e7818c6172f9178b12a3d0960e2d298bca1..2f987e3b6d339d4b5f342cb52b8dc712cf4e585c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
@@ -1,7 +1,7 @@
## @file
# IORT Table Generator
#
-# Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
@@ -18,6 +18,7 @@ [Defines]
[Sources]
IortGenerator.c
+ IortGenerator.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
index 3cb13d7d8fd3df04f3e16cc6161d9736f7579a80..2c7d19513d6ceea29fd9f271f983a6cf6d2c1046 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
@@ -18,6 +18,7 @@ [Defines]
[Sources]
PpttGenerator.c
+ PpttGenerator.h
[Packages]
EmbeddedPkg/EmbeddedPkg.dec
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 5345 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [edk2-devel] [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning
2019-08-23 10:55 ` [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning Sami Mujawar
2019-08-23 11:51 ` [edk2-devel] " Alexei Fedorov
@ 2019-11-21 15:22 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:22 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The edk2 BaseTools report a warning if a local header file
> is not listed under the [Sources] section in the INF file.
>
> Add header files to the [Sources] section in the respective
> INF files to fix the warnings.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf | 1 +
> DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf | 3 ++-
> DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
> index b47a690dd72702a2d7157cbb21249ba1d01d08cd..56b69591ac52ca3ba51250dc01a0846b22442ce9 100644
> --- a/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
> +++ b/DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf
> @@ -25,6 +25,7 @@ [Sources]
> DeviceTreeTableFactory/DeviceTreeTableFactory.c
> DynamicTableFactoryDxe.c
> SmbiosTableFactory/SmbiosTableFactory.c
> + DynamicTableFactory.h
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
> index 3bbd0e7818c6172f9178b12a3d0960e2d298bca1..2f987e3b6d339d4b5f342cb52b8dc712cf4e585c 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/AcpiIortLibArm.inf
> @@ -1,7 +1,7 @@
> ## @file
> # IORT Table Generator
> #
> -# Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
> +# Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> ##
> @@ -18,6 +18,7 @@ [Defines]
>
> [Sources]
> IortGenerator.c
> + IortGenerator.h
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
> index 3cb13d7d8fd3df04f3e16cc6161d9736f7579a80..2c7d19513d6ceea29fd9f271f983a6cf6d2c1046 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/AcpiPpttLibArm.inf
> @@ -18,6 +18,7 @@ [Defines]
>
> [Sources]
> PpttGenerator.c
> + PpttGenerator.h
>
> [Packages]
> EmbeddedPkg/EmbeddedPkg.dec
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
2019-08-23 10:55 ` [PATCH v1 01/19] DynamicTablesPkg: Fix entry point param definition Sami Mujawar
2019-08-23 10:55 ` [PATCH v1 02/19] DynamicTablesPkg: Fix missing local header warning Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:50 ` [edk2-devel] " Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning Sami Mujawar
` (15 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
The VS2017 compiler reports 'error C2016: C requires that
a struct or union has at least one member' for the struct
CM_ARM_CPU_INFO.
Remove struct CM_ARM_CPU_INFO as this is not in use.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index ac451b306dfd7ba299a83209675b21696be235be..f08b8257678c8b6aacc863bbe1db09587b51a291 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -67,10 +67,6 @@ typedef struct CmArmBootArchInfo {
UINT32 BootArchFlags;
} CM_ARM_BOOT_ARCH_INFO;
-typedef struct CmArmCpuInfo {
- // Reserved for use when SMBIOS tables are implemented
-} CM_ARM_CPU_INFO;
-
/** A structure that describes the
Power Management Profile Information for the Platform.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO
2019-08-23 10:55 ` [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO Sami Mujawar
@ 2019-08-23 11:50 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:50 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami Mujawar
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sami Mujawar via Groups.Io <sami.mujawar=arm.com@groups.io>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO
The VS2017 compiler reports 'error C2016: C requires that
a struct or union has at least one member' for the struct
CM_ARM_CPU_INFO.
Remove struct CM_ARM_CPU_INFO as this is not in use.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index ac451b306dfd7ba299a83209675b21696be235be..f08b8257678c8b6aacc863bbe1db09587b51a291 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -67,10 +67,6 @@ typedef struct CmArmBootArchInfo {
UINT32 BootArchFlags;
} CM_ARM_BOOT_ARCH_INFO;
-typedef struct CmArmCpuInfo {
- // Reserved for use when SMBIOS tables are implemented
-} CM_ARM_CPU_INFO;
-
/** A structure that describes the
Power Management Profile Information for the Platform.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 3391 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (2 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 03/19] DynamicTablesPkg: Remove struct CM_ARM_CPU_INFO Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:53 ` [edk2-devel] " Alexei Fedorov
2019-11-21 15:16 ` Philippe Mathieu-Daudé
2019-08-23 10:55 ` [PATCH v1 05/19] DynamicTablesPkg: Fix Proc node length assignment Sami Mujawar
` (14 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
The VS2017 compiler reports 'warning C4244: '=': conversion
from 'UINT16' to 'UINT8', possible loss of data' for the
SPCR InterfaceType field assignment.
The SPCR InterfaceType field uses the same encoding as that
of the DBG2 table Port Subtype field. However SPCR.InterfaceType
is 8-bit while the Port Subtype field in DBG2 table is 16-bit.
Since the Configuration Manager represents the Serial port
information using the struct CM_ARM_SERIAL_PORT_INFO, the
PortSubtype member in this struct is 16-bit.
To fix the warning an explicit type case is added. A validation
is also added to ensure that the Serial Port Subtype value
provided by the Configuration Manager is within the 8-bit
range (less than 256).
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 1404279f828d5c06bb7605cb2fe6d864ef7a080e..4b2580da7df9b4472ae9bcc3df2138a9e13004d0 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -217,8 +217,21 @@ BuildSpcrTable (
goto error_handler;
}
+ // The SPCR InterfaceType uses the same encoding as that of the
+ // DBG2 table Port Subtype field. However InterfaceType is 8-bit
+ // while the Port Subtype field in the DBG2 table is 16-bit.
+ if ((SerialPortInfo->PortSubtype & 0xFF00) != 0) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: SPCR: Invalid Port Sybtype (must be < 256). Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
// Update the serial port subtype
- AcpiSpcr.InterfaceType = SerialPortInfo->PortSubtype;
+ AcpiSpcr.InterfaceType = (UINT8)SerialPortInfo->PortSubtype;
// Update the base address
AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning
2019-08-23 10:55 ` [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning Sami Mujawar
@ 2019-08-23 11:53 ` Alexei Fedorov
2019-11-21 15:16 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:53 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami Mujawar
[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sami Mujawar via Groups.Io <sami.mujawar=arm.com@groups.io>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning
The VS2017 compiler reports 'warning C4244: '=': conversion
from 'UINT16' to 'UINT8', possible loss of data' for the
SPCR InterfaceType field assignment.
The SPCR InterfaceType field uses the same encoding as that
of the DBG2 table Port Subtype field. However SPCR.InterfaceType
is 8-bit while the Port Subtype field in DBG2 table is 16-bit.
Since the Configuration Manager represents the Serial port
information using the struct CM_ARM_SERIAL_PORT_INFO, the
PortSubtype member in this struct is 16-bit.
To fix the warning an explicit type case is added. A validation
is also added to ensure that the Serial Port Subtype value
provided by the Configuration Manager is within the 8-bit
range (less than 256).
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 1404279f828d5c06bb7605cb2fe6d864ef7a080e..4b2580da7df9b4472ae9bcc3df2138a9e13004d0 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -217,8 +217,21 @@ BuildSpcrTable (
goto error_handler;
}
+ // The SPCR InterfaceType uses the same encoding as that of the
+ // DBG2 table Port Subtype field. However InterfaceType is 8-bit
+ // while the Port Subtype field in the DBG2 table is 16-bit.
+ if ((SerialPortInfo->PortSubtype & 0xFF00) != 0) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: SPCR: Invalid Port Sybtype (must be < 256). Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+
// Update the serial port subtype
- AcpiSpcr.InterfaceType = SerialPortInfo->PortSubtype;
+ AcpiSpcr.InterfaceType = (UINT8)SerialPortInfo->PortSubtype;
// Update the base address
AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 4948 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning
2019-08-23 10:55 ` [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning Sami Mujawar
2019-08-23 11:53 ` [edk2-devel] " Alexei Fedorov
@ 2019-11-21 15:16 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:16 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4244: '=': conversion
> from 'UINT16' to 'UINT8', possible loss of data' for the
> SPCR InterfaceType field assignment.
>
> The SPCR InterfaceType field uses the same encoding as that
> of the DBG2 table Port Subtype field. However SPCR.InterfaceType
> is 8-bit while the Port Subtype field in DBG2 table is 16-bit.
>
> Since the Configuration Manager represents the Serial port
> information using the struct CM_ARM_SERIAL_PORT_INFO, the
> PortSubtype member in this struct is 16-bit.
>
> To fix the warning an explicit type case is added. A validation
> is also added to ensure that the Serial Port Subtype value
> provided by the Configuration Manager is within the 8-bit
> range (less than 256).
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> index 1404279f828d5c06bb7605cb2fe6d864ef7a080e..4b2580da7df9b4472ae9bcc3df2138a9e13004d0 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> @@ -217,8 +217,21 @@ BuildSpcrTable (
> goto error_handler;
> }
>
> + // The SPCR InterfaceType uses the same encoding as that of the
> + // DBG2 table Port Subtype field. However InterfaceType is 8-bit
> + // while the Port Subtype field in the DBG2 table is 16-bit.
> + if ((SerialPortInfo->PortSubtype & 0xFF00) != 0) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: SPCR: Invalid Port Sybtype (must be < 256). Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> +
> // Update the serial port subtype
> - AcpiSpcr.InterfaceType = SerialPortInfo->PortSubtype;
> + AcpiSpcr.InterfaceType = (UINT8)SerialPortInfo->PortSubtype;
>
> // Update the base address
> AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 05/19] DynamicTablesPkg: Fix Proc node length assignment
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (3 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:46 ` [edk2-devel] " Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block " Sami Mujawar
` (13 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
The length field for the Processor Hierarchy node structure is
8-bit wide while the number of private resource field is 32-bit
wide. Therefore, the GetProcHierarchyNodeSize() returns the size
as a 32-bit value.
The VS2017 compiler reports 'warning C4244: '=': conversion from
'UINT32' to 'UINT8', possible loss of data' while assigning the
length field of the Processor Hierarchy node structure.
To fix this, a type cast is added. In addition, there is a check
to ensure that the Processor Hierarchy node size does not exceed
MAX_UINT8.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 9e42eee9b75bb330833b0f56c98947563d9eb821..40699ce113caa8530c89ac20562cf5abda26b88e 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -504,6 +504,7 @@ AddProcHierarchyNodes (
PPTT_NODE_INDEXER * ProcNodeIterator;
UINT32 NodeCount;
+ UINT32 Length;
ASSERT (
(Generator != NULL) &&
@@ -539,8 +540,8 @@ AddProcHierarchyNodes (
// imposed on the Processor Hierarchy node by the specification.
// Note: The length field is 8 bit wide while the number of private
// resource field is 32 bit wide.
- if ((sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
- (ProcInfoNode->NoOfPrivateResources * sizeof (UINT32))) > MAX_UINT8) {
+ Length = GetProcHierarchyNodeSize (ProcInfoNode);
+ if (Length > MAX_UINT8) {
Status = EFI_INVALID_PARAMETER;
DEBUG ((
DEBUG_ERROR,
@@ -556,7 +557,7 @@ AddProcHierarchyNodes (
// Populate the node header
ProcStruct->Type = EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR;
- ProcStruct->Length = GetProcHierarchyNodeSize (ProcInfoNode);
+ ProcStruct->Length = (UINT8)Length;
ProcStruct->Reserved[0] = EFI_ACPI_RESERVED_BYTE;
ProcStruct->Reserved[1] = EFI_ACPI_RESERVED_BYTE;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block length assignment
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (4 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 05/19] DynamicTablesPkg: Fix Proc node length assignment Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:52 ` [edk2-devel] " Alexei Fedorov
2019-11-21 15:56 ` Philippe Mathieu-Daudé
2019-08-23 10:55 ` [PATCH v1 07/19] DynamicTablesPkg: Fix Boot arch flag width Sami Mujawar
` (12 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
The VS2017 compiler reports 'warning C4267: '=': conversion from
'size_t' to 'UINT16', possible loss of data'.
The sizeof() operator is used to calculate the size of the
GT Block structure. The length field in the GT Block structure
is 16-bit wide. Since the return type of sizeof() operator
is size_t the VS2017 compiler reports the above warning.
To fix the warning, an explicit type cast is added. An additional
check is also performed to ensure that the calculated GT Block
length does not exceed MAX_UINT16.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 25 ++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index 7e86c30649bd36ecebac75c7e3a86a0c25cd590b..0e996698887aefca8a7240fc27fe3cb9324fd3e2 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -350,6 +350,7 @@ AddGTBlockList (
EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE * GtBlockFrame;
CM_ARM_GTBLOCK_TIMER_FRAME_INFO * GTBlockTimerFrameList;
UINT32 GTBlockTimerFrameCount;
+ UINTN Length;
ASSERT (Gtdt != NULL);
ASSERT (GTBlockInfo != NULL);
@@ -376,11 +377,27 @@ AddGTBlockList (
return Status;
}
+ Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
+ (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
+ GTBlockInfo->GTBlockTimerFrameCount);
+
+ // Check that the length of the GT block does not
+ // exceed MAX_UINT16
+ if (Length > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: GTDT: Too many GT Frames. Count = %d. " \
+ "Maximum supported GT Block size exceeded. " \
+ "Status = %r\n",
+ GTBlockInfo->GTBlockTimerFrameCount,
+ Status
+ ));
+ return Status;
+ }
+
GTBlock->Type = EFI_ACPI_6_3_GTDT_GT_BLOCK;
- GTBlock->Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
- (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
- GTBlockInfo->GTBlockTimerFrameCount);
-
+ GTBlock->Length = (UINT16)Length;
GTBlock->Reserved = EFI_ACPI_RESERVED_BYTE;
GTBlock->CntCtlBase = GTBlockInfo->GTBlockPhysicalAddress;
GTBlock->GTBlockTimerCount = GTBlockInfo->GTBlockTimerFrameCount;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block length assignment
2019-08-23 10:55 ` [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block " Sami Mujawar
@ 2019-08-23 11:52 ` Alexei Fedorov
2019-11-21 15:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:52 UTC (permalink / raw)
To: devel@edk2.groups.io, Sami Mujawar
[-- Attachment #1: Type: text/plain, Size: 3746 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Sami Mujawar via Groups.Io <sami.mujawar=arm.com@groups.io>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block length assignment
The VS2017 compiler reports 'warning C4267: '=': conversion from
'size_t' to 'UINT16', possible loss of data'.
The sizeof() operator is used to calculate the size of the
GT Block structure. The length field in the GT Block structure
is 16-bit wide. Since the return type of sizeof() operator
is size_t the VS2017 compiler reports the above warning.
To fix the warning, an explicit type cast is added. An additional
check is also performed to ensure that the calculated GT Block
length does not exceed MAX_UINT16.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 25 ++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index 7e86c30649bd36ecebac75c7e3a86a0c25cd590b..0e996698887aefca8a7240fc27fe3cb9324fd3e2 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -350,6 +350,7 @@ AddGTBlockList (
EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE * GtBlockFrame;
CM_ARM_GTBLOCK_TIMER_FRAME_INFO * GTBlockTimerFrameList;
UINT32 GTBlockTimerFrameCount;
+ UINTN Length;
ASSERT (Gtdt != NULL);
ASSERT (GTBlockInfo != NULL);
@@ -376,11 +377,27 @@ AddGTBlockList (
return Status;
}
+ Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
+ (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
+ GTBlockInfo->GTBlockTimerFrameCount);
+
+ // Check that the length of the GT block does not
+ // exceed MAX_UINT16
+ if (Length > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: GTDT: Too many GT Frames. Count = %d. " \
+ "Maximum supported GT Block size exceeded. " \
+ "Status = %r\n",
+ GTBlockInfo->GTBlockTimerFrameCount,
+ Status
+ ));
+ return Status;
+ }
+
GTBlock->Type = EFI_ACPI_6_3_GTDT_GT_BLOCK;
- GTBlock->Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
- (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
- GTBlockInfo->GTBlockTimerFrameCount);
-
+ GTBlock->Length = (UINT16)Length;
GTBlock->Reserved = EFI_ACPI_RESERVED_BYTE;
GTBlock->CntCtlBase = GTBlockInfo->GTBlockPhysicalAddress;
GTBlock->GTBlockTimerCount = GTBlockInfo->GTBlockTimerFrameCount;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 6888 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block length assignment
2019-08-23 10:55 ` [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block " Sami Mujawar
2019-08-23 11:52 ` [edk2-devel] " Alexei Fedorov
@ 2019-11-21 15:56 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:56 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: ard.biesheuvel, leif.lindholm, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4267: '=': conversion from
> 'size_t' to 'UINT16', possible loss of data'.
>
> The sizeof() operator is used to calculate the size of the
> GT Block structure. The length field in the GT Block structure
> is 16-bit wide. Since the return type of sizeof() operator
> is size_t the VS2017 compiler reports the above warning.
>
> To fix the warning, an explicit type cast is added. An additional
> check is also performed to ensure that the calculated GT Block
> length does not exceed MAX_UINT16.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 25 ++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> index 7e86c30649bd36ecebac75c7e3a86a0c25cd590b..0e996698887aefca8a7240fc27fe3cb9324fd3e2 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> @@ -350,6 +350,7 @@ AddGTBlockList (
> EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE * GtBlockFrame;
> CM_ARM_GTBLOCK_TIMER_FRAME_INFO * GTBlockTimerFrameList;
> UINT32 GTBlockTimerFrameCount;
> + UINTN Length;
>
> ASSERT (Gtdt != NULL);
> ASSERT (GTBlockInfo != NULL);
> @@ -376,11 +377,27 @@ AddGTBlockList (
> return Status;
> }
>
> + Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
> + (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
> + GTBlockInfo->GTBlockTimerFrameCount);
> +
> + // Check that the length of the GT block does not
> + // exceed MAX_UINT16
> + if (Length > MAX_UINT16) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: GTDT: Too many GT Frames. Count = %d. " \
> + "Maximum supported GT Block size exceeded. " \
> + "Status = %r\n",
> + GTBlockInfo->GTBlockTimerFrameCount,
> + Status
> + ));
> + return Status;
> + }
> +
> GTBlock->Type = EFI_ACPI_6_3_GTDT_GT_BLOCK;
> - GTBlock->Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
> - (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
> - GTBlockInfo->GTBlockTimerFrameCount);
> -
> + GTBlock->Length = (UINT16)Length;
> GTBlock->Reserved = EFI_ACPI_RESERVED_BYTE;
> GTBlock->CntCtlBase = GTBlockInfo->GTBlockPhysicalAddress;
> GTBlock->GTBlockTimerCount = GTBlockInfo->GTBlockTimerFrameCount;
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 07/19] DynamicTablesPkg: Fix Boot arch flag width
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (5 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block " Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2020-03-27 11:40 ` [edk2-devel] " Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width Sami Mujawar
` (11 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The ArmBootArch field of the FADT table is 16-bit wide. The
VS2017 compiler reports 'warning C4244: '=': conversion from
'UINT32' to 'UINT16', possible loss of data' when assigning the
CM_ARM_BOOT_ARCH_INFO.BootArchFlags value as the width of this
field in CM_ARM_BOOT_ARCH_INFO is 32-bit wide.
To fix this warning, update the CM_ARM_BOOT_ARCH_INFO struct
to make the BootArchFlags field 16-bit wide. This also makes
it compatible with the ACPI FADT specification.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index f08b8257678c8b6aacc863bbe1db09587b51a291..cd955379c90d8633c89e324b5567643d2676e557 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -64,7 +64,7 @@ typedef struct CmArmBootArchInfo {
/** This is the ARM_BOOT_ARCH flags field of the FADT Table
described in the ACPI Table Specification.
*/
- UINT32 BootArchFlags;
+ UINT16 BootArchFlags;
} CM_ARM_BOOT_ARCH_INFO;
/** A structure that describes the
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (6 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 07/19] DynamicTablesPkg: Fix Boot arch flag width Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:53 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage Sami Mujawar
` (10 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The VS2017 compiler reports 'warning C4244: '=': conversion from
'const UINT32' to 'UINT8', possible loss of data' when the ACPI
table revision field is being updated.
The width of the revision field in the EFI_ACPI_DESCRIPTION_HEADER
struct is 8-bit wide. Therefore, to fix the above warning make the
ACPI Table revision field usage 8-bit wide across Dynamic Tables
Framework.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/AcpiTableGenerator.h | 4 ++--
DynamicTablesPkg/Include/StandardNameSpaceObjects.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index 7d6d3442276db7b4abaeb3b053ba489258adea0b..d7a1209c4d40c2e6ffb3dac786c276a30d70b6df 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -284,10 +284,10 @@ typedef struct AcpiTableGenerator {
UINT32 AcpiTableSignature;
/// The ACPI table revision.
- UINT32 AcpiTableRevision;
+ UINT8 AcpiTableRevision;
/// The minimum supported ACPI table revision.
- UINT32 MinAcpiTableRevision;
+ UINT8 MinAcpiTableRevision;
/// The ACPI table creator ID.
UINT32 CreatorId;
diff --git a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
index 21bb7de4044ffd6f97156f761e30fc1790f11c0c..0ba6b163691ea127ae7a7d57fb5f9fdf84789338 100644
--- a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
@@ -79,7 +79,7 @@ typedef struct CmAStdObjAcpiTableInfo {
UINT32 AcpiTableSignature;
/// The ACPI table revision
- UINT32 AcpiTableRevision;
+ UINT8 AcpiTableRevision;
/// The ACPI Table Generator ID
ACPI_TABLE_GENERATOR_ID TableGeneratorId;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width
2019-08-23 10:55 ` [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width Sami Mujawar
@ 2019-08-23 11:53 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:53 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width
The VS2017 compiler reports 'warning C4244: '=': conversion from
'const UINT32' to 'UINT8', possible loss of data' when the ACPI
table revision field is being updated.
The width of the revision field in the EFI_ACPI_DESCRIPTION_HEADER
struct is 8-bit wide. Therefore, to fix the above warning make the
ACPI Table revision field usage 8-bit wide across Dynamic Tables
Framework.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/AcpiTableGenerator.h | 4 ++--
DynamicTablesPkg/Include/StandardNameSpaceObjects.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index 7d6d3442276db7b4abaeb3b053ba489258adea0b..d7a1209c4d40c2e6ffb3dac786c276a30d70b6df 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -284,10 +284,10 @@ typedef struct AcpiTableGenerator {
UINT32 AcpiTableSignature;
/// The ACPI table revision.
- UINT32 AcpiTableRevision;
+ UINT8 AcpiTableRevision;
/// The minimum supported ACPI table revision.
- UINT32 MinAcpiTableRevision;
+ UINT8 MinAcpiTableRevision;
/// The ACPI table creator ID.
UINT32 CreatorId;
diff --git a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
index 21bb7de4044ffd6f97156f761e30fc1790f11c0c..0ba6b163691ea127ae7a7d57fb5f9fdf84789338 100644
--- a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h
@@ -79,7 +79,7 @@ typedef struct CmAStdObjAcpiTableInfo {
UINT32 AcpiTableSignature;
/// The ACPI table revision
- UINT32 AcpiTableRevision;
+ UINT8 AcpiTableRevision;
/// The ACPI Table Generator ID
ACPI_TABLE_GENERATOR_ID TableGeneratorId;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 5998 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (7 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 08/19] DynamicTablesPkg: Fix ACPI table rev field width Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:54 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation Sami Mujawar
` (9 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The VS2017 compiler reports 'warning C4366: The result of
the unary '&' operator may be unaligned' if an address of
an unaligned structure member is passed as an argument to
a function.
Fix this warning by using local variables in place of
unaligned structure members.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 24 +++++++++++++++-----
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 13 +++++------
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 40699ce113caa8530c89ac20562cf5abda26b88e..82070403ac8757f54e839fd00eb4acb3292fc60c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1066,6 +1066,9 @@ BuildPpttTable (
EFI_STATUS Status;
UINT32 TableSize;
UINT32 ProcTopologyStructCount;
+ UINT32 ProcHierarchyNodeCount;
+ UINT32 CacheStructCount;
+ UINT32 IdStructCount;
UINT32 ProcHierarchyNodeOffset;
UINT32 CacheStructOffset;
@@ -1113,7 +1116,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&ProcHierarchyNodeList,
- &Generator->ProcHierarchyNodeCount
+ &ProcHierarchyNodeCount
);
if (EFI_ERROR (Status)) {
DEBUG ((
@@ -1124,7 +1127,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount = Generator->ProcHierarchyNodeCount;
+ ProcTopologyStructCount = ProcHierarchyNodeCount;
+ Generator->ProcHierarchyNodeCount = ProcHierarchyNodeCount;
// Get the cache info and update the processor topology structure count with
// Cache Type Structures (Type 1)
@@ -1132,7 +1136,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&CacheStructList,
- &Generator->CacheStructCount
+ &CacheStructCount
);
if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
DEBUG ((
@@ -1143,7 +1147,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount += Generator->CacheStructCount;
+ ProcTopologyStructCount += CacheStructCount;
+ Generator->CacheStructCount = CacheStructCount;
// Get the processor hierarchy node ID info and update the processor topology
// structure count with ID Structures (Type 2)
@@ -1151,7 +1156,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&IdStructList,
- &Generator->IdStructCount
+ &IdStructCount
);
if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
DEBUG ((
@@ -1163,7 +1168,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount += Generator->IdStructCount;
+ ProcTopologyStructCount += IdStructCount;
+ Generator->IdStructCount = IdStructCount;
// Allocate Node Indexer array
NodeIndexer = (PPTT_NODE_INDEXER*)AllocateZeroPool (
@@ -1475,6 +1481,12 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
// Processor topology node count
0,
+ // Count of Processor Hierarchy Nodes
+ 0,
+ // Count of Cache Structures
+ 0,
+ // Count of Id Structures
+ 0,
// Pointer to PPTT Node Indexer
NULL
};
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
index 6a0fdd08e1533c57285f420555586314c70a5ed5..0a14da502d595e27d87262b1bac681318f1d9ced 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
@@ -167,6 +167,12 @@ typedef struct AcpiPpttGenerator {
ACPI_TABLE_GENERATOR Header;
/// PPTT structure count
UINT32 ProcTopologyStructCount;
+ /// Count of Processor Hierarchy Nodes
+ UINT32 ProcHierarchyNodeCount;
+ /// Count of Cache Structures
+ UINT32 CacheStructCount;
+ /// Count of Id Structures
+ UINT32 IdStructCount;
/// List of indexed CM objects for PPTT generation
PPTT_NODE_INDEXER * NodeIndexer;
/// Pointer to the start of Processor Hierarchy nodes in
@@ -176,13 +182,6 @@ typedef struct AcpiPpttGenerator {
PPTT_NODE_INDEXER * CacheStructIndexedList;
/// Pointer to the start of Id Structures in the Node Indexer array
PPTT_NODE_INDEXER * IdStructIndexedList;
- /// Count of Processor Hierarchy Nodes
- UINT32 ProcHierarchyNodeCount;
- /// Count of Cache Structures
- UINT32 CacheStructCount;
- /// Count of Id Structures
- UINT32 IdStructCount;
-
} ACPI_PPTT_GENERATOR;
#pragma pack()
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage
2019-08-23 10:55 ` [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage Sami Mujawar
@ 2019-08-23 11:54 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:54 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 6009 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage
The VS2017 compiler reports 'warning C4366: The result of
the unary '&' operator may be unaligned' if an address of
an unaligned structure member is passed as an argument to
a function.
Fix this warning by using local variables in place of
unaligned structure members.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 24 +++++++++++++++-----
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h | 13 +++++------
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 40699ce113caa8530c89ac20562cf5abda26b88e..82070403ac8757f54e839fd00eb4acb3292fc60c 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1066,6 +1066,9 @@ BuildPpttTable (
EFI_STATUS Status;
UINT32 TableSize;
UINT32 ProcTopologyStructCount;
+ UINT32 ProcHierarchyNodeCount;
+ UINT32 CacheStructCount;
+ UINT32 IdStructCount;
UINT32 ProcHierarchyNodeOffset;
UINT32 CacheStructOffset;
@@ -1113,7 +1116,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&ProcHierarchyNodeList,
- &Generator->ProcHierarchyNodeCount
+ &ProcHierarchyNodeCount
);
if (EFI_ERROR (Status)) {
DEBUG ((
@@ -1124,7 +1127,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount = Generator->ProcHierarchyNodeCount;
+ ProcTopologyStructCount = ProcHierarchyNodeCount;
+ Generator->ProcHierarchyNodeCount = ProcHierarchyNodeCount;
// Get the cache info and update the processor topology structure count with
// Cache Type Structures (Type 1)
@@ -1132,7 +1136,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&CacheStructList,
- &Generator->CacheStructCount
+ &CacheStructCount
);
if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
DEBUG ((
@@ -1143,7 +1147,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount += Generator->CacheStructCount;
+ ProcTopologyStructCount += CacheStructCount;
+ Generator->CacheStructCount = CacheStructCount;
// Get the processor hierarchy node ID info and update the processor topology
// structure count with ID Structures (Type 2)
@@ -1151,7 +1156,7 @@ BuildPpttTable (
CfgMgrProtocol,
CM_NULL_TOKEN,
&IdStructList,
- &Generator->IdStructCount
+ &IdStructCount
);
if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
DEBUG ((
@@ -1163,7 +1168,8 @@ BuildPpttTable (
goto error_handler;
}
- ProcTopologyStructCount += Generator->IdStructCount;
+ ProcTopologyStructCount += IdStructCount;
+ Generator->IdStructCount = IdStructCount;
// Allocate Node Indexer array
NodeIndexer = (PPTT_NODE_INDEXER*)AllocateZeroPool (
@@ -1475,6 +1481,12 @@ ACPI_PPTT_GENERATOR PpttGenerator = {
// Processor topology node count
0,
+ // Count of Processor Hierarchy Nodes
+ 0,
+ // Count of Cache Structures
+ 0,
+ // Count of Id Structures
+ 0,
// Pointer to PPTT Node Indexer
NULL
};
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
index 6a0fdd08e1533c57285f420555586314c70a5ed5..0a14da502d595e27d87262b1bac681318f1d9ced 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.h
@@ -167,6 +167,12 @@ typedef struct AcpiPpttGenerator {
ACPI_TABLE_GENERATOR Header;
/// PPTT structure count
UINT32 ProcTopologyStructCount;
+ /// Count of Processor Hierarchy Nodes
+ UINT32 ProcHierarchyNodeCount;
+ /// Count of Cache Structures
+ UINT32 CacheStructCount;
+ /// Count of Id Structures
+ UINT32 IdStructCount;
/// List of indexed CM objects for PPTT generation
PPTT_NODE_INDEXER * NodeIndexer;
/// Pointer to the start of Processor Hierarchy nodes in
@@ -176,13 +182,6 @@ typedef struct AcpiPpttGenerator {
PPTT_NODE_INDEXER * CacheStructIndexedList;
/// Pointer to the start of Id Structures in the Node Indexer array
PPTT_NODE_INDEXER * IdStructIndexedList;
- /// Count of Processor Hierarchy Nodes
- UINT32 ProcHierarchyNodeCount;
- /// Count of Cache Structures
- UINT32 CacheStructCount;
- /// Count of Id Structures
- UINT32 IdStructCount;
-
} ACPI_PPTT_GENERATOR;
#pragma pack()
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 11353 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (8 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 09/19] DynamicTablesPkg: Fix unaligned pointers usage Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-11-21 15:20 ` [edk2-devel] " Philippe Mathieu-Daudé
2020-03-27 11:43 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check Sami Mujawar
` (8 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The ARM DCC serial port subtype is an option that is
supported by the DBG2 generator. However, the serial
port initialisation should only be done for PL011/SBSA
compatible UARTs.
Add check to conditionally initialise the serial port.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
@@ -175,7 +175,7 @@ GET_OBJECT_LIST (
CM_ARM_SERIAL_PORT_INFO
);
-/** Initialize the PL011 UART with the parameters obtained from
+/** Initialize the PL011/SBSA UART with the parameters obtained from
the Configuration Manager.
@param [in] SerialPortInfo Pointer to the Serial Port Information.
@@ -353,15 +353,22 @@ BuildDbg2Table (
AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
SerialPortInfo->PortSubtype;
- // Initialize the serial port
- Status = SetupDebugUart (SerialPortInfo);
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
- Status
- ));
- goto error_handler;
+ if ((SerialPortInfo->PortSubtype ==
+ EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART) ||
+ (SerialPortInfo->PortSubtype ==
+ EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
+ (SerialPortInfo->PortSubtype ==
+ EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
+ // Initialize the serial port
+ Status = SetupDebugUart (SerialPortInfo);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
}
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-08-23 10:55 ` [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation Sami Mujawar
@ 2019-11-21 15:20 ` Philippe Mathieu-Daudé
2019-11-21 15:23 ` Leif Lindholm
2020-03-27 11:43 ` Alexei Fedorov
1 sibling, 1 reply; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:20 UTC (permalink / raw)
To: devel, sami.mujawar; +Cc: Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The ARM DCC serial port subtype is an option that is
> supported by the DBG2 generator. However, the serial
> port initialisation should only be done for PL011/SBSA
> compatible UARTs.
>
> Add check to conditionally initialise the serial port.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
> CM_ARM_SERIAL_PORT_INFO
> );
>
> -/** Initialize the PL011 UART with the parameters obtained from
> +/** Initialize the PL011/SBSA UART with the parameters obtained from
> the Configuration Manager.
Isn't the SBSA UART a PL011?
>
> @param [in] SerialPortInfo Pointer to the Serial Port Information.
> @@ -353,15 +353,22 @@ BuildDbg2Table (
> AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
> SerialPortInfo->PortSubtype;
>
> - // Initialize the serial port
> - Status = SetupDebugUart (SerialPortInfo);
> - if (EFI_ERROR (Status)) {
> - DEBUG ((
> - DEBUG_ERROR,
> - "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> - Status
> - ));
> - goto error_handler;
> + if ((SerialPortInfo->PortSubtype ==
> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART) ||
> + (SerialPortInfo->PortSubtype ==
> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
> + (SerialPortInfo->PortSubtype ==
> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
> + // Initialize the serial port
> + Status = SetupDebugUart (SerialPortInfo);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> + Status
> + ));
> + goto error_handler;
> + }
> }
>
> *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-11-21 15:20 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-11-21 15:23 ` Leif Lindholm
2019-11-21 15:29 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 15:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: devel, sami.mujawar, Alexei.Fedorov, Matteo.Carlini, nd
On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
> On 8/23/19 12:55 PM, Sami Mujawar wrote:
> > The ARM DCC serial port subtype is an option that is
> > supported by the DBG2 generator. However, the serial
> > port initialisation should only be done for PL011/SBSA
> > compatible UARTs.
> >
> > Add check to conditionally initialise the serial port.
> >
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
> > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
> > 1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
> > CM_ARM_SERIAL_PORT_INFO
> > );
> > -/** Initialize the PL011 UART with the parameters obtained from
> > +/** Initialize the PL011/SBSA UART with the parameters obtained from
> > the Configuration Manager.
>
> Isn't the SBSA UART a PL011?
No. It's a compatible subset.
So a PL011 can be used as an SBSA UART.
/
Leif
> > @param [in] SerialPortInfo Pointer to the Serial Port Information.
> > @@ -353,15 +353,22 @@ BuildDbg2Table (
> > AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
> > SerialPortInfo->PortSubtype;
> > - // Initialize the serial port
> > - Status = SetupDebugUart (SerialPortInfo);
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((
> > - DEBUG_ERROR,
> > - "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> > - Status
> > - ));
> > - goto error_handler;
> > + if ((SerialPortInfo->PortSubtype ==
> > + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART) ||
> > + (SerialPortInfo->PortSubtype ==
> > + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
> > + (SerialPortInfo->PortSubtype ==
> > + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
> > + // Initialize the serial port
> > + Status = SetupDebugUart (SerialPortInfo);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
> > + Status
> > + ));
> > + goto error_handler;
> > + }
> > }
> > *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
> >
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-11-21 15:23 ` Leif Lindholm
@ 2019-11-21 15:29 ` Philippe Mathieu-Daudé
2019-11-21 15:48 ` Leif Lindholm
0 siblings, 1 reply; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:29 UTC (permalink / raw)
To: Leif Lindholm; +Cc: devel, sami.mujawar, Alexei.Fedorov, Matteo.Carlini, nd
On 11/21/19 4:23 PM, Leif Lindholm wrote:
> On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
>> On 8/23/19 12:55 PM, Sami Mujawar wrote:
>>> The ARM DCC serial port subtype is an option that is
>>> supported by the DBG2 generator. However, the serial
>>> port initialisation should only be done for PL011/SBSA
>>> compatible UARTs.
>>>
>>> Add check to conditionally initialise the serial port.
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>> ---
>>> DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
>>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
>>> CM_ARM_SERIAL_PORT_INFO
>>> );
>>> -/** Initialize the PL011 UART with the parameters obtained from
>>> +/** Initialize the PL011/SBSA UART with the parameters obtained from
>>> the Configuration Manager.
>>
>> Isn't the SBSA UART a PL011?
>
> No. It's a compatible subset.
> So a PL011 can be used as an SBSA UART.
OK thanks.
Can you update the comment? Maybe:
"Initialize the PL011 compatible UART with the parameters ..."
Regardless:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>
> /
> Leif
>
>>> @param [in] SerialPortInfo Pointer to the Serial Port Information.
>>> @@ -353,15 +353,22 @@ BuildDbg2Table (
>>> AcpiDbg2.Dbg2DeviceInfo[DBG_PORT_INDEX_PORT1].Dbg2Device.PortSubtype =
>>> SerialPortInfo->PortSubtype;
>>> - // Initialize the serial port
>>> - Status = SetupDebugUart (SerialPortInfo);
>>> - if (EFI_ERROR (Status)) {
>>> - DEBUG ((
>>> - DEBUG_ERROR,
>>> - "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
>>> - Status
>>> - ));
>>> - goto error_handler;
>>> + if ((SerialPortInfo->PortSubtype ==
>>> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART) ||
>>> + (SerialPortInfo->PortSubtype ==
>>> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART_2X) ||
>>> + (SerialPortInfo->PortSubtype ==
>>> + EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_SBSA_GENERIC_UART)) {
>>> + // Initialize the serial port
>>> + Status = SetupDebugUart (SerialPortInfo);
>>> + if (EFI_ERROR (Status)) {
>>> + DEBUG ((
>>> + DEBUG_ERROR,
>>> + "ERROR: DBG2: Failed to configure debug serial port. Status = %r\n",
>>> + Status
>>> + ));
>>> + goto error_handler;
>>> + }
>>> }
>>> *Table = (EFI_ACPI_DESCRIPTION_HEADER*)&AcpiDbg2;
>>>
>>
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-11-21 15:29 ` Philippe Mathieu-Daudé
@ 2019-11-21 15:48 ` Leif Lindholm
2019-11-21 15:55 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 15:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: devel, sami.mujawar, Alexei.Fedorov, Matteo.Carlini, nd
On Thu, Nov 21, 2019 at 16:29:16 +0100, Philippe Mathieu-Daudé wrote:
> On 11/21/19 4:23 PM, Leif Lindholm wrote:
> > On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
> > > On 8/23/19 12:55 PM, Sami Mujawar wrote:
> > > > The ARM DCC serial port subtype is an option that is
> > > > supported by the DBG2 generator. However, the serial
> > > > port initialisation should only be done for PL011/SBSA
> > > > compatible UARTs.
> > > >
> > > > Add check to conditionally initialise the serial port.
> > > >
> > > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > > > ---
> > > > DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
> > > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
> > > > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> > > > @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
> > > > CM_ARM_SERIAL_PORT_INFO
> > > > );
> > > > -/** Initialize the PL011 UART with the parameters obtained from
> > > > +/** Initialize the PL011/SBSA UART with the parameters obtained from
> > > > the Configuration Manager.
> > >
> > > Isn't the SBSA UART a PL011?
> >
> > No. It's a compatible subset.
> > So a PL011 can be used as an SBSA UART.
>
> OK thanks.
>
> Can you update the comment? Maybe:
>
> "Initialize the PL011 compatible UART with the parameters ..."
The original is correct, the suggested alternative is not (an SBSA
UART is a subset, so not fully compatible).
If the comment was to change, I would suggest that dropping the model
name completely and simply refer to it as "the UART" would be
preferable.
/
Leif
> Regardless:
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>
> >
> > /
> > Leif
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-11-21 15:48 ` Leif Lindholm
@ 2019-11-21 15:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:55 UTC (permalink / raw)
To: Leif Lindholm; +Cc: devel, sami.mujawar, Alexei.Fedorov, Matteo.Carlini, nd
On 11/21/19 4:48 PM, Leif Lindholm wrote:
> On Thu, Nov 21, 2019 at 16:29:16 +0100, Philippe Mathieu-Daudé wrote:
>> On 11/21/19 4:23 PM, Leif Lindholm wrote:
>>> On Thu, Nov 21, 2019 at 16:20:31 +0100, Philippe Mathieu-Daudé wrote:
>>>> On 8/23/19 12:55 PM, Sami Mujawar wrote:
>>>>> The ARM DCC serial port subtype is an option that is
>>>>> supported by the DBG2 generator. However, the serial
>>>>> port initialisation should only be done for PL011/SBSA
>>>>> compatible UARTs.
>>>>>
>>>>> Add check to conditionally initialise the serial port.
>>>>>
>>>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>>>> ---
>>>>> DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 27 ++++++++++++--------
>>>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> index 346ab5b22f5402bf87c385558f68f080d1b454ed..51c843d25f75388104694855ce133b3d61860196 100644
>>>>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
>>>>> @@ -175,7 +175,7 @@ GET_OBJECT_LIST (
>>>>> CM_ARM_SERIAL_PORT_INFO
>>>>> );
>>>>> -/** Initialize the PL011 UART with the parameters obtained from
>>>>> +/** Initialize the PL011/SBSA UART with the parameters obtained from
>>>>> the Configuration Manager.
>>>>
>>>> Isn't the SBSA UART a PL011?
>>>
>>> No. It's a compatible subset.
>>> So a PL011 can be used as an SBSA UART.
>>
>> OK thanks.
>>
>> Can you update the comment? Maybe:
>>
>> "Initialize the PL011 compatible UART with the parameters ..."
>
> The original is correct, the suggested alternative is not (an SBSA
> UART is a subset, so not fully compatible).
OK, thanks for clarifying.
>
> If the comment was to change, I would suggest that dropping the model
> name completely and simply refer to it as "the UART" would be
> preferable.
>
> /
> Leif
>
>> Regardless:
>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>
>>>
>>> /
>>> Leif
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [edk2-devel] [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation
2019-08-23 10:55 ` [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation Sami Mujawar
2019-11-21 15:20 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2020-03-27 11:43 ` Alexei Fedorov
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2020-03-27 11:43 UTC (permalink / raw)
To: Sami Mujawar, devel
[-- Attachment #1: Type: text/plain, Size: 54 bytes --]
Reviewed by: Alexei Fedorov <Alexei.Fedorov@arm.com>
[-- Attachment #2: Type: text/html, Size: 60 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (9 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 10/19] DynamicTablesPkg: Serial debug port initialisation Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:52 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment Sami Mujawar
` (7 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
Removing GT Block frame count check from AddGTBlockTimerFrames()
as this is already validated in BuildGtdtTable().
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index 0e996698887aefca8a7240fc27fe3cb9324fd3e2..c109f2ac207311646e5a4ad3aa465b1c2883e465 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -259,16 +259,6 @@ AddGTBlockTimerFrames (
ASSERT (GtBlockFrame != NULL);
ASSERT (GTBlockTimerFrameList != NULL);
- if (GTBlockFrameCount > 8) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: GTDT: GT Block Frame Count %d is greater than 8\n",
- GTBlockFrameCount
- ));
- ASSERT (GTBlockFrameCount <= 8);
- return EFI_INVALID_PARAMETER;
- }
-
IsFrameNumberDuplicated = FindDuplicateValue (
GTBlockTimerFrameList,
GTBlockFrameCount,
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check
2019-08-23 10:55 ` [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check Sami Mujawar
@ 2019-08-23 11:52 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:52 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check
Removing GT Block frame count check from AddGTBlockTimerFrames()
as this is already validated in BuildGtdtTable().
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index 0e996698887aefca8a7240fc27fe3cb9324fd3e2..c109f2ac207311646e5a4ad3aa465b1c2883e465 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -259,16 +259,6 @@ AddGTBlockTimerFrames (
ASSERT (GtBlockFrame != NULL);
ASSERT (GTBlockTimerFrameList != NULL);
- if (GTBlockFrameCount > 8) {
- DEBUG ((
- DEBUG_ERROR,
- "ERROR: GTDT: GT Block Frame Count %d is greater than 8\n",
- GTBlockFrameCount
- ));
- ASSERT (GTBlockFrameCount <= 8);
- return EFI_INVALID_PARAMETER;
- }
-
IsFrameNumberDuplicated = FindDuplicateValue (
GTBlockTimerFrameList,
GTBlockFrameCount,
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 3900 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (10 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 11/19] DynamicTablesPkg: Remove redundant frame count check Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:52 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 13/19] DynamicTablesPkg: IORT: Fix uninitialized memory usage Sami Mujawar
` (6 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The VS2017 compiler reports 'warning C4267: 'return': conversion
from 'size_t' to 'UINT32', possible loss of data' for a number of
functions that compute the IORT node length. Similarly, it reports
warnings for IORT node length field assignments as the length
field is 16-bit wide.
This patch adds type casts at appropriate places and also implements
validations to ensure that the max width of the respective fields
is not exceeded.
This patch also fixes a typo in one of the local variable names.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 352 ++++++++++++++------
1 file changed, 253 insertions(+), 99 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index f303e9f29cc700634d236c08505cab91b9d62cb8..bd71220ca19679792de2bb6d88eed8d1913a7600 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -140,8 +140,8 @@ GetItsGroupNodeSize (
/* Size of ITS Group Node +
Size of ITS Identifier array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE) +
- (Node->ItsIdCount * sizeof (UINT32));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE) +
+ (Node->ItsIdCount * sizeof (UINT32)));
}
/** Returns the total size required for the ITS Group nodes and
@@ -160,7 +160,7 @@ GetItsGroupNodeSize (
@retval Total size of the ITS Group Nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofItsGroupNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_ITS_GROUP_NODE * NodeList,
@@ -168,7 +168,7 @@ GetSizeofItsGroupNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -176,7 +176,7 @@ GetSizeofItsGroupNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -211,10 +211,10 @@ GetNamedComponentNodeSize (
Size of ID mapping array +
Size of ASCII string + 'padding to 32-bit word aligned'.
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
- ALIGN_VALUE (AsciiStrSize (Node->ObjectName), 4);
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
+ ALIGN_VALUE (AsciiStrSize (Node->ObjectName), 4));
}
/** Returns the total size required for the Named Component nodes and
@@ -233,7 +233,7 @@ GetNamedComponentNodeSize (
@retval Total size of the Named Component nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofNamedComponentNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_NAMED_COMPONENT_NODE * NodeList,
@@ -241,7 +241,7 @@ GetSizeofNamedComponentNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -249,7 +249,7 @@ GetSizeofNamedComponentNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -284,9 +284,9 @@ GetRootComplexNodeSize (
/* Size of Root Complex node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the Root Complex nodes and
@@ -305,7 +305,7 @@ GetRootComplexNodeSize (
@retval Total size of the Root Complex nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofRootComplexNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_ROOT_COMPLEX_NODE * NodeList,
@@ -313,7 +313,7 @@ GetSizeofRootComplexNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -321,7 +321,7 @@ GetSizeofRootComplexNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -358,13 +358,13 @@ GetSmmuV1V2NodeSize (
Size of context interrupt array +
Size of PMU interrupt array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
- (Node->ContextInterruptCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
- (Node->PmuInterruptCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
+ (Node->ContextInterruptCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
+ (Node->PmuInterruptCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)));
}
/** Returns the total size required for the SMMUv1/SMMUv2 nodes and
@@ -383,7 +383,7 @@ GetSmmuV1V2NodeSize (
@retval Total size of the SMMUv1/SMMUv2 nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofSmmuV1V2Nodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_SMMUV1_SMMUV2_NODE * NodeList,
@@ -391,7 +391,7 @@ GetSizeofSmmuV1V2Nodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -399,7 +399,7 @@ GetSizeofSmmuV1V2Nodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -433,9 +433,9 @@ GetSmmuV3NodeSize (
/* Size of SMMU v1/SMMU v2 node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the SMMUv3 nodes and
@@ -454,7 +454,7 @@ GetSmmuV3NodeSize (
@retval Total size of the SMMUv3 nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofSmmuV3Nodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_SMMUV3_NODE * NodeList,
@@ -462,7 +462,7 @@ GetSizeofSmmuV3Nodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -470,7 +470,7 @@ GetSizeofSmmuV3Nodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -504,9 +504,9 @@ GetPmcgNodeSize (
/* Size of PMCG node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the PMCG nodes and
@@ -525,7 +525,7 @@ GetPmcgNodeSize (
@retval Total size of the PMCG nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofPmcgNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_PMCG_NODE * NodeList,
@@ -533,7 +533,7 @@ GetSizeofPmcgNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -541,7 +541,7 @@ GetSizeofPmcgNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -735,6 +735,7 @@ AddItsGroupNodes (
CM_ARM_ITS_IDENTIFIER * ItsIdentifier;
UINT32 ItsIdentifierCount;
UINT32 IdIndex;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -742,9 +743,22 @@ AddItsGroupNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetItsGroupNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: ITS Id Array Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
ItsGroupNode->Node.Type = EFI_ACPI_IORT_TYPE_ITS_GROUP;
- ItsGroupNode->Node.Length = GetItsGroupNodeSize (NodeList);
+ ItsGroupNode->Node.Length = (UINT16)NodeLength;
ItsGroupNode->Node.Revision = 0;
ItsGroupNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
ItsGroupNode->Node.NumIdMappings = 0;
@@ -825,8 +839,9 @@ AddNamedComponentNodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE * NcNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
- UINT32 ObjectNameLenght;
CHAR8 * ObjectName;
+ UINTN ObjectNameLength;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -834,18 +849,30 @@ AddNamedComponentNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetNamedComponentNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Named Component Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
NcNode->Node.Type = EFI_ACPI_IORT_TYPE_NAMED_COMP;
- NcNode->Node.Length =
- GetNamedComponentNodeSize (NodeList);
+ NcNode->Node.Length = (UINT16)NodeLength;
NcNode->Node.Revision = 2;
NcNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
NcNode->Node.NumIdMappings = NodeList->IdMappingCount;
- ObjectNameLenght = AsciiStrLen (NodeList->ObjectName) + 1;
+ ObjectNameLength = AsciiStrLen (NodeList->ObjectName) + 1;
NcNode->Node.IdReference =
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
- (ALIGN_VALUE (ObjectNameLenght, 4));
+ (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
+ (ALIGN_VALUE (ObjectNameLength, 4)));
// Named Component specific data
NcNode->Flags = NodeList->Flags;
@@ -860,7 +887,7 @@ AddNamedComponentNodes (
sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE));
Status = AsciiStrCpyS (
ObjectName,
- ObjectNameLenght,
+ ObjectNameLength,
NodeList->ObjectName
);
if (EFI_ERROR (Status)) {
@@ -936,6 +963,7 @@ AddRootComplexNodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_RC_NODE * RcNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -943,9 +971,22 @@ AddRootComplexNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetRootComplexNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Root Complex Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
RcNode->Node.Type = EFI_ACPI_IORT_TYPE_ROOT_COMPLEX;
- RcNode->Node.Length = GetRootComplexNodeSize (NodeList);
+ RcNode->Node.Length = (UINT16)NodeLength;
RcNode->Node.Revision = 1;
RcNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
RcNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1093,6 +1134,7 @@ AddSmmuV1V2Nodes (
EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT * ContextInterruptArray;
EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT * PmuInterruptArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1100,9 +1142,21 @@ AddSmmuV1V2Nodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetSmmuV1V2NodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: SMMU V1/V2 Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
SmmuNode->Node.Type = EFI_ACPI_IORT_TYPE_SMMUv1v2;
- SmmuNode->Node.Length = GetSmmuV1V2NodeSize (NodeList);
+ SmmuNode->Node.Length = (UINT16)NodeLength;
SmmuNode->Node.Revision = 0;
SmmuNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
SmmuNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1239,6 +1293,7 @@ AddSmmuV3Nodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE * SmmuV3Node;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1246,9 +1301,21 @@ AddSmmuV3Nodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetSmmuV3NodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: SMMU V3 Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
SmmuV3Node->Node.Type = EFI_ACPI_IORT_TYPE_SMMUv3;
- SmmuV3Node->Node.Length = GetSmmuV3NodeSize (NodeList);
+ SmmuV3Node->Node.Length = (UINT16)NodeLength;
SmmuV3Node->Node.Revision = 2;
SmmuV3Node->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
SmmuV3Node->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1344,6 +1411,7 @@ AddPmcgNodes (
EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE * PmcgNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
ACPI_IORT_GENERATOR * Generator;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1352,9 +1420,21 @@ AddPmcgNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetPmcgNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: PMCG Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
PmcgNode->Node.Type = EFI_ACPI_IORT_TYPE_PMCG;
- PmcgNode->Node.Length = GetPmcgNodeSize (NodeList);
+ PmcgNode->Node.Length = (UINT16)NodeLength;
PmcgNode->Node.Revision = 1;
PmcgNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
PmcgNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1448,9 +1528,11 @@ BuildIortTable (
)
{
EFI_STATUS Status;
- UINT32 TableSize;
+
+ UINT64 TableSize;
+ UINT64 NodeSize;
+
UINT32 IortNodeCount;
-
UINT32 ItsGroupNodeCount;
UINT32 NamedComponentNodeCount;
UINT32 RootComplexNodeCount;
@@ -1638,81 +1720,141 @@ BuildIortTable (
// ITS Group Nodes
if (ItsGroupNodeCount > 0) {
- ItsGroupOffset = TableSize;
+ ItsGroupOffset = (UINT32)TableSize;
// Size of ITS Group node list.
- TableSize += GetSizeofItsGroupNodes (
- ItsGroupOffset,
- ItsGroupNodeList,
- ItsGroupNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofItsGroupNodes (
+ ItsGroupOffset,
+ ItsGroupNodeList,
+ ItsGroupNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Group Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// Named Component Nodes
if (NamedComponentNodeCount > 0) {
- NamedComponentOffset = TableSize;
+ NamedComponentOffset = (UINT32)TableSize;
// Size of Named Component node list.
- TableSize += GetSizeofNamedComponentNodes (
- NamedComponentOffset,
- NamedComponentNodeList,
- NamedComponentNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofNamedComponentNodes (
+ NamedComponentOffset,
+ NamedComponentNodeList,
+ NamedComponentNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Named Component Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// Root Complex Nodes
if (RootComplexNodeCount > 0) {
- RootComplexOffset = TableSize;
+ RootComplexOffset = (UINT32)TableSize;
// Size of Root Complex node list.
- TableSize += GetSizeofRootComplexNodes (
- RootComplexOffset,
- RootComplexNodeList,
- RootComplexNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofRootComplexNodes (
+ RootComplexOffset,
+ RootComplexNodeList,
+ RootComplexNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Root Complex Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// SMMUv1/SMMUv2 Nodes
if (SmmuV1V2NodeCount > 0) {
- SmmuV1V2Offset = TableSize;
+ SmmuV1V2Offset = (UINT32)TableSize;
// Size of SMMUv1/SMMUv2 node list.
- TableSize += GetSizeofSmmuV1V2Nodes (
- SmmuV1V2Offset,
- SmmuV1V2NodeList,
- SmmuV1V2NodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofSmmuV1V2Nodes (
+ SmmuV1V2Offset,
+ SmmuV1V2NodeList,
+ SmmuV1V2NodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of SMMUv1/v2 Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// SMMUv3 Nodes
if (SmmuV3NodeCount > 0) {
- SmmuV3Offset = TableSize;
+ SmmuV3Offset = (UINT32)TableSize;
// Size of SMMUv3 node list.
- TableSize += GetSizeofSmmuV3Nodes (
- SmmuV3Offset,
- SmmuV3NodeList,
- SmmuV3NodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofSmmuV3Nodes (
+ SmmuV3Offset,
+ SmmuV3NodeList,
+ SmmuV3NodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of SMMUv3 Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// PMCG Nodes
if (PmcgNodeCount > 0) {
- PmcgOffset = TableSize;
+ PmcgOffset = (UINT32)TableSize;
// Size of PMCG node list.
- TableSize += GetSizeofPmcgNodes (
- PmcgOffset,
- PmcgNodeList,
- PmcgNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofPmcgNodes (
+ PmcgOffset,
+ PmcgNodeList,
+ PmcgNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of PMCG Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
DEBUG ((
DEBUG_INFO,
"INFO: IORT:\n" \
" IortNodeCount = %d\n" \
- " TableSize = %d\n",
+ " TableSize = 0x%lx\n",
IortNodeCount,
TableSize
));
@@ -1765,6 +1907,18 @@ BuildIortTable (
PmcgOffset
));
+ if (TableSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: IORT Table Size 0x%lx > MAX_UINT32," \
+ " Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
// Allocate the Buffer for IORT table
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)AllocateZeroPool (TableSize);
if (*Table == NULL) {
@@ -1783,7 +1937,7 @@ BuildIortTable (
DEBUG ((
DEBUG_INFO,
- "IORT: Iort = 0x%p TableSize = 0x%x\n",
+ "IORT: Iort = 0x%p TableSize = 0x%lx\n",
Iort,
TableSize
));
@@ -1793,7 +1947,7 @@ BuildIortTable (
This,
&Iort->Header,
AcpiTableInfo,
- TableSize
+ (UINT32)TableSize
);
if (EFI_ERROR (Status)) {
DEBUG ((
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment
2019-08-23 10:55 ` [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment Sami Mujawar
@ 2019-08-23 11:52 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:52 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 25581 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment
The VS2017 compiler reports 'warning C4267: 'return': conversion
from 'size_t' to 'UINT32', possible loss of data' for a number of
functions that compute the IORT node length. Similarly, it reports
warnings for IORT node length field assignments as the length
field is 16-bit wide.
This patch adds type casts at appropriate places and also implements
validations to ensure that the max width of the respective fields
is not exceeded.
This patch also fixes a typo in one of the local variable names.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 352 ++++++++++++++------
1 file changed, 253 insertions(+), 99 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index f303e9f29cc700634d236c08505cab91b9d62cb8..bd71220ca19679792de2bb6d88eed8d1913a7600 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -140,8 +140,8 @@ GetItsGroupNodeSize (
/* Size of ITS Group Node +
Size of ITS Identifier array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE) +
- (Node->ItsIdCount * sizeof (UINT32));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE) +
+ (Node->ItsIdCount * sizeof (UINT32)));
}
/** Returns the total size required for the ITS Group nodes and
@@ -160,7 +160,7 @@ GetItsGroupNodeSize (
@retval Total size of the ITS Group Nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofItsGroupNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_ITS_GROUP_NODE * NodeList,
@@ -168,7 +168,7 @@ GetSizeofItsGroupNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -176,7 +176,7 @@ GetSizeofItsGroupNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -211,10 +211,10 @@ GetNamedComponentNodeSize (
Size of ID mapping array +
Size of ASCII string + 'padding to 32-bit word aligned'.
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
- ALIGN_VALUE (AsciiStrSize (Node->ObjectName), 4);
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
+ ALIGN_VALUE (AsciiStrSize (Node->ObjectName), 4));
}
/** Returns the total size required for the Named Component nodes and
@@ -233,7 +233,7 @@ GetNamedComponentNodeSize (
@retval Total size of the Named Component nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofNamedComponentNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_NAMED_COMPONENT_NODE * NodeList,
@@ -241,7 +241,7 @@ GetSizeofNamedComponentNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -249,7 +249,7 @@ GetSizeofNamedComponentNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -284,9 +284,9 @@ GetRootComplexNodeSize (
/* Size of Root Complex node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_RC_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the Root Complex nodes and
@@ -305,7 +305,7 @@ GetRootComplexNodeSize (
@retval Total size of the Root Complex nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofRootComplexNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_ROOT_COMPLEX_NODE * NodeList,
@@ -313,7 +313,7 @@ GetSizeofRootComplexNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -321,7 +321,7 @@ GetSizeofRootComplexNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -358,13 +358,13 @@ GetSmmuV1V2NodeSize (
Size of context interrupt array +
Size of PMU interrupt array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
- (Node->ContextInterruptCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
- (Node->PmuInterruptCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)) +
+ (Node->ContextInterruptCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)) +
+ (Node->PmuInterruptCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT)));
}
/** Returns the total size required for the SMMUv1/SMMUv2 nodes and
@@ -383,7 +383,7 @@ GetSmmuV1V2NodeSize (
@retval Total size of the SMMUv1/SMMUv2 nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofSmmuV1V2Nodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_SMMUV1_SMMUV2_NODE * NodeList,
@@ -391,7 +391,7 @@ GetSizeofSmmuV1V2Nodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -399,7 +399,7 @@ GetSizeofSmmuV1V2Nodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -433,9 +433,9 @@ GetSmmuV3NodeSize (
/* Size of SMMU v1/SMMU v2 node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the SMMUv3 nodes and
@@ -454,7 +454,7 @@ GetSmmuV3NodeSize (
@retval Total size of the SMMUv3 nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofSmmuV3Nodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_SMMUV3_NODE * NodeList,
@@ -462,7 +462,7 @@ GetSizeofSmmuV3Nodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -470,7 +470,7 @@ GetSizeofSmmuV3Nodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -504,9 +504,9 @@ GetPmcgNodeSize (
/* Size of PMCG node +
Size of ID mapping array
*/
- return sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE) +
- (Node->IdMappingCount *
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE));
+ return (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE) +
+ (Node->IdMappingCount *
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE)));
}
/** Returns the total size required for the PMCG nodes and
@@ -525,7 +525,7 @@ GetPmcgNodeSize (
@retval Total size of the PMCG nodes.
**/
STATIC
-UINT32
+UINT64
GetSizeofPmcgNodes (
IN CONST UINT32 NodeStartOffset,
IN CONST CM_ARM_PMCG_NODE * NodeList,
@@ -533,7 +533,7 @@ GetSizeofPmcgNodes (
IN OUT IORT_NODE_INDEXER ** CONST NodeIndexer
)
{
- UINT32 Size;
+ UINT64 Size;
ASSERT (NodeList != NULL);
@@ -541,7 +541,7 @@ GetSizeofPmcgNodes (
while (NodeCount-- != 0) {
(*NodeIndexer)->Token = NodeList->Token;
(*NodeIndexer)->Object = (VOID*)NodeList;
- (*NodeIndexer)->Offset = Size + NodeStartOffset;
+ (*NodeIndexer)->Offset = (UINT32)(Size + NodeStartOffset);
DEBUG ((
DEBUG_INFO,
"IORT: Node Indexer = %p, Token = %p, Object = %p, Offset = 0x%x\n",
@@ -735,6 +735,7 @@ AddItsGroupNodes (
CM_ARM_ITS_IDENTIFIER * ItsIdentifier;
UINT32 ItsIdentifierCount;
UINT32 IdIndex;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -742,9 +743,22 @@ AddItsGroupNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetItsGroupNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: ITS Id Array Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
ItsGroupNode->Node.Type = EFI_ACPI_IORT_TYPE_ITS_GROUP;
- ItsGroupNode->Node.Length = GetItsGroupNodeSize (NodeList);
+ ItsGroupNode->Node.Length = (UINT16)NodeLength;
ItsGroupNode->Node.Revision = 0;
ItsGroupNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
ItsGroupNode->Node.NumIdMappings = 0;
@@ -825,8 +839,9 @@ AddNamedComponentNodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE * NcNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
- UINT32 ObjectNameLenght;
CHAR8 * ObjectName;
+ UINTN ObjectNameLength;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -834,18 +849,30 @@ AddNamedComponentNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetNamedComponentNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Named Component Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
NcNode->Node.Type = EFI_ACPI_IORT_TYPE_NAMED_COMP;
- NcNode->Node.Length =
- GetNamedComponentNodeSize (NodeList);
+ NcNode->Node.Length = (UINT16)NodeLength;
NcNode->Node.Revision = 2;
NcNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
NcNode->Node.NumIdMappings = NodeList->IdMappingCount;
- ObjectNameLenght = AsciiStrLen (NodeList->ObjectName) + 1;
+ ObjectNameLength = AsciiStrLen (NodeList->ObjectName) + 1;
NcNode->Node.IdReference =
- sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
- (ALIGN_VALUE (ObjectNameLenght, 4));
+ (UINT32)(sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE) +
+ (ALIGN_VALUE (ObjectNameLength, 4)));
// Named Component specific data
NcNode->Flags = NodeList->Flags;
@@ -860,7 +887,7 @@ AddNamedComponentNodes (
sizeof (EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE));
Status = AsciiStrCpyS (
ObjectName,
- ObjectNameLenght,
+ ObjectNameLength,
NodeList->ObjectName
);
if (EFI_ERROR (Status)) {
@@ -936,6 +963,7 @@ AddRootComplexNodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_RC_NODE * RcNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -943,9 +971,22 @@ AddRootComplexNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetRootComplexNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Root Complex Node length 0x%lx > MAX_UINT16."
+ " Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
RcNode->Node.Type = EFI_ACPI_IORT_TYPE_ROOT_COMPLEX;
- RcNode->Node.Length = GetRootComplexNodeSize (NodeList);
+ RcNode->Node.Length = (UINT16)NodeLength;
RcNode->Node.Revision = 1;
RcNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
RcNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1093,6 +1134,7 @@ AddSmmuV1V2Nodes (
EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT * ContextInterruptArray;
EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT * PmuInterruptArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1100,9 +1142,21 @@ AddSmmuV1V2Nodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetSmmuV1V2NodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: SMMU V1/V2 Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
SmmuNode->Node.Type = EFI_ACPI_IORT_TYPE_SMMUv1v2;
- SmmuNode->Node.Length = GetSmmuV1V2NodeSize (NodeList);
+ SmmuNode->Node.Length = (UINT16)NodeLength;
SmmuNode->Node.Revision = 0;
SmmuNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
SmmuNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1239,6 +1293,7 @@ AddSmmuV3Nodes (
EFI_STATUS Status;
EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE * SmmuV3Node;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1246,9 +1301,21 @@ AddSmmuV3Nodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetSmmuV3NodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: SMMU V3 Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
SmmuV3Node->Node.Type = EFI_ACPI_IORT_TYPE_SMMUv3;
- SmmuV3Node->Node.Length = GetSmmuV3NodeSize (NodeList);
+ SmmuV3Node->Node.Length = (UINT16)NodeLength;
SmmuV3Node->Node.Revision = 2;
SmmuV3Node->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
SmmuV3Node->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1344,6 +1411,7 @@ AddPmcgNodes (
EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE * PmcgNode;
EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE * IdMapArray;
ACPI_IORT_GENERATOR * Generator;
+ UINT64 NodeLength;
ASSERT (Iort != NULL);
@@ -1352,9 +1420,21 @@ AddPmcgNodes (
NodesStartOffset);
while (NodeCount-- != 0) {
+ NodeLength = GetPmcgNodeSize (NodeList);
+ if (NodeLength > MAX_UINT16) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: PMCG Node length 0x%lx > MAX_UINT16. Status = %r\n",
+ NodeLength,
+ Status
+ ));
+ return Status;
+ }
+
// Populate the node header
PmcgNode->Node.Type = EFI_ACPI_IORT_TYPE_PMCG;
- PmcgNode->Node.Length = GetPmcgNodeSize (NodeList);
+ PmcgNode->Node.Length = (UINT16)NodeLength;
PmcgNode->Node.Revision = 1;
PmcgNode->Node.Reserved = EFI_ACPI_RESERVED_DWORD;
PmcgNode->Node.NumIdMappings = NodeList->IdMappingCount;
@@ -1448,9 +1528,11 @@ BuildIortTable (
)
{
EFI_STATUS Status;
- UINT32 TableSize;
+
+ UINT64 TableSize;
+ UINT64 NodeSize;
+
UINT32 IortNodeCount;
-
UINT32 ItsGroupNodeCount;
UINT32 NamedComponentNodeCount;
UINT32 RootComplexNodeCount;
@@ -1638,81 +1720,141 @@ BuildIortTable (
// ITS Group Nodes
if (ItsGroupNodeCount > 0) {
- ItsGroupOffset = TableSize;
+ ItsGroupOffset = (UINT32)TableSize;
// Size of ITS Group node list.
- TableSize += GetSizeofItsGroupNodes (
- ItsGroupOffset,
- ItsGroupNodeList,
- ItsGroupNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofItsGroupNodes (
+ ItsGroupOffset,
+ ItsGroupNodeList,
+ ItsGroupNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Group Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// Named Component Nodes
if (NamedComponentNodeCount > 0) {
- NamedComponentOffset = TableSize;
+ NamedComponentOffset = (UINT32)TableSize;
// Size of Named Component node list.
- TableSize += GetSizeofNamedComponentNodes (
- NamedComponentOffset,
- NamedComponentNodeList,
- NamedComponentNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofNamedComponentNodes (
+ NamedComponentOffset,
+ NamedComponentNodeList,
+ NamedComponentNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Named Component Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// Root Complex Nodes
if (RootComplexNodeCount > 0) {
- RootComplexOffset = TableSize;
+ RootComplexOffset = (UINT32)TableSize;
// Size of Root Complex node list.
- TableSize += GetSizeofRootComplexNodes (
- RootComplexOffset,
- RootComplexNodeList,
- RootComplexNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofRootComplexNodes (
+ RootComplexOffset,
+ RootComplexNodeList,
+ RootComplexNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of Root Complex Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// SMMUv1/SMMUv2 Nodes
if (SmmuV1V2NodeCount > 0) {
- SmmuV1V2Offset = TableSize;
+ SmmuV1V2Offset = (UINT32)TableSize;
// Size of SMMUv1/SMMUv2 node list.
- TableSize += GetSizeofSmmuV1V2Nodes (
- SmmuV1V2Offset,
- SmmuV1V2NodeList,
- SmmuV1V2NodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofSmmuV1V2Nodes (
+ SmmuV1V2Offset,
+ SmmuV1V2NodeList,
+ SmmuV1V2NodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of SMMUv1/v2 Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// SMMUv3 Nodes
if (SmmuV3NodeCount > 0) {
- SmmuV3Offset = TableSize;
+ SmmuV3Offset = (UINT32)TableSize;
// Size of SMMUv3 node list.
- TableSize += GetSizeofSmmuV3Nodes (
- SmmuV3Offset,
- SmmuV3NodeList,
- SmmuV3NodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofSmmuV3Nodes (
+ SmmuV3Offset,
+ SmmuV3NodeList,
+ SmmuV3NodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of SMMUv3 Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
// PMCG Nodes
if (PmcgNodeCount > 0) {
- PmcgOffset = TableSize;
+ PmcgOffset = (UINT32)TableSize;
// Size of PMCG node list.
- TableSize += GetSizeofPmcgNodes (
- PmcgOffset,
- PmcgNodeList,
- PmcgNodeCount,
- &NodeIndexer
- );
+ NodeSize = GetSizeofPmcgNodes (
+ PmcgOffset,
+ PmcgNodeList,
+ PmcgNodeCount,
+ &NodeIndexer
+ );
+ if (NodeSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: Invalid Size of PMCG Nodes. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }
+ TableSize += NodeSize;
}
DEBUG ((
DEBUG_INFO,
"INFO: IORT:\n" \
" IortNodeCount = %d\n" \
- " TableSize = %d\n",
+ " TableSize = 0x%lx\n",
IortNodeCount,
TableSize
));
@@ -1765,6 +1907,18 @@ BuildIortTable (
PmcgOffset
));
+ if (TableSize > MAX_UINT32) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: IORT: IORT Table Size 0x%lx > MAX_UINT32," \
+ " Status = %r\n",
+ TableSize,
+ Status
+ ));
+ goto error_handler;
+ }
+
// Allocate the Buffer for IORT table
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)AllocateZeroPool (TableSize);
if (*Table == NULL) {
@@ -1783,7 +1937,7 @@ BuildIortTable (
DEBUG ((
DEBUG_INFO,
- "IORT: Iort = 0x%p TableSize = 0x%x\n",
+ "IORT: Iort = 0x%p TableSize = 0x%lx\n",
Iort,
TableSize
));
@@ -1793,7 +1947,7 @@ BuildIortTable (
This,
&Iort->Header,
AcpiTableInfo,
- TableSize
+ (UINT32)TableSize
);
if (EFI_ERROR (Status)) {
DEBUG ((
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 54429 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 13/19] DynamicTablesPkg: IORT: Fix uninitialized memory usage
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (11 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 12/19] DynamicTablesPkg: Fix IORT node length assignment Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2020-03-27 11:45 ` [edk2-devel] " Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 14/19] DynamicTablesPkg: PPTT: " Sami Mujawar
` (5 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
On enabling the /analyse option the VS2017 compiler
reports: warning C6001: Using uninitialized memory.
This warning is reported as some variables that were
being logged were uninitialised. To fix this, moved
the logging code after the variables being logged are
initialised.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 96 ++++++++++----------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
index bd71220ca19679792de2bb6d88eed8d1913a7600..65d006c89e668c8b2b8a14326cddf1b69979597f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
@@ -1738,6 +1738,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " ItsGroupNodeCount = %d\n" \
+ " ItsGroupOffset = %d\n",
+ ItsGroupNodeCount,
+ ItsGroupOffset
+ ));
}
// Named Component Nodes
@@ -1760,6 +1768,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " NamedComponentNodeCount = %d\n" \
+ " NamedComponentOffset = %d\n",
+ NamedComponentNodeCount,
+ NamedComponentOffset
+ ));
}
// Root Complex Nodes
@@ -1782,6 +1798,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " RootComplexNodeCount = %d\n" \
+ " RootComplexOffset = %d\n",
+ RootComplexNodeCount,
+ RootComplexOffset
+ ));
}
// SMMUv1/SMMUv2 Nodes
@@ -1804,6 +1828,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " SmmuV1V2NodeCount = %d\n" \
+ " SmmuV1V2Offset = %d\n",
+ SmmuV1V2NodeCount,
+ SmmuV1V2Offset
+ ));
}
// SMMUv3 Nodes
@@ -1826,6 +1858,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " SmmuV3NodeCount = %d\n" \
+ " SmmuV3Offset = %d\n",
+ SmmuV3NodeCount,
+ SmmuV3Offset
+ ));
}
// PMCG Nodes
@@ -1848,6 +1888,14 @@ BuildIortTable (
goto error_handler;
}
TableSize += NodeSize;
+
+ DEBUG ((
+ DEBUG_INFO,
+ " PmcgNodeCount = %d\n" \
+ " PmcgOffset = %d\n",
+ PmcgNodeCount,
+ PmcgOffset
+ ));
}
DEBUG ((
@@ -1859,54 +1907,6 @@ BuildIortTable (
TableSize
));
- DEBUG ((
- DEBUG_INFO,
- " ItsGroupNodeCount = %d\n" \
- " ItsGroupOffset = %d\n",
- ItsGroupNodeCount,
- ItsGroupOffset
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " NamedComponentNodeCount = %d\n" \
- " NamedComponentOffset = %d\n",
- NamedComponentNodeCount,
- NamedComponentOffset
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " RootComplexNodeCount = %d\n" \
- " RootComplexOffset = %d\n",
- RootComplexNodeCount,
- RootComplexOffset
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " SmmuV1V2NodeCount = %d\n" \
- " SmmuV1V2Offset = %d\n",
- SmmuV1V2NodeCount,
- SmmuV1V2Offset
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " SmmuV3NodeCount = %d\n" \
- " SmmuV3Offset = %d\n",
- SmmuV3NodeCount,
- SmmuV3Offset
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " PmcgNodeCount = %d\n" \
- " PmcgOffset = %d\n",
- PmcgNodeCount,
- PmcgOffset
- ));
-
if (TableSize > MAX_UINT32) {
Status = EFI_INVALID_PARAMETER;
DEBUG ((
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH v1 14/19] DynamicTablesPkg: PPTT: Fix uninitialized memory usage
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (12 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 13/19] DynamicTablesPkg: IORT: Fix uninitialized memory usage Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:49 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI Sami Mujawar
` (4 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
On enabling the /analyse option the VS2017 compiler
reports: warning C6001: Using uninitialized memory.
This warning is reported as some variables that were
being logged were uninitialised. To fix this, moved
the logging code after the variables being logged are
initialised.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 59 ++++++++++----------
1 file changed, 29 insertions(+), 30 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 82070403ac8757f54e839fd00eb4acb3292fc60c..d70fc59e754e7d348965b8c3739822a9f1c4b7e6 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1203,6 +1203,17 @@ BuildPpttTable (
Generator->ProcHierarchyNodeCount,
&NodeIndexer
);
+
+ DEBUG ((
+ DEBUG_INFO,
+ " ProcHierarchyNodeCount = %d\n" \
+ " ProcHierarchyNodeOffset = 0x%x\n" \
+ " ProcHierarchyNodeIndexedList = 0x%p\n",
+ Generator->ProcHierarchyNodeCount,
+ ProcHierarchyNodeOffset,
+ Generator->ProcHierarchyNodeIndexedList
+ ));
+
}
// Include the size of Cache Type Structures and index them
@@ -1215,6 +1226,15 @@ BuildPpttTable (
Generator->CacheStructCount,
&NodeIndexer
);
+ DEBUG ((
+ DEBUG_INFO,
+ " CacheStructCount = %d\n" \
+ " CacheStructOffset = 0x%x\n" \
+ " CacheStructIndexedList = 0x%p\n",
+ Generator->CacheStructCount,
+ CacheStructOffset,
+ Generator->CacheStructIndexedList
+ ));
}
// Include the size of ID Type Structures and index them
@@ -1227,6 +1247,15 @@ BuildPpttTable (
Generator->IdStructCount,
&NodeIndexer
);
+ DEBUG ((
+ DEBUG_INFO,
+ " IdStructCount = %d\n" \
+ " IdStructOffset = 0x%x\n" \
+ " IdStructIndexedList = 0x%p\n",
+ Generator->IdStructCount,
+ IdStructOffset,
+ Generator->IdStructIndexedList
+ ));
}
DEBUG ((
@@ -1238,36 +1267,6 @@ BuildPpttTable (
TableSize
));
- DEBUG ((
- DEBUG_INFO,
- " ProcHierarchyNodeCount = %d\n" \
- " ProcHierarchyNodeOffset = 0x%x\n" \
- " ProcHierarchyNodeIndexedList = 0x%p\n",
- Generator->ProcHierarchyNodeCount,
- ProcHierarchyNodeOffset,
- Generator->ProcHierarchyNodeIndexedList
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " CacheStructCount = %d\n" \
- " CacheStructOffset = 0x%x\n" \
- " CacheStructIndexedList = 0x%p\n",
- Generator->CacheStructCount,
- CacheStructOffset,
- Generator->CacheStructIndexedList
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " IdStructCount = %d\n" \
- " IdStructOffset = 0x%x\n" \
- " IdStructIndexedList = 0x%p\n",
- Generator->IdStructCount,
- IdStructOffset,
- Generator->IdStructIndexedList
- ));
-
// Allocate the Buffer for the PPTT table
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)AllocateZeroPool (TableSize);
if (*Table == NULL) {
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 14/19] DynamicTablesPkg: PPTT: Fix uninitialized memory usage
2019-08-23 10:55 ` [PATCH v1 14/19] DynamicTablesPkg: PPTT: " Sami Mujawar
@ 2019-08-23 11:49 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:49 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 4221 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 14/19] DynamicTablesPkg: PPTT: Fix uninitialized memory usage
On enabling the /analyse option the VS2017 compiler
reports: warning C6001: Using uninitialized memory.
This warning is reported as some variables that were
being logged were uninitialised. To fix this, moved
the logging code after the variables being logged are
initialised.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 59 ++++++++++----------
1 file changed, 29 insertions(+), 30 deletions(-)
diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 82070403ac8757f54e839fd00eb4acb3292fc60c..d70fc59e754e7d348965b8c3739822a9f1c4b7e6 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -1203,6 +1203,17 @@ BuildPpttTable (
Generator->ProcHierarchyNodeCount,
&NodeIndexer
);
+
+ DEBUG ((
+ DEBUG_INFO,
+ " ProcHierarchyNodeCount = %d\n" \
+ " ProcHierarchyNodeOffset = 0x%x\n" \
+ " ProcHierarchyNodeIndexedList = 0x%p\n",
+ Generator->ProcHierarchyNodeCount,
+ ProcHierarchyNodeOffset,
+ Generator->ProcHierarchyNodeIndexedList
+ ));
+
}
// Include the size of Cache Type Structures and index them
@@ -1215,6 +1226,15 @@ BuildPpttTable (
Generator->CacheStructCount,
&NodeIndexer
);
+ DEBUG ((
+ DEBUG_INFO,
+ " CacheStructCount = %d\n" \
+ " CacheStructOffset = 0x%x\n" \
+ " CacheStructIndexedList = 0x%p\n",
+ Generator->CacheStructCount,
+ CacheStructOffset,
+ Generator->CacheStructIndexedList
+ ));
}
// Include the size of ID Type Structures and index them
@@ -1227,6 +1247,15 @@ BuildPpttTable (
Generator->IdStructCount,
&NodeIndexer
);
+ DEBUG ((
+ DEBUG_INFO,
+ " IdStructCount = %d\n" \
+ " IdStructOffset = 0x%x\n" \
+ " IdStructIndexedList = 0x%p\n",
+ Generator->IdStructCount,
+ IdStructOffset,
+ Generator->IdStructIndexedList
+ ));
}
DEBUG ((
@@ -1238,36 +1267,6 @@ BuildPpttTable (
TableSize
));
- DEBUG ((
- DEBUG_INFO,
- " ProcHierarchyNodeCount = %d\n" \
- " ProcHierarchyNodeOffset = 0x%x\n" \
- " ProcHierarchyNodeIndexedList = 0x%p\n",
- Generator->ProcHierarchyNodeCount,
- ProcHierarchyNodeOffset,
- Generator->ProcHierarchyNodeIndexedList
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " CacheStructCount = %d\n" \
- " CacheStructOffset = 0x%x\n" \
- " CacheStructIndexedList = 0x%p\n",
- Generator->CacheStructCount,
- CacheStructOffset,
- Generator->CacheStructIndexedList
- ));
-
- DEBUG ((
- DEBUG_INFO,
- " IdStructCount = %d\n" \
- " IdStructOffset = 0x%x\n" \
- " IdStructIndexedList = 0x%p\n",
- Generator->IdStructCount,
- IdStructOffset,
- Generator->IdStructIndexedList
- ));
-
// Allocate the Buffer for the PPTT table
*Table = (EFI_ACPI_DESCRIPTION_HEADER*)AllocateZeroPool (TableSize);
if (*Table == NULL) {
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 8252 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (13 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 14/19] DynamicTablesPkg: PPTT: " Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:50 ` Alexei Fedorov
2019-11-21 15:42 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-08-23 10:55 ` [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis Sami Mujawar
` (3 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
The Dynamic Tables Factory protocol has an erroneous
EFIAPI calling convention macro in the function
pointer declaration.
Remove the erroneous EFIAPI calling convention macro
from the interface declarations.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
index d07d0cac36dedb0568f6f604daa22429565aafd3..ff2331b060019976319fc2707f65252130a5326c 100644
--- a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
+++ b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -53,7 +53,6 @@ typedef struct DynamicTableFactoryInfo EDKII_DYNAMIC_TABLE_FACTORY_INFO;
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_ACPI_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST ACPI_TABLE_GENERATOR_ID GeneratorId,
@@ -73,7 +72,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_ACPI_TABLE_GENERATOR) (
IN CONST ACPI_TABLE_GENERATOR * CONST Generator
);
@@ -89,7 +87,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_ACPI_TABLE_GENERATOR) (
IN CONST ACPI_TABLE_GENERATOR * CONST Generator
);
@@ -109,7 +106,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_SMBIOS_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST SMBIOS_TABLE_GENERATOR_ID GeneratorId,
@@ -129,7 +125,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_SMBIOS_TABLE_GENERATOR) (
IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
);
@@ -145,7 +140,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_SMBIOS_TABLE_GENERATOR) (
IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
);
@@ -165,7 +159,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_DT_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST DT_TABLE_GENERATOR_ID GeneratorId,
@@ -185,7 +178,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_DT_TABLE_GENERATOR) (
IN CONST DT_TABLE_GENERATOR * CONST Generator
);
@@ -204,7 +196,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_DT_TABLE_GENERATOR) (
IN CONST DT_TABLE_GENERATOR * CONST Generator
);
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI
2019-08-23 10:55 ` [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI Sami Mujawar
@ 2019-08-23 11:50 ` Alexei Fedorov
2019-11-21 15:42 ` [edk2-devel] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:50 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 4042 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI
The Dynamic Tables Factory protocol has an erroneous
EFIAPI calling convention macro in the function
pointer declaration.
Remove the erroneous EFIAPI calling convention macro
from the interface declarations.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
index d07d0cac36dedb0568f6f604daa22429565aafd3..ff2331b060019976319fc2707f65252130a5326c 100644
--- a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
+++ b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -53,7 +53,6 @@ typedef struct DynamicTableFactoryInfo EDKII_DYNAMIC_TABLE_FACTORY_INFO;
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_ACPI_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST ACPI_TABLE_GENERATOR_ID GeneratorId,
@@ -73,7 +72,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_ACPI_TABLE_GENERATOR) (
IN CONST ACPI_TABLE_GENERATOR * CONST Generator
);
@@ -89,7 +87,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_ACPI_TABLE_GENERATOR) (
IN CONST ACPI_TABLE_GENERATOR * CONST Generator
);
@@ -109,7 +106,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_SMBIOS_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST SMBIOS_TABLE_GENERATOR_ID GeneratorId,
@@ -129,7 +125,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_SMBIOS_TABLE_GENERATOR) (
IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
);
@@ -145,7 +140,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_SMBIOS_TABLE_GENERATOR) (
IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
);
@@ -165,7 +159,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_DT_TABLE_GENERATOR) (
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
IN CONST DT_TABLE_GENERATOR_ID GeneratorId,
@@ -185,7 +178,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_DT_TABLE_GENERATOR) (
IN CONST DT_TABLE_GENERATOR * CONST Generator
);
@@ -204,7 +196,6 @@ EFIAPI
**/
typedef
EFI_STATUS
-EFIAPI
(EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_DT_TABLE_GENERATOR) (
IN CONST DT_TABLE_GENERATOR * CONST Generator
);
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 6889 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [edk2-devel] [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI
2019-08-23 10:55 ` [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI Sami Mujawar
2019-08-23 11:50 ` Alexei Fedorov
@ 2019-11-21 15:42 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:42 UTC (permalink / raw)
To: devel, sami.mujawar; +Cc: Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The Dynamic Tables Factory protocol has an erroneous
> EFIAPI calling convention macro in the function
> pointer declaration.
>
> Remove the erroneous EFIAPI calling convention macro
> from the interface declarations.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
> index d07d0cac36dedb0568f6f604daa22429565aafd3..ff2331b060019976319fc2707f65252130a5326c 100644
> --- a/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
> +++ b/DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
> @@ -1,6 +1,6 @@
> /** @file
>
> - Copyright (c) 2017 - 2018, ARM Limited. All rights reserved.
> + Copyright (c) 2017 - 2019, ARM Limited. All rights reserved.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -53,7 +53,6 @@ typedef struct DynamicTableFactoryInfo EDKII_DYNAMIC_TABLE_FACTORY_INFO;
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_ACPI_TABLE_GENERATOR) (
> IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
> IN CONST ACPI_TABLE_GENERATOR_ID GeneratorId,
> @@ -73,7 +72,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_ACPI_TABLE_GENERATOR) (
> IN CONST ACPI_TABLE_GENERATOR * CONST Generator
> );
> @@ -89,7 +87,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_ACPI_TABLE_GENERATOR) (
> IN CONST ACPI_TABLE_GENERATOR * CONST Generator
> );
> @@ -109,7 +106,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_SMBIOS_TABLE_GENERATOR) (
> IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
> IN CONST SMBIOS_TABLE_GENERATOR_ID GeneratorId,
> @@ -129,7 +125,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_SMBIOS_TABLE_GENERATOR) (
> IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
> );
> @@ -145,7 +140,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_SMBIOS_TABLE_GENERATOR) (
> IN CONST SMBIOS_TABLE_GENERATOR * CONST Generator
> );
> @@ -165,7 +159,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_GET_DT_TABLE_GENERATOR) (
> IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL * CONST This,
> IN CONST DT_TABLE_GENERATOR_ID GeneratorId,
> @@ -185,7 +178,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_REGISTER_DT_TABLE_GENERATOR) (
> IN CONST DT_TABLE_GENERATOR * CONST Generator
> );
> @@ -204,7 +196,6 @@ EFIAPI
> **/
> typedef
> EFI_STATUS
> -EFIAPI
> (EFIAPI * EDKII_DYNAMIC_TABLE_FACTORY_DEREGISTER_DT_TABLE_GENERATOR) (
> IN CONST DT_TABLE_GENERATOR * CONST Generator
> );
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (14 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 15/19] DynamicTablesPkg: Remove erroneous use of EFIAPI Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:49 ` Alexei Fedorov
2019-08-23 10:55 ` [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning Sami Mujawar
` (2 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel; +Cc: Sami Mujawar, Alexei.Fedorov, leif.lindholm, Matteo.Carlini, nd
Add build option STATIC_ANALYSIS to enable VS2017 static
code analysis.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/DynamicTablesPkg.dsc | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
index dfe6c07111059cdd3eed23becf5000c04270bd9c..6ff6ff019e080f355ca24c9d05f71f8b85ab9077 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -4,7 +4,7 @@
# Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
+# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -39,3 +39,10 @@ [Components.common]
[BuildOptions]
*_*_*_CC_FLAGS = -DDISABLE_NEW_DEPRECATED_INTERFACES
+
+!ifdef STATIC_ANALYSIS
+ # Check all rules
+ # Inhibit C6305: Potential mismatch between sizeof and countof quantities.
+ *_VS2017_*_CC_FLAGS = /wd6305 /analyze
+!endif
+
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis
2019-08-23 10:55 ` [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis Sami Mujawar
@ 2019-08-23 11:49 ` Alexei Fedorov
0 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:49 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis
Add build option STATIC_ANALYSIS to enable VS2017 static
code analysis.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
DynamicTablesPkg/DynamicTablesPkg.dsc | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
index dfe6c07111059cdd3eed23becf5000c04270bd9c..6ff6ff019e080f355ca24c9d05f71f8b85ab9077 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -4,7 +4,7 @@
# Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
+# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -39,3 +39,10 @@ [Components.common]
[BuildOptions]
*_*_*_CC_FLAGS = -DDISABLE_NEW_DEPRECATED_INTERFACES
+
+!ifdef STATIC_ANALYSIS
+ # Check all rules
+ # Inhibit C6305: Potential mismatch between sizeof and countof quantities.
+ *_VS2017_*_CC_FLAGS = /wd6305 /analyze
+!endif
+
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 3433 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (15 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 16/19] DynamicTablesPkg: Option for VS2017 static code analysis Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:48 ` Alexei Fedorov
` (2 more replies)
2019-08-23 10:55 ` [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning Sami Mujawar
2019-08-23 10:55 ` [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use Sami Mujawar
18 siblings, 3 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, Alexei.Fedorov, ard.biesheuvel, leif.lindholm,
Matteo.Carlini, nd
The VS2017 compiler reports 'warning C4244: '=': conversion
from 'UINT64' to 'UINT32', possible loss of data' for the
calculation of the UART Divisor value.
Fix this warning by adding appropriate typecast and a validation
that ensures that the UART divisor value generated does not exceed
MAX_UINT32.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 801990d9551a638c17d560d4226137b8a3ee47bb..2d3c279cce49304959953ec4a34b50e09a7d0045 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -2,7 +2,7 @@
Serial I/O Port library functions with no library constructor/destructor
Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
- Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -78,6 +78,7 @@ PL011UartInitializePort (
UINT32 Integer;
UINT32 Fractional;
UINT32 HardwareFifoDepth;
+ UINT64 DivisorValue;
HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \
> PL011_VER_R1P4) \
@@ -188,7 +189,12 @@ PL011UartInitializePort (
return RETURN_INVALID_PARAMETER;
}
- Divisor = (UartClkInHz * 4) / *BaudRate;
+ DivisorValue = (((UINT64)UartClkInHz * 4) / *BaudRate);
+ if (DivisorValue > MAX_UINT32) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ Divisor = (UINT32)DivisorValue;
Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
Fractional = Divisor & FRACTION_PART_MASK;
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning
2019-08-23 10:55 ` [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning Sami Mujawar
@ 2019-08-23 11:48 ` Alexei Fedorov
2019-11-21 12:35 ` Leif Lindholm
2019-11-21 15:13 ` [edk2-devel] " Philippe Mathieu-Daudé
2 siblings, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:48 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 2822 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning
The VS2017 compiler reports 'warning C4244: '=': conversion
from 'UINT64' to 'UINT32', possible loss of data' for the
calculation of the UART Divisor value.
Fix this warning by adding appropriate typecast and a validation
that ensures that the UART divisor value generated does not exceed
MAX_UINT32.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 801990d9551a638c17d560d4226137b8a3ee47bb..2d3c279cce49304959953ec4a34b50e09a7d0045 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -2,7 +2,7 @@
Serial I/O Port library functions with no library constructor/destructor
Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
- Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -78,6 +78,7 @@ PL011UartInitializePort (
UINT32 Integer;
UINT32 Fractional;
UINT32 HardwareFifoDepth;
+ UINT64 DivisorValue;
HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \
> PL011_VER_R1P4) \
@@ -188,7 +189,12 @@ PL011UartInitializePort (
return RETURN_INVALID_PARAMETER;
}
- Divisor = (UartClkInHz * 4) / *BaudRate;
+ DivisorValue = (((UINT64)UartClkInHz * 4) / *BaudRate);
+ if (DivisorValue > MAX_UINT32) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ Divisor = (UINT32)DivisorValue;
Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
Fractional = Divisor & FRACTION_PART_MASK;
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 4841 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning
2019-08-23 10:55 ` [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning Sami Mujawar
2019-08-23 11:48 ` Alexei Fedorov
@ 2019-11-21 12:35 ` Leif Lindholm
2019-11-21 15:13 ` [edk2-devel] " Philippe Mathieu-Daudé
2 siblings, 0 replies; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 12:35 UTC (permalink / raw)
To: Sami Mujawar; +Cc: devel, Alexei.Fedorov, ard.biesheuvel, Matteo.Carlini, nd
On Fri, Aug 23, 2019 at 11:55:37 +0100, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4244: '=': conversion
> from 'UINT64' to 'UINT32', possible loss of data' for the
> calculation of the UART Divisor value.
>
> Fix this warning by adding appropriate typecast and a validation
> that ensures that the UART divisor value generated does not exceed
> MAX_UINT32.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Apologies for missing this set.
> ---
> ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 801990d9551a638c17d560d4226137b8a3ee47bb..2d3c279cce49304959953ec4a34b50e09a7d0045 100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -2,7 +2,7 @@
> Serial I/O Port library functions with no library constructor/destructor
>
> Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> - Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -78,6 +78,7 @@ PL011UartInitializePort (
> UINT32 Integer;
> UINT32 Fractional;
> UINT32 HardwareFifoDepth;
> + UINT64 DivisorValue;
>
> HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \
> > PL011_VER_R1P4) \
> @@ -188,7 +189,12 @@ PL011UartInitializePort (
> return RETURN_INVALID_PARAMETER;
> }
>
> - Divisor = (UartClkInHz * 4) / *BaudRate;
> + DivisorValue = (((UINT64)UartClkInHz * 4) / *BaudRate);
> + if (DivisorValue > MAX_UINT32) {
> + return RETURN_INVALID_PARAMETER;
> + }
> +
> + Divisor = (UINT32)DivisorValue;
> Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> Fractional = Divisor & FRACTION_PART_MASK;
> }
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [edk2-devel] [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning
2019-08-23 10:55 ` [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning Sami Mujawar
2019-08-23 11:48 ` Alexei Fedorov
2019-11-21 12:35 ` Leif Lindholm
@ 2019-11-21 15:13 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 57+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-21 15:13 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: Alexei.Fedorov, ard.biesheuvel, leif.lindholm, Matteo.Carlini, nd
On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4244: '=': conversion
> from 'UINT64' to 'UINT32', possible loss of data' for the
> calculation of the UART Divisor value.
>
> Fix this warning by adding appropriate typecast and a validation
> that ensures that the UART divisor value generated does not exceed
> MAX_UINT32.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 801990d9551a638c17d560d4226137b8a3ee47bb..2d3c279cce49304959953ec4a34b50e09a7d0045 100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -2,7 +2,7 @@
> Serial I/O Port library functions with no library constructor/destructor
>
> Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> - Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -78,6 +78,7 @@ PL011UartInitializePort (
> UINT32 Integer;
> UINT32 Fractional;
> UINT32 HardwareFifoDepth;
> + UINT64 DivisorValue;
>
> HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \
> > PL011_VER_R1P4) \
> @@ -188,7 +189,12 @@ PL011UartInitializePort (
> return RETURN_INVALID_PARAMETER;
> }
>
> - Divisor = (UartClkInHz * 4) / *BaudRate;
> + DivisorValue = (((UINT64)UartClkInHz * 4) / *BaudRate);
> + if (DivisorValue > MAX_UINT32) {
> + return RETURN_INVALID_PARAMETER;
> + }
> +
> + Divisor = (UINT32)DivisorValue;
> Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> Fractional = Divisor & FRACTION_PART_MASK;
> }
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (16 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 17/19] ArmPlatformPkg: Fix UART divisor warning Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:49 ` Alexei Fedorov
2019-11-21 12:35 ` Leif Lindholm
2019-08-23 10:55 ` [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use Sami Mujawar
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, Alexei.Fedorov, ard.biesheuvel, leif.lindholm,
Matteo.Carlini, nd
The VS2017 compiler reports 'warning C6326: potential
comparison of a constant with another constant' when
a fixed PCD value is compared with a constant value.
The faulting code is as marked by '-->' below:
--> if (FixedPcdGet32 (PL011UartInteger) != 0) {
Integer = FixedPcdGet32 (PL011UartInteger);
Fractional = FixedPcdGet32 (PL011UartFractional);
} else {
...
The macro FixedPcdGet32 (PL011UartInteger) evaluates
to a macro _PCD_VALUE_PL011UartInteger that is defined
by the build system to represent the UART Integer
value. Therefore, the VS2017 compiler reports the above
warning.
Fix this warning by enclosing the code in appropriate
#if .. #else .. #endif directives.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -174,10 +174,10 @@ PL011UartInitializePort (
//
// If PL011 Integer value has been defined then always ignore the BAUD rate
- if (FixedPcdGet32 (PL011UartInteger) != 0) {
+#if (FixedPcdGet32 (PL011UartInteger) != 0)
Integer = FixedPcdGet32 (PL011UartInteger);
Fractional = FixedPcdGet32 (PL011UartFractional);
- } else {
+#else
// If BAUD rate is zero then replace it with the system default value
if (*BaudRate == 0) {
*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
@@ -197,7 +197,7 @@ PL011UartInitializePort (
Divisor = (UINT32)DivisorValue;
Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
Fractional = Divisor & FRACTION_PART_MASK;
- }
+#endif
//
// If PL011 is already initialized, check the current settings
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-08-23 10:55 ` [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning Sami Mujawar
@ 2019-08-23 11:49 ` Alexei Fedorov
2019-11-21 12:35 ` Leif Lindholm
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:49 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 2954 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
The VS2017 compiler reports 'warning C6326: potential
comparison of a constant with another constant' when
a fixed PCD value is compared with a constant value.
The faulting code is as marked by '-->' below:
--> if (FixedPcdGet32 (PL011UartInteger) != 0) {
Integer = FixedPcdGet32 (PL011UartInteger);
Fractional = FixedPcdGet32 (PL011UartFractional);
} else {
...
The macro FixedPcdGet32 (PL011UartInteger) evaluates
to a macro _PCD_VALUE_PL011UartInteger that is defined
by the build system to represent the UART Integer
value. Therefore, the VS2017 compiler reports the above
warning.
Fix this warning by enclosing the code in appropriate
#if .. #else .. #endif directives.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -174,10 +174,10 @@ PL011UartInitializePort (
//
// If PL011 Integer value has been defined then always ignore the BAUD rate
- if (FixedPcdGet32 (PL011UartInteger) != 0) {
+#if (FixedPcdGet32 (PL011UartInteger) != 0)
Integer = FixedPcdGet32 (PL011UartInteger);
Fractional = FixedPcdGet32 (PL011UartFractional);
- } else {
+#else
// If BAUD rate is zero then replace it with the system default value
if (*BaudRate == 0) {
*BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
@@ -197,7 +197,7 @@ PL011UartInitializePort (
Divisor = (UINT32)DivisorValue;
Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
Fractional = Divisor & FRACTION_PART_MASK;
- }
+#endif
//
// If PL011 is already initialized, check the current settings
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 4715 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-08-23 10:55 ` [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning Sami Mujawar
2019-08-23 11:49 ` Alexei Fedorov
@ 2019-11-21 12:35 ` Leif Lindholm
2019-11-21 12:39 ` Ard Biesheuvel
1 sibling, 1 reply; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 12:35 UTC (permalink / raw)
To: Sami Mujawar; +Cc: devel, Alexei.Fedorov, ard.biesheuvel, Matteo.Carlini, nd
On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C6326: potential
> comparison of a constant with another constant' when
> a fixed PCD value is compared with a constant value.
>
> The faulting code is as marked by '-->' below:
>
> --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> Integer = FixedPcdGet32 (PL011UartInteger);
> Fractional = FixedPcdGet32 (PL011UartFractional);
> } else {
> ...
>
> The macro FixedPcdGet32 (PL011UartInteger) evaluates
> to a macro _PCD_VALUE_PL011UartInteger that is defined
> by the build system to represent the UART Integer
> value. Therefore, the VS2017 compiler reports the above
> warning.
>
> Fix this warning by enclosing the code in appropriate
> #if .. #else .. #endif directives.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -174,10 +174,10 @@ PL011UartInitializePort (
> //
>
> // If PL011 Integer value has been defined then always ignore the BAUD rate
> - if (FixedPcdGet32 (PL011UartInteger) != 0) {
> +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> Integer = FixedPcdGet32 (PL011UartInteger);
> Fractional = FixedPcdGet32 (PL011UartFractional);
> - } else {
> +#else
> // If BAUD rate is zero then replace it with the system default value
> if (*BaudRate == 0) {
> *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> @@ -197,7 +197,7 @@ PL011UartInitializePort (
> Divisor = (UINT32)DivisorValue;
> Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> Fractional = Divisor & FRACTION_PART_MASK;
> - }
> +#endif
>
> //
> // If PL011 is already initialized, check the current settings
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-11-21 12:35 ` Leif Lindholm
@ 2019-11-21 12:39 ` Ard Biesheuvel
2019-11-21 12:53 ` Leif Lindholm
0 siblings, 1 reply; 57+ messages in thread
From: Ard Biesheuvel @ 2019-11-21 12:39 UTC (permalink / raw)
To: Leif Lindholm
Cc: Sami Mujawar, edk2-devel-groups-io, Alexei Fedorov,
Matteo Carlini, nd
On Thu, 21 Nov 2019 at 13:35, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > The VS2017 compiler reports 'warning C6326: potential
> > comparison of a constant with another constant' when
> > a fixed PCD value is compared with a constant value.
> >
> > The faulting code is as marked by '-->' below:
> >
> > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > Integer = FixedPcdGet32 (PL011UartInteger);
> > Fractional = FixedPcdGet32 (PL011UartFractional);
> > } else {
> > ...
> >
> > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > by the build system to represent the UART Integer
> > value. Therefore, the VS2017 compiler reports the above
> > warning.
> >
> > Fix this warning by enclosing the code in appropriate
> > #if .. #else .. #endif directives.
> >
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
I don't like this at all.
If two expressions happen to evaluate to the same constant, we should
be able to compare them using C code.
I think we should disable the warning instead.
> > ---
> > ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
> > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> > //
> >
> > // If PL011 Integer value has been defined then always ignore the BAUD rate
> > - if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> > Integer = FixedPcdGet32 (PL011UartInteger);
> > Fractional = FixedPcdGet32 (PL011UartFractional);
> > - } else {
> > +#else
> > // If BAUD rate is zero then replace it with the system default value
> > if (*BaudRate == 0) {
> > *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> > Divisor = (UINT32)DivisorValue;
> > Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> > Fractional = Divisor & FRACTION_PART_MASK;
> > - }
> > +#endif
> >
> > //
> > // If PL011 is already initialized, check the current settings
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-11-21 12:39 ` Ard Biesheuvel
@ 2019-11-21 12:53 ` Leif Lindholm
2019-11-21 13:09 ` Ard Biesheuvel
0 siblings, 1 reply; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 12:53 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sami Mujawar, edk2-devel-groups-io, Alexei Fedorov,
Matteo Carlini, nd
On Thu, Nov 21, 2019 at 13:39:55 +0100, Ard Biesheuvel wrote:
> On Thu, 21 Nov 2019 at 13:35, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > > The VS2017 compiler reports 'warning C6326: potential
> > > comparison of a constant with another constant' when
> > > a fixed PCD value is compared with a constant value.
> > >
> > > The faulting code is as marked by '-->' below:
> > >
> > > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > Integer = FixedPcdGet32 (PL011UartInteger);
> > > Fractional = FixedPcdGet32 (PL011UartFractional);
> > > } else {
> > > ...
> > >
> > > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > > by the build system to represent the UART Integer
> > > value. Therefore, the VS2017 compiler reports the above
> > > warning.
> > >
> > > Fix this warning by enclosing the code in appropriate
> > > #if .. #else .. #endif directives.
> > >
> > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> >
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
>
> I don't like this at all.
>
> If two expressions happen to evaluate to the same constant, we should
> be able to compare them using C code.
>
> I think we should disable the warning instead.
I agree with that as a general rule, and agree that disabling the
warning may be preferable in other contexts.
But I somewhat disagree with referring to *this instance* as two
expressions evaluating to the same constant. This is entirely a
preprocessor event, somewhat obscured by the Pcd syntax.
The test is "has the default baudrate been requested or has a specific
one been requested?".
Now, that might be more cleanly described as a macro than inline like
this - but I think the compiler has usefully warned us about weirdly
written code. So I would really not want to disable the warning on a
global level, and would prefer not to do so here.
/
Leif
>
> > > ---
> > > ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
> > > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> > > //
> > >
> > > // If PL011 Integer value has been defined then always ignore the BAUD rate
> > > - if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> > > Integer = FixedPcdGet32 (PL011UartInteger);
> > > Fractional = FixedPcdGet32 (PL011UartFractional);
> > > - } else {
> > > +#else
> > > // If BAUD rate is zero then replace it with the system default value
> > > if (*BaudRate == 0) {
> > > *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> > > Divisor = (UINT32)DivisorValue;
> > > Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> > > Fractional = Divisor & FRACTION_PART_MASK;
> > > - }
> > > +#endif
> > >
> > > //
> > > // If PL011 is already initialized, check the current settings
> > > --
> > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > >
> > >
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-11-21 12:53 ` Leif Lindholm
@ 2019-11-21 13:09 ` Ard Biesheuvel
2019-11-21 15:15 ` Leif Lindholm
0 siblings, 1 reply; 57+ messages in thread
From: Ard Biesheuvel @ 2019-11-21 13:09 UTC (permalink / raw)
To: Leif Lindholm
Cc: Sami Mujawar, edk2-devel-groups-io, Alexei Fedorov,
Matteo Carlini, nd
On Thu, 21 Nov 2019 at 13:53, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Nov 21, 2019 at 13:39:55 +0100, Ard Biesheuvel wrote:
> > On Thu, 21 Nov 2019 at 13:35, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Fri, Aug 23, 2019 at 11:55:38 +0100, Sami Mujawar wrote:
> > > > The VS2017 compiler reports 'warning C6326: potential
> > > > comparison of a constant with another constant' when
> > > > a fixed PCD value is compared with a constant value.
> > > >
> > > > The faulting code is as marked by '-->' below:
> > > >
> > > > --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > > Integer = FixedPcdGet32 (PL011UartInteger);
> > > > Fractional = FixedPcdGet32 (PL011UartFractional);
> > > > } else {
> > > > ...
> > > >
> > > > The macro FixedPcdGet32 (PL011UartInteger) evaluates
> > > > to a macro _PCD_VALUE_PL011UartInteger that is defined
> > > > by the build system to represent the UART Integer
> > > > value. Therefore, the VS2017 compiler reports the above
> > > > warning.
> > > >
> > > > Fix this warning by enclosing the code in appropriate
> > > > #if .. #else .. #endif directives.
> > > >
> > > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > >
> > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > >
> >
> > I don't like this at all.
> >
> > If two expressions happen to evaluate to the same constant, we should
> > be able to compare them using C code.
> >
> > I think we should disable the warning instead.
>
> I agree with that as a general rule, and agree that disabling the
> warning may be preferable in other contexts.
>
> But I somewhat disagree with referring to *this instance* as two
> expressions evaluating to the same constant. This is entirely a
> preprocessor event, somewhat obscured by the Pcd syntax.
> The test is "has the default baudrate been requested or has a specific
> one been requested?".
>
That may be true, but moving from C conditionals to CPP conditionals
obscures the code, and reduces test coverage, so I think it should be
avoided when possible.
> Now, that might be more cleanly described as a macro than inline like
> this - but I think the compiler has usefully warned us about weirdly
> written code. So I would really not want to disable the warning on a
> global level, and would prefer not to do so here.
>
> /
> Leif
>
> >
> > > > ---
> > > > ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > index 2d3c279cce49304959953ec4a34b50e09a7d0045..dabf099b9bc82e1fb1bd5a2eae3fa4b5878a9e07 100644
> > > > --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> > > > @@ -174,10 +174,10 @@ PL011UartInitializePort (
> > > > //
> > > >
> > > > // If PL011 Integer value has been defined then always ignore the BAUD rate
> > > > - if (FixedPcdGet32 (PL011UartInteger) != 0) {
> > > > +#if (FixedPcdGet32 (PL011UartInteger) != 0)
> > > > Integer = FixedPcdGet32 (PL011UartInteger);
> > > > Fractional = FixedPcdGet32 (PL011UartFractional);
> > > > - } else {
> > > > +#else
> > > > // If BAUD rate is zero then replace it with the system default value
> > > > if (*BaudRate == 0) {
> > > > *BaudRate = FixedPcdGet32 (PcdSerialBaudRate);
> > > > @@ -197,7 +197,7 @@ PL011UartInitializePort (
> > > > Divisor = (UINT32)DivisorValue;
> > > > Integer = Divisor >> FRACTION_PART_SIZE_IN_BITS;
> > > > Fractional = Divisor & FRACTION_PART_MASK;
> > > > - }
> > > > +#endif
> > > >
> > > > //
> > > > // If PL011 is already initialized, check the current settings
> > > > --
> > > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> > > >
> > > >
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-11-21 13:09 ` Ard Biesheuvel
@ 2019-11-21 15:15 ` Leif Lindholm
2019-11-21 15:16 ` Ard Biesheuvel
0 siblings, 1 reply; 57+ messages in thread
From: Leif Lindholm @ 2019-11-21 15:15 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sami Mujawar, edk2-devel-groups-io, Alexei Fedorov,
Matteo Carlini, nd
On Thu, Nov 21, 2019 at 14:09:41 +0100, Ard Biesheuvel wrote:
> > > I don't like this at all.
> > >
> > > If two expressions happen to evaluate to the same constant, we should
> > > be able to compare them using C code.
> > >
> > > I think we should disable the warning instead.
> >
> > I agree with that as a general rule, and agree that disabling the
> > warning may be preferable in other contexts.
> >
> > But I somewhat disagree with referring to *this instance* as two
> > expressions evaluating to the same constant. This is entirely a
> > preprocessor event, somewhat obscured by the Pcd syntax.
> > The test is "has the default baudrate been requested or has a specific
> > one been requested?".
>
> That may be true, but moving from C conditionals to CPP conditionals
> obscures the code, and reduces test coverage, so I think it should be
> avoided when possible.
I don't see what test coverage this would change. Even in the NOOPT
case, only the codegen would be affected - only one of the two cases
would be reachable. In all other cases, the compiler would go
if (115200 != 0) {
/* Oh, I'll just ignore any else clauses then ... */
I think the pre-existing code obscures what is actually going on and
that the change clarifies it (but could be prettified).
Now, of course, why we have a platform-specific (and platform-global)
override variable to make a library ignore its inputs is another
interesting question.
So. Rock-paper-scissors or coin-toss?
/
Leif
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning
2019-11-21 15:15 ` Leif Lindholm
@ 2019-11-21 15:16 ` Ard Biesheuvel
0 siblings, 0 replies; 57+ messages in thread
From: Ard Biesheuvel @ 2019-11-21 15:16 UTC (permalink / raw)
To: Leif Lindholm
Cc: Sami Mujawar, edk2-devel-groups-io, Alexei Fedorov,
Matteo Carlini, nd
On Thu, 21 Nov 2019 at 16:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Nov 21, 2019 at 14:09:41 +0100, Ard Biesheuvel wrote:
> > > > I don't like this at all.
> > > >
> > > > If two expressions happen to evaluate to the same constant, we should
> > > > be able to compare them using C code.
> > > >
> > > > I think we should disable the warning instead.
> > >
> > > I agree with that as a general rule, and agree that disabling the
> > > warning may be preferable in other contexts.
> > >
> > > But I somewhat disagree with referring to *this instance* as two
> > > expressions evaluating to the same constant. This is entirely a
> > > preprocessor event, somewhat obscured by the Pcd syntax.
> > > The test is "has the default baudrate been requested or has a specific
> > > one been requested?".
> >
> > That may be true, but moving from C conditionals to CPP conditionals
> > obscures the code, and reduces test coverage, so I think it should be
> > avoided when possible.
>
> I don't see what test coverage this would change. Even in the NOOPT
> case, only the codegen would be affected - only one of the two cases
> would be reachable. In all other cases, the compiler would go
> if (115200 != 0) {
> /* Oh, I'll just ignore any else clauses then ... */
>
> I think the pre-existing code obscures what is actually going on and
> that the change clarifies it (but could be prettified).
>
> Now, of course, why we have a platform-specific (and platform-global)
> override variable to make a library ignore its inputs is another
> interesting question.
>
> So. Rock-paper-scissors or coin-toss?
>
Yes!
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use
2019-08-23 10:55 [PATCH v1 00/19] Fix warnings reported by VS2017 compiler Sami Mujawar
` (17 preceding siblings ...)
2019-08-23 10:55 ` [PATCH v1 18/19] ArmPlatformPkg: Fix comparison of constants warning Sami Mujawar
@ 2019-08-23 10:55 ` Sami Mujawar
2019-08-23 11:48 ` Alexei Fedorov
2019-08-23 13:50 ` Liming Gao
18 siblings, 2 replies; 57+ messages in thread
From: Sami Mujawar @ 2019-08-23 10:55 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, Alexei.Fedorov, ard.biesheuvel, leif.lindholm,
Matteo.Carlini, michael.d.kinney, liming.gao, nd
The VS2017 compiler reports 'warning C6001: Using
uninitialized memory 'Marker'.' for VA_LIST
variables.
To fix this issue declare a VA_LIST global variable
and use this to initialise VA_LIST variables before
use.
Note: The VA_LIST cannot be assigned a NULL value
because some compilers define VA_LIST to be a
structure.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdePkg/Library/BaseLib/SwitchStack.c | 9 +++++++++
MdePkg/Library/BasePrintLib/PrintLib.c | 5 +++++
MdePkg/Library/BasePrintLib/PrintLibInternal.c | 9 +++++++++
3 files changed, 23 insertions(+)
diff --git a/MdePkg/Library/BaseLib/SwitchStack.c b/MdePkg/Library/BaseLib/SwitchStack.c
index cb9f69f1eaceba690b48e9ca6b8a9af2e348bddd..e1bb524819b3de3521c5461ce681aa3a6c186f2c 100644
--- a/MdePkg/Library/BaseLib/SwitchStack.c
+++ b/MdePkg/Library/BaseLib/SwitchStack.c
@@ -2,12 +2,20 @@
Switch Stack functions.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "BaseLibInternals.h"
+//
+// Declare a VA_LIST global variable that is used to initialise VA_LIST
+// variables before use. The VA_LIST cannot be assigned a NULL value
+// because some compilers define VA_LIST to be a structure.
+//
+STATIC VA_LIST gNullVaList;
+
/**
Transfers control to a function starting with a new stack.
@@ -57,6 +65,7 @@ SwitchStack (
//
ASSERT (((UINTN)NewStack & (CPU_STACK_ALIGNMENT - 1)) == 0);
+ Marker = gNullVaList;
VA_START (Marker, NewStack);
InternalSwitchStack (EntryPoint, Context1, Context2, NewStack, Marker);
diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
index af771652e4b0aebd616973ba1089ae5bc2b6f0c0..67c5f3dd547cea5447075ef88d697879883ba5ab 100644
--- a/MdePkg/Library/BasePrintLib/PrintLib.c
+++ b/MdePkg/Library/BasePrintLib/PrintLib.c
@@ -3,6 +3,7 @@
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -177,6 +178,7 @@ UnicodeSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = UnicodeVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -337,6 +339,7 @@ UnicodeSPrintAsciiFormat (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = UnicodeVSPrintAsciiFormat (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -614,6 +617,7 @@ AsciiSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = AsciiVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -774,6 +778,7 @@ AsciiSPrintUnicodeFormat (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = AsciiVSPrintUnicodeFormat (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index b6ec5ac4fbb98982f8ccaf3908c2a91ce583e31e..11392f2a5d12eb059611c3ff77b27b602f9b9a40 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -2,12 +2,20 @@
Print Library internal worker functions.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "PrintLibInternal.h"
+//
+// Declare a VA_LIST global variable that is used to initialise VA_LIST
+// variables before use. The VA_LIST cannot be assigned a NULL value
+// because some compilers define VA_LIST to be a structure.
+//
+extern VA_LIST gNullVaList;
+
#define WARNING_STATUS_NUMBER 5
#define ERROR_STATUS_NUMBER 33
@@ -1256,6 +1264,7 @@ BasePrintLibSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = BasePrintLibSPrintMarker (StartOfBuffer, BufferSize, Flags, FormatString, Marker, NULL);
VA_END (Marker);
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use
2019-08-23 10:55 ` [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use Sami Mujawar
@ 2019-08-23 11:48 ` Alexei Fedorov
2019-08-23 13:50 ` Liming Gao
1 sibling, 0 replies; 57+ messages in thread
From: Alexei Fedorov @ 2019-08-23 11:48 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 5642 bytes --]
Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>
Alexei
________________________________
From: Sami Mujawar <sami.mujawar@arm.com>
Sent: 23 August 2019 11:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; ard.biesheuvel@linaro.org <ard.biesheuvel@linaro.org>; leif.lindholm@linaro.org <leif.lindholm@linaro.org>; Matteo Carlini <Matteo.Carlini@arm.com>; michael.d.kinney@intel.com <michael.d.kinney@intel.com>; liming.gao@intel.com <liming.gao@intel.com>; nd <nd@arm.com>
Subject: [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use
The VS2017 compiler reports 'warning C6001: Using
uninitialized memory 'Marker'.' for VA_LIST
variables.
To fix this issue declare a VA_LIST global variable
and use this to initialise VA_LIST variables before
use.
Note: The VA_LIST cannot be assigned a NULL value
because some compilers define VA_LIST to be a
structure.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
MdePkg/Library/BaseLib/SwitchStack.c | 9 +++++++++
MdePkg/Library/BasePrintLib/PrintLib.c | 5 +++++
MdePkg/Library/BasePrintLib/PrintLibInternal.c | 9 +++++++++
3 files changed, 23 insertions(+)
diff --git a/MdePkg/Library/BaseLib/SwitchStack.c b/MdePkg/Library/BaseLib/SwitchStack.c
index cb9f69f1eaceba690b48e9ca6b8a9af2e348bddd..e1bb524819b3de3521c5461ce681aa3a6c186f2c 100644
--- a/MdePkg/Library/BaseLib/SwitchStack.c
+++ b/MdePkg/Library/BaseLib/SwitchStack.c
@@ -2,12 +2,20 @@
Switch Stack functions.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "BaseLibInternals.h"
+//
+// Declare a VA_LIST global variable that is used to initialise VA_LIST
+// variables before use. The VA_LIST cannot be assigned a NULL value
+// because some compilers define VA_LIST to be a structure.
+//
+STATIC VA_LIST gNullVaList;
+
/**
Transfers control to a function starting with a new stack.
@@ -57,6 +65,7 @@ SwitchStack (
//
ASSERT (((UINTN)NewStack & (CPU_STACK_ALIGNMENT - 1)) == 0);
+ Marker = gNullVaList;
VA_START (Marker, NewStack);
InternalSwitchStack (EntryPoint, Context1, Context2, NewStack, Marker);
diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
index af771652e4b0aebd616973ba1089ae5bc2b6f0c0..67c5f3dd547cea5447075ef88d697879883ba5ab 100644
--- a/MdePkg/Library/BasePrintLib/PrintLib.c
+++ b/MdePkg/Library/BasePrintLib/PrintLib.c
@@ -3,6 +3,7 @@
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -177,6 +178,7 @@ UnicodeSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = UnicodeVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -337,6 +339,7 @@ UnicodeSPrintAsciiFormat (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = UnicodeVSPrintAsciiFormat (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -614,6 +617,7 @@ AsciiSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = AsciiVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
@@ -774,6 +778,7 @@ AsciiSPrintUnicodeFormat (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = AsciiVSPrintUnicodeFormat (StartOfBuffer, BufferSize, FormatString, Marker);
VA_END (Marker);
diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index b6ec5ac4fbb98982f8ccaf3908c2a91ce583e31e..11392f2a5d12eb059611c3ff77b27b602f9b9a40 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -2,12 +2,20 @@
Print Library internal worker functions.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "PrintLibInternal.h"
+//
+// Declare a VA_LIST global variable that is used to initialise VA_LIST
+// variables before use. The VA_LIST cannot be assigned a NULL value
+// because some compilers define VA_LIST to be a structure.
+//
+extern VA_LIST gNullVaList;
+
#define WARNING_STATUS_NUMBER 5
#define ERROR_STATUS_NUMBER 33
@@ -1256,6 +1264,7 @@ BasePrintLibSPrint (
VA_LIST Marker;
UINTN NumberOfPrinted;
+ Marker = gNullVaList;
VA_START (Marker, FormatString);
NumberOfPrinted = BasePrintLibSPrintMarker (StartOfBuffer, BufferSize, Flags, FormatString, Marker, NULL);
VA_END (Marker);
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
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.
[-- Attachment #2: Type: text/html, Size: 8504 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use
2019-08-23 10:55 ` [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use Sami Mujawar
2019-08-23 11:48 ` Alexei Fedorov
@ 2019-08-23 13:50 ` Liming Gao
1 sibling, 0 replies; 57+ messages in thread
From: Liming Gao @ 2019-08-23 13:50 UTC (permalink / raw)
To: Sami Mujawar, devel@edk2.groups.io
Cc: Alexei.Fedorov@arm.com, ard.biesheuvel@linaro.org,
leif.lindholm@linaro.org, Matteo.Carlini@arm.com,
Kinney, Michael D, nd@arm.com
Is this warning reported on ARM arch?
In Base.h, VA_START is defined as below for ARM arch. Do you mean VS2017 report the warning for below macro?
If so, can you propose the change in VA_START macro to fix this warning, then doesn't need to update consumer source code.
#define VA_START(Marker, Parameter) __va_start (&Marker, &Parameter, _INT_SIZE_OF (Parameter), __alignof(Parameter), &Parameter)
> -----Original Message-----
> From: Sami Mujawar [mailto:sami.mujawar@arm.com]
> Sent: Friday, August 23, 2019 6:56 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Alexei.Fedorov@arm.com; ard.biesheuvel@linaro.org; leif.lindholm@linaro.org;
> Matteo.Carlini@arm.com; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; nd@arm.com
> Subject: [PATCH v1 19/19] MdePkg: Initialise VA_LIST variables before use
>
> The VS2017 compiler reports 'warning C6001: Using
> uninitialized memory 'Marker'.' for VA_LIST
> variables.
>
> To fix this issue declare a VA_LIST global variable
> and use this to initialise VA_LIST variables before
> use.
>
> Note: The VA_LIST cannot be assigned a NULL value
> because some compilers define VA_LIST to be a
> structure.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> MdePkg/Library/BaseLib/SwitchStack.c | 9 +++++++++
> MdePkg/Library/BasePrintLib/PrintLib.c | 5 +++++
> MdePkg/Library/BasePrintLib/PrintLibInternal.c | 9 +++++++++
> 3 files changed, 23 insertions(+)
>
> diff --git a/MdePkg/Library/BaseLib/SwitchStack.c b/MdePkg/Library/BaseLib/SwitchStack.c
> index cb9f69f1eaceba690b48e9ca6b8a9af2e348bddd..e1bb524819b3de3521c5461ce681aa3a6c186f2c 100644
> --- a/MdePkg/Library/BaseLib/SwitchStack.c
> +++ b/MdePkg/Library/BaseLib/SwitchStack.c
> @@ -2,12 +2,20 @@
> Switch Stack functions.
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include "BaseLibInternals.h"
>
> +//
> +// Declare a VA_LIST global variable that is used to initialise VA_LIST
> +// variables before use. The VA_LIST cannot be assigned a NULL value
> +// because some compilers define VA_LIST to be a structure.
> +//
> +STATIC VA_LIST gNullVaList;
> +
> /**
> Transfers control to a function starting with a new stack.
>
> @@ -57,6 +65,7 @@ SwitchStack (
> //
> ASSERT (((UINTN)NewStack & (CPU_STACK_ALIGNMENT - 1)) == 0);
>
> + Marker = gNullVaList;
> VA_START (Marker, NewStack);
>
> InternalSwitchStack (EntryPoint, Context1, Context2, NewStack, Marker);
> diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
> index af771652e4b0aebd616973ba1089ae5bc2b6f0c0..67c5f3dd547cea5447075ef88d697879883ba5ab 100644
> --- a/MdePkg/Library/BasePrintLib/PrintLib.c
> +++ b/MdePkg/Library/BasePrintLib/PrintLib.c
> @@ -3,6 +3,7 @@
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> + Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -177,6 +178,7 @@ UnicodeSPrint (
> VA_LIST Marker;
> UINTN NumberOfPrinted;
>
> + Marker = gNullVaList;
> VA_START (Marker, FormatString);
> NumberOfPrinted = UnicodeVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
> VA_END (Marker);
> @@ -337,6 +339,7 @@ UnicodeSPrintAsciiFormat (
> VA_LIST Marker;
> UINTN NumberOfPrinted;
>
> + Marker = gNullVaList;
> VA_START (Marker, FormatString);
> NumberOfPrinted = UnicodeVSPrintAsciiFormat (StartOfBuffer, BufferSize, FormatString, Marker);
> VA_END (Marker);
> @@ -614,6 +617,7 @@ AsciiSPrint (
> VA_LIST Marker;
> UINTN NumberOfPrinted;
>
> + Marker = gNullVaList;
> VA_START (Marker, FormatString);
> NumberOfPrinted = AsciiVSPrint (StartOfBuffer, BufferSize, FormatString, Marker);
> VA_END (Marker);
> @@ -774,6 +778,7 @@ AsciiSPrintUnicodeFormat (
> VA_LIST Marker;
> UINTN NumberOfPrinted;
>
> + Marker = gNullVaList;
> VA_START (Marker, FormatString);
> NumberOfPrinted = AsciiVSPrintUnicodeFormat (StartOfBuffer, BufferSize, FormatString, Marker);
> VA_END (Marker);
> diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> index b6ec5ac4fbb98982f8ccaf3908c2a91ce583e31e..11392f2a5d12eb059611c3ff77b27b602f9b9a40 100644
> --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> @@ -2,12 +2,20 @@
> Print Library internal worker functions.
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2019, ARM Ltd. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include "PrintLibInternal.h"
>
> +//
> +// Declare a VA_LIST global variable that is used to initialise VA_LIST
> +// variables before use. The VA_LIST cannot be assigned a NULL value
> +// because some compilers define VA_LIST to be a structure.
> +//
> +extern VA_LIST gNullVaList;
> +
> #define WARNING_STATUS_NUMBER 5
> #define ERROR_STATUS_NUMBER 33
>
> @@ -1256,6 +1264,7 @@ BasePrintLibSPrint (
> VA_LIST Marker;
> UINTN NumberOfPrinted;
>
> + Marker = gNullVaList;
> VA_START (Marker, FormatString);
> NumberOfPrinted = BasePrintLibSPrintMarker (StartOfBuffer, BufferSize, Flags, FormatString, Marker, NULL);
> VA_END (Marker);
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
^ permalink raw reply [flat|nested] 57+ messages in thread