* [PULL 01/12] hw/scsi/lsi53c895a: add timer to scripts processing
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 02/12] system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add() Paolo Bonzini
` (11 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Sven Schnelle, Peter Maydell
From: Sven Schnelle <svens@stackframe.org>
HP-UX 10.20 seems to make the lsi53c895a spinning on a memory location
under certain circumstances. As the SCSI controller and CPU are not
running at the same time this loop will never finish. After some
time, the check loop interrupts with a unexpected device disconnect.
This works, but is slow because the kernel resets the scsi controller.
Instead of signaling UDC, start a timer and exit the loop. Until the
timer fires, the CPU can process instructions which might changes the
memory location.
The limit of instructions is also reduced because scripts running on
the SCSI processor are usually very short. This keeps the time until
the loop is exit short.
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Sven Schnelle <svens@stackframe.org>
Message-ID: <20240229204407.1699260-1-svens@stackframe.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/lsi53c895a.c | 43 +++++++++++++++++++++++++++++++++----------
hw/scsi/trace-events | 2 ++
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d607a5f9fb1..4ff94703816 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -188,7 +188,7 @@ static const char *names[] = {
#define LSI_TAG_VALID (1 << 16)
/* Maximum instructions to process. */
-#define LSI_MAX_INSN 10000
+#define LSI_MAX_INSN 100
typedef struct lsi_request {
SCSIRequest *req;
@@ -205,6 +205,7 @@ enum {
LSI_WAIT_RESELECT, /* Wait Reselect instruction has been issued */
LSI_DMA_SCRIPTS, /* processing DMA from lsi_execute_script */
LSI_DMA_IN_PROGRESS, /* DMA operation is in progress */
+ LSI_WAIT_SCRIPTS, /* SCRIPTS stopped because of instruction count limit */
};
enum {
@@ -224,6 +225,7 @@ struct LSIState {
MemoryRegion ram_io;
MemoryRegion io_io;
AddressSpace pci_io_as;
+ QEMUTimer *scripts_timer;
int carry; /* ??? Should this be an a visible register somewhere? */
int status;
@@ -415,6 +417,7 @@ static void lsi_soft_reset(LSIState *s)
s->sbr = 0;
assert(QTAILQ_EMPTY(&s->queue));
assert(!s->current);
+ timer_del(s->scripts_timer);
}
static int lsi_dma_40bit(LSIState *s)
@@ -1127,6 +1130,12 @@ static void lsi_wait_reselect(LSIState *s)
}
}
+static void lsi_scripts_timer_start(LSIState *s)
+{
+ trace_lsi_scripts_timer_start();
+ timer_mod(s->scripts_timer, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + 500);
+}
+
static void lsi_execute_script(LSIState *s)
{
PCIDevice *pci_dev = PCI_DEVICE(s);
@@ -1136,6 +1145,11 @@ static void lsi_execute_script(LSIState *s)
int insn_processed = 0;
static int reentrancy_level;
+ if (s->waiting == LSI_WAIT_SCRIPTS) {
+ timer_del(s->scripts_timer);
+ s->waiting = LSI_NOWAIT;
+ }
+
reentrancy_level++;
s->istat1 |= LSI_ISTAT1_SRUN;
@@ -1143,8 +1157,8 @@ again:
/*
* Some windows drivers make the device spin waiting for a memory location
* to change. If we have executed more than LSI_MAX_INSN instructions then
- * assume this is the case and force an unexpected device disconnect. This
- * is apparently sufficient to beat the drivers into submission.
+ * assume this is the case and start a timer. Until the timer fires, the
+ * host CPU has a chance to run and change the memory location.
*
* Another issue (CVE-2023-0330) can occur if the script is programmed to
* trigger itself again and again. Avoid this problem by stopping after
@@ -1152,13 +1166,8 @@ again:
* which should be enough for all valid use cases).
*/
if (++insn_processed > LSI_MAX_INSN || reentrancy_level > 8) {
- if (!(s->sien0 & LSI_SIST0_UDC)) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "lsi_scsi: inf. loop with UDC masked");
- }
- lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
- lsi_disconnect(s);
- trace_lsi_execute_script_stop();
+ s->waiting = LSI_WAIT_SCRIPTS;
+ lsi_scripts_timer_start(s);
reentrancy_level--;
return;
}
@@ -2197,6 +2206,9 @@ static int lsi_post_load(void *opaque, int version_id)
return -EINVAL;
}
+ if (s->waiting == LSI_WAIT_SCRIPTS) {
+ lsi_scripts_timer_start(s);
+ }
return 0;
}
@@ -2294,6 +2306,15 @@ static const struct SCSIBusInfo lsi_scsi_info = {
.cancel = lsi_request_cancelled
};
+static void scripts_timer_cb(void *opaque)
+{
+ LSIState *s = opaque;
+
+ trace_lsi_scripts_timer_triggered();
+ s->waiting = LSI_NOWAIT;
+ lsi_execute_script(s);
+}
+
static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
{
LSIState *s = LSI53C895A(dev);
@@ -2313,6 +2334,7 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
"lsi-ram", 0x2000);
memory_region_init_io(&s->io_io, OBJECT(s), &lsi_io_ops, s,
"lsi-io", 256);
+ s->scripts_timer = timer_new_us(QEMU_CLOCK_VIRTUAL, scripts_timer_cb, s);
/*
* Since we use the address-space API to interact with ram_io, disable the
@@ -2337,6 +2359,7 @@ static void lsi_scsi_exit(PCIDevice *dev)
LSIState *s = LSI53C895A(dev);
address_space_destroy(&s->pci_io_as);
+ timer_del(s->scripts_timer);
}
static void lsi_class_init(ObjectClass *klass, void *data)
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index d72f741ed85..f0f2a98c2ee 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -302,6 +302,8 @@ lsi_execute_script_stop(void) "SCRIPTS execution stopped"
lsi_awoken(void) "Woken by SIGP"
lsi_reg_read(const char *name, int offset, uint8_t ret) "Read reg %s 0x%x = 0x%02x"
lsi_reg_write(const char *name, int offset, uint8_t val) "Write reg %s 0x%x = 0x%02x"
+lsi_scripts_timer_triggered(void) "SCRIPTS timer triggered"
+lsi_scripts_timer_start(void) "SCRIPTS timer started"
# virtio-scsi.c
virtio_scsi_cmd_req(int lun, uint32_t tag, uint8_t cmd) "virtio_scsi_cmd_req lun=%u tag=0x%x cmd=0x%x"
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 02/12] system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add()
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
2024-03-08 14:55 ` [PULL 01/12] hw/scsi/lsi53c895a: add timer to scripts processing Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 03/12] meson: Remove --warn-common ldflag Paolo Bonzini
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitrii Gavrilov, qemu-stable
From: Dmitrii Gavrilov <ds-gavr@yandex-team.ru>
Original goal of addition of drain_call_rcu to qmp_device_add was to cover
the failure case of qdev_device_add. It seems call of drain_call_rcu was
misplaced in 7bed89958bfbf40df what led to waiting for pending RCU callbacks
under happy path too. What led to overall performance degradation of
qmp_device_add.
In this patch call of drain_call_rcu moved under handling of failure of
qdev_device_add.
Signed-off-by: Dmitrii Gavrilov <ds-gavr@yandex-team.ru>
Message-ID: <20231103105602.90475-1-ds-gavr@yandex-team.ru>
Fixes: 7bed89958bf ("device_core: use drain_call_rcu in in qmp_device_add", 2020-10-12)
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
system/qdev-monitor.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index a13db763e5d..874d65191ce 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -858,19 +858,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
return;
}
dev = qdev_device_add(opts, errp);
-
- /*
- * Drain all pending RCU callbacks. This is done because
- * some bus related operations can delay a device removal
- * (in this case this can happen if device is added and then
- * removed due to a configuration error)
- * to a RCU callback, but user might expect that this interface
- * will finish its job completely once qmp command returns result
- * to the user
- */
- drain_call_rcu();
-
if (!dev) {
+ /*
+ * Drain all pending RCU callbacks. This is done because
+ * some bus related operations can delay a device removal
+ * (in this case this can happen if device is added and then
+ * removed due to a configuration error)
+ * to a RCU callback, but user might expect that this interface
+ * will finish its job completely once qmp command returns result
+ * to the user
+ */
+ drain_call_rcu();
+
qemu_opts_del(opts);
return;
}
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 03/12] meson: Remove --warn-common ldflag
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
2024-03-08 14:55 ` [PULL 01/12] hw/scsi/lsi53c895a: add timer to scripts processing Paolo Bonzini
2024-03-08 14:55 ` [PULL 02/12] system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add() Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 04/12] hw/scsi/lsi53c895a: stop script on phase mismatch Paolo Bonzini
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
--warn-common ldflag causes warnings for multiple definitions of
___asan_globals_registered when enabling AddressSanitizer with clang.
The warning is somewhat obsolete so just remove it.
The common block is used to allow duplicate definitions of uninitialized
global variables. In the past, GCC and clang used to place such
variables in a common block by default, which prevented programmers for
noticing accidental duplicate definitions. Commit 49237acdb725 ("Enable
ld flag --warn-common") added --warn-common ldflag so that ld warns in
such a case.
Today, both of GCC and clang don't use common blocks by default[1][2] so
any remaining use of common blocks should be intentional. Remove
--warn-common ldflag to suppress warnings for intentional use of
common blocks.
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85678
[2]: https://reviews.llvm.org/D75056
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-ID: <20240304-common-v1-1-1a2005d1f350@daynix.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
meson.build | 5 -----
1 file changed, 5 deletions(-)
diff --git a/meson.build b/meson.build
index c59ca496f2d..f9dbe7634e5 100644
--- a/meson.build
+++ b/meson.build
@@ -476,11 +476,6 @@ if host_os == 'windows'
qemu_ldflags += cc.get_supported_link_arguments('-Wl,--dynamicbase', '-Wl,--high-entropy-va')
endif
-# Exclude --warn-common with TSan to suppress warnings from the TSan libraries.
-if host_os != 'sunos' and not get_option('tsan')
- qemu_ldflags += cc.get_supported_link_arguments('-Wl,--warn-common')
-endif
-
if get_option('fuzzing')
# Specify a filter to only instrument code that is directly related to
# virtual-devices.
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 04/12] hw/scsi/lsi53c895a: stop script on phase mismatch
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (2 preceding siblings ...)
2024-03-08 14:55 ` [PULL 03/12] meson: Remove --warn-common ldflag Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 05/12] hw/intc/apic: fix memory leak Paolo Bonzini
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Sven Schnelle, Helge Deller
From: Sven Schnelle <svens@stackframe.org>
Netbsd isn't happy with qemu lsi53c895a emulation:
cd0(esiop0:0:2:0): command with tag id 0 reset
esiop0: autoconfiguration error: phase mismatch without command
esiop0: autoconfiguration error: unhandled scsi interrupt, sist=0x80 sstat1=0x0 DSA=0x23a64b1 DSP=0x50
This is because lsi_bad_phase() triggers a phase mismatch, which
stops SCRIPT processing. However, after returning to
lsi_command_complete(), SCRIPT is restarted with lsi_resume_script().
Fix this by adding a return value to lsi_bad_phase(), and only resume
script processing when lsi_bad_phase() didn't trigger a host interrupt.
Signed-off-by: Sven Schnelle <svens@stackframe.org>
Tested-by: Helge Deller <deller@gmx.de>
Message-ID: <20240302214453.2071388-1-svens@stackframe.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/lsi53c895a.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 4ff94703816..59b88aff3fb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -573,8 +573,9 @@ static inline void lsi_set_phase(LSIState *s, int phase)
s->sstat1 = (s->sstat1 & ~PHASE_MASK) | phase;
}
-static void lsi_bad_phase(LSIState *s, int out, int new_phase)
+static int lsi_bad_phase(LSIState *s, int out, int new_phase)
{
+ int ret = 0;
/* Trigger a phase mismatch. */
if (s->ccntl0 & LSI_CCNTL0_ENPMJ) {
if ((s->ccntl0 & LSI_CCNTL0_PMJCTL)) {
@@ -587,8 +588,10 @@ static void lsi_bad_phase(LSIState *s, int out, int new_phase)
trace_lsi_bad_phase_interrupt();
lsi_script_scsi_interrupt(s, LSI_SIST0_MA, 0);
lsi_stop_script(s);
+ ret = 1;
}
lsi_set_phase(s, new_phase);
+ return ret;
}
@@ -792,7 +795,7 @@ static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
static void lsi_command_complete(SCSIRequest *req, size_t resid)
{
LSIState *s = LSI53C895A(req->bus->qbus.parent);
- int out;
+ int out, stop = 0;
out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
trace_lsi_command_complete(req->status);
@@ -800,7 +803,10 @@ static void lsi_command_complete(SCSIRequest *req, size_t resid)
s->command_complete = 2;
if (s->waiting && s->dbc != 0) {
/* Raise phase mismatch for short transfers. */
- lsi_bad_phase(s, out, PHASE_ST);
+ stop = lsi_bad_phase(s, out, PHASE_ST);
+ if (stop) {
+ s->waiting = 0;
+ }
} else {
lsi_set_phase(s, PHASE_ST);
}
@@ -810,7 +816,9 @@ static void lsi_command_complete(SCSIRequest *req, size_t resid)
lsi_request_free(s, s->current);
scsi_req_unref(req);
}
- lsi_resume_script(s);
+ if (!stop) {
+ lsi_resume_script(s);
+ }
}
/* Callback to indicate that the SCSI layer has completed a transfer. */
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 05/12] hw/intc/apic: fix memory leak
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (3 preceding siblings ...)
2024-03-08 14:55 ` [PULL 04/12] hw/scsi/lsi53c895a: stop script on phase mismatch Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 06/12] oslib-posix: fix memory leak in touch_all_pages Paolo Bonzini
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Bui Quang Minh
deliver_bitmask is allocated on the heap in apic_deliver(), but there
are many paths in the function that return before the corresponding
g_free() is reached. Fix this by switching to g_autofree and, while at
it, also switch to g_new. Do the same in apic_deliver_irq() as well
for consistency.
Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Bui Quang Minh <minhquangbui99@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/intc/apic.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 1d887d66b86..4186c57b34c 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -291,14 +291,13 @@ static void apic_deliver_irq(uint32_t dest, uint8_t dest_mode,
uint8_t delivery_mode, uint8_t vector_num,
uint8_t trigger_mode)
{
- uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
+ g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
trigger_mode);
apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
- g_free(deliver_bitmask);
}
bool is_x2apic_mode(DeviceState *dev)
@@ -662,7 +661,7 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
APICCommonState *s = APIC(dev);
APICCommonState *apic_iter;
uint32_t deliver_bitmask_size = max_apic_words * sizeof(uint32_t);
- uint32_t *deliver_bitmask = g_malloc(deliver_bitmask_size);
+ g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
uint32_t current_apic_id;
if (is_x2apic_mode(dev)) {
@@ -708,7 +707,6 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, uint8_t dest_mode,
}
apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
- g_free(deliver_bitmask);
}
static bool apic_check_pic(APICCommonState *s)
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 06/12] oslib-posix: fix memory leak in touch_all_pages
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (4 preceding siblings ...)
2024-03-08 14:55 ` [PULL 05/12] hw/intc/apic: fix memory leak Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 07/12] mips: do not list individual devices from configs/ Paolo Bonzini
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Mark Kanda
touch_all_pages() can return early, before creating threads. In this case,
however, it leaks the MemsetContext that it has allocated at the
beginning of the function.
Reported by Coverity as CID 1534922.
Fixes: 04accf43df8 ("oslib-posix: initialize backend memory objects in parallel", 2024-02-06)
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/oslib-posix.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3c379f96c26..e76441695bd 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -467,11 +467,13 @@ static int touch_all_pages(char *area, size_t hpagesize, size_t numpages,
* preallocating synchronously.
*/
if (context->num_threads == 1 && !async) {
+ ret = 0;
if (qemu_madvise(area, hpagesize * numpages,
QEMU_MADV_POPULATE_WRITE)) {
- return -errno;
+ ret = -errno;
}
- return 0;
+ g_free(context);
+ return ret;
}
touch_fn = do_madv_populate_write_pages;
} else {
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 07/12] mips: do not list individual devices from configs/
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (5 preceding siblings ...)
2024-03-08 14:55 ` [PULL 06/12] oslib-posix: fix memory leak in touch_all_pages Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 08/12] target/i386: use TSTEQ/TSTNE to test low bits Paolo Bonzini
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel
Add new "select" and "imply" directives if needed. The resulting
config-devices.mak files are the same as before.
Builds without default devices will become much smaller
than before, and qtests fail (as expected, though suboptimal)
for mips64-softmmu because most tests do not use -nodefaults,
so remove it from build-without-defaults
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configs/devices/mips-softmmu/common.mak | 28 +++-----------------
configs/devices/mips64el-softmmu/default.mak | 3 ---
.gitlab-ci.d/buildtest.yml | 2 +-
hw/display/Kconfig | 2 +-
hw/mips/Kconfig | 20 +++++++++++++-
5 files changed, 25 insertions(+), 30 deletions(-)
diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 1a853841b27..416a5d353e8 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -1,28 +1,8 @@
# Common mips*-softmmu CONFIG defines
-CONFIG_ISA_BUS=y
-CONFIG_PCI=y
-CONFIG_PCI_DEVICES=y
-CONFIG_VGA_ISA=y
-CONFIG_VGA_MMIO=y
-CONFIG_VGA_CIRRUS=y
-CONFIG_VMWARE_VGA=y
-CONFIG_SERIAL=y
-CONFIG_SERIAL_ISA=y
-CONFIG_PARALLEL=y
-CONFIG_I8254=y
-CONFIG_PCSPK=y
-CONFIG_PCKBD=y
-CONFIG_FDC=y
-CONFIG_I8257=y
-CONFIG_IDE_ISA=y
-CONFIG_PFLASH_CFI01=y
-CONFIG_I8259=y
-CONFIG_MC146818RTC=y
-CONFIG_MIPS_CPS=y
-CONFIG_MIPS_ITU=y
+# Uncomment the following lines to disable these optional devices:
+# CONFIG_PCI_DEVICES=n
+# CONFIG_TEST_DEVICES=n
+
CONFIG_MALTA=y
-CONFIG_PCNET_PCI=y
CONFIG_MIPSSIM=y
-CONFIG_SMBUS_EEPROM=y
-CONFIG_TEST_DEVICES=y
diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
index d5188f7ea58..88a37cf27f1 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -3,8 +3,5 @@
include ../mips-softmmu/common.mak
CONFIG_FULOONG=y
CONFIG_LOONGSON3V=y
-CONFIG_ATI_VGA=y
-CONFIG_RTL8139_PCI=y
CONFIG_JAZZ=y
-CONFIG_VT82C686=y
CONFIG_MIPS_BOSTON=y
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index a1c030337b1..901265af95d 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -659,7 +659,7 @@ build-without-defaults:
--disable-pie
--disable-qom-cast-debug
--disable-strip
- TARGETS: avr-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
+ TARGETS: avr-softmmu s390x-softmmu sh4-softmmu
sparc64-softmmu hexagon-linux-user i386-linux-user s390x-linux-user
MAKE_CHECK_ARGS: check
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 07acb37dc66..234c7de027c 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -55,7 +55,7 @@ config VGA_MMIO
config VMWARE_VGA
bool
- default y if PCI_DEVICES && PC_PCI
+ default y if PCI_DEVICES && (PC_PCI || MIPS)
depends on PCI
select VGA
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index e57db4f6412..5c83ef49cf6 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,8 +1,15 @@
config MALTA
bool
+ imply PCNET_PCI
+ imply PCI_DEVICES
+ imply TEST_DEVICES
select FDC37M81X
select GT64120
+ select MIPS_CPS
select PIIX
+ select PFLASH_CFI01
+ select SERIAL
+ select SMBUS_EEPROM
config MIPSSIM
bool
@@ -31,17 +38,26 @@ config JAZZ
config FULOONG
bool
+ imply PCI_DEVICES
+ imply TEST_DEVICES
+ imply ATI_VGA
+ imply RTL8139_PCI
select PCI_BONITO
+ select SMBUS_EEPROM
select VT82C686
config LOONGSON3V
bool
+ imply PCI_DEVICES
+ imply TEST_DEVICES
+ imply VIRTIO_PCI
+ imply VIRTIO_NET
imply VIRTIO_VGA
imply QXL if SPICE
+ imply USB_OHCI_PCI
select SERIAL
select GOLDFISH_RTC
select LOONGSON_LIOINTC
- select PCI_DEVICES
select PCI_EXPRESS_GENERIC_BRIDGE
select MSI_NONBROKEN
select FW_CFG_MIPS
@@ -53,6 +69,8 @@ config MIPS_CPS
config MIPS_BOSTON
bool
+ imply PCI_DEVICES
+ imply TEST_DEVICES
select FITLOADER
select MIPS_CPS
select PCI_EXPRESS_XILINX
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 08/12] target/i386: use TSTEQ/TSTNE to test low bits
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (6 preceding siblings ...)
2024-03-08 14:55 ` [PULL 07/12] mips: do not list individual devices from configs/ Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 09/12] target/i386: use TSTEQ/TSTNE to check flags Paolo Bonzini
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
When testing the sign bit or equality to zero of a partial register, it
is useful to use a single TSTEQ or TSTNE operation. It can also be used
to test the parity flag, using bit 0 of the population count.
Do not do this for target_ulong-sized values however; the optimizer would
produce a comparison against zero anyway, and it avoids shifts by 64
which are undefined behavior.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 28 ++++++++++++++++++++--------
target/i386/tcg/emit.c.inc | 5 ++---
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 07f642dc9e9..fe9021833e3 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -927,11 +927,21 @@ typedef struct CCPrepare {
bool no_setcond;
} CCPrepare;
+static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
+{
+ if (size == MO_TL) {
+ return (CCPrepare) { .cond = TCG_COND_LT, .reg = src, .mask = -1 };
+ } else {
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src, .mask = -1,
+ .imm = 1ull << ((8 << size) - 1) };
+ }
+}
+
/* compute eflags.C to reg */
static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
{
TCGv t0, t1;
- int size, shift;
+ MemOp size;
switch (s->cc_op) {
case CC_OP_SUBB ... CC_OP_SUBQ:
@@ -966,9 +976,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
case CC_OP_SHLB ... CC_OP_SHLQ:
/* (CC_SRC >> (DATA_BITS - 1)) & 1 */
size = s->cc_op - CC_OP_SHLB;
- shift = (8 << size) - 1;
- return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = (target_ulong)1 << shift };
+ return gen_prepare_sign_nz(cpu_cc_src, size);
case CC_OP_MULB ... CC_OP_MULQ:
return (CCPrepare) { .cond = TCG_COND_NE,
@@ -1028,8 +1036,7 @@ static CCPrepare gen_prepare_eflags_s(DisasContext *s, TCGv reg)
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
- TCGv t0 = gen_ext_tl(reg, cpu_cc_dst, size, true);
- return (CCPrepare) { .cond = TCG_COND_LT, .reg = t0, .mask = -1 };
+ return gen_prepare_sign_nz(cpu_cc_dst, size);
}
}
}
@@ -1076,8 +1083,13 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg)
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
- TCGv t0 = gen_ext_tl(reg, cpu_cc_dst, size, false);
- return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0, .mask = -1 };
+ if (size == MO_TL) {
+ return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst,
+ .mask = -1 };
+ } else {
+ return (CCPrepare) { .cond = TCG_COND_TSTEQ, .reg = cpu_cc_dst,
+ .mask = -1, .imm = (1ull << (8 << size)) - 1 };
+ }
}
}
}
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 6bcf88ecd71..0e00f6635dd 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1209,7 +1209,7 @@ static void gen_CMPccXADD(DisasContext *s, CPUX86State *env, X86DecodedInsn *dec
[JCC_Z] = TCG_COND_EQ,
[JCC_BE] = TCG_COND_LEU,
[JCC_S] = TCG_COND_LT, /* test sign bit by comparing against 0 */
- [JCC_P] = TCG_COND_EQ, /* even parity - tests low bit of popcount */
+ [JCC_P] = TCG_COND_TSTEQ, /* even parity - tests low bit of popcount */
[JCC_L] = TCG_COND_LT,
[JCC_LE] = TCG_COND_LE,
};
@@ -1260,8 +1260,7 @@ static void gen_CMPccXADD(DisasContext *s, CPUX86State *env, X86DecodedInsn *dec
case JCC_P:
tcg_gen_ext8u_tl(s->tmp0, s->T0);
tcg_gen_ctpop_tl(s->tmp0, s->tmp0);
- tcg_gen_andi_tl(s->tmp0, s->tmp0, 1);
- cmp_lhs = s->tmp0, cmp_rhs = tcg_constant_tl(0);
+ cmp_lhs = s->tmp0, cmp_rhs = tcg_constant_tl(1);
break;
case JCC_S:
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 09/12] target/i386: use TSTEQ/TSTNE to check flags
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (7 preceding siblings ...)
2024-03-08 14:55 ` [PULL 08/12] target/i386: use TSTEQ/TSTNE to test low bits Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 10/12] target/i386: remove mask from CCPrepare Paolo Bonzini
` (3 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
The new conditions obviously come in handy when testing individual bits
of EFLAGS, and they make it possible to remove the .mask field of
CCPrepare.
Lowering to shift+and is done by the optimizer if necessary.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index fe9021833e3..63d520e0cba 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -995,8 +995,8 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
case CC_OP_EFLAGS:
case CC_OP_SARB ... CC_OP_SARQ:
/* CC_SRC & 1 */
- return (CCPrepare) { .cond = TCG_COND_NE,
- .reg = cpu_cc_src, .mask = CC_C };
+ return (CCPrepare) { .cond = TCG_COND_TSTNE,
+ .reg = cpu_cc_src, .mask = -1, .imm = CC_C };
default:
/* The need to compute only C from CC_OP_DYNAMIC is important
@@ -1013,8 +1013,8 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
static CCPrepare gen_prepare_eflags_p(DisasContext *s, TCGv reg)
{
gen_compute_eflags(s);
- return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = CC_P };
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
+ .mask = -1, .imm = CC_P };
}
/* compute eflags.S to reg */
@@ -1028,8 +1028,8 @@ static CCPrepare gen_prepare_eflags_s(DisasContext *s, TCGv reg)
case CC_OP_ADCX:
case CC_OP_ADOX:
case CC_OP_ADCOX:
- return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = CC_S };
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
+ .mask = -1, .imm = CC_S };
case CC_OP_CLR:
case CC_OP_POPCNT:
return (CCPrepare) { .cond = TCG_COND_NEVER, .mask = -1 };
@@ -1057,8 +1057,8 @@ static CCPrepare gen_prepare_eflags_o(DisasContext *s, TCGv reg)
.reg = cpu_cc_src, .mask = -1 };
default:
gen_compute_eflags(s);
- return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = CC_O };
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
+ .mask = -1, .imm = CC_O };
}
}
@@ -1073,8 +1073,8 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg)
case CC_OP_ADCX:
case CC_OP_ADOX:
case CC_OP_ADCOX:
- return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = CC_Z };
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
+ .mask = -1, .imm = CC_Z };
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS, .mask = -1 };
case CC_OP_POPCNT:
@@ -1152,8 +1152,8 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
break;
case JCC_BE:
gen_compute_eflags(s);
- cc = (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = CC_Z | CC_C };
+ cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
+ .mask = -1, .imm = CC_Z | CC_C };
break;
case JCC_S:
cc = gen_prepare_eflags_s(s, reg);
@@ -1167,8 +1167,8 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
reg = s->tmp0;
}
tcg_gen_addi_tl(reg, cpu_cc_src, CC_O - CC_S);
- cc = (CCPrepare) { .cond = TCG_COND_NE, .reg = reg,
- .mask = CC_O };
+ cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = reg,
+ .mask = -1, .imm = CC_O };
break;
default:
case JCC_LE:
@@ -1177,8 +1177,8 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
reg = s->tmp0;
}
tcg_gen_addi_tl(reg, cpu_cc_src, CC_O - CC_S);
- cc = (CCPrepare) { .cond = TCG_COND_NE, .reg = reg,
- .mask = CC_O | CC_Z };
+ cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = reg,
+ .mask = -1, .imm = CC_O | CC_Z };
break;
}
break;
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 10/12] target/i386: remove mask from CCPrepare
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (8 preceding siblings ...)
2024-03-08 14:55 ` [PULL 09/12] target/i386: use TSTEQ/TSTNE to check flags Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 11/12] run-coverity-scan: add --check-upload-only option Paolo Bonzini
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
With the introduction of TSTEQ and TSTNE the .mask field is always -1,
so remove all the now-unnecessary code.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 81 +++++++++++++------------------------
1 file changed, 27 insertions(+), 54 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 63d520e0cba..6b4522c226d 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -922,7 +922,6 @@ typedef struct CCPrepare {
TCGv reg;
TCGv reg2;
target_ulong imm;
- target_ulong mask;
bool use_reg2;
bool no_setcond;
} CCPrepare;
@@ -930,9 +929,9 @@ typedef struct CCPrepare {
static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
{
if (size == MO_TL) {
- return (CCPrepare) { .cond = TCG_COND_LT, .reg = src, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_LT, .reg = src };
} else {
- return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src, .mask = -1,
+ return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src,
.imm = 1ull << ((8 << size) - 1) };
}
}
@@ -961,17 +960,17 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
t0 = gen_ext_tl(reg, cpu_cc_dst, size, false);
add_sub:
return (CCPrepare) { .cond = TCG_COND_LTU, .reg = t0,
- .reg2 = t1, .mask = -1, .use_reg2 = true };
+ .reg2 = t1, .use_reg2 = true };
case CC_OP_LOGICB ... CC_OP_LOGICQ:
case CC_OP_CLR:
case CC_OP_POPCNT:
- return (CCPrepare) { .cond = TCG_COND_NEVER, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_NEVER };
case CC_OP_INCB ... CC_OP_INCQ:
case CC_OP_DECB ... CC_OP_DECQ:
return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src,
- .mask = -1, .no_setcond = true };
+ .no_setcond = true };
case CC_OP_SHLB ... CC_OP_SHLQ:
/* (CC_SRC >> (DATA_BITS - 1)) & 1 */
@@ -980,23 +979,23 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
case CC_OP_MULB ... CC_OP_MULQ:
return (CCPrepare) { .cond = TCG_COND_NE,
- .reg = cpu_cc_src, .mask = -1 };
+ .reg = cpu_cc_src };
case CC_OP_BMILGB ... CC_OP_BMILGQ:
size = s->cc_op - CC_OP_BMILGB;
t0 = gen_ext_tl(reg, cpu_cc_src, size, false);
- return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0 };
case CC_OP_ADCX:
case CC_OP_ADCOX:
return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_dst,
- .mask = -1, .no_setcond = true };
+ .no_setcond = true };
case CC_OP_EFLAGS:
case CC_OP_SARB ... CC_OP_SARQ:
/* CC_SRC & 1 */
return (CCPrepare) { .cond = TCG_COND_TSTNE,
- .reg = cpu_cc_src, .mask = -1, .imm = CC_C };
+ .reg = cpu_cc_src, .imm = CC_C };
default:
/* The need to compute only C from CC_OP_DYNAMIC is important
@@ -1005,7 +1004,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv reg)
gen_helper_cc_compute_c(reg, cpu_cc_dst, cpu_cc_src,
cpu_cc_src2, cpu_cc_op);
return (CCPrepare) { .cond = TCG_COND_NE, .reg = reg,
- .mask = -1, .no_setcond = true };
+ .no_setcond = true };
}
}
@@ -1014,7 +1013,7 @@ static CCPrepare gen_prepare_eflags_p(DisasContext *s, TCGv reg)
{
gen_compute_eflags(s);
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
- .mask = -1, .imm = CC_P };
+ .imm = CC_P };
}
/* compute eflags.S to reg */
@@ -1029,10 +1028,10 @@ static CCPrepare gen_prepare_eflags_s(DisasContext *s, TCGv reg)
case CC_OP_ADOX:
case CC_OP_ADCOX:
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
- .mask = -1, .imm = CC_S };
+ .imm = CC_S };
case CC_OP_CLR:
case CC_OP_POPCNT:
- return (CCPrepare) { .cond = TCG_COND_NEVER, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_NEVER };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
@@ -1048,17 +1047,16 @@ static CCPrepare gen_prepare_eflags_o(DisasContext *s, TCGv reg)
case CC_OP_ADOX:
case CC_OP_ADCOX:
return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src2,
- .mask = -1, .no_setcond = true };
+ .no_setcond = true };
case CC_OP_CLR:
case CC_OP_POPCNT:
- return (CCPrepare) { .cond = TCG_COND_NEVER, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_NEVER };
case CC_OP_MULB ... CC_OP_MULQ:
- return (CCPrepare) { .cond = TCG_COND_NE,
- .reg = cpu_cc_src, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src };
default:
gen_compute_eflags(s);
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
- .mask = -1, .imm = CC_O };
+ .imm = CC_O };
}
}
@@ -1074,21 +1072,19 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg)
case CC_OP_ADOX:
case CC_OP_ADCOX:
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
- .mask = -1, .imm = CC_Z };
+ .imm = CC_Z };
case CC_OP_CLR:
- return (CCPrepare) { .cond = TCG_COND_ALWAYS, .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_ALWAYS };
case CC_OP_POPCNT:
- return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src,
- .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
if (size == MO_TL) {
- return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst,
- .mask = -1 };
+ return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
} else {
return (CCPrepare) { .cond = TCG_COND_TSTEQ, .reg = cpu_cc_dst,
- .mask = -1, .imm = (1ull << (8 << size)) - 1 };
+ .imm = (1ull << (8 << size)) - 1 };
}
}
}
@@ -1116,7 +1112,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
gen_extu(size, s->tmp4);
t0 = gen_ext_tl(s->tmp0, cpu_cc_src, size, false);
cc = (CCPrepare) { .cond = TCG_COND_LEU, .reg = s->tmp4,
- .reg2 = t0, .mask = -1, .use_reg2 = true };
+ .reg2 = t0, .use_reg2 = true };
break;
case JCC_L:
@@ -1129,7 +1125,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
gen_exts(size, s->tmp4);
t0 = gen_ext_tl(s->tmp0, cpu_cc_src, size, true);
cc = (CCPrepare) { .cond = cond, .reg = s->tmp4,
- .reg2 = t0, .mask = -1, .use_reg2 = true };
+ .reg2 = t0, .use_reg2 = true };
break;
default:
@@ -1153,7 +1149,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
case JCC_BE:
gen_compute_eflags(s);
cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
- .mask = -1, .imm = CC_Z | CC_C };
+ .imm = CC_Z | CC_C };
break;
case JCC_S:
cc = gen_prepare_eflags_s(s, reg);
@@ -1168,7 +1164,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
}
tcg_gen_addi_tl(reg, cpu_cc_src, CC_O - CC_S);
cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = reg,
- .mask = -1, .imm = CC_O };
+ .imm = CC_O };
break;
default:
case JCC_LE:
@@ -1178,7 +1174,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, TCGv reg)
}
tcg_gen_addi_tl(reg, cpu_cc_src, CC_O - CC_S);
cc = (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = reg,
- .mask = -1, .imm = CC_O | CC_Z };
+ .imm = CC_O | CC_Z };
break;
}
break;
@@ -1203,16 +1199,6 @@ static void gen_setcc1(DisasContext *s, int b, TCGv reg)
return;
}
- if (cc.cond == TCG_COND_NE && !cc.use_reg2 && cc.imm == 0 &&
- cc.mask != 0 && (cc.mask & (cc.mask - 1)) == 0) {
- tcg_gen_shri_tl(reg, cc.reg, ctztl(cc.mask));
- tcg_gen_andi_tl(reg, reg, 1);
- return;
- }
- if (cc.mask != -1) {
- tcg_gen_andi_tl(reg, cc.reg, cc.mask);
- cc.reg = reg;
- }
if (cc.use_reg2) {
tcg_gen_setcond_tl(cc.cond, reg, cc.reg, cc.reg2);
} else {
@@ -1231,10 +1217,6 @@ static inline void gen_jcc1_noeob(DisasContext *s, int b, TCGLabel *l1)
{
CCPrepare cc = gen_prepare_cc(s, b, s->T0);
- if (cc.mask != -1) {
- tcg_gen_andi_tl(s->T0, cc.reg, cc.mask);
- cc.reg = s->T0;
- }
if (cc.use_reg2) {
tcg_gen_brcond_tl(cc.cond, cc.reg, cc.reg2, l1);
} else {
@@ -1250,10 +1232,6 @@ static inline void gen_jcc1(DisasContext *s, int b, TCGLabel *l1)
CCPrepare cc = gen_prepare_cc(s, b, s->T0);
gen_update_cc_op(s);
- if (cc.mask != -1) {
- tcg_gen_andi_tl(s->T0, cc.reg, cc.mask);
- cc.reg = s->T0;
- }
set_cc_op(s, CC_OP_DYNAMIC);
if (cc.use_reg2) {
tcg_gen_brcond_tl(cc.cond, cc.reg, cc.reg2, l1);
@@ -2518,11 +2496,6 @@ static void gen_cmovcc1(DisasContext *s, int b, TCGv dest, TCGv src)
{
CCPrepare cc = gen_prepare_cc(s, b, s->T1);
- if (cc.mask != -1) {
- TCGv t0 = tcg_temp_new();
- tcg_gen_andi_tl(t0, cc.reg, cc.mask);
- cc.reg = t0;
- }
if (!cc.use_reg2) {
cc.reg2 = tcg_constant_tl(cc.imm);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 11/12] run-coverity-scan: add --check-upload-only option
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (9 preceding siblings ...)
2024-03-08 14:55 ` [PULL 10/12] target/i386: remove mask from CCPrepare Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 14:55 ` [PULL 12/12] gitlab-ci: add manual job to run Coverity Paolo Bonzini
2024-03-08 17:31 ` [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell
Add an option to check if upload is permitted without actually
attempting a build. This can be useful to add a third outcome
beyond success and failure---namely, a CI job can self-cancel
if the uploading quota has been reached.
There is a small change here in that a failure to do the upload
check changes the exit code from 1 to 99. 99 was chosen because
it is what Autotools and Meson use to represent a problem in the
setup (as opposed to a failure in the test).
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/coverity-scan/run-coverity-scan | 59 ++++++++++++++++++-------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index d56c9b66776..43cf770f5e3 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -28,6 +28,7 @@
# project settings, if you have maintainer access there.
# Command line options:
+# --check-upload-only : return success if upload is possible
# --dry-run : run the tools, but don't actually do the upload
# --docker : create and work inside a container
# --docker-engine : specify the container engine to use (docker/podman/auto);
@@ -57,18 +58,18 @@
# putting it in a file and using --tokenfile. Everything else has
# a reasonable default if this is run from a git tree.
-check_upload_permissions() {
- # Check whether we can do an upload to the server; will exit the script
- # with status 1 if the check failed (usually a bad token);
- # will exit the script with status 0 if the check indicated that we
- # can't upload yet (ie we are at quota)
- # Assumes that COVERITY_TOKEN, PROJNAME and DRYRUN have been initialized.
+upload_permitted() {
+ # Check whether we can do an upload to the server; will exit *the script*
+ # with status 99 if the check failed (usually a bad token);
+ # will return from the function with status 1 if the check indicated
+ # that we can't upload yet (ie we are at quota)
+ # Assumes that COVERITY_TOKEN and PROJNAME have been initialized.
echo "Checking upload permissions..."
if ! up_perm="$(wget https://scan.coverity.com/api/upload_permitted --post-data "token=$COVERITY_TOKEN&project=$PROJNAME" -q -O -)"; then
echo "Coverity Scan API access denied: bad token?"
- exit 1
+ exit 99
fi
# Really up_perm is a JSON response with either
@@ -76,25 +77,40 @@ check_upload_permissions() {
# We do some hacky string parsing instead of properly parsing it.
case "$up_perm" in
*upload_permitted*true*)
- echo "Coverity Scan: upload permitted"
+ return 0
;;
*next_upload_permitted_at*)
- if [ "$DRYRUN" = yes ]; then
- echo "Coverity Scan: upload quota reached, continuing dry run"
- else
- echo "Coverity Scan: upload quota reached; stopping here"
- # Exit success as this isn't a build error.
- exit 0
- fi
+ return 1
;;
*)
echo "Coverity Scan upload check: unexpected result $up_perm"
- exit 1
+ exit 99
;;
esac
}
+check_upload_permissions() {
+ # Check whether we can do an upload to the server; will exit the script
+ # with status 99 if the check failed (usually a bad token);
+ # will exit the script with status 0 if the check indicated that we
+ # can't upload yet (ie we are at quota)
+ # Assumes that COVERITY_TOKEN, PROJNAME and DRYRUN have been initialized.
+
+ if upload_permitted; then
+ echo "Coverity Scan: upload permitted"
+ else
+ if [ "$DRYRUN" = yes ]; then
+ echo "Coverity Scan: upload quota reached, continuing dry run"
+ else
+ echo "Coverity Scan: upload quota reached; stopping here"
+ # Exit success as this isn't a build error.
+ exit 0
+ fi
+ fi
+}
+
+
build_docker_image() {
# build docker container including the coverity-scan tools
echo "Building docker container..."
@@ -152,9 +168,14 @@ update_coverity_tools () {
DRYRUN=no
UPDATE=yes
DOCKER=no
+PROJNAME=QEMU
while [ "$#" -ge 1 ]; do
case "$1" in
+ --check-upload-only)
+ shift
+ DRYRUN=check
+ ;;
--dry-run)
shift
DRYRUN=yes
@@ -251,6 +272,11 @@ if [ -z "$COVERITY_TOKEN" ]; then
exit 1
fi
+if [ "$DRYRUN" = check ]; then
+ upload_permitted
+ exit $?
+fi
+
if [ -z "$COVERITY_BUILD_CMD" ]; then
NPROC=$(nproc)
COVERITY_BUILD_CMD="make -j$NPROC"
@@ -266,7 +292,6 @@ if [ -z "$SRCDIR" ]; then
SRCDIR="$PWD"
fi
-PROJNAME=QEMU
TARBALL=cov-int.tar.xz
if [ "$UPDATE" = only ]; then
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 12/12] gitlab-ci: add manual job to run Coverity
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (10 preceding siblings ...)
2024-03-08 14:55 ` [PULL 11/12] run-coverity-scan: add --check-upload-only option Paolo Bonzini
@ 2024-03-08 14:55 ` Paolo Bonzini
2024-03-08 17:31 ` [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-03-08 14:55 UTC (permalink / raw)
To: qemu-devel
Add a job that can be run, either manually or on a schedule, to upload
a build to Coverity Scan. The job uses the run-coverity-scan script
in multiple phases of check, download tools and upload, in order to
avoid both wasting time (skip everything if you are above the upload
quota) and avoid filling the log with the progress of downloading
the tools.
The job is intended to run on a scheduled pipeline run, and scheduled
runs will not get any other job. It requires two variables to be in
GitLab CI, COVERITY_TOKEN and COVERITY_EMAIL. Those are already set up
in qemu-project's configuration as protected and masked variables.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
.gitlab-ci.d/base.yml | 4 ++++
.gitlab-ci.d/buildtest.yml | 37 +++++++++++++++++++++++++++++++++++++
.gitlab-ci.d/opensbi.yml | 4 ++++
3 files changed, 45 insertions(+)
diff --git a/.gitlab-ci.d/base.yml b/.gitlab-ci.d/base.yml
index ef173a34e63..2dd8a9b57cb 100644
--- a/.gitlab-ci.d/base.yml
+++ b/.gitlab-ci.d/base.yml
@@ -41,6 +41,10 @@ variables:
- if: '$CI_PROJECT_NAMESPACE == $QEMU_CI_UPSTREAM && $CI_COMMIT_TAG'
when: never
+ # Scheduled runs on mainline don't get pipelines except for the special Coverity job
+ - if: '$CI_PROJECT_NAMESPACE == $QEMU_CI_UPSTREAM && $CI_PIPELINE_SOURCE == "schedule"'
+ when: never
+
# Cirrus jobs can't run unless the creds / target repo are set
- if: '$QEMU_JOB_CIRRUS && ($CIRRUS_GITHUB_REPO == null || $CIRRUS_API_TOKEN == null)'
when: never
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 901265af95d..c7d92fc3018 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -729,3 +729,40 @@ pages:
- public
variables:
QEMU_JOB_PUBLISH: 1
+
+coverity:
+ image: $CI_REGISTRY_IMAGE/qemu/fedora:$QEMU_CI_CONTAINER_TAG
+ stage: build
+ allow_failure: true
+ timeout: 3h
+ needs:
+ - job: amd64-fedora-container
+ optional: true
+ before_script:
+ - dnf install -y curl wget
+ script:
+ # would be nice to cancel the job if over quota (https://gitlab.com/gitlab-org/gitlab/-/issues/256089)
+ # for example:
+ # curl --request POST --header "PRIVATE-TOKEN: $CI_JOB_TOKEN" "${CI_SERVER_URL}/api/v4/projects/${CI_PROJECT_ID}/jobs/${CI_JOB_ID}/cancel
+ - 'scripts/coverity-scan/run-coverity-scan --check-upload-only || { exitcode=$?; if test $exitcode = 1; then
+ exit 0;
+ else
+ exit $exitcode;
+ fi; };
+ scripts/coverity-scan/run-coverity-scan --update-tools-only > update-tools.log 2>&1 || { cat update-tools.log; exit 1; };
+ scripts/coverity-scan/run-coverity-scan --no-update-tools'
+ rules:
+ - if: '$COVERITY_TOKEN == null'
+ when: never
+ - if: '$COVERITY_EMAIL == null'
+ when: never
+ # Never included on upstream pipelines, except for schedules
+ - if: '$CI_PROJECT_NAMESPACE == $QEMU_CI_UPSTREAM && $CI_PIPELINE_SOURCE == "schedule"'
+ when: on_success
+ - if: '$CI_PROJECT_NAMESPACE == $QEMU_CI_UPSTREAM'
+ when: never
+ # Forks don't get any pipeline unless QEMU_CI=1 or QEMU_CI=2 is set
+ - if: '$QEMU_CI != "1" && $QEMU_CI != "2"'
+ when: never
+ # Always manual on forks even if $QEMU_CI == "2"
+ - when: manual
diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
index fd293e6c317..42f137d624e 100644
--- a/.gitlab-ci.d/opensbi.yml
+++ b/.gitlab-ci.d/opensbi.yml
@@ -24,6 +24,10 @@
- if: '$QEMU_CI == "1" && $CI_PROJECT_NAMESPACE != "qemu-project" && $CI_COMMIT_MESSAGE =~ /opensbi/i'
when: manual
+ # Scheduled runs on mainline don't get pipelines except for the special Coverity job
+ - if: '$CI_PROJECT_NAMESPACE == $QEMU_CI_UPSTREAM && $CI_PIPELINE_SOURCE == "schedule"'
+ when: never
+
# Run if any files affecting the build output are touched
- changes:
- .gitlab-ci.d/opensbi.yml
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08
2024-03-08 14:55 [PULL 00/12] Misc fixes, i386 TSTEQ/TSTNE, coverity CI for 2024-03-08 Paolo Bonzini
` (11 preceding siblings ...)
2024-03-08 14:55 ` [PULL 12/12] gitlab-ci: add manual job to run Coverity Paolo Bonzini
@ 2024-03-08 17:31 ` Peter Maydell
2024-03-08 18:12 ` Paolo Bonzini
12 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2024-03-08 17:31 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Fri, 8 Mar 2024 at 14:58, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 8f6330a807f2642dc2a3cdf33347aa28a4c00a87:
>
> Merge tag 'pull-maintainer-updates-060324-1' of https://gitlab.com/stsquad/qemu into staging (2024-03-06 16:56:20 +0000)
>
> are available in the Git repository at:
>
> https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 47791be8cc6efa0fb9c145a2b92da0417f4137b8:
>
> gitlab-ci: add manual job to run Coverity (2024-03-08 15:52:26 +0100)
>
> ----------------------------------------------------------------
> * target/i386: use TSTEQ/TSTNE
> * move Coverity builds to Gitlab CI
> * fix two memory leaks
> * bug fixes
>
> ----------------------------------------------------------------
> Akihiko Odaki (1):
> meson: Remove --warn-common ldflag
>
> Dmitrii Gavrilov (1):
> system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add()
>
> Paolo Bonzini (8):
> hw/intc/apic: fix memory leak
> oslib-posix: fix memory leak in touch_all_pages
> mips: do not list individual devices from configs/
> target/i386: use TSTEQ/TSTNE to test low bits
> target/i386: use TSTEQ/TSTNE to check flags
> target/i386: remove mask from CCPrepare
> run-coverity-scan: add --check-upload-only option
> gitlab-ci: add manual job to run Coverity
>
> Sven Schnelle (2):
> hw/scsi/lsi53c895a: add timer to scripts processing
> hw/scsi/lsi53c895a: stop script on phase mismatch
Looks like this hits a TCG assertion on aarch64 host:
https://gitlab.com/qemu-project/qemu/-/jobs/6353434430
Several of the qtest tests fail with:
66/853 qemu:qtest+qtest-x86_64 / qtest-x86_64/vmgenid-test ERROR 0.44s
killed by signal 6 SIGABRT
>>> G_TEST_DBUS_DAEMON=/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon PYTHON=/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=139 QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-x86_64 /home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/tests/qtest/vmgenid-test --tap -k
――――――――――――――――――――――――――――――――――――― ✀ ―――――――――――――――――――――――――――――――――――――
stderr:
**
ERROR:/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/tcg/aarch64/tcg-target.c.inc:1511:tcg_out_brcond:
code should not be reached
Broken pipe
../tests/qtest/libqtest.c:204: kill_qemu() detected QEMU death from
signal 6 (Aborted) (core dumped)
(test program exited with status code -6)
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread