* 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code [not found] ` <0C09AFA07DD0434D9E2A0C6AEB048310402DECF5@shsmsx102.ccr.corp.intel.com> @ 2019-05-13 11:27 ` Jerry Zhou(BJ-RD) 2019-05-20 3:32 ` Zeng, Star 0 siblings, 1 reply; 4+ messages in thread From: Jerry Zhou(BJ-RD) @ 2019-05-13 11:27 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org, devel@edk2.groups.io Cc: Yao, Jiewen, Ni, Ray [-- Attachment #1: Type: text/plain, Size: 7596 bytes --] Got it! Thanks for your reply. But you should still poll the B_GSTS_REG_TE bit, not the B_GSTS_REG_RTPS bit, in the judgement code of while() loop. After & operation between Reg32 and B_GSTS_REG_RTPS, the status of B_GSTS_REG_TE will be lost. A more tedious but more reliable operation sequence is recommended in Vt-d specification 2.4 below: to update a bit field in this register at offset X with value of Y, software must follow below steps: 1. Tmp = Read GSTS_REG 2. Status = (Tmp & 96FFFFFFh) // Reset the one-shot bits 3. Command = (Status | (Y << X)) 4. Write Command to GCMD_REG 5. Wait until GSTS_REG[X] indicates command is serviced. 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2019年5月13日 18:54 收件人: Jerry Zhou(BJ-RD); edk2-devel@lists.01.org 抄送: Yao, Jiewen; Ni, Ray; Zeng, Star 主题: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Good question, my understanding is setting B_GMCD_REG_SRTP(BIT30) ONLY also means clearing B_GMCD_REG_TE (BIT31). Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 10:59 AM To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Hi Star, I'am so interested in DMA protection in UEFI. It's a really good design! But I have a question about the implemention of DisableDmar() in IntelSiliconPkg\feature\vtd\intelvtddxe\VtdReg.c Is it a typing error in the code segment below? // // Disable VTd // MmioWrite32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GCMD_REG, B_GMCD_REG_SRTP); do { Reg32 = MmioRead32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GSTS_REG); } while((Reg32 & B_GSTS_REG_RTPS) == 0); The software should program the B_GMCD_REG_TE field in global command register and then poll the B_GSTS_REG_TE field in global status register if the DMAR is expected to be disabled or enabled according to Vt-d specification. Thanks Jerry Zhou Ext:892418 -----邮件原件----- 发件人: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] 代表 Star Zeng 发送时间: 2018年10月24日 11:32 收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Jiewen Yao; Star Zeng 主题: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early access attr request REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1272 To have high confidence in usage for platform, add option (BIT2 of PcdVTdPolicyPropertyMask) to force no IOMMU access attribute request recording before DMAR table is installed. Check PcdVTdPolicyPropertyMask BIT2 before RequestAccessAttribute() and ProcessRequestedAccessAttribute(), then RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables could be optimized by compiler when PcdVTdPolicyPropertyMask BIT2 = 1. Test done: 1: Created case that has IOMMU access attribute request before DMAR table is installed, ASSERT was triggered after setting PcdVTdPolicyPropertyMask BIT2 to 1. 2. Confirmed RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables were optimized by compiler after setting PcdVTdPolicyPropertyMask BIT2 to 1. Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com<mailto:rangasai.v.chaganty@intel.com>> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> --- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++++++- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 7 +++++++ IntelSiliconPkg/IntelSiliconPkg.dec | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c index 86d50eb6f288..7784545631b3 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c @@ -515,7 +515,13 @@ SetupVtd ( ParseDmarAcpiTableRmrr (); - ProcessRequestedAccessAttribute (); + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) == 0) { + // + // Support IOMMU access attribute request recording before DMAR table is installed. + // Here is to process the requests. + // + ProcessRequestedAccessAttribute (); + } for (Index = 0; Index < mVtdUnitNumber; Index++) { DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index, mVtdUnitInformation[Index].Segment)); diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c index 25d7c80af1d4..09948ce50e94 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c @@ -254,6 +254,13 @@ VTdSetAttribute ( // Record the entry to driver global variable. // As such once VTd is activated, the setting can be adopted. // + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) != 0) { + // + // Force no IOMMU access attribute request recording before DMAR table is installed. + // + ASSERT_EFI_ERROR (EFI_NOT_READY); + return EFI_NOT_READY; + } Status = RequestAccessAttribute (Segment, SourceId, DeviceAddress, Length, IoMmuAccess); } else { PERF_CODE ( diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec b/IntelSiliconPkg/IntelSiliconPkg.dec index b9646d773b95..900e8f63c64d 100644 --- a/IntelSiliconPkg/IntelSiliconPkg.dec +++ b/IntelSiliconPkg/IntelSiliconPkg.dec @@ -64,6 +64,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## The mask is used to control VTd behavior.<BR><BR> # BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If VTD_INFO_PPI is installed in PEI.) # BIT1: Enable IOMMU when transfer control to OS (ExitBootService in normal boot. EndOfPEI in S3) + # BIT2: Force no IOMMU access attribute request recording before DMAR table is installed. # @Prompt The policy for VTd driver behavior. gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x00000002 -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. [-- Attachment #2: Type: text/html, Size: 24827 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code 2019-05-13 11:27 ` 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Jerry Zhou(BJ-RD) @ 2019-05-20 3:32 ` Zeng, Star 2019-05-20 6:09 ` Yao, Jiewen 0 siblings, 1 reply; 4+ messages in thread From: Zeng, Star @ 2019-05-20 3:32 UTC (permalink / raw) To: Jerry Zhou(BJ-RD), edk2-devel@lists.01.org, devel@edk2.groups.io Cc: Yao, Jiewen, Ni, Ray, Zeng, Star [-- Attachment #1: Type: text/plain, Size: 8491 bytes --] Actually, I agree with you. Personally, I think more rigorous flow could be like below. 1. Clear B_GMCD_REG_TE, wait B_GSTS_REG_TE to be cleared. 2. Set B_GMCD_REG_SRTP, wait B_GSTS_REG_RTPS to be set. 3. Zero R_RTADDR_REG. Not sure original code developer Jiewen’s thought about this. You may submit Bugzilla at https://bugzilla.tianocore.org if you wait. Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 7:28 PM To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Got it! Thanks for your reply. But you should still poll the B_GSTS_REG_TE bit, not the B_GSTS_REG_RTPS bit, in the judgement code of while() loop. After & operation between Reg32 and B_GSTS_REG_RTPS, the status of B_GSTS_REG_TE will be lost. A more tedious but more reliable operation sequence is recommended in Vt-d specification 2.4 below: to update a bit field in this register at offset X with value of Y, software must follow below steps: 1. Tmp = Read GSTS_REG 2. Status = (Tmp & 96FFFFFFh) // Reset the one-shot bits 3. Command = (Status | (Y << X)) 4. Write Command to GCMD_REG 5. Wait until GSTS_REG[X] indicates command is serviced. 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2019年5月13日 18:54 收件人: Jerry Zhou(BJ-RD); edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Yao, Jiewen; Ni, Ray; Zeng, Star 主题: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Good question, my understanding is setting B_GMCD_REG_SRTP(BIT30) ONLY also means clearing B_GMCD_REG_TE (BIT31). Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 10:59 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Hi Star, I'am so interested in DMA protection in UEFI. It's a really good design! But I have a question about the implemention of DisableDmar() in IntelSiliconPkg\feature\vtd\intelvtddxe\VtdReg.c Is it a typing error in the code segment below? // // Disable VTd // MmioWrite32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GCMD_REG, B_GMCD_REG_SRTP); do { Reg32 = MmioRead32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GSTS_REG); } while((Reg32 & B_GSTS_REG_RTPS) == 0); The software should program the B_GMCD_REG_TE field in global command register and then poll the B_GSTS_REG_TE field in global status register if the DMAR is expected to be disabled or enabled according to Vt-d specification. Thanks Jerry Zhou Ext:892418 -----邮件原件----- 发件人: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] 代表 Star Zeng 发送时间: 2018年10月24日 11:32 收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Jiewen Yao; Star Zeng 主题: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early access attr request REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1272 To have high confidence in usage for platform, add option (BIT2 of PcdVTdPolicyPropertyMask) to force no IOMMU access attribute request recording before DMAR table is installed. Check PcdVTdPolicyPropertyMask BIT2 before RequestAccessAttribute() and ProcessRequestedAccessAttribute(), then RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables could be optimized by compiler when PcdVTdPolicyPropertyMask BIT2 = 1. Test done: 1: Created case that has IOMMU access attribute request before DMAR table is installed, ASSERT was triggered after setting PcdVTdPolicyPropertyMask BIT2 to 1. 2. Confirmed RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables were optimized by compiler after setting PcdVTdPolicyPropertyMask BIT2 to 1. Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com<mailto:rangasai.v.chaganty@intel.com>> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> --- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++++++- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 7 +++++++ IntelSiliconPkg/IntelSiliconPkg.dec | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c index 86d50eb6f288..7784545631b3 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c @@ -515,7 +515,13 @@ SetupVtd ( ParseDmarAcpiTableRmrr (); - ProcessRequestedAccessAttribute (); + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) == 0) { + // + // Support IOMMU access attribute request recording before DMAR table is installed. + // Here is to process the requests. + // + ProcessRequestedAccessAttribute (); + } for (Index = 0; Index < mVtdUnitNumber; Index++) { DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index, mVtdUnitInformation[Index].Segment)); diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c index 25d7c80af1d4..09948ce50e94 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c @@ -254,6 +254,13 @@ VTdSetAttribute ( // Record the entry to driver global variable. // As such once VTd is activated, the setting can be adopted. // + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) != 0) { + // + // Force no IOMMU access attribute request recording before DMAR table is installed. + // + ASSERT_EFI_ERROR (EFI_NOT_READY); + return EFI_NOT_READY; + } Status = RequestAccessAttribute (Segment, SourceId, DeviceAddress, Length, IoMmuAccess); } else { PERF_CODE ( diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec b/IntelSiliconPkg/IntelSiliconPkg.dec index b9646d773b95..900e8f63c64d 100644 --- a/IntelSiliconPkg/IntelSiliconPkg.dec +++ b/IntelSiliconPkg/IntelSiliconPkg.dec @@ -64,6 +64,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## The mask is used to control VTd behavior.<BR><BR> # BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If VTD_INFO_PPI is installed in PEI.) # BIT1: Enable IOMMU when transfer control to OS (ExitBootService in normal boot. EndOfPEI in S3) + # BIT2: Force no IOMMU access attribute request recording before DMAR table is installed. # @Prompt The policy for VTd driver behavior. gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x00000002 -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. [-- Attachment #2: Type: text/html, Size: 27919 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code 2019-05-20 3:32 ` Zeng, Star @ 2019-05-20 6:09 ` Yao, Jiewen 2019-05-20 6:31 ` 答复: " Jerry Zhou(BJ-RD) 0 siblings, 1 reply; 4+ messages in thread From: Yao, Jiewen @ 2019-05-20 6:09 UTC (permalink / raw) To: Zeng, Star, Jerry Zhou(BJ-RD), edk2-devel@lists.01.org, devel@edk2.groups.io Cc: Ni, Ray, Yao, Jiewen [-- Attachment #1: Type: text/plain, Size: 9126 bytes --] Thanks. We are also reviewing the VTd disabling flow and may update recently. If you want to file Bugzilla, please go ahead. Thank you Yao Jiewen From: Zeng, Star Sent: Sunday, May 19, 2019 8:33 PM To: Jerry Zhou(BJ-RD) <JerryZhou@zhaoxin.com>; edk2-devel@lists.01.org; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com> Subject: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Actually, I agree with you. Personally, I think more rigorous flow could be like below. 1. Clear B_GMCD_REG_TE, wait B_GSTS_REG_TE to be cleared. 2. Set B_GMCD_REG_SRTP, wait B_GSTS_REG_RTPS to be set. 3. Zero R_RTADDR_REG. Not sure original code developer Jiewen’s thought about this. You may submit Bugzilla at https://bugzilla.tianocore.org if you wait. Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 7:28 PM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Got it! Thanks for your reply. But you should still poll the B_GSTS_REG_TE bit, not the B_GSTS_REG_RTPS bit, in the judgement code of while() loop. After & operation between Reg32 and B_GSTS_REG_RTPS, the status of B_GSTS_REG_TE will be lost. A more tedious but more reliable operation sequence is recommended in Vt-d specification 2.4 below: to update a bit field in this register at offset X with value of Y, software must follow below steps: 1. Tmp = Read GSTS_REG 2. Status = (Tmp & 96FFFFFFh) // Reset the one-shot bits 3. Command = (Status | (Y << X)) 4. Write Command to GCMD_REG 5. Wait until GSTS_REG[X] indicates command is serviced. 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2019年5月13日 18:54 收件人: Jerry Zhou(BJ-RD); edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Yao, Jiewen; Ni, Ray; Zeng, Star 主题: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Good question, my understanding is setting B_GMCD_REG_SRTP(BIT30) ONLY also means clearing B_GMCD_REG_TE (BIT31). Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 10:59 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Hi Star, I'am so interested in DMA protection in UEFI. It's a really good design! But I have a question about the implemention of DisableDmar() in IntelSiliconPkg\feature\vtd\intelvtddxe\VtdReg.c Is it a typing error in the code segment below? // // Disable VTd // MmioWrite32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GCMD_REG, B_GMCD_REG_SRTP); do { Reg32 = MmioRead32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GSTS_REG); } while((Reg32 & B_GSTS_REG_RTPS) == 0); The software should program the B_GMCD_REG_TE field in global command register and then poll the B_GSTS_REG_TE field in global status register if the DMAR is expected to be disabled or enabled according to Vt-d specification. Thanks Jerry Zhou Ext:892418 -----邮件原件----- 发件人: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] 代表 Star Zeng 发送时间: 2018年10月24日 11:32 收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Jiewen Yao; Star Zeng 主题: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early access attr request REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1272 To have high confidence in usage for platform, add option (BIT2 of PcdVTdPolicyPropertyMask) to force no IOMMU access attribute request recording before DMAR table is installed. Check PcdVTdPolicyPropertyMask BIT2 before RequestAccessAttribute() and ProcessRequestedAccessAttribute(), then RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables could be optimized by compiler when PcdVTdPolicyPropertyMask BIT2 = 1. Test done: 1: Created case that has IOMMU access attribute request before DMAR table is installed, ASSERT was triggered after setting PcdVTdPolicyPropertyMask BIT2 to 1. 2. Confirmed RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables were optimized by compiler after setting PcdVTdPolicyPropertyMask BIT2 to 1. Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com<mailto:rangasai.v.chaganty@intel.com>> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> --- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++++++- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 7 +++++++ IntelSiliconPkg/IntelSiliconPkg.dec | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c index 86d50eb6f288..7784545631b3 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c @@ -515,7 +515,13 @@ SetupVtd ( ParseDmarAcpiTableRmrr (); - ProcessRequestedAccessAttribute (); + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) == 0) { + // + // Support IOMMU access attribute request recording before DMAR table is installed. + // Here is to process the requests. + // + ProcessRequestedAccessAttribute (); + } for (Index = 0; Index < mVtdUnitNumber; Index++) { DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index, mVtdUnitInformation[Index].Segment)); diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c index 25d7c80af1d4..09948ce50e94 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c @@ -254,6 +254,13 @@ VTdSetAttribute ( // Record the entry to driver global variable. // As such once VTd is activated, the setting can be adopted. // + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) != 0) { + // + // Force no IOMMU access attribute request recording before DMAR table is installed. + // + ASSERT_EFI_ERROR (EFI_NOT_READY); + return EFI_NOT_READY; + } Status = RequestAccessAttribute (Segment, SourceId, DeviceAddress, Length, IoMmuAccess); } else { PERF_CODE ( diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec b/IntelSiliconPkg/IntelSiliconPkg.dec index b9646d773b95..900e8f63c64d 100644 --- a/IntelSiliconPkg/IntelSiliconPkg.dec +++ b/IntelSiliconPkg/IntelSiliconPkg.dec @@ -64,6 +64,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## The mask is used to control VTd behavior.<BR><BR> # BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If VTD_INFO_PPI is installed in PEI.) # BIT1: Enable IOMMU when transfer control to OS (ExitBootService in normal boot. EndOfPEI in S3) + # BIT2: Force no IOMMU access attribute request recording before DMAR table is installed. # @Prompt The policy for VTd driver behavior. gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x00000002 -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. [-- Attachment #2: Type: text/html, Size: 35730 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code 2019-05-20 6:09 ` Yao, Jiewen @ 2019-05-20 6:31 ` Jerry Zhou(BJ-RD) 0 siblings, 0 replies; 4+ messages in thread From: Jerry Zhou(BJ-RD) @ 2019-05-20 6:31 UTC (permalink / raw) To: Yao, Jiewen, Zeng, Star, edk2-devel@lists.01.org, devel@edk2.groups.io Cc: Ni, Ray [-- Attachment #1: Type: text/plain, Size: 9927 bytes --] OK. I have submitted to Bugzilla. Bug 1823 - source code about disabling the DMAR of IOMMU in IntelSiliconPkg Thank you Jerry Zhou 发件人: Yao, Jiewen [mailto:jiewen.yao@intel.com] 发送时间: 2019年5月20日 14:09 收件人: Zeng, Star; Jerry Zhou(BJ-RD); edk2-devel@lists.01.org; devel@edk2.groups.io 抄送: Ni, Ray; Yao, Jiewen 主题: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Thanks. We are also reviewing the VTd disabling flow and may update recently. If you want to file Bugzilla, please go ahead. Thank you Yao Jiewen From: Zeng, Star Sent: Sunday, May 19, 2019 8:33 PM To: Jerry Zhou(BJ-RD) <JerryZhou@zhaoxin.com>; edk2-devel@lists.01.org; devel@edk2.groups.io Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com> Subject: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Actually, I agree with you. Personally, I think more rigorous flow could be like below. 1. Clear B_GMCD_REG_TE, wait B_GSTS_REG_TE to be cleared. 2. Set B_GMCD_REG_SRTP, wait B_GSTS_REG_RTPS to be set. 3. Zero R_RTADDR_REG. Not sure original code developer Jiewen’s thought about this. You may submit Bugzilla at https://bugzilla.tianocore.org if you wait. Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 7:28 PM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Got it! Thanks for your reply. But you should still poll the B_GSTS_REG_TE bit, not the B_GSTS_REG_RTPS bit, in the judgement code of while() loop. After & operation between Reg32 and B_GSTS_REG_RTPS, the status of B_GSTS_REG_TE will be lost. A more tedious but more reliable operation sequence is recommended in Vt-d specification 2.4 below: to update a bit field in this register at offset X with value of Y, software must follow below steps: 1. Tmp = Read GSTS_REG 2. Status = (Tmp & 96FFFFFFh) // Reset the one-shot bits 3. Command = (Status | (Y << X)) 4. Write Command to GCMD_REG 5. Wait until GSTS_REG[X] indicates command is serviced. 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2019年5月13日 18:54 收件人: Jerry Zhou(BJ-RD); edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Yao, Jiewen; Ni, Ray; Zeng, Star 主题: RE: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Good question, my understanding is setting B_GMCD_REG_SRTP(BIT30) ONLY also means clearing B_GMCD_REG_TE (BIT31). Thanks, Star From: Jerry Zhou(BJ-RD) [mailto:JerryZhou@zhaoxin.com] Sent: Monday, May 13, 2019 10:59 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Subject: 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Hi Star, I'am so interested in DMA protection in UEFI. It's a really good design! But I have a question about the implemention of DisableDmar() in IntelSiliconPkg\feature\vtd\intelvtddxe\VtdReg.c Is it a typing error in the code segment below? // // Disable VTd // MmioWrite32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GCMD_REG, B_GMCD_REG_SRTP); do { Reg32 = MmioRead32 (mVtdUnitInformation[Index].VtdUnitBaseAddress + R_GSTS_REG); } while((Reg32 & B_GSTS_REG_RTPS) == 0); The software should program the B_GMCD_REG_TE field in global command register and then poll the B_GSTS_REG_TE field in global status register if the DMAR is expected to be disabled or enabled according to Vt-d specification. Thanks Jerry Zhou Ext:892418 -----邮件原件----- 发件人: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] 代表 Star Zeng 发送时间: 2018年10月24日 11:32 收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Jiewen Yao; Star Zeng 主题: [edk2] [PATCH] IntelSiliconPkg VTdDxe: Option to force no early access attr request REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1272 To have high confidence in usage for platform, add option (BIT2 of PcdVTdPolicyPropertyMask) to force no IOMMU access attribute request recording before DMAR table is installed. Check PcdVTdPolicyPropertyMask BIT2 before RequestAccessAttribute() and ProcessRequestedAccessAttribute(), then RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables could be optimized by compiler when PcdVTdPolicyPropertyMask BIT2 = 1. Test done: 1: Created case that has IOMMU access attribute request before DMAR table is installed, ASSERT was triggered after setting PcdVTdPolicyPropertyMask BIT2 to 1. 2. Confirmed RequestAccessAttribute(), ProcessRequestedAccessAttribute() and mAccessRequestXXX variables were optimized by compiler after setting PcdVTdPolicyPropertyMask BIT2 to 1. Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com<mailto:rangasai.v.chaganty@intel.com>> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> --- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++++++- IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 7 +++++++ IntelSiliconPkg/IntelSiliconPkg.dec | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c index 86d50eb6f288..7784545631b3 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c @@ -515,7 +515,13 @@ SetupVtd ( ParseDmarAcpiTableRmrr (); - ProcessRequestedAccessAttribute (); + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) == 0) { + // + // Support IOMMU access attribute request recording before DMAR table is installed. + // Here is to process the requests. + // + ProcessRequestedAccessAttribute (); + } for (Index = 0; Index < mVtdUnitNumber; Index++) { DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index, mVtdUnitInformation[Index].Segment)); diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c index 25d7c80af1d4..09948ce50e94 100644 --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c @@ -254,6 +254,13 @@ VTdSetAttribute ( // Record the entry to driver global variable. // As such once VTd is activated, the setting can be adopted. // + if ((PcdGet8 (PcdVTdPolicyPropertyMask) & BIT2) != 0) { + // + // Force no IOMMU access attribute request recording before DMAR table is installed. + // + ASSERT_EFI_ERROR (EFI_NOT_READY); + return EFI_NOT_READY; + } Status = RequestAccessAttribute (Segment, SourceId, DeviceAddress, Length, IoMmuAccess); } else { PERF_CODE ( diff --git a/IntelSiliconPkg/IntelSiliconPkg.dec b/IntelSiliconPkg/IntelSiliconPkg.dec index b9646d773b95..900e8f63c64d 100644 --- a/IntelSiliconPkg/IntelSiliconPkg.dec +++ b/IntelSiliconPkg/IntelSiliconPkg.dec @@ -64,6 +64,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## The mask is used to control VTd behavior.<BR><BR> # BIT0: Enable IOMMU during boot (If DMAR table is installed in DXE. If VTD_INFO_PPI is installed in PEI.) # BIT1: Enable IOMMU when transfer control to OS (ExitBootService in normal boot. EndOfPEI in S3) + # BIT2: Force no IOMMU access attribute request recording before DMAR table is installed. # @Prompt The policy for VTd driver behavior. gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask|1|UINT8|0x00000002 -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. 保密声明： 本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。 CONFIDENTIAL NOTE: This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited. [-- Attachment #2: Type: text/html, Size: 37084 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-20 6:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <a05c49623cee4f5d9e470787a3e80cf3@zhaoxin.com>
[not found] ` <0C09AFA07DD0434D9E2A0C6AEB048310402DECF5@shsmsx102.ccr.corp.intel.com>
2019-05-13 11:27 ` 答复: [edk2] [PATCH] IntelSiliconPkg VTdDxe: a question about the source code Jerry Zhou(BJ-RD)
2019-05-20 3:32 ` Zeng, Star
2019-05-20 6:09 ` Yao, Jiewen
2019-05-20 6:31 ` 答复: " Jerry Zhou(BJ-RD)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox