[coreboot-gerrit] [S] Change in coreboot[main]: soc/intel/xeon_sp: Reserve MMIO range for VTd BAR dynamically

[Nico Huber (Code Review)]

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.

Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/82431?usp=email )

Change subject: soc/intel/xeon_sp: Reserve MMIO range for VTd BAR dynamically
......................................................................

Patch Set 19: Code-Review+2

(3 comments)

File src/soc/intel/xeon_sp/include/soc/chip_common.h:

https://review.coreboot.org/c/coreboot/+/82431/comment/12094a51_cd9265da?usp... :
PS19, Line 93: uint32_t
`size_t` please.

File src/soc/intel/xeon_sp/uncore.c:

https://review.coreboot.org/c/coreboot/+/82431/comment/1c8cccc8_be13a6c6?usp... :
PS19, Line 61: 	uint32_t size = (~(pci_read_config32(dev, VTD_BAR_CSR) & ((uint32_t)(-4 * KiB)))) + 1;
This line has some unnecessary parentheses, which is more explicitly but
can actually make it harder to read in one go. I'd only use necessary
parentheses:
```
uint32_t size = ~(pci_read_config32(dev, VTD_BAR_CSR) & (uint32_t)(-4 * KiB)) + 1;
```

Also, what could help the reader would be to place the mask in a constant and
use that here and above, e.g.
```
const uint32_t size_mask = (uint32_t)(-4 * KiB);
```

https://review.coreboot.org/c/coreboot/+/82431/comment/103e9d5e_db7b75e2?usp... :
PS19, Line 63: 	pci_write_config32(dev, VTD_BAR_CSR, val);
I'd have guessed that the register is actually 64 bit? It wouldn't matter
in practice, as we don't expect >=4GiB BARs here.

If it is indeed 64 bits, a comment would be nice, though.