public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Haojian Zhuang <haojian.zhuang@linaro.org>
To: "Wu, Hao A" <hao.a.wu@intel.com>
Cc: "Tian, Feng" <feng.tian@intel.com>,
	"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
	"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH] MdeModulePkg: ScsiDiskDxe: fix to support EFI_ERASE_BLOCK_PROTOCOL
Date: Mon, 17 Apr 2017 14:03:05 +0000	[thread overview]
Message-ID: <CY1PR15MB0730427CE3801EE5769AC29997060@CY1PR15MB0730.namprd15.prod.outlook.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A0931C9937B@SHSMSX104.ccr.corp.intel.com>

Hi Hao,

I tried the discard operation on my UFS device. It just return 0.

And erase operation isn’t supported on my UFS device. If I don’t support discard operation, I can’t erase blocks at all.

Best Regards
Haojian

From: Wu, Hao A<mailto:hao.a.wu@intel.com>
Sent: 2017年4月17日 9:59
To: Haojian Zhuang<mailto:haojian.zhuang@linaro.org>
Cc: Tian, Feng<mailto:feng.tian@intel.com>; leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH] MdeModulePkg: ScsiDiskDxe: fix to support EFI_ERASE_BLOCK_PROTOCOL

Hi Haojian,

The reason for me to check both bits TPE and TPRZ being set when
determining whether the Erase Block Protocol should be produced is:

According to the Universal Flash Storage (UFS) Version 2.0 spec Section
12.2.3.2 Discard:
Since the TPRZ bit is set to zero if the discard functionality is enabled,
a READ command specifying a deallocated LBA may return any data.

And according to the UEFI 2.6 spec Section 12.12 in the 'Description' part
of the EFI_ERASE_BLOCK_PROTOCOL.EraseBlocks():
The EraseBlocks() function erases the requested number of device blocks.
Upon the successful execution of EraseBlocks() with an EFI_SUCCESS return
code, any subsequent reads of the same LBA range would return an
initialized/formatted value.

Since after the 'discard' operation, the device may return any data. My
concern is that the 'discard' operation does not match exactly with the
description of the behavior of the EraseBlocks() by the UEFI spec to
return an initialized/formatted value.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org]
> Sent: Saturday, April 15, 2017 9:45 PM
> To: Wu, Hao A; Tian, Feng; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org;
> edk2-devel@lists.01.org
> Cc: Haojian Zhuang
> Subject: [PATCH] MdeModulePkg: ScsiDiskDxe: fix to support
> EFI_ERASE_BLOCK_PROTOCOL
>
> If bit TPZ and bit TPRZ are set, the erase feature is implemented.
> If bit TPZ is set and bit TPRZ is clear, the discard feature is
> implemented. And discard is a non-secure variant of the erase
> functionality.
>
> So the detecting operation of EFI_ERASE_BLOCK_PROTOCOL, we should
> consider to support both functionality. Since discard functionality is
> only supported in some UFS devices.
>
> And both of these two features are relied on UNMAP command.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index b5eff25..6e12e4f 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -5400,11 +5400,11 @@ DetermineInstallEraseBlock (
>    if (CommandStatus == EFI_SUCCESS) {
>      //
>      // Universal Flash Storage (UFS) Version 2.0
> -    // Section 11.3.9.2
> +    // Section 11.3.9.2 & Section 12.2.3
>      // Bits TPE and TPRZ should both be set to enable the erase feature on UFS.
> +    // Setting bit TPE and clearing bit TPRZ to enable the discard feature on UFS.
>      //
> -    if (((CapacityData16->LowestAlignLogic2 & BIT7) == 0) ||
> -        ((CapacityData16->LowestAlignLogic2 & BIT6) == 0)) {
> +    if ((CapacityData16->LowestAlignLogic2 & BIT7) == 0) {
>        DEBUG ((
>          EFI_D_VERBOSE,
>          "ScsiDisk EraseBlock: Either TPE or TPRZ is not set: 0x%x.\n",
> --
> 2.7.4



  reply	other threads:[~2017-04-17 14:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-15 13:44 [PATCH] MdeModulePkg: ScsiDiskDxe: fix to support EFI_ERASE_BLOCK_PROTOCOL Haojian Zhuang
2017-04-17  1:58 ` Wu, Hao A
2017-04-17 14:03   ` Haojian Zhuang [this message]
2017-04-18 13:06     ` Wu, Hao A
2017-04-18 13:17       ` Haojian Zhuang
2017-04-18 15:03 ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CY1PR15MB0730427CE3801EE5769AC29997060@CY1PR15MB0730.namprd15.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox