Re: [edk2-devel] [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV variable functions

2019-10-03 Thread Kubacki, Michael A 
I was debating on keeping this file in the patch series. I don't see a problem 
moving
those other functions. The goal was to break out some of the NV-specific content
from many of the other more generic functions in Variable.c Since you mentioned 
it,
I will make that change in V3, unless I hear back otherwise.

Thanks,
Michael

> -Original Message-
> From: Wu, Hao A 
> Sent: Thursday, October 3, 2019 1:04 AM
> To: Kubacki, Michael A ;
> devel@edk2.groups.io
> Cc: Bi, Dandan ; Ard Biesheuvel
> ; Dong, Eric ; Laszlo Ersek
> ; Gao, Liming ; Kinney, Michael
> D ; Ni, Ray ; Wang, Jian J
> ; Yao, Jiewen 
> Subject: RE: [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV
> variable functions
> 
> > -Original Message-
> > From: Kubacki, Michael A
> > Sent: Saturday, September 28, 2019 9:47 AM
> > To: devel@edk2.groups.io
> > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming;
> > Kinney, Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen
> > Subject: [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV
> > variable functions
> >
> > This change adds a dedicated file for variable operations specific to
> > non-volatile variables. This decreases the overall length of the
> > relatively large Variable.c file.
> 
> 
> It is not clear to me what are the criteria for moving functions into the
> separate new file.
> 
> I guess the new file is for functions related with NV variables, but I saw 
> there
> are functions like:
> 
> InitRealNonVolatileVariableStore
> InitEmuNonVolatileVariableStore
> InitNonVolatileVariableStore
> 
> Not sure if they can be put into the new file as well.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Cc: Dandan Bi 
> > Cc: Ard Biesheuvel 
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Ray Ni 
> > Cc: Jian J Wang 
> > Cc: Hao A Wu 
> > Cc: Jiewen Yao 
> > Signed-off-by: Michael Kubacki 
> > ---
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > |  2 ++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf  |
> 2
> > ++
> >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > |  2 ++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> |
> > 25 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 20 
> > +--
> --
> > -
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
> |
> > 28 
> >  6 files changed, 60 insertions(+), 19 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > index c35e5fe787..08a5490787 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > @@ -36,6 +36,8 @@
> >Variable.c
> >VariableDxe.c
> >Variable.h
> > +  VariableNonVolatile.c
> > +  VariableNonVolatile.h
> >VariableParsing.c
> >VariableParsing.h
> >PrivilegePolymorphic.h
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > index 626738b9c7..6dc2721b81 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > @@ -45,6 +45,8 @@
> >Variable.c
> >VariableTraditionalMm.c
> >VariableSmm.c
> > +  VariableNonVolatile.c
> > +  VariableNonVolatile.h
> >VariableParsing.c
> >VariableParsing.h
> >VarCheck.c
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > nf
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> > inf
> > index 1ba8f9ebfb..ca9d23ce9f 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > nf
> > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> > inf
> > @@ -45,6 +45,8 @@
> >Variable.c
> >VariableSmm.c
> >VariableStandaloneMm.c
> > +  VariableNonVolatile.c
> > +  VariableNonVolatile.h
> >VariableParsing.c
> >VariableParsing.h
> >VarCheck.c
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> > new file mode 100644
> > index 00..82572262ef
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> > @@ -0,0 +1,25 @@
> > +/** @file
> > +  Common variable non-volatile store routines.
> > +
> > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef _VARIABLE_NON_VOLATILE_H_
> > +#define _VARIABLE_NON_VOLATILE_H_
> > +
> > +#include "Variable.h"
> > +
> > +/**
> > +  Get non-volatile maximum

Re: [edk2-devel] [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV variable functions

2019-10-03 Thread Wu, Hao A 
> -Original Message-
> From: Kubacki, Michael A
> Sent: Saturday, September 28, 2019 9:47 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; Kinney,
> Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen
> Subject: [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV variable
> functions
> 
> This change adds a dedicated file for variable operations specific
> to non-volatile variables. This decreases the overall length of the
> relatively large Variable.c file.


It is not clear to me what are the criteria for moving functions into the
separate new file.

I guess the new file is for functions related with NV variables, but I saw
there are functions like:

InitRealNonVolatileVariableStore
InitEmuNonVolatileVariableStore
InitNonVolatileVariableStore

Not sure if they can be put into the new file as well.

Best Regards,
Hao Wu


> 
> Cc: Dandan Bi 
> Cc: Ard Biesheuvel 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Jiewen Yao 
> Signed-off-by: Michael Kubacki 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |  2 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf  |  2
> ++
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |  2 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h|
> 25 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 20 
> +
> -
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c|
> 28 
>  6 files changed, 60 insertions(+), 19 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index c35e5fe787..08a5490787 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> @@ -36,6 +36,8 @@
>Variable.c
>VariableDxe.c
>Variable.h
> +  VariableNonVolatile.c
> +  VariableNonVolatile.h
>VariableParsing.c
>VariableParsing.h
>PrivilegePolymorphic.h
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 626738b9c7..6dc2721b81 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -45,6 +45,8 @@
>Variable.c
>VariableTraditionalMm.c
>VariableSmm.c
> +  VariableNonVolatile.c
> +  VariableNonVolatile.h
>VariableParsing.c
>VariableParsing.h
>VarCheck.c
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> nf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> inf
> index 1ba8f9ebfb..ca9d23ce9f 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> nf
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> inf
> @@ -45,6 +45,8 @@
>Variable.c
>VariableSmm.c
>VariableStandaloneMm.c
> +  VariableNonVolatile.c
> +  VariableNonVolatile.h
>VariableParsing.c
>VariableParsing.h
>VarCheck.c
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> new file mode 100644
> index 00..82572262ef
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
> @@ -0,0 +1,25 @@
> +/** @file
> +  Common variable non-volatile store routines.
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _VARIABLE_NON_VOLATILE_H_
> +#define _VARIABLE_NON_VOLATILE_H_
> +
> +#include "Variable.h"
> +
> +/**
> +  Get non-volatile maximum variable size.
> +
> +  @return Non-volatile maximum variable size.
> +
> +**/
> +UINTN
> +GetNonVolatileMaxVariableSize (
> +  VOID
> +  );
> +
> +#endif
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 53d797152c..5da2354aa5 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> 
>  #include "Variable.h"
> +#include "VariableNonVolatile.h"
>  #include "VariableParsing.h"
> 
>  VARIABLE_MODULE_GLOBAL  *mVariableModuleGlobal;
> @@ -3006,25 +3007,6 @@ ReclaimForOS(
>}
>  }
> 
> -/**
> -  Get non-volatile maximum variable size.
> -
> -  @return Non-volatile maximum variable size.
> -
> -**/
> -UINTN
> -GetNonVolatileMaxVariableSize (
> -  VOID
> -  )
> -{
> -  if (PcdGet32 (PcdHwErrStorageSize) != 0) {
> -return MAX (MAX (PcdGet32 (PcdMaxVariableSize),