From 897c0da96f936217e3e2a04c77486ca93c2f1dea Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 3 Nov 2022 10:50:17 +0900 Subject: [PATCH 01/12] tests/qtest/libqos/e1000e: Refer common PCI ID definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is yet another minor cleanup to ease understanding and future refactoring of the tests. Signed-off-by: Akihiko Odaki Message-Id: <20221103015017.19947-1-akihiko.odaki@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index ed47e34044..5f80035859 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -18,6 +18,7 @@ #include "qemu/osdep.h" #include "hw/net/e1000_regs.h" +#include "hw/pci/pci_ids.h" #include "../libqtest.h" #include "pci-pc.h" #include "qemu/sockets.h" @@ -217,8 +218,8 @@ static void *e1000e_pci_create(void *pci_bus, QGuestAllocator *alloc, static void e1000e_register_nodes(void) { QPCIAddress addr = { - .vendor_id = 0x8086, - .device_id = 0x10D3, + .vendor_id = PCI_VENDOR_ID_INTEL, + .device_id = E1000_DEV_ID_82574L, }; /* FIXME: every test using this node needs to setup a -netdev socket,id=hs0 From ff4f45811fb2ca8f17ef78361128b03dbb679185 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 3 Nov 2022 11:54:51 +0900 Subject: [PATCH 02/12] tests/qtest/libqos/e1000e: Set E1000_CTRL_SLU The later device status check depends on E1000_STATUS_LU, which is enabled by E1000_CTRL_SLU. Though E1000_STATUS_LU is not implemented and E1000_STATUS_LU is always available in the current implementation, be a bit nicer and set E1000_CTRL_SLU just in case the bit is implemented in the future. Signed-off-by: Akihiko Odaki Message-Id: <20221103025451.27446-1-akihiko.odaki@daynix.com> Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 5f80035859..4fd0bd5311 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -122,7 +122,7 @@ static void e1000e_pci_start_hw(QOSGraphObject *obj) /* Reset the device */ val = e1000e_macreg_read(&d->e1000e, E1000_CTRL); - e1000e_macreg_write(&d->e1000e, E1000_CTRL, val | E1000_CTRL_RST); + e1000e_macreg_write(&d->e1000e, E1000_CTRL, val | E1000_CTRL_RST | E1000_CTRL_SLU); /* Enable and configure MSI-X */ qpci_msix_enable(&d->pci_dev); From dfa644b231a55e50571b99bc65a2fff530e6913c Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 3 Nov 2022 18:54:16 +0900 Subject: [PATCH 03/12] tests/qtest/e1000e-test: Use e1000_regs.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The register definitions in tests/qtest/e1000e-test.c had names different from hw/net/e1000_regs.h, which made it hard to understand what test codes corresponds to the implementation. Use hw/net/e1000_regs.h from tests/qtest/libqos/e1000e.c to remove these duplications. Signed-off-by: Akihiko Odaki Message-Id: <20221103095416.110162-1-akihiko.odaki@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/e1000e-test.c | 66 ++++++--------------------------------- 1 file changed, 10 insertions(+), 56 deletions(-) diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c index 4cdd8238f2..08adc5226d 100644 --- a/tests/qtest/e1000e-test.c +++ b/tests/qtest/e1000e-test.c @@ -33,34 +33,11 @@ #include "qemu/bitops.h" #include "libqos/libqos-malloc.h" #include "libqos/e1000e.h" +#include "hw/net/e1000_regs.h" static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *alloc) { - struct { - uint64_t buffer_addr; - union { - uint32_t data; - struct { - uint16_t length; - uint8_t cso; - uint8_t cmd; - } flags; - } lower; - union { - uint32_t data; - struct { - uint8_t status; - uint8_t css; - uint16_t special; - } fields; - } upper; - } descr; - - static const uint32_t dtyp_data = BIT(20); - static const uint32_t dtyp_ext = BIT(29); - static const uint32_t dcmd_rs = BIT(27); - static const uint32_t dcmd_eop = BIT(24); - static const uint32_t dsta_dd = BIT(0); + struct e1000_tx_desc descr; static const int data_len = 64; char buffer[64]; int ret; @@ -73,10 +50,10 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a /* Prepare TX descriptor */ memset(&descr, 0, sizeof(descr)); descr.buffer_addr = cpu_to_le64(data); - descr.lower.data = cpu_to_le32(dcmd_rs | - dcmd_eop | - dtyp_ext | - dtyp_data | + descr.lower.data = cpu_to_le32(E1000_TXD_CMD_RS | + E1000_TXD_CMD_EOP | + E1000_TXD_CMD_DEXT | + E1000_TXD_DTYP_D | data_len); /* Put descriptor to the ring */ @@ -86,7 +63,8 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a e1000e_wait_isr(d, E1000E_TX0_MSG_ID); /* Check DD bit */ - g_assert_cmphex(le32_to_cpu(descr.upper.data) & dsta_dd, ==, dsta_dd); + g_assert_cmphex(le32_to_cpu(descr.upper.data) & E1000_TXD_STAT_DD, ==, + E1000_TXD_STAT_DD); /* Check data sent to the backend */ ret = recv(test_sockets[0], &recv_len, sizeof(recv_len), 0); @@ -101,31 +79,7 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a static void e1000e_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator *alloc) { - union { - struct { - uint64_t buffer_addr; - uint64_t reserved; - } read; - struct { - struct { - uint32_t mrq; - union { - uint32_t rss; - struct { - uint16_t ip_id; - uint16_t csum; - } csum_ip; - } hi_dword; - } lower; - struct { - uint32_t status_error; - uint16_t length; - uint16_t vlan; - } upper; - } wb; - } descr; - - static const uint32_t esta_dd = BIT(0); + union e1000_rx_desc_extended descr; char test[] = "TEST"; int len = htonl(sizeof(test)); @@ -162,7 +116,7 @@ static void e1000e_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator /* Check DD bit */ g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) & - esta_dd, ==, esta_dd); + E1000_RXD_STAT_DD, ==, E1000_RXD_STAT_DD); /* Check data sent to the backend */ memread(data, buffer, sizeof(buffer)); From 5ebafa16433e424127738d694c653101cc3133dd Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 3 Nov 2022 17:34:25 +0900 Subject: [PATCH 04/12] tests/qtest/libqos/e1000e: Use E1000_STATUS_ASDV_1000 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nemonics E1000_STATUS_LAN_INIT_DONE and E1000_STATUS_ASDV_1000 have the same value, and E1000_STATUS_ASDV_1000 should be used here because E1000_STATUS_ASDV_1000 represents the auto-detected speed tested here while E1000_STATUS_LAN_INIT_DONE is a value used for a different purpose with a variant of e1000e family different from the one implemented in QEMU. Signed-off-by: Akihiko Odaki Message-Id: <20221103083425.100590-1-akihiko.odaki@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 4fd0bd5311..05af6f2118 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -130,8 +130,8 @@ static void e1000e_pci_start_hw(QOSGraphObject *obj) /* Check the device status - link and speed */ val = e1000e_macreg_read(&d->e1000e, E1000_STATUS); - g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE), - ==, E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE); + g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_ASDV_1000), + ==, E1000_STATUS_LU | E1000_STATUS_ASDV_1000); /* Initialize TX/RX logic */ e1000e_macreg_write(&d->e1000e, E1000_RCTL, 0); From 624ee20cb9742bb536a778b9585c916b243e78f2 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sat, 5 Nov 2022 14:30:10 +0900 Subject: [PATCH 05/12] tests/qtest/libqos/e1000e: Use IVAR shift definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were still some constants defined in e1000_regs.h. Signed-off-by: Akihiko Odaki Message-Id: <20221105053010.38037-1-akihiko.odaki@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 05af6f2118..80b3e3db90 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -30,9 +30,9 @@ #include "e1000e.h" #define E1000E_IVAR_TEST_CFG \ - (E1000E_RX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID | \ - ((E1000E_TX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << 8) | \ - ((E1000E_OTHER_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << 16) | \ + (((E1000E_RX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_RXQ0_SHIFT) | \ + ((E1000E_TX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_TXQ0_SHIFT) | \ + ((E1000E_OTHER_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_OTHER_SHIFT) | \ E1000_IVAR_TX_INT_EVERY_WB) #define E1000E_RING_LEN (0x1000) From d46e6bba55f858b829251e2f4bd7b150cdb5b1d6 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 5 Nov 2022 12:55:25 +0100 Subject: [PATCH 06/12] tests/qtest: Fix two format strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Weil Message-Id: <20221105115525.623059-1-sw@weilnetz.de> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/migration-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index d2eb107f0c..f574331b7b 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -2188,7 +2188,7 @@ static void calc_dirty_rate(QTestState *who, uint64_t calc_time) qobject_unref(qmp_command(who, "{ 'execute': 'calc-dirty-rate'," "'arguments': { " - "'calc-time': %ld," + "'calc-time': %" PRIu64 "," "'mode': 'dirty-ring' }}", calc_time)); } @@ -2203,7 +2203,7 @@ static void dirtylimit_set_all(QTestState *who, uint64_t dirtyrate) qobject_unref(qmp_command(who, "{ 'execute': 'set-vcpu-dirty-limit'," "'arguments': { " - "'dirty-rate': %ld } }", + "'dirty-rate': %" PRIu64 " } }", dirtyrate)); } From d1695f1839769b62e25086187afc58185f49ba2f Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 1 Nov 2022 11:50:21 +0800 Subject: [PATCH 07/12] tests/qtest: migration-test: Enable TLS PSK tests for win32 Since commit f1018ea0a30f ("tests: avoid DOS line endings in PSK file"), the bug of the helper test_tls_psk_init_common() that caused TLS PSK tests to fail on Windows was fixed. Let's enable these tests on win32. Signed-off-by: Bin Meng Message-Id: <20221101035021.729669-1-bin.meng@windriver.com> Signed-off-by: Thomas Huth --- tests/qtest/migration-test.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index f574331b7b..442998d9eb 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1402,7 +1402,6 @@ static void test_precopy_unix_dirty_ring(void) } #ifdef CONFIG_GNUTLS -#ifndef _WIN32 static void test_precopy_unix_tls_psk(void) { g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -1415,7 +1414,6 @@ static void test_precopy_unix_tls_psk(void) test_precopy_common(&args); } -#endif /* _WIN32 */ #ifdef CONFIG_TASN1 static void test_precopy_unix_tls_x509_default_host(void) @@ -1524,7 +1522,6 @@ static void test_precopy_tcp_plain(void) } #ifdef CONFIG_GNUTLS -#ifndef _WIN32 static void test_precopy_tcp_tls_psk_match(void) { MigrateCommon args = { @@ -1535,7 +1532,6 @@ static void test_precopy_tcp_tls_psk_match(void) test_precopy_common(&args); } -#endif /* _WIN32 */ static void test_precopy_tcp_tls_psk_mismatch(void) { @@ -1933,7 +1929,6 @@ static void test_multifd_tcp_zstd(void) #endif #ifdef CONFIG_GNUTLS -#ifndef _WIN32 static void * test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from, QTestState *to) @@ -1941,7 +1936,6 @@ test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from, test_migrate_precopy_tcp_multifd_start_common(from, to, "none"); return test_migrate_tls_psk_start_match(from, to); } -#endif /* _WIN32 */ static void * test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from, @@ -1993,7 +1987,6 @@ test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from, } #endif /* CONFIG_TASN1 */ -#ifndef _WIN32 static void test_multifd_tcp_tls_psk_match(void) { MigrateCommon args = { @@ -2003,7 +1996,6 @@ static void test_multifd_tcp_tls_psk_match(void) }; test_precopy_common(&args); } -#endif /* _WIN32 */ static void test_multifd_tcp_tls_psk_mismatch(void) { @@ -2505,10 +2497,8 @@ int main(int argc, char **argv) qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain); qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle); #ifdef CONFIG_GNUTLS -#ifndef _WIN32 qtest_add_func("/migration/precopy/unix/tls/psk", test_precopy_unix_tls_psk); -#endif if (has_uffd) { /* @@ -2534,10 +2524,8 @@ int main(int argc, char **argv) qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain); #ifdef CONFIG_GNUTLS -#ifndef _WIN32 qtest_add_func("/migration/precopy/tcp/tls/psk/match", test_precopy_tcp_tls_psk_match); -#endif qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch", test_precopy_tcp_tls_psk_mismatch); #ifdef CONFIG_TASN1 @@ -2581,10 +2569,8 @@ int main(int argc, char **argv) test_multifd_tcp_zstd); #endif #ifdef CONFIG_GNUTLS -#ifndef _WIN32 qtest_add_func("/migration/multifd/tcp/tls/psk/match", test_multifd_tcp_tls_psk_match); -#endif qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch", test_multifd_tcp_tls_psk_mismatch); #ifdef CONFIG_TASN1 From 765de32d87db65abf443d6a5d7097d42fc6e48de Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 4 Nov 2022 07:36:59 -0400 Subject: [PATCH 08/12] gitlab-ci: increase clang-user timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clang-user test exceeds the 1 hour timeout occassionally. Philippe Mathieu-Daudé has pointed out that the number of tcg tests has increased since QEMU 7.1. The execution time therefore probably reflects a legitimate increase in tests rather than a performance regression. Bump the timeout to prevent CI failures. Suggested-by: Thomas Huth Signed-off-by: Stefan Hajnoczi Reviewed-by: Thomas Huth Message-Id: <20221104113659.427690-1-stefanha@redhat.com> Signed-off-by: Thomas Huth --- .gitlab-ci.d/buildtest.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 6c05c46397..7173749c52 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -327,6 +327,7 @@ clang-user: extends: .native_build_job_template needs: job: amd64-debian-user-cross-container + timeout: 70m variables: IMAGE: debian-all-test-cross CONFIGURE_ARGS: --cc=clang --cxx=clang++ --disable-system From f53b033e4cd2e7706df3cca04f3bf3c5ffc6b08c Mon Sep 17 00:00:00 2001 From: Peter Jin Date: Thu, 27 Oct 2022 23:23:41 +0200 Subject: [PATCH 09/12] s390x/css: revert SCSW ctrl/flag bits on error Revert the control and flag bits in the subchannel status word in case the SSCH operation fails with non-zero CC (ditto for CSCH and HSCH). According to POPS, the control and flag bits are only changed if SSCH, CSCH, and HSCH return CC 0, and no other action should be taken otherwise. In order to simulate that after the fact, the bits need to be reverted on non-zero CC. While the do_subchannel_work logic for virtual (virtio) devices will return condition code 0, passthrough (vfio) devices may encounter errors from either the host kernel or real hardware that need to be accounted for after this point. This includes restoring the state of the Subchannel Status Word to reflect the subchannel, as these bits would not be set in the event of a non-zero condition code from the affected instructions. Experimentation has shown that a failure on a START SUBCHANNEL (SSCH) to a passthrough device would leave the subchannel with the START PENDING activity control bit set, thus blocking subsequent SSCH operations in css_do_ssch() until some form of error recovery was undertaken since no interrupt would be expected. Signed-off-by: Peter Jin Message-Id: <20221027212341.2904795-1-pjin@linux.ibm.com> Reviewed-by: Eric Farman Reviewed-by: Matthew Rosato [thuth: Updated the commit description to Eric's suggestion] Signed-off-by: Thomas Huth --- hw/s390x/css.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 7d9523f811..95d1b3a3ce 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -1522,21 +1522,37 @@ IOInstEnding css_do_xsch(SubchDev *sch) IOInstEnding css_do_csch(SubchDev *sch) { SCHIB *schib = &sch->curr_status; + uint16_t old_scsw_ctrl; + IOInstEnding ccode; if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) { return IOINST_CC_NOT_OPERATIONAL; } + /* + * Save the current scsw.ctrl in case CSCH fails and we need + * to revert the scsw to the status quo ante. + */ + old_scsw_ctrl = schib->scsw.ctrl; + /* Trigger the clear function. */ schib->scsw.ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL); schib->scsw.ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_ACTL_CLEAR_PEND; - return do_subchannel_work(sch); + ccode = do_subchannel_work(sch); + + if (ccode != IOINST_CC_EXPECTED) { + schib->scsw.ctrl = old_scsw_ctrl; + } + + return ccode; } IOInstEnding css_do_hsch(SubchDev *sch) { SCHIB *schib = &sch->curr_status; + uint16_t old_scsw_ctrl; + IOInstEnding ccode; if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) { return IOINST_CC_NOT_OPERATIONAL; @@ -1553,6 +1569,12 @@ IOInstEnding css_do_hsch(SubchDev *sch) return IOINST_CC_BUSY; } + /* + * Save the current scsw.ctrl in case HSCH fails and we need + * to revert the scsw to the status quo ante. + */ + old_scsw_ctrl = schib->scsw.ctrl; + /* Trigger the halt function. */ schib->scsw.ctrl |= SCSW_FCTL_HALT_FUNC; schib->scsw.ctrl &= ~SCSW_FCTL_START_FUNC; @@ -1564,7 +1586,13 @@ IOInstEnding css_do_hsch(SubchDev *sch) } schib->scsw.ctrl |= SCSW_ACTL_HALT_PEND; - return do_subchannel_work(sch); + ccode = do_subchannel_work(sch); + + if (ccode != IOINST_CC_EXPECTED) { + schib->scsw.ctrl = old_scsw_ctrl; + } + + return ccode; } static void css_update_chnmon(SubchDev *sch) @@ -1605,6 +1633,8 @@ static void css_update_chnmon(SubchDev *sch) IOInstEnding css_do_ssch(SubchDev *sch, ORB *orb) { SCHIB *schib = &sch->curr_status; + uint16_t old_scsw_ctrl, old_scsw_flags; + IOInstEnding ccode; if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) { return IOINST_CC_NOT_OPERATIONAL; @@ -1626,11 +1656,26 @@ IOInstEnding css_do_ssch(SubchDev *sch, ORB *orb) } sch->orb = *orb; sch->channel_prog = orb->cpa; + + /* + * Save the current scsw.ctrl and scsw.flags in case SSCH fails and we need + * to revert the scsw to the status quo ante. + */ + old_scsw_ctrl = schib->scsw.ctrl; + old_scsw_flags = schib->scsw.flags; + /* Trigger the start function. */ schib->scsw.ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND); schib->scsw.flags &= ~SCSW_FLAGS_MASK_PNO; - return do_subchannel_work(sch); + ccode = do_subchannel_work(sch); + + if (ccode != IOINST_CC_EXPECTED) { + schib->scsw.ctrl = old_scsw_ctrl; + schib->scsw.flags = old_scsw_flags; + } + + return ccode; } static void copy_irb_to_guest(IRB *dest, const IRB *src, const PMCW *pmcw, From 4a8d21ba50fc8625c3bd51dab903872952f95718 Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Fri, 28 Oct 2022 15:47:56 -0400 Subject: [PATCH 10/12] s390x/pci: RPCIT second pass when mappings exhausted If we encounter a new mapping while the number of available DMA entries in vfio is 0, we are currently skipping that mapping which is a problem if we manage to free up DMA space after that within the same RPCIT -- we will return to the guest with CC0 and have not mapped everything within the specified range. This issue was uncovered while testing changes to the s390 linux kernel iommu/dma code, where a different usage pattern was employed (new mappings start at the end of the aperture and work back towards the front, making us far more likely to encounter new mappings before invalidated mappings during a global refresh). Fix this by tracking whether any mappings were skipped due to vfio DMA limit hitting 0; when this occurs, we still continue the range and unmap/map anything we can - then we must re-run the range again to pickup anything that was missed. This must occur in a loop until all requests are satisfied (success) or we detect that we are still unable to complete all mappings (return ZPCI_RPCIT_ST_INSUFF_RES). Link: https://lore.kernel.org/linux-s390/20221019144435.369902-1-schnelle@linux.ibm.com/ Fixes: 37fa32de70 ("s390x/pci: Honor DMA limits set by vfio") Reported-by: Niklas Schnelle Signed-off-by: Matthew Rosato Message-Id: <20221028194758.204007-2-mjrosato@linux.ibm.com> Reviewed-by: Eric Farman Signed-off-by: Thomas Huth --- hw/s390x/s390-pci-inst.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 20a9bcc7af..7cc4bcf850 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -677,8 +677,9 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra) S390PCIBusDevice *pbdev; S390PCIIOMMU *iommu; S390IOTLBEntry entry; - hwaddr start, end; + hwaddr start, end, sstart; uint32_t dma_avail; + bool again; if (env->psw.mask & PSW_MASK_PSTATE) { s390_program_interrupt(env, PGM_PRIVILEGED, ra); @@ -691,7 +692,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra) } fh = env->regs[r1] >> 32; - start = env->regs[r2]; + sstart = start = env->regs[r2]; end = start + env->regs[r2 + 1]; pbdev = s390_pci_find_dev_by_fh(s390_get_phb(), fh); @@ -732,6 +733,9 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra) goto err; } + retry: + start = sstart; + again = false; while (start < end) { error = s390_guest_io_table_walk(iommu->g_iota, start, &entry); if (error) { @@ -739,13 +743,24 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra) } start += entry.len; - while (entry.iova < start && entry.iova < end && - (dma_avail > 0 || entry.perm == IOMMU_NONE)) { - dma_avail = s390_pci_update_iotlb(iommu, &entry); - entry.iova += TARGET_PAGE_SIZE; - entry.translated_addr += TARGET_PAGE_SIZE; + while (entry.iova < start && entry.iova < end) { + if (dma_avail > 0 || entry.perm == IOMMU_NONE) { + dma_avail = s390_pci_update_iotlb(iommu, &entry); + entry.iova += TARGET_PAGE_SIZE; + entry.translated_addr += TARGET_PAGE_SIZE; + } else { + /* + * We are unable to make a new mapping at this time, continue + * on and hopefully free up more space. Then attempt another + * pass. + */ + again = true; + break; + } } } + if (again && dma_avail > 0) + goto retry; err: if (error) { pbdev->state = ZPCI_FS_ERROR; From 1fd396e32288bbf536483c74b68cb3ee86005a9f Mon Sep 17 00:00:00 2001 From: Pierre Morel Date: Thu, 3 Nov 2022 18:01:40 +0100 Subject: [PATCH 11/12] s390x: Register TYPE_S390_CCW_MACHINE properties as class properties Currently, when running 'qemu-system-s390x -M s390-ccw-virtio,help' the s390x-specific properties are not listed anymore. This happens because since commit d8fb7d0969 ("vl: switch -M parsing to keyval") the properties have to be defined at the class level and not at the instance level anymore. Fix it on s390x now, too, by moving the registration of the properties to the class level" Fixes: d8fb7d0969 ("vl: switch -M parsing to keyval") Signed-off-by: Pierre Morel Message-Id: <20221103170150.20789-2-pmorel@linux.ibm.com> [thuth: Add patch description] Signed-off-by: Thomas Huth --- hw/s390x/s390-virtio-ccw.c | 131 +++++++++++++++++++++---------------- 1 file changed, 74 insertions(+), 57 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 806de32034..196773c833 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -43,6 +43,7 @@ #include "sysemu/sysemu.h" #include "hw/s390x/pv.h" #include "migration/blocker.h" +#include "qapi/visitor.h" static Error *pv_mig_blocker; @@ -589,38 +590,6 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) return newsz; } -static void ccw_machine_class_init(ObjectClass *oc, void *data) -{ - MachineClass *mc = MACHINE_CLASS(oc); - NMIClass *nc = NMI_CLASS(oc); - HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); - S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc); - - s390mc->ri_allowed = true; - s390mc->cpu_model_allowed = true; - s390mc->css_migration_enabled = true; - s390mc->hpage_1m_allowed = true; - mc->init = ccw_init; - mc->reset = s390_machine_reset; - mc->block_default_type = IF_VIRTIO; - mc->no_cdrom = 1; - mc->no_floppy = 1; - mc->no_parallel = 1; - mc->no_sdcard = 1; - mc->max_cpus = S390_MAX_CPUS; - mc->has_hotpluggable_cpus = true; - assert(!mc->get_hotplug_handler); - mc->get_hotplug_handler = s390_get_hotplug_handler; - mc->cpu_index_to_instance_props = s390_cpu_index_to_props; - mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids; - /* it is overridden with 'host' cpu *in kvm_arch_init* */ - mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu"); - hc->plug = s390_machine_device_plug; - hc->unplug_request = s390_machine_device_unplug_request; - nc->nmi_monitor_handler = s390_nmi; - mc->default_ram_id = "s390.ram"; -} - static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); @@ -710,19 +679,29 @@ bool hpage_1m_allowed(void) return get_machine_class()->hpage_1m_allowed; } -static char *machine_get_loadparm(Object *obj, Error **errp) +static void machine_get_loadparm(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + char *str = g_strndup((char *) ms->loadparm, sizeof(ms->loadparm)); - /* make a NUL-terminated string */ - return g_strndup((char *) ms->loadparm, sizeof(ms->loadparm)); + visit_type_str(v, name, &str, errp); + g_free(str); } -static void machine_set_loadparm(Object *obj, const char *val, Error **errp) +static void machine_set_loadparm(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + char *val; int i; + if (!visit_type_str(v, name, &val, errp)) { + return; + } + for (i = 0; i < sizeof(ms->loadparm) && val[i]; i++) { uint8_t c = qemu_toupper(val[i]); /* mimic HMC */ @@ -740,34 +719,72 @@ static void machine_set_loadparm(Object *obj, const char *val, Error **errp) ms->loadparm[i] = ' '; /* pad right with spaces */ } } -static inline void s390_machine_initfn(Object *obj) -{ - object_property_add_bool(obj, "aes-key-wrap", - machine_get_aes_key_wrap, - machine_set_aes_key_wrap); - object_property_set_description(obj, "aes-key-wrap", - "enable/disable AES key wrapping using the CPACF wrapping key"); - object_property_set_bool(obj, "aes-key-wrap", true, NULL); - object_property_add_bool(obj, "dea-key-wrap", - machine_get_dea_key_wrap, - machine_set_dea_key_wrap); - object_property_set_description(obj, "dea-key-wrap", +static void ccw_machine_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + NMIClass *nc = NMI_CLASS(oc); + HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); + S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc); + + s390mc->ri_allowed = true; + s390mc->cpu_model_allowed = true; + s390mc->css_migration_enabled = true; + s390mc->hpage_1m_allowed = true; + mc->init = ccw_init; + mc->reset = s390_machine_reset; + mc->block_default_type = IF_VIRTIO; + mc->no_cdrom = 1; + mc->no_floppy = 1; + mc->no_parallel = 1; + mc->no_sdcard = 1; + mc->max_cpus = S390_MAX_CPUS; + mc->has_hotpluggable_cpus = true; + assert(!mc->get_hotplug_handler); + mc->get_hotplug_handler = s390_get_hotplug_handler; + mc->cpu_index_to_instance_props = s390_cpu_index_to_props; + mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids; + /* it is overridden with 'host' cpu *in kvm_arch_init* */ + mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu"); + hc->plug = s390_machine_device_plug; + hc->unplug_request = s390_machine_device_unplug_request; + nc->nmi_monitor_handler = s390_nmi; + mc->default_ram_id = "s390.ram"; + + object_class_property_add_bool(oc, "aes-key-wrap", + machine_get_aes_key_wrap, + machine_set_aes_key_wrap); + object_class_property_set_description(oc, "aes-key-wrap", + "enable/disable AES key wrapping using the CPACF wrapping key"); + + object_class_property_add_bool(oc, "dea-key-wrap", + machine_get_dea_key_wrap, + machine_set_dea_key_wrap); + object_class_property_set_description(oc, "dea-key-wrap", "enable/disable DEA key wrapping using the CPACF wrapping key"); - object_property_set_bool(obj, "dea-key-wrap", true, NULL); - object_property_add_str(obj, "loadparm", - machine_get_loadparm, machine_set_loadparm); - object_property_set_description(obj, "loadparm", + + object_class_property_add(oc, "loadparm", "loadparm", + machine_get_loadparm, machine_set_loadparm, + NULL, NULL); + object_class_property_set_description(oc, "loadparm", "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted" " to upper case) to pass to machine loader, boot manager," " and guest kernel"); - object_property_add_bool(obj, "zpcii-disable", - machine_get_zpcii_disable, - machine_set_zpcii_disable); - object_property_set_description(obj, "zpcii-disable", + object_class_property_add_bool(oc, "zpcii-disable", + machine_get_zpcii_disable, + machine_set_zpcii_disable); + object_class_property_set_description(oc, "zpcii-disable", "disable zPCI interpretation facilties"); - object_property_set_bool(obj, "zpcii-disable", false, NULL); +} + +static inline void s390_machine_initfn(Object *obj) +{ + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + + ms->aes_key_wrap = true; + ms->dea_key_wrap = true; + ms->zpcii_disable = false; } static const TypeInfo ccw_machine_info = { From 6393b29966fce3c0e47746a9646ae439e7fd0728 Mon Sep 17 00:00:00 2001 From: Pierre Morel Date: Thu, 3 Nov 2022 18:01:41 +0100 Subject: [PATCH 12/12] s390x/cpu topology: add max_threads machine class attribute The S390 CPU topology accepts the smp.threads argument while in reality it does not effectively allow multthreading. Let's keep this behavior for machines older than 7.2 and refuse to use threads in newer machines until multithreading is really exposed to the guest by the machine. Signed-off-by: Pierre Morel Message-Id: <20221103170150.20789-3-pmorel@linux.ibm.com> [thuth: Small fixes to the commit description] Signed-off-by: Thomas Huth --- hw/s390x/s390-virtio-ccw.c | 11 +++++++++++ include/hw/s390x/s390-virtio-ccw.h | 1 + 2 files changed, 12 insertions(+) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 196773c833..560ddbb6fb 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -85,8 +85,15 @@ out: static void s390_init_cpus(MachineState *machine) { MachineClass *mc = MACHINE_GET_CLASS(machine); + S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc); int i; + if (machine->smp.threads > s390mc->max_threads) { + error_report("S390 does not support more than %d threads.", + s390mc->max_threads); + exit(1); + } + /* initialize possible_cpus */ mc->possible_cpu_arch_ids(machine); @@ -731,6 +738,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->cpu_model_allowed = true; s390mc->css_migration_enabled = true; s390mc->hpage_1m_allowed = true; + s390mc->max_threads = 1; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->block_default_type = IF_VIRTIO; @@ -859,8 +867,11 @@ static void ccw_machine_7_1_instance_options(MachineState *machine) static void ccw_machine_7_1_class_options(MachineClass *mc) { + S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc); + ccw_machine_7_2_class_options(mc); compat_props_add(mc->compat_props, hw_compat_7_1, hw_compat_7_1_len); + s390mc->max_threads = S390_MAX_CPUS; } DEFINE_CCW_MACHINE(7_1, "7.1", false); diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index 8a0090a071..4f8a39abda 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -40,6 +40,7 @@ struct S390CcwMachineClass { bool cpu_model_allowed; bool css_migration_enabled; bool hpage_1m_allowed; + int max_threads; }; /* runtime-instrumentation allowed by the machine */