* [Stable-8.1.2 01/45] hw/ppc: Introduce functions for conversion between timebase and nanoseconds
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 02/45] host-utils: Add muldiv64_round_up Michael Tokarev
` (44 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
These calculations are repeated several times, and they will become
a little more complicated with subsequent changes.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 7798f5c576d898e7e10c4a2518f3f16411dedeb9)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 0e0a3d93c3..9d14fdcc98 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -482,10 +482,20 @@ void ppce500_set_mpic_proxy(bool enabled)
/*****************************************************************************/
/* PowerPC time base and decrementer emulation */
+static uint64_t ns_to_tb(uint32_t freq, int64_t clock)
+{
+ return muldiv64(clock, freq, NANOSECONDS_PER_SECOND);
+}
+
+static int64_t tb_to_ns(uint32_t freq, uint64_t tb)
+{
+ return muldiv64(tb, NANOSECONDS_PER_SECOND, freq);
+}
+
uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset)
{
/* TB time in tb periods */
- return muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND) + tb_offset;
+ return ns_to_tb(tb_env->tb_freq, vmclk) + tb_offset;
}
uint64_t cpu_ppc_load_tbl (CPUPPCState *env)
@@ -526,8 +536,7 @@ uint32_t cpu_ppc_load_tbu (CPUPPCState *env)
static inline void cpu_ppc_store_tb(ppc_tb_t *tb_env, uint64_t vmclk,
int64_t *tb_offsetp, uint64_t value)
{
- *tb_offsetp = value -
- muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND);
+ *tb_offsetp = value - ns_to_tb(tb_env->tb_freq, vmclk);
trace_ppc_tb_store(value, *tb_offsetp);
}
@@ -690,11 +699,11 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
diff = next - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
if (diff >= 0) {
- decr = muldiv64(diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
+ decr = ns_to_tb(tb_env->decr_freq, diff);
} else if (tb_env->flags & PPC_TIMER_BOOKE) {
decr = 0;
} else {
- decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
+ decr = -ns_to_tb(tb_env->decr_freq, -diff);
}
trace_ppc_decr_load(decr);
@@ -834,7 +843,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
/* Calculate the next timer event */
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + muldiv64(value, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
+ next = now + tb_to_ns(tb_env->decr_freq, value);
*nextp = next;
/* Adjust timer */
@@ -1125,7 +1134,7 @@ static void cpu_4xx_fit_cb (void *opaque)
/* Cannot occur, but makes gcc happy */
return;
}
- next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->tb_freq);
+ next = now + tb_to_ns(tb_env->tb_freq, next);
if (next == now)
next++;
timer_mod(ppc40x_timer->fit_timer, next);
@@ -1153,8 +1162,7 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
} else {
trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + muldiv64(ppc40x_timer->pit_reload,
- NANOSECONDS_PER_SECOND, tb_env->decr_freq);
+ next = now + tb_to_ns(tb_env->decr_freq, ppc40x_timer->pit_reload);
if (is_excp)
next += tb_env->decr_next - now;
if (next == now)
@@ -1213,7 +1221,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
/* Cannot occur, but makes gcc happy */
return;
}
- next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
+ next = now + tb_to_ns(tb_env->decr_freq, next);
if (next == now)
next++;
trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 02/45] host-utils: Add muldiv64_round_up
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 01/45] hw/ppc: Introduce functions for conversion between timebase and nanoseconds Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 03/45] hw/ppc: Round up the decrementer interval when converting to ns Michael Tokarev
` (43 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Richard Henderson,
Cédric Le Goater, Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
This will be used for converting time intervals in different base units
to host units, for the purpose of scheduling timers to emulate target
timers. Timers typically must not fire before their requested expiry
time but may fire some time afterward, so rounding up is the right way
to implement these.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
[ clg: renamed __muldiv64() to muldiv64_rounding() ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 47de6c4c287079744ceb96f606b3c0457addf380)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 011618373e..ead97d354d 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -56,6 +56,11 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
return (__int128_t)a * b / c;
}
+static inline uint64_t muldiv64_round_up(uint64_t a, uint32_t b, uint32_t c)
+{
+ return ((__int128_t)a * b + c - 1) / c;
+}
+
static inline uint64_t divu128(uint64_t *plow, uint64_t *phigh,
uint64_t divisor)
{
@@ -83,7 +88,8 @@ void mulu64(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b);
uint64_t divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
int64_t divs128(uint64_t *plow, int64_t *phigh, int64_t divisor);
-static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+static inline uint64_t muldiv64_rounding(uint64_t a, uint32_t b, uint32_t c,
+ bool round_up)
{
union {
uint64_t ll;
@@ -99,12 +105,25 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
u.ll = a;
rl = (uint64_t)u.l.low * (uint64_t)b;
+ if (round_up) {
+ rl += c - 1;
+ }
rh = (uint64_t)u.l.high * (uint64_t)b;
rh += (rl >> 32);
res.l.high = rh / c;
res.l.low = (((rh % c) << 32) + (rl & 0xffffffff)) / c;
return res.ll;
}
+
+static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
+{
+ return muldiv64_rounding(a, b, c, false);
+}
+
+static inline uint64_t muldiv64_round_up(uint64_t a, uint32_t b, uint32_t c)
+{
+ return muldiv64_rounding(a, b, c, true);
+}
#endif
/**
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 03/45] hw/ppc: Round up the decrementer interval when converting to ns
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 01/45] hw/ppc: Introduce functions for conversion between timebase and nanoseconds Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 02/45] host-utils: Add muldiv64_round_up Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 04/45] hw/ppc: Avoid decrementer rounding errors Michael Tokarev
` (42 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
The rule of timers is typically that they should never expire before the
timeout, but some time afterward. Rounding timer intervals up when doing
conversion is the right thing to do.
Under most circumstances it is impossible observe the decrementer
interrupt before the dec register has triggered. However with icount
timing, problems can arise. For example setting DEC to 0 can schedule
the timer for now, causing it to fire before any more instructions
have been executed and DEC is still 0.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit eab0888418ab44344864965193cf6cd194ab6858)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 9d14fdcc98..3a82c97d5a 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -482,14 +482,26 @@ void ppce500_set_mpic_proxy(bool enabled)
/*****************************************************************************/
/* PowerPC time base and decrementer emulation */
+/*
+ * Conversion between QEMU_CLOCK_VIRTUAL ns and timebase (TB) ticks:
+ * TB ticks are arrived at by multiplying tb_freq then dividing by
+ * ns per second, and rounding down. TB ticks drive all clocks and
+ * timers in the target machine.
+ *
+ * Converting TB intervals to ns for the purpose of setting a
+ * QEMU_CLOCK_VIRTUAL timer should go the other way, but rounding
+ * up. Rounding down could cause the timer to fire before the TB
+ * value has been reached.
+ */
static uint64_t ns_to_tb(uint32_t freq, int64_t clock)
{
return muldiv64(clock, freq, NANOSECONDS_PER_SECOND);
}
-static int64_t tb_to_ns(uint32_t freq, uint64_t tb)
+/* virtual clock in TB ticks, not adjusted by TB offset */
+static int64_t tb_to_ns_round_up(uint32_t freq, uint64_t tb)
{
- return muldiv64(tb, NANOSECONDS_PER_SECOND, freq);
+ return muldiv64_round_up(tb, NANOSECONDS_PER_SECOND, freq);
}
uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset)
@@ -843,7 +855,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
/* Calculate the next timer event */
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + tb_to_ns(tb_env->decr_freq, value);
+ next = now + tb_to_ns_round_up(tb_env->decr_freq, value);
*nextp = next;
/* Adjust timer */
@@ -1134,9 +1146,7 @@ static void cpu_4xx_fit_cb (void *opaque)
/* Cannot occur, but makes gcc happy */
return;
}
- next = now + tb_to_ns(tb_env->tb_freq, next);
- if (next == now)
- next++;
+ next = now + tb_to_ns_round_up(tb_env->tb_freq, next);
timer_mod(ppc40x_timer->fit_timer, next);
env->spr[SPR_40x_TSR] |= 1 << 26;
if ((env->spr[SPR_40x_TCR] >> 23) & 0x1) {
@@ -1162,11 +1172,10 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
} else {
trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + tb_to_ns(tb_env->decr_freq, ppc40x_timer->pit_reload);
+ next = now + tb_to_ns_round_up(tb_env->decr_freq,
+ ppc40x_timer->pit_reload);
if (is_excp)
next += tb_env->decr_next - now;
- if (next == now)
- next++;
timer_mod(tb_env->decr_timer, next);
tb_env->decr_next = next;
}
@@ -1221,9 +1230,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
/* Cannot occur, but makes gcc happy */
return;
}
- next = now + tb_to_ns(tb_env->decr_freq, next);
- if (next == now)
- next++;
+ next = now + tb_to_ns_round_up(tb_env->decr_freq, next);
trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
case 0x0:
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 04/45] hw/ppc: Avoid decrementer rounding errors
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (2 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 03/45] hw/ppc: Round up the decrementer interval when converting to ns Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 05/45] target/ppc: Sign-extend large decrementer to 64-bits Michael Tokarev
` (41 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
The decrementer register contains a relative time in timebase units.
When writing to DECR this is converted and stored as an absolute value
in nanosecond units, reading DECR converts back to relative timebase.
The tb<->ns conversion of the relative part can cause rounding such that
a value writen to the decrementer can read back a different, with time
held constant. This is a particular problem for a deterministic icount
and record-replay trace.
Fix this by storing the absolute value in timebase units rather than
nanoseconds. The math before:
store: decr_next = now_ns + decr * ns_per_sec / tb_per_sec
load: decr = (decr_next - now_ns) * tb_per_sec / ns_per_sec
load(store): decr = decr * ns_per_sec / tb_per_sec * tb_per_sec /
ns_per_sec
After:
store: decr_next = now_ns * tb_per_sec / ns_per_sec + decr
load: decr = decr_next - now_ns * tb_per_sec / ns_per_sec
load(store): decr = decr
Fixes: 9fddaa0c0cab ("PowerPC merge: real time TB and decrementer - faster and simpler exception handling (Jocelyn Mayer)")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 8e0a5ac87800ccc6dd5013f89f27652f4480ab33)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 3a82c97d5a..57d0aae7d7 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -707,16 +707,17 @@ bool ppc_decr_clear_on_delivery(CPUPPCState *env)
static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
{
ppc_tb_t *tb_env = env->tb_env;
- int64_t decr, diff;
+ uint64_t now, n;
+ int64_t decr;
- diff = next - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- if (diff >= 0) {
- decr = ns_to_tb(tb_env->decr_freq, diff);
- } else if (tb_env->flags & PPC_TIMER_BOOKE) {
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ n = ns_to_tb(tb_env->decr_freq, now);
+ if (next > n && tb_env->flags & PPC_TIMER_BOOKE) {
decr = 0;
- } else {
- decr = -ns_to_tb(tb_env->decr_freq, -diff);
+ } else {
+ decr = next - n;
}
+
trace_ppc_decr_load(decr);
return decr;
@@ -853,13 +854,18 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
(*lower_excp)(cpu);
}
- /* Calculate the next timer event */
+ /*
+ * Calculate the next decrementer event and set a timer.
+ * decr_next is in timebase units to keep rounding simple. Note it is
+ * not adjusted by tb_offset because if TB changes via tb_offset changing,
+ * decrementer does not change, so not directly comparable with TB.
+ */
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + tb_to_ns_round_up(tb_env->decr_freq, value);
+ next = ns_to_tb(tb_env->decr_freq, now) + value;
*nextp = next;
/* Adjust timer */
- timer_mod(timer, next);
+ timer_mod(timer, tb_to_ns_round_up(tb_env->decr_freq, next));
}
static inline void _cpu_ppc_store_decr(PowerPCCPU *cpu, target_ulong decr,
@@ -1172,12 +1178,15 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
} else {
trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = now + tb_to_ns_round_up(tb_env->decr_freq,
- ppc40x_timer->pit_reload);
- if (is_excp)
- next += tb_env->decr_next - now;
+
+ if (is_excp) {
+ tb_env->decr_next += ppc40x_timer->pit_reload;
+ } else {
+ tb_env->decr_next = ns_to_tb(tb_env->decr_freq, now)
+ + ppc40x_timer->pit_reload;
+ }
+ next = tb_to_ns_round_up(tb_env->decr_freq, tb_env->decr_next);
timer_mod(tb_env->decr_timer, next);
- tb_env->decr_next = next;
}
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 05/45] target/ppc: Sign-extend large decrementer to 64-bits
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (3 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 04/45] hw/ppc: Avoid decrementer rounding errors Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 06/45] hw/ppc: Always store the decrementer value Michael Tokarev
` (40 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
When storing a large decrementer value with the most significant
implemented bit set, it is to be treated as a negative and sign
extended.
This isn't hit for book3s DEC because of another bug, fixing it
in the next patch exposes this one and can cause additional
problems, so fix this first. It can be hit with HDECR and other
edge triggered types.
Fixes: a8dafa52518 ("target/ppc: Implement large decrementer support for TCG")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[ clg: removed extra cpu and pcc variables shadowing local variables ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit c8fbc6b9f2f3c732ee3307093c1c5c367eaa64ae)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 57d0aae7d7..befa9d95b3 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -739,7 +739,9 @@ target_ulong cpu_ppc_load_decr(CPUPPCState *env)
* to 64 bits, otherwise it is a 32 bit value.
*/
if (env->spr[SPR_LPCR] & LPCR_LD) {
- return decr;
+ PowerPCCPU *cpu = env_archcpu(env);
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ return sextract64(decr, 0, pcc->lrg_decr_bits);
}
return (uint32_t) decr;
}
@@ -758,7 +760,7 @@ target_ulong cpu_ppc_load_hdecr(CPUPPCState *env)
* extended to 64 bits, otherwise it is 32 bits.
*/
if (pcc->lrg_decr_bits > 32) {
- return hdecr;
+ return sextract64(hdecr, 0, pcc->lrg_decr_bits);
}
return (uint32_t) hdecr;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 06/45] hw/ppc: Always store the decrementer value
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (4 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 05/45] target/ppc: Sign-extend large decrementer to 64-bits Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 07/45] hw/ppc: Reset timebase facilities on machine reset Michael Tokarev
` (39 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
When writing a value to the decrementer that raises an exception, the
irq is raised, but the value is not stored so the store doesn't appear
to have changed the register when it is read again.
Always store the write value to the register.
Fixes: e81a982aa53 ("PPC: Clean up DECR implementation")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit febb71d543a8f747b2f8aaf0182d0a385c6a02c3)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index befa9d95b3..06f3ad0719 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -835,6 +835,16 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
return;
}
+ /*
+ * Calculate the next decrementer event and set a timer.
+ * decr_next is in timebase units to keep rounding simple. Note it is
+ * not adjusted by tb_offset because if TB changes via tb_offset changing,
+ * decrementer does not change, so not directly comparable with TB.
+ */
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ next = ns_to_tb(tb_env->decr_freq, now) + value;
+ *nextp = next; /* nextp is in timebase units */
+
/*
* Going from 1 -> 0 or 0 -> -1 is the event to generate a DEC interrupt.
*
@@ -856,16 +866,6 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
(*lower_excp)(cpu);
}
- /*
- * Calculate the next decrementer event and set a timer.
- * decr_next is in timebase units to keep rounding simple. Note it is
- * not adjusted by tb_offset because if TB changes via tb_offset changing,
- * decrementer does not change, so not directly comparable with TB.
- */
- now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- next = ns_to_tb(tb_env->decr_freq, now) + value;
- *nextp = next;
-
/* Adjust timer */
timer_mod(timer, tb_to_ns_round_up(tb_env->decr_freq, next));
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 07/45] hw/ppc: Reset timebase facilities on machine reset
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (5 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 06/45] hw/ppc: Always store the decrementer value Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 08/45] hw/ppc: Read time only once to perform decrementer write Michael Tokarev
` (38 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Mark Cave-Ayland, BALATON Zoltan,
Cédric Le Goater, Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
Lower interrupts, delete timers, and set time facility registers
back to initial state on machine reset.
This is not so important for record-replay since timebase and
decrementer are migrated, but it gives a cleaner reset state.
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[ clg: checkpatch.pl fixes ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 30d0647bcfa99d4a141eaa843a9fb5b091ddbb76)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 510ff0eaaf..9acc7adfc9 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -81,6 +81,7 @@ static void ppc_heathrow_reset(void *opaque)
{
PowerPCCPU *cpu = opaque;
+ cpu_ppc_tb_reset(&cpu->env);
cpu_reset(CPU(cpu));
}
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 075367d94d..bd397cf2b5 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -99,6 +99,7 @@ static void pegasos2_cpu_reset(void *opaque)
cpu->env.gpr[1] = 2 * VOF_STACK_SIZE - 0x20;
cpu->env.nip = 0x100;
}
+ cpu_ppc_tb_reset(&cpu->env);
}
static void pegasos2_pci_irq(void *opaque, int n, int level)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 9b39d527de..8c7afe037f 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -61,6 +61,8 @@ static void pnv_core_cpu_reset(PnvCore *pc, PowerPCCPU *cpu)
hreg_compute_hflags(env);
ppc_maybe_interrupt(env);
+ cpu_ppc_tb_reset(env);
+
pcc->intc_reset(pc->chip, cpu);
}
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 06f3ad0719..8d66b8618a 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -938,23 +938,6 @@ void cpu_ppc_store_purr(CPUPPCState *env, uint64_t value)
&tb_env->purr_offset, value);
}
-static void cpu_ppc_set_tb_clk (void *opaque, uint32_t freq)
-{
- CPUPPCState *env = opaque;
- PowerPCCPU *cpu = env_archcpu(env);
- ppc_tb_t *tb_env = env->tb_env;
-
- tb_env->tb_freq = freq;
- tb_env->decr_freq = freq;
- /* There is a bug in Linux 2.4 kernels:
- * if a decrementer exception is pending when it enables msr_ee at startup,
- * it's not ready to handle it...
- */
- _cpu_ppc_store_decr(cpu, 0xFFFFFFFF, 0xFFFFFFFF, 32);
- _cpu_ppc_store_hdecr(cpu, 0xFFFFFFFF, 0xFFFFFFFF, 32);
- cpu_ppc_store_purr(env, 0x0000000000000000ULL);
-}
-
static void timebase_save(PPCTimebase *tb)
{
uint64_t ticks = cpu_get_host_ticks();
@@ -1056,7 +1039,7 @@ const VMStateDescription vmstate_ppc_timebase = {
};
/* Set up (once) timebase frequency (in Hz) */
-clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq)
+void cpu_ppc_tb_init(CPUPPCState *env, uint32_t freq)
{
PowerPCCPU *cpu = env_archcpu(env);
ppc_tb_t *tb_env;
@@ -1076,9 +1059,33 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq)
} else {
tb_env->hdecr_timer = NULL;
}
- cpu_ppc_set_tb_clk(env, freq);
- return &cpu_ppc_set_tb_clk;
+ tb_env->tb_freq = freq;
+ tb_env->decr_freq = freq;
+}
+
+void cpu_ppc_tb_reset(CPUPPCState *env)
+{
+ PowerPCCPU *cpu = env_archcpu(env);
+ ppc_tb_t *tb_env = env->tb_env;
+
+ timer_del(tb_env->decr_timer);
+ ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 0);
+ tb_env->decr_next = 0;
+ if (tb_env->hdecr_timer != NULL) {
+ timer_del(tb_env->hdecr_timer);
+ ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 0);
+ tb_env->hdecr_next = 0;
+ }
+
+ /*
+ * There is a bug in Linux 2.4 kernels:
+ * if a decrementer exception is pending when it enables msr_ee at startup,
+ * it's not ready to handle it...
+ */
+ cpu_ppc_store_decr(env, -1);
+ cpu_ppc_store_hdecr(env, -1);
+ cpu_ppc_store_purr(env, 0x0000000000000000ULL);
}
void cpu_ppc_tb_free(CPUPPCState *env)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index d9231c7317..f6fd35fcb9 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -67,6 +67,7 @@ static void ppc_prep_reset(void *opaque)
PowerPCCPU *cpu = opaque;
cpu_reset(CPU(cpu));
+ cpu_ppc_tb_reset(&cpu->env);
}
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index b482d9754a..91fae56573 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -74,6 +74,8 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
kvm_check_mmu(cpu, &error_fatal);
+ cpu_ppc_tb_reset(env);
+
spapr_irq_cpu_intc_reset(spapr, cpu);
}
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index e095c002dc..17a8dfc107 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -54,7 +54,8 @@ struct ppc_tb_t {
*/
uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
-clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+void cpu_ppc_tb_init(CPUPPCState *env, uint32_t freq);
+void cpu_ppc_tb_reset(CPUPPCState *env);
void cpu_ppc_tb_free(CPUPPCState *env);
void cpu_ppc_hdecr_init(CPUPPCState *env);
void cpu_ppc_hdecr_exit(CPUPPCState *env);
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 08/45] hw/ppc: Read time only once to perform decrementer write
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (6 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 07/45] hw/ppc: Reset timebase facilities on machine reset Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 09/45] linux-user/hppa: clear the PSW 'N' bit when delivering signals Michael Tokarev
` (37 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Cédric Le Goater,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
Reading the time more than once to perform an operation always increases
complexity and fragility due to introduced deltas. Simplify the
decrementer write by reading the clock once for the operation.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit ea62f8a5172cf5fcd97df143b758730f6865a625)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 8d66b8618a..28a661cba9 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -704,13 +704,13 @@ bool ppc_decr_clear_on_delivery(CPUPPCState *env)
return ((tb_env->flags & flags) == PPC_DECR_UNDERFLOW_TRIGGERED);
}
-static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
+static inline int64_t __cpu_ppc_load_decr(CPUPPCState *env, int64_t now,
+ uint64_t next)
{
ppc_tb_t *tb_env = env->tb_env;
- uint64_t now, n;
+ uint64_t n;
int64_t decr;
- now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
n = ns_to_tb(tb_env->decr_freq, now);
if (next > n && tb_env->flags & PPC_TIMER_BOOKE) {
decr = 0;
@@ -723,16 +723,12 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
return decr;
}
-target_ulong cpu_ppc_load_decr(CPUPPCState *env)
+static target_ulong _cpu_ppc_load_decr(CPUPPCState *env, int64_t now)
{
ppc_tb_t *tb_env = env->tb_env;
uint64_t decr;
- if (kvm_enabled()) {
- return env->spr[SPR_DECR];
- }
-
- decr = _cpu_ppc_load_decr(env, tb_env->decr_next);
+ decr = __cpu_ppc_load_decr(env, now, tb_env->decr_next);
/*
* If large decrementer is enabled then the decrementer is signed extened
@@ -746,14 +742,23 @@ target_ulong cpu_ppc_load_decr(CPUPPCState *env)
return (uint32_t) decr;
}
-target_ulong cpu_ppc_load_hdecr(CPUPPCState *env)
+target_ulong cpu_ppc_load_decr(CPUPPCState *env)
+{
+ if (kvm_enabled()) {
+ return env->spr[SPR_DECR];
+ } else {
+ return _cpu_ppc_load_decr(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ }
+}
+
+static target_ulong _cpu_ppc_load_hdecr(CPUPPCState *env, int64_t now)
{
PowerPCCPU *cpu = env_archcpu(env);
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
ppc_tb_t *tb_env = env->tb_env;
uint64_t hdecr;
- hdecr = _cpu_ppc_load_decr(env, tb_env->hdecr_next);
+ hdecr = __cpu_ppc_load_decr(env, now, tb_env->hdecr_next);
/*
* If we have a large decrementer (POWER9 or later) then hdecr is sign
@@ -765,6 +770,11 @@ target_ulong cpu_ppc_load_hdecr(CPUPPCState *env)
return (uint32_t) hdecr;
}
+target_ulong cpu_ppc_load_hdecr(CPUPPCState *env)
+{
+ return _cpu_ppc_load_hdecr(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+}
+
uint64_t cpu_ppc_load_purr (CPUPPCState *env)
{
ppc_tb_t *tb_env = env->tb_env;
@@ -809,7 +819,7 @@ static inline void cpu_ppc_hdecr_lower(PowerPCCPU *cpu)
ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 0);
}
-static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
+static void __cpu_ppc_store_decr(PowerPCCPU *cpu, int64_t now, uint64_t *nextp,
QEMUTimer *timer,
void (*raise_excp)(void *),
void (*lower_excp)(PowerPCCPU *),
@@ -818,7 +828,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
{
CPUPPCState *env = &cpu->env;
ppc_tb_t *tb_env = env->tb_env;
- uint64_t now, next;
+ uint64_t next;
int64_t signed_value;
int64_t signed_decr;
@@ -830,18 +840,12 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
trace_ppc_decr_store(nr_bits, decr, value);
- if (kvm_enabled()) {
- /* KVM handles decrementer exceptions, we don't need our own timer */
- return;
- }
-
/*
* Calculate the next decrementer event and set a timer.
* decr_next is in timebase units to keep rounding simple. Note it is
* not adjusted by tb_offset because if TB changes via tb_offset changing,
* decrementer does not change, so not directly comparable with TB.
*/
- now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
next = ns_to_tb(tb_env->decr_freq, now) + value;
*nextp = next; /* nextp is in timebase units */
@@ -870,12 +874,13 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
timer_mod(timer, tb_to_ns_round_up(tb_env->decr_freq, next));
}
-static inline void _cpu_ppc_store_decr(PowerPCCPU *cpu, target_ulong decr,
- target_ulong value, int nr_bits)
+static inline void _cpu_ppc_store_decr(PowerPCCPU *cpu, int64_t now,
+ target_ulong decr, target_ulong value,
+ int nr_bits)
{
ppc_tb_t *tb_env = cpu->env.tb_env;
- __cpu_ppc_store_decr(cpu, &tb_env->decr_next, tb_env->decr_timer,
+ __cpu_ppc_store_decr(cpu, now, &tb_env->decr_next, tb_env->decr_timer,
tb_env->decr_timer->cb, &cpu_ppc_decr_lower,
tb_env->flags, decr, value, nr_bits);
}
@@ -884,13 +889,22 @@ void cpu_ppc_store_decr(CPUPPCState *env, target_ulong value)
{
PowerPCCPU *cpu = env_archcpu(env);
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ int64_t now;
+ target_ulong decr;
int nr_bits = 32;
+ if (kvm_enabled()) {
+ /* KVM handles decrementer exceptions, we don't need our own timer */
+ return;
+ }
+
if (env->spr[SPR_LPCR] & LPCR_LD) {
nr_bits = pcc->lrg_decr_bits;
}
- _cpu_ppc_store_decr(cpu, cpu_ppc_load_decr(env), value, nr_bits);
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ decr = _cpu_ppc_load_decr(env, now);
+ _cpu_ppc_store_decr(cpu, now, decr, value, nr_bits);
}
static void cpu_ppc_decr_cb(void *opaque)
@@ -900,14 +914,15 @@ static void cpu_ppc_decr_cb(void *opaque)
cpu_ppc_decr_excp(cpu);
}
-static inline void _cpu_ppc_store_hdecr(PowerPCCPU *cpu, target_ulong hdecr,
- target_ulong value, int nr_bits)
+static inline void _cpu_ppc_store_hdecr(PowerPCCPU *cpu, int64_t now,
+ target_ulong hdecr, target_ulong value,
+ int nr_bits)
{
ppc_tb_t *tb_env = cpu->env.tb_env;
if (tb_env->hdecr_timer != NULL) {
/* HDECR (Book3S 64bit) is edge-based, not level like DECR */
- __cpu_ppc_store_decr(cpu, &tb_env->hdecr_next, tb_env->hdecr_timer,
+ __cpu_ppc_store_decr(cpu, now, &tb_env->hdecr_next, tb_env->hdecr_timer,
tb_env->hdecr_timer->cb, &cpu_ppc_hdecr_lower,
PPC_DECR_UNDERFLOW_TRIGGERED,
hdecr, value, nr_bits);
@@ -918,9 +933,12 @@ void cpu_ppc_store_hdecr(CPUPPCState *env, target_ulong value)
{
PowerPCCPU *cpu = env_archcpu(env);
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ int64_t now;
+ target_ulong hdecr;
- _cpu_ppc_store_hdecr(cpu, cpu_ppc_load_hdecr(env), value,
- pcc->lrg_decr_bits);
+ now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ hdecr = _cpu_ppc_load_hdecr(env, now);
+ _cpu_ppc_store_hdecr(cpu, now, hdecr, value, pcc->lrg_decr_bits);
}
static void cpu_ppc_hdecr_cb(void *opaque)
@@ -930,12 +948,16 @@ static void cpu_ppc_hdecr_cb(void *opaque)
cpu_ppc_hdecr_excp(cpu);
}
-void cpu_ppc_store_purr(CPUPPCState *env, uint64_t value)
+static void _cpu_ppc_store_purr(CPUPPCState *env, int64_t now, uint64_t value)
{
ppc_tb_t *tb_env = env->tb_env;
- cpu_ppc_store_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
- &tb_env->purr_offset, value);
+ cpu_ppc_store_tb(tb_env, now, &tb_env->purr_offset, value);
+}
+
+void cpu_ppc_store_purr(CPUPPCState *env, uint64_t value)
+{
+ _cpu_ppc_store_purr(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), value);
}
static void timebase_save(PPCTimebase *tb)
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 09/45] linux-user/hppa: clear the PSW 'N' bit when delivering signals
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (7 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 08/45] hw/ppc: Read time only once to perform decrementer write Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 10/45] linux-user/hppa: lock both words of function descriptor Michael Tokarev
` (36 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Mikulas Patocka, Helge Deller, Michael Tokarev
From: Mikulas Patocka <mpatocka@redhat.com>
qemu-hppa may crash when delivering a signal. It can be demonstrated with
this program. Compile the program with "hppa-linux-gnu-gcc -O2 signal.c"
and run it with "qemu-hppa -one-insn-per-tb a.out". It reports that the
address of the flag is 0xb4 and it crashes when attempting to touch it.
#include <stdio.h>
#include <stdlib.h>
#include <sys/time.h>
#include <signal.h>
sig_atomic_t flag;
void sig(int n)
{
printf("&flag: %p\n", &flag);
flag = 1;
}
int main(void)
{
struct sigaction sa;
struct itimerval it;
sa.sa_handler = sig;
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESTART;
if (sigaction(SIGALRM, &sa, NULL)) perror("sigaction"), exit(1);
it.it_interval.tv_sec = 0;
it.it_interval.tv_usec = 100;
it.it_value.tv_sec = it.it_interval.tv_sec;
it.it_value.tv_usec = it.it_interval.tv_usec;
if (setitimer(ITIMER_REAL, &it, NULL)) perror("setitimer"), exit(1);
while (1) {
}
}
The reason for the crash is that the signal handling routine doesn't clear
the 'N' flag in the PSW. If the signal interrupts a thread when the 'N'
flag is set, the flag remains set at the beginning of the signal handler
and the first instruction of the signal handler is skipped.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Acked-by: Helge Deller <deller@gmx.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Helge Deller <deller@gmx.de>
(cherry picked from commit 2529497cb6b298e732e8dbe5212da7925240b4f4)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/linux-user/hppa/signal.c b/linux-user/hppa/signal.c
index f253a15864..3a976ac693 100644
--- a/linux-user/hppa/signal.c
+++ b/linux-user/hppa/signal.c
@@ -159,6 +159,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
}
env->iaoq_f = haddr;
env->iaoq_b = haddr + 4;
+ env->psw_n = 0;
return;
give_sigsegv:
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 10/45] linux-user/hppa: lock both words of function descriptor
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (8 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 09/45] linux-user/hppa: clear the PSW 'N' bit when delivering signals Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 11/45] hw/cxl: Fix CFMW config memory leak Michael Tokarev
` (35 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Mikulas Patocka, Helge Deller, Michael Tokarev
From: Mikulas Patocka <mpatocka@redhat.com>
The code in setup_rt_frame reads two words at haddr, but locks only one.
This patch fixes it to lock both.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Acked-by: Helge Deller <deller@gmx.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Helge Deller <deller@gmx.de>
(cherry picked from commit 5b1270ef1477bb7f240c3bfe2cd8b0fe4721fd51)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/linux-user/hppa/signal.c b/linux-user/hppa/signal.c
index 3a976ac693..bda6e54655 100644
--- a/linux-user/hppa/signal.c
+++ b/linux-user/hppa/signal.c
@@ -149,12 +149,13 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
target_ulong *fdesc, dest;
haddr &= -4;
- if (!lock_user_struct(VERIFY_READ, fdesc, haddr, 1)) {
+ fdesc = lock_user(VERIFY_READ, haddr, 2 * sizeof(target_ulong), 1);
+ if (!fdesc) {
goto give_sigsegv;
}
__get_user(dest, fdesc);
__get_user(env->gr[19], fdesc + 1);
- unlock_user_struct(fdesc, haddr, 1);
+ unlock_user(fdesc, haddr, 0);
haddr = dest;
}
env->iaoq_f = haddr;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 11/45] hw/cxl: Fix CFMW config memory leak
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (9 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 10/45] linux-user/hppa: lock both words of function descriptor Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 12/45] hw/cxl: Fix out of bound array access Michael Tokarev
` (34 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Li Zhijian, Philippe Mathieu-Daudé,
Jonathan Cameron, Fan Ni, Michael Tokarev
From: Li Zhijian <lizhijian@cn.fujitsu.com>
Allocate targets and targets[n] resources when all sanity checks are
passed to avoid memory leaks.
Cc: qemu-stable@nongnu.org
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(cherry picked from commit 7b165fa164022b756c2b001d0a1525f98199d3ac)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 034c7805b3..f0920da956 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -39,12 +39,6 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state,
return;
}
- fw->targets = g_malloc0_n(fw->num_targets, sizeof(*fw->targets));
- for (i = 0, target = object->targets; target; i++, target = target->next) {
- /* This link cannot be resolved yet, so stash the name for now */
- fw->targets[i] = g_strdup(target->value);
- }
-
if (object->size % (256 * MiB)) {
error_setg(errp,
"Size of a CXL fixed memory window must be a multiple of 256MiB");
@@ -64,6 +58,12 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state,
fw->enc_int_gran = 0;
}
+ fw->targets = g_malloc0_n(fw->num_targets, sizeof(*fw->targets));
+ for (i = 0, target = object->targets; target; i++, target = target->next) {
+ /* This link cannot be resolved yet, so stash the name for now */
+ fw->targets[i] = g_strdup(target->value);
+ }
+
cxl_state->fixed_windows = g_list_append(cxl_state->fixed_windows,
g_steal_pointer(&fw));
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 12/45] hw/cxl: Fix out of bound array access
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (10 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 11/45] hw/cxl: Fix CFMW config memory leak Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 13/45] file-posix: Clear bs->bl.zoned on error Michael Tokarev
` (33 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Dmitry Frolov, Michael Tokarev, Jonathan Cameron
From: Dmitry Frolov <frolov@swemel.ru>
According to cxl_interleave_ways_enc(), fw->num_targets is allowed to be up
to 16. This also corresponds to CXL r3.0 spec. So, the fw->target_hbs[]
array is iterated from 0 to 15. But it is statically declared of length 8.
Thus, out of bound array access may occur.
Fixes: c28db9e000 ("hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from TYPE_PXB_DEV")
Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Link: https://lore.kernel.org/r/20230913101055.754709-1-frolov@swemel.ru
Cc: qemu-stable@nongnu.org
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(cherry picked from commit de5bbfc602ef1b9b79c494a914c6083a1a23cca2)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 56c9e7676e..4944725849 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -29,7 +29,7 @@ typedef struct PXBCXLDev PXBCXLDev;
typedef struct CXLFixedWindow {
uint64_t size;
char **targets;
- PXBCXLDev *target_hbs[8];
+ PXBCXLDev *target_hbs[16];
uint8_t num_targets;
uint8_t enc_int_ways;
uint8_t enc_int_gran;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 13/45] file-posix: Clear bs->bl.zoned on error
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (11 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 12/45] hw/cxl: Fix out of bound array access Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 14/45] file-posix: Check bs->bl.zoned for zone info Michael Tokarev
` (32 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Hanna Czenczek, Sam Li, Michael Tokarev
From: Hanna Czenczek <hreitz@redhat.com>
bs->bl.zoned is what indicates whether the zone information is present
and valid; it is the only thing that raw_refresh_zoned_limits() sets if
CONFIG_BLKZONED is not defined, and it is also the only thing that it
sets if CONFIG_BLKZONED is defined, but there are no zones.
Make sure that it is always set to BLK_Z_NONE if there is an error
anywhere in raw_refresh_zoned_limits() so that we do not accidentally
announce zones while our information is incomplete or invalid.
This also fixes a memory leak in the last error path in
raw_refresh_zoned_limits().
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-2-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
(cherry picked from commit 56d1a022a77ea2125564913665eeadf3e303a671)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/file-posix.c b/block/file-posix.c
index b16e9c21a1..2b88b9eefa 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1412,11 +1412,9 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
BlockZoneModel zoned;
int ret;
- bs->bl.zoned = BLK_Z_NONE;
-
ret = get_sysfs_zoned_model(st, &zoned);
if (ret < 0 || zoned == BLK_Z_NONE) {
- return;
+ goto no_zoned;
}
bs->bl.zoned = zoned;
@@ -1437,10 +1435,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
if (ret < 0) {
error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
"sysfs attribute");
- return;
+ goto no_zoned;
} else if (!ret) {
error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
- return;
+ goto no_zoned;
}
bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
@@ -1448,10 +1446,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
if (ret < 0) {
error_setg_errno(errp, -ret, "Unable to read nr_zones "
"sysfs attribute");
- return;
+ goto no_zoned;
} else if (!ret) {
error_setg(errp, "Read 0 from nr_zones sysfs attribute");
- return;
+ goto no_zoned;
}
bs->bl.nr_zones = ret;
@@ -1472,10 +1470,15 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
if (ret < 0) {
error_setg_errno(errp, -ret, "report wps failed");
- bs->wps = NULL;
- return;
+ goto no_zoned;
}
qemu_co_mutex_init(&bs->wps->colock);
+ return;
+
+no_zoned:
+ bs->bl.zoned = BLK_Z_NONE;
+ g_free(bs->wps);
+ bs->wps = NULL;
}
#else /* !defined(CONFIG_BLKZONED) */
static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 14/45] file-posix: Check bs->bl.zoned for zone info
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (12 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 13/45] file-posix: Clear bs->bl.zoned on error Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 15/45] file-posix: Fix zone update in I/O error path Michael Tokarev
` (31 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Hanna Czenczek, Sam Li, Michael Tokarev
From: Hanna Czenczek <hreitz@redhat.com>
Instead of checking bs->wps or bs->bl.zone_size for whether zone
information is present, check bs->bl.zoned. That is the flag that
raw_refresh_zoned_limits() reliably sets to indicate zone support. If
it is set to something other than BLK_Z_NONE, other values and objects
like bs->wps and bs->bl.zone_size must be non-null/zero and valid; if it
is not, we cannot rely on their validity.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-3-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
(cherry picked from commit 4b5d80f3d02096a9bb1f651f6b3401ba40877159)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/file-posix.c b/block/file-posix.c
index 2b88b9eefa..46e22403fe 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2455,9 +2455,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
if (fd_open(bs) < 0)
return -EIO;
#if defined(CONFIG_BLKZONED)
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && bs->wps) {
+ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+ bs->bl.zoned != BLK_Z_NONE) {
qemu_co_mutex_lock(&bs->wps->colock);
- if (type & QEMU_AIO_ZONE_APPEND && bs->bl.zone_size) {
+ if (type & QEMU_AIO_ZONE_APPEND) {
int index = offset / bs->bl.zone_size;
offset = bs->wps->wp[index];
}
@@ -2508,8 +2509,8 @@ out:
{
BlockZoneWps *wps = bs->wps;
if (ret == 0) {
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
- && wps && bs->bl.zone_size) {
+ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+ bs->bl.zoned != BLK_Z_NONE) {
uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
if (!BDRV_ZT_IS_CONV(*wp)) {
if (type & QEMU_AIO_ZONE_APPEND) {
@@ -2529,7 +2530,8 @@ out:
}
}
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
+ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+ bs->blk.zoned != BLK_Z_NONE) {
qemu_co_mutex_unlock(&wps->colock);
}
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 15/45] file-posix: Fix zone update in I/O error path
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (13 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 14/45] file-posix: Check bs->bl.zoned for zone info Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 16/45] file-posix: Simplify raw_co_prw's 'out' zone code Michael Tokarev
` (30 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Hanna Czenczek, Sam Li, Michael Tokarev
From: Hanna Czenczek <hreitz@redhat.com>
We must check that zone information is present before running
update_zones_wp().
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
Fixes: Coverity CID 1512459
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-4-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
(cherry picked from commit deab5c9a4ed74f76a713008a42527762b30a7e84)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/file-posix.c b/block/file-posix.c
index 46e22403fe..a050682e97 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2525,7 +2525,8 @@ out:
}
}
} else {
- if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
+ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+ bs->bl.zoned != BLK_Z_NONE) {
update_zones_wp(bs, s->fd, 0, 1);
}
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 16/45] file-posix: Simplify raw_co_prw's 'out' zone code
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (14 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 15/45] file-posix: Fix zone update in I/O error path Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 17/45] tests/file-io-error: New test Michael Tokarev
` (29 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Hanna Czenczek, Sam Li, Michael Tokarev
From: Hanna Czenczek <hreitz@redhat.com>
We duplicate the same condition three times here, pull it out to the top
level.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-5-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
(cherry picked from commit d31b50a15dd25a560749b25fc40b6484fd1a57b7)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/block/file-posix.c b/block/file-posix.c
index a050682e97..aa89789737 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2506,11 +2506,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
out:
#if defined(CONFIG_BLKZONED)
-{
- BlockZoneWps *wps = bs->wps;
- if (ret == 0) {
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
- bs->bl.zoned != BLK_Z_NONE) {
+ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+ bs->bl.zoned != BLK_Z_NONE) {
+ BlockZoneWps *wps = bs->wps;
+ if (ret == 0) {
uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
if (!BDRV_ZT_IS_CONV(*wp)) {
if (type & QEMU_AIO_ZONE_APPEND) {
@@ -2523,19 +2522,12 @@ out:
*wp = offset + bytes;
}
}
- }
- } else {
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
- bs->bl.zoned != BLK_Z_NONE) {
+ } else {
update_zones_wp(bs, s->fd, 0, 1);
}
- }
- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
- bs->blk.zoned != BLK_Z_NONE) {
qemu_co_mutex_unlock(&wps->colock);
}
-}
#endif
return ret;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 17/45] tests/file-io-error: New test
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (15 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 16/45] file-posix: Simplify raw_co_prw's 'out' zone code Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 18/45] include/exec: Widen tlb_hit/tlb_hit_page() Michael Tokarev
` (28 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Hanna Czenczek, Michael Tokarev
From: Hanna Czenczek <hreitz@redhat.com>
This is a regression test for
https://bugzilla.redhat.com/show_bug.cgi?id=2234374.
All this test needs to do is trigger an I/O error inside of file-posix
(specifically raw_co_prw()). One reliable way to do this without
requiring special privileges is to use a FUSE export, which allows us to
inject any error that we want, e.g. via blkdebug.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-6-hreitz@redhat.com>
[hreitz: Fixed test to be skipped when there is no FUSE support, to
suppress fusermount's allow_other warning, and to be skipped
with $IMGOPTSSYNTAX enabled]
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
(cherry picked from commit 380448464dd89291cf7fd7434be6c225482a334d)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/tests/qemu-iotests/tests/file-io-error b/tests/qemu-iotests/tests/file-io-error
new file mode 100755
index 0000000000..88ee5f670c
--- /dev/null
+++ b/tests/qemu-iotests/tests/file-io-error
@@ -0,0 +1,119 @@
+#!/usr/bin/env bash
+# group: rw
+#
+# Produce an I/O error in file-posix, and hope that it is not catastrophic.
+# Regression test for: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
+#
+# Copyright (C) 2023 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=$(basename "$0")
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_qemu
+ rm -f "$TEST_DIR/fuse-export"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ../common.rc
+. ../common.filter
+. ../common.qemu
+
+# Format-agnostic (we do not use any), but we do test the file protocol
+_supported_proto file
+_require_drivers blkdebug null-co
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+ # We need `$QEMU_IO -f file` to work; IMGOPTSSYNTAX uses --image-opts,
+ # breaking -f.
+ _unsupported_fmt $IMGFMT
+fi
+
+# This is a regression test of a bug in which flie-posix would access zone
+# information in case of an I/O error even when there is no zone information,
+# resulting in a division by zero.
+# To reproduce the problem, we need to trigger an I/O error inside of
+# file-posix, which can be done (rootless) by providing a FUSE export that
+# presents only errors when accessed.
+
+_launch_qemu
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'qmp_capabilities'}" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': 'blkdebug',
+ 'node-name': 'node0',
+ 'inject-error': [{'event': 'none'}],
+ 'image': {
+ 'driver': 'null-co'
+ }
+ }}" \
+ 'return'
+
+# FUSE mountpoint must exist and be a regular file
+touch "$TEST_DIR/fuse-export"
+
+# The grep -v to filter fusermount's (benign) error when /etc/fuse.conf does
+# not contain user_allow_other and the subsequent check for missing FUSE support
+# have both been taken from iotest 308.
+output=$(_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'block-export-add',
+ 'arguments': {
+ 'id': 'exp0',
+ 'type': 'fuse',
+ 'node-name': 'node0',
+ 'mountpoint': '$TEST_DIR/fuse-export',
+ 'writable': true
+ }}" \
+ 'return' \
+ | grep -v 'option allow_other only allowed if')
+
+if echo "$output" | grep -q "Parameter 'type' does not accept value 'fuse'"; then
+ _notrun 'No FUSE support'
+fi
+echo "$output"
+
+echo
+# This should fail, but gracefully, i.e. just print an I/O error, not crash.
+$QEMU_IO -f file -c 'write 0 64M' "$TEST_DIR/fuse-export" | _filter_qemu_io
+echo
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'block-export-del',
+ 'arguments': {'id': 'exp0'}}" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ '' \
+ 'BLOCK_EXPORT_DELETED'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-del',
+ 'arguments': {'node-name': 'node0'}}" \
+ 'return'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/tests/file-io-error.out b/tests/qemu-iotests/tests/file-io-error.out
new file mode 100644
index 0000000000..0f46455a94
--- /dev/null
+++ b/tests/qemu-iotests/tests/file-io-error.out
@@ -0,0 +1,33 @@
+QA output created by file-io-error
+{'execute': 'qmp_capabilities'}
+{"return": {}}
+{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': 'blkdebug',
+ 'node-name': 'node0',
+ 'inject-error': [{'event': 'none'}],
+ 'image': {
+ 'driver': 'null-co'
+ }
+ }}
+{"return": {}}
+{'execute': 'block-export-add',
+ 'arguments': {
+ 'id': 'exp0',
+ 'type': 'fuse',
+ 'node-name': 'node0',
+ 'mountpoint': 'TEST_DIR/fuse-export',
+ 'writable': true
+ }}
+{"return": {}}
+
+write failed: Input/output error
+
+{'execute': 'block-export-del',
+ 'arguments': {'id': 'exp0'}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "exp0"}}
+{'execute': 'blockdev-del',
+ 'arguments': {'node-name': 'node0'}}
+{"return": {}}
+*** done
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 18/45] include/exec: Widen tlb_hit/tlb_hit_page()
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (16 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 17/45] tests/file-io-error: New test Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 19/45] hw/arm/boot: Set SCR_EL3.FGTEn when booting kernel Michael Tokarev
` (27 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Anton Johansson, Richard Henderson, Michael Tokarev
From: Anton Johansson <anjo@rev.ng>
tlb_addr is changed from target_ulong to uint64_t to match the type of
a CPUTLBEntry value, and the addressed is changed to vaddr.
Signed-off-by: Anton Johansson <anjo@rev.ng>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230807155706.9580-8-anjo@rev.ng>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit c78edb563942ce80c9c6c03b07397725b006b625)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 94f44f1f59..c2c62160c6 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -397,7 +397,7 @@ QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
* @addr: virtual address to test (must be page aligned)
* @tlb_addr: TLB entry address (a CPUTLBEntry addr_read/write/code value)
*/
-static inline bool tlb_hit_page(target_ulong tlb_addr, target_ulong addr)
+static inline bool tlb_hit_page(uint64_t tlb_addr, vaddr addr)
{
return addr == (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK));
}
@@ -408,7 +408,7 @@ static inline bool tlb_hit_page(target_ulong tlb_addr, target_ulong addr)
* @addr: virtual address to test (need not be page aligned)
* @tlb_addr: TLB entry address (a CPUTLBEntry addr_read/write/code value)
*/
-static inline bool tlb_hit(target_ulong tlb_addr, target_ulong addr)
+static inline bool tlb_hit(uint64_t tlb_addr, vaddr addr)
{
return tlb_hit_page(tlb_addr, addr & TARGET_PAGE_MASK);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 19/45] hw/arm/boot: Set SCR_EL3.FGTEn when booting kernel
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (17 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 18/45] include/exec: Widen tlb_hit/tlb_hit_page() Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 20/45] target/arm: Don't skip MTE checks for LDRT/STRT at EL0 Michael Tokarev
` (26 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Fabian Vogt, Peter Maydell, Michael Tokarev
From: Fabian Vogt <fvogt@suse.de>
Just like d7ef5e16a17c sets SCR_EL3.HXEn for FEAT_HCX, this commit
handles SCR_EL3.FGTEn for FEAT_FGT:
When we direct boot a kernel on a CPU which emulates EL3, we need to
set up the EL3 system registers as the Linux kernel documentation
specifies:
https://www.kernel.org/doc/Documentation/arm64/booting.rst
> For CPUs with the Fine Grained Traps (FEAT_FGT) extension present:
> - If EL3 is present and the kernel is entered at EL2:
> - SCR_EL3.FGTEn (bit 27) must be initialised to 0b1.
Cc: qemu-stable@nongnu.org
Signed-off-by: Fabian Vogt <fvogt@suse.de>
Message-id: 4831384.GXAFRqVoOG@linux-e202.suse.de
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit 32b214384e1e1472ddfa875196c57f6620172301)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 720f22531a..24fa169060 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -761,6 +761,10 @@ static void do_cpu_reset(void *opaque)
if (cpu_isar_feature(aa64_hcx, cpu)) {
env->cp15.scr_el3 |= SCR_HXEN;
}
+ if (cpu_isar_feature(aa64_fgt, cpu)) {
+ env->cp15.scr_el3 |= SCR_FGTEN;
+ }
+
/* AArch64 kernels never boot in secure mode */
assert(!info->secure_boot);
/* This hook is only supported for AArch32 currently:
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 20/45] target/arm: Don't skip MTE checks for LDRT/STRT at EL0
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (18 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 19/45] hw/arm/boot: Set SCR_EL3.FGTEn when booting kernel Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 21/45] meson.build: Make keyutils independent from keyring Michael Tokarev
` (25 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Peter Maydell, Richard Henderson, Michael Tokarev
From: Peter Maydell <peter.maydell@linaro.org>
The LDRT/STRT "unprivileged load/store" instructions behave like
normal ones if executed at EL0. We handle this correctly for
the load/store semantics, but get the MTE checking wrong.
We always look at s->mte_active[is_unpriv] to see whether we should
be doing MTE checks, but in hflags.c when we set the TB flags that
will be used to fill the mte_active[] array we only set the
MTE0_ACTIVE bit if UNPRIV is true (i.e. we are not at EL0).
This means that a LDRT at EL0 will see s->mte_active[1] as 0,
and will not do MTE checks even when MTE is enabled.
To avoid the translate-time code having to do an explicit check on
s->unpriv to see if it is OK to index into the mte_active[] array,
duplicate MTE_ACTIVE into MTE0_ACTIVE when UNPRIV is false.
(This isn't a very serious bug because generally nobody executes
LDRT/STRT at EL0, because they have no use there.)
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20230912140434.1333369-2-peter.maydell@linaro.org
(cherry picked from commit 903dbefc2b6918c10d12d9aafa0168cee8d287c7)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 616c5fa723..ea642384f5 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -306,6 +306,15 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
&& !(env->pstate & PSTATE_TCO)
&& (sctlr & (el == 0 ? SCTLR_TCF0 : SCTLR_TCF))) {
DP_TBFLAG_A64(flags, MTE_ACTIVE, 1);
+ if (!EX_TBFLAG_A64(flags, UNPRIV)) {
+ /*
+ * In non-unpriv contexts (eg EL0), unpriv load/stores
+ * act like normal ones; duplicate the MTE info to
+ * avoid translate-a64.c having to check UNPRIV to see
+ * whether it is OK to index into MTE_ACTIVE[].
+ */
+ DP_TBFLAG_A64(flags, MTE0_ACTIVE, 1);
+ }
}
}
/* And again for unprivileged accesses, if required. */
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 21/45] meson.build: Make keyutils independent from keyring
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (19 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 20/45] target/arm: Don't skip MTE checks for LDRT/STRT at EL0 Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 22/45] accel/tcg: mttcg remove false-negative halted assertion Michael Tokarev
` (24 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Thomas Huth, Daniel P. Berrangé,
Michael Tokarev
From: Thomas Huth <thuth@redhat.com>
Commit 0db0fbb5cf ("Add conditional dependency for libkeyutils")
tried to provide a possibility for the user to disable keyutils
if not required by makeing it depend on the keyring feature. This
looked reasonable at a first glance (the unit test in tests/unit/
needs both), but the condition in meson.build fails if the feature
is meant to be detected automatically, and there is also another
spot in backends/meson.build where keyutils is used independently
from keyring. So let's remove the dependency on keyring again and
introduce a proper meson build option instead.
Cc: qemu-stable@nongnu.org
Fixes: 0db0fbb5cf ("Add conditional dependency for libkeyutils")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1842
Message-ID: <20230824094208.255279-1-thuth@redhat.com>
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
(cherry picked from commit c64023b0ba677cfa6b878e82ea8e18507a597396)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/meson.build b/meson.build
index 912c7e7847..a9c4f28247 100644
--- a/meson.build
+++ b/meson.build
@@ -1771,8 +1771,9 @@ if gnutls.found()
method: 'pkg-config')
endif
keyutils = not_found
-if get_option('keyring').enabled()
- keyutils = dependency('libkeyutils', required: false, method: 'pkg-config')
+if not get_option('libkeyutils').auto() or have_block
+ keyutils = dependency('libkeyutils', required: get_option('libkeyutils'),
+ method: 'pkg-config')
endif
has_gettid = cc.has_function('gettid')
@@ -4211,6 +4212,7 @@ endif
summary_info += {'AF_ALG support': have_afalg}
summary_info += {'rng-none': get_option('rng_none')}
summary_info += {'Linux keyring': have_keyring}
+summary_info += {'Linux keyutils': keyutils}
summary(summary_info, bool_yn: true, section: 'Crypto')
# UI
diff --git a/meson_options.txt b/meson_options.txt
index aaea5ddd77..ae6d8f469d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -119,6 +119,8 @@ option('avx512bw', type: 'feature', value: 'auto',
description: 'AVX512BW optimizations')
option('keyring', type: 'feature', value: 'auto',
description: 'Linux keyring support')
+option('libkeyutils', type: 'feature', value: 'auto',
+ description: 'Linux keyutils support')
option('attr', type : 'feature', value : 'auto',
description: 'attr/xattr support')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 9da3fe299b..d7020af175 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -120,6 +120,7 @@ meson_options_help() {
printf "%s\n" ' libdaxctl libdaxctl support'
printf "%s\n" ' libdw debuginfo support'
printf "%s\n" ' libiscsi libiscsi userspace initiator'
+ printf "%s\n" ' libkeyutils Linux keyutils support'
printf "%s\n" ' libnfs libnfs block device driver'
printf "%s\n" ' libpmem libpmem support'
printf "%s\n" ' libssh ssh block device support'
@@ -341,6 +342,8 @@ _meson_option_parse() {
--libexecdir=*) quote_sh "-Dlibexecdir=$2" ;;
--enable-libiscsi) printf "%s" -Dlibiscsi=enabled ;;
--disable-libiscsi) printf "%s" -Dlibiscsi=disabled ;;
+ --enable-libkeyutils) printf "%s" -Dlibkeyutils=enabled ;;
+ --disable-libkeyutils) printf "%s" -Dlibkeyutils=disabled ;;
--enable-libnfs) printf "%s" -Dlibnfs=enabled ;;
--disable-libnfs) printf "%s" -Dlibnfs=disabled ;;
--enable-libpmem) printf "%s" -Dlibpmem=enabled ;;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 22/45] accel/tcg: mttcg remove false-negative halted assertion
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (20 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 21/45] meson.build: Make keyutils independent from keyring Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 23/45] hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467] Michael Tokarev
` (23 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Nicholas Piggin, Ivan Warren, Richard Henderson,
Michael Tokarev
From: Nicholas Piggin <npiggin@gmail.com>
mttcg asserts that an execution ending with EXCP_HALTED must have
cpu->halted. However between the event or instruction that sets
cpu->halted and requests exit and the assertion here, an
asynchronous event could clear cpu->halted.
This leads to crashes running AIX on ppc/pseries because it uses
H_CEDE/H_PROD hcalls, where H_CEDE sets self->halted = 1 and
H_PROD sets other cpu->halted = 0 and kicks it.
H_PROD could be turned into an interrupt to wake, but several other
places in ppc, sparc, and semihosting follow what looks like a similar
pattern setting halted = 0 directly. So remove this assertion.
Reported-by: Ivan Warren <ivan@vmfacility.fr>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20230829010658.8252-1-npiggin@gmail.com>
[rth: Keep the case label and adjust the comment.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 0e5903436de712844b0e6cdd862b499c767e09e9)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index b276262007..4b0dfb4be7 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -100,14 +100,9 @@ static void *mttcg_cpu_thread_fn(void *arg)
break;
case EXCP_HALTED:
/*
- * during start-up the vCPU is reset and the thread is
- * kicked several times. If we don't ensure we go back
- * to sleep in the halted state we won't cleanly
- * start-up when the vCPU is enabled.
- *
- * cpu->halted should ensure we sleep in wait_io_event
+ * Usually cpu->halted is set, but may have already been
+ * reset by another thread by the time we arrive here.
*/
- g_assert(cpu->halted);
break;
case EXCP_ATOMIC:
qemu_mutex_unlock_iothread();
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 23/45] hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (21 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 22/45] accel/tcg: mttcg remove false-negative halted assertion Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 24/45] ui/vnc: fix debug output for invalid audio message Michael Tokarev
` (22 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Thomas Huth, Paolo Bonzini, Michael Tokarev
From: Thomas Huth <thuth@redhat.com>
We are doing things like
nb_sectors /= (s->qdev.blocksize / BDRV_SECTOR_SIZE);
in the code here (e.g. in scsi_disk_emulate_mode_sense()), so if
the blocksize is smaller than BDRV_SECTOR_SIZE (=512), this crashes
with a division by 0 exception. Thus disallow block sizes of 256
bytes to avoid this situation.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1813
CVE: 2023-42467
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230925091854.49198-1-thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 7cfcc79b0ab800959716738aff9419f53fc68c9c)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e0d79c7966..477ee2bcd4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1628,9 +1628,10 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
* Since the existing code only checks/updates bits 8-15 of the block
* size, restrict ourselves to the same requirement for now to ensure
* that a block size set by a block descriptor and then read back by
- * a subsequent SCSI command will be the same
+ * a subsequent SCSI command will be the same. Also disallow a block
+ * size of 256 since we cannot handle anything below BDRV_SECTOR_SIZE.
*/
- if (bs && !(bs & ~0xff00) && bs != s->qdev.blocksize) {
+ if (bs && !(bs & ~0xfe00) && bs != s->qdev.blocksize) {
s->qdev.blocksize = bs;
trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 24/45] ui/vnc: fix debug output for invalid audio message
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (22 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 23/45] hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467] Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 25/45] ui/vnc: fix handling of VNC_FEATURE_XVP Michael Tokarev
` (21 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Paolo Bonzini, Daniel P . Berrangé,
Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
The debug message was cut and pasted from the invalid audio format
case, but the audio message is at bytes 2-3.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 0cb9c5880e6b8dedc4e20026ce859dd1ea9aac84)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/ui/vnc.c b/ui/vnc.c
index 92964dcc0c..538581929a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2551,7 +2551,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
vs, vs->ioc, vs->as.fmt, vs->as.nchannels, vs->as.freq);
break;
default:
- VNC_DEBUG("Invalid audio message %d\n", read_u8(data, 4));
+ VNC_DEBUG("Invalid audio message %d\n", read_u8(data, 2));
vnc_client_error(vs);
break;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 25/45] ui/vnc: fix handling of VNC_FEATURE_XVP
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (23 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 24/45] ui/vnc: fix debug output for invalid audio message Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 26/45] migration: Fix race that dest preempt thread close too early Michael Tokarev
` (20 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Paolo Bonzini, Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
VNC_FEATURE_XVP was not shifted left before adding it to vs->features,
so it was never enabled; but it was also checked the wrong way with
a logical AND instead of vnc_has_feature. Fix both places.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 477b301000d665313217f65e3a368d2cb7769c42)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/ui/vnc.c b/ui/vnc.c
index 538581929a..293ba5db5f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2205,7 +2205,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
break;
case VNC_ENCODING_XVP:
if (vs->vd->power_control) {
- vs->features |= VNC_FEATURE_XVP;
+ vs->features |= VNC_FEATURE_XVP_MASK;
send_xvp_message(vs, VNC_XVP_CODE_INIT);
}
break;
@@ -2454,7 +2454,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
vnc_client_cut_text(vs, read_u32(data, 4), data + 8);
break;
case VNC_MSG_CLIENT_XVP:
- if (!(vs->features & VNC_FEATURE_XVP)) {
+ if (!vnc_has_feature(vs, VNC_FEATURE_XVP)) {
error_report("vnc: xvp client message while disabled");
vnc_client_error(vs);
break;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 26/45] migration: Fix race that dest preempt thread close too early
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (24 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 25/45] ui/vnc: fix handling of VNC_FEATURE_XVP Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 27/45] migration: Fix possible race when setting rp_state.error Michael Tokarev
` (19 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Peter Xu, Fabiano Rosas, Stefan Hajnoczi,
Michael Tokarev
From: Peter Xu <peterx@redhat.com>
We hit intermit CI issue on failing at migration-test over the unit test
preempt/plain:
qemu-system-x86_64: Unable to read from socket: Connection reset by peer
Memory content inconsistency at 5b43000 first_byte = bd last_byte = bc current = 4f hit_edge = 1
**
ERROR:../tests/qtest/migration-test.c:300:check_guests_ram: assertion failed: (bad == 0)
(test program exited with status code -6)
Fabiano debugged into it and found that the preempt thread can quit even
without receiving all the pages, which can cause guest not receiving all
the pages and corrupt the guest memory.
To make sure preempt thread finished receiving all the pages, we can rely
on the page_requested_count being zero because preempt channel will only
receive requested page faults. Note, not all the faulted pages are required
to be sent via the preempt channel/thread; imagine the case when a
requested page is just queued into the background main channel for
migration, the src qemu will just still send it via the background channel.
Here instead of spinning over reading the count, we add a condvar so the
main thread can wait on it if that unusual case happened, without burning
the cpu for no good reason, even if the duration is short; so even if we
spin in this rare case is probably fine. It's just better to not do so.
The condvar is only used when that special case is triggered. Some memory
ordering trick is needed to guarantee it from happening (against the
preempt thread status field), so the main thread will always get a kick
when that triggers correctly.
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1886
Debugged-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-2-farosas@suse.de>
(cherry picked from commit cf02f29e1e3843784630d04783e372fa541a77e5)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index 5528acb65e..6a7122694a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -153,6 +153,7 @@ void migration_object_init(void)
qemu_sem_init(¤t_incoming->postcopy_qemufile_dst_done, 0);
qemu_mutex_init(¤t_incoming->page_request_mutex);
+ qemu_cond_init(¤t_incoming->page_request_cond);
current_incoming->page_requested = g_tree_new(page_request_addr_cmp);
migration_object_check(current_migration, &error_fatal);
@@ -367,7 +368,7 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
* things like g_tree_lookup() will return TRUE (1) when found.
*/
g_tree_insert(mis->page_requested, aligned, (gpointer)1);
- mis->page_requested_count++;
+ qatomic_inc(&mis->page_requested_count);
trace_postcopy_page_req_add(aligned, mis->page_requested_count);
}
}
diff --git a/migration/migration.h b/migration/migration.h
index 6eea18db36..9a30216895 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -196,7 +196,10 @@ struct MigrationIncomingState {
/* A tree of pages that we requested to the source VM */
GTree *page_requested;
- /* For debugging purpose only, but would be nice to keep */
+ /*
+ * For postcopy only, count the number of requested page faults that
+ * still haven't been resolved.
+ */
int page_requested_count;
/*
* The mutex helps to maintain the requested pages that we sent to the
@@ -210,6 +213,14 @@ struct MigrationIncomingState {
* contains valid information.
*/
QemuMutex page_request_mutex;
+ /*
+ * If postcopy preempt is enabled, there is a chance that the main
+ * thread finished loading its data before the preempt channel has
+ * finished loading the urgent pages. If that happens, the two threads
+ * will use this condvar to synchronize, so the main thread will always
+ * wait until all pages received.
+ */
+ QemuCond page_request_cond;
/*
* Number of devices that have yet to approve switchover. When this reaches
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 29aea9456d..5408e028c6 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -599,6 +599,30 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
if (mis->preempt_thread_status == PREEMPT_THREAD_CREATED) {
/* Notify the fast load thread to quit */
mis->preempt_thread_status = PREEMPT_THREAD_QUIT;
+ /*
+ * Update preempt_thread_status before reading count. Note: mutex
+ * lock only provide ACQUIRE semantic, and it doesn't stops this
+ * write to be reordered after reading the count.
+ */
+ smp_mb();
+ /*
+ * It's possible that the preempt thread is still handling the last
+ * pages to arrive which were requested by guest page faults.
+ * Making sure nothing is left behind by waiting on the condvar if
+ * that unlikely case happened.
+ */
+ WITH_QEMU_LOCK_GUARD(&mis->page_request_mutex) {
+ if (qatomic_read(&mis->page_requested_count)) {
+ /*
+ * It is guaranteed to receive a signal later, because the
+ * count>0 now, so it's destined to be decreased to zero
+ * very soon by the preempt thread.
+ */
+ qemu_cond_wait(&mis->page_request_cond,
+ &mis->page_request_mutex);
+ }
+ }
+ /* Notify the fast load thread to quit */
if (mis->postcopy_qemufile_dst) {
qemu_file_shutdown(mis->postcopy_qemufile_dst);
}
@@ -1277,8 +1301,20 @@ static int qemu_ufd_copy_ioctl(MigrationIncomingState *mis, void *host_addr,
*/
if (g_tree_lookup(mis->page_requested, host_addr)) {
g_tree_remove(mis->page_requested, host_addr);
- mis->page_requested_count--;
+ int left_pages = qatomic_dec_fetch(&mis->page_requested_count);
+
trace_postcopy_page_req_del(host_addr, mis->page_requested_count);
+ /* Order the update of count and read of preempt status */
+ smp_mb();
+ if (mis->preempt_thread_status == PREEMPT_THREAD_QUIT &&
+ left_pages == 0) {
+ /*
+ * This probably means the main thread is waiting for us.
+ * Notify that we've finished receiving the last requested
+ * page.
+ */
+ qemu_cond_signal(&mis->page_request_cond);
+ }
}
qemu_mutex_unlock(&mis->page_request_mutex);
mark_postcopy_blocktime_end((uintptr_t)host_addr);
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 27/45] migration: Fix possible race when setting rp_state.error
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (25 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 26/45] migration: Fix race that dest preempt thread close too early Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 28/45] migration: Fix possible races when shutting down the return path Michael Tokarev
` (18 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
We don't need to set the rp_state.error right after a shutdown because
qemu_file_shutdown() always sets the QEMUFile error, so the return
path thread would have seen it and set the rp error itself.
Setting the error outside of the thread is also racy because the
thread could clear it after we set it.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-3-farosas@suse.de>
(cherry picked from commit 28a8347281e24c2e7bba6d3301472eda41d4c096)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index 6a7122694a..b92c6ae436 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2063,7 +2063,6 @@ static int await_return_path_close_on_source(MigrationState *ms)
* waiting for the destination.
*/
qemu_file_shutdown(ms->rp_state.from_dst_file);
- mark_source_rp_bad(ms);
}
trace_await_return_path_close_on_source_joining();
qemu_thread_join(&ms->rp_state.rp_thread);
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 28/45] migration: Fix possible races when shutting down the return path
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (26 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 27/45] migration: Fix possible race when setting rp_state.error Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 29/45] migration: Fix possible race when shutting down to_dst_file Michael Tokarev
` (17 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
We cannot call qemu_file_shutdown() on the return path file without
taking the file lock. The return path thread could be running it's
cleanup code and have just cleared the from_dst_file pointer.
Checking ms->to_dst_file for errors could also race with
migrate_fd_cleanup() which clears the to_dst_file pointer.
Protect both accesses by taking the file lock.
This was caught by inspection, it should be rare, but the next patches
will start calling this code from other places, so let's do the
correct thing.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-4-farosas@suse.de>
(cherry picked from commit 639decf529793fc544c8055b82be8abe77fa48fa)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index b92c6ae436..517b3e04d2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2053,17 +2053,18 @@ static int open_return_path_on_source(MigrationState *ms,
static int await_return_path_close_on_source(MigrationState *ms)
{
/*
- * If this is a normal exit then the destination will send a SHUT and the
- * rp_thread will exit, however if there's an error we need to cause
- * it to exit.
+ * If this is a normal exit then the destination will send a SHUT
+ * and the rp_thread will exit, however if there's an error we
+ * need to cause it to exit. shutdown(2), if we have it, will
+ * cause it to unblock if it's stuck waiting for the destination.
*/
- if (qemu_file_get_error(ms->to_dst_file) && ms->rp_state.from_dst_file) {
- /*
- * shutdown(2), if we have it, will cause it to unblock if it's stuck
- * waiting for the destination.
- */
- qemu_file_shutdown(ms->rp_state.from_dst_file);
+ WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) {
+ if (ms->to_dst_file && ms->rp_state.from_dst_file &&
+ qemu_file_get_error(ms->to_dst_file)) {
+ qemu_file_shutdown(ms->rp_state.from_dst_file);
+ }
}
+
trace_await_return_path_close_on_source_joining();
qemu_thread_join(&ms->rp_state.rp_thread);
ms->rp_state.rp_thread_created = false;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 29/45] migration: Fix possible race when shutting down to_dst_file
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (27 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 28/45] migration: Fix possible races when shutting down the return path Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 30/45] migration: Remove redundant cleanup of postcopy_qemufile_src Michael Tokarev
` (16 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
It's not safe to call qemu_file_shutdown() on the to_dst_file without
first checking for the file's presence under the lock. The cleanup of
this file happens at postcopy_pause() and migrate_fd_cleanup() which
are not necessarily running in the same thread as migrate_fd_cancel().
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-5-farosas@suse.de>
(cherry picked from commit 7478fb0df914f0a5ab551ff74b1df62dd250500e)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index 517b3e04d2..169e6bdce8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1246,7 +1246,7 @@ static void migrate_fd_error(MigrationState *s, const Error *error)
static void migrate_fd_cancel(MigrationState *s)
{
int old_state ;
- QEMUFile *f = migrate_get_current()->to_dst_file;
+
trace_migrate_fd_cancel();
WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
@@ -1272,11 +1272,13 @@ static void migrate_fd_cancel(MigrationState *s)
* If we're unlucky the migration code might be stuck somewhere in a
* send/write while the network has failed and is waiting to timeout;
* if we've got shutdown(2) available then we can force it to quit.
- * The outgoing qemu file gets closed in migrate_fd_cleanup that is
- * called in a bh, so there is no race against this cancel.
*/
- if (s->state == MIGRATION_STATUS_CANCELLING && f) {
- qemu_file_shutdown(f);
+ if (s->state == MIGRATION_STATUS_CANCELLING) {
+ WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
+ if (s->to_dst_file) {
+ qemu_file_shutdown(s->to_dst_file);
+ }
+ }
}
if (s->state == MIGRATION_STATUS_CANCELLING && s->block_inactive) {
Error *local_err = NULL;
@@ -1520,12 +1522,14 @@ void qmp_migrate_pause(Error **errp)
{
MigrationState *ms = migrate_get_current();
MigrationIncomingState *mis = migration_incoming_get_current();
- int ret;
+ int ret = 0;
if (ms->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
/* Source side, during postcopy */
qemu_mutex_lock(&ms->qemu_file_lock);
- ret = qemu_file_shutdown(ms->to_dst_file);
+ if (ms->to_dst_file) {
+ ret = qemu_file_shutdown(ms->to_dst_file);
+ }
qemu_mutex_unlock(&ms->qemu_file_lock);
if (ret) {
error_setg(errp, "Failed to pause source migration");
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 30/45] migration: Remove redundant cleanup of postcopy_qemufile_src
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (28 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 29/45] migration: Fix possible race when shutting down to_dst_file Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 31/45] migration: Consolidate return path closing code Michael Tokarev
` (15 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
This file is owned by the return path thread which is already doing
cleanup.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-6-farosas@suse.de>
(cherry picked from commit b3b101157d4651f12e6b3361af2de6bace7f9b4a)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index 169e6bdce8..e2e4a7d8ae 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1178,12 +1178,6 @@ static void migrate_fd_cleanup(MigrationState *s)
qemu_fclose(tmp);
}
- if (s->postcopy_qemufile_src) {
- migration_ioc_unregister_yank_from_file(s->postcopy_qemufile_src);
- qemu_fclose(s->postcopy_qemufile_src);
- s->postcopy_qemufile_src = NULL;
- }
-
assert(!migration_is_active(s));
if (s->state == MIGRATION_STATUS_CANCELLING) {
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 31/45] migration: Consolidate return path closing code
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (29 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 30/45] migration: Remove redundant cleanup of postcopy_qemufile_src Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 32/45] migration: Replace the return path retry logic Michael Tokarev
` (14 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
We'll start calling the await_return_path_close_on_source() function
from other parts of the code, so move all of the related checks and
tracepoints into it.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-7-farosas@suse.de>
(cherry picked from commit d50f5dc075cbb891bfe4a9378600a4871264468a)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index e2e4a7d8ae..ac4541b971 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2050,6 +2050,14 @@ static int open_return_path_on_source(MigrationState *ms,
/* Returns 0 if the RP was ok, otherwise there was an error on the RP */
static int await_return_path_close_on_source(MigrationState *ms)
{
+ int ret;
+
+ if (!ms->rp_state.rp_thread_created) {
+ return 0;
+ }
+
+ trace_migration_return_path_end_before();
+
/*
* If this is a normal exit then the destination will send a SHUT
* and the rp_thread will exit, however if there's an error we
@@ -2067,7 +2075,10 @@ static int await_return_path_close_on_source(MigrationState *ms)
qemu_thread_join(&ms->rp_state.rp_thread);
ms->rp_state.rp_thread_created = false;
trace_await_return_path_close_on_source_close();
- return ms->rp_state.error;
+
+ ret = ms->rp_state.error;
+ trace_migration_return_path_end_after(ret);
+ return ret;
}
static inline void
@@ -2363,20 +2374,8 @@ static void migration_completion(MigrationState *s)
goto fail;
}
- /*
- * If rp was opened we must clean up the thread before
- * cleaning everything else up (since if there are no failures
- * it will wait for the destination to send it's status in
- * a SHUT command).
- */
- if (s->rp_state.rp_thread_created) {
- int rp_error;
- trace_migration_return_path_end_before();
- rp_error = await_return_path_close_on_source(s);
- trace_migration_return_path_end_after(rp_error);
- if (rp_error) {
- goto fail;
- }
+ if (await_return_path_close_on_source(s)) {
+ goto fail;
}
if (qemu_file_get_error(s->to_dst_file)) {
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 32/45] migration: Replace the return path retry logic
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (30 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 31/45] migration: Consolidate return path closing code Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 33/45] migration: Move return path cleanup to main migration thread Michael Tokarev
` (13 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
Replace the return path retry logic with finishing and restarting the
thread. This fixes a race when resuming the migration that leads to a
segfault.
Currently when doing postcopy we consider that an IO error on the
return path file could be due to a network intermittency. We then keep
the thread alive but have it do cleanup of the 'from_dst_file' and
wait on the 'postcopy_pause_rp' semaphore. When the user issues a
migrate resume, a new return path is opened and the thread is allowed
to continue.
There's a race condition in the above mechanism. It is possible for
the new return path file to be setup *before* the cleanup code in the
return path thread has had a chance to run, leading to the *new* file
being closed and the pointer set to NULL. When the thread is released
after the resume, it tries to dereference 'from_dst_file' and crashes:
Thread 7 "return path" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffd1dbf700 (LWP 9611)]
0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154
154 return f->last_error;
(gdb) bt
#0 0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154
#1 0x00005555560e4983 in qemu_file_get_error (f=0x0) at ../migration/qemu-file.c:206
#2 0x0000555555b9a1df in source_return_path_thread (opaque=0x555556e06000) at ../migration/migration.c:1876
#3 0x000055555602e14f in qemu_thread_start (args=0x55555782e780) at ../util/qemu-thread-posix.c:541
#4 0x00007ffff38d76ea in start_thread (arg=0x7fffd1dbf700) at pthread_create.c:477
#5 0x00007ffff35efa6f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Here's the race (important bit is open_return_path happening before
migration_release_dst_files):
migration | qmp | return path
--------------------------+-----------------------------+---------------------------------
qmp_migrate_pause()
shutdown(ms->to_dst_file)
f->last_error = -EIO
migrate_detect_error()
postcopy_pause()
set_state(PAUSED)
wait(postcopy_pause_sem)
qmp_migrate(resume)
migrate_fd_connect()
resume = state == PAUSED
open_return_path <-- TOO SOON!
set_state(RECOVER)
post(postcopy_pause_sem)
(incoming closes to_src_file)
res = qemu_file_get_error(rp)
migration_release_dst_files()
ms->rp_state.from_dst_file = NULL
post(postcopy_pause_rp_sem)
postcopy_pause_return_path_thread()
wait(postcopy_pause_rp_sem)
rp = ms->rp_state.from_dst_file
goto retry
qemu_file_get_error(rp)
SIGSEGV
-------------------------------------------------------------------------------------------
We can keep the retry logic without having the thread alive and
waiting. The only piece of data used by it is the 'from_dst_file' and
it is only allowed to proceed after a migrate resume is issued and the
semaphore released at migrate_fd_connect().
Move the retry logic to outside the thread by waiting for the thread
to finish before pausing the migration.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-8-farosas@suse.de>
(cherry picked from commit ef796ee93b313ed2f0b427ef30320417387d2ad5)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index ac4541b971..a0782c64a1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1776,18 +1776,6 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,
}
}
-/* Return true to retry, false to quit */
-static bool postcopy_pause_return_path_thread(MigrationState *s)
-{
- trace_postcopy_pause_return_path();
-
- qemu_sem_wait(&s->postcopy_pause_rp_sem);
-
- trace_postcopy_pause_return_path_continued();
-
- return true;
-}
-
static int migrate_handle_rp_recv_bitmap(MigrationState *s, char *block_name)
{
RAMBlock *block = qemu_ram_block_by_name(block_name);
@@ -1871,7 +1859,6 @@ static void *source_return_path_thread(void *opaque)
trace_source_return_path_thread_entry();
rcu_register_thread();
-retry:
while (!ms->rp_state.error && !qemu_file_get_error(rp) &&
migration_is_setup_or_active(ms->state)) {
trace_source_return_path_thread_loop_top();
@@ -1993,26 +1980,7 @@ retry:
}
out:
- res = qemu_file_get_error(rp);
- if (res) {
- if (res && migration_in_postcopy()) {
- /*
- * Maybe there is something we can do: it looks like a
- * network down issue, and we pause for a recovery.
- */
- migration_release_dst_files(ms);
- rp = NULL;
- if (postcopy_pause_return_path_thread(ms)) {
- /*
- * Reload rp, reset the rest. Referencing it is safe since
- * it's reset only by us above, or when migration completes
- */
- rp = ms->rp_state.from_dst_file;
- ms->rp_state.error = false;
- goto retry;
- }
- }
-
+ if (qemu_file_get_error(rp)) {
trace_source_return_path_thread_bad_end();
mark_source_rp_bad(ms);
}
@@ -2023,8 +1991,7 @@ out:
return NULL;
}
-static int open_return_path_on_source(MigrationState *ms,
- bool create_thread)
+static int open_return_path_on_source(MigrationState *ms)
{
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2033,11 +2000,6 @@ static int open_return_path_on_source(MigrationState *ms,
trace_open_return_path_on_source();
- if (!create_thread) {
- /* We're done */
- return 0;
- }
-
qemu_thread_create(&ms->rp_state.rp_thread, "return path",
source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
ms->rp_state.rp_thread_created = true;
@@ -2077,6 +2039,7 @@ static int await_return_path_close_on_source(MigrationState *ms)
trace_await_return_path_close_on_source_close();
ret = ms->rp_state.error;
+ ms->rp_state.error = false;
trace_migration_return_path_end_after(ret);
return ret;
}
@@ -2552,6 +2515,13 @@ static MigThrError postcopy_pause(MigrationState *s)
qemu_file_shutdown(file);
qemu_fclose(file);
+ /*
+ * We're already pausing, so ignore any errors on the return
+ * path and just wait for the thread to finish. It will be
+ * re-created when we resume.
+ */
+ await_return_path_close_on_source(s);
+
migrate_set_state(&s->state, s->state,
MIGRATION_STATUS_POSTCOPY_PAUSED);
@@ -2569,12 +2539,6 @@ static MigThrError postcopy_pause(MigrationState *s)
if (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER) {
/* Woken up by a recover procedure. Give it a shot */
- /*
- * Firstly, let's wake up the return path now, with a new
- * return path channel.
- */
- qemu_sem_post(&s->postcopy_pause_rp_sem);
-
/* Do the resume logic */
if (postcopy_do_resume(s) == 0) {
/* Let's continue! */
@@ -3264,7 +3228,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
* QEMU uses the return path.
*/
if (migrate_postcopy_ram() || migrate_return_path()) {
- if (open_return_path_on_source(s, !resume)) {
+ if (open_return_path_on_source(s)) {
error_setg(&local_err, "Unable to open return-path for postcopy");
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
migrate_set_error(s, local_err);
@@ -3328,7 +3292,6 @@ static void migration_instance_finalize(Object *obj)
qemu_sem_destroy(&ms->rate_limit_sem);
qemu_sem_destroy(&ms->pause_sem);
qemu_sem_destroy(&ms->postcopy_pause_sem);
- qemu_sem_destroy(&ms->postcopy_pause_rp_sem);
qemu_sem_destroy(&ms->rp_state.rp_sem);
qemu_sem_destroy(&ms->rp_state.rp_pong_acks);
qemu_sem_destroy(&ms->postcopy_qemufile_src_sem);
@@ -3348,7 +3311,6 @@ static void migration_instance_init(Object *obj)
migrate_params_init(&ms->parameters);
qemu_sem_init(&ms->postcopy_pause_sem, 0);
- qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
qemu_sem_init(&ms->rp_state.rp_sem, 0);
qemu_sem_init(&ms->rp_state.rp_pong_acks, 0);
qemu_sem_init(&ms->rate_limit_sem, 0);
diff --git a/migration/migration.h b/migration/migration.h
index 9a30216895..1034d617bf 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -393,7 +393,6 @@ struct MigrationState {
/* Needed by postcopy-pause state */
QemuSemaphore postcopy_pause_sem;
- QemuSemaphore postcopy_pause_rp_sem;
/*
* Whether we abort the migration if decompression errors are
* detected at the destination. It is left at false for qemu
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 33/45] migration: Move return path cleanup to main migration thread
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (31 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 32/45] migration: Replace the return path retry logic Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 34/45] softmmu: Use async_run_on_cpu in tcg_commit Michael Tokarev
` (12 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Peter Xu, Stefan Hajnoczi,
Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
Now that the return path thread is allowed to finish during a paused
migration, we can move the cleanup of the QEMUFiles to the main
migration thread.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-9-farosas@suse.de>
(cherry picked from commit 36e9aab3c569d4c9ad780473596e18479838d1aa)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/migration/migration.c b/migration/migration.c
index a0782c64a1..7a4c8beb5d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -98,6 +98,7 @@ static int migration_maybe_pause(MigrationState *s,
int *current_active_state,
int new_state);
static void migrate_fd_cancel(MigrationState *s);
+static int await_return_path_close_on_source(MigrationState *s);
static bool migration_needs_multiple_sockets(void)
{
@@ -1178,6 +1179,12 @@ static void migrate_fd_cleanup(MigrationState *s)
qemu_fclose(tmp);
}
+ /*
+ * We already cleaned up to_dst_file, so errors from the return
+ * path might be due to that, ignore them.
+ */
+ await_return_path_close_on_source(s);
+
assert(!migration_is_active(s));
if (s->state == MIGRATION_STATUS_CANCELLING) {
@@ -1986,7 +1993,6 @@ out:
}
trace_source_return_path_thread_end();
- migration_release_dst_files(ms);
rcu_unregister_thread();
return NULL;
}
@@ -2040,6 +2046,9 @@ static int await_return_path_close_on_source(MigrationState *ms)
ret = ms->rp_state.error;
ms->rp_state.error = false;
+
+ migration_release_dst_files(ms);
+
trace_migration_return_path_end_after(ret);
return ret;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 34/45] softmmu: Use async_run_on_cpu in tcg_commit
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (32 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 33/45] migration: Move return path cleanup to main migration thread Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 35/45] accel/tcg: Avoid load of icount_decr if unused Michael Tokarev
` (11 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Alex Bennée, Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
After system startup, run the update to memory_dispatch
and the tlb_flush on the cpu. This eliminates a race,
wherein a running cpu sees the memory_dispatch change
but has not yet seen the tlb_flush.
Since the update now happens on the cpu, we need not use
qatomic_rcu_read to protect the read of memory_dispatch.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1826
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1834
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1846
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 0d58c660689f6da1e3feff8a997014003d928b3b)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index 9a5fabf625..7e35d7f4b5 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -33,36 +33,6 @@ void cpu_loop_exit_noexc(CPUState *cpu)
cpu_loop_exit(cpu);
}
-#if defined(CONFIG_SOFTMMU)
-void cpu_reloading_memory_map(void)
-{
- if (qemu_in_vcpu_thread() && current_cpu->running) {
- /* The guest can in theory prolong the RCU critical section as long
- * as it feels like. The major problem with this is that because it
- * can do multiple reconfigurations of the memory map within the
- * critical section, we could potentially accumulate an unbounded
- * collection of memory data structures awaiting reclamation.
- *
- * Because the only thing we're currently protecting with RCU is the
- * memory data structures, it's sufficient to break the critical section
- * in this callback, which we know will get called every time the
- * memory map is rearranged.
- *
- * (If we add anything else in the system that uses RCU to protect
- * its data structures, we will need to implement some other mechanism
- * to force TCG CPUs to exit the critical section, at which point this
- * part of this callback might become unnecessary.)
- *
- * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
- * only protects cpu->as->dispatch. Since we know our caller is about
- * to reload it, it's safe to split the critical section.
- */
- rcu_read_unlock();
- rcu_read_lock();
- }
-}
-#endif
-
void cpu_loop_exit(CPUState *cpu)
{
/* Undo the setting in cpu_tb_exec. */
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 87dc9a752c..41788c0bdd 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -133,7 +133,6 @@ static inline void cpu_physical_memory_write(hwaddr addr,
{
cpu_physical_memory_rw(addr, (void *)buf, len, true);
}
-void cpu_reloading_memory_map(void);
void *cpu_physical_memory_map(hwaddr addr,
hwaddr *plen,
bool is_write);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 7597dc1c39..18277ddd67 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -680,8 +680,7 @@ address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr orig_addr,
IOMMUTLBEntry iotlb;
int iommu_idx;
hwaddr addr = orig_addr;
- AddressSpaceDispatch *d =
- qatomic_rcu_read(&cpu->cpu_ases[asidx].memory_dispatch);
+ AddressSpaceDispatch *d = cpu->cpu_ases[asidx].memory_dispatch;
for (;;) {
section = address_space_translate_internal(d, addr, &addr, plen, false);
@@ -2412,7 +2411,7 @@ MemoryRegionSection *iotlb_to_section(CPUState *cpu,
{
int asidx = cpu_asidx_from_attrs(cpu, attrs);
CPUAddressSpace *cpuas = &cpu->cpu_ases[asidx];
- AddressSpaceDispatch *d = qatomic_rcu_read(&cpuas->memory_dispatch);
+ AddressSpaceDispatch *d = cpuas->memory_dispatch;
int section_index = index & ~TARGET_PAGE_MASK;
MemoryRegionSection *ret;
@@ -2487,23 +2486,42 @@ static void tcg_log_global_after_sync(MemoryListener *listener)
}
}
+static void tcg_commit_cpu(CPUState *cpu, run_on_cpu_data data)
+{
+ CPUAddressSpace *cpuas = data.host_ptr;
+
+ cpuas->memory_dispatch = address_space_to_dispatch(cpuas->as);
+ tlb_flush(cpu);
+}
+
static void tcg_commit(MemoryListener *listener)
{
CPUAddressSpace *cpuas;
- AddressSpaceDispatch *d;
+ CPUState *cpu;
assert(tcg_enabled());
/* since each CPU stores ram addresses in its TLB cache, we must
reset the modified entries */
cpuas = container_of(listener, CPUAddressSpace, tcg_as_listener);
- cpu_reloading_memory_map();
- /* The CPU and TLB are protected by the iothread lock.
- * We reload the dispatch pointer now because cpu_reloading_memory_map()
- * may have split the RCU critical section.
+ cpu = cpuas->cpu;
+
+ /*
+ * Defer changes to as->memory_dispatch until the cpu is quiescent.
+ * Otherwise we race between (1) other cpu threads and (2) ongoing
+ * i/o for the current cpu thread, with data cached by mmu_lookup().
+ *
+ * In addition, queueing the work function will kick the cpu back to
+ * the main loop, which will end the RCU critical section and reclaim
+ * the memory data structures.
+ *
+ * That said, the listener is also called during realize, before
+ * all of the tcg machinery for run-on is initialized: thus halt_cond.
*/
- d = address_space_to_dispatch(cpuas->as);
- qatomic_rcu_set(&cpuas->memory_dispatch, d);
- tlb_flush(cpuas->cpu);
+ if (cpu->halt_cond) {
+ async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_HOST_PTR(cpuas));
+ } else {
+ tcg_commit_cpu(cpu, RUN_ON_CPU_HOST_PTR(cpuas));
+ }
}
static void memory_map_init(void)
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 35/45] accel/tcg: Avoid load of icount_decr if unused
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (33 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 34/45] softmmu: Use async_run_on_cpu in tcg_commit Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 36/45] accel/tcg: Hoist CF_MEMI_ONLY check outside translation loop Michael Tokarev
` (10 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
With CF_NOIRQ and without !CF_USE_ICOUNT, the load isn't used.
Avoid emitting it.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit f47a90dacca8f74210a2675bdde7ab3856872b94)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 1a6a5448c8..a3983019a5 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -49,12 +49,15 @@ bool translator_io_start(DisasContextBase *db)
static TCGOp *gen_tb_start(uint32_t cflags)
{
- TCGv_i32 count = tcg_temp_new_i32();
+ TCGv_i32 count = NULL;
TCGOp *icount_start_insn = NULL;
- tcg_gen_ld_i32(count, cpu_env,
- offsetof(ArchCPU, neg.icount_decr.u32) -
- offsetof(ArchCPU, env));
+ if ((cflags & CF_USE_ICOUNT) || !(cflags & CF_NOIRQ)) {
+ count = tcg_temp_new_i32();
+ tcg_gen_ld_i32(count, cpu_env,
+ offsetof(ArchCPU, neg.icount_decr.u32) -
+ offsetof(ArchCPU, env));
+ }
if (cflags & CF_USE_ICOUNT) {
/*
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 36/45] accel/tcg: Hoist CF_MEMI_ONLY check outside translation loop
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (34 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 35/45] accel/tcg: Avoid load of icount_decr if unused Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 37/45] accel/tcg: Track current value of can_do_io in the TB Michael Tokarev
` (9 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
The condition checked is loop invariant; check it only once.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 5d97e94638100fd3e5b8d76ab30e1066cd4b1823)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index a3983019a5..b6ab9f3d33 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -158,7 +158,13 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
ops->tb_start(db, cpu);
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
- plugin_enabled = plugin_gen_tb_start(cpu, db, cflags & CF_MEMI_ONLY);
+ if (cflags & CF_MEMI_ONLY) {
+ /* We should only see CF_MEMI_ONLY for io_recompile. */
+ assert(cflags & CF_LAST_IO);
+ plugin_enabled = plugin_gen_tb_start(cpu, db, true);
+ } else {
+ plugin_enabled = plugin_gen_tb_start(cpu, db, false);
+ }
while (true) {
*max_insns = ++db->num_insns;
@@ -176,12 +182,8 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
if (db->num_insns == db->max_insns && (cflags & CF_LAST_IO)) {
/* Accept I/O on the last instruction. */
gen_io_start();
- ops->translate_insn(db, cpu);
- } else {
- /* we should only see CF_MEMI_ONLY for io_recompile */
- tcg_debug_assert(!(cflags & CF_MEMI_ONLY));
- ops->translate_insn(db, cpu);
}
+ ops->translate_insn(db, cpu);
/*
* We can't instrument after instructions that change control
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 37/45] accel/tcg: Track current value of can_do_io in the TB
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (35 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 36/45] accel/tcg: Hoist CF_MEMI_ONLY check outside translation loop Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:01 ` [Stable-8.1.2 38/45] accel/tcg: Improve setting of can_do_io at start of TB Michael Tokarev
` (8 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
Simplify translator_io_start by recording the current
known value of can_do_io within DisasContextBase.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 0ca41ccf1c555f97873b8e02a47390fd6af4b18f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index b6ab9f3d33..850d82e26f 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -16,11 +16,14 @@
#include "tcg/tcg-op-common.h"
#include "internal.h"
-static void gen_io_start(void)
+static void set_can_do_io(DisasContextBase *db, bool val)
{
- tcg_gen_st_i32(tcg_constant_i32(1), cpu_env,
- offsetof(ArchCPU, parent_obj.can_do_io) -
- offsetof(ArchCPU, env));
+ if (db->saved_can_do_io != val) {
+ db->saved_can_do_io = val;
+ tcg_gen_st_i32(tcg_constant_i32(val), cpu_env,
+ offsetof(ArchCPU, parent_obj.can_do_io) -
+ offsetof(ArchCPU, env));
+ }
}
bool translator_io_start(DisasContextBase *db)
@@ -30,12 +33,8 @@ bool translator_io_start(DisasContextBase *db)
if (!(cflags & CF_USE_ICOUNT)) {
return false;
}
- if (db->num_insns == db->max_insns && (cflags & CF_LAST_IO)) {
- /* Already started in translator_loop. */
- return true;
- }
- gen_io_start();
+ set_can_do_io(db, true);
/*
* Ensure that this instruction will be the last in the TB.
@@ -47,7 +46,7 @@ bool translator_io_start(DisasContextBase *db)
return true;
}
-static TCGOp *gen_tb_start(uint32_t cflags)
+static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
{
TCGv_i32 count = NULL;
TCGOp *icount_start_insn = NULL;
@@ -91,12 +90,9 @@ static TCGOp *gen_tb_start(uint32_t cflags)
* cpu->can_do_io is cleared automatically here at the beginning of
* each translation block. The cost is minimal and only paid for
* -icount, plus it would be very easy to forget doing it in the
- * translator. Doing it here means we don't need a gen_io_end() to
- * go with gen_io_start().
+ * translator.
*/
- tcg_gen_st_i32(tcg_constant_i32(0), cpu_env,
- offsetof(ArchCPU, parent_obj.can_do_io) -
- offsetof(ArchCPU, env));
+ set_can_do_io(db, false);
}
return icount_start_insn;
@@ -147,6 +143,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
db->num_insns = 0;
db->max_insns = *max_insns;
db->singlestep_enabled = cflags & CF_SINGLE_STEP;
+ db->saved_can_do_io = -1;
db->host_addr[0] = host_pc;
db->host_addr[1] = NULL;
@@ -154,7 +151,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
/* Start translating. */
- icount_start_insn = gen_tb_start(cflags);
+ icount_start_insn = gen_tb_start(db, cflags);
ops->tb_start(db, cpu);
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
@@ -181,7 +178,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
the next instruction. */
if (db->num_insns == db->max_insns && (cflags & CF_LAST_IO)) {
/* Accept I/O on the last instruction. */
- gen_io_start();
+ set_can_do_io(db, true);
}
ops->translate_insn(db, cpu);
diff --git a/include/exec/translator.h b/include/exec/translator.h
index a53d3243d4..0f4ecad7a2 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -72,6 +72,7 @@ typedef enum DisasJumpType {
* @num_insns: Number of translated instructions (including current).
* @max_insns: Maximum number of instructions to be translated in this TB.
* @singlestep_enabled: "Hardware" single stepping enabled.
+ * @saved_can_do_io: Known value of cpu->neg.can_do_io, or -1 for unknown.
*
* Architecture-agnostic disassembly context.
*/
@@ -83,6 +84,7 @@ typedef struct DisasContextBase {
int num_insns;
int max_insns;
bool singlestep_enabled;
+ int8_t saved_can_do_io;
void *host_addr[2];
} DisasContextBase;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 38/45] accel/tcg: Improve setting of can_do_io at start of TB
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (36 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 37/45] accel/tcg: Track current value of can_do_io in the TB Michael Tokarev
@ 2023-10-04 8:01 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 39/45] accel/tcg: Always set CF_LAST_IO with CF_NOIRQ Michael Tokarev
` (7 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
Initialize can_do_io to true if this the TB has CF_LAST_IO
and will consist of a single instruction. This avoids a
set to 0 followed immediately by a set to 1.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit a2f99d484c54adda13e62bf75ba512618a3fe470)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 850d82e26f..dd507cd471 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -87,12 +87,12 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
offsetof(ArchCPU, neg.icount_decr.u16.low) -
offsetof(ArchCPU, env));
/*
- * cpu->can_do_io is cleared automatically here at the beginning of
+ * cpu->can_do_io is set automatically here at the beginning of
* each translation block. The cost is minimal and only paid for
* -icount, plus it would be very easy to forget doing it in the
* translator.
*/
- set_can_do_io(db, false);
+ set_can_do_io(db, db->max_insns == 1 && (cflags & CF_LAST_IO));
}
return icount_start_insn;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 39/45] accel/tcg: Always set CF_LAST_IO with CF_NOIRQ
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (37 preceding siblings ...)
2023-10-04 8:01 ` [Stable-8.1.2 38/45] accel/tcg: Improve setting of can_do_io at start of TB Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 40/45] accel/tcg: Always require can_do_io Michael Tokarev
` (6 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
Without this we can get see loops through cpu_io_recompile,
in which the cpu makes no progress.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 200c1f904f46c209cb022e711a48b89e46512902)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index e2c494e75e..c724e8b6f1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -720,7 +720,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
&& cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0) {
/* Execute just one insn to trigger exception pending in the log */
cpu->cflags_next_tb = (curr_cflags(cpu) & ~CF_USE_ICOUNT)
- | CF_NOIRQ | 1;
+ | CF_LAST_IO | CF_NOIRQ | 1;
}
#endif
return false;
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index c406b2f7b7..85684f2b3d 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -1083,7 +1083,8 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc)
if (current_tb_modified) {
/* Force execution of one insn next time. */
CPUState *cpu = current_cpu;
- cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
+ cpu->cflags_next_tb =
+ 1 | CF_LAST_IO | CF_NOIRQ | curr_cflags(current_cpu);
return true;
}
return false;
@@ -1153,7 +1154,8 @@ tb_invalidate_phys_page_range__locked(struct page_collection *pages,
if (current_tb_modified) {
page_collection_unlock(pages);
/* Force execution of one insn next time. */
- current_cpu->cflags_next_tb = 1 | CF_NOIRQ | curr_cflags(current_cpu);
+ current_cpu->cflags_next_tb =
+ 1 | CF_LAST_IO | CF_NOIRQ | curr_cflags(current_cpu);
mmap_unlock();
cpu_loop_exit_noexc(current_cpu);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 40/45] accel/tcg: Always require can_do_io
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (38 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 39/45] accel/tcg: Always set CF_LAST_IO with CF_NOIRQ Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 41/45] target/tricore: Fix RCPW/RRPW_INSERT insns for width = 0 Michael Tokarev
` (5 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Richard Henderson, Philippe Mathieu-Daudé,
Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
Require i/o as the last insn of a TranslationBlock always,
not only with icount. This is required for i/o that alters
the address space, such as a pci config space write.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1866
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 18a536f1f8d6222e562f59179e837fdfd8b92718)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index dd507cd471..358214d526 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -28,12 +28,6 @@ static void set_can_do_io(DisasContextBase *db, bool val)
bool translator_io_start(DisasContextBase *db)
{
- uint32_t cflags = tb_cflags(db->tb);
-
- if (!(cflags & CF_USE_ICOUNT)) {
- return false;
- }
-
set_can_do_io(db, true);
/*
@@ -86,15 +80,15 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
tcg_gen_st16_i32(count, cpu_env,
offsetof(ArchCPU, neg.icount_decr.u16.low) -
offsetof(ArchCPU, env));
- /*
- * cpu->can_do_io is set automatically here at the beginning of
- * each translation block. The cost is minimal and only paid for
- * -icount, plus it would be very easy to forget doing it in the
- * translator.
- */
- set_can_do_io(db, db->max_insns == 1 && (cflags & CF_LAST_IO));
}
+ /*
+ * cpu->can_do_io is set automatically here at the beginning of
+ * each translation block. The cost is minimal, plus it would be
+ * very easy to forget doing it in the translator.
+ */
+ set_can_do_io(db, db->max_insns == 1 && (cflags & CF_LAST_IO));
+
return icount_start_insn;
}
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 9bb40f1849..593fc80458 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -11212,7 +11212,6 @@ static void gen_branch(DisasContext *ctx, int insn_bytes)
/* Branches completion */
clear_branch_hflags(ctx);
ctx->base.is_jmp = DISAS_NORETURN;
- /* FIXME: Need to clear can_do_io. */
switch (proc_hflags & MIPS_HFLAG_BMASK_BASE) {
case MIPS_HFLAG_FBNSLOT:
gen_goto_tb(ctx, 0, ctx->base.pc_next + insn_bytes);
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 41/45] target/tricore: Fix RCPW/RRPW_INSERT insns for width = 0
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (39 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 40/45] accel/tcg: Always require can_do_io Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 42/45] optionrom: Remove build-id section Michael Tokarev
` (4 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Bastian Koppelmann, Philippe Mathieu-Daudé,
Richard Henderson, Michael Tokarev
From: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
we would crash if width was 0 for these insns, as tcg_gen_deposit() is
undefined for that case. For TriCore, width = 0 is a mov from the src reg
to the dst reg, so we special case this here.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-9-kbastian@mail.uni-paderborn.de>
(cherry picked from commit 23fa6f56b33f8fddf86ba4d027fb7d3081440cd9)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 1947733870..e7fa5a825a 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -5317,8 +5317,11 @@ static void decode_rcpw_insert(DisasContext *ctx)
}
break;
case OPC2_32_RCPW_INSERT:
+ /* tcg_gen_deposit_tl() does not handle the case of width = 0 */
+ if (width == 0) {
+ tcg_gen_mov_tl(cpu_gpr_d[r2], cpu_gpr_d[r1]);
/* if pos + width > 32 undefined result */
- if (pos + width <= 32) {
+ } else if (pos + width <= 32) {
temp = tcg_constant_i32(const4);
tcg_gen_deposit_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], temp, pos, width);
}
@@ -6558,7 +6561,10 @@ static void decode_rrpw_extract_insert(DisasContext *ctx)
break;
case OPC2_32_RRPW_INSERT:
- if (pos + width <= 32) {
+ /* tcg_gen_deposit_tl() does not handle the case of width = 0 */
+ if (width == 0) {
+ tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
+ } else if (pos + width <= 32) {
tcg_gen_deposit_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
pos, width);
}
diff --git a/tests/tcg/tricore/asm/macros.h b/tests/tcg/tricore/asm/macros.h
index b5087b5c97..51f6191ef2 100644
--- a/tests/tcg/tricore/asm/macros.h
+++ b/tests/tcg/tricore/asm/macros.h
@@ -161,6 +161,21 @@ test_ ## num: \
insn DREG_CALC_RESULT, DREG_RS1, imm1, DREG_RS2, imm2; \
)
+#define TEST_D_DDII(insn, num, result, rs1, rs2, imm1, imm2) \
+ TEST_CASE(num, DREG_CALC_RESULT, result, \
+ LI(DREG_RS1, rs1); \
+ LI(DREG_RS2, rs2); \
+ rstv; \
+ insn DREG_CALC_RESULT, DREG_RS1, DREG_RS2, imm1, imm2; \
+ )
+
+#define TEST_D_DIII(insn, num, result, rs1, imm1, imm2, imm3)\
+ TEST_CASE(num, DREG_CALC_RESULT, result, \
+ LI(DREG_RS1, rs1); \
+ rstv; \
+ insn DREG_CALC_RESULT, DREG_RS1, imm1, imm2, imm3; \
+ )
+
#define TEST_E_ED(insn, num, res_hi, res_lo, rs1_hi, rs1_lo, rs2) \
TEST_CASE_E(num, res_lo, res_hi, \
LI(EREG_RS1_LO, rs1_lo); \
diff --git a/tests/tcg/tricore/asm/test_insert.S b/tests/tcg/tricore/asm/test_insert.S
index d5fd2237e1..3978810121 100644
--- a/tests/tcg/tricore/asm/test_insert.S
+++ b/tests/tcg/tricore/asm/test_insert.S
@@ -6,4 +6,13 @@ _start:
# | | | | | | |
TEST_D_DIDI(insert, 1, 0x7fffffff, 0xffffffff, 0xa, 0x10, 0x8)
+# insn num result rs1 imm1 imm2 imm3
+# | | | | | | |
+ TEST_D_DIII(insert, 2, 0xd38fe370, 0xd38fe370, 0x4, 0x4 , 0x0)
+ TEST_D_DIII(insert, 3, 0xd38fe374, 0xd38fe370, 0x4, 0x0 , 0x4)
+
+# insn num result rs1 rs2 pos width
+# | | | | | | |
+ TEST_D_DDII(insert, 4, 0x03c1e53c, 0x03c1e53c, 0x45821385, 0x7 ,0x0)
+
TEST_PASSFAIL
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 42/45] optionrom: Remove build-id section
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (40 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 41/45] target/tricore: Fix RCPW/RRPW_INSERT insns for width = 0 Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 43/45] esp: use correct type for esp_dma_enable() in sysbus_esp_gpio_demux() Michael Tokarev
` (3 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Fabiano Rosas, Vasiliy Ulyanov, Thomas Huth,
Paolo Bonzini, Michael Tokarev
From: Fabiano Rosas <farosas@suse.de>
Our linker script for optionroms specifies only the placement of the
.text section, leaving the linker free to place the remaining sections
at arbitrary places in the file.
Since at least binutils 2.39, the .note.gnu.build-id section is now
being placed at the start of the file, which causes label addresses to
be shifted. For linuxboot_dma.bin that means that the PnP header
(among others) will not be found when determining the type of ROM at
optionrom_setup():
(0x1c is the label _pnph, where the magic "PnP" is)
$ xxd /usr/share/qemu/linuxboot_dma.bin | grep "PnP"
00000010: 0000 0000 0000 0000 0000 1c00 2450 6e50 ............$PnP
$ xxd pc-bios/optionrom/linuxboot_dma.bin | grep "PnP"
00000010: 0000 0000 0000 0000 0000 4c00 2450 6e50 ............$PnP
^bad
Using a freshly built linuxboot_dma.bin ROM results in a broken boot:
SeaBIOS (version rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org)
Booting from Hard Disk...
Boot failed: could not read the boot disk
Booting from Floppy...
Boot failed: could not read the boot disk
No bootable device.
We're not using the build-id section, so pass the --build-id=none
option to the linker to remove it entirely.
Note: In theory, this same issue could happen with any other
section. The ideal solution would be to have all unused sections
discarded in the linker script. However that would be a larger change,
specially for the pvh rom which uses the .bss and COMMON sections so
I'm addressing only the immediate issue here.
Reported-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230926192502.15986-1-farosas@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 35ed01ba5448208695ada5fa20a13c0a4689a1c1)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(mjt: remove unrelated stable@vger)
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index b1fff0ba6c..30d07026c7 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -36,7 +36,7 @@ config-cc.mak: Makefile
$(call cc-option,-Wno-array-bounds)) 3> config-cc.mak
-include config-cc.mak
-override LDFLAGS = -nostdlib -Wl,-T,$(SRC_DIR)/flat.lds
+override LDFLAGS = -nostdlib -Wl,--build-id=none,-T,$(SRC_DIR)/flat.lds
pvh.img: pvh.o pvh_main.o
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 43/45] esp: use correct type for esp_dma_enable() in sysbus_esp_gpio_demux()
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (41 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 42/45] optionrom: Remove build-id section Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 44/45] esp: restrict non-DMA transfer length to that of available data Michael Tokarev
` (2 subsequent siblings)
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Philippe Mathieu-Daudé,
Thomas Huth, Paolo Bonzini, Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
The call to esp_dma_enable() was being made with the SYSBUS_ESP type instead of
the ESP type. This meant that when GPIO 1 was being used to trigger a DMA
request from an external DMA controller, the setting of ESPState's dma_enabled
field would clobber unknown memory whilst the dma_cb callback pointer would
typically return NULL so the DMA request would never start.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230913204410.65650-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit b86dc5cb0b4105fa8ad29e822ab5d21c589c5ec5)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index e52188d022..4218a6a960 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1395,7 +1395,7 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
parent_esp_reset(s, irq, level);
break;
case 1:
- esp_dma_enable(opaque, irq, level);
+ esp_dma_enable(s, irq, level);
break;
}
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 44/45] esp: restrict non-DMA transfer length to that of available data
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (42 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 43/45] esp: use correct type for esp_dma_enable() in sysbus_esp_gpio_demux() Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:02 ` [Stable-8.1.2 45/45] scsi-disk: ensure that FORMAT UNIT commands are terminated Michael Tokarev
2023-10-04 8:19 ` [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Olaf Hering
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Thomas Huth, Paolo Bonzini,
Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
In the case where a SCSI layer transfer is incorrectly terminated, it is
possible for a TI command to cause a SCSI buffer overflow due to the
expected transfer data length being less than the available data in the
FIFO. When this occurs the unsigned async_len variable underflows and
becomes a large offset which writes past the end of the allocated SCSI
buffer.
Restrict the non-DMA transfer length to be the smallest of the expected
transfer length and the available FIFO data to ensure that it is no longer
possible for the SCSI buffer overflow to occur.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1810
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230913204410.65650-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 77668e4b9bca03a856c27ba899a2513ddf52bb52)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 4218a6a960..9b11d8c573 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -759,7 +759,8 @@ static void esp_do_nodma(ESPState *s)
}
if (to_device) {
- len = MIN(fifo8_num_used(&s->fifo), ESP_FIFO_SZ);
+ len = MIN(s->async_len, ESP_FIFO_SZ);
+ len = MIN(len, fifo8_num_used(&s->fifo));
esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
s->async_buf += len;
s->async_len -= len;
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Stable-8.1.2 45/45] scsi-disk: ensure that FORMAT UNIT commands are terminated
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (43 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 44/45] esp: restrict non-DMA transfer length to that of available data Michael Tokarev
@ 2023-10-04 8:02 ` Michael Tokarev
2023-10-04 8:19 ` [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Olaf Hering
45 siblings, 0 replies; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:02 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Thomas Huth, Paolo Bonzini,
Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Otherwise when a FORMAT UNIT command is issued, the SCSI layer can become
confused because it can find itself in the situation where it thinks there
is still data to be transferred which can cause the next emulated SCSI
command to fail.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: 6ab71761 ("scsi-disk: add FORMAT UNIT command")
Tested-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230913204410.65650-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit be2b619a17345d007bcf9987a3e4afd1edea3e4f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 477ee2bcd4..6691f5edb8 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1959,6 +1959,10 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
scsi_disk_emulate_write_same(r, r->iov.iov_base);
break;
+ case FORMAT_UNIT:
+ scsi_req_complete(&r->req, GOOD);
+ break;
+
default:
abort();
}
--
2.39.2
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14
2023-10-04 8:01 [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Michael Tokarev
` (44 preceding siblings ...)
2023-10-04 8:02 ` [Stable-8.1.2 45/45] scsi-disk: ensure that FORMAT UNIT commands are terminated Michael Tokarev
@ 2023-10-04 8:19 ` Olaf Hering
2023-10-04 8:44 ` Michael Tokarev
45 siblings, 1 reply; 49+ messages in thread
From: Olaf Hering @ 2023-10-04 8:19 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-stable
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
Wed, 4 Oct 2023 11:01:21 +0300 Michael Tokarev <mjt@tls.msk.ru>:
> Please respond here or CC qemu-stable@nongnu.org on any additional patches
> you think should (or shouldn't) be included in the release.
How about this change for 8.1.x? This will allow usage in openSUSE Tumbleweed.
c01196bddd subprojects/berkeley-testfloat-3: Update to fix a problem with compiler warnings
Olaf
[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14
2023-10-04 8:19 ` [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14 Olaf Hering
@ 2023-10-04 8:44 ` Michael Tokarev
2023-10-04 8:57 ` Olaf Hering
0 siblings, 1 reply; 49+ messages in thread
From: Michael Tokarev @ 2023-10-04 8:44 UTC (permalink / raw)
To: Olaf Hering; +Cc: qemu-devel, qemu-stable
04.10.2023 11:19, Olaf Hering wrote:
> How about this change for 8.1.x? This will allow usage in openSUSE Tumbleweed.
>
> c01196bddd subprojects/berkeley-testfloat-3: Update to fix a problem with compiler warnings
Hm. I don't think this one is a good candidate (not that it can't be included).
This just fixes compiler warning (the original code is good, and I'd even question
the patch "fixing" the warnings in berkeley-testfloat-3, - at the very least, this
"default:" case warrants a comment, since all possible values of the switch are
already explicitly specified. This is a clear compiler defect, the changes does
not affect the result in any way.
Second, this is not even the production code, it is testing code.
And the most important, third: even with the warning being emitted, 8.1 is still
usable on openSUSE Tumbleweed: all qemu released tarballs are built WITHOUT
-Werror option, so when building a release there, these warnings wont fail the
build. Currently, this is controlled by presence of .git in the top-level
source dir, -Werror is only enabled if .git is present.
I'm including this change, if not only because we both spent quite some time
already on this ;) Overall though, it is not needed.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Stable-8.1.2 00/45] Patch Round-up for stable 8.1.2, freeze on 2023-10-14
2023-10-04 8:44 ` Michael Tokarev
@ 2023-10-04 8:57 ` Olaf Hering
0 siblings, 0 replies; 49+ messages in thread
From: Olaf Hering @ 2023-10-04 8:57 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-stable
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
Wed, 4 Oct 2023 11:44:53 +0300 Michael Tokarev <mjt@tls.msk.ru>:
> Second, this is not even the production code, it is testing code.
I need to double check if there is indeed a way to omit this code.
A quick search indicates that disabling TCG may be required.
> And the most important, third: even with the warning being emitted, 8.1 is still
> usable on openSUSE Tumbleweed: all qemu released tarballs are built WITHOUT
> -Werror option, so when building a release there, these warnings wont fail the
> build. Currently, this is controlled by presence of .git in the top-level
> source dir, -Werror is only enabled if .git is present.
The failure comes from outside, Werror=return-type in CFLAGS, not from
QEMU's built-in Werror.
Olaf
[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread