From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.11474.1586182354680595470 for ; Mon, 06 Apr 2020 07:12:34 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F01C011D4; Mon, 6 Apr 2020 07:12:33 -0700 (PDT) Received: from [10.37.8.45] (unknown [10.37.8.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AB1363F73D; Mon, 6 Apr 2020 07:12:31 -0700 (PDT) Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W. To: Leif Lindholm , Gaurav Jain Cc: devel@edk2.groups.io, Pankaj Bansal , Haojian Zhuang , "Loh, Tien Hock" References: <1585905847-16380-1-git-send-email-gaurav.jain@nxp.com> <20200406140819.GI14075@vanye> From: "Ard Biesheuvel" Message-ID: <3d55018b-8751-bbe1-b1ac-98ac36e16e1c@arm.com> Date: Mon, 6 Apr 2020 16:12:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200406140819.GI14075@vanye> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 4/6/20 4:08 PM, Leif Lindholm wrote: > 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 Hello Gaurav, Could you please elaborate on the underlying need for this change? If you are considering using this driver for future NXP platforms, I should point out that this legacy driver is only kept around for existing users, and new users should use the driver stack in MdeModulePkg, which is based on the UEFI spec. -- Ard. >> --- >> 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 >>