Evelyn,
Your patch is too big. Can you create separate patches with each addressing one
purpose?
Based on your commit message, I can see at least 4 patches.
Detailed comments:
1. Usually the data structure for HOB is put under header files in
Include/Guid/ directory.
How about /Include/Guid/VtdPmrInfo.h? And the header file addition can be a
standalone patch.
2. The comments inside the HOB header file may be modified to have more precise
information. Currently it's as below:
// Platforms may request no overlap between PMR regions
// and system reserve memory. Create an interface to control PLMR/PHMR
// regions. It allows silicon code to adjust PLMR/PHMR region base on
// the project needs.
I think the word "control" in above comments means this interface is used to
report the PLMR and PHMR region reserved
by memory initialization code (memory reference code). The region reservation
is done by memory initialization code and
this interface reports the info. We should avoid misunderstanding that this
interface is to tell some silicon code to reserve
the PLMR and PHMR. In another word, this interface is an OUT data, not an IN
parameter. That's my understanding. Correct me
if I am wrong.
3. Can you add more comments inside InitDmaProtection() function? The comments
can explain the two ways of getting PLMR and PHMR.
One is to directly get the info from a new defined interface VtdPmrInfo HOB.
The regions are reserved by silicon code.
The other is to reserved the regions using AllocatePages() which meet the
alignment requirement (the old way).
Thanks,
Ray
> -Original Message-
> From: Wang, Iwen Evelyn
> Sent: Friday, August 30, 2019 12:57 PM
> To: devel@edk2.groups.io
> Cc: Huang, Jenny ; Shih, More
> ; Ni, Ray ; Chaganty, Rangasai V
>
> Subject: [PATCH v5] IntelSiliconPkg-Vtd: A new PMR interface
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1770
>
> 1) IOMMU PMR feature should be generic to support different hardware
> architecture. Platforms may request no overlap between PMR regions and
> system reserve memory. Create an interface to control PLMR/PHMR regions.
> It allows silicon code to adjust PLMR/PHMR region base on the project needs.
>
> 2) DisableDMAr Function Code Optimization Optimize the flow to follow the
> VT-d spec requirements.
>
> 3) Renamed InitDmar() to InitGlobalVtd() The oringal function name is
> misleading
>
> 4) A new GetVtdPmrAlignmentLib for silicon code to get PMR alignment
> values.
>
> Signed-off-by: Evelyn Wang
> Cc: Jenny Huang
> Cc: More Shih
> Cc: Ray Ni
> Cc: Rangasai V Chaganty
>
> ---
> In V2:
> 1) Fixed the EFIAPI is missing in library API issue
> 2) Logs will be provided to make sure the backwards compatibility
> 3) Replaced BIT0 with EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL
> 4) Renamed GetVtdPmrAlignmentLib to PeiGetVtdPmrAlignmentLib
> 5) Fixed the indent in IntelVTdPmrPei.c
> 6) Follow VTd spec to define the data type of the SYSTEM_MEM_INFO_HOB
>Applied few changes coordinately
>
> ---
> In V3:
> 1) Fixed the EFIAPI is missing in library API issue
> 2) Fixed the S3 resume assert
>
> ---
> In V4:
> Fixed the missing EFIAPI in .h file and added few more comments
>
> ---
> In V5:
> In order to align with the future planning, changed the hob name from
> SYSTEM_MEM_INFO_HOB to VTD_PMR_INFO_HOB
> ---
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> | 30 +++---
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmr.c
> | 4 ++--
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> | 73 ++
> ---
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/VtdReg.c
> | 29 ++---
>
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/Platfor
> mVTdInfoSamplePei.c | 9 +
>
> Silicon/Intel/IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdP
> mrAlignmentLib.c | 166
> ++
> ++
> ++
>
> Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.in
> f | 5 -
> Silicon/Intel/IntelSiliconPkg/Include/Library/PeiGetVtdPmrAlignmentLib.h
> | 22 ++
> Silicon/Intel/IntelSiliconPkg/Include/VtdPmrInfoHob.h
> |
> 25 +
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> | 11
> +--
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> | 3 ++-
>
> Silicon/Intel/IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdP
> mrAlignmentLib.inf | 32
> 12 files changed, 370 insertions(+), 39 deletions(-)
>
> diff
