Series comparison

It's been quiet on the arm front this week, so all I have is
these coverity fixes I posted a while back...

-- PMM

The following changes since commit 853546f8128476eefb701d4a55b2781bb3a46faa:

Merge tag 'pull-loongarch-20240322' of https://gitlab.com/gaosong/qemu into staging (2024-03-22 10:59:57 +0000)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20240325

for you to fetch changes up to 55c79639d553c1b7a82b4cde781ad5f316f45b0e:

tests/qtest/libqtest.c: Check for g_setenv() failure (2024-03-25 10:41:01 +0000)

----------------------------------------------------------------
target-arm queue:
 * Fixes for seven minor coverity issues

----------------------------------------------------------------
Peter Maydell (7):
      tests/qtest/npcm7xx_emc_test: Don't leak cmd_line
      tests/unit/socket-helpers: Don't close(-1)
      net/af-xdp.c: Don't leak sock_fds array in net_init_af_xdp()
      hw/misc/pca9554: Correct error check bounds in get/set pin functions
      hw/nvram/mac_nvram: Report failure to write data
      tests/unit/test-throttle: Avoid unintended integer division
      tests/qtest/libqtest.c: Check for g_setenv() failure

In test_rx() and test_tx() we allocate a GString *cmd_line
but never free it. This is pretty harmless in a test case, but
Coverity spotted it.

Resolves: Coverity CID 1507122
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-2-peter.maydell@linaro.org
---
 tests/qtest/npcm7xx_emc-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/npcm7xx_emc-test.c
