Re: [edk2-devel] [PATCH 17/17] OvmfPkg/PvScsiDxe: Enable device 64-bit DMA addresses

["Laszlo Ersek" <lersek@redhat.com>]

On 03/16/20 16:01, Liran Alon wrote:
> Enable PCI dual-address cycle attribute to signal device supports
> 64-bit DMA addresses.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  OvmfPkg/PvScsiDxe/PvScsi.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index 33167c177b42..e673ed8ba6a0 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -814,6 +814,27 @@ PvScsiSetPCIAttributes (
>      return Status;
>    }
>  
> +  //
> +  // Signal device supports 64-bit DMA addresses
> +  //
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationEnable,
> +                         EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
> +                         NULL
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Warn user device will only be using 32-bit DMA addresses.
> +    //
> +    // Note that this does not prevent device/driver from working
> +    // and therefore we only warn and continue as usual.
> +    //
> +    DEBUG ((DEBUG_WARN,
> +            "%a: failed to enable 64-bit DMA addresses\n",
> +            __FUNCTION__));
> +  }
> +
>    return EFI_SUCCESS;
>  }
>  
> 

I've consulted the UEFI spec, and it indeed makes sense to attempt
enabling this attribute in a separate step. So, good idea.

Regarding the DEBUG message:

(1) The indentation of the 2nd and 3rd argument is incorrect. Please pick:

    DEBUG ((DEBUG_WARN, "%a: failed to enable 64-bit DMA addresses\n",
      __FUNCTION__));

or:

    DEBUG ((
      DEBUG_WARN,
      "%a: failed to enable 64-bit DMA addresses\n",
      __FUNCTION__
      ));

Thanks!
Laszlo