John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31917 )
Change subject: soc/intel/cnl: Generate DMAR ACPI table ......................................................................
Patch Set 12:
(4 comments)
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/acpi.c File src/soc/intel/cannonlake/acpi.c:
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/acpi.c@337 PS10, Line 337: const u16 ibdf = (V_P2SB_CFG_IBDF_BUS << 8) | : (V_P2SB_CFG_IBDF_DEV << 3) | V_P2SB_CFG_IBDF_FUNC; : const u16 hbdf = (V_P2SB_CFG_HBDF_BUS << 8) | : (V_P2SB_CFG_HBDF_DEV << 3) | V_P2SB_CFG_HBDF_FUNC; :
We are amidst early boot stages, why is POSTBOOT_SAI already effective?
P2sbconfigure/P2sbLock/P2sbSaiLock have been executed in silicon initialization. It appears relative late stage while we try to fill acpi table.
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/include/so... File src/soc/intel/cannonlake/include/soc/systemagent.h:
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/include/so... PS10, Line 59: #define V_P2SB_CFG_IBDF_BUS 0 : #define V_P2SB_CFG_IBDF_DEV 30 : #define V_P2SB_CFG_IBDF_FUNC 7 : #define V_P2SB_CFG_HBDF_BUS 0 : #define V_P2SB_CFG_HBDF_DEV 30 : #define V_P2SB_CFG_HBDF_FUNC 6
From where? is it document somewhere? or only in FSP source code?
From both of Cannon Lake PCH bios specification and fsp source code. IOxAPIC: IBDF(0x6c), the static BDF is 0:30:7 HPET: HBDF (0x70), the static BDF is 0:30:6
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/romstage/s... File src/soc/intel/cannonlake/romstage/systemagent.c:
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/romstage/s... PS10, Line 32: if (dev) : config = dev->chip_info; : if (config && config->VtdDisable) : return; :
But why? the devicetree is about the *device* i.e. hardware. How […]
Along with other fsp upd elements in the devicetree, VT-d can be optionally configured with VtdDisable flag. If VtdDisable is set true, fsp would skip VT-d configuration and no DMAR table generation. It appears properly to associate vmx with VT-d setting. We have Vtd test application running at uefi shell which validate all relevant VT-d configuration along with vmx enable/lock features.
https://review.coreboot.org/#/c/31917/10/src/soc/intel/cannonlake/romstage/s... PS10, Line 42: sa_set_mch_bar(soc_vtd_resources, ARRAY_SIZE(soc_vtd_resources));
I was refering to the Cannon Lake BWG. AFAICS, this code here […]
It is assumed you refer to the following: DMA Remapping hardware unit #2, IPUVTDBAR (for IPU) – BIOS must provide a 4KB memory region with the base address programmed in IPU Configuration register B0.D5.F0.R0F4h[31:0] and MCHBAR IPUVTDBAR register offset 0x7884. The IPU configuration register should be programmed first and BIOS should write to higher offset F4h first and then write to lower offset F0h. Next the MCHBAR TBD offset must be programmed with higher DW offset first and then write lower DW offset followed by enable bit. BIOS must make sure IPU DMA remapping engine base address values in IPU Configuration space and MCHBAR are the same.
Coreboot configures IPUVTDBAR (offset 0x7884) first. fsp code implements the following:
PciSegmentWrite32 (PCI_SEGMENT_LIB_ADDRESS (SA_SEG_NUM, SA_IPU_BUS_NUM, SA_IPU_DEV_NUM, SA_IPU_FUN_NUM, R_SA_VTD_IPU_UBAR), 0);
PciSegmentWrite32 (PCI_SEGMENT_LIB_ADDRESS (SA_SEG_NUM, SA_IPU_BUS_NUM, SA_IPU_DEV_NUM, SA_IPU_FUN_NUM, R_SA_VTD_IPU_LBAR), Data32Or);
MmioWrite32 (MchBar + R_SA_MCHBAR_VTD2_HIGH_OFFSET, 0); MmioWrite32 (MchBar + R_SA_MCHBAR_VTD2_LOW_OFFSET, Data32Or);
It seems the configuration has been properly handled.