Attention is currently required from: Angel Pons, Benjamin Doron, Christian Walter, Lean Sheng Tan, Patrick Rudolph, Paul Menzel.
Leon Groß has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77905?usp=email )
Change subject: mb/emulation: Add SIMICS QSP support ......................................................................
Patch Set 13:
(5 comments)
File src/mainboard/emulation/simics-qsp/Kconfig:
https://review.coreboot.org/c/coreboot/+/77905/comment/4876a82d_ed6c1ad8 : PS4, Line 112: 128
1 as the supported CPU count isn't passed from emulator to guest like it's done on qemu. […]
Done
File src/mainboard/emulation/simics-qsp/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/77905/comment/830a62dc_0ec33eb6 : PS4, Line 365: /**************************************************************** : * General purpose events : ****************************************************************/ : : Scope(_GPE) { : Name(_HID, "ACPI0006") : : Method(_L00) { : } : Method(_L01) { : } : Method(_L02) { : } : Method(_L03) { : } : Method(_L04) { : } : Method(_L05) { : } : Method(_L06) { : } : Method(_L07) { : } : Method(_L08) { : } : Method(_L09) { : } : Method(_L0A) { : } : Method(_L0B) { : } : Method(_L0C) { : } : Method(_L0D) { : } : Method(_L0E) { : } : Method(_L0F) { : } : }
remove?
Done
File src/mainboard/emulation/simics-qsp/gpio.c:
https://review.coreboot.org/c/coreboot/+/77905/comment/bdd442ba_d1f0907a : PS4, Line 87: mainboard_gpio_map
No, I think we can remove this.
Correction: Since the init function of the southbridge relies on that, we should maybe mock it instead of removing it entirely / creating a new south bridge init that does not rely on this.
File src/mainboard/emulation/simics-qsp/memmap.c:
https://review.coreboot.org/c/coreboot/+/77905/comment/21c00699_82032ed0 : PS4, Line 39: uint32_t make_pciexbar(void)
Yes, we can remove this as well as the encoding and the defintion in the header.
Done
File src/mainboard/emulation/simics-qsp/northbridge.c:
https://review.coreboot.org/c/coreboot/+/77905/comment/8ea9899a_8cc8dada : PS4, Line 218: tatic void northbridge_enable(struct device *dev) : { : /* Set the operations if it is a special bus type */ : if (dev->path.type == DEVICE_PATH_DOMAIN) { : dev->ops = &pci_domain_ops; : } : else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) { : dev->ops = &cpu_bus_ops; : } : }
Set these with ops in devicetree rather than at runtime.
Do you mean putting something in the device tree like `device cpu_cluster 0 on ops cpu_bus_ops end` and then the same for the the domain or am I misunderstanding?
-
Leon Groß (Code Review)