[PATCH 0/8] Testcase improvements and Power8E/NVL removal
Overview ======== This patch series merges two patch series sent earlier: "Testcase improvements for PowerNV" "Remove deprecated Power8E and 8NVL CPUs" since the Power8E removal series depended on the former due to pnv-spi test Broadly contains 3 changes: 1. Testcase improvements: Add a testcase to detect regression in remote interrupts when XIVE changes, since such regression had occurred during Power11 XIVE changes: https://lore.kernel.org/qemu-devel/baf6c854-832b-4a2e-922f-d34e6dadf821@redh... Also add Power11 chip and testcase in qtests, and fadump functional test 2. Power8E/Power8NVL removal Power8E and Power8NVL were deprecated since QEMU 10.1, with commit 264a604e7163 ("target/ppc: Deprecate Power8E and Power8NVL") Hence, remove usage (chip and core types) of Power8E and 8NVL from PowerNV, PSeries as well as testcases. 3. Maintainer changes Add myself as maintainer for PowerNV machine Testing ======= * Git tree for testing: https://gitlab.com/adi-g15-ibm/qemu/tree/feat_maintainer_refresh * Gitlab pipeline: https://gitlab.com/adi-g15-ibm/qemu/-/pipelines/2622995621 * Successful ppc64 functional tests, qtests, boot tests, builds at all commits Changelog ========= from previous split series: + #1: added checks to see if network is up and interrupts happened, based on shivang's review + #3: replaced common if-else-if check for machine type with pnv_get_machine_type + #7: added doc in removed-features, as suggested by cedric + #6,7: added rb from philippe + #8: new patch, maintainer change Aditya Gupta (8): tests/functional: Add remote interrupts test for PowerNV tests/qtest/pnv_spi: Test Power11 PNV_SPI tests/qtest: Add Power11 chip & machine to qtests tests/functional: Use default powernv machine instead of power10 ppc/pnv: Replace Power8E with Power11 for 'none' machine test ppc/pnv: Remove Power8E and Power8NVL pnv chips ppc/pnv: Remove Power8E and Power8NVL CPUs MAINTAINERS: Add self as maintainer for PowerNV MAINTAINERS | 3 +- docs/about/deprecated.rst | 9 --- docs/about/removed-features.rst | 8 +++ docs/system/ppc/powernv.rst | 2 +- docs/system/ppc/pseries.rst | 2 +- hw/ppc/pnv.c | 62 ------------------- hw/ppc/pnv_core.c | 2 - hw/ppc/spapr_cpu_core.c | 2 - include/hw/ppc/pnv.h | 8 --- target/ppc/cpu-models.c | 6 -- target/ppc/cpu-models.h | 4 -- target/ppc/cpu_init.c | 6 -- target/ppc/kvm.c | 4 +- tests/functional/ppc64/test_fadump.py | 2 +- tests/functional/ppc64/test_powernv.py | 86 ++++++++++++++++++++++++++ tests/qtest/machine-none-test.c | 2 +- tests/qtest/pnv-host-i2c-test.c | 7 +-- tests/qtest/pnv-spi-seeprom-test.c | 21 +++++-- tests/qtest/pnv-xive2-test.c | 2 +- tests/qtest/pnv-xscom-test.c | 19 ++---- tests/qtest/pnv-xscom.h | 42 +++++++++---- 21 files changed, 154 insertions(+), 145 deletions(-) base-commit: b83371668192a705b878e909c5ae9c1233cbd5fb -- 2.54.0
In the past there have been hard to recreate issues where XIVE changes cause qemu crashes due to multi-socket interrupts such as in [1]. Add a functional test explicitly to test whether remote interrupts work. The test can also work as additional boot test for multi-socket boot, initrd boot test, as well as a check for e1000e to be working in powernv, though that's not a target goal, and are additional benefits. From docs/system/devices/net.rst: In order to check that the user mode network is working, you can ping the address 10.0.2.2 and verify that you got an address in the range 10.0.2.x from the QEMU virtual DHCP server. Hence use 10.0.2.2 with ping. [1]: https://lore.kernel.org/qemu-devel/baf6c854-832b-4a2e-922f-d34e6dadf821@redh... Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/functional/ppc64/test_powernv.py | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/functional/ppc64/test_powernv.py b/tests/functional/ppc64/test_powernv.py index 0ea6c93e4287..17d2447815f9 100755 --- a/tests/functional/ppc64/test_powernv.py +++ b/tests/functional/ppc64/test_powernv.py @@ -9,6 +9,7 @@ from qemu_test import LinuxKernelTest, Asset from qemu_test import wait_for_console_pattern +from qemu_test import exec_command_and_wait_for_pattern class PowernvMachine(LinuxKernelTest): @@ -27,6 +28,9 @@ class PowernvMachine(LinuxKernelTest): 'buildroot/qemu_ppc64le_powernv8-2025.02/rootfs.ext2'), 'aee2192b692077c4bde31cb56ce474424b358f17cec323d5c94af3970c9aada2') + def helper_exec_shell_command(self, command): + exec_command_and_wait_for_pattern(self, command, '#', self.panic_message) + def do_test_linux_boot(self, command_line = KERNEL_COMMON_COMMAND_LINE): self.require_accelerator("tcg") kernel_path = self.ASSET_KERNEL.fetch() @@ -71,6 +75,88 @@ def test_linux_smt_boot(self): wait_for_console_pattern(self, console_pattern, self.panic_message) wait_for_console_pattern(self, self.good_message, self.panic_message) + def test_linux_remote_interrupts(self): + self.require_accelerator("tcg") + self.set_machine('powernv') + + # have more sockets with as few CPUs as possible, increasing the + # probability to have remote interrupts from one chip to another + # also have e1000e network device to generate interrupts + self.vm.add_args('-smp', '4,sockets=4,threads=1') + self.vm.add_args('-device', 'e1000e,netdev=net0') + self.vm.add_args('-netdev', 'user,id=net0') + + kernel_path = self.ASSET_KERNEL.fetch() + rootfs_path = self.ASSET_INITRD.fetch() + self.vm.set_console() + self.vm.add_args('-kernel', kernel_path, + '-drive', + f'file={rootfs_path},format=raw,if=none,id=drive0,readonly=on', + '-append', 'root=/dev/nvme0n1 console=hvc0', + '-device', 'nvme,drive=drive0,bus=pcie.2,addr=0x0,serial=1234') + self.vm.launch() + + # Wait for boot to complete + console_pattern = 'CPU maps initialized for 1 thread per core' + wait_for_console_pattern(self, console_pattern, self.panic_message) + console_pattern = 'smp: Brought up 4 nodes, 4 CPUs' + wait_for_console_pattern(self, console_pattern, self.panic_message) + wait_for_console_pattern(self, 'Run /sbin/init as init process', + self.panic_message) + + # Wait for login prompt and login as root (no password in buildroot) + wait_for_console_pattern(self, 'login:', self.panic_message) + exec_command_and_wait_for_pattern(self, 'root', '#', self.panic_message) + + # RX, TX interrupts to chip/cpu 1 & 2 respectively + self.helper_exec_shell_command( + "export RX_IRQ=$(awk '/eth0-rx/ {print $1}' /proc/interrupts | tr -d ':')") + self.helper_exec_shell_command( + "export TX_IRQ=$(awk '/eth0-tx/ {print $1}' /proc/interrupts | tr -d ':')") + self.helper_exec_shell_command("echo 1 > /proc/irq/$RX_IRQ/smp_affinity_list") + self.helper_exec_shell_command("echo 2 > /proc/irq/$TX_IRQ/smp_affinity_list") + + # Capture interrupt counts before generating traffic + self.helper_exec_shell_command( + "export RX_BEFORE=$(awk '/eth0-rx/ {print $3}' /proc/interrupts)") + self.helper_exec_shell_command( + "export TX_BEFORE=$(awk '/eth0-tx/ {print $4}' /proc/interrupts)") + + # Wait up to 15 seconds for eth0 link to come up + self.helper_exec_shell_command( + "c=0; while ! ip addr show eth0 | grep 'inet 10.0.2'; do " + "sleep 1; c=$((c+1)); [ $c -gt 15 ] && break; done") + + fail_msg="Fail" + self.helper_exec_shell_command(f"export FAIL={fail_msg}") + exec_command_and_wait_for_pattern(self, + "ip addr show eth0 | grep 'inet 10.0.2' || echo $FAIL", + '#', fail_msg) + + # Generate network traffic to trigger remote interrupts + # Ping QEMU's user-mode network gateway (10.0.2.2) + self.helper_exec_shell_command("ping -W2 -c5 10.0.2.2") + + # Show final interrupt counts to verify remote interrupts occurred + self.helper_exec_shell_command("cat /proc/interrupts | grep eth0") + + # Verify interrupt counts increased (whether interrupts were delivered) + self.helper_exec_shell_command( + "export RX_AFTER=$(awk '/eth0-rx/ {print $3}' /proc/interrupts)") + self.helper_exec_shell_command( + "export TX_AFTER=$(awk '/eth0-tx/ {print $4}' /proc/interrupts)") + + # Check that interrupt counts increased + success_msg = "Interrupts increased" + self.helper_exec_shell_command(f"export PASS='{success_msg}'") + + exec_command_and_wait_for_pattern(self, + "[ $RX_AFTER -gt $RX_BEFORE ] && echo $PASS || echo $FAIL", + success_msg, fail_msg) + exec_command_and_wait_for_pattern(self, + "[ $TX_AFTER -gt $TX_BEFORE ] && echo $PASS || echo $FAIL", + success_msg, fail_msg) + def test_linux_big_boot(self): self.set_machine('powernv') self.vm.add_args('-smp', '16,threads=4,cores=2,sockets=2') -- 2.54.0
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+ # Check that interrupt counts increased + success_msg = "Interrupts increased" + self.helper_exec_shell_command(f"export PASS='{success_msg}'") This is a great idea. Maybe in future we can pull this to a saperate function? something like the following.
self.is_booted = True shell_check_pass_fail_command("asdf") ~Shivang.
On 26/06/23 09:17PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+ # Check that interrupt counts increased + success_msg = "Interrupts increased" + self.helper_exec_shell_command(f"export PASS='{success_msg}'") This is a great idea. Maybe in future we can pull this to a saperate function? something like the following.
self.is_booted = True shell_check_pass_fail_command("asdf")
Makes sense. Will have a helper in v2. Thanks, - Aditya G
~Shivang.
On 6/23/26 16:19, Aditya Gupta wrote:
In the past there have been hard to recreate issues where XIVE changes cause qemu crashes due to multi-socket interrupts such as in [1].
Add a functional test explicitly to test whether remote interrupts work.
The test can also work as additional boot test for multi-socket boot, initrd boot test, as well as a check for e1000e to be working in powernv, though that's not a target goal, and are additional benefits.
From docs/system/devices/net.rst:
In order to check that the user mode network is working, you can ping the address 10.0.2.2 and verify that you got an address in the range 10.0.2.x from the QEMU virtual DHCP server.
Hence use 10.0.2.2 with ping.
[1]: https://lore.kernel.org/qemu-devel/baf6c854-832b-4a2e-922f-d34e6dadf821@redh...
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/functional/ppc64/test_powernv.py | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)
diff --git a/tests/functional/ppc64/test_powernv.py b/tests/functional/ppc64/test_powernv.py index 0ea6c93e4287..17d2447815f9 100755 --- a/tests/functional/ppc64/test_powernv.py +++ b/tests/functional/ppc64/test_powernv.py @@ -9,6 +9,7 @@
from qemu_test import LinuxKernelTest, Asset from qemu_test import wait_for_console_pattern +from qemu_test import exec_command_and_wait_for_pattern
class PowernvMachine(LinuxKernelTest):
@@ -27,6 +28,9 @@ class PowernvMachine(LinuxKernelTest): 'buildroot/qemu_ppc64le_powernv8-2025.02/rootfs.ext2'), 'aee2192b692077c4bde31cb56ce474424b358f17cec323d5c94af3970c9aada2')
+ def helper_exec_shell_command(self, command): + exec_command_and_wait_for_pattern(self, command, '#', self.panic_message) + def do_test_linux_boot(self, command_line = KERNEL_COMMON_COMMAND_LINE): self.require_accelerator("tcg") kernel_path = self.ASSET_KERNEL.fetch() @@ -71,6 +75,88 @@ def test_linux_smt_boot(self): wait_for_console_pattern(self, console_pattern, self.panic_message) wait_for_console_pattern(self, self.good_message, self.panic_message)
+ def test_linux_remote_interrupts(self): + self.require_accelerator("tcg") + self.set_machine('powernv') + + # have more sockets with as few CPUs as possible, increasing the + # probability to have remote interrupts from one chip to another + # also have e1000e network device to generate interrupts
Can you attach the PCI device to a PCI bus known to be on the second socket ? and then offline cpus on the second socket, so that all IRQ servers are on the first socket.
+ self.vm.add_args('-smp', '4,sockets=4,threads=1') + self.vm.add_args('-device', 'e1000e,netdev=net0') + self.vm.add_args('-netdev', 'user,id=net0') + + kernel_path = self.ASSET_KERNEL.fetch() + rootfs_path = self.ASSET_INITRD.fetch() + self.vm.set_console() + self.vm.add_args('-kernel', kernel_path, + '-drive', + f'file={rootfs_path},format=raw,if=none,id=drive0,readonly=on', + '-append', 'root=/dev/nvme0n1 console=hvc0', + '-device', 'nvme,drive=drive0,bus=pcie.2,addr=0x0,serial=1234') + self.vm.launch() + + # Wait for boot to complete + console_pattern = 'CPU maps initialized for 1 thread per core' + wait_for_console_pattern(self, console_pattern, self.panic_message) + console_pattern = 'smp: Brought up 4 nodes, 4 CPUs' + wait_for_console_pattern(self, console_pattern, self.panic_message) + wait_for_console_pattern(self, 'Run /sbin/init as init process', + self.panic_message) + + # Wait for login prompt and login as root (no password in buildroot) + wait_for_console_pattern(self, 'login:', self.panic_message) + exec_command_and_wait_for_pattern(self, 'root', '#', self.panic_message) + + # RX, TX interrupts to chip/cpu 1 & 2 respectively + self.helper_exec_shell_command( + "export RX_IRQ=$(awk '/eth0-rx/ {print $1}' /proc/interrupts | tr -d ':')") + self.helper_exec_shell_command( + "export TX_IRQ=$(awk '/eth0-tx/ {print $1}' /proc/interrupts | tr -d ':')") + self.helper_exec_shell_command("echo 1 > /proc/irq/$RX_IRQ/smp_affinity_list") + self.helper_exec_shell_command("echo 2 > /proc/irq/$TX_IRQ/smp_affinity_list") + + # Capture interrupt counts before generating traffic + self.helper_exec_shell_command( + "export RX_BEFORE=$(awk '/eth0-rx/ {print $3}' /proc/interrupts)") + self.helper_exec_shell_command( + "export TX_BEFORE=$(awk '/eth0-tx/ {print $4}' /proc/interrupts)") + + # Wait up to 15 seconds for eth0 link to come up + self.helper_exec_shell_command( + "c=0; while ! ip addr show eth0 | grep 'inet 10.0.2'; do " + "sleep 1; c=$((c+1)); [ $c -gt 15 ] && break; done") + + fail_msg="Fail" + self.helper_exec_shell_command(f"export FAIL={fail_msg}") + exec_command_and_wait_for_pattern(self, + "ip addr show eth0 | grep 'inet 10.0.2' || echo $FAIL", + '#', fail_msg) + + # Generate network traffic to trigger remote interrupts + # Ping QEMU's user-mode network gateway (10.0.2.2) + self.helper_exec_shell_command("ping -W2 -c5 10.0.2.2") + + # Show final interrupt counts to verify remote interrupts occurred + self.helper_exec_shell_command("cat /proc/interrupts | grep eth0") + + # Verify interrupt counts increased (whether interrupts were delivered) + self.helper_exec_shell_command( + "export RX_AFTER=$(awk '/eth0-rx/ {print $3}' /proc/interrupts)") + self.helper_exec_shell_command( + "export TX_AFTER=$(awk '/eth0-tx/ {print $4}' /proc/interrupts)") + + # Check that interrupt counts increased + success_msg = "Interrupts increased" + self.helper_exec_shell_command(f"export PASS='{success_msg}'") + + exec_command_and_wait_for_pattern(self, + "[ $RX_AFTER -gt $RX_BEFORE ] && echo $PASS || echo $FAIL", + success_msg, fail_msg) + exec_command_and_wait_for_pattern(self, + "[ $TX_AFTER -gt $TX_BEFORE ] && echo $PASS || echo $FAIL", + success_msg, fail_msg) + def test_linux_big_boot(self): self.set_machine('powernv') self.vm.add_args('-smp', '16,threads=4,cores=2,sockets=2')
On 26/06/24 01:44PM, Cédric Le Goater wrote:
On 6/23/26 16:19, Aditya Gupta wrote:
<...snip...> def do_test_linux_boot(self, command_line = KERNEL_COMMON_COMMAND_LINE): self.require_accelerator("tcg") kernel_path = self.ASSET_KERNEL.fetch() @@ -71,6 +75,88 @@ def test_linux_smt_boot(self): wait_for_console_pattern(self, console_pattern, self.panic_message) wait_for_console_pattern(self, self.good_message, self.panic_message) + def test_linux_remote_interrupts(self): + self.require_accelerator("tcg") + self.set_machine('powernv') + + # have more sockets with as few CPUs as possible, increasing the + # probability to have remote interrupts from one chip to another + # also have e1000e network device to generate interrupts
Can you attach the PCI device to a PCI bus known to be on the second socket ? and then offline cpus on the second socket, so that all IRQ servers are on the first socket.
Makes sense, thanks cedric ! will keep the device attached to 4th socket, and offline it's CPU, and let the other setting affinity part same Thanks, - Aditya G
Currently pnv-spi-seeprom-test was hardcoded to test the 4th chip in pnv_chips (power10 as of now). This requires ensuring to update the index when removing/adding entries in pnv_chips, such as when Power8E or Power11 gets removed/added in future commits. Iterate over all the chips instead, similar to other tests in pnv-xscom-test.c and pnv-host-i2c-test.c, but skip older chips, since TYPE_PNV_SPI only exists from Power10 onwards, hence skip older machines Tests all the pnv_chips similar to other qtests Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/qtest/pnv-spi-seeprom-test.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/qtest/pnv-spi-seeprom-test.c b/tests/qtest/pnv-spi-seeprom-test.c index 44e0b92730b4..721129cd39ab 100644 --- a/tests/qtest/pnv-spi-seeprom-test.c +++ b/tests/qtest/pnv-spi-seeprom-test.c @@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; + const char *machine = "powernv10"; int ret; int fd; @@ -87,11 +88,11 @@ static void test_spi_seeprom(const void *data) g_assert(ret == 0); close(fd); - qts = qtest_initf("-machine powernv10 -smp 2,cores=2," + qts = qtest_initf("-machine %s -smp 2,cores=2," "threads=1 -accel tcg,thread=single -nographic " "-blockdev node-name=pib_spic2,driver=file," "filename=%s -device 25csm04,bus=chip0.spi.2,cs=0," - "drive=pib_spic2", tmp_path); + "drive=pib_spic2", machine, tmp_path); spi_seeprom_transaction(qts, chip); qtest_quit(qts); unlink(tmp_path); @@ -100,9 +101,17 @@ static void test_spi_seeprom(const void *data) int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); - char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", - pnv_chips[3].cpu_model); - qtest_add_data_func(tname, &pnv_chips[3], test_spi_seeprom); - g_free(tname); + + for (int i = 0; i < ARRAY_SIZE(pnv_chips); i++) { + /* TYPE_PNV_SPI is not instantiated for older Power8/9 machines */ + if (pnv_chips[i].chip_type < PNV_CHIP_POWER10) { + continue; + } + + char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", + pnv_chips[i].cpu_model); + qtest_add_data_func(tname, &pnv_chips[i], test_spi_seeprom); + g_free(tname); + } return g_test_run(); } -- 2.54.0
On 6/23/26 16:19, Aditya Gupta wrote:
Currently pnv-spi-seeprom-test was hardcoded to test the 4th chip in pnv_chips (power10 as of now).
This requires ensuring to update the index when removing/adding entries in pnv_chips, such as when Power8E or Power11 gets removed/added in future commits.
Iterate over all the chips instead, similar to other tests in pnv-xscom-test.c and pnv-host-i2c-test.c, but skip older chips, since TYPE_PNV_SPI only exists from Power10 onwards, hence skip older machines
Tests all the pnv_chips similar to other qtests
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/qtest/pnv-spi-seeprom-test.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/pnv-spi-seeprom-test.c b/tests/qtest/pnv-spi-seeprom-test.c index 44e0b92730b4..721129cd39ab 100644 --- a/tests/qtest/pnv-spi-seeprom-test.c +++ b/tests/qtest/pnv-spi-seeprom-test.c @@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; + const char *machine = "powernv10"; int ret; int fd;
@@ -87,11 +88,11 @@ static void test_spi_seeprom(const void *data) g_assert(ret == 0); close(fd);
- qts = qtest_initf("-machine powernv10 -smp 2,cores=2," + qts = qtest_initf("-machine %s -smp 2,cores=2," "threads=1 -accel tcg,thread=single -nographic " "-blockdev node-name=pib_spic2,driver=file," "filename=%s -device 25csm04,bus=chip0.spi.2,cs=0," - "drive=pib_spic2", tmp_path); + "drive=pib_spic2", machine, tmp_path); spi_seeprom_transaction(qts, chip); qtest_quit(qts); unlink(tmp_path); @@ -100,9 +101,17 @@ static void test_spi_seeprom(const void *data) int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); - char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", - pnv_chips[3].cpu_model); - qtest_add_data_func(tname, &pnv_chips[3], test_spi_seeprom); - g_free(tname); + + for (int i = 0; i < ARRAY_SIZE(pnv_chips); i++) { + /* TYPE_PNV_SPI is not instantiated for older Power8/9 machines */ + if (pnv_chips[i].chip_type < PNV_CHIP_POWER10) {
Can we remove this chip_type field from the tests too ? see 248e4e924e87.
+ continue; + } + + char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", + pnv_chips[i].cpu_model);
please consider g_autofree variables.
+ qtest_add_data_func(tname, &pnv_chips[i], test_spi_seeprom); + g_free(tname); + } return g_test_run(); }
On 26/06/24 01:47PM, Cédric Le Goater wrote:
On 6/23/26 16:19, Aditya Gupta wrote:
<...snip...>
g_test_init(&argc, &argv, NULL); - char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", - pnv_chips[3].cpu_model); - qtest_add_data_func(tname, &pnv_chips[3], test_spi_seeprom); - g_free(tname); + + for (int i = 0; i < ARRAY_SIZE(pnv_chips); i++) { + /* TYPE_PNV_SPI is not instantiated for older Power8/9 machines */ + if (pnv_chips[i].chip_type < PNV_CHIP_POWER10) {
Can we remove this chip_type field from the tests too ? see 248e4e924e87.
I went through the usage, and don't see any fix currently, as the PnvChip structure these qtests are using is basic, and we need some way to identify the chip type, for things like which machine to use, xscom addresses, and to skip/do specific things for particular chip type. I went through the mentioned commit and its parent, seems it was possible with the compat property being handled by respective class_init. So currently I don't see a better way, what do you think ?
+ continue; + } + + char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", + pnv_chips[i].cpu_model);
please consider g_autofree variables.
Sure cedric, will use it in v2. Thanks, - Aditya G
+ qtest_add_data_func(tname, &pnv_chips[i], test_spi_seeprom); + g_free(tname); + } return g_test_run(); }
On 6/24/26 16:28, Aditya Gupta wrote:
On 26/06/24 01:47PM, Cédric Le Goater wrote:
On 6/23/26 16:19, Aditya Gupta wrote:
<...snip...>
g_test_init(&argc, &argv, NULL); - char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", - pnv_chips[3].cpu_model); - qtest_add_data_func(tname, &pnv_chips[3], test_spi_seeprom); - g_free(tname); + + for (int i = 0; i < ARRAY_SIZE(pnv_chips); i++) { + /* TYPE_PNV_SPI is not instantiated for older Power8/9 machines */ + if (pnv_chips[i].chip_type < PNV_CHIP_POWER10) {
Can we remove this chip_type field from the tests too ? see 248e4e924e87.
I went through the usage, and don't see any fix currently, as the PnvChip structure these qtests are using is basic, and we need some way to identify the chip type, for things like which machine to use, xscom addresses, and to skip/do specific things for particular chip type. I went through the mentioned commit and its parent, seems it was possible with the compat property being handled by respective class_init. So currently I don't see a better way, what do you think ?
OK. Thanks for looking. C.
+ continue; + } + + char *tname = g_strdup_printf("pnv-xscom/spi-seeprom/%s", + pnv_chips[i].cpu_model);
please consider g_autofree variables.
Sure cedric, will use it in v2.
Thanks, - Aditya G
+ qtest_add_data_func(tname, &pnv_chips[i], test_spi_seeprom); + g_free(tname); + } return g_test_run(); }
Hi Aditya, One comment below. On 23/06/26 19:49, Aditya Gupta wrote:
@@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; + const char *machine = "powernv10";
Would it make sense to introduce `pnv_get_machine_type()` in patch 2 instead of patch 3? That would avoid hardcoding `"powernv10"` here and allow this patch to use the helper directly. Otherwise, the patch looks good to me. Thanks, Nikhil
On 26/06/30 09:55AM, Nikhil Kumar Singh wrote:
Hi Aditya,
One comment below.
On 23/06/26 19:49, Aditya Gupta wrote:
@@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; + const char *machine = "powernv10"; Would it make sense to introduce `pnv_get_machine_type()` in patch 2 instead of patch 3? That would avoid hardcoding `"powernv10"` here and allow this patch to use the helper directly.
Could be done, but i will prefer to keep it in current state, as of this patch, power11 wasn't added to this test, and the test only runs for powernv10, later as powern11 gets added in next patch this is changed to use pnv_get_machine_type. If pnv_get_machine_type gets introduced in this patch, it makes sense to modify all other tests also to use it, which is more than what this patch intends to do (just remove hardcoded index in spi test). To me, it seems okay to keep it as is, what do you say ?
Otherwise, the patch looks good to me.
Thanks for reviewing, Nikhil ! - Aditya G
Thanks, Nikhil
Hi Aditya,
One comment below.
On 23/06/26 19:49, Aditya Gupta wrote:
@@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; + const char *machine = "powernv10"; Would it make sense to introduce `pnv_get_machine_type()` in patch 2 instead of patch 3? That would avoid hardcoding `"powernv10"` here and allow this patch to use the helper directly. Could be done, but i will prefer to keep it in current state, as of this
On 26/06/30 09:55AM, Nikhil Kumar Singh wrote: patch, power11 wasn't added to this test, and the test only runs for powernv10, later as powern11 gets added in next patch this is changed to use pnv_get_machine_type.
If pnv_get_machine_type gets introduced in this patch, it makes sense to modify all other tests also to use it, which is more than what this patch intends to do (just remove hardcoded index in spi test). To me, it seems okay to keep it as is, what do you say ? Thanks for the explanation. I agree with keeping it as is for this
On 01/07/26 00:22, Aditya Gupta wrote: patch. Looks good to me. ~ Nikhil
Otherwise, the patch looks good to me. Thanks for reviewing, Nikhil !
- Aditya G
Thanks, Nikhil
Previously the machines/chips tested by qtest was till Power10, update the tests to also test PowerNV11 and Power11 PNV Chip Since if-else-if ladder was common pattern to get machine type, implement pnv_get_machine_type so new processor cases can be implemented in one location in pnv_get_machine_type Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/qtest/pnv-host-i2c-test.c | 7 +++---- tests/qtest/pnv-spi-seeprom-test.c | 2 +- tests/qtest/pnv-xive2-test.c | 2 +- tests/qtest/pnv-xscom-test.c | 19 ++++------------- tests/qtest/pnv-xscom.h | 33 +++++++++++++++++++++++++++--- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/tests/qtest/pnv-host-i2c-test.c b/tests/qtest/pnv-host-i2c-test.c index 51e613ebdcb2..d3f9c49d8271 100644 --- a/tests/qtest/pnv-host-i2c-test.c +++ b/tests/qtest/pnv-host-i2c-test.c @@ -402,15 +402,14 @@ static void reset_all(QTestState *qts, const PnvChip *chip) static void test_host_i2c(const void *data) { const PnvChip *chip = data; + const char *machine = pnv_get_machine_type(chip->chip_type); QTestState *qts; - const char *machine = "powernv8"; PnvI2cCtlr ctlr; PnvI2cDev pca9552; PnvI2cDev pca9554; - if (chip->chip_type == PNV_CHIP_POWER9) { - machine = "powernv9"; - } else if (chip->chip_type == PNV_CHIP_POWER10) { + /* i2c is initialised for rainier in case of P10 */ + if (chip->chip_type == PNV_CHIP_POWER10) { machine = "powernv10-rainier"; } diff --git a/tests/qtest/pnv-spi-seeprom-test.c b/tests/qtest/pnv-spi-seeprom-test.c index 721129cd39ab..5141df2dae01 100644 --- a/tests/qtest/pnv-spi-seeprom-test.c +++ b/tests/qtest/pnv-spi-seeprom-test.c @@ -77,7 +77,7 @@ static void test_spi_seeprom(const void *data) const PnvChip *chip = data; QTestState *qts = NULL; g_autofree char *tmp_path = NULL; - const char *machine = "powernv10"; + const char *machine = pnv_get_machine_type(chip->chip_type); int ret; int fd; diff --git a/tests/qtest/pnv-xive2-test.c b/tests/qtest/pnv-xive2-test.c index 5313d4ef18b7..80de9d09ed09 100644 --- a/tests/qtest/pnv-xive2-test.c +++ b/tests/qtest/pnv-xive2-test.c @@ -548,7 +548,7 @@ static void test_xive(void) { QTestState *qts; - qts = qtest_initf("-M powernv10 -smp %d,cores=1,threads=%d -nographic " + qts = qtest_initf("-M powernv11 -smp %d,cores=1,threads=%d -nographic " "-nodefaults -serial mon:stdio -S " "-d guest_errors -trace '*xive*'", SMT, SMT); diff --git a/tests/qtest/pnv-xscom-test.c b/tests/qtest/pnv-xscom-test.c index c814c0f4f5b1..6e52d71f4aaa 100644 --- a/tests/qtest/pnv-xscom-test.c +++ b/tests/qtest/pnv-xscom-test.c @@ -28,15 +28,9 @@ static void test_xscom_cfam_id(QTestState *qts, const PnvChip *chip) static void test_cfam_id(const void *data) { const PnvChip *chip = data; - const char *machine = "powernv8"; + const char *machine = pnv_get_machine_type(chip->chip_type); QTestState *qts; - if (chip->chip_type == PNV_CHIP_POWER9) { - machine = "powernv9"; - } else if (chip->chip_type == PNV_CHIP_POWER10) { - machine = "powernv10"; - } - qts = qtest_initf("-M %s -accel tcg -cpu %s", machine, chip->cpu_model); test_xscom_cfam_id(qts, chip); @@ -57,7 +51,8 @@ static void test_cfam_id(const void *data) static void test_xscom_core(QTestState *qts, const PnvChip *chip) { - if (chip->chip_type == PNV_CHIP_POWER10) { + if ((chip->chip_type == PNV_CHIP_POWER10) || + (chip->chip_type == PNV_CHIP_POWER11)) { uint32_t first_core_thread_state = PNV_XSCOM_P10_EC_BASE(chip->first_core) + 0x412; uint64_t thread_state; @@ -84,14 +79,8 @@ static void test_xscom_core(QTestState *qts, const PnvChip *chip) static void test_core(const void *data) { const PnvChip *chip = data; + const char *machine = pnv_get_machine_type(chip->chip_type); QTestState *qts; - const char *machine = "powernv8"; - - if (chip->chip_type == PNV_CHIP_POWER9) { - machine = "powernv9"; - } else if (chip->chip_type == PNV_CHIP_POWER10) { - machine = "powernv10"; - } qts = qtest_initf("-M %s -accel tcg -cpu %s", machine, chip->cpu_model); diff --git a/tests/qtest/pnv-xscom.h b/tests/qtest/pnv-xscom.h index 5aa1701ea768..8f05f59a1436 100644 --- a/tests/qtest/pnv-xscom.h +++ b/tests/qtest/pnv-xscom.h @@ -9,6 +9,9 @@ #ifndef PNV_XSCOM_H #define PNV_XSCOM_H +#include <stdint.h> +#include <assert.h> + #define SMT 4 /* some tests will break if less than 4 */ typedef enum PnvChipType { @@ -17,6 +20,7 @@ typedef enum PnvChipType { PNV_CHIP_POWER8NVL, /* AKA Naples */ PNV_CHIP_POWER9, /* AKA Nimbus */ PNV_CHIP_POWER10, + PNV_CHIP_POWER11, } PnvChipType; typedef struct PnvChip { @@ -60,15 +64,23 @@ static const PnvChip pnv_chips[] = { .first_core = 0x0, .num_i2c = 4, }, + { + .chip_type = PNV_CHIP_POWER11, + .cpu_model = "Power11", + .xscom_base = 0x000603fc00000000ull, + .cfam_id = 0x220da04980000000ull, + .first_core = 0x0, + .num_i2c = 0, + }, }; static inline uint64_t pnv_xscom_addr(const PnvChip *chip, uint32_t pcba) { uint64_t addr = chip->xscom_base; - if (chip->chip_type == PNV_CHIP_POWER10) { - addr |= ((uint64_t) pcba << 3); - } else if (chip->chip_type == PNV_CHIP_POWER9) { + if ((chip->chip_type == PNV_CHIP_POWER11) || + (chip->chip_type == PNV_CHIP_POWER10) || + (chip->chip_type == PNV_CHIP_POWER9)) { addr |= ((uint64_t) pcba << 3); } else { addr |= (((uint64_t) pcba << 4) & ~0xffull) | @@ -77,4 +89,19 @@ static inline uint64_t pnv_xscom_addr(const PnvChip *chip, uint32_t pcba) return addr; } +static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +} + #endif /* PNV_XSCOM_H */ -- 2.54.0
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following? static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x]; ~Shivang.
On 26/06/23 09:24PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following?
static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x];
Has a subtle difference, that it may not assert/segfault on unknown value for x, I want the test to crash/fail if this is called with an unknown/new processor chip. I believe the if-else/switch is better as it handles (by 'crashing') unknown values too. - Aditya G
~Shivang.
On 24/06/26 7:14 pm, Aditya Gupta wrote:
On 26/06/23 09:24PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following?
static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x];
Has a subtle difference, that it may not assert/segfault on unknown value for x, I want the test to crash/fail if this is called with an unknown/new processor chip. I believe the if-else/switch is better as it handles (by 'crashing') unknown values too.
We could just do: if (chip_type >= PNV_CHIP_MAX) { g_assert_not_reached(); } else { return machine_types[chip_type]; } That way just adding new array entry would work for future chips.
- Aditya G
~Shivang.
On Thu, 25 Jun 2026, Harsh Prateek Bora wrote:
On 24/06/26 7:14 pm, Aditya Gupta wrote:
On 26/06/23 09:24PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following?
static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x];
Has a subtle difference, that it may not assert/segfault on unknown value for x, I want the test to crash/fail if this is called with an unknown/new processor chip. I believe the if-else/switch is better as it handles (by 'crashing') unknown values too.
We could just do:
if (chip_type >= PNV_CHIP_MAX) { g_assert_not_reached(); } else { return machine_types[chip_type]; }
Or even g_assert(chip_type < PNV_CHIP_MAX); without the if? Regards, BALATON Zoltan
That way just adding new array entry would work for future chips.
- Aditya G
~Shivang.
On 25/06/26 11:33 am, BALATON Zoltan wrote:
On Thu, 25 Jun 2026, Harsh Prateek Bora wrote:
On 24/06/26 7:14 pm, Aditya Gupta wrote:
On 26/06/23 09:24PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following?
static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x];
Has a subtle difference, that it may not assert/segfault on unknown value for x, I want the test to crash/fail if this is called with an unknown/new processor chip. I believe the if-else/switch is better as it handles (by 'crashing') unknown values too.
We could just do:
if (chip_type >= PNV_CHIP_MAX) { g_assert_not_reached(); } else { return machine_types[chip_type]; }
Or even g_assert(chip_type < PNV_CHIP_MAX); without the if?
Absolutely :)
Regards, BALATON Zoltan
That way just adding new array entry would work for future chips.
- Aditya G
~Shivang.
On 26/06/25 08:03AM, BALATON Zoltan wrote:
On Thu, 25 Jun 2026, Harsh Prateek Bora wrote:
On 24/06/26 7:14 pm, Aditya Gupta wrote:
On 26/06/23 09:24PM, Shivang Upadhyay wrote:
On Tue, 2026-06-23 at 19:49 +0530, Aditya Gupta wrote:
+static const char *pnv_get_machine_type(enum PnvChipType chip_type) +{ + if (chip_type == PNV_CHIP_POWER8) { + return "powernv8"; + } else if (chip_type == PNV_CHIP_POWER9) { + return "powernv9"; + } else if (chip_type == PNV_CHIP_POWER10) { + return "powernv10"; + } else if (chip_type == PNV_CHIP_POWER11) { + return "powernv11"; + } else { + g_assert_not_reached(); + } +}
How about refactoring to the following?
static const char *const machine_types[] = { [PNV_CHIP_POWER8] = "powernv8", [PNV_CHIP_POWER9] = "powernv9", [PNV_CHIP_POWER10] = "powernv10", [PNV_CHIP_POWER11] = "powernv11", }; return machine_types[x];
Has a subtle difference, that it may not assert/segfault on unknown value for x, I want the test to crash/fail if this is called with an unknown/new processor chip. I believe the if-else/switch is better as it handles (by 'crashing') unknown values too.
We could just do:
if (chip_type >= PNV_CHIP_MAX) { g_assert_not_reached(); } else { return machine_types[chip_type]; }
Or even g_assert(chip_type < PNV_CHIP_MAX); without the if?
Regards, BALATON Zoltan
That way just adding new array entry would work for future chips.
will do in v3. thanks ! - Aditya G
Hi Aditya, One comment below regarding test coverage. On 23/06/26 19:49, Aditya Gupta wrote:
@@ -548,7 +548,7 @@ static void test_xive(void) { QTestState *qts;
- qts = qtest_initf("-M powernv10 -smp %d,cores=1,threads=%d -nographic " + qts = qtest_initf("-M powernv11 -smp %d,cores=1,threads=%d -nographic " "-nodefaults -serial mon:stdio -S " "-d guest_errors -trace '*xive*'", SMT, SMT); By replacing the hardcoded machine type here instead of testing both, QEMU will no longer exercise the XIVE2 path on Power10.
If a future change breaks the Power10 machine initialization while Power11 continues to work, this test would still pass. Would it make sense to run this test for both `powernv10` and `powernv11`, similar to how the SPI qtest iterates over the supported machines? Thanks, Nikhil
On 26/06/30 10:08AM, Nikhil Kumar Singh wrote:
Hi Aditya,
One comment below regarding test coverage.
On 23/06/26 19:49, Aditya Gupta wrote:
@@ -548,7 +548,7 @@ static void test_xive(void) { QTestState *qts; - qts = qtest_initf("-M powernv10 -smp %d,cores=1,threads=%d -nographic " + qts = qtest_initf("-M powernv11 -smp %d,cores=1,threads=%d -nographic " "-nodefaults -serial mon:stdio -S " "-d guest_errors -trace '*xive*'", SMT, SMT); By replacing the hardcoded machine type here instead of testing both, QEMU will no longer exercise the XIVE2 path on Power10.
If a future change breaks the Power10 machine initialization while Power11 continues to work, this test would still pass.
Would it make sense to run this test for both `powernv10` and `powernv11`, similar to how the SPI qtest iterates over the supported machines?
Since power11's interrupt controller is the same as power10, power10's xive breakage should break power11 too. But I get it, safer to have both, will do it in v4. Thanks, - Aditya G
Thanks, Nikhil
The default powernv machine has been recently changed to powernv11, though fadump functional test used powernv10 Change it to use default 'powernv' machine for the tests instead of being fixed to powernv10 Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/functional/ppc64/test_fadump.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/ppc64/test_fadump.py b/tests/functional/ppc64/test_fadump.py index 7ea65974e0ea..c75095076b52 100755 --- a/tests/functional/ppc64/test_fadump.py +++ b/tests/functional/ppc64/test_fadump.py @@ -53,7 +53,7 @@ def do_test_fadump(self, is_kvm=False, is_powernv=False): self.require_accelerator("tcg") if is_powernv: - self.set_machine("powernv10") + self.set_machine("powernv") else: # SLOF takes upto >20s in startup time, use VOF self.set_machine("pseries") -- 2.54.0
Power8E and Power8NVL were deprecated since QEMU 10.1, with commit 264a604e7163 ("target/ppc: Deprecate Power8E and Power8NVL") As Power8E chip is removed in future commits, remove the use of Power8E chip for use with the none machine test, and replace with Power11 for ppc64 test coverage for the test Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- tests/qtest/machine-none-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/machine-none-test.c b/tests/qtest/machine-none-test.c index bafd7d660ec7..3205df190f35 100644 --- a/tests/qtest/machine-none-test.c +++ b/tests/qtest/machine-none-test.c @@ -38,7 +38,7 @@ static struct arch2cpu cpus_map[] = { { "mips64el", "I6500" }, { "or1k", "or1200" }, { "ppc", "604" }, - { "ppc64", "power8e_v2.1" }, + { "ppc64", "power11_v2.0" }, { "s390x", "qemu" }, { "sh4", "sh7750r" }, { "sh4eb", "sh7751r" }, -- 2.54.0
Power8E and Power8NVL were deprecated since QEMU 10.1, with commit 264a604e7163 ("target/ppc: Deprecate Power8E and Power8NVL") Accordingly, remove usage of 8E and 8NVL chips from powernv, as it's old and unmaintained now. Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- docs/system/ppc/powernv.rst | 2 +- hw/ppc/pnv.c | 62 ------------------------------------- hw/ppc/pnv_core.c | 2 -- include/hw/ppc/pnv.h | 8 ----- tests/qtest/pnv-xscom.h | 9 ------ 5 files changed, 1 insertion(+), 82 deletions(-) diff --git a/docs/system/ppc/powernv.rst b/docs/system/ppc/powernv.rst index 5154794cc8cd..1de696b588a8 100644 --- a/docs/system/ppc/powernv.rst +++ b/docs/system/ppc/powernv.rst @@ -15,7 +15,7 @@ beyond the scope of what QEMU addresses today. Supported devices ----------------- - * Multi processor support for POWER8, POWER8NVL, POWER9, Power10 and Power11. + * Multi processor support for POWER8, POWER9, Power10 and Power11. * XSCOM, serial communication sideband bus to configure chiplets. * Simple LPC Controller. * Processor Service Interface (PSI) Controller. diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 62812f22f8cd..b669df9f175f 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -865,16 +865,6 @@ static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) return pnv_lpc_isa_create(&chip8->lpc, true, errp); } -static ISABus *pnv_chip_power8nvl_isa_create(PnvChip *chip, Error **errp) -{ - Pnv8Chip *chip8 = PNV8_CHIP(chip); - qemu_irq irq = qdev_get_gpio_in(DEVICE(&chip8->psi), PSIHB_IRQ_LPC_I2C); - - qdev_connect_gpio_out_named(DEVICE(&chip8->lpc), "LPCHC", 0, irq); - - return pnv_lpc_isa_create(&chip8->lpc, false, errp); -} - static ISABus *pnv_chip_power9_isa_create(PnvChip *chip, Error **errp) { Pnv9Chip *chip9 = PNV9_CHIP(chip); @@ -1642,7 +1632,6 @@ static void *pnv_chip_power11_intc_get(PnvChip *chip) * EX14 * <EX15 reserved> */ -#define POWER8E_CORE_MASK (0x7070ull) #define POWER8_CORE_MASK (0x7e7eull) /* @@ -1823,30 +1812,6 @@ static uint32_t pnv_chip_power8_xscom_pcba(PnvChip *chip, uint64_t addr) return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf); } -static void pnv_chip_power8e_class_init(ObjectClass *klass, const void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - PnvChipClass *k = PNV_CHIP_CLASS(klass); - - k->chip_cfam_id = 0x221ef04980000000ull; /* P8 Murano DD2.1 */ - k->cores_mask = POWER8E_CORE_MASK; - k->num_phbs = 3; - k->get_pir_tir = pnv_get_pir_tir_p8; - k->intc_create = pnv_chip_power8_intc_create; - k->intc_reset = pnv_chip_power8_intc_reset; - k->intc_destroy = pnv_chip_power8_intc_destroy; - k->intc_print_info = pnv_chip_power8_intc_print_info; - k->isa_create = pnv_chip_power8_isa_create; - k->dt_populate = pnv_chip_power8_dt_populate; - k->pic_print_info = pnv_chip_power8_pic_print_info; - k->xscom_core_base = pnv_chip_power8_xscom_core_base; - k->xscom_pcba = pnv_chip_power8_xscom_pcba; - dc->desc = "PowerNV Chip POWER8E"; - - device_class_set_parent_realize(dc, pnv_chip_power8_realize, - &k->parent_realize); -} - static void pnv_chip_power8_class_init(ObjectClass *klass, const void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1871,30 +1836,6 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, const void *data) &k->parent_realize); } -static void pnv_chip_power8nvl_class_init(ObjectClass *klass, const void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - PnvChipClass *k = PNV_CHIP_CLASS(klass); - - k->chip_cfam_id = 0x120d304980000000ull; /* P8 Naples DD1.0 */ - k->cores_mask = POWER8_CORE_MASK; - k->num_phbs = 4; - k->get_pir_tir = pnv_get_pir_tir_p8; - k->intc_create = pnv_chip_power8_intc_create; - k->intc_reset = pnv_chip_power8_intc_reset; - k->intc_destroy = pnv_chip_power8_intc_destroy; - k->intc_print_info = pnv_chip_power8_intc_print_info; - k->isa_create = pnv_chip_power8nvl_isa_create; - k->dt_populate = pnv_chip_power8_dt_populate; - k->pic_print_info = pnv_chip_power8_pic_print_info; - k->xscom_core_base = pnv_chip_power8_xscom_core_base; - k->xscom_pcba = pnv_chip_power8_xscom_pcba; - dc->desc = "PowerNV Chip POWER8NVL"; - - device_class_set_parent_realize(dc, pnv_chip_power8_realize, - &k->parent_realize); -} - static void pnv_chip_power9_instance_init(Object *obj) { PnvChip *chip = PNV_CHIP(obj); @@ -3781,9 +3722,6 @@ static const TypeInfo types[] = { .instance_size = sizeof(Pnv8Chip), }, DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init), - DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init), - DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL, - pnv_chip_power8nvl_class_init), }; DEFINE_TYPES(types) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index 8939515c2c34..61dc8211e32f 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -515,9 +515,7 @@ static const TypeInfo pnv_core_infos[] = { .class_init = pnv_core_class_init, .abstract = true, }, - DEFINE_PNV_CORE_TYPE(power8, "power8e_v2.1"), DEFINE_PNV_CORE_TYPE(power8, "power8_v2.0"), - DEFINE_PNV_CORE_TYPE(power8, "power8nvl_v1.0"), DEFINE_PNV_CORE_TYPE(power9, "power9_v2.2"), DEFINE_PNV_CORE_TYPE(power10, "power10_v2.0"), DEFINE_PNV_CORE_TYPE(power11, "power11_v2.0"), diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index f8234fb3cd31..60e902d9c5bb 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -39,18 +39,10 @@ typedef struct Pnv10Chip Pnv11Chip; #define PNV_CHIP_TYPE_SUFFIX "-" TYPE_PNV_CHIP #define PNV_CHIP_TYPE_NAME(cpu_model) cpu_model PNV_CHIP_TYPE_SUFFIX -#define TYPE_PNV_CHIP_POWER8E PNV_CHIP_TYPE_NAME("power8e_v2.1") -DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER8E, - TYPE_PNV_CHIP_POWER8E) - #define TYPE_PNV_CHIP_POWER8 PNV_CHIP_TYPE_NAME("power8_v2.0") DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER8, TYPE_PNV_CHIP_POWER8) -#define TYPE_PNV_CHIP_POWER8NVL PNV_CHIP_TYPE_NAME("power8nvl_v1.0") -DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER8NVL, - TYPE_PNV_CHIP_POWER8NVL) - #define TYPE_PNV_CHIP_POWER9 PNV_CHIP_TYPE_NAME("power9_v2.2") DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER9, TYPE_PNV_CHIP_POWER9) diff --git a/tests/qtest/pnv-xscom.h b/tests/qtest/pnv-xscom.h index 8f05f59a1436..f184118ae5a6 100644 --- a/tests/qtest/pnv-xscom.h +++ b/tests/qtest/pnv-xscom.h @@ -15,9 +15,7 @@ #define SMT 4 /* some tests will break if less than 4 */ typedef enum PnvChipType { - PNV_CHIP_POWER8E, /* AKA Murano (default) */ PNV_CHIP_POWER8, /* AKA Venice */ - PNV_CHIP_POWER8NVL, /* AKA Naples */ PNV_CHIP_POWER9, /* AKA Nimbus */ PNV_CHIP_POWER10, PNV_CHIP_POWER11, @@ -40,13 +38,6 @@ static const PnvChip pnv_chips[] = { .cfam_id = 0x220ea04980000000ull, .first_core = 0x1, .num_i2c = 0, - }, { - .chip_type = PNV_CHIP_POWER8NVL, - .cpu_model = "POWER8NVL", - .xscom_base = 0x0003fc0000000000ull, - .cfam_id = 0x120d304980000000ull, - .first_core = 0x1, - .num_i2c = 0, }, { .chip_type = PNV_CHIP_POWER9, -- 2.54.0
Power8E and Power8NVL were deprecated since QEMU 10.1, with commit 264a604e7163 ("target/ppc: Deprecate Power8E and Power8NVL") Remove the corresponding 8E and 8NVL CPU cores from spapr/pseries Also, with no use of 8E and 8NVL, in powernv chips or spapr cores, remove the CPU definitions for the cores Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- docs/about/deprecated.rst | 9 --------- docs/about/removed-features.rst | 8 ++++++++ docs/system/ppc/pseries.rst | 2 +- hw/ppc/spapr_cpu_core.c | 2 -- target/ppc/cpu-models.c | 6 ------ target/ppc/cpu-models.h | 4 ---- target/ppc/cpu_init.c | 6 ------ target/ppc/kvm.c | 4 +--- 8 files changed, 10 insertions(+), 31 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 97750f5edc91..3930ef08ed4b 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -215,15 +215,6 @@ embedded 405 for power management (OCC) and other internal tasks, it is theoretically possible to use QEMU to model them. Let's keep the CPU implementation for a while before removing all support. -Power8E and Power8NVL CPUs and corresponding Pnv chips (since 10.1) -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' - -The Power8E and Power8NVL variants of Power8 are not really useful anymore -in qemu, and are old and unmaintained now. - -The CPUs as well as corresponding Power8NVL and Power8E PnvChips will also -be considered deprecated. - System emulator machines ------------------------ diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index d1bea4d75bb4..f7a2aa326dec 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -1038,6 +1038,14 @@ initial RISC-V QEMU port. Its usage was always been unclear: users don't know what to expect from a CPU called 'any', and in fact the CPU does not do anything special that isn't already done by the default CPUs rv32/rv64. +Power8E and Power8NVL CPUs (removed in 11.1) +'''''''''''''''''''''''''''''''''''''''''''' + +The Power8E and Power8NVL variants of Power8 are not really useful anymore +in qemu, and are old and unmaintained. +Hence, the CPUs as well as corresponding Power8NVL and Power8E PnvChips have +been removed + System accelerators ------------------- diff --git a/docs/system/ppc/pseries.rst b/docs/system/ppc/pseries.rst index bbc51aa7fcdb..1477028bc5ea 100644 --- a/docs/system/ppc/pseries.rst +++ b/docs/system/ppc/pseries.rst @@ -16,7 +16,7 @@ Supported devices * Multi processor support for many Power processors generations: - POWER7, POWER7+ - - POWER8, POWER8NVL + - POWER8 - POWER9 - Power10 - Power11 diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 41e37103c043..9a9351991c95 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -408,8 +408,6 @@ static const TypeInfo spapr_cpu_core_type_infos[] = { DEFINE_SPAPR_CPU_CORE_TYPE("power7_v2.3"), DEFINE_SPAPR_CPU_CORE_TYPE("power7p_v2.1"), DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"), - DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"), - DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"), DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"), DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"), DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"), diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 26b6debcfc96..aa13654e83e8 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -734,12 +734,8 @@ "POWER7 v2.3") POWERPC_DEF("power7p_v2.1", CPU_POWERPC_POWER7P_v21, POWER7, "POWER7+ v2.1") - POWERPC_DEPRECATED_CPU("power8e_v2.1", CPU_POWERPC_POWER8E_v21, POWER8, - "POWER8E v2.1", "CPU is unmaintained.") POWERPC_DEF("power8_v2.0", CPU_POWERPC_POWER8_v20, POWER8, "POWER8 v2.0") - POWERPC_DEPRECATED_CPU("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8, - "POWER8NVL v1.0", "CPU is unmaintained.") POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20, POWER9, "POWER9 v2.0") POWERPC_DEF("power9_v2.2", CPU_POWERPC_POWER9_DD22, POWER9, @@ -918,9 +914,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "power7", "power7_v2.3" }, { "power7+", "power7p_v2.1" }, { "power7+_v2.1", "power7p_v2.1" }, - { "power8e", "power8e_v2.1" }, { "power8", "power8_v2.0" }, - { "power8nvl", "power8nvl_v1.0" }, { "power9", "power9_v2.2" }, { "power10", "power10_v2.0" }, { "power11", "power11_v2.0" }, diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h index a439eb37ee41..ef019cdb1f3c 100644 --- a/target/ppc/cpu-models.h +++ b/target/ppc/cpu-models.h @@ -341,12 +341,8 @@ enum { CPU_POWERPC_POWER7_v23 = 0x003F0203, CPU_POWERPC_POWER7P_BASE = 0x004A0000, CPU_POWERPC_POWER7P_v21 = 0x004A0201, - CPU_POWERPC_POWER8E_BASE = 0x004B0000, - CPU_POWERPC_POWER8E_v21 = 0x004B0201, CPU_POWERPC_POWER8_BASE = 0x004D0000, CPU_POWERPC_POWER8_v20 = 0x004D0200, - CPU_POWERPC_POWER8NVL_BASE = 0x004C0000, - CPU_POWERPC_POWER8NVL_v10 = 0x004C0100, CPU_POWERPC_POWER9_BASE = 0x004E0000, CPU_POWERPC_POWER9_DD1 = 0x004E1100, CPU_POWERPC_POWER9_DD20 = 0x004E1200, diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index a02187ce5afb..96daa7e2ab0b 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -6311,12 +6311,6 @@ static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr, bool best) if (base == CPU_POWERPC_POWER8_BASE) { return true; } - if (base == CPU_POWERPC_POWER8E_BASE) { - return true; - } - if (base == CPU_POWERPC_POWER8NVL_BASE) { - return true; - } } if (base != pcc_base) { return false; diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index b94c2997a07f..78a1c4a8393e 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -2437,9 +2437,7 @@ static bool kvmppc_power8_host(void) #ifdef TARGET_PPC64 { uint32_t base_pvr = CPU_POWERPC_POWER_SERVER_MASK & mfpvr(); - ret = (base_pvr == CPU_POWERPC_POWER8E_BASE) || - (base_pvr == CPU_POWERPC_POWER8NVL_BASE) || - (base_pvr == CPU_POWERPC_POWER8_BASE); + ret = (base_pvr == CPU_POWERPC_POWER8_BASE); } #endif /* TARGET_PPC64 */ return ret; -- 2.54.0
Add myself as maintainer for PowerNV machine, and remove the reviewer entry Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 93df53d87f63..67f65bff3913 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1646,7 +1646,8 @@ F: tests/functional/ppc64/test_tuxrun.py PowerNV (Non-Virtualized) M: Nicholas Piggin <npiggin@gmail.com> -R: Aditya Gupta <adityag@linux.ibm.com> +M: Aditya Gupta <adityag@linux.ibm.com> +G: @adi-g15-ibm R: Glenn Miles <milesg@linux.ibm.com> R: Harsh Prateek Bora <harshpb@linux.ibm.com> L: qemu-ppc@nongnu.org -- 2.54.0
On Tue, Jun 23, 2026 at 07:49:33PM +0530, Aditya Gupta wrote:
Add myself as maintainer for PowerNV machine, and remove the reviewer entry
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 93df53d87f63..67f65bff3913 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1646,7 +1646,8 @@ F: tests/functional/ppc64/test_tuxrun.py
PowerNV (Non-Virtualized) M: Nicholas Piggin <npiggin@gmail.com> -R: Aditya Gupta <adityag@linux.ibm.com> +M: Aditya Gupta <adityag@linux.ibm.com> +G: @adi-g15-ibm
Please drop the G: line, as I'm changing the approach for recording this info to be more automatic.
R: Glenn Miles <milesg@linux.ibm.com> R: Harsh Prateek Bora <harshpb@linux.ibm.com> L: qemu-ppc@nongnu.org -- 2.54.0
With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
On 26/06/24 11:47AM, Daniel P. Berrangé wrote:
On Tue, Jun 23, 2026 at 07:49:33PM +0530, Aditya Gupta wrote:
Add myself as maintainer for PowerNV machine, and remove the reviewer entry
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 93df53d87f63..67f65bff3913 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1646,7 +1646,8 @@ F: tests/functional/ppc64/test_tuxrun.py
PowerNV (Non-Virtualized) M: Nicholas Piggin <npiggin@gmail.com> -R: Aditya Gupta <adityag@linux.ibm.com> +M: Aditya Gupta <adityag@linux.ibm.com> +G: @adi-g15-ibm
Please drop the G: line, as I'm changing the approach for recording this info to be more automatic.
Sure Daniel, will remove it in v2. Thanks, - Aditya G
Hello Nick, Are you still active on the qemu-ppc subsystem ? Thanks, C. On 6/23/26 16:19, Aditya Gupta wrote:
Add myself as maintainer for PowerNV machine, and remove the reviewer entry
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com> --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 93df53d87f63..67f65bff3913 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1646,7 +1646,8 @@ F: tests/functional/ppc64/test_tuxrun.py
PowerNV (Non-Virtualized) M: Nicholas Piggin <npiggin@gmail.com> -R: Aditya Gupta <adityag@linux.ibm.com> +M: Aditya Gupta <adityag@linux.ibm.com> +G: @adi-g15-ibm R: Glenn Miles <milesg@linux.ibm.com> R: Harsh Prateek Bora <harshpb@linux.ibm.com> L: qemu-ppc@nongnu.org
participants (7)
-
Aditya Gupta -
BALATON Zoltan -
Cédric Le Goater -
Daniel P. Berrangé -
Harsh Prateek Bora -
Nikhil Kumar Singh -
Shivang Upadhyay