* [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
@ 2021-01-11 10:57 Ard Biesheuvel
2021-01-11 15:54 ` [edk2-devel] " Laszlo Ersek
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2021-01-11 10:57 UTC (permalink / raw)
To: devel
Cc: leif, Ard Biesheuvel, Vijayenthiran Subramaniam, Masahisa Kojima,
Sami Mujawar
Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
64-bit address space") updated the NorFlash DXE and StMM drivers to
take alternate PCDs into account when discovering the base of the
NOR flash regions.
This introduced a disparity between the declarations of the PCD references
in the .INF files, which permits the use of dynamic PCDs, and the code
itself, which now uses FixedPcdGet() accessors. On platforms that actually
use dynamic PCDs, this results in a build error.
So let's clean this up:
- for the DXE version, use the generic PcdGet() accessors, so dynamic PCDs
are permitted
- for the standalone MM version, redeclare the PCDs as [FixedPcd] in the
.INF description, and switch to the FixedPcdGet() accessors.
Cc: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf | 3 ++-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 4 ++--
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 10 +++++-----
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
index b2f72fb4de20..56347a8bc7c1 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
@@ -49,7 +49,7 @@ [Guids]
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid
-[Pcd.common]
+[FixedPcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
@@ -60,6 +60,7 @@ [Pcd.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+[FeaturePcd]
gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
[Depex]
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 28dc8e125c78..f412731200cf 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -422,8 +422,8 @@ NorFlashFvbInitialize (
EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
ASSERT_EFI_ERROR (Status);
- mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
- FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
+ mFlashNvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
+ PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
// Set the index of the first LBA for the FVB
Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
index 8a4fb395d286..4ebbc06e1de3 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
@@ -299,15 +299,15 @@ NorFlashInitialise (
for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
// Check if this NOR Flash device contain the variable storage region
- if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+ if (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
ContainVariableStorage =
- (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
- (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+ (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet64 (PcdFlashNvStorageVariableBase64)) &&
+ (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
} else {
ContainVariableStorage =
- (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
- (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+ (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet32 (PcdFlashNvStorageVariableBase)) &&
+ (FixedPcdGet32 (PcdFlashNvStorageVariableBase) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
2021-01-11 10:57 [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors Ard Biesheuvel
@ 2021-01-11 15:54 ` Laszlo Ersek
2021-01-12 11:13 ` Vijayenthiran Subramaniam
2021-01-12 11:42 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2021-01-11 15:54 UTC (permalink / raw)
To: devel, ard.biesheuvel
Cc: leif, Vijayenthiran Subramaniam, Masahisa Kojima, Sami Mujawar
On 01/11/21 11:57, Ard Biesheuvel wrote:
> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
> 64-bit address space") updated the NorFlash DXE and StMM drivers to
> take alternate PCDs into account when discovering the base of the
> NOR flash regions.
>
> This introduced a disparity between the declarations of the PCD references
> in the .INF files, which permits the use of dynamic PCDs, and the code
> itself, which now uses FixedPcdGet() accessors. On platforms that actually
> use dynamic PCDs, this results in a build error.
>
> So let's clean this up:
> - for the DXE version, use the generic PcdGet() accessors, so dynamic PCDs
> are permitted
> - for the standalone MM version, redeclare the PCDs as [FixedPcd] in the
> .INF description, and switch to the FixedPcdGet() accessors.
>
> Cc: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf | 3 ++-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 4 ++--
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 10 +++++-----
> 3 files changed, 9 insertions(+), 8 deletions(-)
Looks justified to me.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks,
Laszlo
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> index b2f72fb4de20..56347a8bc7c1 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> @@ -49,7 +49,7 @@ [Guids]
> [Protocols]
> gEfiSmmFirmwareVolumeBlockProtocolGuid
>
> -[Pcd.common]
> +[FixedPcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> @@ -60,6 +60,7 @@ [Pcd.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
> +[FeaturePcd]
> gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
>
> [Depex]
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 28dc8e125c78..f412731200cf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -422,8 +422,8 @@ NorFlashFvbInitialize (
> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> ASSERT_EFI_ERROR (Status);
>
> - mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> - FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
> + mFlashNvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
>
> // Set the index of the first LBA for the FVB
> Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> index 8a4fb395d286..4ebbc06e1de3 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> @@ -299,15 +299,15 @@ NorFlashInitialise (
> for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
> // Check if this NOR Flash device contain the variable storage region
>
> - if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> + if (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> - (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> + (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> } else {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet32 (PcdFlashNvStorageVariableBase)) &&
> + (FixedPcdGet32 (PcdFlashNvStorageVariableBase) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
2021-01-11 10:57 [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors Ard Biesheuvel
2021-01-11 15:54 ` [edk2-devel] " Laszlo Ersek
@ 2021-01-12 11:13 ` Vijayenthiran Subramaniam
2021-01-12 11:42 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-12 11:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: leif, devel, Vijayenthiran Subramaniam, Masahisa Kojima,
Sami Mujawar, Thomas Abraham
On Mon, Jan 11, 2021 at 10:57 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
>
> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
> 64-bit address space") updated the NorFlash DXE and StMM drivers to
> take alternate PCDs into account when discovering the base of the
> NOR flash regions.
>
> This introduced a disparity between the declarations of the PCD references
> in the .INF files, which permits the use of dynamic PCDs, and the code
> itself, which now uses FixedPcdGet() accessors. On platforms that actually
> use dynamic PCDs, this results in a build error.
>
> So let's clean this up:
> - for the DXE version, use the generic PcdGet() accessors, so dynamic PCDs
> are permitted
> - for the standalone MM version, redeclare the PCDs as [FixedPcd] in the
> .INF description, and switch to the FixedPcdGet() accessors.
>
> Cc: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
I've tested this change on Arm Reference Design platforms (SgiPkg).
Tested-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
Also, can you let us know which platform (that used dynamic PCD) broke the
build?
Thanks,
Vijay
> ---
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf | 3 ++-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 4 ++--
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 10 +++++-----
> 3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> index b2f72fb4de20..56347a8bc7c1 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> @@ -49,7 +49,7 @@ [Guids]
> [Protocols]
> gEfiSmmFirmwareVolumeBlockProtocolGuid
>
> -[Pcd.common]
> +[FixedPcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> @@ -60,6 +60,7 @@ [Pcd.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
> +[FeaturePcd]
> gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
>
> [Depex]
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 28dc8e125c78..f412731200cf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -422,8 +422,8 @@ NorFlashFvbInitialize (
> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> ASSERT_EFI_ERROR (Status);
>
> - mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> - FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
> + mFlashNvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
>
> // Set the index of the first LBA for the FVB
> Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> index 8a4fb395d286..4ebbc06e1de3 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> @@ -299,15 +299,15 @@ NorFlashInitialise (
> for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
> // Check if this NOR Flash device contain the variable storage region
>
> - if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> + if (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> - (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> + (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> } else {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet32 (PcdFlashNvStorageVariableBase)) &&
> + (FixedPcdGet32 (PcdFlashNvStorageVariableBase) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> }
>
> --
> 2.17.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
2021-01-11 10:57 [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors Ard Biesheuvel
2021-01-11 15:54 ` [edk2-devel] " Laszlo Ersek
2021-01-12 11:13 ` Vijayenthiran Subramaniam
@ 2021-01-12 11:42 ` Philippe Mathieu-Daudé
2021-01-12 12:10 ` Ard Biesheuvel
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-12 11:42 UTC (permalink / raw)
To: devel, ard.biesheuvel
Cc: leif, Vijayenthiran Subramaniam, Masahisa Kojima, Sami Mujawar,
Michael D Kinney
Hi Ard,
On 1/11/21 11:57 AM, Ard Biesheuvel wrote:
> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
> 64-bit address space") updated the NorFlash DXE and StMM drivers to
> take alternate PCDs into account when discovering the base of the
> NOR flash regions.
>
> This introduced a disparity between the declarations of the PCD references
> in the .INF files, which permits the use of dynamic PCDs, and the code
> itself, which now uses FixedPcdGet() accessors. On platforms that actually
> use dynamic PCDs, this results in a build error.
So there is no (mainstream) CI coverage for these platforms?
Could we add at least one?
Thanks,
Phil.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
2021-01-12 11:42 ` Philippe Mathieu-Daudé
@ 2021-01-12 12:10 ` Ard Biesheuvel
2021-01-12 12:20 ` Sami Mujawar
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2021-01-12 12:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, devel
Cc: leif, Vijayenthiran Subramaniam, Masahisa Kojima, Sami Mujawar,
Michael D Kinney
On 1/12/21 12:42 PM, Philippe Mathieu-Daudé wrote:
> Hi Ard,
>
> On 1/11/21 11:57 AM, Ard Biesheuvel wrote:
>> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
>> 64-bit address space") updated the NorFlash DXE and StMM drivers to
>> take alternate PCDs into account when discovering the base of the
>> NOR flash regions.
>>
>> This introduced a disparity between the declarations of the PCD references
>> in the .INF files, which permits the use of dynamic PCDs, and the code
>> itself, which now uses FixedPcdGet() accessors. On platforms that actually
>> use dynamic PCDs, this results in a build error.
>
> So there is no (mainstream) CI coverage for these platforms?
> Could we add at least one?
>
We could. It was KvmTool.dsc, which lives in the main repo.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
2021-01-12 12:10 ` Ard Biesheuvel
@ 2021-01-12 12:20 ` Sami Mujawar
0 siblings, 0 replies; 6+ messages in thread
From: Sami Mujawar @ 2021-01-12 12:20 UTC (permalink / raw)
To: Ard Biesheuvel, Philippe Mathieu-Daudé, devel@edk2.groups.io
Cc: leif@nuviainc.com, Vijayenthiran Subramaniam, Masahisa Kojima,
Michael D Kinney
Hi Ard, Philippe,
I will check if the Kvmtool.dsc build can be supported using the edk2 Core CI.
Regards,
Sami Mujawar
-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@arm.com>
Sent: 12 January 2021 12:10 PM
To: Philippe Mathieu-Daudé <philmd@redhat.com>; devel@edk2.groups.io
Cc: leif@nuviainc.com; Vijayenthiran Subramaniam <Vijayenthiran.Subramaniam@arm.com>; Masahisa Kojima <masahisa.kojima@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
On 1/12/21 12:42 PM, Philippe Mathieu-Daudé wrote:
> Hi Ard,
>
> On 1/11/21 11:57 AM, Ard Biesheuvel wrote:
>> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
>> 64-bit address space") updated the NorFlash DXE and StMM drivers to
>> take alternate PCDs into account when discovering the base of the
>> NOR flash regions.
>>
>> This introduced a disparity between the declarations of the PCD references
>> in the .INF files, which permits the use of dynamic PCDs, and the code
>> itself, which now uses FixedPcdGet() accessors. On platforms that actually
>> use dynamic PCDs, this results in a build error.
>
> So there is no (mainstream) CI coverage for these platforms?
> Could we add at least one?
>
We could. It was KvmTool.dsc, which lives in the main repo.
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-12 12:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-11 10:57 [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors Ard Biesheuvel
2021-01-11 15:54 ` [edk2-devel] " Laszlo Ersek
2021-01-12 11:13 ` Vijayenthiran Subramaniam
2021-01-12 11:42 ` Philippe Mathieu-Daudé
2021-01-12 12:10 ` Ard Biesheuvel
2021-01-12 12:20 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox