From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v1 20/21] ArmVirtualizationPkg/VirtFdtDxe: wire up XenBusDxe to "xen, xen" DT node Date: Fri, 23 Jan 2015 19:03:53 +0000 Message-ID: References: <1422025390-8036-1-git-send-email-ard.biesheuvel@linaro.org> <1422025390-8036-21-git-send-email-ard.biesheuvel@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1422025390-8036-21-git-send-email-ard.biesheuvel@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ard Biesheuvel Cc: ian.campbell@citrix.com, olivier.martin@arm.com, stefano.stabellini@eu.citrix.com, edk2-devel@lists.sourceforge.net, leif.lindholm@linaro.org, xen-devel@lists.xen.org, roy.franz@linaro.org, ilias.biris@linaro.org, anthony.perard@citrix.com, lersek@redhat.com, christoffer.dall@linaro.org List-Id: xen-devel@lists.xenproject.org On Fri, 23 Jan 2015, Ard Biesheuvel wrote: > This patchs adds support to VirtFdtDxe for the Xen DT node which > contains the base address of the Grant Table. This data is communicated > to XenBusDxe using a XENIO_PROTOCOL instance. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c | 96 +++++++++++++++++++--- > .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf | 2 + > 2 files changed, 86 insertions(+), 12 deletions(-) > > diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c > index 96aeec61ee7f..d8071f3e72aa 100644 > --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c > +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c > @@ -30,6 +30,9 @@ > #include > #include > #include > +#include > + > +#include > > #pragma pack (1) > typedef struct { > @@ -39,6 +42,13 @@ typedef struct { > } VIRTIO_TRANSPORT_DEVICE_PATH; > #pragma pack () > > +#pragma pack (1) > +typedef struct { > + VENDOR_DEVICE_PATH Vendor; > + EFI_DEVICE_PATH_PROTOCOL End; > +} XENBUS_ROOT_DEVICE_PATH; > +#pragma pack () > + > typedef enum { > PropertyTypeUnknown, > PropertyTypeGic, > @@ -49,6 +59,7 @@ typedef enum { > PropertyTypePsci, > PropertyTypeFwCfg, > PropertyTypeGicV3, > + PropertyTypeXen, > } PROPERTY_TYPE; > > typedef struct { > @@ -66,6 +77,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = { > { PropertyTypePsci, "arm,psci-0.2" }, > { PropertyTypeFwCfg, "qemu,fw-cfg-mmio" }, > { PropertyTypeGicV3, "arm,gic-v3" }, > + { PropertyTypeXen, "xen,xen" }, > { PropertyTypeUnknown, "" } > }; > > @@ -116,7 +128,7 @@ InitializeVirtFdtDxe ( > INT32 Len; > PROPERTY_TYPE PropType; > CONST VOID *RegProp; > - VIRTIO_TRANSPORT_DEVICE_PATH *DevicePath; > + VIRTIO_TRANSPORT_DEVICE_PATH *VirtIoDevicePath; Why are you renaming this variable as part of this patch? It makes reading this patch harder than necessary. If it is required, I would do it in a separate patch. At the very least it should be mentioned in the commit message. > EFI_HANDLE Handle; > UINT64 RegBase; > UINT64 DistBase, CpuBase; > @@ -127,6 +139,8 @@ InitializeVirtFdtDxe ( > UINT64 FwCfgSelectorSize; > UINT64 FwCfgDataAddress; > UINT64 FwCfgDataSize; > + XENIO_PROTOCOL *XenIo; > + XENBUS_ROOT_DEVICE_PATH *XenBusDevicePath; > > Hob = GetFirstGuidHob(&gFdtHobGuid); > if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof DeviceTreeBase) { > @@ -209,31 +223,31 @@ InitializeVirtFdtDxe ( > // Create a unique device path for this transport on the fly > // > RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > - DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode ( > + VirtIoDevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode ( > HARDWARE_DEVICE_PATH, > HW_VENDOR_DP, > sizeof (VIRTIO_TRANSPORT_DEVICE_PATH)); > - if (DevicePath == NULL) { > + if (VirtIoDevicePath == NULL) { > DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__)); > break; > } > > - CopyMem (&DevicePath->Vendor.Guid, &gVirtioMmioTransportGuid, > + CopyMem (&VirtIoDevicePath->Vendor.Guid, &gVirtioMmioTransportGuid, > sizeof (EFI_GUID)); > - DevicePath->PhysBase = RegBase; > - SetDevicePathNodeLength (&DevicePath->Vendor, > - sizeof (*DevicePath) - sizeof (DevicePath->End)); > - SetDevicePathEndNode (&DevicePath->End); > + VirtIoDevicePath->PhysBase = RegBase; > + SetDevicePathNodeLength (&VirtIoDevicePath->Vendor, > + sizeof (*VirtIoDevicePath) - sizeof (VirtIoDevicePath->End)); > + SetDevicePathEndNode (&VirtIoDevicePath->End); > > Handle = NULL; > Status = gBS->InstallProtocolInterface (&Handle, > &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > - DevicePath); > + VirtIoDevicePath); > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH " > "protocol on a new handle (Status == %r)\n", > __FUNCTION__, Status)); > - FreePool (DevicePath); > + FreePool (VirtIoDevicePath); > break; > } > > @@ -244,9 +258,9 @@ InitializeVirtFdtDxe ( > Handle, Status)); > > Status = gBS->UninstallProtocolInterface (Handle, > - &gEfiDevicePathProtocolGuid, DevicePath); > + &gEfiDevicePathProtocolGuid, VirtIoDevicePath); > ASSERT_EFI_ERROR (Status); > - FreePool (DevicePath); > + FreePool (VirtIoDevicePath); > } > break; > > @@ -332,6 +346,64 @@ InitializeVirtFdtDxe ( > } > break; > > + case PropertyTypeXen: > + ASSERT (Len == 16); > + > + // > + // Retrieve the reg base from this node and add it to a > + // XENIO_PROTOCOL instance installed on a new handle. > + // > + XenIo = AllocateZeroPool (sizeof *XenIo); > + ASSERT (XenIo != NULL); > + XenIo->GrantTableAddress = fdt64_to_cpu (((UINT64 *)RegProp)[0]); Shouldn't we read the event channel notification irq too? Or maybe Tianocore doesn't need to receive evtchn notifications? > + XenBusDevicePath = (XENBUS_ROOT_DEVICE_PATH *)CreateDeviceNode ( > + HARDWARE_DEVICE_PATH, > + HW_VENDOR_DP, > + sizeof (XENBUS_ROOT_DEVICE_PATH)); > + if (XenBusDevicePath == NULL) { > + DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__)); > + break; > + } > + > + CopyMem (&XenBusDevicePath->Vendor.Guid, &gXenBusRootDeviceGuid, > + sizeof (EFI_GUID)); > + SetDevicePathNodeLength (&XenBusDevicePath->Vendor, > + sizeof (*XenBusDevicePath) - sizeof (XenBusDevicePath->End)); > + SetDevicePathEndNode (&XenBusDevicePath->End); > + > + Handle = NULL; > + Status = gBS->InstallProtocolInterface (&Handle, > + &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > + XenBusDevicePath); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH " > + "protocol on a new handle (Status == %r)\n", > + __FUNCTION__, Status)); > + FreePool (XenBusDevicePath); > + break; > + } > + > + Status = gBS->InstallProtocolInterface (&Handle, > + &gXenIoProtocolGuid, EFI_NATIVE_INTERFACE, > + XenIo); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "%a: Failed to install XENIO_PROTOCOL on handle %p " > + "(Status == %r)\n", __FUNCTION__, Status)); > + > + Status = gBS->UninstallProtocolInterface (Handle, > + &gEfiDevicePathProtocolGuid, XenBusDevicePath); > + ASSERT_EFI_ERROR (Status); > + FreePool (XenBusDevicePath); > + FreePool (XenIo); > + break; > + } > + > + DEBUG ((EFI_D_INFO, "Found Xen node with Grant table @ 0x%p\n", > + XenIo->GrantTableAddress)); > + > + break; > + > default: > break; > } > diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf > index 1392c7c3fa45..01a154ef1b8a 100644 > --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf > +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf > @@ -46,6 +46,7 @@ > gFdtTableGuid > gVirtioMmioTransportGuid > gFdtHobGuid > + gXenBusRootDeviceGuid > > [Pcd] > gArmVirtualizationTokenSpaceGuid.PcdArmPsciMethod > @@ -61,6 +62,7 @@ > > [Protocols] > gEfiDevicePathProtocolGuid > + gXenIoProtocolGuid > > [Depex] > TRUE > -- > 1.8.3.2 >