1. Patchew
  2. EDK2
  3. [edk2] [PATCH edk2-platforms v4 0/9] Platform/ARM/Sgi: Add Arm's SGI platform support
  4. View patch

[edk2] [PATCH edk2-platforms v4 4/9] Platform/ARM/Sgi: add support for virtio block device

Thomas Abraham posted 9 patches 7 years, 8 months ago
There is a newer version of this series
[edk2] [PATCH edk2-platforms v4 4/9] Platform/ARM/Sgi: add support for virtio block device
Posted by Thomas Abraham 7 years, 8 months ago 
From: Daniil Egranov <daniil.egranov@arm.com>

Add the registration of the virtio block device.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
Signed-off-by: Thomas Abraham <thomas.abraham@arm.com>
---
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 18 ++++-
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  1 +
 .../ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c | 76 ++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
index eb26fde..fb1e390 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -15,11 +15,27 @@
 #include <Library/DebugLib.h>
 
 EFI_STATUS
+InitVirtioBlockIo (
+  IN EFI_HANDLE         ImageHandle
+);
+
+EFI_STATUS
 EFIAPI
 ArmSgiPkgEntryPoint (
   IN EFI_HANDLE         ImageHandle,
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
-  return EFI_SUCCESS;
+  EFI_STATUS              Status;
+
+  // Install Virtio Block IO.
+  if (FeaturePcdGet (PcdVirtioSupported) == TRUE) {
+    Status = InitVirtioBlockIo (ImageHandle);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio Block IO\n"));
+      return Status;
+    }
+  }
+
+  return Status;
 }
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
index dbe04c5..995f80d 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -20,6 +20,7 @@
 
 [Sources.common]
   PlatformDxe.c
+  VirtioBlockIo.c
 
 [Packages]
   ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
