I'll skip the DSC, FDF and INF files; I assume those are customized copies from existing platform files. I'll also skip the INF files for now.
On 08/26/14 15:03, 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 | 223 +++++++++++++ > .../AArch64Virtualization-KVM.fdf | 307 ++++++++++++++++++ > .../AArch64Virtualization.dsc.inc | 354 > +++++++++++++++++++++ > .../AArch64VirtualizationPkg/Driver/VirtFdt.inf | 60 ++++ > .../AArch64VirtualizationPkg/Driver/VirtFdtDxe.c | 229 +++++++++++++ > .../Include/Platform/KVM/ArmPlatform.h | 32 ++ > .../AArch64VirtualizationLibKVM/AArch64KVMLib.inf | 53 +++ > .../Library/AArch64VirtualizationLibKVM/KVM.c | 127 ++++++++ > .../AArch64VirtualizationLibKVM/KVMHelper.S | 70 ++++ > .../Library/AArch64VirtualizationLibKVM/KVMMem.c | 106 ++++++ > .../AArch64VirtualizationSysConfigLibKVM.c | 95 ++++++ > .../AArch64VirtualizationSysConfigLibKVM.inf | 35 ++ > .../Library/ResetSystemLib/ResetSystemLib.c | 96 ++++++ > .../Library/ResetSystemLib/ResetSystemLib.inf | 33 ++ > 14 files changed, 1820 insertions(+) > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.fdf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization.dsc.inc > create mode 100644 ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdt.inf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Include/Platform/KVM/ArmPlatform.h > 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 > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationSysConfigLibKVM/AArch64VirtualizationSysConfigLibKVM.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationSysConfigLibKVM/AArch64VirtualizationSysConfigLibKVM.inf > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.c > create mode 100644 > ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.inf [snip] > diff --git a/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > new file mode 100644 > index 000000000000..04840e5d1a57 > --- /dev/null > +++ b/ArmPlatformPkg/AArch64VirtualizationPkg/Driver/VirtFdtDxe.c > @@ -0,0 +1,229 @@ > +/** @file > +* Device tree enumeration DXE driver for AArch64 VMs > +* > +* 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> > + > +#pragma pack (1) > +typedef struct { > + VENDOR_DEVICE_PATH Vendor; > + UINT32 Instance; > + EFI_DEVICE_PATH_PROTOCOL End; > +} VIRTIO_BLK_DEVICE_PATH; > +#pragma pack () Ugh. :) Have you seen my earlier patches where the vendor device path node corresponding to a virtio-mmio transport actually included the base address of the MMIO register block? I think such a hwvendor devpath node is much more useful; it can actually help you debug. In any case, if you decide to stick with this hwvendor devpath node, plase don't call it VIRTIO_BLK. Call it VIRTIO_MMIO, or VIRTIO_TRANSPORT, or something else virtio-device-type-agnostic. > + > +static struct _Ptypes { > + enum _Ptype { > + UNKNOWN, GIC, RTC, VIRTIO, UART, TIMER > + } Ptype; > + CHAR8 Comp[20]; > +} const Ptypes[] = { > + { GIC, "arm,cortex-a15-gic" }, > + { RTC, "arm,pl031" }, > + { VIRTIO, "virtio,mmio" }, > + { UART, "arm,pl011" }, > + { TIMER, "arm,armv7-timer" }, > + { TIMER, "arm,armv8-timer" }, > + { UNKNOWN, "" } > +}; This is a smorgasbord of edk2 coding style violations. Rather than explaining each, let me just rewrite the definition. I'll assume that "P" stands for "Property". typedef enum { PropertyTypeUnknown, PropertyTypeGic, PropertyTypeRtc, PropertyTypeVirtio, ... } PROPERTY_TYPE; typedef struct { PROPERTY_TYPE Type; CHAR8 Compatible[20]; } PROPERTY; STATIC CONST PROPERTY[] = { ... }; > + > +enum _Ptype GetTypeFromNode (CHAR8 CONST *NodeType, INT32 Size) STATIC PROPERTY_TYPE GetTypeFromNode ( IN CONST CHAR8 *NodeType, IN UINTN Size ) { ... } > +{ > + CHAR8 CONST *s; CONST goes first. Please rename "s" to something sensible, and make it CamelCase. (... Yes, I know.) > + > + /* > + * A 'compatible' node may contain a sequence of NULL terminated > + * compatible strings so check each one > + */ // // A 'compatible' node ... // ... // > + for (s = NodeType; s < NodeType + Size && *s; s += 1 + AsciiStrLen (s)) { > + struct _Ptypes const *Type; > + > + for (Type = Ptypes; Type->Comp[0]; Type++) Braces are mandatory. > + if (AsciiStrCmp (Type->Comp, s) == 0) > + return Type->Ptype; > + } > + return UNKNOWN; > +} > + > +EFI_STATUS > +EFIAPI > +InitializeVirtFdtDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + VOID *FdtImageData; > + VOID *MemoryBase; > + INT32 Node, Prev, VirtIoInstance; > + EFI_STATUS Status; > + > + MemoryBase = (VOID *)PcdGet64 (PcdSystemMemoryBase); Given that this code is exclusively 64-bit, this should be fine. > + > + if (fdt_check_header (MemoryBase) != 0) { > + DEBUG ((EFI_D_ERROR, "InitializeVirtFdtDxe: No DTB found @ 0x%08x\n", > + MemoryBase)); I suggest "%a" instead of "InitializeVirtFdtDxe" in the format string, and __FUNCTION__ in the variable argument list, accordingly. And, referencing my v2 6/8 review, the DTB may have been corrupted by the time we get here. > + return EFI_NOT_FOUND; > + } > + > + FdtImageData = AllocatePages (EFI_SIZE_TO_PAGES (fdt_totalsize > (MemoryBase))); What's wrong with AllocatePool()? Does the EFI stub require full pages, and page alignment? > + if (FdtImageData == NULL) { > + DEBUG ((EFI_D_ERROR, "InitializeVirtFdtDxe: AllocatePages() failed\n")); "%a", __FUNCTION > + return EFI_OUT_OF_RESOURCES; > + } > + > + CopyMem (FdtImageData, MemoryBase, fdt_totalsize (MemoryBase)); > + Status = gBS->InstallConfigurationTable (&gFdtTableGuid, FdtImageData); > + ASSERT_EFI_ERROR (Status); > + > + DEBUG ((EFI_D_ERROR, "InitializeVirtFdtDxe: DTB @ 0x%08x\n", > FdtImageData)); Yow. The frequent problem of ARM builds, namely that they suppress all debug messages under EFI_D_ERROR, should not be remedied by printing everything as EFI_D_ERROR. EFI_D_ERROR should be reserved for, well, errors. This is an EFI_D_INFO or EFI_D_VERBOSE message. The problem of over-suppressing is remedied by enabling EFI_D_INFO and maybe even EFI_D_VERBOSE in the DSC file, PcdDebugPrintErrorLevel. In addition, the %x format specifier is inappropriate for a pointer. You should use %p. If you'd like zero padding (and let's then also consider this is 64-bit code only), then: DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%016Lx\n", __FUNCTION__, (UINT64)(UINTN)FdtImageData)); > + > + /* > + * Now enumerate the nodes and install peripherals that we are interested > in, > + * i.e., GIC, RTC and virtio MMIO nodes > + */ // // comment // > + Status = EFI_SUCCESS; > + > + for (Prev = VirtIoInstance = 0; Status == EFI_SUCCESS; Prev = Node) { Please say Prev = 0; VirtIoInstance = 0; > + CONST CHAR8 *Type; > + INT32 Len; (Usually UINTN is preferred for sizes and lengths.) > + enum _Ptype Ptype; > + VOID CONST *RegProp; > + > + Node = fdt_next_node (MemoryBase, Prev, NULL); > + if (Node < 0) > + break; > + > + Type = fdt_getprop (MemoryBase, Node, "compatible", &Len); > + if (Type == NULL) > + continue; > + > + Ptype = GetTypeFromNode (Type, Len); > + if (Ptype == UNKNOWN) > + continue; > + > + /* > + * 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 > + */ Prior comments apply (braces, CONST, comment style, etc). > + RegProp = fdt_getprop (MemoryBase, Node, "reg", &Len); > + ASSERT (RegProp != NULL || Ptype == TIMER); > + > + switch (Ptype) { > + VIRTIO_BLK_DEVICE_PATH *DevicePath; > + EFI_HANDLE Handle; > + UINT64 RegBase; > + UINT32 DistBase, CpuBase; Yikes! Definition of automatic storage duration objects in a switch block. Very valid, yet very unacceptable for edk2. :) Please move them out. > + > + case VIRTIO: > + /* Create a unique device path for this transport on the fly */ // // comment // > + DevicePath = (VIRTIO_BLK_DEVICE_PATH *)CreateDeviceNode ( > + HARDWARE_DEVICE_PATH, > + HW_VENDOR_DP, > + sizeof (VIRTIO_BLK_DEVICE_PATH)); > + > + CopyMem (&DevicePath->Vendor.Guid, &gEfiCallerIdGuid, sizeof > (EFI_GUID)); > + DevicePath->Instance = VirtIoInstance++; > + SetDevicePathNodeLength (&DevicePath->Vendor, > + sizeof (*DevicePath) - sizeof > (DevicePath->End)); > + SetDevicePathEndNode (&DevicePath->End); Seems sane. > + > + Handle = NULL; > + Status = gBS->InstallProtocolInterface (&Handle, > + &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE, > + DevicePath); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "Failed to install VirtIO protocol > interface\n")); > + break; > + } Incorrect error message; what failed was the handle creation (requested by installing a devpath on an initially NULL handle). Also, if this fails, we'll correctly terminate the loop. However we will also simply leave the function without undoing / rolling back the virtio installations that did succeed. This may or may not be intended. > + > + RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + Status = VirtioMmioInstallDevice (RegBase, Handle); > + if (EFI_ERROR (Status)) > + DEBUG ((EFI_D_ERROR, "Failed to install VirtIO transport @ 0x%x\n", > + RegBase)); 0x%Lx (or lx, as you prefer) > + break; > + > + case GIC: > + ASSERT (Len == 32); > + > + DistBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + CpuBase = fdt64_to_cpu (((UINT64 *)RegProp)[2]); > + PcdSet32 (PcdGicDistributorBase, DistBase); > + PcdSet32 (PcdGicInterruptInterfaceBase, CpuBase); DistBase and CpuBase are UINT32s, matching the PCDs, but they don't match the FDT (UINT64). Is that okay? > + > + DEBUG ((EFI_D_ERROR, "Found GIC @ 0x%x/0x%x\n", DistBase, CpuBase)); EFI_D_INFO > + break; > + > + case RTC: > + ASSERT (Len == 16); > + > + RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]); > + PcdSet32 (PcdPL031RtcBase, (UINT32)RegBase); Since we're asserting the length, we could also assert that conversion to UINT32 doesn't cause truncation. > + > + DEBUG ((EFI_D_ERROR, "Found PL031 RTC @ 0x%x\n", RegBase)); EFI_D_INFO, 0x%Lx > + break; > + > + case TIMER: { > + struct { > + UINT32 Type; > + UINT32 Number; > + UINT32 Flags; > + } const *Interrupts; please move the struct def out, and give it a typedef, as seen above > + INT32 SecIntrNum, IntrNum, VirtIntrNum, HypIntrNum; > + > + /* > + * - interrupts : Interrupt list for secure, non-secure, virtual and > + * hypervisor timers, in that order. > + */ // // comment // > + Interrupts = fdt_getprop (MemoryBase, Node, "interrupts", &Len); > + ASSERT (Len == 48); > + > + SecIntrNum = fdt32_to_cpu (Interrupts[0].Number) > + + (Interrupts[0].Type ? 16 : 0); > + IntrNum = fdt32_to_cpu (Interrupts[1].Number) > + + (Interrupts[1].Type ? 16 : 0); > + VirtIntrNum = fdt32_to_cpu (Interrupts[2].Number) > + + (Interrupts[2].Type ? 16 : 0); > + HypIntrNum = fdt32_to_cpu (Interrupts[3].Number) > + + (Interrupts[3].Type ? 16 : 0); > + > + DEBUG ((EFI_D_ERROR, "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; > +} [snip] > diff --git > a/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVM.c > > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVM.c > new file mode 100644 > index 000000000000..dcbe076f5d58 > --- /dev/null > +++ > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVM.c > @@ -0,0 +1,127 @@ > +/** @file > +* > +* Copyright (c) 2011-2013, 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 <Library/IoLib.h> > +#include <Library/ArmPlatformLib.h> > +#include <Library/DebugLib.h> > +#include <Library/PcdLib.h> > +#include <ArmPlatform.h> > +#include <libfdt.h> > + > +/** > + Return the current Boot Mode > + > + This function returns the boot reason on the platform > + > + @return Return the current Boot Mode of the platform > + > +**/ > +EFI_BOOT_MODE > +ArmPlatformGetBootMode ( > + VOID > + ) > +{ > + return BOOT_WITH_FULL_CONFIGURATION; > +} > + > +/** > + Initialize controllers that must setup in the normal world > + > + This function is called by the ArmPlatformPkg/Pei or > ArmPlatformPkg/Pei/PlatformPeim > + in the PEI phase. > + > +**/ > +RETURN_STATUS > +ArmPlatformInitialize ( > + IN UINTN MpId > + ) > +{ > + return RETURN_SUCCESS; > +} > + > +/** > + Initialize the system (or sometimes called permanent) memory > + > + This memory is generally represented by the DRAM. > + > +**/ > +VOID > +ArmPlatformInitializeSystemMemory ( > + VOID > + ) > +{ > + VOID *SystemMemoryBase; > + INT32 Node, Prev; > + > + SystemMemoryBase = (VOID *)PcdGet64 (PcdSystemMemoryBase); > + > + /* > + * Make sure we have a valid device tree blob at the base of DRAM > + */ > + if (fdt_check_header (SystemMemoryBase) != 0) > + return; > + > + /* > + * Look for a memory node > + * TODO handle disjoint memory > + */ > + for (Prev = 0;; Prev = Node) { > + CONST CHAR8 *Type; > + INT32 Len; > + > + Node = fdt_next_node (SystemMemoryBase, Prev, NULL); > + if (Node < 0) > + return; > + > + Type = fdt_getprop (SystemMemoryBase, Node, "device_type", &Len); > + if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) { > + UINT64 CONST *RegProp; > + > + /* > + * Get the 'reg' property of this node. For now, we will assume > + * two 8 byte quantities for base and size, respectively. > + * TODO use #cells root properties instead > + */ > + RegProp = fdt_getprop (SystemMemoryBase, Node, "reg", &Len); > + if (RegProp != 0 && Len == (2 * sizeof (UINT64))) { > + UINT64 NewBase, NewSize; > + > + NewBase = fdt64_to_cpu (RegProp[0]); > + NewSize = fdt64_to_cpu (RegProp[1]); > + > + PcdSet64 (PcdSystemMemoryBase, NewBase); > + PcdSet64 (PcdSystemMemorySize, NewSize); > + > + DEBUG ((EFI_D_ERROR, "KVM: System RAM @ 0x%lx - 0x%lx\n", > + NewBase, NewBase + NewSize - 1)); > + } > + continue; > + } > + } > +} Too tired to check semantics overly deeply. Prior style remarks apply. In addition, I think the "continue" is superfluous. > + > +EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = { > +}; GNUism, please avoid. Please spell out ... gPlatformPpiTable[] = { { 0 } }; > + > +VOID > +ArmPlatformGetPlatformPpiList ( > + OUT UINTN *PpiListSize, > + OUT EFI_PEI_PPI_DESCRIPTOR **PpiList > + ) > +{ > + *PpiListSize = sizeof (gPlatformPpiTable); > + *PpiList = gPlatformPpiTable; > +} [snip KVMHelper.S] > diff --git > a/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVMMem.c > > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVMMem.c > new file mode 100644 > index 000000000000..55dcf31a5666 > --- /dev/null > +++ > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/AArch64VirtualizationLibKVM/KVMMem.c > @@ -0,0 +1,106 @@ > +/** @file > +* > +* Copyright (c) 2011-2013, 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. > +* > +**/ > + > +#include <Library/ArmPlatformLib.h> > +#include <Library/DebugLib.h> > +#include <Library/PcdLib.h> > +#include <Library/IoLib.h> > +#include <Library/MemoryAllocationLib.h> > +#include <Library/ArmPlatformGlobalVariableLib.h> > +#include <ArmPlatform.h> > + > +// Number of Virtual Memory Map Descriptors without a Logic Tile > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 4 > + > +// DDR attributes > +#define DDR_ATTRIBUTES_CACHED ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK > +#define DDR_ATTRIBUTES_UNCACHED > ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED > + > +/** > + Return the Virtual Memory Map of your platform > + > + This Virtual Memory Map is used by MemoryInitPei Module to initialize the > MMU > + on your platform. > + > + @param[out] VirtualMemoryMap Array of ARM_MEMORY_REGION_DESCRIPTOR > + describing a Physical-to-Virtual Memory > + mapping. This array must be ended by a > + zero-filled entry > + > +**/ > +VOID > +ArmPlatformGetVirtualMemoryMap ( > + IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap > + ) > +{ > + ARM_MEMORY_REGION_ATTRIBUTES CacheAttributes; > + ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable; > + EFI_PHYSICAL_ADDRESS PhysAddrTop; > + > + static INT16 const PARange[] = { 32, 36, 40, 42, 44, 48, -1, -1 }; > + > + ASSERT (VirtualMemoryMap != NULL); > + > + /* > + * Get the max phys address from the CPU config registers > + */ > + asm ("mrs %0, id_aa64mmfr0_el1" : "=r" (PhysAddrTop)); This should be moved into a dedicated helper function / assembly language file. > + PhysAddrTop = 1ULL << PARange[PhysAddrTop & 7]; > + ASSERT (PhysAddrTop != 0); > + > + VirtualMemoryTable = AllocatePages (EFI_SIZE_TO_PAGES ( > + sizeof (ARM_MEMORY_REGION_DESCRIPTOR) > + * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS)); > + if (VirtualMemoryTable == NULL) { > + DEBUG ((EFI_D_ERROR, "ArmPlatformGetVirtualMemoryMap: Error: Failed > AllocatePages()\n")); > + return; > + } > + > + if (FeaturePcdGet (PcdCacheEnable) == TRUE) { > + CacheAttributes = DDR_ATTRIBUTES_CACHED; > + } else { > + CacheAttributes = DDR_ATTRIBUTES_UNCACHED; > + } > + > + // System DRAM > + VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase); > + VirtualMemoryTable[0].VirtualBase = VirtualMemoryTable[0].PhysicalBase; > + VirtualMemoryTable[0].Length = PcdGet64 (PcdSystemMemorySize); > + VirtualMemoryTable[0].Attributes = CacheAttributes; > + > + DEBUG ((EFI_D_ERROR, "ArmPlatformGetVirtualMemoryMap: Dumping DDR Memory > Map:\n \ > + DDR PhysicalBase: 0x%lX\n \ > + VirtualBase: 0x%lX\n \ > + Length: 0x%lX\n", > + VirtualMemoryTable[0].PhysicalBase, > + VirtualMemoryTable[0].VirtualBase, > + VirtualMemoryTable[0].Length)); > + > + // Peripheral space before DRAM > + VirtualMemoryTable[1].PhysicalBase = 0x0; > + VirtualMemoryTable[1].VirtualBase = 0x0; > + VirtualMemoryTable[1].Length = VirtualMemoryTable[0].PhysicalBase; > + VirtualMemoryTable[1].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > + > + // Peripheral space after DRAM > + VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + > VirtualMemoryTable[1].Length; > + VirtualMemoryTable[2].VirtualBase = VirtualMemoryTable[2].PhysicalBase; > + VirtualMemoryTable[2].Length = PhysAddrTop - > VirtualMemoryTable[2].PhysicalBase; > + VirtualMemoryTable[2].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > + > + // End of Table > + VirtualMemoryTable[3] = (ARM_MEMORY_REGION_DESCRIPTOR){}; > + > + *VirtualMemoryMap = VirtualMemoryTable; > +} No comments as to validity, but style observations apply. In particular, kill those line-trailing backslashes, please; it's "perfectly possible " "to " "concatenate string " "literals". [snip] > diff --git > a/ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.c > > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.c > new file mode 100644 > index 000000000000..e5edbc407b56 > --- /dev/null > +++ > b/ArmPlatformPkg/AArch64VirtualizationPkg/Library/ResetSystemLib/ResetSystemLib.c > @@ -0,0 +1,96 @@ > +/** @file > + Template library implementation to support ResetSystem Runtime call. > + > + Fill in the templates with what ever makes you system reset. > + > + Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR> > + Copyright (c) 2013, ARM 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 <PiDxe.h> > + > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/EfiResetSystemLib.h> > + > +#include <ArmPlatform.h> > + > +/** > + Resets the entire platform. > + > + @param ResetType The type of reset to perform. > + @param ResetStatus The status code for the reset. > + @param DataSize The size, in bytes, of WatchdogData. > + @param ResetData For a ResetType of EfiResetCold, > EfiResetWarm, or > + EfiResetShutdown the data buffer starts with > a Null-terminated > + Unicode string, optionally followed by > additional binary data. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibResetSystem ( > + IN EFI_RESET_TYPE ResetType, > + IN EFI_STATUS ResetStatus, > + IN UINTN DataSize, > + IN CHAR16 *ResetData OPTIONAL > + ) > +{ > + register UINTN Rx asm ("x0"); > + > + switch (ResetType) { > + > + case EfiResetPlatformSpecific: > + // Map the platform specific reset as reboot > + case EfiResetWarm: > + // Map a warm reset into a cold reset > + case EfiResetCold: > + // Send a PSCI 0.2 SYSTEM_RESET command > + Rx = PSCI_0_2_FN_SYSTEM_RESET; > + break; > + case EfiResetShutdown: > + // Send a PSCI 0.2 SYSTEM_OFF command > + Rx = PSCI_0_2_FN_SYSTEM_OFF; > + break; > + > + default: > + ASSERT (FALSE); > + return EFI_UNSUPPORTED; > + } > + > + /* > + * Being a virtualized platform, we are allowed to assume that we > + * are running at EL1, so HVC is appropriate for making PSCI calls. > + */ > + asm ("hvc #0" :: "r" (Rx)); Should be a separate... etc etc > + > + // We should never be here > + while (1); > +} CpuDeadLoop() is more idiomatic in edk2. > + > +/** > + Initialize any infrastructure required for LibResetSystem () to function. > + > + @param ImageHandle The firmware allocated handle for the EFI image. > + @param SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +LibInitializeResetSystem ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return EFI_SUCCESS; > +} 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
