1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/2] Enable IOMMU for CSMM
  4. View patch

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

Jiewen Yao posted 2 patches 8 years, 2 months ago
[edk2] [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.
Posted by Jiewen Yao 8 years, 2 months ago 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.
Posted by Zeng, Star 8 years, 2 months ago 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.
Posted by Yao, Jiewen 8 years, 2 months ago 
Good suggestion. It can make error message more readable.

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 6, 2017 10:14 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.
> 
> 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

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.