Re: [edk2-devel] [PATCH 10/17] OvmfPkg/PvScsiDxe: Enable IOSpace & Bus-Mastering in PCI attributes

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

Okay I can make a quick comment on this patch too, and on patch#17 too.

(And I appreciate *very much* that you have broken up this work into
small patches like this, because it allows me to do a thorough review
with reasonable effort!)

On 03/16/20 16:01, Liran Alon wrote:
> Enable IOSpace & Bus-Mastering PCI attributes when device is started.
> Note that original PCI attributes is restored when device is stopped.

(1) typo: s/attributes is/attributes are/

> 
> 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 | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index 92e0f4a98965..ff6b50b7020f 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -309,6 +309,20 @@ PvScsiSetPCIAttributes (
>      return Status;
>    }
>  
> +  //
> +  // Enable IOSpace & Bus-Mastering
> +  //
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationEnable,
> +                         (EFI_PCI_IO_ATTRIBUTE_MEMORY |
> +                          EFI_PCI_IO_ATTRIBUTE_BUS_MASTER),
> +                         NULL
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
>    return EFI_SUCCESS;
>  }
>  
> 

(2) The patch looks OK, but the "IOSpace" comment is wrong. "IOSpace"
would be appropriate for EFI_PCI_IO_ATTRIBUTE_IO (IO ports). Please say
"MMIO space" in the comment.

Thanks
Laszlo