Re: [PATCH V2 05/15] MdeModulePkg: Add PcdEmuVariableNvModeEnable in dec

["Zeng, Star" <star.zeng@intel.com>]

On 2019/1/15 13:05, Wang, Jian J wrote:
> Hi Star,
> 
> 
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Monday, January 14, 2019 11:20 PM
>> To: edk2-devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
>> Wu, Hao A <hao.a.wu@intel.com>
>> Subject: [PATCH V2 05/15] MdeModulePkg: Add PcdEmuVariableNvModeEnable
>> in dec
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323
>> Merge EmuVariable and Real variable driver.
>>
>> Add PcdEmuVariableNvModeEnable (support both static and
>> dynamic) to indicate if Variable driver will enable
>> emulated variable NV mode.
>>
>> This patch prepares for adding emulated variable NV mode
>> support in VariableRuntimeDxe.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>   MdeModulePkg/MdeModulePkg.dec | 10 ++++++++--
>>   MdeModulePkg/MdeModulePkg.uni | 10 ++++++++--
>>   2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>> b/MdeModulePkg/MdeModulePkg.dec
>> index 217ede1f7163..8fbc0af61365 100644
>> --- a/MdeModulePkg/MdeModulePkg.dec
>> +++ b/MdeModulePkg/MdeModulePkg.dec
>> @@ -3,7 +3,7 @@
>>   # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library
>> classes)
>>   # and libraries instances, which are used for those modules.
>>   #
>> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>>   # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>>   # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>   # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>> @@ -1586,7 +1586,13 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
>> PcdsDynamic, PcdsDynamicEx]
>>     # @Prompt 64-bit Base address of flash FTW working block range.
>>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0
>> |UINT64|0x80000010
>>
>> -  ## This PCD defines a reserved memory range for the EMU Variable driver's NV
>> Variable Store.
>> +  ## Indicates if Variable driver will enable emulated variable NV
>> mode.<BR><BR>
>> +  #   TRUE  - An EMU variable NV storage will be allocated or reserved for NV
>> variables.<BR>
>> +  #   FALSE - No EMU variable NV storage will be allocated or reserved for NV
>> variables.<BR>
>> +  # @Prompt EMU variable NV mode enable.
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|FALSE|BO
>> OLEAN|0x01100001
>> +
>> +  ## This PCD defines a reserved memory range for EMU variable NV storage.
>>     #  The range is valid if non-zero. The memory range size must be
>> PcdVariableStoreSize.
>>     # @Prompt Reserved memory range for EMU variable NV storage.
>>
> 
> The description is a little bit confuse to me. The value of this PCD is actually the
> base address of reserved memory range, but not the range (size). A complete
> memory range includes both base address and its size. But this PCD contains
> only base address information. Maybe something like below would be better
> (just for example):
> 
> ----------------------
> This PCD defines the base address of reserved memory range for EMU variable
> NV storage. A non-ZERO value indicates a valid range reserved with size given by
> PcdVariableStoreSize.
> 
> @Prompt Base of reserved memory range for EMU variable NV storage.
> ----------------------
> 
> If you agree, please update the uni help string below as well.

In fact, it is not related to this task directly.
But I agree we can make the description to be more clear, I can handle 
it in a new separated patch.


Thanks,
Star

> 
>> gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0|UINT6
>> 4|0x40000008
>> diff --git a/MdeModulePkg/MdeModulePkg.uni
>> b/MdeModulePkg/MdeModulePkg.uni
>> index 35af744d89be..9c413a98f65d 100644
>> --- a/MdeModulePkg/MdeModulePkg.uni
>> +++ b/MdeModulePkg/MdeModulePkg.uni
>> @@ -4,7 +4,7 @@
>>   // It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library
>> classes)
>>   // and libraries instances, which are used for those modules.
>>   //
>> -// Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>> +// Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>>   //
>>   // This program and the accompanying materials are licensed and made
>> available under
>>   // the terms and conditions of the BSD License that accompanies this
>> distribution.
>> @@ -389,9 +389,15 @@
>>
>>   #string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFlashNvStorageFtwWorkingBase6
>> 4_HELP  #language en-US "64-bit Base address of the FTW working block range
>> in flash device. If PcdFlashNvStorageFtwWorkingSize is larger than one block
>> size, this value should be block size aligned."
>>
>> +#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEmuVariableNvModeEnable_PRO
>> MPT  #language en-US "EMU variable NV mode enable"
>> +
>> +#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEmuVariableNvModeEnable_HELP
>> #language en-US "Indicates if Variable driver will enable emulated variable NV
>> mode.<BR><BR>"
>> +                                                                                  "TRUE  - An EMU variable NV
>> storage will be allocated or reserved for NV variables.<BR>"
>> +                                                                                  "FALSE - No EMU variable NV
>> storage will be allocated or reserved for NV variables.<BR>"
>> +
>>   #string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEmuVariableNvStoreReserved_PR
>> OMPT  #language en-US "Reserved memory range for EMU variable NV storage"
>>
>> -#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEmuVariableNvStoreReserved_HE
>> LP  #language en-US "This PCD defines a reserved memory range for the EMU
>> Variable driver's NV Variable Store. The range is valid if non-zero. The memory
>> range size must be PcdVariableStoreSize."
>> +#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEmuVariableNvStoreReserved_HE
>> LP  #language en-US "This PCD defines a reserved memory range for EMU
>> variable NV storage. The range is valid if non-zero. The memory range size must
>> be PcdVariableStoreSize."
>>
>>   #string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdHelloWorldPrintTimes_PROMPT
>> #language en-US "HelloWorld print times"
>>
>> --
>> 2.7.0.windows.1