[XL] Change in coreboot[main]: mainboard/emulation/qemu-sbsa: Add qemu-sbsa board

[David Milosevic (Code Review)]

Attention is currently required from: Angel Pons, Arthur Heymans, Benjamin Doron, Christian Walter, Lean Sheng Tan, Maximilian Brune, Naresh Solanki.

David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79086?usp=email )

Change subject: mainboard/emulation/qemu-sbsa: Add qemu-sbsa board
......................................................................

Patch Set 4:

(15 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/79086/comment/bf864d34_e93bc55f :
PS3, Line 7: sbsa
> Would be good to explain what the SBSA acronym means (Server Base System Architecture) somewhere.

Done

File Documentation/mainboard/emulation/qemu-sbsa.md:

https://review.coreboot.org/c/coreboot/+/79086/comment/9be93d17_09a3f1a1 :
PS1, Line 6: <TF-A>
> Can building TF-A be added to the makefile? It can be a in a follow-up.

I would prefer doing that in a follow up.

https://review.coreboot.org/c/coreboot/+/79086/comment/ac1c6b66_ce17c730 :
PS1, Line 6: qemu-system-aarch64 -nographic -m 1024 -M sbsa-ref -pflash <TF-A> \
           :                                                    -pflash ./build/coreboot.rom \
> Add a 'qemu' make target so that it is as simple as running 'make qemu'. […]

I would prefer doing that in a follow up.

File src/mainboard/emulation/qemu-sbsa/Kconfig:

https://review.coreboot.org/c/coreboot/+/79086/comment/59473875_279b4aa7 :
PS3, Line 41: 	default y
> Why?

Done

File src/mainboard/emulation/qemu-sbsa/Kconfig.name:

https://review.coreboot.org/c/coreboot/+/79086/comment/7e49734a_488f28e6 :
PS3, Line 4: o execute, do:
           : 	  qemu-system-aarch64 -nographic -m 1024 -M sbsa-ref -pflash <TF-A> -pflash ./build/coreboot.rom
> These instructions would be better placed in documentation

Done

File src/mainboard/emulation/qemu-sbsa/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/79086/comment/f53f2f14_e73e252e :
PS3, Line 23: 	truncate -s 256M $(obj)/coreboot.rom
> Why is this needed?

qemu-sbsa requires this, otherwise qemu does not start.

File src/mainboard/emulation/qemu-sbsa/acpi.c:

https://review.coreboot.org/c/coreboot/+/79086/comment/a6e26f0a_10fa45b6 :
PS3, Line 36: his
> Typo: his ---> This

Done

File src/mainboard/emulation/qemu-sbsa/board_info.txt:

https://review.coreboot.org/c/coreboot/+/79086/comment/ba28d20b_488d5d56 :
PS3, Line 3: http
> HTTPS, please. […]

Done

File src/mainboard/emulation/qemu-sbsa/flash.fmd:

https://review.coreboot.org/c/coreboot/+/79086/comment/68690061_685c3b29 :
PS1, Line 17: @0x0 CONFIG_ROM_SIZE
> not needed to specify

Done

https://review.coreboot.org/c/coreboot/+/79086/comment/5a033ee4_d54c45f6 :
PS1, Line 20: @0x20000
> can also be dropped.

Done

File src/mainboard/emulation/qemu-sbsa/include/mainboard/addressmap.h:

https://review.coreboot.org/c/coreboot/+/79086/comment/7e615446_8221ebf1 :
PS3, Line 4:  * Base addreses for QEMU sbsa-ref machine
> > `'addreses' may be misspelled - perhaps 'addresses'?` […]

Done

https://review.coreboot.org/c/coreboot/+/79086/comment/d7f7421b_eca97250 :
PS3, Line 32: #define SBSA_PCIE_MMIO_HIGH_SIZE 0xff00000000
> Maybe align the values with tabs?

Done

File src/mainboard/emulation/qemu-sbsa/mainboard.c:

https://review.coreboot.org/c/coreboot/+/79086/comment/d9fc4da1_a5dbd0b4 :
PS3, Line 19: 	return (size_t)cbmem_top() - (uintptr_t)_dram;;
> > `Statements terminations use 1 semicolon` […]

Done

https://review.coreboot.org/c/coreboot/+/79086/comment/1484947c_06607ddd :
PS3, Line 161: 		        struct device *cpu = alloc_dev(dev->link_list, &devpath);
> > `code indent should use tabs where possible` […]

Done

https://review.coreboot.org/c/coreboot/+/79086/comment/b93842c5_a60a88d8 :
PS3, Line 168:
> nit: drop empty line

Done