> -Original Message-
> From: Zeng, Star
> Sent: Friday, August 11, 2017 10:37 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Wu, Hao A; Ni, Ruiyu; Zeng, Star
> Subject: RE: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra
> data is erased by EraseBlocks
>
> Two minor comments, others are good to me.
>
> 1. Could the code use (LastLba + 1 - EndGroupLba) instead of (LastLba -
> EndGroupLba + 1)?
>
> 2. Could the code have the debug message "DEBUG ((EFI_D_ERROR,
> "EmmcEraseBlocks(): Lba 0x%x BlkNo 0x%x Event %p with %r\n", Lba,
> BlockNum, Token->Event, Status))" for the new return paths the patch added?
> And the debug level should be DEBUG_INFO instead of EFI_D_ERROR?
>
Thanks for the feedbacks. I will submit a new version of the patch.
Best Regards,
Hao Wu
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao
> Wu
> Sent: Thursday, August 10, 2017 9:21 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ruiyu ; Zeng,
> Star
> Subject: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra
> data is erased by EraseBlocks
>
> V2 changes:
>
> The Trim command is not supported on all eMMC devices. For those devices
> that do not support such command, add codes to handle the scenario.
>
> Commit message:
>
> The current implementation of the Erase Block Protocol service
> EraseBlocks() uses the erase command. According to spec eMMC Electrical
> Standard 5.1, Section 6.6.9:
>
> The erasable unit of the eMMC is the "Erase Group"; Erase group is measured
> in write blocks that are the basic writable units of the Device.
> ...
> When the Erase is executed it will apply to all write blocks within an erase
> group.
>
> However, code logic in function EmmcEraseBlocks() does not check whether
> the blocks to be erased form complete erase groups. Missing such checks will
> lead to erasing extra data on the device.
>
> This commit will:
> a. If the device support the Trim command, use the Trim command to perform
> the erase operations for eMMC devices.
>
> According to the spec:
> Unlike the Erase command, the Trim function applies the erase operation to
> write blocks instead of erase groups.
>
> b. If the device does not support the Trim command, use the Erase command to
> erase the data in the erase groups. And write zeros to those blocks that
> cannot
> form a complete erase group.
>
> Cc: Star Zeng
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu
> ---
> MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 127
> +-
> 1 file changed, 125 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> index c432d26801..916f15d429 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> @@ -1851,6 +1851,14 @@ EmmcEraseBlock (
>EraseBlock->SdMmcCmdBlk.CommandIndex = EMMC_ERASE;
>EraseBlock->SdMmcCmdBlk.CommandType = SdMmcCommandTypeAc;
>EraseBlock->SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
> + if ((Device->ExtCsd.SecFeatureSupport & BIT4) != 0) {
> +//
> +// Perform a Trim operation which applies the erase operation to write
> blocks
> +// instead of erase groups. (Spec JESD84-B51, eMMC Electrical Standard
> 5.1,
> +// Section 6.6.10 and 6.10.4)
> +//
> +EraseBlock->SdMmcCmdBlk.CommandArgument = 1; }
>
>EraseBlock->IsEnd = IsEnd;
>EraseBlock->Token = Token;
> @@ -1903,6 +1911,43 @@ Error:
> }
>
> /**
> + Write zeros to specified blocks.
> +
> + @param[in] Partition A pointer to the EMMC_PARTITION instance.
> + @param[in] StartLba The starting logical block address to write
> zeros.
> + @param[in] Size The size in bytes to fill with zeros. This
> must be a
> multiple of
> +the physical block size of the device.
> +
> + @retval EFI_SUCCESS The request is executed successfully.
> + @retval EFI_OUT_OF_RESOURCES The request could not be executed due to
> a lack of resources.
> + @retval OthersThe request could not be executed
> successfully.
> +
> +**/
> +EFI_STATUS
> +EmmcWriteZeros (
> + IN EMMC_PARTITION*Partition,
> + IN EFI_LBA StartLba,
> + IN UINTN Size
> + )
> +{
> + EFI_STATUS Status;
> + UINT8*Buffer;
> + UINT32 MediaId;
> +
> + Buffer = AllocateZeroPool (Size);
> + if (Buffer == NULL) {
> +return EFI_OUT_OF_RESOURCES;
> + }
> +
> + MediaId = Partition->BlockMedia.MediaId;
> +
> + Status = EmmcReadWrite (Partition, MediaId, StartLba, Buffer, Size,
> + FALSE, NULL);