public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io, nathaniel.l.desimone@intel.com,
	Bret Barkelew <Bret.Barkelew@microsoft.com>,
	"Oram, Isaac W" <isaac.w.oram@intel.com>
Cc: "Chiu, Chasel" <chasel.chiu@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	"Dong, Eric" <eric.dong@intel.com>,
	Michael Kubacki <Michael.Kubacki@microsoft.com>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v2 0/4] Add Large Variable Libraries
Date: Mon, 5 Apr 2021 14:33:22 -0700	[thread overview]
Message-ID: <16c2cea4-4dd8-b090-9305-d84e6057049a@linux.microsoft.com> (raw)
In-Reply-To: <MWHPR1101MB2160FF0CB14DECCC05EDAB7FCD779@MWHPR1101MB2160.namprd11.prod.outlook.com>

I have not reviewed the code yet but I think that's an important point 
to capture. Could you please add that use case to the commit? Perhaps 
the commit message and INF file description for the LargeVariableLibs?

Thanks,
Michael

On 4/5/2021 1:25 PM, Nate DeSimone wrote:
> The max variable size is a build time configurable option. For those 
> that produce binary compatible drivers/OpROMs which need to work both 
> with TianoCore and other vendor provided UEFI PI implementations, the 
> luxury of recompiling the entire BIOS image or dictating the platform’s 
> choice for PcdMaxVariableSize image may not exist.
> 
> *From:* Bret Barkelew <Bret.Barkelew@microsoft.com>
> *Sent:* Monday, April 5, 2021 12:31 PM
> *To:* devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>; 
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> *Cc:* Chiu, Chasel <chasel.chiu@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Michael 
> Kubacki <Michael.Kubacki@microsoft.com>
> *Subject:* RE: [edk2-platforms] [PATCH v2 0/4] Add Large Variable Libraries
> 
> Naïve question: if max variable size is not already configurable, why 
> not just do that?
> 
> - Bret
> 
> *From: *Oram, Isaac W via groups.io 
> <mailto:isaac.w.oram=intel.com@groups.io>
> *Sent: *Monday, April 5, 2021 12:27 PM
> *To: *Desimone, Nathaniel L <mailto:nathaniel.l.desimone@intel.com>; 
> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> *Cc: *Chiu, Chasel <mailto:chasel.chiu@intel.com>; Liming Gao 
> <mailto:gaoliming@byosoft.com.cn>; Dong, Eric 
> <mailto:eric.dong@intel.com>; Michael Kubacki 
> <mailto:Michael.Kubacki@microsoft.com>
> *Subject: *[EXTERNAL] Re: [edk2-devel] [edk2-platforms] [PATCH v2 0/4] 
> Add Large Variable Libraries
> 
> Series Reviewed-by: Isaac Oram <isaac.w.oram@intel.com 
> <mailto:isaac.w.oram@intel.com>>
> 
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com 
> <mailto:nathaniel.l.desimone@intel.com>>
> Sent: Sunday, April 4, 2021 2:41 AM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Chiu, Chasel <chasel.chiu@intel.com <mailto:chasel.chiu@intel.com>>; 
> Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>; 
> Dong, Eric <eric.dong@intel.com <mailto:eric.dong@intel.com>>; Michael 
> Kubacki <michael.kubacki@microsoft.com 
> <mailto:michael.kubacki@microsoft.com>>; Oram, Isaac W 
> <isaac.w.oram@intel.com <mailto:isaac.w.oram@intel.com>>
> Subject: [edk2-platforms] [PATCH v2 0/4] Add Large Variable Libraries
> 
> Changes from V1:
>   - Changed prefix from "Min" to "VarLib"
>   - Better comments
>   - Added more whitespace for readability
>   - Removed unused INF sections
>   - Better debug messages
> 
> This patch series introduces libaries that enable large data sets to be 
> stored using the UEFI Variable Services. At present, most UEFI Variable 
> Services implementations have a maximum variable size of <=64KB. The 
> exact value varies depending on platform.
> 
> These libaries enable a data set to use as much space as needed, up to 
> the remaining space in the UEFI Variable non-volatile storage.
> 
> To implement this, I have broken the problem down into two parts:
> 
>   1. Phase angostic UEFI Variable access.
>   2. Storage of data across multiple UEFI Variables.
> 
> For the first part, I have created two new LibraryClasses:
> VariableReadLib and VariableWriteLib. I have provided implementation 
> instances of VariableReadLib for PEI, DXE, and SMM.
> For VariableWriteLib, I have provided implementation instances for DXE 
> and SMM. This enables code that accesses UEFI variables to be written in 
> a matter than is phase agnostic, so the same code can be used in PEI, 
> DXE, or SMM without modification.
> 
> The second part involves another two new LibaryClasses:
> LargeVariableReadLib and LargeVariableWriteLib. Only one BASE 
> implementation is needed for both of these as the phase dependent code 
> was seperated out in the first piece. These libraries provide logic to 
> calculate the maximum size of an individual UEFI variable and split the 
> data into as many smaller pieces as needed to store the entire data set 
> in the UEFI Variable storage. They also provide the ability to stitch 
> the data back together when it is read.
> Deleting the data will delete all variables used to store it.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com <mailto:chasel.chiu@intel.com>>
> Cc: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>
> Cc: Eric Dong <eric.dong@intel.com <mailto:eric.dong@intel.com>>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com 
> <mailto:michael.kubacki@microsoft.com>>
> Cc: Isaac Oram <isaac.w.oram@intel.com <mailto:isaac.w.oram@intel.com>>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com 
> <mailto:nathaniel.l.desimone@intel.com>>
> 
> Nate DeSimone (4):
>    MinPlatformPkg: Add VariableReadLib
>    MinPlatformPkg: Add VariableWriteLib
>    MinPlatformPkg: Add LargeVariableReadLib
>    MinPlatformPkg: Add LargeVariableWriteLib
> 
>   .../Include/Dsc/CoreCommonLib.dsc             |   6 +-
>   .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  12 +-
>   .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
>   .../Include/Library/LargeVariableReadLib.h    |  50 ++
>   .../Include/Library/LargeVariableWriteLib.h   |  58 +++
>   .../Include/Library/VariableReadLib.h         |  87 ++++
>   .../Include/Library/VariableWriteLib.h        | 129 +++++
>   .../BaseLargeVariableReadLib.inf              |  44 ++
>   .../LargeVariableReadLib.c                    | 199 ++++++++
>   .../BaseLargeVariableWriteLib.inf             |  44 ++
>   .../LargeVariableWriteLib.c                   | 479 ++++++++++++++++++
>   .../DxeRuntimeVariableReadLib.c               | 115 +++++
>   .../DxeRuntimeVariableReadLib.inf             |  41 ++
>   .../DxeRuntimeVariableWriteLib.c              | 256 ++++++++++
>   .../DxeRuntimeVariableWriteLib.inf            |  49 ++
>   .../PeiVariableReadLib/PeiVariableReadLib.c   | 153 ++++++
>   .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 ++
>   .../SmmVariableReadCommon.c                   | 114 +++++
>   .../StandaloneMmVariableReadLib.inf           |  50 ++
>   .../StandaloneMmVariableReadLibConstructor.c  |  48 ++
>   .../TraditionalMmVariableReadLib.inf          |  49 ++
>   .../TraditionalMmVariableReadLibConstructor.c |  48 ++
>   .../SmmVariableWriteCommon.c                  | 167 ++++++
>   .../StandaloneMmVariableWriteLib.inf          |  45 ++
>   .../StandaloneMmVariableWriteLibConstructor.c |  48 ++
>   .../TraditionalMmVariableWriteLib.inf         |  44 ++
>   ...TraditionalMmVariableWriteLibConstructor.c |  48 ++
>   .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   4 +-
>   28 files changed, 2428 insertions(+), 10 deletions(-)  create mode 
> 100644 Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/BaseLargeVariableWriteLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/LargeVariableWriteLib.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/SmmVariableReadCommon.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLibConstructor.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLibConstructor.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c
> 
> --
> 2.27.0.windows.1
> 
> 
> 
> 

      reply	other threads:[~2021-04-05 21:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-04  9:40 [edk2-platforms] [PATCH v2 0/4] Add Large Variable Libraries Nate DeSimone
2021-04-04  9:40 ` [edk2-platforms] [PATCH v2 1/4] MinPlatformPkg: Add VariableReadLib Nate DeSimone
2021-04-04  9:40 ` [edk2-platforms] [PATCH v2 2/4] MinPlatformPkg: Add VariableWriteLib Nate DeSimone
2021-04-04  9:40 ` [edk2-platforms] [PATCH v2 3/4] MinPlatformPkg: Add LargeVariableReadLib Nate DeSimone
2021-04-04  9:40 ` [edk2-platforms] [PATCH v2 4/4] MinPlatformPkg: Add LargeVariableWriteLib Nate DeSimone
2021-04-05 19:26 ` [edk2-platforms] [PATCH v2 0/4] Add Large Variable Libraries Oram, Isaac W
2021-04-05 19:30   ` Bret Barkelew
2021-04-05 20:25     ` Nate DeSimone
2021-04-05 21:33       ` Michael Kubacki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16c2cea4-4dd8-b090-9305-d84e6057049a@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox