[PULL v2 00/11] Misc patches for 2023-02-08

[PULL v2 00/11] Misc patches for 2023-02-08

[Paolo Bonzini]

The following changes since commit ae2b5d8381a73b27f35f19c988d45c78bb4d5768:

  Merge tag 'pull-include-2023-02-06-v2' of https://repo.or.cz/qemu/armbru into staging (2023-02-08 10:40:06 +0000)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 92f5d4606bedf0e2af0169a32aff9443084c5d0a:

  target/i386: fix ADOX followed by ADCX (2023-02-09 10:17:34 +0100)

----------------------------------------------------------------
* block/iscsi: fix double-free on BUSY or similar statuses
* catch [accel] entry without accelerator
* target/i386: various fixes for BMI and ADX instructions
* make the contents of meson-buildoptions.sh stable

----------------------------------------------------------------

* Include review comments from Thomas and Philippe
* Fix year in subject :)

Paolo Bonzini (8):
      build: make meson-buildoptions.sh stable
      remove unnecessary extern "C" blocks
      block/iscsi: fix double-free on BUSY or similar statuses
      vl: catch [accel] entry without accelerator
      libqtest: split qtest_spawn_qemu function
      libqtest: ensure waitpid() is only called once
      readconfig-test: add test for accelerator configuration
      target/i386: fix ADOX followed by ADCX

Richard Henderson (3):
      tests/tcg/i386: Introduce and use reg_t consistently
      target/i386: Fix BEXTR instruction
      target/i386: Fix C flag for BLSI, BLSMSK, BLSR

 block/iscsi.c                    |   1 +
 include/disas/dis-asm.h          |   8 --
 include/qemu/bswap.h             |   8 --
 include/qemu/envlist.h           |   8 --
 include/qemu/rcu.h               |   8 --
 include/qemu/rcu_queue.h         |   8 --
 include/qemu/uri.h               |   7 --
 scripts/meson-buildoptions.py    |   7 +-
 scripts/meson-buildoptions.sh    |   2 +-
 softmmu/vl.c                     |  15 ++-
 target/i386/tcg/emit.c.inc       |  45 +++++----
 tests/qtest/libqtest.c           | 186 ++++++++++++++++++++++---------------
 tests/qtest/libqtest.h           |  12 +++
 tests/qtest/readconfig-test.c    |  45 +++++++--
 tests/tcg/i386/Makefile.target   |   6 +-
 tests/tcg/i386/test-i386-adcox.c |  75 +++++++++++++++
 tests/tcg/i386/test-i386-bmi2.c  | 194 +++++++++++++++++++++------------------
 17 files changed, 386 insertions(+), 249 deletions(-)
 create mode 100644 tests/tcg/i386/test-i386-adcox.c
-- 
2.39.1

[PULL v2 04/11] vl: catch [accel] entry without accelerator

[Paolo Bonzini]

Avoid a SIGSEGV and return an error instead.

Reported-by: Thomas Huth <thuth@redhat.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1439
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index b2ee3fee3f06..459588aa7d14 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2204,14 +2204,18 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     int ret;
     bool qtest_with_kvm;
 
+    if (!acc) {
+        error_setg(errp, QERR_MISSING_PARAMETER, "accel");
+        goto bad;
+    }
+
     qtest_with_kvm = g_str_equal(acc, "kvm") && qtest_chrdev != NULL;
 
     if (!ac) {
-        *p_init_failed = true;
         if (!qtest_with_kvm) {
             error_report("invalid accelerator %s", acc);
         }
-        return 0;
+        goto bad;
     }
     accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
     object_apply_compat_props(OBJECT(accel));
@@ -2221,14 +2225,17 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
 
     ret = accel_init_machine(accel, current_machine);
     if (ret < 0) {
-        *p_init_failed = true;
         if (!qtest_with_kvm || ret != -ENOENT) {
             error_report("failed to initialize %s: %s", acc, strerror(-ret));
         }
-        return 0;
+        goto bad;
     }
 
     return 1;
+
+bad:
+    *p_init_failed = true;
+    return 0;
 }
 
 static void configure_accelerators(const char *progname)
-- 
2.39.1

[PULL v2 07/11] readconfig-test: add test for accelerator configuration

