Re: [edk2-devel] [PATCH 2/4] OvmfPkg/VirtNorFlashDxe: clarify block write logic

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 12 Jan 2024 at 12:38, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Introduce Start and End variables to make it easier to follow the
> logic and code flow.  Also replace the two NorFlashWriteBuffer calls
> with a loop containing one call.
>
> With the changes in place the code is able to handle updates larger
> than two P30_MAX_BUFFER_SIZE_IN_BYTES blocks, even though the patch
> does not actually change the size limit.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Nit below

> ---
>  OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c | 31 +++++++++++++-------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> index cdc809d75e3d..90db12716a4c 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> @@ -560,13 +560,18 @@ NorFlashWriteSingleBlock (
>      // If the destination bits are only changing from 1s to 0s we can just write.
>      // After a block is erased all bits in the block is set to 1.
>      // If any byte requires us to erase we just give up and rewrite all of it.
> +    UINTN   Start, End;
> +    UINT32  Index, Count;
> +

These var declarations should be at the start of the function.

> +    Start = Offset & ~BOUNDARY_OF_32_WORDS;
> +    End   = (Offset + *NumBytes + BOUNDARY_OF_32_WORDS) & ~BOUNDARY_OF_32_WORDS;
>
>      // Read the old version of the data into the shadow buffer
>      Status = NorFlashRead (
>                 Instance,
>                 Lba,
> -               Offset & ~BOUNDARY_OF_32_WORDS,
> -               (((Offset & BOUNDARY_OF_32_WORDS) + *NumBytes) | BOUNDARY_OF_32_WORDS) + 1,
> +               Start,
> +               End - Start,
>                 Instance->ShadowBuffer
>                 );
>      if (EFI_ERROR (Status)) {
> @@ -599,25 +604,19 @@ NorFlashWriteSingleBlock (
>        goto Exit;
>      }
>
> -    Status = NorFlashWriteBuffer (
> -               Instance,
> -               BlockAddress + (Offset & ~BOUNDARY_OF_32_WORDS),
> -               P30_MAX_BUFFER_SIZE_IN_BYTES,
> -               Instance->ShadowBuffer
> -               );
> -    if (EFI_ERROR (Status)) {
> -      goto Exit;
> -    }
> -
> -    if ((*NumBytes + (Offset & BOUNDARY_OF_32_WORDS)) > P30_MAX_BUFFER_SIZE_IN_BYTES) {
> -      BlockAddress += P30_MAX_BUFFER_SIZE_IN_BYTES;
> -
> +    for (Index = 0, Count = (End - Start) / P30_MAX_BUFFER_SIZE_IN_BYTES;
> +         Index < Count;
> +         Index++, BlockAddress += P30_MAX_BUFFER_SIZE_IN_BYTES)
> +    {
>        Status = NorFlashWriteBuffer (
>                   Instance,
>                   BlockAddress + (Offset & ~BOUNDARY_OF_32_WORDS),
>                   P30_MAX_BUFFER_SIZE_IN_BYTES,
> -                 Instance->ShadowBuffer + P30_MAX_BUFFER_SIZE_IN_BYTES
> +                 Instance->ShadowBuffer + P30_MAX_BUFFER_SIZE_IN_BYTES * Index
>                   );
> +      if (EFI_ERROR (Status)) {
> +        goto Exit;
> +      }
>      }
>
>  Exit:
> --
> 2.43.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113727): https://edk2.groups.io/g/devel/message/113727
Mute This Topic: https://groups.io/mt/103680931/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-