On Tue, Jan 10, 2017 at 02:06:47PM +0800, Heyi Guo wrote:
> Hi Leif,
>
> That make sense, thanks.
>
> I refine the commit message  acorrding to your comment.

Much improved, thanks.
But a suggestion and a couple of requests for clarifications:

> That could be found in my branch
> ssh://git@git.linaro.org/people/heyi.guo/OpenPlatformPkg.git.
>
> Thanks and Regards.
>
>  Hisilicon D03/D05: add SCSI cdb16 command
>

Suggest add:
---
The D03/D05 SAS driver will fail to access disks beyond 2^32 sectors.
Symptoms can include the system entering grub rescue mode...
---

>     The system may enter grub rescure mode if the user use a HDD larger than
> 2TB.
>     The issue log like this:
>
>         error: file `/boot/grub/arm64-efi/normal.mod' not found.
>         Entering rescue mode...
>         grub rescue>
>
>     This is because the sas driver in uefi always use CDB10 SCSI command,
>     which is a 32 bit value, this command is used to read install data from
> sectors,
>     if the sector index higher than 4G, for such data read, the uefi sas

Perhaps better worded as 2^32 than 4G, to reduce risk of confusion.

> driver
>     previous will drop the higher bits.
>     The sector index is passed from LBA in Block IO protocol.
>     The sector size is 512 bytes, so when the HDD larger than 2TB will
>     trigger this issue.
>
>     This version we switch CDB16(64bit) mode for accessing disks of size
>     larger than 2TB.

Larger than 2TB (regardless of sector size) or larger than 2^32 sectors?

>
>     This commit resolves the issue
> https://bugs.linaro.org/show_bug.cgi?id=2393

While this is a clear fix, and should definitely go in - I still do
not understand how this can resolve #2393 as described.
The CDB16 support should make no difference between 1 or 2 drives
being connected, right?

Do you have any ideas?

Regards,

Leif

>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Chenhui Sun <sunchenhui@huawei.com>
>     Signed-off-by: Jason Zhang <zhangjinsong2@huawei.com>
>     Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
>
>
> 在 2017/1/10 1:11, Leif Lindholm 写道:
> >Hi Heyi,
> >
> >(You forgot to add me to cc on the original submission - quite likely
> >I would have missed this without the separate email.)
> >
> >On Mon, Jan 09, 2017 at 02:11:36PM +0800, Heyi Guo wrote:
> >>This patch resolves the issue 2393: https://bugs.linaro.org.
> >>The cdb16 command should be used if the hard disk size larger than 4TB.
> >Since I have no source code to reconciliate the commit message
> >against, the commit message needs to be more detailed than
> >this. It should contain the following:
> >
> >- The user-visible problem that was seen before.
> >- What type of internal failure caused that problem.
> >- Why this change resolves the issue.
> >
> >The Linaro bug is usful to include for reference, but it is not part
> >of the description - it should go last in the commit message.
> >
> >The summary (subject line) is fine and does not need changing.
> >
> >Regards,
> >
> >Leif
> >
> >>Contributed-under: TianoCore Contribution Agreement 1.0
> >>Signed-off-by: Chenhui Sun <sunchenhui@huawei.com>
> >>Signed-off-by: Jason Zhang <zhangjinsong2@huawei.com>
> >>---
> >>  .../Binary/D03/Drivers/Sas/SasDriverDxe.efi        | Bin 210752 -> 208288 bytes
> >>  .../Binary/D05/Drivers/Sas/SasDriverDxe.efi        | Bin 210400 -> 230912 bytes
> >>  2 files changed, 0 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/Platforms/Hisilicon/Binary/D03/Drivers/Sas/SasDriverDxe.efi b/Platforms/Hisilicon/Binary/D03/Drivers/Sas/SasDriverDxe.efi
> >>index b956b19..19cd038 100644
> >>Binary files a/Platforms/Hisilicon/Binary/D03/Drivers/Sas/SasDriverDxe.efi and b/Platforms/Hisilicon/Binary/D03/Drivers/Sas/SasDriverDxe.efi differ
> >>diff --git a/Platforms/Hisilicon/Binary/D05/Drivers/Sas/SasDriverDxe.efi b/Platforms/Hisilicon/Binary/D05/Drivers/Sas/SasDriverDxe.efi
> >>index 6021d12..8d9ecd2 100644
> >>Binary files a/Platforms/Hisilicon/Binary/D05/Drivers/Sas/SasDriverDxe.efi and b/Platforms/Hisilicon/Binary/D05/Drivers/Sas/SasDriverDxe.efi differ
> >>--
> >>1.9.1
> >>
> >>_______________________________________________
> >>Linaro-uefi mailing list
> >>Linaro-uefi@lists.linaro.org
> >>https://lists.linaro.org/mailman/listinfo/linaro-uefi
>