[Paolo Bonzini]

Test that it does not cause a SIGSEGV, and cover a valid configuration
as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqtest.c        | 28 +++++++++++++++++-----
 tests/qtest/libqtest.h        | 12 ++++++++++
 tests/qtest/readconfig-test.c | 45 ++++++++++++++++++++++++++++-------
 3 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index ce5f235e25f1..5f9d7440781c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -420,6 +420,26 @@ static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
     return s;
 }
 
+QTestState *qtest_init_bare(const char *args)
+{
+    QTestState *s = qtest_spawn_qemu("%s", args);
+
+    /*
+     * Stopping QEMU for debugging is not supported on Windows.
+     *
+     * Using DebugActiveProcess() API can suspend the QEMU process,
+     * but gdb cannot attach to the process. Using the undocumented
+     * NtSuspendProcess() can suspend the QEMU process and gdb can
+     * attach to the process, but gdb cannot resume it.
+     */
+#ifndef _WIN32
+    if (getenv("QTEST_STOP")) {
+        kill(s->qemu_pid, SIGSTOP);
+    }
+#endif
+    return s;
+}
+
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
     QTestState *s;
@@ -477,12 +497,8 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     }
 
     /*
-     * Stopping QEMU for debugging is not supported on Windows.
-     *
-     * Using DebugActiveProcess() API can suspend the QEMU process,
-     * but gdb cannot attach to the process. Using the undocumented
-     * NtSuspendProcess() can suspend the QEMU process and gdb can
-     * attach to the process, but gdb cannot resume it.
+     * Stopping QEMU for debugging is not supported on Windows;
+     * see qtest_init_bare for more information.
      */
 #ifndef _WIN32
     if (getenv("QTEST_STOP")) {
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b36f..7ca7df26a2c0 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -23,6 +23,18 @@
 
 typedef struct QTestState QTestState;
 
+/**
+ * qtest_init_bare:
+ * @extra_args: other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
+ *
+ * Return a QTestState instance without automatically creating any
+ * sockets for QMP and qtest communication.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_bare(const char *args);
+
 /**
  * qtest_initf:
  * @fmt: Format for creating other arguments to pass to QEMU, formatted
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 9ef870643dcd..4c11883e36eb 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -19,13 +19,11 @@
 #include "qapi/qmp/qstring.h"
 #include "qemu/units.h"
 
-static QTestState *qtest_init_with_config(const char *cfgdata)
+static char *qtest_write_config(const char *cfgdata)
 {
     GError *error = NULL;
-    g_autofree char *args = NULL;
     int cfgfd = -1;
-    g_autofree char *cfgpath = NULL;
-    QTestState *qts;
+    char *cfgpath;
     ssize_t ret;
 
     cfgfd = g_file_open_tmp("readconfig-test-XXXXXX", &cfgpath, &error);
@@ -38,13 +36,14 @@ static QTestState *qtest_init_with_config(const char *cfgdata)
         unlink(cfgpath);
     }
     g_assert_cmpint(ret, ==, strlen(cfgdata));
+    return cfgpath;
+}
 
-    args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
-
-    qts = qtest_init(args);
-
+static QTestState *qtest_init_with_config(const char *cfgdata)
+{
+    g_autofree char *cfgpath = qtest_write_config(cfgdata);
+    QTestState *qts = qtest_initf("-nodefaults -machine none -readconfig %s", cfgpath);
     unlink(cfgpath);
-
     return qts;
 }
 
@@ -176,6 +175,32 @@ static void test_object_rng(void)
     qtest_quit(qts);
 }
 
+static void test_valid_accel(void)
+{
+    const char *cfgdata =
+        "[accel]\n"
+        "accel = \"qtest\"\n";
+
+    QTestState *qts = qtest_init_with_config(cfgdata);
+    qtest_quit(qts);
+}
+
+static void test_invalid_accel(void)
+{
+    const char *cfgdata =
+        "[accel]\n"
+        "foo = \"bar\"\n";
+
+    g_autofree char *cfgpath = qtest_write_config(cfgdata);
+    g_autofree char *args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
+    QTestState *qts = qtest_init_bare(args);
+
+    qtest_set_expected_status(qts, 1);
+    qtest_wait_qemu(qts);
+    g_free(qts);
+    unlink(cfgpath);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch;
@@ -192,6 +217,8 @@ int main(int argc, char *argv[])
 #endif
 
     qtest_add_func("readconfig/object-rng", test_object_rng);
+    qtest_add_func("readconfig/invalid-accel", test_invalid_accel);
+    qtest_add_func("readconfig/valid-accel", test_valid_accel);
 
     return g_test_run();
 }
-- 
2.39.1

Re: [PULL v2 00/11] Misc patches for 2023-02-08

[Peter Maydell]

On Thu, 9 Feb 2023 at 09:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit ae2b5d8381a73b27f35f19c988d45c78bb4d5768:
>
>   Merge tag 'pull-include-2023-02-06-v2' of https://repo.or.cz/qemu/armbru into staging (2023-02-08 10:40:06 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 92f5d4606bedf0e2af0169a32aff9443084c5d0a:
>
>   target/i386: fix ADOX followed by ADCX (2023-02-09 10:17:34 +0100)
>
> ----------------------------------------------------------------
> * block/iscsi: fix double-free on BUSY or similar statuses
> * catch [accel] entry without accelerator
> * target/i386: various fixes for BMI and ADX instructions
> * make the contents of meson-buildoptions.sh stable
>
> ----------------------------------------------------------------

This seems to consistently fail on the BSD jobs:
https://gitlab.com/qemu-project/qemu/-/jobs/3742560167
https://gitlab.com/qemu-project/qemu/-/jobs/3742560168
https://gitlab.com/qemu-project/qemu/-/jobs/3743330014
https://gitlab.com/qemu-project/qemu/-/jobs/3743330018
https://gitlab.com/qemu-project/qemu/-/jobs/3743909430

Unfortunately the logs don't seem very informative :-(

-- PMM

Re: [PULL v2 00/11] Misc patches for 2023-02-08

[Thomas Huth]

On 09/02/2023 16.17, Peter Maydell wrote:
> On Thu, 9 Feb 2023 at 09:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> The following changes since commit ae2b5d8381a73b27f35f19c988d45c78bb4d5768:
>>
>>    Merge tag 'pull-include-2023-02-06-v2' of https://repo.or.cz/qemu/armbru into staging (2023-02-08 10:40:06 +0000)
>>
>> are available in the Git repository at:
>>
>>    https://gitlab.com/bonzini/qemu.git tags/for-upstream
>>
>> for you to fetch changes up to 92f5d4606bedf0e2af0169a32aff9443084c5d0a:
>>
>>    target/i386: fix ADOX followed by ADCX (2023-02-09 10:17:34 +0100)
>>
>> ----------------------------------------------------------------
>> * block/iscsi: fix double-free on BUSY or similar statuses
>> * catch [accel] entry without accelerator
>> * target/i386: various fixes for BMI and ADX instructions
>> * make the contents of meson-buildoptions.sh stable
>>
>> ----------------------------------------------------------------
> 
> This seems to consistently fail on the BSD jobs:
> https://gitlab.com/qemu-project/qemu/-/jobs/3742560167
> https://gitlab.com/qemu-project/qemu/-/jobs/3742560168
> https://gitlab.com/qemu-project/qemu/-/jobs/3743330014
> https://gitlab.com/qemu-project/qemu/-/jobs/3743330018
> https://gitlab.com/qemu-project/qemu/-/jobs/3743909430
> 
> Unfortunately the logs don't seem very informative :-(

Out of curiosity, I added some debug printfs and ran the test a couple of 
more times in the CI. It's pretty weird... There are two problems in 
qtest_wait_qemu() :

1) In these FreeBSD runners, that "do { waitpid(..., WNOHANG) } while" loop 
does not finish in time, so the code sends a SIGKILL signal to the QEMU 
process. This then causes qtest_check_status() to abort() later since QEMU 
finished with a bad status. Should we handle this more gracefully so that 
the code does not abort() after the SIGKILL?

2) By increasing WAITPID_TIMEOUT to more than 75 seconds, I can make the 
test succeed. It's really weird, but it seems like the do-while loop always 
takes 75 seconds to succeed in case of the "invalid-accel" test - but I've 
got no clue where that delay could come from... Any ideas?

  Thomas