Nico Huber has posted comments on this change. ( https://review.coreboot.org/23682 )
Change subject: soc/intel/skylake: Add IOMMU support ......................................................................
Patch Set 1:
(3 comments)
I don't see any changes to the FSP UPDs. I guess you didn't test (or verify otherwise) which part of the configuration is done by the blob?
I think we should start with a minimal setup (i.e. only setup BARs) and see what the blob configures.
https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/bootblock/pch.... File src/soc/intel/skylake/bootblock/pch.c:
PS1: I don't think the boot block is the right place for this.
https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/bootblock/pch.... PS1, Line 64: /* Assign unique bus/dev/fn for I/O APIC and HPET */ Actually I've so far only read that you can assign unique numbers, but never why or for what it would be required. Do you know why?
https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/iommu.c File src/soc/intel/skylake/iommu.c:
https://review.coreboot.org/#/c/23682/1/src/soc/intel/skylake/iommu.c@40 PS1, Line 40: /* lock policies */ : write32((void *)(IOMMU_BASE_ADDRESS1 + 0xff0), 0x80000000); : : const struct device *const azalia = dev_find_slot(0, PCH_DEVFN_HDA); : if (azalia && azalia->enabled) { : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0x20000000); : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0xa0000000); : } else { : write32((void *)(IOMMU_BASE_ADDRESS2 + 0xff0), 0x80000000); : } I suppose this is just copy-pasta? Or is there any clue that Skylake works like this too?