Change in coreboot[master]: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform
John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform
This change disables VT-d support and no DMAR table generation for ES platform. VT-d and DMAR ACPI table will only be supported for other platforms like QS.
BUG=b:152242800,161215918 TEST=Validated VT-d is disabled for ES(cpu:0x806c0) and enabled for QS(cpu:0x806c1). Kernel walks through ACPI tables. If VT-d is disabled and no DMAR table exists, IOMMU will not be enabled.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I98a9f6df185002a4e68eaa910f867acd0b96ec2b --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 21 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/43657/1
diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index 662ca06..2d7c05f 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -5,6 +5,7 @@ #include <cpu/x86/msr.h> #include <fsp/util.h> #include <intelblocks/cpulib.h> +#include <intelblocks/mp_init.h> #include <soc/gpio_soc_defs.h> #include <soc/iomap.h> #include <soc/msr.h> @@ -17,7 +18,7 @@ const struct soc_intel_tigerlake_config *config) { unsigned int i; - uint32_t mask = 0; + uint32_t cpu_id, mask = 0; const struct device *dev;
dev = pcidev_path_on_root(SA_DEVFN_IGD); @@ -182,18 +183,25 @@ sizeof(m_cfg->PchHdaAudioLinkSndwEnable));
/* Vt-D config */ - m_cfg->VtdDisable = 0; - m_cfg->VtdIgdEnable = 0x1; - m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; - m_cfg->VtdIpuEnable = 0x1; - m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; - m_cfg->VtdIopEnable = 0x1; - m_cfg->VtdBaseAddress[2] = VTVC0_BASE_ADDRESS; - m_cfg->VtdItbtEnable = 0x1; - m_cfg->VtdBaseAddress[3] = TBT0_BASE_ADDRESS; - m_cfg->VtdBaseAddress[4] = TBT1_BASE_ADDRESS; - m_cfg->VtdBaseAddress[5] = TBT2_BASE_ADDRESS; - m_cfg->VtdBaseAddress[6] = TBT3_BASE_ADDRESS; + cpu_id = cpu_get_cpuid(); + if (cpu_id == CPUID_TIGERLAKE_A0) { + /* Disable Vt-D support for ES platform */ + m_cfg->VtdDisable = 1; + } else { + /* Enable Vt-D support for other platforms */ + m_cfg->VtdDisable = 0; + m_cfg->VtdIgdEnable = 0x1; + m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; + m_cfg->VtdIpuEnable = 0x1; + m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; + m_cfg->VtdIopEnable = 0x1; + m_cfg->VtdBaseAddress[2] = VTVC0_BASE_ADDRESS; + m_cfg->VtdItbtEnable = 0x1; + m_cfg->VtdBaseAddress[3] = TBT0_BASE_ADDRESS; + m_cfg->VtdBaseAddress[4] = TBT1_BASE_ADDRESS; + m_cfg->VtdBaseAddress[5] = TBT2_BASE_ADDRESS; + m_cfg->VtdBaseAddress[6] = TBT3_BASE_ADDRESS; + }
/* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
Patch Set 1: Code-Review+1
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43657/1//COMMIT_MSG@9 PS1, Line 9: his change disables It will be also good to mention the reason "Why".
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Shamile Khan, Lalithambika Krishnakumar, Patrick Rudolph, Rajat Jain, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43657
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform
Enabling VT-d on pre-QS silicon may have issues like rendering the Thunderbolt driver useless. This change will ensure that VT-d is disabled for pre-QS silicon and enabled for QS.
BUG=b:152242800,161215918,158519322 TEST=Validated VT-d is disabled for ES(cpu:0x806c0) and enabled for QS(cpu:0x806c1). Kernel walks through ACPI tables. If VT-d is disabled and no DMAR table exists, IOMMU will not be enabled.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I98a9f6df185002a4e68eaa910f867acd0b96ec2b --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 21 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/43657/2
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43657/1//COMMIT_MSG@9 PS1, Line 9: his change disables
It will be also good to mention the reason "Why".
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for ES platform ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@9 PS2, Line 9: pre-QS silicon Please mention ES platform somewhere. Maybe:
pre-QS silicon (ES platform)
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@9 PS2, Line 9: Enabling VT-d on pre-QS silicon may have issues like rendering the : Thunderbolt driver useless. Please reference the datasheet/errata, where this is documented.
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@14 PS2, Line 14: ES(cpu:0x806c0) Please add a space before the (.
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@15 PS2, Line 15: QS(cpu:0x806c1) Please add a space before the (.
https://review.coreboot.org/c/coreboot/+/43657/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/2/src/soc/intel/tigerlake/rom... PS2, Line 188: Vt-D VT-d
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Shamile Khan, Lalithambika Krishnakumar, Patrick Rudolph, Rajat Jain, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43657
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform
Enabling VT-d on pre-QS silicon may have issues like rendering the Thunderbolt driver useless. This change will ensure that VT-d is disabled for pre-QS silicon and enabled for QS.
BUG=b:152242800,161215918,158519322 TEST=Validated VT-d is disabled for pre-QS (cpu:0x806c0) and enabled for QS (cpu:0x806c1). Kernel walks through ACPI tables. If VT-d is disabled and no DMAR table exists, IOMMU will not be enabled.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I98a9f6df185002a4e68eaa910f867acd0b96ec2b --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 21 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/43657/3
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@9 PS2, Line 9: pre-QS silicon
Please mention ES platform somewhere. Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@9 PS2, Line 9: Enabling VT-d on pre-QS silicon may have issues like rendering the : Thunderbolt driver useless.
Please reference the datasheet/errata, where this is documented.
The errata was discussed in b:158519322 and is disclosed under NDA, so we would prefer not adding it here.
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@14 PS2, Line 14: ES(cpu:0x806c0)
Please add a space before the (.
Done
https://review.coreboot.org/c/coreboot/+/43657/2//COMMIT_MSG@15 PS2, Line 15: QS(cpu:0x806c1)
Please add a space before the (.
Done
https://review.coreboot.org/c/coreboot/+/43657/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/2/src/soc/intel/tigerlake/rom... PS2, Line 188: Vt-D
VT-d
Done
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 3: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/3/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/3/src/soc/intel/tigerlake/rom... PS3, Line 199: 0x1 I know this isn't added in this change, but shouldn't VtdItbtEnable be set based on whether mainboard has TBT enabled? Same for the the base addresses below i.e. pass in address only for the devices that are really enabled by mainboard?
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Shamile Khan, Lalithambika Krishnakumar, Patrick Rudolph, Rajat Jain, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43657
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform
Enabling VT-d on pre-QS silicon may have issues like rendering the Thunderbolt driver useless. This change will ensure that VT-d is disabled for pre-QS silicon and enabled for QS.
BUG=b:152242800,161215918,158519322 TEST=Validated VT-d is disabled for pre-QS (cpu:0x806c0) and enabled for QS (cpu:0x806c1). Kernel walks through ACPI tables. If VT-d is disabled and no DMAR table exists, IOMMU will not be enabled.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I98a9f6df185002a4e68eaa910f867acd0b96ec2b --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 27 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/43657/4
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/3/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/3/src/soc/intel/tigerlake/rom... PS3, Line 199: 0x1
I know this isn't added in this change, but shouldn't VtdItbtEnable be set based on whether mainboar […]
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... PS4, Line 200: m_cfg->TcssDma0En || m_cfg->TcssDma1En I think this should be (m_cfg->TcssItbtPcie0En || m_cfg->TcssItbtPcie1En || m_cfg->TcssItbtPcie2En || m_cfg->TcssItbtPcie3En)
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... PS4, Line 200: m_cfg->TcssDma0En || m_cfg->TcssDma1En
I think this should be (m_cfg->TcssItbtPcie0En || m_cfg->TcssItbtPcie1En || m_cfg->TcssItbtPcie2En | […]
TcssDma0 is grouped with Pcie root ports 0/1 and TcssDma1 together with Pcie 2/3. fsp refers to VtdItbtEnable along with Pcie 0/1/2/3 enabling(through m_cfg->TcssItbtPcieXEn) while configuring Vtd->BaseAddress (fsp: ConfigureITbtVtBar function). It seems proper for VtdItbtEnable to be based on TcssDma0 and TcssDma1 enabling.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... PS4, Line 200: m_cfg->TcssDma0En || m_cfg->TcssDma1En
TcssDma0 is grouped with Pcie root ports 0/1 and TcssDma1 together with Pcie 2/3.
Yes, but still those are 2 separate PCI devices. And VtdItbtEnable is associated with TBT/USB4 devices and not the DMA controller devices.
It seems proper for VtdItbtEnable to be based on TcssDma0 and TcssDma1 enabling.
I am curious why is that? It seems like VtdItbtEnable is used to set the vtd base address for USB4 devices.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43657/4/src/soc/intel/tigerlake/rom... PS4, Line 200: m_cfg->TcssDma0En || m_cfg->TcssDma1En
TcssDma0 is grouped with Pcie root ports 0/1 and TcssDma1 together with Pcie 2/3. […]
TcssDmaX is actually referred to as USB4/Thunderbolt controller. Their acpi device names are associated as TcssDma0->TDM0 and TcssDma1->TDM1. Please ignore the name confusion.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43657 )
Change subject: soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform ......................................................................
soc/intel/tigerlake: Disable VT-d and no DMAR table for pre-QS platform
Enabling VT-d on pre-QS silicon may have issues like rendering the Thunderbolt driver useless. This change will ensure that VT-d is disabled for pre-QS silicon and enabled for QS.
BUG=b:152242800,161215918,158519322 TEST=Validated VT-d is disabled for pre-QS (cpu:0x806c0) and enabled for QS (cpu:0x806c1). Kernel walks through ACPI tables. If VT-d is disabled and no DMAR table exists, IOMMU will not be enabled.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I98a9f6df185002a4e68eaa910f867acd0b96ec2b Reviewed-on: https://review.coreboot.org/c/coreboot/+/43657 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 27 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index 662ca06..4a45fd4 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -5,6 +5,7 @@ #include <cpu/x86/msr.h> #include <fsp/util.h> #include <intelblocks/cpulib.h> +#include <intelblocks/mp_init.h> #include <soc/gpio_soc_defs.h> #include <soc/iomap.h> #include <soc/msr.h> @@ -17,7 +18,7 @@ const struct soc_intel_tigerlake_config *config) { unsigned int i; - uint32_t mask = 0; + uint32_t cpu_id, mask = 0; const struct device *dev;
dev = pcidev_path_on_root(SA_DEVFN_IGD); @@ -182,18 +183,31 @@ sizeof(m_cfg->PchHdaAudioLinkSndwEnable));
/* Vt-D config */ - m_cfg->VtdDisable = 0; - m_cfg->VtdIgdEnable = 0x1; - m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; - m_cfg->VtdIpuEnable = 0x1; - m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; - m_cfg->VtdIopEnable = 0x1; - m_cfg->VtdBaseAddress[2] = VTVC0_BASE_ADDRESS; - m_cfg->VtdItbtEnable = 0x1; - m_cfg->VtdBaseAddress[3] = TBT0_BASE_ADDRESS; - m_cfg->VtdBaseAddress[4] = TBT1_BASE_ADDRESS; - m_cfg->VtdBaseAddress[5] = TBT2_BASE_ADDRESS; - m_cfg->VtdBaseAddress[6] = TBT3_BASE_ADDRESS; + cpu_id = cpu_get_cpuid(); + if (cpu_id == CPUID_TIGERLAKE_A0) { + /* Disable VT-d support for pre-QS platform */ + m_cfg->VtdDisable = 1; + } else { + /* Enable VT-d support for QS platform */ + m_cfg->VtdDisable = 0; + m_cfg->VtdIgdEnable = 0x1; + m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; + m_cfg->VtdIpuEnable = 0x1; + m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; + m_cfg->VtdIopEnable = 0x1; + m_cfg->VtdBaseAddress[2] = VTVC0_BASE_ADDRESS; + + if (m_cfg->TcssDma0En || m_cfg->TcssDma1En) + m_cfg->VtdItbtEnable = 0x1; + if (m_cfg->TcssItbtPcie0En) + m_cfg->VtdBaseAddress[3] = TBT0_BASE_ADDRESS; + if (m_cfg->TcssItbtPcie1En) + m_cfg->VtdBaseAddress[4] = TBT1_BASE_ADDRESS; + if (m_cfg->TcssItbtPcie2En) + m_cfg->VtdBaseAddress[5] = TBT2_BASE_ADDRESS; + if (m_cfg->TcssItbtPcie3En) + m_cfg->VtdBaseAddress[6] = TBT3_BASE_ADDRESS; + }
/* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
-
Furquan Shaikh (Code Review) -
John Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Rajat Jain (Code Review)