On 08/27/14 11:35, Ard Biesheuvel wrote:
> On 27 August 2014 11:07, Laszlo Ersek <[email protected]> wrote:
>> On 08/27/14 09:10, Ard Biesheuvel wrote:
>>> On 27 August 2014 00:24, Laszlo Ersek <[email protected]> wrote:
>>>> (b) Predictably, my second question is about the second world. The
>>>> permanent PEI memory must be distinct from the DTB.
>>>>
>>>> InitializeMemory() calculates where to place the permanent PEI memory.
>>>> The permanent PEI memory does not cover the entire system RAM. It's
>>>> (calculated) base address is UefiMemoryBase, and it's size comes from
>>>> PcdSystemMemoryUefiRegionSize (64MB, see the next patch).
>>>>
>>>
>>> Yes, so running with 64 MB will result in this region colliding with the
>>> DTB.
>>
>> Right.
>>
>> Note that this question can be short-circuited by a list of guarantees,
>> for example:
>> - the firmware volume will always (or never) be shadowed from flash to
>> system RAM (--> because this helps us see which outer branch in
>> InitializeMemory() will be taken),
>> - and we'll never boot a guest with less than 128 MB RAM (--> this
>> determines the inner branch, and given the size of the firmware
>> volume, allows deducing the exact location of the permanent PEI RAM)
>>
>> If we can guarantee such things, I'm completely fine with that as an answer.
>>
>
> OK, so by shadowing in this context, you mean invoking QEMU in such a
> way that the UEFI image is preloaded into DRAM?
No. What I mean is the following. In edk2 you can build flash images (FD
files) that contain various firmware volumes. A firmware volume's FFS
(firmware volume file system) may contain an LZMA compressed file that
is itself a firmware volume, after decompression.
When your platform starts up, the reset vector and SEC run from flash.
On some platforms, SEC locates these compressed FFS files in the initial
boot firmware volume (mapped into flash), and decompresses them to
system RAM. Thereby creating "shadowed" firmware volumes.
I have no clue how this looks on the ARM platforms, but it's not
something that changes from run to run. It's a design choice that is
reflected in the FDF file, and the SEC code. If you just instrument
these branches in question with a few DEBUG() calls, you'll see what the
case is for this new platform.
We "agreed" that PcdSystemMemoryInitializeInSec would be FALSE in our
case. Given that SEC doesn't initialize the system memory, it's hard to
imagine it would decompress firmware volumes to it. Hence I expect your
debug messages should prove that there's no shadowing here. (Actually
you might be able to deduce that statically, by tracking down
PcdFdBaseAddress.)
> I don't think there is
> any point at all in doing that, so I don't see why we should support
> it. And having at least 128 MB of DRAM is reasonable as well.
I agree that requiring at least 128 MB of DRAM is reasonable.
> So I could perhaps enforce these rules in the same place the system
> DRAM is discovered from the DTB?
You mean your implementation of ArmPlatformInitializeSystemMemory().
Yes, that would be reasonable. You can assert there that the size of the
system DRAM is at least 128 MB.
The idea is that by controlling all of the circumstances (system DRAM
size, PcdFdBaseAddress, PcdFdSize, PcdSystemMemoryInitializeInSec), you
would *force* InitializeMemory() to a pre-determined path.
This would then protect the DTB.
>>>> (c) Third, before we enter DXE (and after we installed permanent PEI
>>>> memory), we must ensure that DXE won't trample on the DTB.
>>>>
>>>> This is possible in two ways. First, don't install a memory resource
>>>> descriptor HOB in MemoryPeim() that would cover the DTB. (This is very
>>>> unlikely to work if the DTB's range is otherwise covered by the full
>>>> system memory, as reported in the DTB's own self.)
>>>>
>>>> Second (the canonical way), create a memory allocation HOB (with the
>>>> BuildMemoryAllocationHob() function of HobLib) that covers the DTB. This
>>>> way, DXE will create a memory *space* map (== hw properties map), based
>>>> on MemoryPeim()'s memory resource descriptor HOBs, that covers the DTB;
>>>> but seeing the memory allocation HOB, DXE will also start with a UEFI
>>>> memory map (== sw properties map) that already lists the DTB as covered.
>>>> This way DXE will steer clear of that memory range.
>> In short,
>>
>> EFI_PHYSICAL_ADDRESS Base;
>> UINTN FdtSize;
>>
>> Base = PcdGet64 (PcdSystemMemoryBase);
>> FdtSize = fdt_totalsize ((VOID *)(UINTN)Base);
>>
>> BuildMemoryAllocationHob (
>> Base,
>> EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (FdtSize)),
>> EfiBootServicesData
>> );
>>
>> would be correct. No need for relocation or for a later AllocatePages()
>> call. This would answer the third question, and you could link the DTB
>> at 0x4000_0000 into the config table directly.
>>
>
> Indeed.
>
> The only problem is the following: the EFI stub relocates the DTB to
> somewhere higher up in memory (it needs to add and modify entries, and
> doing so in place is not feasible)
> Then, it uses AllocatePages() to find the lowest 2 MB aligned region
> of memory to relocate the kernel Image itself, which should ideally be
> the base of DRAM. However, with the DTB located there, and protected
> by the HOB, it will disregard the base of DRAM and move the kernel
> Image 2 MB up instead.
This analysis seems to be correct. However, I think this should be no
problem.
For example, in OVMF guests, the kernel is also located "higher than
usual". This usually works transparently, and the kernel itself can
(should) later reuse the then-free low area for buffers and some such.
Some tools might need adjustment. For example Dave Anderson is adapting
the "crash" tool so that the user is not required to pass
--machdep phys_base=16m
manually to "crash", when the vmcore to be analyzed originates from an
OVMF guest.
Alternatively, QEMU and AArch64VirtualizationPkg can agree on a
different location of the DTB as well. For example, QEMU could place the
DTB at 16MB, not 0MB (relative to 0x4000_0000, of course!)
- This would require your patchset to state "as a minimum, 17 MB DRAM is
assumed", rather than just 1 MB; in the blurb and the commit messages.
- The temporary SEC/PEI RAM would remain distinct just the same (still
below the 1 MB).
- Assuming 128 MB of DRAM to be guaranteed, the above reasoning about
the permanent PEI RAM would continue to hold. It would still not
overlap with the DTB at 16 MB (relative to 0x4000_0000, again).
- PlatformPei could create the EfiBootServicesData memalloc HOB at 16 MB
(relative to 0x4000_0000). Then DXE would stay away just the same,
but the EFI stub would find address 0 (relative to 0x4000_0000)
unoccupied.
>
>> We just need to find a good spot for this code -- it must run in a PEIM,
>> after permanent PEI RAM has been installed.
>>
>> [snip]
>>
>>>> Summary:
>>>> (a) please prove, with specific addresses, that the temporary RAM range,
>>>> used in SEC and the first half of PEI, is distinct from the
>>>> pre-placed DTB.
>>>> (b) please prove, with specific range sizes, and by walking me through
>>>> the exact branches in InitializeMemory(), that the permanent PEI RAM
>>>> will not overlap the pre-placed DTB.
>>>> (c) please find a good spot in some PEIM -- that runs on permanent PEI
>>>> RAM -- for a BuildMemoryAllocationHob() call.
>>>>
>>>
>>> I will try to follow up with a v3 that relocates the DTB as early as
>>> possible. What would be the best way to communicate the new offset?
>>> Another dynamic PCD?
>>
>> Let's not relocate it. :) If I understand correctly, your main concern
>> with BuildMemoryAllocationHob() was that it would reserve the memory
>> area even during OS runtime. This is not the case, see above.
>>
>
> Not entirely, see above
Well, relocation *is* possible, if you want to do it.
Namely, rather than calling BuildMemoryAllocationHob(), in order to
cover the DTB *in place*, you can simply call AllocatePages() or
AllocatePool() and copy the DTB there, from its original place. Importantly,
- this allocation and copying needs to be done in the exact same place
as you'd call BuildMemoryAllocationHob() -- namely, still in PEI, and
after the permanent PEI RAM has been installed;
- the AllocatePages() / AllocatePool() call itself would be internally
served *from* the permanent PEI RAM,
- and, internally again, a very similar memory allocation HOB would be
created.
Again: you can implement the protection from DXE with a relocation that
is done in PEI (after permanent PEI RAM has been installed), instead of
with an in-place BuildMemoryAllocationHob(). The only difference would
be that ultimately the DTB would go on to live in the permanent PEI RAM
range, not at it's original address, where QEMU has placed it.
The problem in this case, however, is that in DXE you'd have a hard time
finding the relocated DTB (for parsing it). As you said, you'd need yet
another dynamic PCD, to store the address.
>> But, again, Q1 has been answered, Q2 can be answered simply by
>> guaranteeing a few constants, and Q3 can be covered by a small snippet
>> of code, which just has to be inserted in a good spot.
>>
>> Not sure if in ARM world there are "platform PEIMs". In OVMF we have
>> such BuildMemoryAllocationHob() calls in OvmfPkg/PlatformPei/ (which
>> take place after permanent PEI RAM gets installed; same as discussed above).
>>
>> According to patch v2 7/8, this is our list of PEIMs:
>>
>> + APRIORI PEI {
>> + INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>> + }
>> + INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> + INF MdeModulePkg/Core/Pei/PeiMain.inf
>> + INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>> + INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> + INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>> + INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>> + INF IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>> + INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>
>> So, yes, there is a platform PEIM:
>>
>> ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>>
>> The by-the-book way would be to
>> - create a *deep copy* of this module, under
>> ArmPlatformPkg/AArch64VirtualizationPkg/,
>> - reference that copy in the new DSC and FDF file(s),
>> - and add the above code snippet to the PlatformPeim() function.
>>
>> (This could be made more lightweight by exposing PlatformPeim() as a
>> standalone library class in ARM, and then you wouldn't need to copy the
>> entire ArmPlatformPkg/PlatformPei directory to
>> AArch64VirtualizationPkg/, just provide a new library instance.)
>>
>> Another, independent possibility would be to introduce a FeaturePcd
>> (called PcdReserveFdtRange or some such), and dependent on that, run the
>> code snippet in "ArmPlatformPkg/PlatformPei/PlatformPeiLib.c" directly.
>> This is probably the lightest solution, but it would take the most
>> convincing with Olivier :)
>>
>> *** In addition, I *think* that the platform PEIM's [Depex] section
>> should spell out a [Depex] on gEfiPeiMemoryDiscoveredPpiGuid, if the
>> platform PEIM calls BuildMemoryAllocationHob().
>>
>> "ArmPkg/Drivers/CpuPei/CpuPei.inf" (listed above) does so already, for
>> example. (Likely for other reasons; I'm just saying there's already a
>> Depex example one could copy.)
>>
>> The presence of this PPI guarantees that the PEIM runs after permanent
>> PEI RAM has been installed. Refer to:
>> - "MdePkg/Include/Ppi/MemoryDiscovered.h",
>> - Chapter 11, PEI Physical Memory Usage, in Vol 1 of the PI spec.
>> (EFI_PEI_PERMANENT_MEMORY_INSTALLED_PPI)
>>
>> In OVMF's PlatformPei, we don't depend on
>> gEfiPeiMemoryDiscoveredPpiGuid, but for a good reason: we install the
>> permanent PEI RAM and build the memalloc HOBs in that *same* PEIM, hence
>> we can ensure their correct ordering easily.
>>
>> I'll ask for confirmation on the list separately.
>>
>
> So even if we do end up relocating the DTB (which I expect), I assume
> this would still be the place to implement that?
> If so, I will go and implement the PlatformPeim() while we continue
> the discussion regarding the relocation.
Yes. You can decide between
- in-place coverage with BuildMemoryAllocationHob()
- and relocation with AllocateCopyPool() + PcdSet64(PcdRelocatedFdtBase)
but whichever you pick, it must occur in the same place: in PEI, after
permanent PEI RAM installation, and preferably in the platform PEI
(which should spell out the depex).
... Okay, summary time.
- Temporary SEC/PEI RAM seems to be safely distinct.
- permanent PEI RAM can be made distinct, by controlling a handful of
PCDs, and by asserting 128 MB minimum required memory size in
ArmPlatformInitializeSystemMemory(), when you parse the DTB.
- We discussed four options for DXE protection:
- in-place DTB coverage at 0x4000_0000, with a memalloc HOB. Pro:
simple to do; con: might surprise the kernel and some tools.
- in-place DTB coverage at 0x4100_0000, with a memalloc HOB. Pro:
simple to do and likely no surprises for the kernel; con: QEMU
needs to be adapted.
- relocation from 0x4000_0000 to a dynamically allocated chunk of the
permanent PEI RAM. Pro: no surprises for the kernel; con: needs at
least one further dynamic PCD.
- (more than just DXE protection, actually): present the DTB over an
MMIO "channel". Whoever cares can fetch it into a temporary array,
parse it, then throw it away. Pro: DTB is unable to overlap with
system DRAM, and this discussion goes away. Con: needs new hardware
in machtype virt.
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
