[edk2,v1,20/21] ArmVirtualizationPkg/VirtFdtDxe: wire up XenBusDxe to "xen, xen" DT node

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 <ard.biesheuvel@linaro.org>
---
 .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c   | 96 +++++++++++++++++++---
 .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf |  2 +
 2 files changed, 86 insertions(+), 12 deletions(-)

On 23 January 2015 at 19:03, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> 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 <ard.biesheuvel@linaro.org>
>> ---
>>  .../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 <Guid/Fdt.h>
>>  #include <Guid/VirtioMmioTransport.h>
>>  #include <Guid/FdtHob.h>
>> +#include <Guid/XenBusRootDevice.h>
>> +
>> +#include <Protocol/XenIo.h>
>>
>>  #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.

Not strictly necessary, I suppose. There is a second device path
pointer now with a different type, and I wanted to distinguish between
them.

> If it is required, I would do it in a separate patch. At the very least
> it should be mentioned in the commit message.
>

OK

>
>>    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?
>

Tianocore only uses interrupts for the timer, and polling for everything else.
The preexisting x86 code apparently works without an interrupt, so I
didn't give it any thought tbh

>
>> +      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
>>

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet