Re: [PATCH 04/11] IntelSiliconPkg/VTdDxe: Disable PMR

["Yao, Jiewen" <jiewen.yao@intel.com>]

Yes, I think the usage model is: Any DMA buffer MUST be requested explicitly.
We do not want to allow a driver calling AllocatePage(), then start to use the memory as DMA.

It can help:

1)       In case that the DMA buffer address is not identical.

2)       DMA protection.

Thank you
Yao Jiewen


From: Zeng, Star
Sent: Thursday, September 14, 2017 2:10 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 04/11] IntelSiliconPkg/VTdDxe: Disable PMR

Then, if there is a driver wants to do DMA at early DXE, it must need a PEIM to allocate the DMA buffers in PEI and transfer them to DXE by HOB.
Is that what we want? I want to confirm that.


Thanks,
Star
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, September 14, 2017 2:03 PM
To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH 04/11] IntelSiliconPkg/VTdDxe: Disable PMR

I did consider that before.
I do not disable at EndOfPei purposely that because I want to make sure that the DMA protect is still available in early DXE phase, just in case there is bug in other module which forgets disabling BME.

Later it is VTdDxe driver that disable PME, *after* it sets up translation table. As such, the DMA protection is always there.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 14, 2017 1:36 PM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: RE: [PATCH 04/11] IntelSiliconPkg/VTdDxe: Disable PMR
>
> A minor comment.
>
> Should or need IntelVTdPmrPei disable PMR at endofpei?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Friday, September 8, 2017 11:04 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: [PATCH 04/11] IntelSiliconPkg/VTdDxe: Disable PMR
>
> When VTd translation is enabled, PMR can be disable.
> Or the DMA will be blocked by PMR.
>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
>  IntelSiliconPkg/IntelVTdDxe/VtdReg.c | 51 +++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/IntelSiliconPkg/IntelVTdDxe/VtdReg.c
> b/IntelSiliconPkg/IntelVTdDxe/VtdReg.c
> index 7402d81..1404af7 100644
> --- a/IntelSiliconPkg/IntelVTdDxe/VtdReg.c
> +++ b/IntelSiliconPkg/IntelVTdDxe/VtdReg.c
> @@ -196,6 +196,39 @@ PrepareVtdConfig (  }
>
>  /**
> +  Disable PMR in all VTd engine.
> +**/
> +VOID
> +DisablePmr (
> +  VOID
> +  )
> +{
> +  UINT32        Reg32;
> +  VTD_CAP_REG   CapReg;
> +  UINTN         Index;
> +
> +  DEBUG ((DEBUG_INFO,"DisablePmr\n"));  for (Index = 0; Index <
> + mVtdUnitNumber; Index++) {
> +    CapReg.Uint64 = MmioRead64
> (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_CAP_REG);
> +    if (CapReg.Bits.PLMR == 0 || CapReg.Bits.PHMR == 0) {
> +      continue ;
> +    }
> +
> +    Reg32 = MmioRead32 (mVtdUnitInformation[Index].VtdUnitBaseAddress
> + +
> R_PMEN_ENABLE_REG);
> +    if ((Reg32 & BIT0) != 0) {
> +      MmioWrite32 (mVtdUnitInformation[Index].VtdUnitBaseAddress +
> R_PMEN_ENABLE_REG, 0x0);
> +      do {
> +        Reg32 = MmioRead32
> (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_PMEN_ENABLE_REG);
> +      } while((Reg32 & BIT0) != 0);
> +      DEBUG ((DEBUG_INFO,"Pmr(%d) disabled\n", Index));
> +    } else {
> +      DEBUG ((DEBUG_INFO,"Pmr(%d) not enabled\n", Index));
> +    }
> +  }
> +  return ;
> +}
> +
> +/**
>    Enable DMAR translation.
>
>    @retval EFI_SUCCESS           DMAR translation is enabled.
> @@ -259,6 +292,11 @@ EnableDmar (
>      DEBUG ((DEBUG_INFO,"VTD (%d) enabled!<<<<<<\n",Index));
>    }
>
> +  //
> +  // Need disable PMR, since we already setup translation table.
> +  //
> +  DisablePmr ();
> +
>    mVtdEnabled = TRUE;
>
>    return EFI_SUCCESS;
> @@ -502,7 +540,7 @@ DumpVtdIfError (
>      for (Index = 0; Index < (UINTN)CapReg.Bits.NFR + 1; Index++) {
>        FrcdReg.Uint64[0] = MmioRead64
> (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16)
> + (Index * 16) + R_FRCD_REG));
>        FrcdReg.Uint64[1] = MmioRead64
> (mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16)
> + (Index * 16) + R_FRCD_REG + sizeof(UINT64)));
> -      if ((FrcdReg.Uint64[0] != 0) || (FrcdReg.Uint64[1] != 0)) {
> +      if (FrcdReg.Bits.F != 0) {
>          HasError = TRUE;
>        }
>      }
> @@ -511,6 +549,17 @@ DumpVtdIfError (
>        DEBUG((DEBUG_INFO, "\n#### ERROR ####\n"));
>        DumpVtdRegs (Num);
>        DEBUG((DEBUG_INFO, "#### ERROR ####\n\n"));
> +      //
> +      // Clear
> +      //
> +      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;
> +          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));
> +      }
>      }
>    }
>  }
> --
> 2.7.4.windows.1