* [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27
@ 2024-01-23 8:48 Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 09/20] load_elf: fix iterator's type for elf file processing Michael Tokarev
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Michael Tokarev
The following patches are queued for QEMU stable v7.2.9:
https://gitlab.com/qemu-project/qemu/-/commits/staging-7.2
Patch freeze is 2024-01-27, and the release is planned for 2024-01-29:
https://wiki.qemu.org/Planning/7.2
Please respond here or CC qemu-stable@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.
The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.
Thanks!
/mjt
--------------------------------------
01* d3007d348ada Kevin Wolf:
block: Fix crash when loading snapshot on inactive node
02* 5a7f21efaf99 Kevin Wolf:
vl: Improve error message for conflicting -incoming and -loadvm
03* bb6e2511eb48 Kevin Wolf:
iotests: Basic tests for internal snapshots
04* 5cb0e7abe163 Xu Lu:
target/riscv: Fix mcycle/minstret increment behavior
05* 4ad87cd4b225 Michael Tokarev:
chardev/char.c: fix "abstract device type" error message
06* 82a65e3188ab Peter Maydell:
hw/intc/arm_gicv3_cpuif: handle LPIs in in the list registers
07* e358a25a97c7 Ilya Leoshkevich:
target/s390x: Fix LAE setting a wrong access register
08* 52a21689cd82 Peter Maydell:
.gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large
09 410c2a4d75f5 Anastasia Belova:
load_elf: fix iterator's type for elf file processing
10 b5e0d5d22fbf Richard Henderson:
target/i386: Fix 32-bit wrapping of pc/eip computation
11 a58506b748b8 Richard Henderson:
target/i386: Do not re-compute new pc with CF_PCREL
12 2926eab89699 guoguangyao:
target/i386: fix incorrect EIP in PC-relative translation blocks
13 729ba8e933f8 Paolo Bonzini:
target/i386: pcrel: store low bits of physical address in data[0]
14 3b14a555fdb6 Gerd Hoffmann:
hw/pflash: refactor pflash_data_write()
15 5dd58358a570 Gerd Hoffmann:
hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p
16 284a7ee2e290 Gerd Hoffmann:
hw/pflash: implement update buffer for block writes
17 84a6835e004c Mark Cave-Ayland:
hw/scsi/esp-pci: use correct address register for PCI DMA transfers
18 6b41417d934b Mark Cave-Ayland:
hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources
19 1e8e6644e063 Mark Cave-Ayland:
hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt
20 c2d7de557d19 Mark Cave-Ayland:
hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued
(commit(s) marked with * were in previous series and are not resent)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Stable-7.2.9 09/20] load_elf: fix iterator's type for elf file processing
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 10/20] target/i386: Fix 32-bit wrapping of pc/eip computation Michael Tokarev
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Anastasia Belova, Peter Maydell, Michael Tokarev
From: Anastasia Belova <abelova@astralinux.ru>
j is used while loading an ELF file to byteswap segments'
data. If data is larger than 2GB an overflow may happen.
So j should be elf_word.
This commit fixes a minor bug: it's unlikely anybody is trying to
load ELF files with 2GB+ segments for wrong-endianness targets,
but if they did, it wouldn't work correctly.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Cc: qemu-stable@nongnu.org
Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf")
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit 410c2a4d75f52f6a2fe978eda5a9b6f854afe5ea)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index fbe0b1e956..78f45c8115 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -499,7 +499,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
}
if (data_swab) {
- int j;
+ elf_word j;
for (j = 0; j < file_size; j += (1 << data_swab)) {
uint8_t *dp = data + j;
switch (data_swab) {
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 10/20] target/i386: Fix 32-bit wrapping of pc/eip computation
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 09/20] load_elf: fix iterator's type for elf file processing Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 11/20] target/i386: Do not re-compute new pc with CF_PCREL Michael Tokarev
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Richard Henderson, Paolo Bonzini, Michael Tokarev
From: Richard Henderson <richard.henderson@linaro.org>
In 32-bit mode, pc = eip + cs_base is also 32-bit, and must wrap.
Failure to do so results in incorrect memory exceptions to the guest.
Before 732d548732ed, this was implicitly done via truncation to
target_ulong but only in qemu-system-i386, not qemu-system-x86_64.
To fix this, we must add conditional zero-extensions.
Since we have to test for 32 vs 64-bit anyway, note that cs_base
is always zero in 64-bit mode.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2022
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20231212172510.103305-1-richard.henderson@linaro.org>
(cherry picked from commit b5e0d5d22fbffc3d8f7d3e86d7a2d05a1a974e27)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: context fix in target/i386/tcg/tcg-cpu.c for v8.1.0-1190-gb77af26e97
"accel/tcg: Replace CPUState.env_ptr with cpu_env()")
(Mjt: fixup in target/i386/tcg/tcg-cpu.c for v7.2.0-1854-g34a39c2443
"target/i386: Replace `tb_pc()` with `tb->pc`")
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d4bc19577a..f67cee477a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2217,10 +2217,15 @@ static inline int cpu_mmu_index_kernel(CPUX86State *env)
static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc,
target_ulong *cs_base, uint32_t *flags)
{
- *cs_base = env->segs[R_CS].base;
- *pc = *cs_base + env->eip;
*flags = env->hflags |
(env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK | AC_MASK));
+ if (env->hflags & HF_CS64_MASK) {
+ *cs_base = 0;
+ *pc = env->eip;
+ } else {
+ *cs_base = env->segs[R_CS].base;
+ *pc = (uint32_t)(*cs_base + env->eip);
+ }
}
void do_cpu_init(X86CPU *cpu);
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 79ac5908f7..563f42ee4e 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -52,7 +52,12 @@ static void x86_cpu_synchronize_from_tb(CPUState *cs,
/* The instruction pointer is always up to date with TARGET_TB_PCREL. */
if (!TARGET_TB_PCREL) {
CPUX86State *env = cs->env_ptr;
- env->eip = tb_pc(tb) - tb->cs_base;
+
+ if (tb->flags & HF_CS64_MASK) {
+ env->eip = tb_pc(tb);
+ } else {
+ env->eip = (uint32_t)(tb_pc(tb) - tb->cs_base);
+ }
}
}
@@ -66,8 +71,10 @@ static void x86_restore_state_to_opc(CPUState *cs,
if (TARGET_TB_PCREL) {
env->eip = (env->eip & TARGET_PAGE_MASK) | data[0];
+ } else if (tb->flags & HF_CS64_MASK) {
+ env->eip = data[0];
} else {
- env->eip = data[0] - tb->cs_base;
+ env->eip = (uint32_t)(data[0] - tb->cs_base);
}
if (cc_op != CC_OP_DYNAMIC) {
env->cc_op = cc_op;
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 7e0b2a709a..15b030d84b 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -547,8 +547,10 @@ static void gen_update_eip_cur(DisasContext *s)
assert(s->pc_save != -1);
if (TARGET_TB_PCREL) {
tcg_gen_addi_tl(cpu_eip, cpu_eip, s->base.pc_next - s->pc_save);
+ } else if (CODE64(s)) {
+ tcg_gen_movi_tl(cpu_eip, s->base.pc_next);
} else {
- tcg_gen_movi_tl(cpu_eip, s->base.pc_next - s->cs_base);
+ tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->base.pc_next - s->cs_base));
}
s->pc_save = s->base.pc_next;
}
@@ -558,8 +560,10 @@ static void gen_update_eip_next(DisasContext *s)
assert(s->pc_save != -1);
if (TARGET_TB_PCREL) {
tcg_gen_addi_tl(cpu_eip, cpu_eip, s->pc - s->pc_save);
+ } else if (CODE64(s)) {
+ tcg_gen_movi_tl(cpu_eip, s->base.pc_next);
} else {
- tcg_gen_movi_tl(cpu_eip, s->pc - s->cs_base);
+ tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->base.pc_next - s->cs_base));
}
s->pc_save = s->pc;
}
@@ -605,8 +609,10 @@ static TCGv eip_next_tl(DisasContext *s)
TCGv ret = tcg_temp_new();
tcg_gen_addi_tl(ret, cpu_eip, s->pc - s->pc_save);
return ret;
+ } else if (CODE64(s)) {
+ return tcg_constant_tl(s->pc);
} else {
- return tcg_constant_tl(s->pc - s->cs_base);
+ return tcg_constant_tl((uint32_t)(s->pc - s->cs_base));
}
}
@@ -617,8 +623,10 @@ static TCGv eip_cur_tl(DisasContext *s)
TCGv ret = tcg_temp_new();
tcg_gen_addi_tl(ret, cpu_eip, s->base.pc_next - s->pc_save);
return ret;
+ } else if (CODE64(s)) {
+ return tcg_constant_tl(s->base.pc_next);
} else {
- return tcg_constant_tl(s->base.pc_next - s->cs_base);
+ return tcg_constant_tl((uint32_t)(s->base.pc_next - s->cs_base));
}
}
@@ -2875,6 +2883,10 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
}
}
new_eip &= mask;
+ new_pc = new_eip + s->cs_base;
+ if (!CODE64(s)) {
+ new_pc = (uint32_t)new_pc;
+ }
gen_update_cc_op(s);
set_cc_op(s, CC_OP_DYNAMIC);
@@ -2892,8 +2904,7 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
}
}
- if (use_goto_tb &&
- translator_use_goto_tb(&s->base, new_eip + s->cs_base)) {
+ if (use_goto_tb && translator_use_goto_tb(&s->base, new_pc)) {
/* jump to same page: we can use a direct jump */
tcg_gen_goto_tb(tb_num);
if (!TARGET_TB_PCREL) {
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 11/20] target/i386: Do not re-compute new pc with CF_PCREL
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 09/20] load_elf: fix iterator's type for elf file processing Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 10/20] target/i386: Fix 32-bit wrapping of pc/eip computation Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 12/20] target/i386: fix incorrect EIP in PC-relative translation blocks Michael Tokarev
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Richard Henderson, Michael Tokarev, Paolo Bonzini
From: Richard Henderson <richard.henderson@linaro.org>
With PCREL, we have a page-relative view of EIP, and an
approximation of PC = EIP+CSBASE that is good enough to
detect page crossings. If we try to recompute PC after
masking EIP, we will mess up that approximation and write
a corrupt value to EIP.
We already handled masking properly for PCREL, so the
fix in b5e0d5d2 was only needed for the !PCREL path.
Cc: qemu-stable@nongnu.org
Fixes: b5e0d5d22fbf ("target/i386: Fix 32-bit wrapping of pc/eip computation")
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20240101230617.129349-1-richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit a58506b748b8988a95f4fa1a2420ac5c17038b30)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 15b030d84b..fa2e1dc3b3 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2883,10 +2883,6 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
}
}
new_eip &= mask;
- new_pc = new_eip + s->cs_base;
- if (!CODE64(s)) {
- new_pc = (uint32_t)new_pc;
- }
gen_update_cc_op(s);
set_cc_op(s, CC_OP_DYNAMIC);
@@ -2902,6 +2898,8 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
tcg_gen_andi_tl(cpu_eip, cpu_eip, mask);
use_goto_tb = false;
}
+ } else if (!CODE64(s)) {
+ new_pc = (uint32_t)(new_eip + s->cs_base);
}
if (use_goto_tb && translator_use_goto_tb(&s->base, new_pc)) {
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 12/20] target/i386: fix incorrect EIP in PC-relative translation blocks
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (2 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 11/20] target/i386: Do not re-compute new pc with CF_PCREL Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 13/20] target/i386: pcrel: store low bits of physical address in data[0] Michael Tokarev
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, guoguangyao, Richard Henderson, Paolo Bonzini,
Michael Tokarev
From: guoguangyao <guoguangyao18@mails.ucas.ac.cn>
The PCREL patches introduced a bug when updating EIP in the !CF_PCREL case.
Using s->pc in func gen_update_eip_next() solves the problem.
Cc: qemu-stable@nongnu.org
Fixes: b5e0d5d22fbf ("target/i386: Fix 32-bit wrapping of pc/eip computation")
Signed-off-by: guoguangyao <guoguangyao18@mails.ucas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20240115020804.30272-1-guoguangyao18@mails.ucas.ac.cn>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 2926eab8969908bc068629e973062a0fb6ff3759)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index fa2e1dc3b3..fc1d79e866 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -561,9 +561,9 @@ static void gen_update_eip_next(DisasContext *s)
if (TARGET_TB_PCREL) {
tcg_gen_addi_tl(cpu_eip, cpu_eip, s->pc - s->pc_save);
} else if (CODE64(s)) {
- tcg_gen_movi_tl(cpu_eip, s->base.pc_next);
+ tcg_gen_movi_tl(cpu_eip, s->pc);
} else {
- tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->base.pc_next - s->cs_base));
+ tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->pc - s->cs_base));
}
s->pc_save = s->pc;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 13/20] target/i386: pcrel: store low bits of physical address in data[0]
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (3 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 12/20] target/i386: fix incorrect EIP in PC-relative translation blocks Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 14/20] hw/pflash: refactor pflash_data_write() Michael Tokarev
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Paolo Bonzini, Mark Cave-Ayland, Richard Henderson,
Michael Tokarev
From: Paolo Bonzini <pbonzini@redhat.com>
For PC-relative translation blocks, env->eip changes during the
execution of a translation block, Therefore, QEMU must be able to
recover an instruction's PC just from the TranslationBlock struct and
the instruction data with. Because a TB will not span two pages, QEMU
stores all the low bits of EIP in the instruction data and replaces them
in x86_restore_state_to_opc. Bits 12 and higher (which may vary between
executions of a PCREL TB, since these only use the physical address in
the hash key) are kept unmodified from env->eip. The assumption is that
these bits of EIP, unlike bits 0-11, will not change as the translation
block executes.
Unfortunately, this is incorrect when the CS base is not aligned to a page.
Then the linear address of the instructions (i.e. the one with the
CS base addred) indeed will never span two pages, but bits 12+ of EIP
can actually change. For example, if CS base is 0x80262200 and EIP =
0x6FF4, the first instruction in the translation block will be at linear
address 0x802691F4. Even a very small TB will cross to EIP = 0x7xxx,
while the linear addresses will remain comfortably within a single page.
The fix is simply to use the low bits of the linear address for data[0],
since those don't change. Then x86_restore_state_to_opc uses tb->cs_base
to compute a temporary linear address (referring to some unknown
instruction in the TB, but with the correct values of bits 12 and higher);
the low bits are replaced with data[0], and EIP is obtained by subtracting
again the CS base.
Huge thanks to Mark Cave-Ayland for the image and initial debugging,
and to Gitlab user @kjliew for help with bisecting another occurrence
of (hopefully!) the same bug.
It should be relatively easy to write a testcase that performs MMIO on
an EIP with different bits 12+ than the first instruction of the translation
block; any help is welcome.
Fixes: e3a79e0e878 ("target/i386: Enable TARGET_TB_PCREL", 2022-10-11)
Cc: qemu-stable@nongnu.org
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Richard Henderson <richard.henderson@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1759
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1964
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2012
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 729ba8e933f8af5800c3a92b37e630e9bdaa9f1e)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: fixup in target/i386/tcg/tcg-cpu.c target/i386/tcg/translate.c for
v7.2.0-1839-g2e3afe8e19 "target/i386: Replace `TARGET_TB_PCREL` with `CF_PCREL`")
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 563f42ee4e..cd49f86341 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -68,14 +68,26 @@ static void x86_restore_state_to_opc(CPUState *cs,
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = &cpu->env;
int cc_op = data[1];
+ uint64_t new_pc;
if (TARGET_TB_PCREL) {
- env->eip = (env->eip & TARGET_PAGE_MASK) | data[0];
- } else if (tb->flags & HF_CS64_MASK) {
- env->eip = data[0];
+ /*
+ * data[0] in PC-relative TBs is also a linear address, i.e. an address with
+ * the CS base added, because it is not guaranteed that EIP bits 12 and higher
+ * stay the same across the translation block. Add the CS base back before
+ * replacing the low bits, and subtract it below just like for !CF_PCREL.
+ */
+ uint64_t pc = env->eip + tb->cs_base;
+ new_pc = (pc & TARGET_PAGE_MASK) | data[0];
} else {
- env->eip = (uint32_t)(data[0] - tb->cs_base);
+ new_pc = data[0];
}
+ if (tb->flags & HF_CS64_MASK) {
+ env->eip = new_pc;
+ } else {
+ env->eip = (uint32_t)(new_pc - tb->cs_base);
+ }
+
if (cc_op != CC_OP_DYNAMIC) {
env->cc_op = cc_op;
}
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index fc1d79e866..68c42fd9ff 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -6983,7 +6983,6 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
dc->prev_insn_end = tcg_last_op();
if (TARGET_TB_PCREL) {
- pc_arg -= dc->cs_base;
pc_arg &= ~TARGET_PAGE_MASK;
}
tcg_gen_insn_start(pc_arg, dc->cc_op);
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 14/20] hw/pflash: refactor pflash_data_write()
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (4 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 13/20] target/i386: pcrel: store low bits of physical address in data[0] Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 15/20] hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p Michael Tokarev
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Gerd Hoffmann, Philippe Mathieu-Daudé,
Michael Tokarev
From: Gerd Hoffmann <kraxel@redhat.com>
Move the offset calculation, do it once at the start of the function and
let the 'p' variable point directly to the memory location which should
be updated. This makes it simpler to update other buffers than
pfl->storage in an upcoming patch. No functional change.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240108160900.104835-2-kraxel@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 3b14a555fdb627ac091559ef5931c887d06590d8)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 0cbc2fb4cb..c250214944 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -404,33 +404,35 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
uint32_t value, int width, int be)
{
- uint8_t *p = pfl->storage;
+ uint8_t *p;
trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
+ p = pfl->storage + offset;
+
switch (width) {
case 1:
- p[offset] = value;
+ p[0] = value;
break;
case 2:
if (be) {
- p[offset] = value >> 8;
- p[offset + 1] = value;
+ p[0] = value >> 8;
+ p[1] = value;
} else {
- p[offset] = value;
- p[offset + 1] = value >> 8;
+ p[0] = value;
+ p[1] = value >> 8;
}
break;
case 4:
if (be) {
- p[offset] = value >> 24;
- p[offset + 1] = value >> 16;
- p[offset + 2] = value >> 8;
- p[offset + 3] = value;
+ p[0] = value >> 24;
+ p[1] = value >> 16;
+ p[2] = value >> 8;
+ p[3] = value;
} else {
- p[offset] = value;
- p[offset + 1] = value >> 8;
- p[offset + 2] = value >> 16;
- p[offset + 3] = value >> 24;
+ p[0] = value;
+ p[1] = value >> 8;
+ p[2] = value >> 16;
+ p[3] = value >> 24;
}
break;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 15/20] hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (5 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 14/20] hw/pflash: refactor pflash_data_write() Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 16/20] hw/pflash: implement update buffer for block writes Michael Tokarev
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Gerd Hoffmann, Philippe Mathieu-Daudé,
Michael Tokarev
From: Gerd Hoffmann <kraxel@redhat.com>
Use the helper functions we have to read/write multi-byte values
in correct byte order.
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240108160900.104835-3-kraxel@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 5dd58358a57048e5ceabf5c91c0544f4f56afdcd)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index c250214944..dbb3d0312d 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -226,34 +226,10 @@ static uint32_t pflash_data_read(PFlashCFI01 *pfl, hwaddr offset,
uint32_t ret;
p = pfl->storage;
- switch (width) {
- case 1:
- ret = p[offset];
- break;
- case 2:
- if (be) {
- ret = p[offset] << 8;
- ret |= p[offset + 1];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- }
- break;
- case 4:
- if (be) {
- ret = p[offset] << 24;
- ret |= p[offset + 1] << 16;
- ret |= p[offset + 2] << 8;
- ret |= p[offset + 3];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- ret |= p[offset + 2] << 16;
- ret |= p[offset + 3] << 24;
- }
- break;
- default:
- abort();
+ if (be) {
+ ret = ldn_be_p(p + offset, width);
+ } else {
+ ret = ldn_le_p(p + offset, width);
}
trace_pflash_data_read(pfl->name, offset, width, ret);
return ret;
@@ -409,34 +385,11 @@ static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
p = pfl->storage + offset;
- switch (width) {
- case 1:
- p[0] = value;
- break;
- case 2:
- if (be) {
- p[0] = value >> 8;
- p[1] = value;
- } else {
- p[0] = value;
- p[1] = value >> 8;
- }
- break;
- case 4:
- if (be) {
- p[0] = value >> 24;
- p[1] = value >> 16;
- p[2] = value >> 8;
- p[3] = value;
- } else {
- p[0] = value;
- p[1] = value >> 8;
- p[2] = value >> 16;
- p[3] = value >> 24;
- }
- break;
+ if (be) {
+ stn_be_p(p, width, value);
+ } else {
+ stn_le_p(p, width, value);
}
-
}
static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 16/20] hw/pflash: implement update buffer for block writes
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (6 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 15/20] hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 17/20] hw/scsi/esp-pci: use correct address register for PCI DMA transfers Michael Tokarev
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Gerd Hoffmann, Philippe Mathieu-Daudé,
Michael Tokarev
From: Gerd Hoffmann <kraxel@redhat.com>
Add an update buffer where all block updates are staged.
Flush or discard updates properly, so we should never see
half-completed block writes in pflash storage.
Drop a bunch of FIXME comments ;)
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240108160900.104835-4-kraxel@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 284a7ee2e290e0c9b8cd3ea6164d92386933054f)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: drop const in hw/block/pflash_cfi01.c for before
v8.2.0-220-g7d5dc0a367 "hw/block: Constify VMState")
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index dbb3d0312d..6bc00254cc 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -81,16 +81,39 @@ struct PFlashCFI01 {
uint16_t ident3;
uint8_t cfi_table[0x52];
uint64_t counter;
- unsigned int writeblock_size;
+ uint32_t writeblock_size;
MemoryRegion mem;
char *name;
void *storage;
VMChangeStateEntry *vmstate;
bool old_multiple_chip_handling;
+
+ /* block update buffer */
+ unsigned char *blk_bytes;
+ uint32_t blk_offset;
};
static int pflash_post_load(void *opaque, int version_id);
+static bool pflash_blk_write_state_needed(void *opaque)
+{
+ PFlashCFI01 *pfl = opaque;
+
+ return (pfl->blk_offset != -1);
+}
+
+static const VMStateDescription vmstate_pflash_blk_write = {
+ .name = "pflash_cfi01_blk_write",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = pflash_blk_write_state_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_VBUFFER_UINT32(blk_bytes, PFlashCFI01, 0, NULL, writeblock_size),
+ VMSTATE_UINT32(blk_offset, PFlashCFI01),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_pflash = {
.name = "pflash_cfi01",
.version_id = 1,
@@ -102,6 +125,10 @@ static const VMStateDescription vmstate_pflash = {
VMSTATE_UINT8(status, PFlashCFI01),
VMSTATE_UINT64(counter, PFlashCFI01),
VMSTATE_END_OF_LIST()
+ },
+ .subsections = (const VMStateDescription * []) {
+ &vmstate_pflash_blk_write,
+ NULL
}
};
@@ -377,13 +404,55 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
}
}
+/* copy current flash content to block update buffer */
+static void pflash_blk_write_start(PFlashCFI01 *pfl, hwaddr offset)
+{
+ hwaddr mask = ~(pfl->writeblock_size - 1);
+
+ trace_pflash_write_block_start(pfl->name, pfl->counter);
+ pfl->blk_offset = offset & mask;
+ memcpy(pfl->blk_bytes, pfl->storage + pfl->blk_offset,
+ pfl->writeblock_size);
+}
+
+/* commit block update buffer changes */
+static void pflash_blk_write_flush(PFlashCFI01 *pfl)
+{
+ g_assert(pfl->blk_offset != -1);
+ trace_pflash_write_block_flush(pfl->name);
+ memcpy(pfl->storage + pfl->blk_offset, pfl->blk_bytes,
+ pfl->writeblock_size);
+ pflash_update(pfl, pfl->blk_offset, pfl->writeblock_size);
+ pfl->blk_offset = -1;
+}
+
+/* discard block update buffer changes */
+static void pflash_blk_write_abort(PFlashCFI01 *pfl)
+{
+ trace_pflash_write_block_abort(pfl->name);
+ pfl->blk_offset = -1;
+}
+
static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
uint32_t value, int width, int be)
{
uint8_t *p;
- trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
- p = pfl->storage + offset;
+ if (pfl->blk_offset != -1) {
+ /* block write: redirect writes to block update buffer */
+ if ((offset < pfl->blk_offset) ||
+ (offset + width > pfl->blk_offset + pfl->writeblock_size)) {
+ pfl->status |= 0x10; /* Programming error */
+ return;
+ }
+ trace_pflash_data_write_block(pfl->name, offset, width, value,
+ pfl->counter);
+ p = pfl->blk_bytes + (offset - pfl->blk_offset);
+ } else {
+ /* write directly to storage */
+ trace_pflash_data_write(pfl->name, offset, width, value);
+ p = pfl->storage + offset;
+ }
if (be) {
stn_be_p(p, width, value);
@@ -504,9 +573,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
} else {
value = extract32(value, 0, pfl->bank_width * 8);
}
- trace_pflash_write_block(pfl->name, value);
pfl->counter = value;
pfl->wcycle++;
+ pflash_blk_write_start(pfl, offset);
break;
case 0x60:
if (cmd == 0xd0) {
@@ -537,12 +606,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
switch (pfl->cmd) {
case 0xe8: /* Block write */
/* FIXME check @offset, @width */
- if (!pfl->ro) {
- /*
- * FIXME writing straight to memory is *wrong*. We
- * should write to a buffer, and flush it to memory
- * only on confirm command (see below).
- */
+ if (!pfl->ro && (pfl->blk_offset != -1)) {
pflash_data_write(pfl, offset, value, width, be);
} else {
pfl->status |= 0x10; /* Programming error */
@@ -551,18 +615,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
pfl->status |= 0x80;
if (!pfl->counter) {
- hwaddr mask = pfl->writeblock_size - 1;
- mask = ~mask;
-
trace_pflash_write(pfl->name, "block write finished");
pfl->wcycle++;
- if (!pfl->ro) {
- /* Flush the entire write buffer onto backing storage. */
- /* FIXME premature! */
- pflash_update(pfl, offset & mask, pfl->writeblock_size);
- } else {
- pfl->status |= 0x10; /* Programming error */
- }
}
pfl->counter--;
@@ -574,20 +628,17 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
case 3: /* Confirm mode */
switch (pfl->cmd) {
case 0xe8: /* Block write */
- if (cmd == 0xd0) {
- /* FIXME this is where we should write out the buffer */
+ if ((cmd == 0xd0) && !(pfl->status & 0x10)) {
+ pflash_blk_write_flush(pfl);
pfl->wcycle = 0;
pfl->status |= 0x80;
} else {
- qemu_log_mask(LOG_UNIMP,
- "%s: Aborting write to buffer not implemented,"
- " the data is already written to storage!\n"
- "Flash device reset into READ mode.\n",
- __func__);
+ pflash_blk_write_abort(pfl);
goto mode_read_array;
}
break;
default:
+ pflash_blk_write_abort(pfl);
goto error_flash;
}
break;
@@ -821,6 +872,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->cmd = 0x00;
pfl->status = 0x80; /* WSM ready */
pflash_cfi01_fill_cfi_table(pfl);
+
+ pfl->blk_bytes = g_malloc(pfl->writeblock_size);
+ pfl->blk_offset = -1;
}
static void pflash_cfi01_system_reset(DeviceState *dev)
@@ -840,6 +894,8 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
* This model deliberately ignores this delay.
*/
pfl->status = 0x80;
+
+ pfl->blk_offset = -1;
}
static Property pflash_cfi01_properties[] = {
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2a99b286b0..6fa56f14c0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -546,7 +546,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
}
goto reset_flash;
}
- trace_pflash_data_write(pfl->name, offset, width, value, 0);
+ trace_pflash_data_write(pfl->name, offset, width, value);
if (!pfl->ro) {
p = (uint8_t *)pfl->storage + offset;
if (pfl->be) {
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 2c45a62bd5..196493feae 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -12,7 +12,8 @@ fdctrl_tc_pulse(int level) "TC pulse: %u"
pflash_chip_erase_invalid(const char *name, uint64_t offset) "%s: chip erase: invalid address 0x%" PRIx64
pflash_chip_erase_start(const char *name) "%s: start chip erase"
pflash_data_read(const char *name, uint64_t offset, unsigned size, uint32_t value) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x"
-pflash_data_write(const char *name, uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64
+pflash_data_write(const char *name, uint64_t offset, unsigned size, uint32_t value) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x"
+pflash_data_write_block(const char *name, uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64
pflash_device_id(const char *name, uint16_t id) "%s: read device ID: 0x%04x"
pflash_device_info(const char *name, uint64_t offset) "%s: read device information offset:0x%04" PRIx64
pflash_erase_complete(const char *name) "%s: sector erase complete"
@@ -32,7 +33,9 @@ pflash_unlock0_failed(const char *name, uint64_t offset, uint8_t cmd, uint16_t a
pflash_unlock1_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: unlock0 failed 0x%" PRIx64 " 0x%02x"
pflash_unsupported_device_configuration(const char *name, uint8_t width, uint8_t max) "%s: unsupported device configuration: device_width:%d max_device_width:%d"
pflash_write(const char *name, const char *str) "%s: %s"
-pflash_write_block(const char *name, uint32_t value) "%s: block write: bytes:0x%x"
+pflash_write_block_start(const char *name, uint32_t value) "%s: block write start: bytes:0x%x"
+pflash_write_block_flush(const char *name) "%s: block write flush"
+pflash_write_block_abort(const char *name) "%s: block write abort"
pflash_write_block_erase(const char *name, uint64_t offset, uint64_t len) "%s: block erase offset:0x%" PRIx64 " bytes:0x%" PRIx64
pflash_write_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: command failed 0x%" PRIx64 " 0x%02x"
pflash_write_invalid(const char *name, uint8_t cmd) "%s: invalid write for command 0x%02x"
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 17/20] hw/scsi/esp-pci: use correct address register for PCI DMA transfers
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (7 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 16/20] hw/pflash: implement update buffer for block writes Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 18/20] hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources Michael Tokarev
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Guenter Roeck,
Philippe Mathieu-Daudé, Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
The current code in esp_pci_dma_memory_rw() sets the DMA address to the value
of the DMA_SPA (Starting Physical Address) register which is incorrect: this
means that for each callback from the SCSI layer the DMA address is set back
to the starting address.
In the case where only a single SCSI callback occurs (currently for transfer
lengths < 128kB) this works fine, however for larger transfers the DMA address
wraps back to the initial starting address, corrupting the buffer holding the
data transferred to the guest.
Fix esp_pci_dma_memory_rw() to use the DMA_WAC (Working Address Counter) for
the DMA address which is correctly incremented across multiple SCSI layer
transfers.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-ID: <20240112131529.515642-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 84a6835e004c257037492167d4f266dbb54dc33e)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 1792f84cea..40c6c926b6 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -275,7 +275,7 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
qemu_log_mask(LOG_UNIMP, "am53c974: MDL transfer not implemented\n");
}
- addr = pci->dma_regs[DMA_SPA];
+ addr = pci->dma_regs[DMA_WAC];
if (pci->dma_regs[DMA_WBC] < len) {
len = pci->dma_regs[DMA_WBC];
}
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 18/20] hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (8 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 17/20] hw/scsi/esp-pci: use correct address register for PCI DMA transfers Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 19/20] hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt Michael Tokarev
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Guenter Roeck,
Philippe Mathieu-Daudé, Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
The am53c974/dc390 PCI interrupt has two separate sources: the first is from the
internal ESP device, and the second is from the PCI DMA transfer logic.
Update the ESP interrupt handler so that it sets DMA_STAT_SCSIINT rather than
driving the PCI IRQ directly, and introduce a new esp_pci_update_irq() function
to generate the correct PCI IRQ level. In particular this fixes spurious interrupts
being generated by setting DMA_STAT_DONE at the end of a transfer if DMA_CMD_INTE_D
isn't set in the DMA_CMD register.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-ID: <20240112131529.515642-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 6b41417d934b2640b7ccf893544d656eea92a2e7)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: fixup in hw/scsi/esp-pci.c due to v8.0.0-1556-g7d5b0d6864
"bulk: Remove pointless QOM casts")
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 40c6c926b6..59ce14f843 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -77,6 +77,29 @@ struct PCIESPState {
ESPState esp;
};
+static void esp_pci_update_irq(PCIESPState *pci)
+{
+ int scsi_level = !!(pci->dma_regs[DMA_STAT] & DMA_STAT_SCSIINT);
+ int dma_level = (pci->dma_regs[DMA_CMD] & DMA_CMD_INTE_D) ?
+ !!(pci->dma_regs[DMA_STAT] & DMA_STAT_DONE) : 0;
+ int level = scsi_level || dma_level;
+
+ pci_set_irq(PCI_DEVICE(pci), level);
+}
+
+static void esp_irq_handler(void *opaque, int irq_num, int level)
+{
+ PCIESPState *pci = PCI_ESP(opaque);
+
+ if (level) {
+ pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
+ } else {
+ pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
+ }
+
+ esp_pci_update_irq(pci);
+}
+
static void esp_pci_handle_idle(PCIESPState *pci, uint32_t val)
{
ESPState *s = ESP(&pci->esp);
@@ -151,6 +174,7 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
/* clear some bits on write */
uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE;
pci->dma_regs[DMA_STAT] &= ~(val & mask);
+ esp_pci_update_irq(pci);
}
break;
default:
@@ -161,17 +185,14 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr)
{
- ESPState *s = ESP(&pci->esp);
uint32_t val;
val = pci->dma_regs[saddr];
if (saddr == DMA_STAT) {
- if (s->rregs[ESP_RSTAT] & STAT_INT) {
- val |= DMA_STAT_SCSIINT;
- }
if (!(pci->sbac & SBAC_STATUS)) {
pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT |
DMA_STAT_DONE);
+ esp_pci_update_irq(pci);
}
}
@@ -350,6 +371,7 @@ static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
esp_command_complete(req, resid);
pci->dma_regs[DMA_WBC] = 0;
pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
+ esp_pci_update_irq(pci);
}
static const struct SCSIBusInfo esp_pci_scsi_info = {
@@ -386,7 +408,7 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp)
"esp-io", 0x80);
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
- s->irq = pci_allocate_irq(dev);
+ s->irq = qemu_allocate_irq(esp_irq_handler, pci, 0);
scsi_bus_init(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 19/20] hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (9 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 18/20] hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 20/20] hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued Michael Tokarev
2024-01-23 11:22 ` [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Guenter Roeck,
Philippe Mathieu-Daudé, Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
The setting of DMA_STAT_DONE at the end of a DMA transfer can be configured to
generate an interrupt, however the Linux driver manually checks for DMA_STAT_DONE
being set and if it is, considers that a DMA transfer has completed.
If DMA_STAT_DONE is set but the ESP device isn't indicating an interrupt then
the Linux driver considers this to be a spurious interrupt. However this can
occur in QEMU as there is a delay between the end of DMA transfer where
DMA_STAT_DONE is set, and the ESP device raising its completion interrupt.
This appears to be an incorrect assumption in the Linux driver as the ESP and
PCI DMA interrupt sources are separate (and may not be raised exactly
together), however we can work around this by synchronising the setting of
DMA_STAT_DONE at the end of a DMA transfer with the ESP completion interrupt.
In conjunction with the previous commit Linux is now able to correctly boot
from an am53c974 PCI SCSI device on the hppa C3700 machine without emitting
"iget: checksum invalid" and "Spurious irq, sreg=10" errors.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-ID: <20240112131529.515642-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit 1e8e6644e063b20ad391140fae13d00ad7750b33)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 59ce14f843..e9e94070ec 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -93,6 +93,18 @@ static void esp_irq_handler(void *opaque, int irq_num, int level)
if (level) {
pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT;
+
+ /*
+ * If raising the ESP IRQ to indicate end of DMA transfer, set
+ * DMA_STAT_DONE at the same time. In theory this should be done in
+ * esp_pci_dma_memory_rw(), however there is a delay between setting
+ * DMA_STAT_DONE and the ESP IRQ arriving which is visible to the
+ * guest that can cause confusion e.g. Linux
+ */
+ if ((pci->dma_regs[DMA_CMD] & DMA_CMD_MASK) == 0x3 &&
+ pci->dma_regs[DMA_WBC] == 0) {
+ pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
+ }
} else {
pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT;
}
@@ -306,9 +318,6 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len,
/* update status registers */
pci->dma_regs[DMA_WBC] -= len;
pci->dma_regs[DMA_WAC] += len;
- if (pci->dma_regs[DMA_WBC] == 0) {
- pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
- }
}
static void esp_pci_dma_memory_read(void *opaque, uint8_t *buf, int len)
@@ -363,24 +372,13 @@ static const VMStateDescription vmstate_esp_pci_scsi = {
}
};
-static void esp_pci_command_complete(SCSIRequest *req, size_t resid)
-{
- ESPState *s = req->hba_private;
- PCIESPState *pci = container_of(s, PCIESPState, esp);
-
- esp_command_complete(req, resid);
- pci->dma_regs[DMA_WBC] = 0;
- pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE;
- esp_pci_update_irq(pci);
-}
-
static const struct SCSIBusInfo esp_pci_scsi_info = {
.tcq = false,
.max_target = ESP_MAX_DEVS,
.max_lun = 7,
.transfer_data = esp_transfer_data,
- .complete = esp_pci_command_complete,
+ .complete = esp_command_complete,
.cancel = esp_request_cancelled,
};
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Stable-7.2.9 20/20] hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (10 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 19/20] hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt Michael Tokarev
@ 2024-01-23 8:48 ` Michael Tokarev
2024-01-23 11:22 ` [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Michael Tokarev @ 2024-01-23 8:48 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Mark Cave-Ayland, Guenter Roeck,
Philippe Mathieu-Daudé, Michael Tokarev
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Even though the BLAST command isn't fully implemented in QEMU, the DMA_STAT_BCMBLT
bit should be set after the command has been issued to indicate that the command
has completed.
This fixes an issue with the DC390 DOS driver which issues the BLAST command as
part of its normal error recovery routine at startup, and otherwise sits in a
tight loop waiting for DMA_STAT_BCMBLT to be set before continuing.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-ID: <20240112131529.515642-5-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
(cherry picked from commit c2d7de557d19ec76eb83b87b6bf77c8114e2f183)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index e9e94070ec..6b7b963409 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -124,6 +124,7 @@ static void esp_pci_handle_blast(PCIESPState *pci, uint32_t val)
{
trace_esp_pci_dma_blast(val);
qemu_log_mask(LOG_UNIMP, "am53c974: cmd BLAST not implemented\n");
+ pci->dma_regs[DMA_STAT] |= DMA_STAT_BCMBLT;
}
static void esp_pci_handle_abort(PCIESPState *pci, uint32_t val)
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
` (11 preceding siblings ...)
2024-01-23 8:48 ` [Stable-7.2.9 20/20] hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued Michael Tokarev
@ 2024-01-23 11:22 ` Paolo Bonzini
12 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2024-01-23 11:22 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel; +Cc: qemu-stable
On 1/23/24 09:48, Michael Tokarev wrote:
> The following patches are queued for QEMU stable v7.2.9:
>
> https://gitlab.com/qemu-project/qemu/-/commits/staging-7.2
>
> Patch freeze is 2024-01-27, and the release is planned for 2024-01-29:
>
> https://wiki.qemu.org/Planning/7.2
>
> Please respond here or CC qemu-stable@nongnu.org on any additional patches
> you think should (or shouldn't) be included in the release.
>
> The changes which are staging for inclusion, with the original commit hash
> from master branch, are given below the bottom line.
I would like to include a minimal revert of the PCREL enablement, since
it is causing regressions that are not well understood. I've just sent
a patch for 7.2 and I am currently testing the forward port for 8.1.
Paolo
> Thanks!
>
> /mjt
>
> --------------------------------------
> 01* d3007d348ada Kevin Wolf:
> block: Fix crash when loading snapshot on inactive node
> 02* 5a7f21efaf99 Kevin Wolf:
> vl: Improve error message for conflicting -incoming and -loadvm
> 03* bb6e2511eb48 Kevin Wolf:
> iotests: Basic tests for internal snapshots
> 04* 5cb0e7abe163 Xu Lu:
> target/riscv: Fix mcycle/minstret increment behavior
> 05* 4ad87cd4b225 Michael Tokarev:
> chardev/char.c: fix "abstract device type" error message
> 06* 82a65e3188ab Peter Maydell:
> hw/intc/arm_gicv3_cpuif: handle LPIs in in the list registers
> 07* e358a25a97c7 Ilya Leoshkevich:
> target/s390x: Fix LAE setting a wrong access register
> 08* 52a21689cd82 Peter Maydell:
> .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large
> 09 410c2a4d75f5 Anastasia Belova:
> load_elf: fix iterator's type for elf file processing
> 10 b5e0d5d22fbf Richard Henderson:
> target/i386: Fix 32-bit wrapping of pc/eip computation
> 11 a58506b748b8 Richard Henderson:
> target/i386: Do not re-compute new pc with CF_PCREL
> 12 2926eab89699 guoguangyao:
> target/i386: fix incorrect EIP in PC-relative translation blocks
> 13 729ba8e933f8 Paolo Bonzini:
> target/i386: pcrel: store low bits of physical address in data[0]
> 14 3b14a555fdb6 Gerd Hoffmann:
> hw/pflash: refactor pflash_data_write()
> 15 5dd58358a570 Gerd Hoffmann:
> hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p
> 16 284a7ee2e290 Gerd Hoffmann:
> hw/pflash: implement update buffer for block writes
> 17 84a6835e004c Mark Cave-Ayland:
> hw/scsi/esp-pci: use correct address register for PCI DMA transfers
> 18 6b41417d934b Mark Cave-Ayland:
> hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources
> 19 1e8e6644e063 Mark Cave-Ayland:
> hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt
> 20 c2d7de557d19 Mark Cave-Ayland:
> hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued
>
> (commit(s) marked with * were in previous series and are not resent)
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-01-23 15:33 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-23 8:48 [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 09/20] load_elf: fix iterator's type for elf file processing Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 10/20] target/i386: Fix 32-bit wrapping of pc/eip computation Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 11/20] target/i386: Do not re-compute new pc with CF_PCREL Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 12/20] target/i386: fix incorrect EIP in PC-relative translation blocks Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 13/20] target/i386: pcrel: store low bits of physical address in data[0] Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 14/20] hw/pflash: refactor pflash_data_write() Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 15/20] hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 16/20] hw/pflash: implement update buffer for block writes Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 17/20] hw/scsi/esp-pci: use correct address register for PCI DMA transfers Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 18/20] hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 19/20] hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt Michael Tokarev
2024-01-23 8:48 ` [Stable-7.2.9 20/20] hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued Michael Tokarev
2024-01-23 11:22 ` [Stable-7.2.9 00/20] Patch Round-up for stable 7.2.9, freeze on 2024-01-27 Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).