| Message ID | 1422299011-2409-29-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ard, On 26/01/15 19:03, Ard Biesheuvel wrote: > typedef struct { > @@ -66,6 +68,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = { > { PropertyTypePsci, "arm,psci-0.2" }, > { PropertyTypeFwCfg, "qemu,fw-cfg-mmio" }, > { PropertyTypeGicV3, "arm,gic-v3" }, > + { PropertyTypeXen, "xen,xen" }, > { PropertyTypeUnknown, "" } > }; > > @@ -332,6 +335,26 @@ InitializeVirtFdtDxe ( > } > break; > > + case PropertyTypeXen: > + ASSERT (Len == 16); > + I would not assume that the reg property is always 16 bytes (8 bytes for the address and 8 bytes for the size). We may decide to change it in the future. That's why #address-cells and #size-cells exist in the DT. But it looks like that the other part of the code in this function always assume a fixed length. I guess we could live with it. I would add a comment explaining this restriction. Regards,
On 27 January 2015 at 11:57, Julien Grall <julien.grall@linaro.org> wrote: > Hi Ard, > > On 26/01/15 19:03, Ard Biesheuvel wrote: >> typedef struct { >> @@ -66,6 +68,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = { >> { PropertyTypePsci, "arm,psci-0.2" }, >> { PropertyTypeFwCfg, "qemu,fw-cfg-mmio" }, >> { PropertyTypeGicV3, "arm,gic-v3" }, >> + { PropertyTypeXen, "xen,xen" }, >> { PropertyTypeUnknown, "" } >> }; >> >> @@ -332,6 +335,26 @@ InitializeVirtFdtDxe ( >> } >> break; >> >> + case PropertyTypeXen: >> + ASSERT (Len == 16); >> + > > I would not assume that the reg property is always 16 bytes (8 bytes for > the address and 8 bytes for the size). We may decide to change it in the > future. That's why #address-cells and #size-cells exist in the DT. > Yes, you are quite correct. However, this code was originally created as a counterpart to QEMU/mach-virt, which is known to use 64-bit quantities for memory ranges, and adding variable address size support to it implies that we need to start caring about how the nodes are nested, which we currently don't. (#address-cells and #size-cells properties are inherited by child nodes) For Xen on arm64, #address-cells = 2 and #size-cells = 2 is the only meaningful option anyway, but I agree that blindly assuming it is not the most elegant approach. > But it looks like that the other part of the code in this function > always assume a fixed length. I guess we could live with it. I would add > a comment explaining this restriction. > Indeed.
diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c index 34fac40fa803..1ceb85146430 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c @@ -26,6 +26,7 @@ #include <Library/DxeServicesLib.h> #include <Library/HobLib.h> #include <libfdt.h> +#include <Library/XenIoMmioLib.h> #include <Guid/Fdt.h> #include <Guid/VirtioMmioTransport.h> @@ -49,6 +50,7 @@ typedef enum { PropertyTypePsci, PropertyTypeFwCfg, PropertyTypeGicV3, + PropertyTypeXen, } PROPERTY_TYPE; typedef struct { @@ -66,6 +68,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = { { PropertyTypePsci, "arm,psci-0.2" }, { PropertyTypeFwCfg, "qemu,fw-cfg-mmio" }, { PropertyTypeGicV3, "arm,gic-v3" }, + { PropertyTypeXen, "xen,xen" }, { PropertyTypeUnknown, "" } }; @@ -332,6 +335,26 @@ 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. + // + RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); + Handle = NULL; + Status = XenIoMmioInstall (&Handle, RegBase); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_ERROR, "%a: Failed to install XENIO_PROTOCOL on a new handle " + "(Status == %r)\n", __FUNCTION__, Status)); + break; + } + + DEBUG ((EFI_D_INFO, "Found Xen node with Grant table @ 0x%p\n", RegBase)); + + break; + default: break; } diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf index 1392c7c3fa45..f8a58238c37b 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf @@ -41,6 +41,7 @@ FdtLib VirtioMmioDeviceLib HobLib + XenIoMmioLib [Guids] gFdtTableGuid
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> --- ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c | 23 +++++++++++++++++++++++ ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf | 1 + 2 files changed, 24 insertions(+)