Re: [PATCH v3 07/13] OvmfPkg/MptScsiDxe: Build DevicePath for discovered devices

[Liran Alon <liran.alon@oracle.com>]

On 04/03/2020 21:22, Nikita Leshenko wrote:
> Used to identify the individual disks in the hardware tree
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> ---
>   OvmfPkg/MptScsiDxe/MptScsi.c | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
> index e898a6024f73..cba564b5f648 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsi.c
> +++ b/OvmfPkg/MptScsiDxe/MptScsi.c
> @@ -129,7 +129,34 @@ MptScsiBuildDevicePath (
>     IN OUT EFI_DEVICE_PATH_PROTOCOL                  **DevicePath
>     )
>   {
> -  return EFI_UNSUPPORTED;
> +  SCSI_DEVICE_PATH *ScsiDevicePath;
> +
> +  if (DevicePath == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // This device support 256 targets only, so it's enough to dereference
> +  // the LSB of Target.
> +  //
> +  if (*Target > 0 || Lun > 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  ScsiDevicePath = AllocateZeroPool (sizeof (*ScsiDevicePath));

Nit: It should also be sufficient to use AllocatePool() as below 
overrides all fields of SCSI_DEVICE_PATH.

Reviewed-by: Liran Alon <liran.alon@oracle.com>

> +  if (ScsiDevicePath == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  ScsiDevicePath->Header.Type      = MESSAGING_DEVICE_PATH;
> +  ScsiDevicePath->Header.SubType   = MSG_SCSI_DP;
> +  ScsiDevicePath->Header.Length[0] = (UINT8)sizeof (*ScsiDevicePath);
> +  ScsiDevicePath->Header.Length[1] = (UINT8)(sizeof (*ScsiDevicePath) >> 8);
> +  ScsiDevicePath->Pun              = *Target;
> +  ScsiDevicePath->Lun              = (UINT16)Lun;
> +
> +  *DevicePath = &ScsiDevicePath->Header;
> +  return EFI_SUCCESS;
>   }
>   
>   STATIC