Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43804 )
Change subject: /soc/amd/acpi: Move ACPI IVRS generation to coreboot ......................................................................
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43804/8/src/include/acpi/acpi_ivrs.... File src/include/acpi/acpi_ivrs.h:
https://review.coreboot.org/c/coreboot/+/43804/8/src/include/acpi/acpi_ivrs.... PS8, Line 59: #define IOMMU_FEATURE_HATS_SHIFT 20 /* Type 10h only */ : #define IOMMU_FEATURE_GATS_SHIFT 16 /* Type 10h only */ : #define IOMMU_FEATURE_MSI_NUM_PPR_SHIFT 5 : #define IOMMU_FEATURE_PN_BANKS_SHIFT 5 : #define IOMMU_FEATURE_PN_COUNTERS_SHIFT 6
Huh? Were the previous definitions wrong?
These values let you shift from the hw value read. I think they intended to use these with bitfields, but they aren't used anywhere else.
https://review.coreboot.org/c/coreboot/+/43804/8/src/include/acpi/acpi_ivrs.... PS8, Line 74: 1
(1 << 0) is more consistent with the rest
Ack
https://review.coreboot.org/c/coreboot/+/43804/8/src/include/acpi/acpi_ivrs.... PS8, Line 113: /// MMIO Offset 0x30
Are these hardware-agnostic definitions?
As far as i can tell, they are.