Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31917 )
Change subject: soc/intel/cnl: Generate DMAR ACPI table ......................................................................
Patch Set 16:
(5 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@362 PS10, Line 362: !(MCHBAR32(VTVC0BAR) & VTBAR_ENABLED)
If VtdDisable is false, sa_set_mch_bar(soc_vtd_resources, ... […]
Done
https://review.coreboot.org/#/c/31917/15/src/soc/intel/cannonlake/acpi.c File src/soc/intel/cannonlake/acpi.c:
https://review.coreboot.org/#/c/31917/15/src/soc/intel/cannonlake/acpi.c@355 PS15, Line 355: !(MCHBAR32(VTVC0BAR) & VTBAR_ENABLED))
Just added additional comments. […]
Ack
https://review.coreboot.org/#/c/31917/12/src/soc/intel/cannonlake/romstage/f... File src/soc/intel/cannonlake/romstage/fsp_params.c:
https://review.coreboot.org/#/c/31917/12/src/soc/intel/cannonlake/romstage/f... PS12, Line 119: tconfig->VtdDisable = config->VtdDisable;
VtdDisable and VtdBaseAddress[3] had been deprecated from FSP-S. They are in FSP-M. […]
Ack
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 42: sa_set_mch_bar(soc_vtd_resources, ARRAY_SIZE(soc_vtd_resources));
Sorry, I was lost and considered others. […]
Ah, I've realized now, that you dropped the entry in soc_vtd_resources[]. That fixes the ordering problem, but now the resource won't be reserved.
https://review.coreboot.org/#/c/31917/16/src/soc/intel/cannonlake/systemagen... File src/soc/intel/cannonlake/systemagent.c:
https://review.coreboot.org/#/c/31917/16/src/soc/intel/cannonlake/systemagen... PS16, Line 68: ARRAY_SIZE(soc_vtd_resources)); Here, the resources get reserved, but the IPU resource is missing now.
As we know that FSP will properly set the MCHBAR (and IPU config) registers, maybe we should only reserve soc_vtd_resources[] here and never set them in romstage?