[PULL 00/15] virtio,pc,pci: fixes

[PULL 00/15] virtio,pc,pci: fixes

[Michael S. Tsirkin]

We have a reported regression because of the switch to
smbios 3.0, and maybe need to revert that, but Igor asked
for a bit more time to investigate.

The following changes since commit abf635ddfe3242df907f58967f3c1e6763bbca2d:

  Update version for v8.2.0-rc2 release (2023-11-28 16:31:16 -0500)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to d4ad718733df4c87b9133bb57ea77b0040923541:

  vhost-user-scsi: free the inflight area when reset (2023-12-01 12:01:45 -0500)

----------------------------------------------------------------
virtio,pc,pci: fixes

Fixes all over the place

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Akihiko Odaki (1):
      pcie_sriov: Remove g_new assertion

Ani Sinha (1):
      tests/acpi/bios-tables-test: do not write new blobs unless there are changes

Daniel Hoffman (1):
      hw/i386: fix short-circuit logic with non-optimizing builds

Eric Auger (1):
      virtio-iommu: Remove useless !sdev check in virtio_iommu_probe()

Li Feng (2):
      vhost-user: fix the reconnect error
      vhost-user-scsi: free the inflight area when reset

Manos Pitsidianakis (2):
      virtio-snd: check AUD_register_card return value
      virtio-sound: add realize() error cleanup path

Michael S. Tsirkin (2):
      osdep: add getloadavg
      netdev: set timeout depending on loadavg

Philippe Mathieu-Daudé (1):
      hw/acpi/erst: Do not ignore Error* in realize handler

Robert Hoo (1):
      msix: unset PCIDevice::msix_vector_poll_notifier in rollback

Volker Rümelin (3):
      hw/audio/virtio-snd-pci: fix the PCI class code
      hw/audio/hda-codec: fix multiplication overflow
      hw/audio/hda-codec: reenable the audio mixer

 include/qemu/osdep.h           | 10 ++++++++++
 hw/acpi/erst.c                 | 10 ++++++++++
 hw/audio/hda-codec.c           | 41 ++++++++++++++++++++++++++--------------
 hw/audio/virtio-snd-pci.c      |  2 ++
 hw/audio/virtio-snd.c          | 43 ++++++++++++++++++++++++------------------
 hw/block/vhost-user-blk.c      |  8 +++-----
 hw/i386/x86.c                  | 15 ++++++++++++---
 hw/pci/msix.c                  |  1 +
 hw/pci/pcie_sriov.c            |  1 -
 hw/scsi/vhost-user-scsi.c      | 19 ++++++++++++++++++-
 hw/virtio/vhost-user-gpio.c    |  3 ++-
 hw/virtio/virtio-iommu.c       |  3 ---
 hw/virtio/virtio.c             |  2 +-
 tests/qtest/bios-tables-test.c | 14 +++++++++++++-
 tests/qtest/netdev-socket.c    | 28 ++++++++++++++++++++++++++-
 meson.build                    |  1 +
 16 files changed, 152 insertions(+), 49 deletions(-)

[PULL 01/15] osdep: add getloadavg

[Michael S. Tsirkin]

getloadavg is supported on Linux, BSDs, Solaris.

Following man page:
RETURN VALUE
       If the load average was unobtainable, -1 is returned; otherwise,
       the number of samples actually retrieved is returned.

accordingly, make stub for systems which don't support this function return -1
for consistency.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/osdep.h | 10 ++++++++++
 meson.build          |  1 +
 2 files changed, 11 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 475a1c62ff..d30ba73eda 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -779,6 +779,16 @@ static inline int platform_does_not_support_system(const char *command)
 }
 #endif /* !HAVE_SYSTEM_FUNCTION */
 
+/**
+ * If the load average was unobtainable, -1 is returned
+ */
+#ifndef HAVE_GETLOADAVG_FUNCTION
+static inline int getloadavg(double loadavg[], int nelem)
+{
+    return -1;
+}
+#endif /* !HAVE_GETLOADAVG_FUNCTION */
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/meson.build b/meson.build
index ec01f8b138..d2c4c2adb3 100644
--- a/meson.build
+++ b/meson.build
@@ -2293,6 +2293,7 @@ config_host_data.set('HAVE_GLIB_WITH_SLICE_ALLOCATOR', glib_has_gslice)
 config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
 config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
+config_host_data.set('HAVE_GETLOADAVG_FUNCTION', cc.has_function('getloadavg', prefix: '#include <stdlib.h>'))
 if rbd.found()
   config_host_data.set('HAVE_RBD_NAMESPACE_EXISTS',
                        cc.has_function('rbd_namespace_exists',
-- 
MST

[PULL 02/15] netdev: set timeout depending on loadavg

[Michael S. Tsirkin]

netdev test keeps failing sometimes.
I don't think we should increase the timeout some more:
let's try something else instead, testing how busy the
system is.

Seems to work for me.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/qtest/netdev-socket.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 7ba1eff120..bb99d08b5e 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -18,6 +18,32 @@
 
 #define CONNECTION_TIMEOUT    120
 
+static double connection_timeout(void)
+{
+    double load;
+    int ret = getloadavg(&load, 1);
+
+    /*
+     * If we can't get load data, or load is low because we just started
+     * running, assume load of 1 (we are alone in this system).
+     */
+    if (ret < 1 || load < 1.0) {
+        load = 1.0;
+    }
+    /*
+     * No one wants to wait more than 10 minutes for this test. Higher load?
+     * Too bad.
+     */
+    if (load > 10.0) {
+        fprintf(stderr, "Warning: load %f higher than 10 - test might timeout\n",
+                load);
+        load = 10.0;
+    }
+
+    /* if load is high increase timeout as we might not get a chance to run */
+    return load * CONNECTION_TIMEOUT;
+}
+
 #define EXPECT_STATE(q, e, t)                             \
 do {                                                      \
     char *resp = NULL;                                    \
@@ -31,7 +57,7 @@ do {                                                      \
         if (g_str_equal(resp, e)) {                       \
             break;                                        \
         }                                                 \
-    } while (g_test_timer_elapsed() < CONNECTION_TIMEOUT); \
+    } while (g_test_timer_elapsed() < connection_timeout()); \
     g_assert_cmpstr(resp, ==, e);                         \
     g_free(resp);                                         \
 } while (0)
-- 
MST

[PULL 03/15] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Michael S. Tsirkin]

