Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Kyösti Mälkki, Michał Żygowski, Nico Huber.
Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email )
Change subject: device/pciexp_device.c: Fix setting Max Payload Size ......................................................................
Patch Set 18:
(8 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/78d71582_991bf0e7 : PS11, Line 630: root->max_payload_set = 1;
Current implementation looks good to me. Krystian, Patrick, do you want to […]
I've left comments in where I think the implementation still needs fixing for specific corner case. Marking this discussion as resolved.
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/ebd69af5_3fcd0bf3 : PS17, Line 771: Paylaod
Done
Done
https://review.coreboot.org/c/coreboot/+/77338/comment/ac6261e2_6bf46520 : PS17, Line 774: dvices
Done
Done
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/f2027548_09f8c322 : PS18, Line 596: unsigned int u16
https://review.coreboot.org/c/coreboot/+/77338/comment/459583c5_3dc0dc54 : PS18, Line 610: unsigned int u16
https://review.coreboot.org/c/coreboot/+/77338/comment/1820542c_09da347a : PS18, Line 623: unsigned int u16
https://review.coreboot.org/c/coreboot/+/77338/comment/dd4ec3a3_6e8a00fe : PS18, Line 653: pciexp_dev_set_max_payload_size(parent, max_payload); Would it make sense to put this inside `if` below?
https://review.coreboot.org/c/coreboot/+/77338/comment/2ad76b33_6d9a5214 : PS18, Line 721: parent's Shouldn't the root's (real one, not what `root` variable points to) Max Payload Size be limited here instead? If I understand this correctly, it only limits immediate parent's MPS and stops there, without propagating it further, so it may be overwritten by `pciexp_sync_max_payload_size()` called from `pciexp_scan_bus()`. This only matters for multi-level bridges, but it should be an easy fix.