diff mbox series

[Linaro-uefi,2/6] Platforms/AMD/Overdrive: add dynamic PCD to control SMMU availibility

Message ID 20170601094353.16235-3-ard.biesheuvel@linaro.org
State Accepted
Commit 1fabc8818669fc7147b892d991a6bbbc531d7045
Headers show
Series OpenPlatformPkg/AMD updates | expand

Commit Message

Ard Biesheuvel June 1, 2017, 9:43 a.m. UTC 
Introduce a PCD that can be linked to a EFI variable 'StyxEnableSmmus',
controlling whether the SMMU descriptions will be exposed to the OS via
the DT or ACPI tables.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platforms/AMD/Styx/AmdStyx.dec                       | 7 +++++++
 Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Leif Lindholm June 1, 2017, 1:36 p.m. UTC | #1 
On Thu, Jun 01, 2017 at 09:43:49AM +0000, Ard Biesheuvel wrote:
> Introduce a PCD that can be linked to a EFI variable 'StyxEnableSmmus',
> controlling whether the SMMU descriptions will be exposed to the OS via
> the DT or ACPI tables.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platforms/AMD/Styx/AmdStyx.dec                       | 7 +++++++
>  Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 4 +++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Platforms/AMD/Styx/AmdStyx.dec b/Platforms/AMD/Styx/AmdStyx.dec
> index 8d4b4927c6b1..ddd5bf4c3609 100644
> --- a/Platforms/AMD/Styx/AmdStyx.dec
> +++ b/Platforms/AMD/Styx/AmdStyx.dec
> @@ -35,6 +35,10 @@
>    gAmdStyxTokenSpaceGuid     = { 0x220d9653, 0x4a0e, 0x40bc, { 0xb3, 0x65, 0x2f, 0xbb, 0xa2, 0xd9, 0x03, 0x45 } }
>    gAmdStyxMpCoreInfoGuid     = { 0x68efeabd, 0xcb77, 0x4aa5, { 0xbf, 0x0c, 0xa3, 0x31, 0xfc, 0xcf, 0x76, 0x66 } }
>  
> +  # 2a5e4deb-4445-4fb6-8b14-366b8e779b69
> +  # EFI variable scope for Styx
> +  gAmdStyxVariableGuid       = { 0x2a5e4deb, 0x4445, 0x4fb6, { 0x8b, 0x14, 0x36, 0x6b, 0x8e, 0x77, 0x9b, 0x69 } }
> +

This is strictly a question of style (by which I don't mean strict
coding style), but is this new GUID necessary?
I mean, all it needs to be is a GUID to distinguish component
... namespaces(?) ... I see both variants in edk2: have a special
VariableGuid, or just reuse the TokenSpaceGuid.

Just trying to gauge preferences.

I'm happy with the patch either way:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


>  [PcdsDynamic]
>    gAmdStyxTokenSpaceGuid.PcdSocCoreCount|1|UINT32|0x00000100
>    gAmdStyxTokenSpaceGuid.PcdSocCpuId|1|UINT32|0x00000101
> @@ -108,3 +112,6 @@
>    gAmdStyxTokenSpaceGuid.PcdFlashNvStorageOriginalBase|0|UINT64|0x000c0000
>    # block size to use when invoking the ISCP FV methods
>    gAmdStyxTokenSpaceGuid.PcdFlashNvStorageBlockSize|0x1000|UINT32|0x000c0001
> +
> +[PcdsFixedAtBuild,PcdsDynamic]
> +  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|FALSE|BOOLEAN|0xe0000000
> diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
> index 65d229884aa7..b4893ca34587 100644
> --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
> +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
> @@ -515,9 +515,11 @@ DEFINE DO_KCS       = 1
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0
>  
> -[PcdsDynamicExHii.common.DEFAULT]
> +[PcdsDynamicHii]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
>  
> +  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|L"StyxEnableSmmus"|gAmdStyxVariableGuid|0x0|FALSE
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> -- 
> 2.9.3
>
Ard Biesheuvel June 2, 2017, 12:02 p.m. UTC | #2 
On 1 June 2017 at 13:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Jun 01, 2017 at 09:43:49AM +0000, Ard Biesheuvel wrote:
>> Introduce a PCD that can be linked to a EFI variable 'StyxEnableSmmus',
>> controlling whether the SMMU descriptions will be exposed to the OS via
>> the DT or ACPI tables.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Platforms/AMD/Styx/AmdStyx.dec                       | 7 +++++++
>>  Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 4 +++-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platforms/AMD/Styx/AmdStyx.dec b/Platforms/AMD/Styx/AmdStyx.dec
>> index 8d4b4927c6b1..ddd5bf4c3609 100644
>> --- a/Platforms/AMD/Styx/AmdStyx.dec
>> +++ b/Platforms/AMD/Styx/AmdStyx.dec
>> @@ -35,6 +35,10 @@
>>    gAmdStyxTokenSpaceGuid     = { 0x220d9653, 0x4a0e, 0x40bc, { 0xb3, 0x65, 0x2f, 0xbb, 0xa2, 0xd9, 0x03, 0x45 } }
>>    gAmdStyxMpCoreInfoGuid     = { 0x68efeabd, 0xcb77, 0x4aa5, { 0xbf, 0x0c, 0xa3, 0x31, 0xfc, 0xcf, 0x76, 0x66 } }
>>
>> +  # 2a5e4deb-4445-4fb6-8b14-366b8e779b69
>> +  # EFI variable scope for Styx
>> +  gAmdStyxVariableGuid       = { 0x2a5e4deb, 0x4445, 0x4fb6, { 0x8b, 0x14, 0x36, 0x6b, 0x8e, 0x77, 0x9b, 0x69 } }
>> +
>
> This is strictly a question of style (by which I don't mean strict
> coding style), but is this new GUID necessary?
> I mean, all it needs to be is a GUID to distinguish component
> ... namespaces(?) ... I see both variants in edk2: have a special
> VariableGuid, or just reuse the TokenSpaceGuid.
>
> Just trying to gauge preferences.
>

