On Mon, May 21, 2018 at 2:28 PM, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > On 21 May 2018 at 10:25, Thomas Abraham <thomas.abra...@arm.com> wrote: >> From: Daniil Egranov <daniil.egra...@arm.com> >> >> Add the registration of the virtio block device. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Daniil Egranov <daniil.egra...@arm.com> >> Signed-off-by: Thomas Abraham <thomas.abra...@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