Attention is currently required from: Nico Huber, Angel Pons, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50857 )
Change subject: sb/intel/common: Refactor _PRT generation to support GSI-based tables ......................................................................
Patch Set 15:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50857/comment/39d406d3_170b8280 PS14, Line 10: insteade
Nit, *instead*
Done.
Patchset:
PS14:
Sorry for leaving this unattended for long. Ping me to test […]
Agreed, I am hesitant to submit without testing it on at least 1 of the older platforms. If this is broken, the OS will have a lot of trouble (experienced this plenty of times while this was in development...) I need to get my hands on some of these older mainboards...
File src/southbridge/intel/common/acpi_pirq_gen.h:
https://review.coreboot.org/c/coreboot/+/50857/comment/e0a59bb8_714346ac PS14, Line 28: PIRQ_MAX = PIRQ_H,
With `PIRQ_NONE` removed, this is the last valid index now.
Ack, added PIRQ_COUNT instead of PIRQ_MAX.
https://review.coreboot.org/c/coreboot/+/50857/comment/9bd087cf_8ea1d6ec PS14, Line 47: dveices
*devices*
Done
https://review.coreboot.org/c/coreboot/+/50857/comment/163f671a_c3d246cf PS14, Line 69: PIRQ_MAX
PIRQ_MAX is the last valid index, not the count of PIRQs.
Ouch, that's a good catch Nico! Will fix this.
https://review.coreboot.org/c/coreboot/+/50857/comment/448cf277_7ce6b748 PS14, Line 69: unsigned int gsi[PIRQ_MAX]; : char source_path[PIRQ_MAX][DEVICE_PATH_MAX];
This could be a union, just to emphasize that they are mutually exclusive.
That's a good hint to the user, will modify.
https://review.coreboot.org/c/coreboot/+/50857/comment/ee318c3e_259e8bb9 PS14, Line 79: num_devs
This made me think for a second what it's about. I suppose it's […]
You're right, it's map entries (or pins, that's valid too), will rename it.
File src/southbridge/intel/common/acpi_pirq_gen.c:
https://review.coreboot.org/c/coreboot/+/50857/comment/9beab3c4_e95f2952 PS14, Line 58: 16
This little number is nowhere to be found in the new implementation.
Good catch! related to your other comment about apic_gsi ;)
File src/southbridge/intel/common/rcba_pirq.c:
https://review.coreboot.org/c/coreboot/+/50857/comment/2e3aea70_4ab887e3 PS14, Line 85: pin_irq_map[num_devs].pic_pirq = map_pirq(dev, int_pin);
I don't think gen_apic_route() would be too happy if we leave . […]
Whoops this is where that little `16` was supposed to go 😄