new file mode 100644
index 0000000..58dfd22
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
@@ -0,0 +1,76 @@
+/** @file
+
+  Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/VirtioMmioDeviceLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <SgiPlatform.h>
+
+#pragma pack (1)
+typedef struct {
+  VENDOR_DEVICE_PATH                  Vendor;
+  EFI_DEVICE_PATH_PROTOCOL            End;
+} VIRTIO_BLK_DEVICE_PATH;
+#pragma pack ()
+
+STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath =
+{
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8)( sizeof (VENDOR_DEVICE_PATH) ),
+        (UINT8)( (sizeof (VENDOR_DEVICE_PATH) ) >> 8)
+      }
+    },
+    EFI_CALLER_ID_GUID,
+  },
+  {
+    END_DEVICE_PATH_TYPE,
+    END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    {
+      sizeof (EFI_DEVICE_PATH_PROTOCOL),
+      0
+    }
+  }
+};
+
+/**
+ * Entrypoint for 'VirtioBlockIo' driver
+ */
+EFI_STATUS
+InitVirtioBlockIo (
+   IN EFI_HANDLE         ImageHandle
+  )
+{
+  EFI_STATUS Status = 0;
+
+  Status = gBS->InstallProtocolInterface (&ImageHandle,
+               &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
+               &mVirtioBlockDevicePath);
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Declare the Virtio BlockIo device
+  Status = VirtioMmioInstallDevice (SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE, ImageHandle);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio block device\n"));
+  }
+
+  return Status;
+}
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v4 4/9] Platform/ARM/Sgi: add support for virtio block device
Posted by Ard Biesheuvel 7 years, 8 months ago 
On 21 May 2018 at 10:25, Thomas Abraham <thomas.abraham@arm.com> wrote:
> From: Daniil Egranov <daniil.egranov@arm.com>
>
> Add the registration of the virtio block device.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@arm.com>
> ---
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 18 ++++-
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  1 +
>  .../ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c | 76 ++++++++++++++++++++++
>  3 files changed, 94 insertions(+), 1 deletion(-)
>  create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index eb26fde..fb1e390 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -15,11 +15,27 @@
>  #include <Library/DebugLib.h>
>
>  EFI_STATUS
> +InitVirtioBlockIo (
> +  IN EFI_HANDLE         ImageHandle
> +);
> +
> +EFI_STATUS
>  EFIAPI
>  ArmSgiPkgEntryPoint (
>    IN EFI_HANDLE         ImageHandle,
>    IN EFI_SYSTEM_TABLE   *SystemTable
>    )
>  {
> -  return EFI_SUCCESS;
> +  EFI_STATUS              Status;
> +
> +  // Install Virtio Block IO.
> +  if (FeaturePcdGet (PcdVirtioSupported) == TRUE) {

No need to compare booleans with TRUE or FALSE.

> +    Status = InitVirtioBlockIo (ImageHandle);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio Block IO\n"));
> +      return Status;
> +    }
> +  }
> +
> +  return Status;
>  }
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index dbe04c5..995f80d 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -20,6 +20,7 @@
>
>  [Sources.common]
>    PlatformDxe.c
> +  VirtioBlockIo.c
>
>  [Packages]
>    ArmPkg/ArmPkg.dec
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
> new file mode 100644
> index 0000000..58dfd22
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
> @@ -0,0 +1,76 @@
> +/** @file
> +
> +  Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/VirtioMmioDeviceLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <SgiPlatform.h>
> +
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH                  Vendor;
> +  EFI_DEVICE_PATH_PROTOCOL            End;
> +} VIRTIO_BLK_DEVICE_PATH;
> +#pragma pack ()
> +
> +STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath =
> +{
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_VENDOR_DP,
> +      {
> +        (UINT8)( sizeof (VENDOR_DEVICE_PATH) ),
> +        (UINT8)( (sizeof (VENDOR_DEVICE_PATH) ) >> 8)

No spaces after ( or before ) please

> +      }
> +    },
> +    EFI_CALLER_ID_GUID,
> +  },
> +  {
> +    END_DEVICE_PATH_TYPE,
> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    {
> +      sizeof (EFI_DEVICE_PATH_PROTOCOL),
> +      0
> +    }
> +  }
> +};
> +
> +/**
> + * Entrypoint for 'VirtioBlockIo' driver
> + */
> +EFI_STATUS
> +InitVirtioBlockIo (
> +   IN EFI_HANDLE         ImageHandle
> +  )
> +{
> +  EFI_STATUS Status = 0;
> +
> +  Status = gBS->InstallProtocolInterface (&ImageHandle,
> +               &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
> +               &mVirtioBlockDevicePath);
> +

Please use correct indentation.

> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  // Declare the Virtio BlockIo device
> +  Status = VirtioMmioInstallDevice (SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE, ImageHandle);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio block device\n"));
> +  }
> +

Please handle this error correctly. If the call to
VirtioMmioInstallDevice() fails, you will exit this function and hence
the DXE entry point with an error, which will result in the driver to
be unloaded. However, you just installed gEfiDevicePathProtocolGuid
with references to the driver's code, and attempts to invoke those
will result in a crash after this.

