Re: [RFC v4 06/13] OvmfPkg:AmdSevDxe: add AmdSevDxe driver

[Brijesh Singh <brijesh.singh@amd.com>]

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.yao@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