From: Ani Sinha <anisinha@redhat.com>

When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
test variants are dumped regardless of whether there are any actual changes to
the tables or not. This creates lot of new files for various test variants that
are not part of the git repository. This is because we do not check in all table
blobs for all test variants into the repository. Only those blobs for those
variants that are different from the generic test-variant agnostic blob are
checked in.

This change makes the test smarter by checking if at all there are any changes
in the tables from the checked-in gold master blobs and take actions
accordingly.

When there are no changes:
 - No new table blobs would be written.
 - Existing table blobs will be refreshed (git diff will show no changes).
When there are changes:
 - New table blob files will be dumped.
 - Existing table blobs will be refreshed (git diff will show that the files
   changed, asl diff will show the actual changes).
When new tables are introduced:
 - Zero byte empty file blobs for new tables as instructed in the header of
   bios-tables-test.c will be regenerated to actual table blobs.

This would make analyzing changes to tables less confusing and there would
be no need to clean useless untracked files when there are no table changes.

CC: peter.maydell@linaro.org
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Message-Id: <20231107044952.5461-1-anisinha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/qtest/bios-tables-test.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 71af5cf69f..fe6a9a8563 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -112,6 +112,7 @@ static const char *iasl;
 #endif
 
 static int verbosity_level;
+static GArray *load_expected_aml(test_data *data);
 
 static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
 {
@@ -244,21 +245,32 @@ static void test_acpi_fadt_table(test_data *data)
 
 static void dump_aml_files(test_data *data, bool rebuild)
 {
-    AcpiSdtTable *sdt;
+    AcpiSdtTable *sdt, *exp_sdt;
     GError *error = NULL;
     gchar *aml_file = NULL;
+    test_data exp_data = {};
     gint fd;
     ssize_t ret;
     int i;
 
+    exp_data.tables = load_expected_aml(data);
     for (i = 0; i < data->tables->len; ++i) {
         const char *ext = data->variant ? data->variant : "";
         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
+        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
         g_assert(sdt->aml);
+        g_assert(exp_sdt->aml);
 
         if (rebuild) {
             aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
                                        sdt->aml, ext);
+            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
+                sdt->aml_len == exp_sdt->aml_len &&
+                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
+                /* identical tables, no need to write new files */
+                g_free(aml_file);
+                continue;
+            }
             fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
                         S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
             if (fd < 0) {
-- 
MST

Re: [PULL 03/15] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Ani Sinha]

> On 01-Dec-2023, at 10:45 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> From: Ani Sinha <anisinha@redhat.com>
> 
> When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
> test variants are dumped regardless of whether there are any actual changes to
> the tables or not. This creates lot of new files for various test variants that
> are not part of the git repository. This is because we do not check in all table
> blobs for all test variants into the repository. Only those blobs for those
> variants that are different from the generic test-variant agnostic blob are
> checked in.
> 
> This change makes the test smarter by checking if at all there are any changes
> in the tables from the checked-in gold master blobs and take actions
> accordingly.
> 
> When there are no changes:
> - No new table blobs would be written.
> - Existing table blobs will be refreshed (git diff will show no changes).
> When there are changes:
> - New table blob files will be dumped.
> - Existing table blobs will be refreshed (git diff will show that the files
>   changed, asl diff will show the actual changes).
> When new tables are introduced:
> - Zero byte empty file blobs for new tables as instructed in the header of
>   bios-tables-test.c will be regenerated to actual table blobs.
> 
> This would make analyzing changes to tables less confusing and there would
> be no need to clean useless untracked files when there are no table changes.
> 
> CC: peter.maydell@linaro.org
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> Message-Id: <20231107044952.5461-1-anisinha@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

You missed DanPB and Igor’s tags :(

> ---
> tests/qtest/bios-tables-test.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 71af5cf69f..fe6a9a8563 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -112,6 +112,7 @@ static const char *iasl;
> #endif
> 
> static int verbosity_level;
> +static GArray *load_expected_aml(test_data *data);
> 
> static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
> {
> @@ -244,21 +245,32 @@ static void test_acpi_fadt_table(test_data *data)
> 
> static void dump_aml_files(test_data *data, bool rebuild)
> {
> -    AcpiSdtTable *sdt;
> +    AcpiSdtTable *sdt, *exp_sdt;
>     GError *error = NULL;
>     gchar *aml_file = NULL;
> +    test_data exp_data = {};
>     gint fd;
>     ssize_t ret;
>     int i;
> 
> +    exp_data.tables = load_expected_aml(data);
>     for (i = 0; i < data->tables->len; ++i) {
>         const char *ext = data->variant ? data->variant : "";
>         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
> +        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
>         g_assert(sdt->aml);
> +        g_assert(exp_sdt->aml);
> 
>         if (rebuild) {
>             aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
>                                        sdt->aml, ext);
> +            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
> +                sdt->aml_len == exp_sdt->aml_len &&
> +                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
> +                /* identical tables, no need to write new files */
> +                g_free(aml_file);
> +                continue;
> +            }
>             fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
>                         S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
>             if (fd < 0) {
> -- 
> MST
> 

Re: [PULL 03/15] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Michael S. Tsirkin]

On Fri, Dec 01, 2023 at 10:57:04PM +0530, Ani Sinha wrote:
> 
> 
> > On 01-Dec-2023, at 10:45 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > 
> > From: Ani Sinha <anisinha@redhat.com>
> > 
> > When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
> > test variants are dumped regardless of whether there are any actual changes to
> > the tables or not. This creates lot of new files for various test variants that
> > are not part of the git repository. This is because we do not check in all table
> > blobs for all test variants into the repository. Only those blobs for those
> > variants that are different from the generic test-variant agnostic blob are
> > checked in.
> > 
> > This change makes the test smarter by checking if at all there are any changes
> > in the tables from the checked-in gold master blobs and take actions
> > accordingly.
> > 
> > When there are no changes:
> > - No new table blobs would be written.
> > - Existing table blobs will be refreshed (git diff will show no changes).
> > When there are changes:
> > - New table blob files will be dumped.
> > - Existing table blobs will be refreshed (git diff will show that the files
> >   changed, asl diff will show the actual changes).
> > When new tables are introduced:
> > - Zero byte empty file blobs for new tables as instructed in the header of
> >   bios-tables-test.c will be regenerated to actual table blobs.
> > 
> > This would make analyzing changes to tables less confusing and there would
> > be no need to clean useless untracked files when there are no table changes.
> > 
> > CC: peter.maydell@linaro.org
> > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > Message-Id: <20231107044952.5461-1-anisinha@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> You missed DanPB and Igor’s tags :(

fixed now, thanks!

> > ---
> > tests/qtest/bios-tables-test.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 71af5cf69f..fe6a9a8563 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -112,6 +112,7 @@ static const char *iasl;
> > #endif
> > 
> > static int verbosity_level;
> > +static GArray *load_expected_aml(test_data *data);
> > 
> > static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
> > {
> > @@ -244,21 +245,32 @@ static void test_acpi_fadt_table(test_data *data)
> > 
> > static void dump_aml_files(test_data *data, bool rebuild)
> > {
> > -    AcpiSdtTable *sdt;
> > +    AcpiSdtTable *sdt, *exp_sdt;
> >     GError *error = NULL;
> >     gchar *aml_file = NULL;
> > +    test_data exp_data = {};
> >     gint fd;
> >     ssize_t ret;
> >     int i;
> > 
> > +    exp_data.tables = load_expected_aml(data);
> >     for (i = 0; i < data->tables->len; ++i) {
> >         const char *ext = data->variant ? data->variant : "";
> >         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
> > +        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
> >         g_assert(sdt->aml);
> > +        g_assert(exp_sdt->aml);
> > 
> >         if (rebuild) {
> >             aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
> >                                        sdt->aml, ext);
> > +            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
> > +                sdt->aml_len == exp_sdt->aml_len &&
> > +                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
> > +                /* identical tables, no need to write new files */
> > +                g_free(aml_file);
> > +                continue;
> > +            }
> >             fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
> >                         S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
> >             if (fd < 0) {
> > -- 
> > MST
> > 

Re: [PULL 03/15] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Igor Mammedov]

On Fri, 1 Dec 2023 12:15:16 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> From: Ani Sinha <anisinha@redhat.com>
> 
> When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
> test variants are dumped regardless of whether there are any actual changes to
> the tables or not. This creates lot of new files for various test variants that
> are not part of the git repository. This is because we do not check in all table
> blobs for all test variants into the repository. Only those blobs for those
> variants that are different from the generic test-variant agnostic blob are
> checked in.
> 
> This change makes the test smarter by checking if at all there are any changes
> in the tables from the checked-in gold master blobs and take actions
> accordingly.
> 
> When there are no changes:
>  - No new table blobs would be written.
>  - Existing table blobs will be refreshed (git diff will show no changes).
> When there are changes:
>  - New table blob files will be dumped.
>  - Existing table blobs will be refreshed (git diff will show that the files
>    changed, asl diff will show the actual changes).

> When new tables are introduced:
>  - Zero byte empty file blobs for new tables as instructed in the header of
>    bios-tables-test.c will be regenerated to actual table blobs.

Ani,

what previously worked is the is there weren't _any_ expected blobs
(exact match/fallback) found a new table would be dumped.
So then later 'git status' would show a list of new files. 
With this commit it's however not dumping new files files,
and explodes at
#5  0x000055555556808e in load_expected_aml (data=0x7fffffffd7b0) at tests/qtest/bios-tables-test.c:414
#6  0x00005555555676b0 in dump_aml_files (data=0x7fffffffd7b0, rebuild=true) at tests/qtest/bios-tables-test.c:256

sure it can be worked around by manually creating empty files
for expected files but if it's a new machine type, one has to create
a bunch of them basically running multiple iterations of rebuild in V=2
mode to see what's missing.
(IMHO cure turned out to be worse than illness)

for reproducing create a test for other than default machine type, ex:

    test_data data = {                                                           
        .machine = "pc-i440fx-2.0",                                              
        .variant = ".pc_legacy",                                                              
    }; 


is it possible to fix it so that one doesn't have to create empty files manually?

> 
> This would make analyzing changes to tables less confusing and there would
> be no need to clean useless untracked files when there are no table changes.
> 
> CC: peter.maydell@linaro.org
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> Message-Id: <20231107044952.5461-1-anisinha@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  tests/qtest/bios-tables-test.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 71af5cf69f..fe6a9a8563 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -112,6 +112,7 @@ static const char *iasl;
>  #endif
>  
>  static int verbosity_level;
> +static GArray *load_expected_aml(test_data *data);
>  
>  static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
>  {
> @@ -244,21 +245,32 @@ static void test_acpi_fadt_table(test_data *data)
>  
>  static void dump_aml_files(test_data *data, bool rebuild)
>  {
> -    AcpiSdtTable *sdt;
> +    AcpiSdtTable *sdt, *exp_sdt;
>      GError *error = NULL;
>      gchar *aml_file = NULL;
> +    test_data exp_data = {};
>      gint fd;
>      ssize_t ret;
>      int i;
>  
> +    exp_data.tables = load_expected_aml(data);
>      for (i = 0; i < data->tables->len; ++i) {
>          const char *ext = data->variant ? data->variant : "";
>          sdt = &g_array_index(data->tables, AcpiSdtTable, i);
> +        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
>          g_assert(sdt->aml);
> +        g_assert(exp_sdt->aml);
>  
>          if (rebuild) {
>              aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
>                                         sdt->aml, ext);
> +            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
> +                sdt->aml_len == exp_sdt->aml_len &&
> +                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
> +                /* identical tables, no need to write new files */
> +                g_free(aml_file);
> +                continue;
> +            }
>              fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
>                          S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
>              if (fd < 0) {

Re: [PULL 03/15] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Ani Sinha]

> On 24-Jan-2024, at 14:28, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> On Fri, 1 Dec 2023 12:15:16 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> From: Ani Sinha <anisinha@redhat.com>
>> 
>> When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
>> test variants are dumped regardless of whether there are any actual changes to
>> the tables or not. This creates lot of new files for various test variants that
>> are not part of the git repository. This is because we do not check in all table
>> blobs for all test variants into the repository. Only those blobs for those
>> variants that are different from the generic test-variant agnostic blob are
>> checked in.
>> 
>> This change makes the test smarter by checking if at all there are any changes
>> in the tables from the checked-in gold master blobs and take actions
>> accordingly.
>> 
>> When there are no changes:
>> - No new table blobs would be written.
>> - Existing table blobs will be refreshed (git diff will show no changes).
>> When there are changes:
>> - New table blob files will be dumped.
>> - Existing table blobs will be refreshed (git diff will show that the files
>>   changed, asl diff will show the actual changes).
> 
>> When new tables are introduced:
>> - Zero byte empty file blobs for new tables as instructed in the header of
>>   bios-tables-test.c will be regenerated to actual table blobs.
> 
> Ani,
> 
> what previously worked is the is there weren't _any_ expected blobs
> (exact match/fallback) found a new table would be dumped.
> So then later 'git status' would show a list of new files.

Yes this was the original complaint from Peter. Lots of new files were generated when basically they were identical but differed in naming.

> With this commit it's however not dumping new files files,
> and explodes at
> #5  0x000055555556808e in load_expected_aml (data=0x7fffffffd7b0) at tests/qtest/bios-tables-test.c:414
> #6  0x00005555555676b0 in dump_aml_files (data=0x7fffffffd7b0, rebuild=true) at tests/qtest/bios-tables-test.c:256
> 
> sure it can be worked around by manually creating empty files
> for expected files

Yes this has already been documented in the commit log.

> but if it's a new machine type, one has to create
> a bunch of them basically running multiple iterations of rebuild in V=2
> mode to see what's missing.
> (IMHO cure turned out to be worse than illness)

I don’t know how common it is to create new machines. 

> 
> for reproducing create a test for other than default machine type, ex:
> 
>    test_data data = {                                                           
>        .machine = "pc-i440fx-2.0",                                              
>        .variant = ".pc_legacy",                                                              
>    }; 
> 
> 
> is it possible to fix it so that one doesn't have to create empty files manually?
> 
>> 
>> This would make analyzing changes to tables less confusing and there would
>> be no need to clean useless untracked files when there are no table changes.
>> 
>> CC: peter.maydell@linaro.org
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>> Message-Id: <20231107044952.5461-1-anisinha@redhat.com>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>> tests/qtest/bios-tables-test.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>> index 71af5cf69f..fe6a9a8563 100644
>> --- a/tests/qtest/bios-tables-test.c
>> +++ b/tests/qtest/bios-tables-test.c
>> @@ -112,6 +112,7 @@ static const char *iasl;
>> #endif
>> 
>> static int verbosity_level;
>> +static GArray *load_expected_aml(test_data *data);
>> 
>> static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
>> {
>> @@ -244,21 +245,32 @@ static void test_acpi_fadt_table(test_data *data)
>> 
>> static void dump_aml_files(test_data *data, bool rebuild)
>> {
>> -    AcpiSdtTable *sdt;
>> +    AcpiSdtTable *sdt, *exp_sdt;
>>     GError *error = NULL;
>>     gchar *aml_file = NULL;
>> +    test_data exp_data = {};
>>     gint fd;
>>     ssize_t ret;
>>     int i;
>> 
>> +    exp_data.tables = load_expected_aml(data);
>>     for (i = 0; i < data->tables->len; ++i) {
>>         const char *ext = data->variant ? data->variant : "";
>>         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
>> +        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
>>         g_assert(sdt->aml);
>> +        g_assert(exp_sdt->aml);
>> 
>>         if (rebuild) {
>>             aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
>>                                        sdt->aml, ext);
>> +            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
>> +                sdt->aml_len == exp_sdt->aml_len &&
>> +                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
>> +                /* identical tables, no need to write new files */
>> +                g_free(aml_file);
>> +                continue;
>> +            }
>>             fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
>>                         S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
>>             if (fd < 0) {
> 

[PULL 04/15] hw/audio/virtio-snd-pci: fix the PCI class code

[Michael S. Tsirkin]

From: Volker Rümelin <vr_qemu@t-online.de>

The virtio sound device is currently an unclassified PCI device.

~> sudo lspci -s '00:02.0' -v -nn | head -n 2
00:02.0 Unclassified device [00ff]:
 Red Hat, Inc. Device [1af4:1059] (rev 01)
        Subsystem: Red Hat, Inc. Device [1af4:1100]

Set the correct PCI class code to change the device to a
multimedia audio controller.

~> sudo lspci -s '00:02.0' -v -nn | head -n 2
00:02.0 Multimedia audio controller [0401]:
 Red Hat, Inc. Device [1af4:1059] (rev 01)
        Subsystem: Red Hat, Inc. Device [1af4:1100]

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20231107185034.6434-1-vr_qemu@t-online.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/audio/virtio-snd-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/audio/virtio-snd-pci.c b/hw/audio/virtio-snd-pci.c
index 0f92e0752b..ab58c6410e 100644
--- a/hw/audio/virtio-snd-pci.c
+++ b/hw/audio/virtio-snd-pci.c
@@ -47,12 +47,14 @@ static void virtio_snd_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+    PCIDeviceClass *pcidevklass = PCI_DEVICE_CLASS(klass);
 
     device_class_set_props(dc, virtio_snd_pci_properties);
     dc->desc = "Virtio Sound";
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
 
     vpciklass->realize = virtio_snd_pci_realize;
+    pcidevklass->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
 }
 
 static void virtio_snd_pci_instance_init(Object *obj)
-- 
MST

[PULL 05/15] hw/audio/hda-codec: fix multiplication overflow

[Michael S. Tsirkin]

From: Volker Rümelin <vr_qemu@t-online.de>

After a relatively short time, there is an multiplication overflow
when multiplying (now - buft_start) with hda_bytes_per_second().
While the uptime now - buft_start only overflows after 2**63 ns
= 292.27 years, this happens hda_bytes_per_second() times faster
with the multiplication. At 44100 samples/s * 2 channels
* 2 bytes/channel = 176400 bytes/s that is 14.52 hours. After the
multiplication overflow the affected audio stream stalls.

Replace the multiplication and following division with muldiv64()
to prevent a multiplication overflow.

Fixes: 280c1e1cdb ("audio/hda: create millisecond timers that handle IO")
Reported-by: M_O_Bz <m_o_bz@163.com>
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20231105172552.8405-1-vr_qemu@t-online.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/audio/hda-codec.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index b9ad1f4c39..f756e419bb 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -22,6 +22,7 @@
 #include "hw/qdev-properties.h"
 #include "intel-hda.h"
 #include "migration/vmstate.h"
+#include "qemu/host-utils.h"
 #include "qemu/module.h"
 #include "intel-hda-defs.h"
 #include "audio/audio.h"
@@ -189,9 +190,9 @@ struct HDAAudioState {
     bool     use_timer;
 };
 
-static inline int64_t hda_bytes_per_second(HDAAudioStream *st)
+static inline uint32_t hda_bytes_per_second(HDAAudioStream *st)
 {
-    return 2LL * st->as.nchannels * st->as.freq;
+    return 2 * (uint32_t)st->as.nchannels * (uint32_t)st->as.freq;
 }
 
 static inline void hda_timer_sync_adjust(HDAAudioStream *st, int64_t target_pos)
@@ -222,12 +223,18 @@ static void hda_audio_input_timer(void *opaque)
 
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
-    int64_t buft_start = st->buft_start;
+    int64_t uptime = now - st->buft_start;
     int64_t wpos = st->wpos;
     int64_t rpos = st->rpos;
+    int64_t wanted_rpos;
 
-    int64_t wanted_rpos = hda_bytes_per_second(st) * (now - buft_start)
-                          / NANOSECONDS_PER_SECOND;
+    if (uptime <= 0) {
+        /* wanted_rpos <= 0 */
+        goto out_timer;
+    }
+
+    wanted_rpos = muldiv64(uptime, hda_bytes_per_second(st),
+                           NANOSECONDS_PER_SECOND);
     wanted_rpos &= -4; /* IMPORTANT! clip to frames */
 
     if (wanted_rpos <= rpos) {
@@ -286,12 +293,18 @@ static void hda_audio_output_timer(void *opaque)
 
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
-    int64_t buft_start = st->buft_start;
+    int64_t uptime = now - st->buft_start;
     int64_t wpos = st->wpos;
     int64_t rpos = st->rpos;
+    int64_t wanted_wpos;
 
-    int64_t wanted_wpos = hda_bytes_per_second(st) * (now - buft_start)
-                          / NANOSECONDS_PER_SECOND;
+    if (uptime <= 0) {
+        /* wanted_wpos <= 0 */
+        goto out_timer;
+    }
+
+    wanted_wpos = muldiv64(uptime, hda_bytes_per_second(st),
+                           NANOSECONDS_PER_SECOND);
     wanted_wpos &= -4; /* IMPORTANT! clip to frames */
 
     if (wanted_wpos <= wpos) {
-- 
MST

[PULL 06/15] hw/audio/hda-codec: reenable the audio mixer

[Michael S. Tsirkin]

From: Volker Rümelin <vr_qemu@t-online.de>

Commit b7639b7dd0 ("hw/audio: Simplify hda audio init") inverted
the sense of hda codec property mixer during initialization.
Change the code so that mixer=on enables the hda mixer emulation
and mixer=off disables the hda mixer emulation.

With this change audio playback and recording streams don't start
muted by default.

Fixes: b7639b7dd0 ("hw/audio: Simplify hda audio init")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20231105172552.8405-2-vr_qemu@t-online.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/audio/hda-codec.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index f756e419bb..0bc20d49f6 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -868,10 +868,10 @@ static Property hda_audio_properties[] = {
 static void hda_audio_init_output(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
-    const struct desc_codec *desc = &output_nomixemu;
+    const struct desc_codec *desc = &output_mixemu;
 
     if (!a->mixer) {
-        desc = &output_mixemu;
+        desc = &output_nomixemu;
     }
 
     hda_audio_init(hda, desc, errp);
@@ -880,10 +880,10 @@ static void hda_audio_init_output(HDACodecDevice *hda, Error **errp)
 static void hda_audio_init_duplex(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
-    const struct desc_codec *desc = &duplex_nomixemu;
+    const struct desc_codec *desc = &duplex_mixemu;
 
     if (!a->mixer) {
-        desc = &duplex_mixemu;
+        desc = &duplex_nomixemu;
     }
 
     hda_audio_init(hda, desc, errp);
@@ -892,10 +892,10 @@ static void hda_audio_init_duplex(HDACodecDevice *hda, Error **errp)
 static void hda_audio_init_micro(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
-    const struct desc_codec *desc = &micro_nomixemu;
+    const struct desc_codec *desc = &micro_mixemu;
 
     if (!a->mixer) {
-        desc = &micro_mixemu;
+        desc = &micro_nomixemu;
     }
 
     hda_audio_init(hda, desc, errp);
-- 
MST

[PULL 07/15] virtio-snd: check AUD_register_card return value

[Michael S. Tsirkin]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

AUD_register_card might fail. Even though errp was passed as an
argument, the call's return value was not checked for failure.

Fixes: Coverity CID 1523899
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Message-Id: <20231109162034.2108018-1-manos.pitsidianakis@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/audio/virtio-snd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 2fe966e311..83e97858e0 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1102,7 +1102,9 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    AUD_register_card("virtio-sound", &vsnd->card, errp);
+    if (!AUD_register_card("virtio-sound", &vsnd->card, errp)) {
+        return;
+    }
 
     /* set default params for all streams */
     default_params.features = 0;
-- 
MST

[PULL 08/15] virtio-sound: add realize() error cleanup path

[Michael S. Tsirkin]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

QEMU crashes on exit when a virtio-sound device has failed to
realise. Its vmstate field was not cleaned up properly with
qemu_del_vm_change_state_handler().

This patch changes the realize() order as

1. Validate the given configuration values (no resources allocated
   by us either on success or failure)
2. Try AUD_register_card() and return on failure (no resources allocated
   by us on failure)
3. Initialize vmstate, virtio device, heap allocations and stream
   parameters at once.
   If error occurs, goto error_cleanup label which calls
   virtio_snd_unrealize(). This cleans up all resources made in steps
   1-3.

Reported-by: Volker Rümelin <vr_qemu@t-online.de>
Fixes: 2880e676c000 ("Add virtio-sound device stub")
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Message-Id: <20231116072046.4002957-1-manos.pitsidianakis@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/audio/virtio-snd.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 83e97858e0..3c9f94e94a 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -36,6 +36,7 @@ static void virtio_snd_pcm_out_cb(void *data, int available);
 static void virtio_snd_process_cmdq(VirtIOSound *s);
 static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream);
 static void virtio_snd_pcm_in_cb(void *data, int available);
+static void virtio_snd_unrealize(DeviceState *dev);
 
 static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8)
                                   | BIT(VIRTIO_SND_PCM_FMT_U8)
@@ -1065,23 +1066,9 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
     virtio_snd_pcm_set_params default_params = { 0 };
     uint32_t status;
 
-    vsnd->pcm = NULL;
-    vsnd->vmstate =
-        qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
-
     trace_virtio_snd_realize(vsnd);
 
-    vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
-    vsnd->pcm->snd = vsnd;
-    vsnd->pcm->streams =
-        g_new0(VirtIOSoundPCMStream *, vsnd->snd_conf.streams);
-    vsnd->pcm->pcm_params =
-        g_new0(virtio_snd_pcm_set_params, vsnd->snd_conf.streams);
-
-    virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
-    virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
-
-    /* set number of jacks and streams */
+    /* check number of jacks and streams */
     if (vsnd->snd_conf.jacks > 8) {
         error_setg(errp,
                    "Invalid number of jacks: %"PRIu32,
@@ -1106,6 +1093,19 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    vsnd->vmstate =
+        qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
+
+    vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
+    vsnd->pcm->snd = vsnd;
+    vsnd->pcm->streams =
+        g_new0(VirtIOSoundPCMStream *, vsnd->snd_conf.streams);
+    vsnd->pcm->pcm_params =
+        g_new0(virtio_snd_pcm_set_params, vsnd->snd_conf.streams);
+
+    virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
+    virtio_add_feature(&vsnd->features, VIRTIO_F_VERSION_1);
+
     /* set default params for all streams */
     default_params.features = 0;
     default_params.buffer_bytes = cpu_to_le32(8192);
@@ -1130,16 +1130,21 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
             error_setg(errp,
                        "Can't initialize stream params, device responded with %s.",
                        print_code(status));
-            return;
+            goto error_cleanup;
         }
         status = virtio_snd_pcm_prepare(vsnd, i);
         if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
             error_setg(errp,
                        "Can't prepare streams, device responded with %s.",
                        print_code(status));
-            return;
+            goto error_cleanup;
         }
     }
+
+    return;
+
+error_cleanup:
+    virtio_snd_unrealize(dev);
 }
 
 static inline void return_tx_buffer(VirtIOSoundPCMStream *stream,
-- 
MST

[PULL 09/15] pcie_sriov: Remove g_new assertion

[Michael S. Tsirkin]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

g_new() aborts if the allocation fails so it returns NULL only if the
requested allocation size is zero. register_vfs() makes such an
allocation if NumVFs is zero so it should not assert that g_new()
returns a non-NULL value.

Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
Buglink: https://issues.redhat.com/browse/RHEL-17209
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-Id: <20231123075630.12057-1-akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Yanghang Liu<yanghliu@redhat.com>
Reviewed-by: Cédric Le Goater &lt;<a href="mailto:clg@redhat.com" target="_blank">clg@redhat.com</a>&gt;<br>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_sriov.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 5ef8950940..a1fe65f5d8 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
     num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
 
     dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
-    assert(dev->exp.sriov_pf.vf);
 
     trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
                              PCI_FUNC(dev->devfn), num_vfs);
-- 
MST

[PULL 10/15] hw/acpi/erst: Do not ignore Error* in realize handler

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

erst_realizefn() passes @errp to functions without checking for
failure.  If it runs into another failure, it trips error_setv()'s
assertion.

Use the ERRP_GUARD() macro and check *errp, as suggested in commit
ae7c80a7bd ("error: New macro ERRP_GUARD()").

Cc: qemu-stable@nongnu.org
Fixes: f7e26ffa59 ("ACPI ERST: support for ACPI ERST feature")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20231120130017.81286-1-philmd@linaro.org>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/erst.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
index 35007d8017..ba751dc60e 100644
--- a/hw/acpi/erst.c
+++ b/hw/acpi/erst.c
@@ -947,6 +947,7 @@ static const VMStateDescription erst_vmstate  = {
 
 static void erst_realizefn(PCIDevice *pci_dev, Error **errp)
 {
+    ERRP_GUARD();
     ERSTDeviceState *s = ACPIERST(pci_dev);
 
     trace_acpi_erst_realizefn_in();
@@ -964,9 +965,15 @@ static void erst_realizefn(PCIDevice *pci_dev, Error **errp)
 
     /* HostMemoryBackend size will be multiple of PAGE_SIZE */
     s->storage_size = object_property_get_int(OBJECT(s->hostmem), "size", errp);
+    if (*errp) {
+        return;
+    }
 
     /* Initialize backend storage and record_count */
     check_erst_backend_storage(s, errp);
+    if (*errp) {
+        return;
+    }
 
     /* BAR 0: Programming registers */
     memory_region_init_io(&s->iomem_mr, OBJECT(pci_dev), &erst_reg_ops, s,
@@ -977,6 +984,9 @@ static void erst_realizefn(PCIDevice *pci_dev, Error **errp)
     memory_region_init_ram(&s->exchange_mr, OBJECT(pci_dev),
                             "erst.exchange",
                             le32_to_cpu(s->header->record_size), errp);
+    if (*errp) {
+        return;
+    }
     pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
                         &s->exchange_mr);
 
-- 
MST

[PULL 11/15] hw/i386: fix short-circuit logic with non-optimizing builds

[Michael S. Tsirkin]

From: Daniel Hoffman <dhoff749@gmail.com>

`kvm_enabled()` is compiled down to `0` and short-circuit logic is
used to remove references to undefined symbols at the compile stage.
Some build configurations with some compilers don't attempt to
simplify this logic down in some cases (the pattern appears to be
that the literal false must be the first term) and this was causing
some builds to emit references to undefined symbols.

An example of such a configuration is clang 16.0.6 with the following
configure: ./configure --enable-debug --without-default-features
--target-list=x86_64-softmmu --enable-tcg-interpreter

Signed-off-by: Daniel Hoffman <dhoff749@gmail.com>
Message-Id: <20231119203116.3027230-1-dhoff749@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/x86.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b3d054889b..2b6291ad8d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
     /*
      * Can we support APIC ID 255 or higher?  With KVM, that requires
      * both in-kernel lapic and X2APIC userspace API.
+     *
+     * kvm_enabled() must go first to ensure that kvm_* references are
+     * not emitted for the linker to consume (kvm_enabled() is
+     * a literal `0` in configurations where kvm_* aren't defined)
      */
-    if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
+    if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
         (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
         error_report("current -smp configuration requires kernel "
                      "irqchip and X2APIC API support.");
@@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
     }
     cpu->thread_id = topo_ids.smt_id;
 
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
-        kvm_enabled() && !kvm_hv_vpindex_settable()) {
+    /*
+    * kvm_enabled() must go first to ensure that kvm_* references are
+    * not emitted for the linker to consume (kvm_enabled() is
+    * a literal `0` in configurations where kvm_* aren't defined)
+    */
+    if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
+        !kvm_hv_vpindex_settable()) {
         error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
         return;
     }
-- 
MST

[PULL 12/15] virtio-iommu: Remove useless !sdev check in virtio_iommu_probe()

[Michael S. Tsirkin]

From: Eric Auger <eric.auger@redhat.com>

The code already checks iommu_mr is not NULL so there is no
need to check container_of() is not NULL. Remove the check.

Fixes: CID 1523901
Fixes: 09b4c3d6a2 ("virtio-iommu: Record whether a probe request has
been issued")
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Coverity (CID 1523901)
Message-Id: <20231109170715.259520-1-eric.auger@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-iommu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 89fb5767d1..9d463efc52 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -698,9 +698,6 @@ static int virtio_iommu_probe(VirtIOIOMMU *s,
     }
 
     sdev = container_of(iommu_mr, IOMMUDevice, iommu_mr);
-    if (!sdev) {
-        return -EINVAL;
-    }
 
     count = virtio_iommu_fill_resv_mem_prop(sdev, ep_id, buf, free);
     if (count < 0) {
-- 
MST

[PULL 13/15] msix: unset PCIDevice::msix_vector_poll_notifier in rollback

[Michael S. Tsirkin]

From: Robert Hoo <robert.hoo.linux@gmail.com>

In the rollback in msix_set_vector_notifiers(), original patch forgot to
undo msix_vector_poll_notifier pointer.

Fixes: bbef882cc193 ("msi: add API to get notified about pending bit poll")
Signed-off-by: Robert Hoo <robert.hoo.linux@gmail.com>
Message-Id: <20231113081349.1307-1-robert.hoo.linux@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/msix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index ab8869d9d0..cd817f4ca8 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -648,6 +648,7 @@ undo:
     }
     dev->msix_vector_use_notifier = NULL;
     dev->msix_vector_release_notifier = NULL;
+    dev->msix_vector_poll_notifier = NULL;
     return ret;
 }
 
-- 
MST

[PULL 14/15] vhost-user: fix the reconnect error

[Michael S. Tsirkin]

From: Li Feng <fengli@smartx.com>

If the error occurs in vhost_dev_init, the value of s->connected is set to true
in advance, and there is no chance to enter this function execution again
in the future.

Signed-off-by: Li Feng <fengli@smartx.com>
Message-Id: <20231123055431.217792-2-fengli@smartx.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/vhost-user-blk.c   | 8 +++-----
 hw/scsi/vhost-user-scsi.c   | 3 ++-
 hw/virtio/vhost-user-gpio.c | 3 ++-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 818b833108..2863d80d15 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -326,7 +326,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
     if (s->connected) {
         return 0;
     }
-    s->connected = true;
 
     s->dev.num_queues = s->num_queues;
     s->dev.nvqs = s->num_queues;
@@ -343,15 +342,14 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
         return ret;
     }
 
+    s->connected = true;
+
     /* restore vhost state */
     if (virtio_device_started(vdev, vdev->status)) {
         ret = vhost_user_blk_start(vdev, errp);
-        if (ret < 0) {
-            return ret;
-        }
     }
 
-    return 0;
+    return ret;
 }
 
 static void vhost_user_blk_disconnect(DeviceState *dev)
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 4486500cac..2060f9f94b 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -147,7 +147,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
     if (s->connected) {
         return 0;
     }
-    s->connected = true;
 
     vsc->dev.num_queues = vs->conf.num_queues;
     vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -161,6 +160,8 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
         return ret;
     }
 
+    s->connected = true;
+
     /* restore vhost state */
     if (virtio_device_started(vdev, vdev->status)) {
         ret = vhost_user_scsi_start(s, errp);
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index aff2d7eff6..a83437a5da 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -229,7 +229,6 @@ static int vu_gpio_connect(DeviceState *dev, Error **errp)
     if (gpio->connected) {
         return 0;
     }
-    gpio->connected = true;
 
     vhost_dev_set_config_notifier(vhost_dev, &gpio_ops);
     gpio->vhost_user.supports_config = true;
@@ -243,6 +242,8 @@ static int vu_gpio_connect(DeviceState *dev, Error **errp)
         return ret;
     }
 
+    gpio->connected = true;
+
     /* restore vhost state */
     if (virtio_device_started(vdev, vdev->status)) {
         vu_gpio_start(vdev);
-- 
MST

[PULL 15/15] vhost-user-scsi: free the inflight area when reset

[Michael S. Tsirkin]

From: Li Feng <fengli@smartx.com>

Keep it the same to vhost-user-blk.
At the same time, fix the vhost_reset_device.

Signed-off-by: Li Feng <fengli@smartx.com>
Message-Id: <20231123055431.217792-3-fengli@smartx.com>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/scsi/vhost-user-scsi.c | 16 ++++++++++++++++
 hw/virtio/virtio.c        |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 2060f9f94b..780f10559d 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -360,6 +360,20 @@ static Property vhost_user_scsi_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void vhost_user_scsi_reset(VirtIODevice *vdev)
+{
+    VHostUserSCSI *s = VHOST_USER_SCSI(vdev);
+    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+
+    vhost_dev_free_inflight(vsc->inflight);
+}
+
+static struct vhost_dev *vhost_user_scsi_get_vhost(VirtIODevice *vdev)
+{
+    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
+    return &vsc->dev;
+}
+
 static const VMStateDescription vmstate_vhost_scsi = {
     .name = "virtio-scsi",
     .minimum_version_id = 1,
@@ -385,6 +399,8 @@ static void vhost_user_scsi_class_init(ObjectClass *klass, void *data)
     vdc->set_config = vhost_scsi_common_set_config;
     vdc->set_status = vhost_user_scsi_set_status;
     fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path;
+    vdc->reset = vhost_user_scsi_reset;
+    vdc->get_vhost = vhost_user_scsi_get_vhost;
 }
 
 static void vhost_user_scsi_instance_init(Object *obj)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e5105571cf..3a160f86ed 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2137,7 +2137,7 @@ void virtio_reset(void *opaque)
         vdev->device_endian = virtio_default_endian();
     }
 
-    if (vdev->vhost_started) {
+    if (vdev->vhost_started && k->get_vhost) {
         vhost_reset_device(k->get_vhost(vdev));
     }
 
-- 
MST

Re: [PULL 00/15] virtio,pc,pci: fixes

[Michael S. Tsirkin]

On Fri, Dec 01, 2023 at 12:15:11PM -0500, Michael S. Tsirkin wrote:
> We have a reported regression because of the switch to
> smbios 3.0, and maybe need to revert that, but Igor asked
> for a bit more time to investigate.
> 
> The following changes since commit abf635ddfe3242df907f58967f3c1e6763bbca2d:
> 
>   Update version for v8.2.0-rc2 release (2023-11-28 16:31:16 -0500)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> 
> for you to fetch changes up to d4ad718733df4c87b9133bb57ea77b0040923541:

95e1019a4a9b4b6e2caeb3fd392525e522a747db now - teaked some tags

>   vhost-user-scsi: free the inflight area when reset (2023-12-01 12:01:45 -0500)
> 
> ----------------------------------------------------------------
> virtio,pc,pci: fixes
> 
> Fixes all over the place
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ----------------------------------------------------------------



> Akihiko Odaki (1):
>       pcie_sriov: Remove g_new assertion
> 
> Ani Sinha (1):
>       tests/acpi/bios-tables-test: do not write new blobs unless there are changes
> 
> Daniel Hoffman (1):
>       hw/i386: fix short-circuit logic with non-optimizing builds
> 
> Eric Auger (1):
>       virtio-iommu: Remove useless !sdev check in virtio_iommu_probe()
> 
> Li Feng (2):
>       vhost-user: fix the reconnect error
>       vhost-user-scsi: free the inflight area when reset
> 
> Manos Pitsidianakis (2):
>       virtio-snd: check AUD_register_card return value
>       virtio-sound: add realize() error cleanup path
> 
> Michael S. Tsirkin (2):
>       osdep: add getloadavg
>       netdev: set timeout depending on loadavg
> 
> Philippe Mathieu-Daudé (1):
>       hw/acpi/erst: Do not ignore Error* in realize handler
> 
> Robert Hoo (1):
>       msix: unset PCIDevice::msix_vector_poll_notifier in rollback
> 
> Volker Rümelin (3):
>       hw/audio/virtio-snd-pci: fix the PCI class code
>       hw/audio/hda-codec: fix multiplication overflow
>       hw/audio/hda-codec: reenable the audio mixer
> 
>  include/qemu/osdep.h           | 10 ++++++++++
>  hw/acpi/erst.c                 | 10 ++++++++++
>  hw/audio/hda-codec.c           | 41 ++++++++++++++++++++++++++--------------
>  hw/audio/virtio-snd-pci.c      |  2 ++
>  hw/audio/virtio-snd.c          | 43 ++++++++++++++++++++++++------------------
>  hw/block/vhost-user-blk.c      |  8 +++-----
>  hw/i386/x86.c                  | 15 ++++++++++++---
>  hw/pci/msix.c                  |  1 +
>  hw/pci/pcie_sriov.c            |  1 -
>  hw/scsi/vhost-user-scsi.c      | 19 ++++++++++++++++++-
>  hw/virtio/vhost-user-gpio.c    |  3 ++-
>  hw/virtio/virtio-iommu.c       |  3 ---
>  hw/virtio/virtio.c             |  2 +-
>  tests/qtest/bios-tables-test.c | 14 +++++++++++++-
>  tests/qtest/netdev-socket.c    | 28 ++++++++++++++++++++++++++-
>  meson.build                    |  1 +
>  16 files changed, 152 insertions(+), 49 deletions(-)
> 

Re: [PULL 00/15] virtio,pc,pci: fixes

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]