Re: [PATCH v2] MdeModulePkg/ScsiDiskDxe: Update proper device name for ScsiDisk drive

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

Thanks.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Will wait a couple of days before merging to see if comments from other reviewers.

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheripally
> Gopi via groups.io
> Sent: Thursday, October 27, 2022 3:07 PM
> To: devel@edk2.groups.io
> Cc: Selvaraj, Sundaresan <sundaresans@ami.com>; Sambandan, Vasudevan
> <vasudevans@ami.com>; Cheripally Gopi <gopic@ami.com>
> Subject: [edk2-devel] [PATCH v2] MdeModulePkg/ScsiDiskDxe: Update proper
> device name for ScsiDisk drive
> 
> ScsiDiskDxe driver updates ControllerNameTable with common string
> "SCSI Disk Device" for all SCSI disks. 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. Updated "ControllerNameTable" with
> Vendor and Product information. So that, device specific name can be
> retrieved using ComponentName protocol.
> 
> Cc: Vasudevan Sambandan <vasudevans@ami.com>
> Cc: Sundaresan Selvaraj <sundaresans@ami.com>
> Signed-off-by: Cheripally Gopi <gopic@ami.com>
> ---
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c  | 53 ++++++++++++++++++-
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h  |  9 ++++
>  .../Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf      |  2 +
>  3 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index 98e84b4ea8..0928605dc4 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2,6 +2,7 @@
>    SCSI disk driver that layers on every SCSI IO protocol in the system.
> 
> 
> 
>  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 1985 - 2022, American Megatrends International LLC.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -67,6 +68,33 @@ 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;
> 
> +
> 
> +  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 +231,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 +385,37 @@ ScsiDiskDriverBindingStart (
>            }
> 
>          }
> 
> 
> 
> +        CopyMem (
> 
> +          VendorStr,
> 
> +          &ScsiDiskDevice-
> >InquiryData.Reserved_5_95[VENDOR_IDENTIFICATION_OFFSET],
> 
> +          VENDOR_IDENTIFICATION_LENGTH
> 
> +          );
> 
> +        VendorStr[VENDOR_IDENTIFICATION_LENGTH] = 0;
> 
> +        RemoveTrailingSpaces (VendorStr);
> 
> +
> 
> +        CopyMem (
> 
> +          ProductStr,
> 
> +          &ScsiDiskDevice-
> >InquiryData.Reserved_5_95[PRODUCT_IDENTIFICATION_OFFSET],
> 
> +          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..5b4047e1db 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -2,6 +2,7 @@
>    Header file for SCSI Disk Driver.
> 
> 
> 
>  Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 1985 - 2022, American Megatrends International LLC.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -30,6 +31,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 +181,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..26227132e8 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> @@ -4,6 +4,7 @@
>  #  the device handle.
> 
>  #
> 
>  #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +#  Copyright (c) 1985 - 2022, American Megatrends International LLC.<BR>
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -46,6 +47,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.
> 
> 
> 
>