On 11/30/18 12:28, Ard Biesheuvel wrote: > QEMU/mach-virt is rather unhelpful when it comes to tracking down > NULL pointer dereferences that occur while running in UEFI: since > we have NOR flash mapped at address 0x0, inadvertent reads go > unnoticed, and even most writes are silently dropped, unless you're > unlucky and the instruction in question is one that KVM cannot > emulate, in which case you end up with a QEMU crash like this: > > error: kvm run failed Function not implemented > PC=000000013f7ff804 X00=000000013f7ab108 X01=0000000000000064 > X02=000000013f801988 X03=00000000800003c4 X04=0000000000000000 > X05=0000000096000044 X06=fffffffffffd8270 X07=000000013f7ab4a0 > X08=0000000000000001 X09=000000013f803b88 X10=000000013f7e88d0 > X11=0000000000000009 X12=000000013f7ab554 X13=0000000000000008 > X14=0000000000000002 X15=0000000000000000 X16=0000000000000000 > X17=0000000000000000 X18=0000000000000000 X19=0000000000000000 > X20=000000013f81c000 X21=000000013f7ab170 X22=000000013f81c000 > X23=0000000009000018 X24=000000013f407020 X25=000000013f81c000 > X26=000000013f803530 X27=000000013f802000 X28=000000013f7ab270 > X29=000000013f7ab0d0 X30=000000013f7fee10 SP=000000013f7a6f30 > PSTATE=800003c5 N--- EL1h > > and a warning in the host kernel log that load/store instruction > decoding is not supported by KVM. > > Given that the first page of the flash device is not actually > used anyway, let's reduce the mappings of the peripheral space > and the flash device (both of which cover page #0) to only cover > what is actually required: > > ArmVirtQemu.fdf: > > 0x00001000|0x001ff000 > > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize > > ArmVirtQemuKernel.fdf: > > 0x00008000|0x001f8000 > > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize > > For ArmVirtQemu, the resulting virtual mapping looks roughly like: > - [0, 4K) : flash, unmapped > - [4K, 2M) : flash, mapped as WB+X RAM > - [2M, 64M) : flash, unmapped > - [64M, 128M) : varstore flash, will be mapped by the NOR flash driver > - [128M, 256M) : peripherals, mapped as device > - [256M, 1GB) : 32-bit MMIO aperture, translated IO aperture, ECAM, > will be mapped by the PCI host bridge driver > - [1GB, ...) : RAM, mapped. > > After this change, any inadvertent read or write from/to the first > physical page will trigger a translation fault inside the guest, > regardless of the nature of the instruction, without crashing QEMU. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > --- > ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf | 4 ++-- > ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf | 2 ++ > ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 23 > ++++++++++++++------ > 3 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > index 5c5b841051ad..b6abc52531a8 100644 > --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > @@ -39,9 +39,9 @@ [LibraryClasses] > PcdLib > > [Pcd] > - gArmTokenSpaceGuid.PcdFdBaseAddress > + gArmTokenSpaceGuid.PcdFvBaseAddress > gArmTokenSpaceGuid.PcdSystemMemoryBase > gArmTokenSpaceGuid.PcdSystemMemorySize > > [FixedPcd] > - gArmTokenSpaceGuid.PcdFdSize > + gArmTokenSpaceGuid.PcdFvSize > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf > b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf > index d12089760b22..16802c5c414b 100644 > --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf > @@ -43,9 +43,11 @@ [LibraryClasses] > > [Pcd] > gArmTokenSpaceGuid.PcdFdBaseAddress > + gArmTokenSpaceGuid.PcdFvBaseAddress > gArmTokenSpaceGuid.PcdSystemMemoryBase > gArmTokenSpaceGuid.PcdSystemMemorySize > > [FixedPcd] > gArmTokenSpaceGuid.PcdFdSize > + gArmTokenSpaceGuid.PcdFvSize > gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > index 0285a11b1d77..a26b2fbad9be 100644 > --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > @@ -21,6 +21,15 @@ > // Number of Virtual Memory Map Descriptors > #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 5 > > +// > +// mach-virt's core peripherals such as the UART, the GIC and the RTC are > +// all mapped in the 'miscellaneous device I/O' region, which we just map > +// in its entirety rather than device by device. Note that it does not > +// cover any of the NOR flash banks or PCI resource windows. > +// > +#define MACH_VIRT_PERIPH_BASE 0x08000000 > +#define MACH_VIRT_PERIPH_SIZE SIZE_128MB > + > /** > Return the Virtual Memory Map of your platform > > @@ -66,16 +75,16 @@ ArmVirtGetMemoryMap ( > VirtualMemoryTable[0].VirtualBase, > VirtualMemoryTable[0].Length)); > > - // Peripheral space before DRAM > - VirtualMemoryTable[1].PhysicalBase = 0x0; > - VirtualMemoryTable[1].VirtualBase = 0x0; > - VirtualMemoryTable[1].Length = VirtualMemoryTable[0].PhysicalBase; > + // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc) > + VirtualMemoryTable[1].PhysicalBase = MACH_VIRT_PERIPH_BASE; > + VirtualMemoryTable[1].VirtualBase = MACH_VIRT_PERIPH_BASE; > + VirtualMemoryTable[1].Length = MACH_VIRT_PERIPH_SIZE; > VirtualMemoryTable[1].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > > - // Remap the FD region as normal executable memory > - VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFdBaseAddress); > + // Map the FV region as normal executable memory > + VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFvBaseAddress); > VirtualMemoryTable[2].VirtualBase = VirtualMemoryTable[2].PhysicalBase; > - VirtualMemoryTable[2].Length = FixedPcdGet32 (PcdFdSize); > + VirtualMemoryTable[2].Length = FixedPcdGet32 (PcdFvSize); > VirtualMemoryTable[2].Attributes = > ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; > > // End of Table >
Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
