Some comments below On 08/27/14 17:12, Ard Biesheuvel wrote: > This adds support for executing UEFI in a QEMU/mach-virt emulated environment. > The following assumptions are made about the target: > - ARM architected timer > - PL011 UART at 0x9000000 > - at least 1 MB of DRAM at 0x40000000, containing the device tree blob > > The following information is retrieved from the device tree: > - GIC base addresses > - virtual timer interrupt > - RTC base address > - system memory base and size > - virtio MMIO transports > > The DTB image is relocated and installed as a configuration table so an > EFI stub enabled kernel can be booted directly without the need for a > bootloader. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Michael Casadevall <[email protected]> > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > .../AArch64Virtualization-KVM.dsc | 222 ++++++++++++++ > .../AArch64Virtualization-KVM.fdf | 307 +++++++++++++++++++ > .../AArch64Virtualization.dsc.inc | 336 > +++++++++++++++++++++ > .../AArch64VirtualizationPkg/Driver/VirtFdt.inf | 61 ++++ > .../AArch64VirtualizationPkg/Driver/VirtFdtDxe.c | 249 +++++++++++++++ > .../Include/Platform/KVM/ArmPlatform.h | 27 ++ > .../AArch64VirtualizationLibKVM/AArch64KVMLib.inf | 57 ++++ > .../Library/AArch64VirtualizationLibKVM/KVM.c | 143 +++++++++ > .../AArch64VirtualizationLibKVM/KVMHelper.S | 86 ++++++ > .../Library/AArch64VirtualizationLibKVM/KVMMem.c | 102 +++++++ > .../AArch64VirtualizationSysConfigLibKVM.c | 95 ++++++ > .../AArch64VirtualizationSysConfigLibKVM.inf | 35 +++ > .../Library/PlatformPei/PlatformPeiLib.c | 47 +++ > .../Library/PlatformPei/PlatformPeiLib.inf | 58 ++++ > .../Library/ResetSystemLib/ResetSystemLib.c | 97 ++++++ > .../Library/ResetSystemLib/ResetSystemLib.inf | 36 +++ > .../Library/ResetSystemLib/ResetSystemPsci.S | 40 +++ > 17 files changed, 1998 insertions(+)
First of all, this ginormous patch should be split up in several smaller ones. I apologize for requesting this only now, at v3. edk2 is very modular, and this patch dumps 5 (FIVE) modules at once at the poor reviewer. It is impenetrable, and the granularity of any Acked-by or Reviewed-by is very bad -- you can't carry an A-b or an R-b forward that has been given only for one module among the five. So let's see: > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.fdf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization.dsc.inc Please include these in a zeroth patch. It's not a requirement that AArch64VirtualizationPkg be buildable at each stage, during bisection -- for example, it's not buildable before you introduce it *at all*. If it isn't too hard, please shave these off to a separate patch. > create mode 100644 ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdt.inf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c This should be module #1. > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Include/Platform/KVM/ArmPlatform.h Yikes. After a while I managed to understand the dependency that this header file resolves. Please make it a separate patch. Can be an early one. > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/AArch64KVMLib.inf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVM.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVMHelper.S > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVMMem.c Module #2. > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationSysConfigLibKVM/AArch64VirtualizationSysConfigLibKVM.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationSysConfigLibKVM/AArch64VirtualizationSysConfigLibKVM.inf Module #3. > create mode 100755 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.c > create mode 100755 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.inf Module #4. > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.inf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemPsci.S Module #5. If I'm counting right, that's 7 new patches, in place of this huge one. Five module patches, one patch for the header file, and one patch for the DSC / FDF / etc meta files. The module patches should be ordered logically, if such a logical ordering exists. Right now I'll try to review "Library/PlatformPei" and "Driver/". Some of the modules added here I probably won't ever review (due to lack of architectural knowledge). > diff --git > a/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.c > > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.c Directories holding library instances should have basename WhateverLib. Hence, "Library/PlatformPeiLib". > new file mode 100755 > index 000000000000..bb873f04e46d > --- /dev/null > +++ > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.c > @@ -0,0 +1,47 @@ > +/** @file > +* > +* Copyright (c) 2011-2012, ARM Limited. All rights reserved. > +* Copyright (c) 2014, Linaro 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. > +* > +**/ > + > +#include <PiPei.h> > + > +#include <Library/MemoryAllocationLib.h> > +#include <Library/ArmPlatformLib.h> > +#include <Library/DebugLib.h> > +#include <Library/HobLib.h> > +#include <Library/PcdLib.h> > +#include <libfdt.h> > + > +EFI_STATUS > +EFIAPI > +PlatformPeim ( > + VOID > + ) > +{ > + UINT64 Base; > + VOID *NewBase; The asterisk should line up with "Base" above. Don't ask. :) > + UINTN FdtSize; > + > + BuildFvHob (PcdGet32(PcdFvBaseAddress), PcdGet32(PcdFvSize)); Okay, this has been preserved from the thus far only PlatformPeim() implementation, in "ArmPlatformPkg/PlatformPei/PlatformPeiLib.c". Good. > + > + Base = PcdGet64 (PcdDeviceTreeBaseAddress); OK, I can see this dynamic PCD gets 0x40000000 as default value in the DSC. > + FdtSize = fdt_totalsize ((VOID *)(UINTN)Base); > + > + NewBase = AllocatePages (EFI_SIZE_TO_PAGES (FdtSize)); > + ASSERT (NewBase != NULL); > + > + CopyMem (NewBase, (VOID *)(UINTN)Base, FdtSize); > + PcdSet64 (PcdDeviceTreeBaseAddress, (UINT64)NewBase); > + > + return EFI_SUCCESS; > +} Looks okay to me. Please consider logging an EFI_D_INFO message here, so that we can feel warm and fuzzy about the DTB's copy being in permanent PEI RAM. Something like: DEBUG ((EFI_D_INFO, "%a: copied DTB of size 0x%Lx to address %p\n", __FUNCTION__, (UINT64)FdtSize, NewBase)); You can ignore this if you want to. > diff --git > a/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.inf > > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.inf > new file mode 100755 > index 000000000000..b1e8bc986efc > --- /dev/null > +++ > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/PlatformPei/PlatformPeiLib.inf > @@ -0,0 +1,58 @@ > +#/** @file > +# > +# Copyright (c) 2011-2012, 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. > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = ArmPlatformPeiLib This should say "PlatformPeiLib" (after you rename the containing directory to PlatformPeiLib). > + FILE_GUID = 49d37060-70b5-11e0-aa2d-0002a5d5c51b Please don't forget to update the FILE_GUID with "uuidgen" each time you introduce a new INF file. The GUID visible above is from "ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf". > + MODULE_TYPE = SEC > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = PlatformPeiLib > + > +[Sources] > + PlatformPeiLib.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec You can most likely drop a few false package dependencies here, but it's not a requirement. > + > +[LibraryClasses] > + DebugLib > + HobLib > + ArmPlatformLib > + FdtLib MemoryAllocationLib is missing (probably inherited anyway through one of these); ArmPlatformLib and HobLib look superfluous. Saying just for pedantry. > + > +[Ppis] > + gEfiPeiMasterBootModePpiGuid # PPI ALWAYS_PRODUCED > + gEfiPeiBootInRecoveryModePpiGuid # PPI SOMETIMES_PRODUCED Ugh, no, I don't think so. The *library* doesn't produce these. > + > +[FixedPcd] > + gArmTokenSpaceGuid.PcdFdBaseAddress > + gArmTokenSpaceGuid.PcdFdSize > + > + gArmTokenSpaceGuid.PcdFvBaseAddress > + gArmTokenSpaceGuid.PcdFvSize > + > + gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize > + gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize > + All of these should be removed. > +[Pcd] > + gArmTokenSpaceGuid.PcdDeviceTreeBaseAddress Yes, we need this. > + > +[depex] > + gEfiPeiMemoryDiscoveredPpiGuid > + Nice! You didn't forget about this dependency. The AllocatePages() call in the C file *must* be served from permanent PEI RAM. Good. > diff --git a/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdt.inf > b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdt.inf > new file mode 100644 > index 000000000000..464829607871 > --- /dev/null > +++ b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdt.inf > @@ -0,0 +1,61 @@ > +## @file > +# Device tree enumeration DXE driver for AArch64 VMs The AArch64 reference should probably be generalized. > +# > +# Copyright (c) 2014, Linaro 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. > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = VirtFdt > + FILE_GUID = 837DCA9E-E874-4D82-B29A-23FE0E23D1E2 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + > + ENTRY_POINT = InitializeVirtFdtDxe > + > +[Sources] > + VirtFdtDxe.c > + > +[Packages] > + MdePkg/MdePkg.dec > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + BaseLib > + PcdLib > + UefiDriverEntryPoint > + DxeServicesLib > + FdtLib > + VirtioMmioDeviceLib > + > +[Guids] > + gFdtTableGuid > + > +[FeaturePcd] Please drop this empty section. > + > +[Pcd] > + gArmTokenSpaceGuid.PcdSystemMemoryBase This PCD is no longer needed. > + gArmTokenSpaceGuid.PcdGicDistributorBase > + gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase > + gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum > + gArmTokenSpaceGuid.PcdArmArchTimerIntrNum > + gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum > + gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum > + gArmPlatformTokenSpaceGuid.PcdPL031RtcBase > + gArmTokenSpaceGuid.PcdDeviceTreeBaseAddress > + > +[Protocols] > + Strictly speaking, you might want to list gEfiDevicePathProtocolGuid here, but stuff will work anyway. > +[Depex] > + TRUE > diff --git a/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > new file mode 100644 > index 000000000000..1ad9e929ae62 > --- /dev/null > +++ b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > @@ -0,0 +1,249 @@ > +/** @file > +* Device tree enumeration DXE driver for AArch64 VMs AArch64 reference > +* > +* Copyright (c) 2014, Linaro 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/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/UefiLib.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/UefiDriverEntryPoint.h> > +#include <Library/MemoryAllocationLib.h> > +#include <Library/UefiBootServicesTableLib.h> > +#include <Library/VirtioMmioDeviceLib.h> > +#include <Library/DevicePathLib.h> > +#include <Library/PcdLib.h> > +#include <Library/DxeServicesLib.h> > +#include <libfdt.h> > + > +#include <Guid/Fdt.h> > + > +CONST UINT32 mUint32Max = 0xFFFFFFFFU; > + > +#pragma pack (1) > +typedef struct { > + VENDOR_DEVICE_PATH Vendor; > + UINT64 PhysBase; > + EFI_DEVICE_PATH_PROTOCOL End; > +} VIRTIO_TRANSPORT_DEVICE_PATH; > +#pragma pack () Thanks! > + > +typedef enum { > + PropertyTypeUnknown, > + PropertyTypeGic, > + PropertyTypeRtc, > + PropertyTypeVirtio, > + PropertyTypeUart, > + PropertyTypeTimer, > +} PROPERTY_TYPE; Thanks! > + > +typedef struct { > + PROPERTY_TYPE Type; > + CHAR8 Compatible[20]; > +} PROPERTY; Thanks! > + > +STATIC CONST PROPERTY CompatibleProperties[] = { > + { PropertyTypeGic, "arm,cortex-a15-gic" }, > + { PropertyTypeRtc, "arm,pl031" }, > + { PropertyTypeVirtio, "virtio,mmio" }, > + { PropertyTypeUart, "arm,pl011" }, > + { PropertyTypeTimer, "arm,armv7-timer" }, > + { PropertyTypeTimer, "arm,armv8-timer" }, > + { PropertyTypeUnknown, "" } > +}; > + > +typedef struct { > + UINT32 Type; > + UINT32 Number; > + UINT32 Flags; > +} INTERRUPT_PROP; > + > +STATIC > +PROPERTY_TYPE > +GetTypeFromNode ( > + IN CONST CHAR8 *NodeType, > + IN UINTN Size > + ) > +{ > + CONST CHAR8 *Compatible; > + > + // > + // A 'compatible' node may contain a sequence of NULL terminated > + // compatible strings so check each one > + // > + for (Compatible = NodeType; Compatible < NodeType + Size && *Compatible; > + Compatible += 1 + AsciiStrLen (Compatible)) { > + > + CONST PROPERTY *CompProp; > + > + for (CompProp = CompatibleProperties; CompProp->Compatible[0]; > CompProp++) { > + if (AsciiStrCmp (CompProp->Compatible, Compatible) == 0) { > + return CompProp->Type; > + } > + } > + } > + return PropertyTypeUnknown; > +} Thanks! > + > +EFI_STATUS > +EFIAPI > +InitializeVirtFdtDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + VOID *DeviceTreeBase; (Please line up the asterisk with Node below.) > + INT32 Node, Prev; > + EFI_STATUS Status; > + > + DeviceTreeBase = (VOID *)PcdGet64 (PcdDeviceTreeBaseAddress); > + ASSERT (DeviceTreeBase != NULL); This assert will be trivially true, since the DSC-default value of the PCD is 0x4000_0000. It might make sense to preserve PcdSystemMemoryBase after all, in the INF file, and here assert that DeviceTreeBase is unequal to PcdSystemMemoryBase. If you agree. > + > + if (fdt_check_header (DeviceTreeBase) != 0) { > + DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%Lx\n", __FUNCTION__, > DeviceTreeBase)); %p for (VOID*), please > + return EFI_NOT_FOUND; > + } > + > + Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DeviceTreeBase); > + ASSERT_EFI_ERROR (Status); Okay. > + > + DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, DeviceTreeBase)); Looks good, thanks. > + > + // > + // Now enumerate the nodes and install peripherals that we are interested > in, > + // i.e., GIC, RTC and virtio MMIO nodes > + // > + for (Prev = 0;; Prev = Node) { > + CONST CHAR8 *Type; > + INT32 Len; > + PROPERTY_TYPE PropType; > + CONST VOID *RegProp; > + VIRTIO_TRANSPORT_DEVICE_PATH *DevicePath; > + EFI_HANDLE Handle; > + UINT64 RegBase; > + UINT64 DistBase, CpuBase; > + CONST INTERRUPT_PROP *InterruptProp; > + INT32 SecIntrNum, IntrNum, VirtIntrNum, > HypIntrNum; (please line up the asterisks if you care enough :)) > + > + Node = fdt_next_node (DeviceTreeBase, Prev, NULL); > + if (Node < 0) > + break; braces > + > + Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len); > + if (Type == NULL) > + continue; braces > + > + PropType = GetTypeFromNode (Type, Len); > + if (PropType == PropertyTypeUnknown) > + continue; braces > + > + // > + // Get the 'reg' property of this node. For now, we will assume > + // 8 byte quantities for base and size, respectively. > + // TODO use #cells root properties instead > + // > + RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len); > + ASSERT (RegProp != NULL || PropType == PropertyTypeTimer); > + > + switch (PropType) { > + > + case PropertyTypeVirtio: > + ASSERT (Len == 16); > + // > + // Create a unique device path for this transport on the fly > + // > + RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode ( > + HARDWARE_DEVICE_PATH, > + HW_VENDOR_DP, > + sizeof (VIRTIO_TRANSPORT_DEVICE_PATH)); > + > + CopyMem (&DevicePath->Vendor.Guid, &gEfiCallerIdGuid, sizeof > (EFI_GUID)); > + DevicePath->PhysBase = RegBase; > + SetDevicePathNodeLength (&DevicePath->Vendor, > + sizeof (*DevicePath) - sizeof > (DevicePath->End)); > + SetDevicePathEndNode (&DevicePath->End); > + > + Handle = NULL; > + Status = gBS->InstallProtocolInterface (&Handle, > + &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > + DevicePath); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH > protocol on a new handle\n", > + __FUNCTION__)); > + break; > + } OK, thanks. No need to update this, just two generic hints: - lines shorter than 80 chars are preferred, - the format specifier %r formats values of type EFI_STATUS and RETURN_STATUS to textual representation. > + > + Status = VirtioMmioInstallDevice (RegBase, Handle); > + if (EFI_ERROR (Status)) > + DEBUG ((EFI_D_ERROR, "%a: Failed to install VirtIO transport @ 0x%Lx > on handle %p\n", > + __FUNCTION__, RegBase, Handle)); braces, please. Also, if VirtioMmioInstallDevice() fails, we shouldn't leak the just-created handle with the devpath protocol instance on it. Please uninstall that protocol here, in the error branch, and then the handle itself will go away. See UninstallProtocolInterface() in the UEFI spec: "If the last protocol interface is removed from a handle, the handle is freed and is no longer valid." I apologize for noticing this only now; I was very tired when I looked at the last version of this code. > + break; > + > + case PropertyTypeGic: > + ASSERT (Len == 32); > + > + DistBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + CpuBase = fdt64_to_cpu (((UINT64 *)RegProp)[2]); > + ASSERT (DistBase < mUint32Max); > + ASSERT (CpuBase < mUint32Max); less-than-or-equal, but it makes no difference in practice, since these base addresses can't point to single-byte "things". You're erring on the safe side. Thanks for adding the asserts! > + > + PcdSet32 (PcdGicDistributorBase, (UINT32)DistBase); > + PcdSet32 (PcdGicInterruptInterfaceBase, (UINT32)CpuBase); > + > + DEBUG ((EFI_D_INFO, "Found GIC @ 0x%x/0x%x\n", DistBase, CpuBase)); But here you should use %Lx, these are UINT64's. > + break; > + > + case PropertyTypeRtc: > + ASSERT (Len == 16); > + > + RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + ASSERT (RegBase < mUint32Max); > + > + PcdSet32 (PcdPL031RtcBase, (UINT32)RegBase); > + > + DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%x\n", RegBase)); %Lx > + break; > + > + case PropertyTypeTimer: > + > + // > + // - interrupts : Interrupt list for secure, non-secure, virtual and > + // hypervisor timers, in that order. > + // > + InterruptProp = fdt_getprop (DeviceTreeBase, Node, "interrupts", &Len); > + ASSERT (Len == 48); > + > + SecIntrNum = fdt32_to_cpu (InterruptProp[0].Number) > + + (InterruptProp[0].Type ? 16 : 0); > + IntrNum = fdt32_to_cpu (InterruptProp[1].Number) > + + (InterruptProp[1].Type ? 16 : 0); > + VirtIntrNum = fdt32_to_cpu (InterruptProp[2].Number) > + + (InterruptProp[2].Type ? 16 : 0); > + HypIntrNum = fdt32_to_cpu (InterruptProp[3].Number) > + + (InterruptProp[3].Type ? 16 : 0); > + > + DEBUG ((EFI_D_INFO, "Found Timer interrupts %d, %d, %d, %d\n", > + SecIntrNum, IntrNum, VirtIntrNum, HypIntrNum)); > + > + PcdSet32 (PcdArmArchTimerSecIntrNum, SecIntrNum); > + PcdSet32 (PcdArmArchTimerIntrNum, IntrNum); > + PcdSet32 (PcdArmArchTimerVirtIntrNum, VirtIntrNum); > + PcdSet32 (PcdArmArchTimerHypIntrNum, HypIntrNum); > + break; > + > + default: > + break; > + } > + } > + return Status; > +} Looks good otherwise; thanks! Laszlo ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
