Re: [PATCH v3 06/13] OvmfPkg/MptScsiDxe: Report one Target and one LUN

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

On 04/03/2020 21:22, Nikita Leshenko wrote:
> Support for multiple targets will be implemented in a later commit in
> this series.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   OvmfPkg/MptScsiDxe/MptScsi.c      | 38 +++++++++++++++++++++++++++++--
>   OvmfPkg/MptScsiDxe/MptScsiDxe.inf |  1 +
>   2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
> index 9598b82fda53..e898a6024f73 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsi.c
> +++ b/OvmfPkg/MptScsiDxe/MptScsi.c
> @@ -17,6 +17,7 @@
>   
>   #include <IndustryStandard/FusionMptScsi.h>
>   #include <IndustryStandard/Pci.h>
> +#include <Library/BaseMemoryLib.h>
>   #include <Library/DebugLib.h>
>   #include <Library/MemoryAllocationLib.h>
>   #include <Library/UefiBootServicesTableLib.h>
> @@ -62,6 +63,22 @@ MptScsiPassThru (
>     return EFI_UNSUPPORTED;
>   }
>   
> +STATIC
> +BOOLEAN
> +IsTargetInitialized (
> +  IN UINT8                                          *Target
> +  )
> +{
> +  UINTN Idx;
> +
> +  for (Idx = 0; Idx < TARGET_MAX_BYTES; ++Idx) {
> +    if (Target[Idx] != 0xFF) {
> +      return TRUE;
> +    }
> +  }
> +  return FALSE;
> +}
> +
>   STATIC
>   EFI_STATUS
>   EFIAPI
> @@ -71,7 +88,16 @@ MptScsiGetNextTargetLun (
>     IN OUT UINT64                                     *Lun
>     )
>   {
> -  return EFI_UNSUPPORTED;
> +  //
> +  // Currently support only target 0 LUN 0, so hardcode it
> +  //
> +  if (!IsTargetInitialized (*Target)) {
> +    ZeroMem (*Target, TARGET_MAX_BYTES);
> +    *Lun = 0;
> +    return EFI_SUCCESS;
> +  } else {
> +    return EFI_NOT_FOUND;
> +  }

Nit: I prefer to remove the "else" clause and just write afterwards 
return EFI_NOT_FOUND. To make it clear that all code-flows have return 
statements.
In addition, the comment should be above "return EFI_NOT_FOUND;".

>   }
>   
>   STATIC
> @@ -82,7 +108,15 @@ MptScsiGetNextTarget (
>     IN OUT UINT8                                     **Target
>     )
>   {
> -  return EFI_UNSUPPORTED;
> +  //
> +  // Currently support only target 0 LUN 0, so hardcode it
> +  //
> +  if (!IsTargetInitialized (*Target)) {
> +    ZeroMem (*Target, TARGET_MAX_BYTES);
> +    return EFI_SUCCESS;
> +  } else {
> +    return EFI_NOT_FOUND;
> +  }
Same as above.
>   }
>   
>   STATIC
> diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> index a253c5d96916..8f366b92eb72 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> +++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> @@ -30,6 +30,7 @@ [Packages]
>     OvmfPkg/OvmfPkg.dec
>   
>   [LibraryClasses]
> +  BaseMemoryLib
>     DebugLib
>     MemoryAllocationLib
>     UefiBootServicesTableLib