Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
Actually, I was assuming FVB was manipulating block size data. But I have just realized it can do byte size accesses... I would need to check what our driver does... Thanks, Olivier From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: 11 March 2014 16:27 To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() Olivier - Actually, the current NOR technology assumes that we can do a byte level write only when changing bits from 1 to 0. In order to change from 0 to 1, you need to do an erase. I understood (correct me if I'm wrong) that NAND works the opposite way (can change from 0 to 1, but 1 to 0 requires and erase) That is why there is a polarity bit in the FV header, to tell the FV code which way to do it. Tim From: Olivier Martin [mailto:olivier.mar...@arm.com] Sent: Tuesday, March 11, 2014 7:47 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() As Andrew says you have assumed a hardware specific technology (you assume you have SPI Flash which allows to write a single byte in NV memory) to manage your atomicity. For most block devices, it is not possible to update a single byte without reading/writing the full block. It means the current logic of UpdateVariable() is not correct. If we keep the assumption that we can only do block access then it would be more efficient/faster to use a checksum to ensure the consistency of our variables (actually there is a spare field into VARIABLE_HEADER). Most Flash memory needs their block to be erased before to be written. It means with the current flow, we have (example of 256 Byte page): Step 1: Write the header: Erase 256B. Write 256B Step 2: Update the state: Erase 256B. Write 256B Step 3: Write the Data: Erase 256B. Write 256B Step 4: Update the state: Erase 256B. Write 256B From: Andrew Fish [mailto:af...@apple.com] Sent: 11 March 2014 02:29 To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() On Mar 10, 2014, at 7:04 PM, Zeng, Star wrote: Hi Olivier, As we know, the BlockSize is the unit of FVB, FVB needs to depend on lower protocol(SPI protocol) to do real write operation, SPI Protocol does not seem to be part of the Open source or industry standard. FirmwareVolumeBlock is the last standardized/open source hop. Thanks, Andrew Fish but the unit of write operation may be one byte, or other bytes. Anyway, we should not assume the operation unit of real flash chip. To keep the atomicity, we need to be very careful. Thanks, Star From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Tuesday, March 11, 2014 5:42 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() Olivier - I don't have the whole code section memorized, but if I remember right there is the problem when there is a power failure as the variable header is being written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire variable header was not written, wouldn't this be another inconsistent state? Tim From: Olivier Martin [ <mailto:olivier.mar...@arm.com> mailto:olivier.mar...@arm.com] Sent: Monday, March 10, 2014 12:52 PM To: <mailto:edk2-devel@lists.sourceforge.net> edk2-devel@lists.sourceforge.net Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() Dear MdeModulePkg maintainers, We have seen on some platforms that flash writing/erasing counts for most of the boot time. We have also noticed UpdateVariable() (in MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the same region of Flash to update a non-volatile variable: // 1. Write variable header UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset, sizeof (VARIABLE_HEADER), (UINT8 *) NextVariable ); // 2. Set variable state to header valid NextVariable->State = VAR_HEADER_VALID_ONLY; UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State), sizeof (UINT8), &NextVariable->State ); // 3. Write variable data UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof (VARIABLE_HEADER), (UINT32) VarSize - sizeof (VARIABLE_HEADER), (UINT8 *) NextVariable + sizeof (VARIABLE_HEADER) ); // 4. Set variable state to valid NextVariable->State = VAR_ADDED; UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State), sizeof (UINT8), &NextVar
Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
Olivier - Actually, the current NOR technology assumes that we can do a byte level write only when changing bits from 1 to 0. In order to change from 0 to 1, you need to do an erase. I understood (correct me if I'm wrong) that NAND works the opposite way (can change from 0 to 1, but 1 to 0 requires and erase) That is why there is a polarity bit in the FV header, to tell the FV code which way to do it. Tim From: Olivier Martin [mailto:olivier.mar...@arm.com] Sent: Tuesday, March 11, 2014 7:47 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() As Andrew says you have assumed a hardware specific technology (you assume you have SPI Flash which allows to write a single byte in NV memory) to manage your atomicity. For most block devices, it is not possible to update a single byte without reading/writing the full block. It means the current logic of UpdateVariable() is not correct. If we keep the assumption that we can only do block access then it would be more efficient/faster to use a checksum to ensure the consistency of our variables (actually there is a spare field into VARIABLE_HEADER). Most Flash memory needs their block to be erased before to be written. It means with the current flow, we have (example of 256 Byte page): Step 1: Write the header: Erase 256B. Write 256B Step 2: Update the state: Erase 256B. Write 256B Step 3: Write the Data: Erase 256B. Write 256B Step 4: Update the state: Erase 256B. Write 256B From: Andrew Fish [mailto:af...@apple.com] Sent: 11 March 2014 02:29 To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() On Mar 10, 2014, at 7:04 PM, Zeng, Star mailto:star.z...@intel.com>> wrote: Hi Olivier, As we know, the BlockSize is the unit of FVB, FVB needs to depend on lower protocol(SPI protocol) to do real write operation, SPI Protocol does not seem to be part of the Open source or industry standard. FirmwareVolumeBlock is the last standardized/open source hop. Thanks, Andrew Fish but the unit of write operation may be one byte, or other bytes. Anyway, we should not assume the operation unit of real flash chip. To keep the atomicity, we need to be very careful. Thanks, Star From: Tim Lewis [mailto:tim.le...@insyde.com] Sent: Tuesday, March 11, 2014 5:42 AM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() Olivier - I don't have the whole code section memorized, but if I remember right there is the problem when there is a power failure as the variable header is being written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire variable header was not written, wouldn't this be another inconsistent state? Tim From: Olivier Martin [mailto:olivier.mar...@arm.com] Sent: Monday, March 10, 2014 12:52 PM To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable() Dear MdeModulePkg maintainers, We have seen on some platforms that flash writing/erasing counts for most of the boot time. We have also noticed UpdateVariable() (in MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the same region of Flash to update a non-volatile variable: // 1. Write variable header UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset, sizeof (VARIABLE_HEADER), (UINT8 *) NextVariable ); // 2. Set variable state to header valid NextVariable->State = VAR_HEADER_VALID_ONLY; UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State), sizeof (UINT8), &NextVariable->State ); // 3. Write variable data UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof (VARIABLE_HEADER), (UINT32) VarSize - sizeof (VARIABLE_HEADER), (UINT8 *) NextVariable + sizeof (VARIABLE_HEADER) ); // 4. Set variable state to valid NextVariable->State = VAR_ADDED; UpdateVariableStore (..., mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State), sizeof (UINT8), &NextVariable->State ); I understand the 4 steps are to prevent the flash to be inconsistent in case of accidental reset. Actually the steps 1 and 2 could easily be merged (ie: do 'NextVariable->State = VAR_HEADER_VALID_ONLY' in step 1). For most variables, it is likely these 4 accesses would be into the same block of flash. It means this block of flash would be written 4 times. Could we potent
Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
As Andrew says you have assumed a hardware specific technology (you assume
you have SPI Flash which allows to write a single byte in NV memory) to
manage your atomicity.
For most block devices, it is not possible to update a single byte without
reading/writing the full block.
It means the current logic of UpdateVariable() is not correct.
If we keep the assumption that we can only do block access then it would be
more efficient/faster to use a checksum to ensure the consistency of our
variables (actually there is a spare field into VARIABLE_HEADER).
Most Flash memory needs their block to be erased before to be written. It
means with the current flow, we have (example of 256 Byte page):
Step 1: Write the header: Erase 256B. Write 256B
Step 2: Update the state: Erase 256B. Write 256B
Step 3: Write the Data: Erase 256B. Write 256B
Step 4: Update the state: Erase 256B. Write 256B
From: Andrew Fish [mailto:af...@apple.com]
Sent: 11 March 2014 02:29
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a
NV variable in UpdateVariable()
On Mar 10, 2014, at 7:04 PM, Zeng, Star wrote:
Hi Olivier,
As we know, the BlockSize is the unit of FVB, FVB needs to depend on lower
protocol(SPI protocol) to do real write operation,
SPI Protocol does not seem to be part of the Open source or industry
standard.
FirmwareVolumeBlock is the last standardized/open source hop.
Thanks,
Andrew Fish
but the unit of write operation may be one byte, or other
bytes. Anyway, we should not assume the operation unit of real flash chip.
To keep the atomicity, we need to be very careful.
Thanks,
Star
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, March 11, 2014 5:42 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a
NV variable in UpdateVariable()
Olivier -
I don't have the whole code section memorized, but if I remember right there
is the problem when there is a power failure as the variable header is being
written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire
variable header was not written, wouldn't this be another inconsistent
state?
Tim
From: Olivier Martin [ <mailto:olivier.mar...@arm.com>
mailto:olivier.mar...@arm.com]
Sent: Monday, March 10, 2014 12:52 PM
To: <mailto:edk2-devel@lists.sourceforge.net>
edk2-devel@lists.sourceforge.net
Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
variable in UpdateVariable()
Dear MdeModulePkg maintainers,
We have seen on some platforms that flash writing/erasing counts for most of
the boot time.
We have also noticed UpdateVariable() (in
MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the
same region of Flash to update a non-volatile variable:
// 1. Write variable header
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Set variable state to header valid
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
// 3. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 4. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
I understand the 4 steps are to prevent the flash to be inconsistent in case
of accidental reset.
Actually the steps 1 and 2 could easily be merged (ie: do
'NextVariable->State = VAR_HEADER_VALID_ONLY' in step 1).
For most variables, it is likely these 4 accesses would be into the same
block of flash. It means this block of flash would be written 4 times.
Could we potentially check if we do a write to the same block of flash and
do a single write?
For instance (untested code):
BlockSize = Fvb->GetBlockSize ();
BlockStart = mVariableModuleGlobal->NonVolatileLastVariableOffset &
~(BlockSize - 1);
BlockEnd = (mVariableModuleGlobal->NonVolatileLastVariableOffset + VarSize)
& ~(BlockSize - 1);
if (BlockStart == BlockEnd) {
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
VarSize,
(UINT8 *) NextVariable
);
} else {
// 1. Write va
Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
On Mar 10, 2014, at 7:04 PM, Zeng, Star wrote:
> Hi Olivier,
>
> As we know, the BlockSize is the unit of FVB, FVB needs to depend on lower
> protocol(SPI protocol) to do real write operation,
SPI Protocol does not seem to be part of the Open source or industry standard.
FirmwareVolumeBlock is the last standardized/open source hop.
Thanks,
Andrew Fish
> but the unit of write operation may be one byte, or other
> bytes. Anyway, we should not assume the operation unit of real flash chip. To
> keep the atomicity, we need to be very careful.
>
> Thanks,
> Star
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Tuesday, March 11, 2014 5:42 AM
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
> variable in UpdateVariable()
>
> Olivier –
>
> I don’t have the whole code section memorized, but if I remember right there
> is the problem when there is a power failure as the variable header is being
> written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire
> variable header was not written, wouldn’t this be another inconsistent state?
>
> Tim
>
> From: Olivier Martin [mailto:olivier.mar...@arm.com]
> Sent: Monday, March 10, 2014 12:52 PM
> To: edk2-devel@lists.sourceforge.net
> Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
> variable in UpdateVariable()
>
> Dear MdeModulePkg maintainers,
>
> We have seen on some platforms that flash writing/erasing counts for most of
> the boot time.
> We have also noticed UpdateVariable() (in
> MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the
> same region of Flash to update a non-volatile variable:
>
> // 1. Write variable header
> UpdateVariableStore (...,
> mVariableModuleGlobal->NonVolatileLastVariableOffset,
> sizeof (VARIABLE_HEADER),
> (UINT8 *) NextVariable
> );
> // 2. Set variable state to header valid
> NextVariable->State = VAR_HEADER_VALID_ONLY;
> UpdateVariableStore (...,
>mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
> (VARIABLE_HEADER, State),
>sizeof (UINT8),
>&NextVariable->State
>);
> // 3. Write variable data
> UpdateVariableStore (...,
>mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
> (VARIABLE_HEADER),
>(UINT32) VarSize - sizeof (VARIABLE_HEADER),
>(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
>);
> // 4. Set variable state to valid
> NextVariable->State = VAR_ADDED;
> UpdateVariableStore (...,
>mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
> (VARIABLE_HEADER, State),
>sizeof (UINT8),
>&NextVariable->State
>);
>
> I understand the 4 steps are to prevent the flash to be inconsistent in case
> of accidental reset.
> Actually the steps 1 and 2 could easily be merged (ie: do
> 'NextVariable->State = VAR_HEADER_VALID_ONLY' in step 1).
>
> For most variables, it is likely these 4 accesses would be into the same
> block of flash. It means this block of flash would be written 4 times.
> Could we potentially check if we do a write to the same block of flash and do
> a single write?
>
> For instance (untested code):
> BlockSize = Fvb->GetBlockSize ();
> BlockStart = mVariableModuleGlobal->NonVolatileLastVariableOffset &
> ~(BlockSize – 1);
> BlockEnd = (mVariableModuleGlobal->NonVolatileLastVariableOffset + VarSize) &
> ~(BlockSize – 1);
> if (BlockStart == BlockEnd) {
> NextVariable->State = VAR_ADDED;
> UpdateVariableStore (...,
>mVariableModuleGlobal->NonVolatileLastVariableOffset,
>VarSize,
>(UINT8 *) NextVariable
>);
> } else {
> // 1. Write variable header
> NextVariable->State = VAR_HEADER_VALID_ONLY;
> UpdateVariableStore (...,
>mVariableModuleGlobal->NonVolatileLastVariableOffset,
>sizeof (VARIABLE_HEADER),
>(UINT8 *) NextVariable
>);
> // 2. Write variable data
> UpdateVariableStore (...,
> mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
> (VARIABLE_HEADER),
> (UINT32) VarSize - sizeof (VARIABLE_HEADER),
> (UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
>);
> // 3. Set variable state to valid
> NextVariable->State = VAR_ADDED;
> UpdateVariableStore (...,
> mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
> (VARIABLE_HEADER, State),
>
Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
Hi Olivier,
As we know, the BlockSize is the unit of FVB, FVB needs to depend on lower
protocol(SPI protocol) to do real write operation, but the unit of write
operation may be one byte[cid:image001.png@01CF3D11.5A7ACF50], or other bytes.
Anyway, we should not assume the operation unit of real flash chip. To keep the
atomicity, we need to be very careful.
Thanks,
Star
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, March 11, 2014 5:42 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
variable in UpdateVariable()
Olivier -
I don't have the whole code section memorized, but if I remember right there is
the problem when there is a power failure as the variable header is being
written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire variable
header was not written, wouldn't this be another inconsistent state?
Tim
From: Olivier Martin [mailto:olivier.mar...@arm.com]
Sent: Monday, March 10, 2014 12:52 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
variable in UpdateVariable()
Dear MdeModulePkg maintainers,
We have seen on some platforms that flash writing/erasing counts for most of
the boot time.
We have also noticed UpdateVariable() (in
MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the same
region of Flash to update a non-volatile variable:
// 1. Write variable header
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Set variable state to header valid
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
// 3. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 4. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
I understand the 4 steps are to prevent the flash to be inconsistent in case of
accidental reset.
Actually the steps 1 and 2 could easily be merged (ie: do 'NextVariable->State
= VAR_HEADER_VALID_ONLY' in step 1).
For most variables, it is likely these 4 accesses would be into the same block
of flash. It means this block of flash would be written 4 times.
Could we potentially check if we do a write to the same block of flash and do a
single write?
For instance (untested code):
BlockSize = Fvb->GetBlockSize ();
BlockStart = mVariableModuleGlobal->NonVolatileLastVariableOffset & ~(BlockSize
- 1);
BlockEnd = (mVariableModuleGlobal->NonVolatileLastVariableOffset + VarSize) &
~(BlockSize - 1);
if (BlockStart == BlockEnd) {
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
VarSize,
(UINT8 *) NextVariable
);
} else {
// 1. Write variable header
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 3. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
}
Regards,
Olivier
<>--
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Re: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
Olivier -
I don't have the whole code section memorized, but if I remember right there is
the problem when there is a power failure as the variable header is being
written. If VAR_HEADER_VALID_ONLY was set in this case, but the entire variable
header was not written, wouldn't this be another inconsistent state?
Tim
From: Olivier Martin [mailto:olivier.mar...@arm.com]
Sent: Monday, March 10, 2014 12:52 PM
To: edk2-devel@lists.sourceforge.net
Subject: [edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV
variable in UpdateVariable()
Dear MdeModulePkg maintainers,
We have seen on some platforms that flash writing/erasing counts for most of
the boot time.
We have also noticed UpdateVariable() (in
MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the same
region of Flash to update a non-volatile variable:
// 1. Write variable header
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Set variable state to header valid
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
// 3. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 4. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
I understand the 4 steps are to prevent the flash to be inconsistent in case of
accidental reset.
Actually the steps 1 and 2 could easily be merged (ie: do 'NextVariable->State
= VAR_HEADER_VALID_ONLY' in step 1).
For most variables, it is likely these 4 accesses would be into the same block
of flash. It means this block of flash would be written 4 times.
Could we potentially check if we do a write to the same block of flash and do a
single write?
For instance (untested code):
BlockSize = Fvb->GetBlockSize ();
BlockStart = mVariableModuleGlobal->NonVolatileLastVariableOffset & ~(BlockSize
- 1);
BlockEnd = (mVariableModuleGlobal->NonVolatileLastVariableOffset + VarSize) &
~(BlockSize - 1);
if (BlockStart == BlockEnd) {
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
VarSize,
(UINT8 *) NextVariable
);
} else {
// 1. Write variable header
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 3. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
}
Regards,
Olivier
--
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
[edk2] MdeModulePkg: (at least) 4 writes to Flash to update a NV variable in UpdateVariable()
Dear MdeModulePkg maintainers,
We have seen on some platforms that flash writing/erasing counts for most of
the boot time.
We have also noticed UpdateVariable() (in
MdeModulePkg/Universal/RuntimeDxe/Variable.c) might make 4 accesses to the
same region of Flash to update a non-volatile variable:
// 1. Write variable header
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Set variable state to header valid
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
// 3. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 4. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
I understand the 4 steps are to prevent the flash to be inconsistent in case
of accidental reset.
Actually the steps 1 and 2 could easily be merged (ie: do
'NextVariable->State = VAR_HEADER_VALID_ONLY' in step 1).
For most variables, it is likely these 4 accesses would be into the same
block of flash. It means this block of flash would be written 4 times.
Could we potentially check if we do a write to the same block of flash and
do a single write?
For instance (untested code):
BlockSize = Fvb->GetBlockSize ();
BlockStart = mVariableModuleGlobal->NonVolatileLastVariableOffset &
~(BlockSize - 1);
BlockEnd = (mVariableModuleGlobal->NonVolatileLastVariableOffset + VarSize)
& ~(BlockSize - 1);
if (BlockStart == BlockEnd) {
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
VarSize,
(UINT8 *) NextVariable
);
} else {
// 1. Write variable header
NextVariable->State = VAR_HEADER_VALID_ONLY;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable
);
// 2. Write variable data
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + sizeof
(VARIABLE_HEADER),
(UINT32) VarSize - sizeof (VARIABLE_HEADER),
(UINT8 *) NextVariable + sizeof (VARIABLE_HEADER)
);
// 3. Set variable state to valid
NextVariable->State = VAR_ADDED;
UpdateVariableStore (...,
mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF
(VARIABLE_HEADER, State),
sizeof (UINT8),
&NextVariable->State
);
}
Regards,
Olivier
--
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
