Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45056 )
Change subject: soc/amd/picasso/acpi: Add MADT entry for NB ioapic, if initialized. ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45056/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45056/2//COMMIT_MSG@21 PS2, Line 21: Signed-off-by
you could ditch one of these lines.
Ack
https://review.coreboot.org/c/coreboot/+/45056/2/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45056/2/src/soc/amd/picasso/acpi.c@... PS2, Line 55: 0 - max logical cpus - 1
This reads as 0 - max logical cpus. i.e., a negative number. […]
Ack
https://review.coreboot.org/c/coreboot/+/45056/2/src/soc/amd/picasso/acpi.c@... PS2, Line 64: nb_ioapic_base
you'll need to mask off some of the lower bits
Ack
https://review.coreboot.org/c/coreboot/+/45056/5/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45056/5/src/soc/amd/picasso/acpi.c@... PS5, Line 59: CONFIG_MAX_CPUS
I think we should do one of two things: […]
Agree. We are planning a separate change to add a upd so coreboot can dictate what ids it wants to use.