Re: [PATCH] MdeModulePkg: Improved ScsiDiskDxe driver updates ControllerNameTable with common string SCSI Disk Device for all SCSI disk

["Wu, Hao A" <hao.a.wu@intel.com>]

Thanks for the patch.
Could you help to do a favor and create a pull request for the proposed patch?

You can follow the steps 10 & 11 in the below wiki link to do so:
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-developer-process-for-the-edk-ii-project

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheripally
> Gopi via groups.io
> Sent: Monday, October 17, 2022 2:53 PM
> To: devel@edk2.groups.io
> Cc: Selvaraj, Sundaresan <sundaresans@ami.com>; Sambandan, Vasudevan
> <vasudevans@ami.com>; Sambandan, Vasudevan <vasudevans@ami.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH] MdeModulePkg: Improved ScsiDiskDxe driver
> updates ControllerNameTable with common string SCSI Disk Device for all SCSI
> disk
> 
> ScsiDiskDxe driver updates ControllerNameTable with common string SCSI Disk
> Device for all SCSI disk.due to this, when multiple SCSI disk devices connected,
> facing difficulty in identifying correct SCSI disk device.
> As per SCSI spec, standard Inquiry Data is having the fields to know Vendor and
> Product information.
> Update "ControllerNameTable" with Vendor and Product information. So that,
> device specific name can be retrieved using ComponentName protocol.
> 
> Signed-off-by:
> Cheripally Gopi <gopic@ami.com>
> 
> CC: Sundaresan S <sundaresans@ami.com>
> CC: Vasudevan S <vasudevans@ami.com>
> CC: Gaoliming <gaoliming@byosoft.com.cn>
> 
> 
> ---
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c  | 49 ++++++++++++++++++-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h  |  8 +++
>  .../Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf      |  1 +
>  3 files changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index 98e84b4ea8..3d05b01f8d 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -67,6 +67,32 @@ FreeAlignedBuffer (
>    }
> 
>  }
> 
> 
> 
> +/**
> 
> +  Remove trailing spaces from the string
> 
> +
> 
> +  @param String   The ASCII string to remove the trailing spaces
> 
> +
> 
> +  @retval the new length of the string
> 
> +**/
> 
> +UINTN
> 
> +RemoveTrailingSpaces (
> 
> +  IN OUT CHAR8   *String
> 
> +)
> 
> +{
> 
> +  UINTN  Length = AsciiStrLen (String);
> 
> +
> 
> +  if (Length == 0) {
> 
> +    return 0;
> 
> +  }
> 
> +
> 
> +  while ((Length > 0) && (String[Length-1] == ' ')) {
> 
> +    Length--;
> 
> +  }
> 
> +
> 
> +  String[Length] = 0;
> 
> +  return Length;
> 
> +}
> 
> +
> 
>  /**
> 
>    The user Entry Point for module ScsiDisk.
> 
> 
> 
> @@ -203,6 +229,9 @@ ScsiDiskDriverBindingStart (
>    UINT8                 MaxRetry;
> 
>    BOOLEAN               NeedRetry;
> 
>    BOOLEAN               MustReadCapacity;
> 
> +  CHAR8                 VendorStr[VENDOR_IDENTIFICATION_LENGTH + 1];
> 
> +  CHAR8                 ProductStr[PRODUCT_IDENTIFICATION_LENGTH + 1];
> 
> +  CHAR16                DeviceStr[VENDOR_IDENTIFICATION_LENGTH +
> PRODUCT_IDENTIFICATION_LENGTH + 2];
> 
> 
> 
>    MustReadCapacity = TRUE;
> 
> 
> 
> @@ -354,19 +383,35 @@ ScsiDiskDriverBindingStart (
>            }
> 
>          }
> 
> 
> 
> +        CopyMem (
> 
> +            VendorStr,
> 
> +
> + &ScsiDiskDevice-
> >InquiryData.Reserved_5_95[VENDOR_IDENTIFICATION_OFFSE
> + T],
> 
> +            VENDOR_IDENTIFICATION_LENGTH);
> 
> +        VendorStr[VENDOR_IDENTIFICATION_LENGTH] = 0;
> 
> +        RemoveTrailingSpaces (VendorStr);
> 
> +
> 
> +        CopyMem (
> 
> +            ProductStr,
> 
> +
> + &ScsiDiskDevice-
> >InquiryData.Reserved_5_95[PRODUCT_IDENTIFICATION_OFFS
> + ET],
> 
> +            PRODUCT_IDENTIFICATION_LENGTH);
> 
> +        ProductStr[PRODUCT_IDENTIFICATION_LENGTH] = 0;
> 
> +        RemoveTrailingSpaces (ProductStr);
> 
> +
> 
> +        UnicodeSPrint (DeviceStr, sizeof (DeviceStr), L"%a %a",
> + VendorStr, ProductStr);
> 
> +
> 
>          ScsiDiskDevice->ControllerNameTable = NULL;
> 
>          AddUnicodeString2 (
> 
>            "eng",
> 
>            gScsiDiskComponentName.SupportedLanguages,
> 
>            &ScsiDiskDevice->ControllerNameTable,
> 
> -          L"SCSI Disk Device",
> 
> +          DeviceStr,
> 
>            TRUE
> 
>            );
> 
>          AddUnicodeString2 (
> 
>            "en",
> 
>            gScsiDiskComponentName2.SupportedLanguages,
> 
>            &ScsiDiskDevice->ControllerNameTable,
> 
> -          L"SCSI Disk Device",
> 
> +          DeviceStr,
> 
>            FALSE
> 
>            );
> 
>          return EFI_SUCCESS;
> 
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> index d54282df5f..1a43c5030e 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
> <Library/UefiScsiLib.h>
> 
>  #include <Library/UefiBootServicesTableLib.h>
> 
>  #include <Library/DevicePathLib.h>
> 
> +#include <Library/PrintLib.h>
> 
> 
> 
>  #include <IndustryStandard/Scsi.h>
> 
>  #include <IndustryStandard/Atapi.h>
> 
> @@ -179,6 +180,13 @@ extern EFI_COMPONENT_NAME2_PROTOCOL
> gScsiDiskComponentName2;  #define SCSI_COMMAND_VERSION_2  0x02
> 
>  #define SCSI_COMMAND_VERSION_3  0x03
> 
> 
> 
> +// Per SCSI spec, EFI_SCSI_INQUIRY_DATA.Reserved_5_95[3 - 10] has the
> +Vendor identification
> 
> +// EFI_SCSI_INQUIRY_DATA.Reserved_5_95[11 - 26] has the product
> +identification
> 
> +#define VENDOR_IDENTIFICATION_OFFSET   3
> 
> +#define VENDOR_IDENTIFICATION_LENGTH   8
> 
> +#define PRODUCT_IDENTIFICATION_OFFSET  11
> 
> +#define PRODUCT_IDENTIFICATION_LENGTH  16
> 
> +
> 
>  //
> 
>  // SCSI Disk Timeout Experience Value
> 
>  //
> 
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> index 40818e669b..f03ba1b1ea 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> @@ -46,6 +46,7 @@
>    UefiDriverEntryPoint
> 
>    DebugLib
> 
>    DevicePathLib
> 
> +  PrintLib
> 
> 
> 
>  [Protocols]
> 
>    gEfiDiskInfoProtocolGuid                      ## BY_START
> 
> --
> 2.33.0.windows.2
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended to
> be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are on
> notice that any distribution of this message, in any form, is strictly prohibited.
> Please promptly notify the sender by reply e-mail or by telephone at 770-246-
> 8600, and then delete or destroy all copies of the transmission.
> 
> 
> 
>