+++ b/tests/qtest/npcm7xx_emc-test.c
@@ -XXX,XX +XXX,XX @@ static void emc_test_ptle(QTestState *qts, const EMCModule *mod, int fd)
 static void test_tx(gconstpointer test_data)
 {
     const TestData *td = test_data;
-    GString *cmd_line = g_string_new("-machine quanta-gsj");
+    g_autoptr(GString) cmd_line = g_string_new("-machine quanta-gsj");
     int *test_sockets = packet_test_init(emc_module_index(td->module),
                                          cmd_line);
     QTestState *qts = qtest_init(cmd_line->str);
@@ -XXX,XX +XXX,XX @@ static void test_tx(gconstpointer test_data)
 static void test_rx(gconstpointer test_data)
 {
     const TestData *td = test_data;
-    GString *cmd_line = g_string_new("-machine quanta-gsj");
+    g_autoptr(GString) cmd_line = g_string_new("-machine quanta-gsj");
     int *test_sockets = packet_test_init(emc_module_index(td->module),
                                          cmd_line);
     QTestState *qts = qtest_init(cmd_line->str);
-- 
2.34.1

In socket_check_afunix_support() we call socket(PF_UNIX, SOCK_STREAM, 0)
to see if it works, but we call close() on the result whether it
worked or not. Only close the fd if the socket() call succeeded.
Spotted by Coverity.

Resolves: Coverity CID 1497481

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-3-peter.maydell@linaro.org
---
 tests/unit/socket-helpers.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/unit/socket-helpers.c b/tests/unit/socket-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/socket-helpers.c
+++ b/tests/unit/socket-helpers.c
@@ -XXX,XX +XXX,XX @@ void socket_check_afunix_support(bool *has_afunix)
     int fd;
 
     fd = socket(PF_UNIX, SOCK_STREAM, 0);
-    close(fd);
 
 #ifdef _WIN32
     *has_afunix = (fd != (int)INVALID_SOCKET);
@@ -XXX,XX +XXX,XX @@ void socket_check_afunix_support(bool *has_afunix)
     *has_afunix = (fd >= 0);
 #endif
 
+    if (*has_afunix) {
+        close(fd);
+    }
     return;
 }
-- 
2.34.1

In net_init_af_xdp() we parse the arguments and allocate
a buffer of ints into sock_fds. However, although we
free this in the error exit path, we don't ever free it
in the successful return path. Coverity spots this leak.

Switch to g_autofree so we don't need to manually free the
array.

Resolves: Coverity CID 1534906
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-4-peter.maydell@linaro.org
---
 net/af-xdp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/af-xdp.c b/net/af-xdp.c
index XXXXXXX..XXXXXXX 100644
--- a/net/af-xdp.c
+++ b/net/af-xdp.c
@@ -XXX,XX +XXX,XX @@ int net_init_af_xdp(const Netdev *netdev,
     NetClientState *nc, *nc0 = NULL;
     unsigned int ifindex;
     uint32_t prog_id = 0;
-    int *sock_fds = NULL;
+    g_autofree int *sock_fds = NULL;
     int64_t i, queues;
     Error *err = NULL;
     AFXDPState *s;
@@ -XXX,XX +XXX,XX @@ int net_init_af_xdp(const Netdev *netdev,
     return 0;
 
 err:
-    g_free(sock_fds);
     if (nc0) {
         qemu_del_net_client(nc0);
     }
-- 
2.34.1

In pca9554_get_pin() and pca9554_set_pin(), we try to detect an
incorrect pin value, but we get the condition wrong, using ">"
when ">=" was intended.

This has no actual effect, because in pca9554_initfn() we
use the correct test when creating the properties and so
we'll never be called with an out of range value. However,
Coverity complains about the mismatch between the check and
the later use of the pin value in a shift operation.

Use the correct condition.

Resolves: Coverity CID 1534917
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-5-peter.maydell@linaro.org
---
 hw/misc/pca9554.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/misc/pca9554.c
+++ b/hw/misc/pca9554.c
@@ -XXX,XX +XXX,XX @@ static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
+    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
         error_setg(errp, "%s invalid pin %s", __func__, name);
         return;
     }
@@ -XXX,XX +XXX,XX @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
+    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
         error_setg(errp, "%s invalid pin %s", __func__, name);
         return;
     }
-- 
2.34.1

There's no way for the macio_nvram device to report failure to write
data, but we can at least report it to the user with error_report()
as we do in other devices like xlnx-efuse.

Spotted by Coverity.

Resolves: Coverity CID 1507628
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-6-peter.maydell@linaro.org
---
 hw/nvram/mac_nvram.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/nvram/mac_nvram.c
+++ b/hw/nvram/mac_nvram.c
@@ -XXX,XX +XXX,XX @@ static void macio_nvram_writeb(void *opaque, hwaddr addr,
     trace_macio_nvram_write(addr, value);
     s->data[addr] = value;
     if (s->blk) {
-        blk_pwrite(s->blk, addr, 1, &s->data[addr], 0);
+        if (blk_pwrite(s->blk, addr, 1, &s->data[addr], 0) < 0) {
+            error_report("%s: write of NVRAM data to backing store failed",
+                         blk_name(s->blk));
+        }
     }
 }
 
-- 
2.34.1

In test_compute_wait() we do
 double units = bkt.max / 10;
which does an integer division and then assigns it to a double variable,
and similarly later on in the expression for an assertion.

Use 10.0 so that we do a floating point division and calculate the
exact value, rather than doing an integer division.

Spotted by Coverity.

Resolves: Coverity CID 1432564
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-7-peter.maydell@linaro.org
---
 tests/unit/test-throttle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -XXX,XX +XXX,XX @@ static void test_compute_wait(void)
     bkt.avg = 10;
     bkt.max = 200;
     for (i = 0; i < 22; i++) {
-        double units = bkt.max / 10;
+        double units = bkt.max / 10.0;
         bkt.level += units;
         bkt.burst_level += units;
         throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 10);
         wait = throttle_compute_wait(&bkt);
         g_assert(double_cmp(bkt.burst_level, 0));
-        g_assert(double_cmp(bkt.level, (i + 1) * (bkt.max - bkt.avg) / 10));
+        g_assert(double_cmp(bkt.level, (i + 1) * (bkt.max - bkt.avg) / 10.0));
         /* We can do bursts for the 2 seconds we have configured in
          * burst_length. We have 100 extra milliseconds of burst
          * because bkt.level has been leaking during this time.
-- 
2.34.1

Coverity points out that g_setenv() can fail and we don't
check for this in qtest_inproc_init(). In practice this will
only fail if a memory allocation failed in setenv() or if
the caller passed an invalid architecture name (e.g. one
with an '=' in it), so rather than requiring the callsite
to check for failure, make g_setenv() failure fatal here,
similarly to what we did in commit aca68d95c515.

Resolves: Coverity CID 1497485
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-8-peter.maydell@linaro.org
---
 tests/qtest/libqtest.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -XXX,XX +XXX,XX @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
      * way, qtest_get_arch works for inproc qtest.
      */
     gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
-    g_setenv("QTEST_QEMU_BINARY", bin_path, 0);
+    if (!g_setenv("QTEST_QEMU_BINARY", bin_path, 0)) {
+        fprintf(stderr,
+                "Could not set environment variable QTEST_QEMU_BINARY\n");
+        exit(1);
+    }
     g_free(bin_path);
 
     return qts;
-- 
2.34.1

v3->v4: Windows headers define an INT type which clashed
with an enum value name in arm_gicv3_its.c...

The following changes since commit eae587e8e3694b1aceab23239493fb4c7e1a80f5:

Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-09-13' into staging (2021-09-13 11:00:30 +0100)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210913-3

for you to fetch changes up to 28e987a7e7edaa3ca7feeac65edca26145df8814:

hw/arm/mps2.c: Mark internal-only I2C buses as 'full' (2021-09-13 21:01:08 +0100)

----------------------------------------------------------------
target-arm queue:
 * mark MPS2/MPS3 board-internal i2c buses as 'full' so that command
   line user-created devices are not plugged into them
 * Take an exception if PSTATE.IL is set
 * Support an emulated ITS in the virt board
 * Add support for kudo-bmc board
 * Probe for KVM_CAP_ARM_VM_IPA_SIZE when creating scratch VM
 * cadence_uart: Fix clock handling issues that prevented
   u-boot from running

----------------------------------------------------------------
Bin Meng (6):
      hw/misc: zynq_slcr: Correctly compute output clocks in the reset exit phase
      hw/char: cadence_uart: Disable transmit when input clock is disabled
      hw/char: cadence_uart: Move clock/reset check to uart_can_receive()
      hw/char: cadence_uart: Convert to memop_with_attrs() ops
      hw/char: cadence_uart: Ignore access when unclocked or in reset for uart_{read, write}()
      hw/char: cadence_uart: Log a guest error when device is unclocked or in reset

Chris Rauer (1):
      hw/arm: Add support for kudo-bmc board.

Marc Zyngier (1):
      hw/arm/virt: KVM: Probe for KVM_CAP_ARM_VM_IPA_SIZE when creating scratch VM

Peter Maydell (5):
      target/arm: Take an exception if PSTATE.IL is set
      qdev: Support marking individual buses as 'full'
      hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn
      hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full'
      hw/arm/mps2.c: Mark internal-only I2C buses as 'full'

Richard Henderson (1):
      target/arm: Merge disas_a64_insn into aarch64_tr_translate_insn

Shashi Mallela (9):
      hw/intc: GICv3 ITS initial framework
      hw/intc: GICv3 ITS register definitions added
      hw/intc: GICv3 ITS command queue framework
      hw/intc: GICv3 ITS Command processing
      hw/intc: GICv3 ITS Feature enablement
      hw/intc: GICv3 redistributor ITS processing
      tests/data/acpi/virt: Add IORT files for ITS
      hw/arm/virt: add ITS support in virt GIC
      tests/data/acpi/virt: Update IORT files for ITS