On 05/11/2017 10:19 AM, Laszlo Ersek wrote:

(1) Please mention that the C bit is cleared for MMIO GCD entries in
order to cover the ranges that were added during the PEI phase (through
memory resource descriptor HOBs).

Also mention that the NonExistent ranges are processed in order to
cover, in advance, MMIO ranges added later in the DXE phase by various
device drivers, via the appropriate DXE memory space services.

Finally, please mention that the approach is not transparent for later
addition of system memory ranges to the GCD memory space map. (Such
ranges should be encrypted.) OVMF does not do such a thing at the
moment, so this approach should be OK.

I think we should also credit Jiewen for both ideas, namely the IOMMU
stuff and the handling of NonExistent ranges (in anticipation of future
MMIO additions), so please add

Suggested-by: Jiewen Yao <jiewen....@intel.com>


Agreed :)

I will definitely give credit to Jiewen for it. Additionally, I borrowed
the IOMMU driver implementation from Jiewen's sample driver hence I believe I've
retained the Intel copyright in both header and source file, if not then I will
make sure to include it in next patch.


[snip...]


(4) Right here I think you have a memory leak; gDS->GetMemorySpaceMap()
allocates AllDescMap dynamically (on success). Please free it with
FreePool().


Ah good point. I will make sure to free the memory.


-Brijesh
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to