From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=vijayenthiran.subramaniam@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 9DBA0211944A0 for ; Thu, 6 Dec 2018 21:50:00 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4B5015BF for ; Thu, 6 Dec 2018 21:49:59 -0800 (PST) Received: from mail-lj1-f180.google.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 647973F71D for ; Thu, 6 Dec 2018 21:49:59 -0800 (PST) Received: by mail-lj1-f180.google.com with SMTP id v15-v6so2464816ljh.13 for ; Thu, 06 Dec 2018 21:49:59 -0800 (PST) X-Gm-Message-State: AA+aEWZwVWhAYpR52I89m3vw/hZZ7524LqW13I3BO5uMGLeQyf5LY35u F8+D0ubaHXPAr49i/lvE622ym1fAzKRKNJ6C0zA= X-Google-Smtp-Source: AFSGD/XtKSlS9P3YXpznjKnYfdaMdvi/xicpGvE/4+Yf9YSeAHgbUpb1mjxVkInQwRueWhHdRSi4lEEX+/93+DG/s6E= X-Received: by 2002:a2e:9bc3:: with SMTP id w3-v6mr453193ljj.70.1544161797435; Thu, 06 Dec 2018 21:49:57 -0800 (PST) MIME-Version: 1.0 References: <1543914749-8252-1-git-send-email-vijayenthiran.subramaniam@arm.com> <1543914749-8252-2-git-send-email-vijayenthiran.subramaniam@arm.com> In-Reply-To: From: Vijayenthiran Subramaniam Date: Fri, 7 Dec 2018 05:49:19 +0000 X-Gmail-Original-Message-ID: Message-ID: To: Ard Biesheuvel Cc: edk2-devel@lists.01.org Subject: Re: [PATCH v1 edk2-platforms 1/8] Platform/ARM/SgiPkg: Restructure virtio device registration X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Dec 2018 05:50:00 -0000 Content-Type: text/plain; charset="UTF-8" On Thu, Dec 6, 2018 at 5:18 PM Ard Biesheuvel wrote: > > On Wed, 5 Dec 2018 at 07:10, Vijayenthiran Subramaniam > wrote: > > > > Hi Ard, > > > > The virtio block device and virtio network device are available in software model only. As of now, it exposes only one instance of each device. > > > > Are the virtio devices described by the device tree obtained from the > secure firmware? No. The device tree from secure firmware does not carry any devices information other than platform/config id. > > > On Tue, Dec 4, 2018 at 8:16 PM Ard Biesheuvel wrote: > >> > >> On Tue, 4 Dec 2018 at 10:12, Vijayenthiran Subramaniam > >> wrote: > >> > > >> > From: Daniil Egranov > >> > > >> > SGI platforms support multiple virtio devices. So the existing code, that > >> > supports registration of only the virtio disk, is restructured to > >> > accommodate the registration of additional virtio devices. > >> > > >> > In addition to this, PCDs to represent the virtio controller base and > >> > address space size are introduced. > >> > > >> > Contributed-under: TianoCore Contribution Agreement 1.1 > >> > Signed-off-by: Daniil Egranov > >> > --- > >> > Platform/ARM/SgiPkg/SgiPlatform.dec | 8 ++- > >> > Platform/ARM/SgiPkg/SgiPlatform.dsc | 7 +- > >> > Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 8 ++- > >> > Platform/ARM/SgiPkg/Include/Guid/SgiVirtioDevicesFormSet.h | 21 ++++++ > >> > Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 14 +--- > >> > Platform/ARM/SgiPkg/Drivers/PlatformDxe/{VirtioBlockIo.c => VirtioDevices.c} | 67 ++++++++++++-------- > >> > 6 files changed, 81 insertions(+), 44 deletions(-) > >> > > >> > >> Can these platforms only ever expose a single block device and a > >> single network device? > >> > >> > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec > >> > index f6e0ba1e927a..ed29a4d5d91f 100644 > >> > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec > >> > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec > >> > @@ -37,12 +37,16 @@ [Guids.common] > >> > gSgiClarkHeliosAcpiTablesFileGuid = { 0x2af40815, 0xa84e, 0x4de9, { 0x8c, 0x38, 0x91, 0x40, 0xb3, 0x54, 0x40, 0x73 } } > >> > > >> > [PcdsFeatureFlag.common] > >> > - # Set this PCD to TRUE to enable virtio support. > >> > - gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001 > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSupported|FALSE|BOOLEAN|0x00000001 > >> > > >> > [PcdsFixedAtBuild] > >> > gArmSgiTokenSpaceGuid.PcdDramBlock2Base|0|UINT64|0x00000002 > >> > gArmSgiTokenSpaceGuid.PcdDramBlock2Size|0|UINT64|0x00000003 > >> > > >> > + # Virtio Block device > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress|0x00000000|UINT32|0x00000004 > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSize|0x00000000|UINT32|0x00000005 > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt|0x00000000|UINT32|0x00000006 > >> > + > >> > [Ppis] > >> > gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } } > >> > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc > >> > index b3f76d2d9720..ada72be72f8a 100644 > >> > --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc > >> > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc > >> > @@ -98,7 +98,7 @@ [LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION, Libr > >> > ################################################################################ > >> > > >> > [PcdsFeatureFlag.common] > >> > - gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSupported|TRUE > >> > > >> > [PcdsFixedAtBuild.common] > >> > gArmTokenSpaceGuid.PcdVFPEnabled|1 > >> > @@ -180,6 +180,11 @@ [PcdsFixedAtBuild.common] > >> > # Ethernet > >> > gEmbeddedTokenSpaceGuid.PcdLan91xDxeBaseAddress|0x18000000 > >> > > >> > + # Virtio Disk > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress|0x1c130000 > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSize|0x10000 > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt|202 > >> > + > >> > ################################################################################ > >> > # > >> > # Components Section - list of all EDK II Modules needed by this Platform > >> > diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf > >> > index d903ed8d3375..f920f6ecafb8 100644 > >> > --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf > >> > +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf > >> > @@ -20,7 +20,7 @@ [Defines] > >> > > >> > [Sources.common] > >> > PlatformDxe.c > >> > - VirtioBlockIo.c > >> > + VirtioDevices.c > >> > > >> > [Packages] > >> > EmbeddedPkg/EmbeddedPkg.dec > >> > @@ -41,7 +41,11 @@ [Guids] > >> > gSgiClarkHeliosAcpiTablesFileGuid > >> > > >> > [FeaturePcd] > >> > - gArmSgiTokenSpaceGuid.PcdVirtioSupported > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSupported > >> > + > >> > +[FixedPcd] > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress > >> > + gArmSgiTokenSpaceGuid.PcdVirtioBlkSize > >> > > >> > [Depex] > >> > TRUE > >> > diff --git a/Platform/ARM/SgiPkg/Include/Guid/SgiVirtioDevicesFormSet.h b/Platform/ARM/SgiPkg/Include/Guid/SgiVirtioDevicesFormSet.h > >> > new file mode 100644 > >> > index 000000000000..80d3e3ae4f91 > >> > --- /dev/null > >> > +++ b/Platform/ARM/SgiPkg/Include/Guid/SgiVirtioDevicesFormSet.h > >> > @@ -0,0 +1,21 @@ > >> > +/** @file > >> > +* > >> > +* Copyright (c) 2018, ARM Limited. All rights reserved. > >> > +* > >> > +* 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. > >> > +* > >> > +**/ > >> > + > >> > +#ifndef __SGI_VIRTIO_DEVICES_FORMSET_H__ > >> > +#define __SGI_VIRTIO_DEVICES_FORMSET_H__ > >> > + > >> > +#define SGI_VIRTIO_BLOCK_GUID \ > >> > + { 0x5a96cdcd, 0x6116, 0x4929, { 0xb7, 0x01, 0x3a, 0xc2, 0xfb, 0x1c, 0xe2, 0x28 } } > >> > + > >> > +#endif > >> > diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c > >> > index a8708a5c4ce5..aa032c41f686 100644 > >> > --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c > >> > +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c > >> > @@ -17,9 +17,8 @@ > >> > #include > >> > #include > >> > > >> > -EFI_STATUS > >> > -InitVirtioBlockIo ( > >> > - IN EFI_HANDLE ImageHandle > >> > +VOID > >> > +InitVirtioDevices ( > >> > ); > >> > > >> > EFI_STATUS > >> > @@ -63,14 +62,7 @@ ArmSgiPkgEntryPoint ( > >> > return Status; > >> > } > >> > > >> > - Status = EFI_REQUEST_UNLOAD_IMAGE; > >> > - if (FeaturePcdGet (PcdVirtioSupported)) { > >> > - Status = InitVirtioBlockIo (ImageHandle); > >> > - if (EFI_ERROR (Status)) { > >> > - DEBUG ((DEBUG_ERROR, "%a: Failed to install Virtio Block device\n", > >> > - __FUNCTION__)); > >> > - } > >> > - } > >> > + InitVirtioDevices (); > >> > > >> > return Status; > >> > } > >> > diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c > >> > similarity index 41% > >> > rename from Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c > >> > rename to Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c > >> > index 5b5863f4b806..4703aec06968 100644 > >> > --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioBlockIo.c > >> > +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c > >> > @@ -12,6 +12,7 @@ > >> > > >> > **/ > >> > > >> > +#include > >> > #include > >> > #include > >> > #include > >> > @@ -20,12 +21,12 @@ > >> > > >> > #pragma pack (1) > >> > typedef struct { > >> > - VENDOR_DEVICE_PATH Vendor; > >> > + VENDOR_DEVICE_PATH VendorDevicePath; > >> > EFI_DEVICE_PATH_PROTOCOL End; > >> > -} VIRTIO_BLK_DEVICE_PATH; > >> > +} VIRTIO_DEVICE_PATH; > >> > #pragma pack () > >> > > >> > -STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath = > >> > +STATIC VIRTIO_DEVICE_PATH mVirtioBlockDevicePath = > >> > { > >> > { > >> > { > >> > @@ -36,7 +37,7 @@ STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath = > >> > (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8) > >> > } > >> > }, > >> > - EFI_CALLER_ID_GUID, > >> > + SGI_VIRTIO_BLOCK_GUID, > >> > }, > >> > { > >> > END_DEVICE_PATH_TYPE, > >> > @@ -49,33 +50,43 @@ STATIC VIRTIO_BLK_DEVICE_PATH mVirtioBlockDevicePath = > >> > }; > >> > > >> > /** > >> > - * Entrypoint for 'VirtioBlockIo' driver > >> > - */ > >> > -EFI_STATUS > >> > -InitVirtioBlockIo ( > >> > - IN EFI_HANDLE ImageHandle > >> > + Initialize platform Virtio devices. > >> > + > >> > + @return None. > >> > +**/ > >> > +VOID > >> > +InitVirtioDevices ( > >> > ) > >> > { > >> > EFI_STATUS Status; > >> > + STATIC EFI_HANDLE mVirtIoBlkController = NULL; > >> > > >> > - Status = gBS->InstallProtocolInterface (&ImageHandle, > >> > - &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > >> > - &mVirtioBlockDevicePath); > >> > - if (EFI_ERROR (Status)) { > >> > - DEBUG ((DEBUG_ERROR, "%a: Failed to install the EFI_DEVICE_PATH " > >> > - "protocol for the virtio block device (Status == %r)\n", > >> > - __FUNCTION__, Status)); > >> > - return Status; > >> > + // Install protocol interface for storage device > >> > + if ((FeaturePcdGet (PcdVirtioBlkSupported)) && > >> > + (FixedPcdGet32 (PcdVirtioBlkBaseAddress))) { > >> > + Status = gBS->InstallProtocolInterface (&mVirtIoBlkController, > >> > + &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > >> > + &mVirtioBlockDevicePath); > >> > + if (EFI_ERROR (Status)) { > >> > + DEBUG ((DEBUG_ERROR, "%a: Failed to install EFI_DEVICE_PATH protocol " > >> > + "for Virtio Block device (Status = %r)\n", > >> > + __FUNCTION__, Status)); > >> > + } else { > >> > + // Declare the Virtio BlockIo device > >> > + Status = VirtioMmioInstallDevice (FixedPcdGet32 (PcdVirtioBlkBaseAddress), > >> > + mVirtIoBlkController); > >> > + if (EFI_ERROR (Status)) { > >> > + DEBUG ((DEBUG_ERROR, "%a: Unable to find Virtio Block MMIO device " > >> > + "(Status == %r)\n", __FUNCTION__, Status)); > >> > + gBS->UninstallProtocolInterface ( > >> > + mVirtIoBlkController, > >> > + &gEfiDevicePathProtocolGuid, > >> > + &mVirtioBlockDevicePath > >> > + ); > >> > + } else { > >> > + DEBUG ((DEBUG_INIT, "%a: Installed Virtio Block device\n", > >> > + __FUNCTION__)); > >> > + } > >> > + } > >> > } > >> > - > >> > - Status = VirtioMmioInstallDevice (SGI_EXP_SYSPH_VIRTIO_BLOCK_BASE, ImageHandle); > >> > - if (EFI_ERROR (Status)) { > >> > - DEBUG ((DEBUG_ERROR, > >> > - "%a: Failed to install Virtio block device (Status == %r)\n", > >> > - __FUNCTION__, Status)); > >> > - gBS->UninstallProtocolInterface (ImageHandle, > >> > - &gEfiDevicePathProtocolGuid, &mVirtioBlockDevicePath); > >> > - } > >> > - > >> > - 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