> +  return Status;
> +}
> --
> 2.7.4
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v4 4/9] Platform/ARM/Sgi: add support for virtio block device
Posted by Thomas Abraham 7 years, 8 months ago 
On Mon, May 21, 2018 at 2:28 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 21 May 2018 at 10:25, Thomas Abraham <thomas.abraham@arm.com> wrote:
>> From: Daniil Egranov <daniil.egranov@arm.com>
>>
>> Add the registration of the virtio block device.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@arm.com>
>> ---
>>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 18 ++++-
>>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  1 +
>>  .../ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c | 76 ++++++++++++++++++++++
>>  3 files changed, 94 insertions(+), 1 deletion(-)
>>  create mode 100644 Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
>>
>> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
>> index eb26fde..fb1e390 100644
>> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
>> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
>> @@ -15,11 +15,27 @@
>>  #include <Library/DebugLib.h>
>>
>>  EFI_STATUS
>> +InitVirtioBlockIo (
>> +  IN EFI_HANDLE         ImageHandle
>> +);
>> +
>> +EFI_STATUS
>>  EFIAPI
>>  ArmSgiPkgEntryPoint (
>>    IN EFI_HANDLE         ImageHandle,
>>    IN EFI_SYSTEM_TABLE   *SystemTable
>>    )
>>  {
>> -  return EFI_SUCCESS;
>> +  EFI_STATUS              Status;
>> +
>> +  // Install Virtio Block IO.
>> +  if (FeaturePcdGet (PcdVirtioSupported) == TRUE) {
>
> No need to compare booleans with TRUE or FALSE.
>
>> +    Status = InitVirtioBlockIo (ImageHandle);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio Block IO\n"));
>> +      return Status;
>> +    }
>> +  }
>> +
>> +  return Status;
>>  }
>> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>> index dbe04c5..995f80d 100644
>> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>> @@ -20,6 +20,7 @@
>>
>>  [Sources.common]
>>    PlatformDxe.c
>> +  VirtioBlockIo.c
>>
>>  [Packages]
>>    ArmPkg/ArmPkg.dec
>> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
>> new file mode 100644
>> index 0000000..58dfd22
>> --- /dev/null
>> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c
>> @@ -0,0 +1,76 @@
>> +/** @file
>> +
>> +  Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License which accompanies this
>> +  distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <Library/VirtioMmioDeviceLib.h>
>> +#include <Library/DevicePathLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <SgiPlatform.h>
>> +
>> +#pragma pack (1)
>> +typedef struct {
>> +  VENDOR_DEVICE_PATH                  Vendor;
>> +  EFI_DEVICE_PATH_PROTOCOL            End;
>> +} VIRTIO_BLK_DEVICE_PATH;
>> +#pragma pack ()
>> +
>> +STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath =
>> +{
>> +  {
>> +    {
>> +      HARDWARE_DEVICE_PATH,
>> +      HW_VENDOR_DP,
>> +      {
>> +        (UINT8)( sizeof (VENDOR_DEVICE_PATH) ),
>> +        (UINT8)( (sizeof (VENDOR_DEVICE_PATH) ) >> 8)
>
> No spaces after ( or before ) please
>
>> +      }
>> +    },
>> +    EFI_CALLER_ID_GUID,
>> +  },
>> +  {
>> +    END_DEVICE_PATH_TYPE,
>> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
>> +    {
>> +      sizeof (EFI_DEVICE_PATH_PROTOCOL),
>> +      0
>> +    }
>> +  }
>> +};
>> +
>> +/**
>> + * Entrypoint for 'VirtioBlockIo' driver
>> + */
>> +EFI_STATUS
>> +InitVirtioBlockIo (
>> +   IN EFI_HANDLE         ImageHandle
>> +  )
>> +{
>> +  EFI_STATUS Status = 0;
>> +
>> +  Status = gBS->InstallProtocolInterface (&ImageHandle,
>> +               &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
>> +               &mVirtioBlockDevicePath);
>> +
>
> Please use correct indentation.
>
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  // Declare the Virtio BlockIo device
>> +  Status = VirtioMmioInstallDevice (SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE, ImageHandle);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "PlatformDxe: Failed to install Virtio block device\n"));
>> +  }
>> +
>
> Please handle this error correctly. If the call to
> VirtioMmioInstallDevice() fails, you will exit this function and hence
> the DXE entry point with an error, which will result in the driver to
> be unloaded. However, you just installed gEfiDevicePathProtocolGuid
> with references to the driver's code, and attempts to invoke those
> will result in a crash after this.

Okay, thanks for pointing this out. This will be fixed in the next version.

Thanks,
Thomas.

>
>> +  return Status;
>> +}
>> --
>> 2.7.4
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.