I am leaning towards keeping it, since everything is a GUID anyway in
UEFI, and having a dedicated one avoids confusion. But I don't feel
strongly about it.

> I'm happy with the patch either way:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

OK, so let's just keep it as is then.


>
>>  [PcdsDynamic]
>>    gAmdStyxTokenSpaceGuid.PcdSocCoreCount|1|UINT32|0x00000100
>>    gAmdStyxTokenSpaceGuid.PcdSocCpuId|1|UINT32|0x00000101
>> @@ -108,3 +112,6 @@
>>    gAmdStyxTokenSpaceGuid.PcdFlashNvStorageOriginalBase|0|UINT64|0x000c0000
>>    # block size to use when invoking the ISCP FV methods
>>    gAmdStyxTokenSpaceGuid.PcdFlashNvStorageBlockSize|0x1000|UINT32|0x000c0001
>> +
>> +[PcdsFixedAtBuild,PcdsDynamic]
>> +  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|FALSE|BOOLEAN|0xe0000000
>> diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
>> index 65d229884aa7..b4893ca34587 100644
>> --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
>> +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
>> @@ -515,9 +515,11 @@ DEFINE DO_KCS       = 1
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0
>>
>> -[PcdsDynamicExHii.common.DEFAULT]
>> +[PcdsDynamicHii]
>>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
>>
>> +  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|L"StyxEnableSmmus"|gAmdStyxVariableGuid|0x0|FALSE
>> +
>>  ################################################################################
>>  #
>>  # Components Section - list of all EDK II Modules needed by this Platform
>> --
>> 2.9.3
>>
diff mbox series

Patch

diff --git a/Platforms/AMD/Styx/AmdStyx.dec b/Platforms/AMD/Styx/AmdStyx.dec
index 8d4b4927c6b1..ddd5bf4c3609 100644
--- a/Platforms/AMD/Styx/AmdStyx.dec
+++ b/Platforms/AMD/Styx/AmdStyx.dec
@@ -35,6 +35,10 @@ 
   gAmdStyxTokenSpaceGuid     = { 0x220d9653, 0x4a0e, 0x40bc, { 0xb3, 0x65, 0x2f, 0xbb, 0xa2, 0xd9, 0x03, 0x45 } }
   gAmdStyxMpCoreInfoGuid     = { 0x68efeabd, 0xcb77, 0x4aa5, { 0xbf, 0x0c, 0xa3, 0x31, 0xfc, 0xcf, 0x76, 0x66 } }
 
+  # 2a5e4deb-4445-4fb6-8b14-366b8e779b69
+  # EFI variable scope for Styx
+  gAmdStyxVariableGuid       = { 0x2a5e4deb, 0x4445, 0x4fb6, { 0x8b, 0x14, 0x36, 0x6b, 0x8e, 0x77, 0x9b, 0x69 } }
+
 [PcdsDynamic]
   gAmdStyxTokenSpaceGuid.PcdSocCoreCount|1|UINT32|0x00000100
   gAmdStyxTokenSpaceGuid.PcdSocCpuId|1|UINT32|0x00000101
@@ -108,3 +112,6 @@ 
   gAmdStyxTokenSpaceGuid.PcdFlashNvStorageOriginalBase|0|UINT64|0x000c0000
   # block size to use when invoking the ISCP FV methods
   gAmdStyxTokenSpaceGuid.PcdFlashNvStorageBlockSize|0x1000|UINT32|0x000c0001
+
+[PcdsFixedAtBuild,PcdsDynamic]
+  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|FALSE|BOOLEAN|0xe0000000
diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
index 65d229884aa7..b4893ca34587 100644
--- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc
@@ -515,9 +515,11 @@  DEFINE DO_KCS       = 1
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0
 
-[PcdsDynamicExHii.common.DEFAULT]
+[PcdsDynamicHii]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
 
+  gAmdStyxTokenSpaceGuid.PcdEnableSmmus|L"StyxEnableSmmus"|gAmdStyxVariableGuid|0x0|FALSE
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform