Re: [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.

["Zeng, Star" <star.zeng@intel.com>]

How about we also enhance the debug message like below?

    if (HasError) {
      DEBUG((DEBUG_INFO, "#### ERROR ####\n"));
      DumpVtdRegs (Num);
      DEBUG((DEBUG_INFO, "#### ERROR ####\n"));
    }

->

    if (HasError) {
      DEBUG((DEBUG_INFO, "\n#### ERROR ####\n"));
      DumpVtdRegs (Num);
      DEBUG((DEBUG_INFO, "#### ERROR ####\n\n"));
    }

Then the error message could be at a separated block for easy understanding.


Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Tuesday, September 5, 2017 11:30 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.

Remove zero address check in IoMmuMap.
The reason is that a CSM legacy driver may use legacy memory for DMA.
As such, the legacyBios need allow below 1M to the legacy device.

This patch also fixed some typo.

Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 IntelSiliconPkg/IntelVTdDxe/BmDma.c            | 8 ++++----
 IntelSiliconPkg/IntelVTdDxe/TranslationTable.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelSiliconPkg/IntelVTdDxe/BmDma.c b/IntelSiliconPkg/IntelVTdDxe/BmDma.c
index 5dcee00..7a5f361 100644
--- a/IntelSiliconPkg/IntelVTdDxe/BmDma.c
+++ b/IntelSiliconPkg/IntelVTdDxe/BmDma.c
@@ -77,14 +77,14 @@ IoMmuMap (
   EFI_PHYSICAL_ADDRESS                              DmaMemoryTop;
   BOOLEAN                                           NeedRemap;
 
-  DEBUG ((DEBUG_VERBOSE, "IoMmuMap: ==> 0x%08x - 0x%08x (%x)\n", HostAddress, NumberOfBytes, Operation));
-
-  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
+  if (NumberOfBytes == NULL || DeviceAddress == NULL ||
       Mapping == NULL) {
     DEBUG ((DEBUG_ERROR, "IoMmuMap: %r\n", EFI_INVALID_PARAMETER));
     return EFI_INVALID_PARAMETER;
   }
 
+  DEBUG ((DEBUG_VERBOSE, "IoMmuMap: ==> 0x%08x - 0x%08x (%x)\n", 
+ HostAddress, *NumberOfBytes, Operation));
+
   //
   // Make sure that Operation is valid
   //
@@ -135,7 +135,7 @@ IoMmuMap (
     if (NeedRemap) {
       //
       // Common Buffer operations can not be remapped.  If the common buffer
-      // if above 4GB, then it is not possible to generate a mapping, so return
+      // is above 4GB, then it is not possible to generate a mapping, 
+ so return
       // an error.
       //
       DEBUG ((DEBUG_ERROR, "IoMmuMap: %r\n", EFI_UNSUPPORTED)); diff --git a/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c b/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
index cd3111c..ccecc95 100644
--- a/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
+++ b/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
@@ -891,7 +891,7 @@ SetAccessAttribute (
 
   SecondLevelPagingEntry = NULL;
 
-  DEBUG ((DEBUG_INFO,"SetAccessAttribute (S%04x B%02x D%02x F%02x) (0x%016lx - 0x%08x, %x)\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function, BaseAddress, (UINTN)Length, IoMmuAccess));
+  DEBUG ((DEBUG_VERBOSE,"SetAccessAttribute (S%04x B%02x D%02x F%02x) 
+ (0x%016lx - 0x%08x, %x)\n", Segment, SourceId.Bits.Bus, 
+ SourceId.Bits.Device, SourceId.Bits.Function, BaseAddress, 
+ (UINTN)Length, IoMmuAccess));
 
   VtdIndex = FindVtdIndexByPciDevice (Segment, SourceId, &ExtContextEntry, &ContextEntry);
   if (VtdIndex == (UINTN)-1) {
--
2.7.4.windows.1