Re: [PATCH 1/2] MdeModulePkg/Variable: Check EFI_MEMORY_RUNTIME attribute before setting it

[Brijesh Singh <brijesh.singh@amd.com>]

On 7/2/18 10:21 PM, Zeng, Star wrote:
> Please use
> if ((GcdDescriptor.Attributes & EFI_MEMORY_RUNTIME) == 0) {
> instead of
> if (!(GcdDescriptor.Attributes & EFI_MEMORY_RUNTIME)) {
>
> There is coding style and ECC tool check about it. See https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/57_c_programming.html#table-10-predicate-expression-examples.

thanks for quick feedback, I will update the patch with coding style fix
in next rev.

>
> With the change above, Reviewed-by: Star Zeng <star.zeng@intel.com>
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Brijesh Singh [mailto:brijesh.singh@amd.com] 
> Sent: Tuesday, July 3, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Dong, Eric <eric.dong@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH 1/2] MdeModulePkg/Variable: Check EFI_MEMORY_RUNTIME attribute before setting it
>
> Set the EFI_MEMORY_RUNTIME attribute in FtwNotificationEvent() only if the attribute is not already present. This will ensure that the attributes set by the platform drivers (e.g Ovmf pflash) is not lost.
>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Justen Jordan L <jordan.l.justen@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 6b04f4f7b394..f5ab6641ef28 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -412,13 +412,15 @@ FtwNotificationEvent (
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_WARN, "Variable driver failed to get flash memory attribute.\n"));
>    } else {
> -    Status = gDS->SetMemorySpaceAttributes (
> -                    BaseAddress,
> -                    Length,
> -                    GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
> -                    );
> -    if (EFI_ERROR (Status)) {
> -      DEBUG ((DEBUG_WARN, "Variable driver failed to add EFI_MEMORY_RUNTIME attribute to Flash.\n"));
> +    if (!(GcdDescriptor.Attributes & EFI_MEMORY_RUNTIME)) {
> +      Status = gDS->SetMemorySpaceAttributes (
> +                      BaseAddress,
> +                      Length,
> +                      GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME
> +                      );
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_WARN, "Variable driver failed to add EFI_MEMORY_RUNTIME attribute to Flash.\n"));
> +      }
>      }
>    }
>  
> --
> 2.7.4
>