Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.

["Leif Lindholm" <leif@nuviainc.com>]

Hi Gaurav,

Haojian, Tien Hock - can you help review/test this change?

Best Regards,

Leif

On Fri, Apr 03, 2020 at 14:54:07 +0530, Gaurav Jain wrote:
> Moved BlockCount calculation below BufferSize Validation checks.
> First Ensure Buffersize is Not Zero and multiple of Media BlockSize.
> then calculate BlockCount and perform Block checks.
> 
> Corrected BlockCount calculation, as BufferSize is multiple of BlockSize,
> So adding (BlockSize-1) bytes to BufferSize and
> then divide by BlockSize will have no impact on BlockCount.
> 
> Reading Large Images from MMC causes errors.
> As per SD Host Controller Spec version 4.20,
> Restriction of 16-bit Block Count transfer is 65535.
> Max block transfer limit in single cmd is 65535 blocks.
> Added Max Block check that can be processed is 0xFFFF.
> then Update BlockCount on the basis of MaxBlock.
> 
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> ---
>  EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 38 ++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> index 17c20c0159ba..b508c466d9c5 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> @@ -242,6 +242,8 @@ MmcIoBlocks (
>    UINTN                   BytesRemainingToBeTransfered;
>    UINTN                   BlockCount;
>    UINTN                   ConsumeSize;
> +  UINT32                  MaxBlock;
> +  UINTN                   RemainingBlock;
>  
>    BlockCount = 1;
>    MmcHostInstance = MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS (This);
> @@ -262,19 +264,6 @@ MmcIoBlocks (
>      return EFI_NO_MEDIA;
>    }
>  
> -  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost->IsMultiBlock(MmcHost)) {
> -    BlockCount = (BufferSize + This->Media->BlockSize - 1) / This->Media->BlockSize;
> -  }
> -
> -  // All blocks must be within the device
> -  if ((Lba + (BufferSize / This->Media->BlockSize)) > (This->Media->LastBlock + 1)) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly == TRUE)) {
> -    return EFI_WRITE_PROTECTED;
> -  }
> -
>    // Reading 0 Byte is valid
>    if (BufferSize == 0) {
>      return EFI_SUCCESS;
> @@ -285,14 +274,36 @@ MmcIoBlocks (
>      return EFI_BAD_BUFFER_SIZE;
>    }
>  
> +  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost->IsMultiBlock(MmcHost)) {
> +    BlockCount = BufferSize / This->Media->BlockSize;
> +  }
> +
> +  // All blocks must be within the device
> +  if ((Lba + (BufferSize / This->Media->BlockSize)) > (This->Media->LastBlock + 1)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly == TRUE)) {
> +    return EFI_WRITE_PROTECTED;
> +  }
> +
>    // Check the alignment
>    if ((This->Media->IoAlign > 2) && (((UINTN)Buffer & (This->Media->IoAlign - 1)) != 0)) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  // Max block number in single cmd is 65535 blocks.
> +  MaxBlock = 0xFFFF;
> +  RemainingBlock = BlockCount;
>    BytesRemainingToBeTransfered = BufferSize;
>    while (BytesRemainingToBeTransfered > 0) {
>  
> +    if (RemainingBlock <= MaxBlock) {
> +      BlockCount = RemainingBlock;
> +    } else {
> +      BlockCount = MaxBlock;
> +    }
> +
>      // Check if the Card is in Ready status
>      CmdArg = MmcHostInstance->CardInfo.RCA << 16;
>      Response[0] = 0;
> @@ -338,6 +349,7 @@ MmcIoBlocks (
>        DEBUG ((EFI_D_ERROR, "%a(): Failed to transfer block and Status:%r\n", __func__, Status));
>      }
>  
> +    RemainingBlock -= BlockCount;
>      BytesRemainingToBeTransfered -= ConsumeSize;
>      if (BytesRemainingToBeTransfered > 0) {
>        Lba    += BlockCount;
> -- 
> 2.7.4
>