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 <[email protected]>; [email protected] > Cc: Zeng, Star <[email protected]> > 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: [email protected] > Cc: Zeng, Star <[email protected]> > 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 <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jiewen Yao <[email protected]> > --- > 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

