[PATCH] IntelSiliconPkg IntelVTdDxe: Fix incorrect code to clear VTd error

[PATCH] IntelSiliconPkg IntelVTdDxe: Fix incorrect code to clear VTd error

[Star Zeng]

According to VTd spec, Software writes the value read from this
field (F) to Clear it. But current code is using 0 to clear the
field, that is incorrect.

And R_FSTS_REG register value clearing should be not in the for loop.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
index 8dbc83fa2d67..e564d373c756 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
@@ -554,11 +554,13 @@ DumpVtdIfError (
       for (Index = 0; Index < (UINTN)CapReg.Bits.NFR + 1; Index++) {
         FrcdReg.Uint64[1] = MmioRead64 (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16) + (Index * 16) + R_FRCD_REG + sizeof(UINT64)));
         if (FrcdReg.Bits.F != 0) {
-          FrcdReg.Bits.F = 0;
+          //
+          // Software writes the value read from this field (F) to Clear it.
+          //
           MmioWrite64 (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16) + (Index * 16) + R_FRCD_REG + sizeof(UINT64)), FrcdReg.Uint64[1]);
         }
-        MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
       }
+      MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
     }
   }
 }
-- 
2.7.0.windows.1

Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Fix incorrect code to clear VTd error

[Yao, Jiewen]

Thanks to fix it.

Reviewed by: jiewen.yao@intel.com

thank you!
Yao, Jiewen


> 在 2018年6月5日，上午1:07，Zeng, Star <star.zeng@intel.com> 写道：
> 
> According to VTd spec, Software writes the value read from this
> field (F) to Clear it. But current code is using 0 to clear the
> field, that is incorrect.
> 
> And R_FSTS_REG register value clearing should be not in the for loop.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> index 8dbc83fa2d67..e564d373c756 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
> @@ -554,11 +554,13 @@ DumpVtdIfError (
>       for (Index = 0; Index < (UINTN)CapReg.Bits.NFR + 1; Index++) {
>         FrcdReg.Uint64[1] = MmioRead64 (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16) + (Index * 16) + R_FRCD_REG + sizeof(UINT64)));
>         if (FrcdReg.Bits.F != 0) {
> -          FrcdReg.Bits.F = 0;
> +          //
> +          // Software writes the value read from this field (F) to Clear it.
> +          //
>           MmioWrite64 (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16) + (Index * 16) + R_FRCD_REG + sizeof(UINT64)), FrcdReg.Uint64[1]);
>         }
> -        MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
>       }
> +      MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
>     }
>   }
> }
> -- 
> 2.7.0.windows.1
>