[edk2-devel] [PATCH v5 09/12] OvmfPkg/MptScsiDxe: Set and restore PCI attributes

Nikita Leshenko posted 12 patches 5 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH v5 09/12] OvmfPkg/MptScsiDxe: Set and restore PCI attributes
Posted by Nikita Leshenko 5 years, 9 months ago
Enable the IO Space and Bus Mastering and restore the original values
when the device is stopped. This is a standard procedure in PCI
drivers.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/MptScsiDxe/MptScsi.c | 65 +++++++++++++++++++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
index ed7fabf8b471..e88ac2867a75 100644
--- a/OvmfPkg/MptScsiDxe/MptScsi.c
+++ b/OvmfPkg/MptScsiDxe/MptScsi.c
@@ -37,6 +37,7 @@ typedef struct {
   EFI_EXT_SCSI_PASS_THRU_MODE     PassThruMode;
   UINT8                           MaxTarget;
   EFI_PCI_IO_PROTOCOL             *PciIo;
+  UINT64                          OriginalPciAttributes;
 } MPT_SCSI_DEV;
 
 #define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
@@ -334,6 +335,53 @@ MptScsiControllerStart (
     goto FreePool;
   }
 
+  Status = Dev->PciIo->Attributes (
+                         Dev->PciIo,
+                         EfiPciIoAttributeOperationGet,
+                         0,
+                         &Dev->OriginalPciAttributes
+                         );
+  if (EFI_ERROR (Status)) {
+    goto CloseProtocol;
+  }
+
+  //
+  // Enable I/O Space & Bus-Mastering
+  //
+  Status = Dev->PciIo->Attributes (
+                         Dev->PciIo,
+                         EfiPciIoAttributeOperationEnable,
+                         (EFI_PCI_IO_ATTRIBUTE_IO |
+                          EFI_PCI_IO_ATTRIBUTE_BUS_MASTER),
+                         NULL
+                         );
+  if (EFI_ERROR (Status)) {
+    goto CloseProtocol;
+  }
+
+  //
+  // 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 that device will only be using 32-bit DMA addresses.
+    //
+    // Note that this does not prevent the 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__
+      ));
+  }
+
   //
   // Host adapter channel, doesn't exist
   //
@@ -358,11 +406,19 @@ MptScsiControllerStart (
                   &Dev->PassThru
                   );
   if (EFI_ERROR (Status)) {
-    goto CloseProtocol;
+    goto RestoreAttributes;
   }
 
   return EFI_SUCCESS;
 
+RestoreAttributes:
+  Dev->PciIo->Attributes (
+                Dev->PciIo,
+                EfiPciIoAttributeOperationSet,
+                Dev->OriginalPciAttributes,
+                NULL
+                );
+
 CloseProtocol:
   gBS->CloseProtocol (
          ControllerHandle,
@@ -414,6 +470,13 @@ MptScsiControllerStop (
     return Status;
   }
 
+  Dev->PciIo->Attributes (
+                Dev->PciIo,
+                EfiPciIoAttributeOperationSet,
+                Dev->OriginalPciAttributes,
+                NULL
+                );
+
   gBS->CloseProtocol (
          ControllerHandle,
          &gEfiPciIoProtocolGuid,
-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58085): https://edk2.groups.io/g/devel/message/58085
Mute This Topic: https://groups.io/mt/73247269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 09/12] OvmfPkg/MptScsiDxe: Set and restore PCI attributes
Posted by Laszlo Ersek 5 years, 9 months ago
On 04/24/20 19:59, Nikita Leshenko wrote:
> Enable the IO Space and Bus Mastering and restore the original values
> when the device is stopped. This is a standard procedure in PCI
> drivers.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Reviewed-by: Liran Alon <liran.alon@oracle.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/MptScsiDxe/MptScsi.c | 65 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 64 insertions(+), 1 deletion(-)

This patch has undergone (since v4) more changes than what I had tied my
R-b to, in the v4 review. So I think including the R-b upfront, in v5,
is not justified.

However, the updates all look good, in the end. Therefore:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


> 
> diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
> index ed7fabf8b471..e88ac2867a75 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsi.c
> +++ b/OvmfPkg/MptScsiDxe/MptScsi.c
> @@ -37,6 +37,7 @@ typedef struct {
>    EFI_EXT_SCSI_PASS_THRU_MODE     PassThruMode;
>    UINT8                           MaxTarget;
>    EFI_PCI_IO_PROTOCOL             *PciIo;
> +  UINT64                          OriginalPciAttributes;
>  } MPT_SCSI_DEV;
>  
>  #define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
> @@ -334,6 +335,53 @@ MptScsiControllerStart (
>      goto FreePool;
>    }
>  
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationGet,
> +                         0,
> +                         &Dev->OriginalPciAttributes
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    goto CloseProtocol;
> +  }
> +
> +  //
> +  // Enable I/O Space & Bus-Mastering
> +  //
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationEnable,
> +                         (EFI_PCI_IO_ATTRIBUTE_IO |
> +                          EFI_PCI_IO_ATTRIBUTE_BUS_MASTER),
> +                         NULL
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    goto CloseProtocol;
> +  }
> +
> +  //
> +  // 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 that device will only be using 32-bit DMA addresses.
> +    //
> +    // Note that this does not prevent the 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__
> +      ));
> +  }
> +
>    //
>    // Host adapter channel, doesn't exist
>    //
> @@ -358,11 +406,19 @@ MptScsiControllerStart (
>                    &Dev->PassThru
>                    );
>    if (EFI_ERROR (Status)) {
> -    goto CloseProtocol;
> +    goto RestoreAttributes;
>    }
>  
>    return EFI_SUCCESS;
>  
> +RestoreAttributes:
> +  Dev->PciIo->Attributes (
> +                Dev->PciIo,
> +                EfiPciIoAttributeOperationSet,
> +                Dev->OriginalPciAttributes,
> +                NULL
> +                );
> +
>  CloseProtocol:
>    gBS->CloseProtocol (
>           ControllerHandle,
> @@ -414,6 +470,13 @@ MptScsiControllerStop (
>      return Status;
>    }
>  
> +  Dev->PciIo->Attributes (
> +                Dev->PciIo,
> +                EfiPciIoAttributeOperationSet,
> +                Dev->OriginalPciAttributes,
> +                NULL
> +                );
> +
>    gBS->CloseProtocol (
>           ControllerHandle,
>           &gEfiPciIoProtocolGuid,
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58308): https://edk2.groups.io/g/devel/message/58308
Mute This Topic: https://groups.io/mt/73247269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-