Re: [PATCH] docs/devel: remove incorrect claim about git send-email
Linus Heckemann writes: > While it's unclear to me what git send-email actually does with the > -v2 parameter (it is not documented, but also not rejected), it does > not add a v2 tag to the email's subject, which is what led to the > mishap in [1]. > > [1]: https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg00679.html It does for me! Tested with: git send-email -v2 --to h...@alyssa.is HEAD~ X-Mailer: git-send-email 2.37.1 signature.asc Description: PGP signature
Re: [PATCH] docs/devel: remove incorrect claim about git send-email
Alyssa Ross writes: > Linus Heckemann writes: > >> While it's unclear to me what git send-email actually does with the >> -v2 parameter (it is not documented, but also not rejected), it does >> not add a v2 tag to the email's subject, which is what led to the >> mishap in [1]. >> >> [1]: https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg00679.html > > It does for me! > > Tested with: > >git send-email -v2 --to h...@alyssa.is HEAD~ > > X-Mailer: git-send-email 2.37.1 I wouldn't be surprised if it only adds it when it's generating the patch though. Did you perhaps run git format-patch first to generate a patch file, and then use git send-email to send it? signature.asc Description: PGP signature
[RISU PATCH 3/5] loongarch: Implement risugen module
Signed-off-by: Song Gao --- risugen_loongarch64.pm | 502 + 1 file changed, 502 insertions(+) create mode 100644 risugen_loongarch64.pm diff --git a/risugen_loongarch64.pm b/risugen_loongarch64.pm new file mode 100644 index 000..693fb71 --- /dev/null +++ b/risugen_loongarch64.pm @@ -0,0 +1,502 @@ +#!/usr/bin/perl -w +### +# Copyright (c) 2022 Loongson Technology Corporation Limited +# All rights reserved. This program and the accompanying materials +# are made available under the terms of the Eclipse Public License v1.0 +# which accompanies this distribution, and is available at +# http://www.eclipse.org/legal/epl-v10.html +# +# Contributors: +# based on Peter Maydell (Linaro) - initial implementation +### + +# risugen -- generate a test binary file for use with risu +# See 'risugen --help' for usage information. +package risugen_loongarch64; + +use strict; +use warnings; + +use risugen_common; + +require Exporter; + +our @ISA= qw(Exporter); +our @EXPORT = qw(write_test_code); + +my $periodic_reg_random = 1; + +# Maximum alignment restriction permitted for a memory op. +my $MAXALIGN = 64; + +my $OP_COMPARE = 0;# compare registers +my $OP_TESTEND = 1;# end of test, stop +my $OP_SETMEMBLOCK = 2;# r4 is address of memory block (8192 bytes) +my $OP_GETMEMBLOCK = 3;# add the address of memory block to r4 +my $OP_COMPAREMEM = 4; # compare memory block + +sub write_risuop($) +{ +my ($op) = @_; +insn32(0x01f0 | $op); +} + +sub write_set_fcsr($) +{ +my ($fcsr) = @_; +# movgr2fcsr r0, r0 +insn32(0x0114c000); +} + +# Global used to communicate between align(x) and reg() etc. +my $alignment_restriction; + +sub set_reg_w($) +{ +my($reg)=@_; +# Set reg [0x0, 0x7FFF] + +# $reg << 33 +# slli.d $reg, $reg, 33 +insn32(0x41 | 33 << 10 | $reg << 5 | $reg); +# $reg >> 33 +# srli.d $reg, $reg, 33 +insn32(0x45 | 33 << 10 | $reg << 5 | $reg); + +return $reg; +} + +sub align($) +{ +my ($a) = @_; +if (!is_pow_of_2($a) || ($a < 0) || ($a > $MAXALIGN)) { +die "bad align() value $a\n"; +} +$alignment_restriction = $a; +} + +sub write_sub_rrr($$$) +{ +my ($rd, $rj, $rk) = @_; +# sub.d rd, rj, rk +insn32(0x00118000 | $rk << 10 | $rj << 5 | $rd); +} + +sub write_mov_rr($$$) +{ +my($rd, $rj, $rk) = @_; +# add.d rd, rj, r0 +insn32(0x00108000 | 0 << 10 | $rj << 5 | $rd); +} + +sub write_mov_positive_ri($$) +{ +# Use lu12i.w and ori instruction +my ($rd, $imm) = @_; +my $high_20 = ($imm >> 12) & 0xf; + +if ($high_20) { +# lu12i.w rd, si20 +insn32(0x1400 | $high_20 << 5 | $rd); +# ori rd, rd, ui12 +insn32(0x0380 | ($imm & 0xfff) << 10 | $rd << 5 | $rd); +} else { +# ori rd, 0, ui12 +insn32(0x0380 | ($imm & 0xfff) << 10 | 0 << 5 | $rd); +} +} + +sub write_mov_ri($$) +{ +my ($rd, $imm) = @_; + +if ($imm < 0) { +my $tmp = 0 - $imm ; +write_mov_positive_ri($rd, $tmp); +write_sub_rrr($rd, 0, $rd); +} else { +write_mov_positive_ri($rd, $imm); +} +} + +sub write_get_offset() +{ +# Emit code to get a random offset within the memory block, of the +# right alignment, into r4 +# We require the offset to not be within 256 bytes of either +# end, to (more than) allow for the worst case data transfer, which is +# 16 * 64 bit regs +my $offset = (rand(2048 - 512) + 256) & ~($alignment_restriction - 1); +write_mov_ri(4, $offset); +write_risuop($OP_GETMEMBLOCK); +} + +sub reg_plus_reg($$@) +{ +my ($base, $idx, @trashed) = @_; +my $savedidx = 0; +if ($idx == 4) { +# Save the index into some other register for the +# moment, because the risuop will trash r4. +$idx = 5; +$idx++ if $idx == $base; +$savedidx = 1; +write_mov_rr($idx, 4, 0); +} +# Get a random offset within the memory block, of the +# right alignment. +write_get_offset(); + +write_sub_rrr($base, 4, $idx); +if ($base != 4) { +if ($savedidx) { +write_mov_rr(4, $idx, 0); +write_mov_ri($idx, 0); +} else { +write_mov_ri(4, 0); +} +} else { + if ($savedidx) { +write_mov_ri($idx, 0); + } +} + +if (grep $_ == $base, @trashed) { +return -1; +} +return $base; +} + +sub reg_plus_imm($$@) +{ +# Handle reg + immediate addressing mode +my ($base, $imm, @trashed) = @_; + +write_get_offset(); +# Now r4 is the address we want to do the access to, +# so set the basereg by doing the inverse of the +# addressing mode calculation, ie base = r4 - imm +# We could do this more cleverly wi
[RISU PATCH 2/5] loongarch: Add LoongArch basic test support
This patch adds LoongArch server, client support, and basic test file. Signed-off-by: Song Gao --- risu_loongarch64.c | 50 ++ risu_reginfo_loongarch64.c | 183 + risu_reginfo_loongarch64.h | 25 + test_loongarch64.s | 92 +++ 4 files changed, 350 insertions(+) create mode 100644 risu_loongarch64.c create mode 100644 risu_reginfo_loongarch64.c create mode 100644 risu_reginfo_loongarch64.h create mode 100644 test_loongarch64.s diff --git a/risu_loongarch64.c b/risu_loongarch64.c new file mode 100644 index 000..24599e1 --- /dev/null +++ b/risu_loongarch64.c @@ -0,0 +1,50 @@ +/** + * Copyright (c) 2022 Loongson Technology Corporation Limited + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * based on Peter Maydell's risu_arm.c + */ + +#include +#include +#include + +#include "risu.h" + +void advance_pc(void *vuc) +{ +struct ucontext *uc = vuc; +uc->uc_mcontext.sc_pc += 4; +} + +void set_ucontext_paramreg(void *vuc, uint64_t value) +{ +struct ucontext *uc = vuc; +uc->uc_mcontext.sc_regs[4] = value; +} + +uint64_t get_reginfo_paramreg(struct reginfo *ri) +{ +return ri->regs[4]; +} + +int get_risuop(struct reginfo *ri) +{ +/* Return the risuop we have been asked to do + * (or -1 if this was a SIGILL for a non-risuop insn) + */ +uint32_t insn = ri->faulting_insn; +uint32_t op = insn & 0xf; +uint32_t key = insn & ~0xf; +uint32_t risukey = 0x01f0; +return (key != risukey) ? -1 : op; +} + +uintptr_t get_pc(struct reginfo *ri) +{ + return ri->pc; +} diff --git a/risu_reginfo_loongarch64.c b/risu_reginfo_loongarch64.c new file mode 100644 index 000..af6ab77 --- /dev/null +++ b/risu_reginfo_loongarch64.c @@ -0,0 +1,183 @@ +/** + * Copyright (c) 2022 Loongson Technology Corporation Limited + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * based on Peter Maydell's risu_reginfo_arm.c + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "risu.h" +#include "risu_reginfo_loongarch64.h" + +const struct option * const arch_long_opts; +const char * const arch_extra_help; + +struct _ctx_layout { +struct sctx_info *addr; +unsigned int size; +}; + +struct extctx_layout { +unsigned long size; +unsigned int flags; +struct _ctx_layout fpu; +struct _ctx_layout end; +}; + +void process_arch_opt(int opt, const char *arg) +{ +abort(); +} + +void arch_init(void) +{ +} + +int reginfo_size(struct reginfo *ri) +{ +return sizeof(*ri); +} + +static int parse_extcontext(struct sigcontext *sc, struct extctx_layout *extctx) +{ +uint32_t magic, size; +struct sctx_info *info = (struct sctx_info *)&sc->sc_extcontext; + +while(1) { +magic = (uint32_t)info->magic; +size = (uint32_t)info->size; +switch (magic) { +case 0: /* END*/ +return 0; +case FPU_CTX_MAGIC: +if (size < (sizeof(struct sctx_info) + +sizeof(struct fpu_context))) { +return -1; +} +extctx->fpu.addr = info; +break; +default: +return -1; + } + info = (struct sctx_info *)((char *)info +size); +} +return 0; +} + +/* reginfo_init: initialize with a ucontext */ +void reginfo_init(struct reginfo *ri, ucontext_t *context) +{ +int i; +struct ucontext *uc = (struct ucontext *)context; +struct extctx_layout extctx; + +memset(&extctx, 0, sizeof(struct extctx_layout)); +memset(ri, 0, sizeof(*ri)); + +for (i = 1; i < 32; i++) { +ri->regs[i] = uc->uc_mcontext.sc_regs[i]; //sp:r3, tp:r2 +} + +ri->regs[2] = 0xdeadbeefdeadbeef; +ri->pc = uc->uc_mcontext.sc_pc - (unsigned long)image_start_address; +ri->flags = uc->uc_mcontext.sc_flags; +ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.sc_pc; + +parse_extcontext(&uc->uc_mcontext, &extctx); +if (extctx.fpu.addr) { +struct sctx_info *info = extctx.fpu.addr; +struct fpu_context *fpu_ctx = (struct fpu_context *)((char *)info + +
[RISU PATCH 0/5] Add LoongArch architectures support
hi, This series adds LoongArch architectures support, we had tested two mode: 1. LoongArch host server + LoongArch host client; 2. LoongArch host server + qemu client. You can find all LoongArch instructions at [1]. This series not contains all LoongArch instructions, such as pcadd, syscalls, rdtime and jumps. [1]: https://github.com/loongson/LoongArch-Documentation/releases/download/2022.08.12/LoongArch-Vol1-v1.02-EN.pdf Thanks. Song Gao Song Gao (5): risu: Use alternate stack loongarch: Add LoongArch basic test support loongarch: Implement risugen module loongarch: Add risufile with loongarch instructions loongarch: Add block 'safefloat' and nanbox_s() loongarch64.risu | 612 + risu.c | 16 +- risu_loongarch64.c | 50 +++ risu_reginfo_loongarch64.c | 183 +++ risu_reginfo_loongarch64.h | 25 ++ risugen| 2 +- risugen_loongarch64.pm | 532 test_loongarch64.s | 92 ++ 8 files changed, 1510 insertions(+), 2 deletions(-) create mode 100644 loongarch64.risu create mode 100644 risu_loongarch64.c create mode 100644 risu_reginfo_loongarch64.c create mode 100644 risu_reginfo_loongarch64.h create mode 100644 risugen_loongarch64.pm create mode 100644 test_loongarch64.s -- 2.31.1
[RISU PATCH 5/5] loongarch: Add block 'safefloat' and nanbox_s()
Some LoongArch instructions don't care the high 32bit, so use nanbox_s() set the high 32bit 0x. Signed-off-by: Song Gao --- loongarch64.risu | 119 +++-- risugen| 2 +- risugen_loongarch64.pm | 30 +++ 3 files changed, 110 insertions(+), 41 deletions(-) diff --git a/loongarch64.risu b/loongarch64.risu index d059811..d625a12 100644 --- a/loongarch64.risu +++ b/loongarch64.risu @@ -62,7 +62,7 @@ mulw_d_wu LA64 0001 1 rk:5 rj:5 rd:5 \ !constraints { $rk != 2 && $rj != 2 && $rd != 2; } #div.{w[u]/d[u]} rd,rj,rk -# the docement 2.2.13, rk, rj, need in 32bit [0x0 ~0x7FFF] +# div.w{u}, mod.w[u] rk, rj, need in [0x0 ~0x7FFF] # use function set_reg_w($reg) div_w LA64 0010 0 rk:5 rj:5 rd:5 \ !constraints { $rk != 2 && $rj != 2 && $rd != 2; } \ @@ -436,47 +436,68 @@ crcc_w_d_w LA64 0010 0 rk:5 rj:5 rd:5 \ # # Floating point arithmetic operation instruction # -fadd_s LA64 0001 1 fk:5 fj:5 fd:5 +fadd_s LA64 0001 1 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fadd_d LA64 0001 00010 fk:5 fj:5 fd:5 -fsub_s LA64 0001 00101 fk:5 fj:5 fd:5 +fsub_s LA64 0001 00101 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fsub_d LA64 0001 00110 fk:5 fj:5 fd:5 -fmul_s LA64 0001 01001 fk:5 fj:5 fd:5 +fmul_s LA64 0001 01001 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmul_d LA64 0001 01010 fk:5 fj:5 fd:5 -fdiv_s LA64 0001 01101 fk:5 fj:5 fd:5 +fdiv_s LA64 0001 01101 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fdiv_d LA64 0001 01110 fk:5 fj:5 fd:5 -fmadd_s LA64 1001 fa:5 fk:5 fj:5 fd:5 +fmadd_s LA64 1001 fa:5 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmadd_d LA64 1010 fa:5 fk:5 fj:5 fd:5 -fmsub_s LA64 1101 fa:5 fk:5 fj:5 fd:5 +fmsub_s LA64 1101 fa:5 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmsub_d LA64 1110 fa:5 fk:5 fj:5 fd:5 -fnmadd_s LA64 10001001 fa:5 fk:5 fj:5 fd:5 +fnmadd_s LA64 10001001 fa:5 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fnmadd_d LA64 10001010 fa:5 fk:5 fj:5 fd:5 -fnmsub_s LA64 10001101 fa:5 fk:5 fj:5 fd:5 +fnmsub_s LA64 10001101 fa:5 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fnmsub_d LA64 10001110 fa:5 fk:5 fj:5 fd:5 -fmax_s LA64 0001 10001 fk:5 fj:5 fd:5 +fmax_s LA64 0001 10001 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmax_d LA64 0001 10010 fk:5 fj:5 fd:5 -fmin_s LA64 0001 10101 fk:5 fj:5 fd:5 +fmin_s LA64 0001 10101 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmin_d LA64 0001 10110 fk:5 fj:5 fd:5 -fmaxa_s LA64 0001 11001 fk:5 fj:5 fd:5 +fmaxa_s LA64 0001 11001 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmaxa_d LA64 0001 11010 fk:5 fj:5 fd:5 -fmina_s LA64 0001 11101 fk:5 fj:5 fd:5 +fmina_s LA64 0001 11101 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fmina_d LA64 0001 0 fk:5 fj:5 fd:5 -fabs_s LA64 00010001 01000 1 fj:5 fd:5 +fabs_s LA64 00010001 01000 1 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fabs_d LA64 00010001 01000 00010 fj:5 fd:5 -fneg_s LA64 00010001 01000 00101 fj:5 fd:5 +fneg_s LA64 00010001 01000 00101 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fneg_d LA64 00010001 01000 00110 fj:5 fd:5 -fsqrt_s LA64 00010001 01000 10001 fj:5 fd:5 +fsqrt_s LA64 00010001 01000 10001 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fsqrt_d LA64 00010001 01000 10010 fj:5 fd:5 -frecip_s LA64 00010001 01000 10101 fj:5 fd:5 +frecip_s LA64 00010001 01000 10101 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } frecip_d LA64 00010001 01000 10110 fj:5 fd:5 -frsqrt_s LA64 00010001 01000 11001 fj:5 fd:5 +frsqrt_s LA64 00010001 01000 11001 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } frsqrt_d LA64 00010001 01000 11010 fj:5 fd:5 -fscaleb_s LA64 00010001 1 fk:5 fj:5 fd:5 +fscaleb_s LA64 00010001 1 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fscaleb_d LA64 00010001 00010 fk:5 fj:5 fd:5 -flogb_s LA64 00010001 01000 01001 fj:5 fd:5 +flogb_s LA64 00010001 01000 01001 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } flogb_d LA64 00010001 01000 01010 fj:5 fd:5 -fcopysign_s LA64 00010001 00101 fk:5 fj:5 fd:5 +fcopysign_s LA64 00010001 00101 fk:5 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fcopysign_d LA64 00010001 00110 fk:5 fj:5 fd:5 -fclass_s LA64 00010001 01000 01101 fj:5 fd:5 +fclass_s LA64 00010001 01000 01101 fj:5 fd:5 \ +!safefloat { nanbox_s($fd); } fclass_d LA64 00010001 01000 01110 fj:5 fd:5 # @@ -490,43 +511,59 @@ fcmp_cond_d LA64 1110 cond:5 fk:5 fj:5 00 cd:3 \ # # Floating point con
[RISU PATCH 4/5] loongarch: Add risufile with loongarch instructions
Signed-off-by: Song Gao --- loongarch64.risu | 573 +++ 1 file changed, 573 insertions(+) create mode 100644 loongarch64.risu diff --git a/loongarch64.risu b/loongarch64.risu new file mode 100644 index 000..d059811 --- /dev/null +++ b/loongarch64.risu @@ -0,0 +1,573 @@ +### +# Copyright (c) 2022 Loongson Technology Corporation Limited +# All rights reserved. This program and the accompanying materials +# are made available under the terms of the Eclipse Public License v1.0 +# which accompanies this distribution, and is available at +# http://www.eclipse.org/legal/epl-v10.html +# +# Contributors: +# based on aarch64.risu by Claudio Fontana +# based on arm.risu by Peter Maydell +### + +# Input file for risugen defining LoongArch64 instructions +.mode loongarch64 + +# +# Fixed point arithmetic operation instruction +# +add_w LA64 0001 0 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +add_d LA64 0001 1 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +sub_w LA64 0001 00010 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +sub_d LA64 0001 00011 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +slt LA64 0001 00100 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +sltu LA64 0001 00101 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +slti LA64 001000 si12:12 rj:5 rd:5 \ +!constraints { $rj != 2 && $rd != 2; } +sltui LA64 001001 si12:12 rj:5 rd:5 \ +!constraints { $rj != 2 && $rd != 2; } +nor LA64 0001 01000 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +and LA64 0001 01001 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +or LA64 0001 01010 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +xor LA64 0001 01011 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +orn LA64 0001 01100 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +andn LA64 0001 01101 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mul_w LA64 0001 11000 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mul_d LA64 0001 11011 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulh_w LA64 0001 11001 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulh_d LA64 0001 11100 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulh_wu LA64 0001 11010 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulh_du LA64 0001 11101 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulw_d_w LA64 0001 0 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mulw_d_wu LA64 0001 1 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } + +#div.{w[u]/d[u]} rd,rj,rk +# the docement 2.2.13, rk, rj, need in 32bit [0x0 ~0x7FFF] +# use function set_reg_w($reg) +div_w LA64 0010 0 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } \ +!memory { set_reg_w($rj); set_reg_w($rk); } +div_wu LA64 0010 00010 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } \ +!memory { set_reg_w($rj); set_reg_w($rk); } +div_d LA64 0010 00100 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +div_du LA64 0010 00110 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mod_w LA64 0010 1 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } \ +!memory { set_reg_w($rj); set_reg_w($rk); } +mod_wu LA64 0010 00011 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } \ +!memory { set_reg_w($rj); set_reg_w($rk); } +mod_d LA64 0010 00101 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +mod_du LA64 0010 00111 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } + +alsl_w LA64 010 sa2:2 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +alsl_wu LA64 011 sa2:2 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +alsl_d LA64 0010 110 sa2:2 rk:5 rj:5 rd:5 \ +!constraints { $rk != 2 && $rj != 2 && $rd != 2; } +lu12i_w LA64 0001 010 si20:20 rd:5 \ +!constraints { $rd != 2; } +lu32i_d LA64 0001 011 si20:20 rd:5 \ +!constraints { $rd != 2; } +lu52i_d LA64 001100 si12:12 rj:5 r
[RISU PATCH 1/5] risu: Use alternate stack
We can use alternate stack, so that we can use sp register as intput/ouput register. I had tested aarch64/LoongArch architecture. Signed-off-by: Song Gao --- risu.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/risu.c b/risu.c index 1c096a8..714074e 100644 --- a/risu.c +++ b/risu.c @@ -329,7 +329,7 @@ static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *)) memset(&sa, 0, sizeof(struct sigaction)); sa.sa_sigaction = fn; -sa.sa_flags = SA_SIGINFO; +sa.sa_flags = SA_SIGINFO | SA_ONSTACK; sigemptyset(&sa.sa_mask); if (sigaction(SIGILL, &sa, 0) != 0) { perror("sigaction"); @@ -550,6 +550,7 @@ int main(int argc, char **argv) char *trace_fn = NULL; struct option *longopts; char *shortopts; +stack_t ss; longopts = setup_options(&shortopts); @@ -617,6 +618,19 @@ int main(int argc, char **argv) load_image(imgfile); +/* create alternate stack */ +ss.ss_sp = malloc(SIGSTKSZ); +if (ss.ss_sp == NULL) { +perror("malloc"); +exit(EXIT_FAILURE); +} +ss.ss_size = SIGSTKSZ; +ss.ss_flags = 0; +if (sigaltstack(&ss, NULL) == -1) { +perror("sigaltstac"); +exit(EXIT_FAILURE); +} + /* E.g. select requested SVE vector length. */ arch_init(); -- 2.31.1
[PATCH 2/5] target/loongarch: bstrins.w need set dest register EXT_SIGN
Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_bit.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc index 9337714ec4..33e94878fd 100644 --- a/target/loongarch/insn_trans/trans_bit.c.inc +++ b/target/loongarch/insn_trans/trans_bit.c.inc @@ -37,7 +37,7 @@ static bool gen_rr_ms_ls(DisasContext *ctx, arg_rr_ms_ls *a, DisasExtend src_ext, DisasExtend dst_ext, void (*func)(TCGv, TCGv, unsigned int, unsigned int)) { -TCGv dest = gpr_dst(ctx, a->rd, dst_ext); +TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE); TCGv src1 = gpr_src(ctx, a->rj, src_ext); if (a->ls > a->ms) { @@ -206,7 +206,7 @@ TRANS(maskeqz, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_maskeqz) TRANS(masknez, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_masknez) TRANS(bytepick_w, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_w) TRANS(bytepick_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_d) -TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) +TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, gen_bstrins) TRANS(bstrins_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) TRANS(bstrpick_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, tcg_gen_extract_tl) TRANS(bstrpick_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, tcg_gen_extract_tl) -- 2.31.1
[PATCH 4/5] target/loongarch: flogb_{s/d} add set float_flag_divbyzero
if fj ==0 or fj == INT32_MIN/INT64_MIN, LoongArch host set fcsr cause exception FP_DIV0, So we need set exception flags float_flagdivbyzero if fj ==0. Signed-off-by: Song Gao --- target/loongarch/fpu_helper.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/target/loongarch/fpu_helper.c b/target/loongarch/fpu_helper.c index 1a24667eaf..8ddfbd1abd 100644 --- a/target/loongarch/fpu_helper.c +++ b/target/loongarch/fpu_helper.c @@ -322,6 +322,13 @@ uint64_t helper_flogb_s(CPULoongArchState *env, uint64_t fj) fp = float32_log2((uint32_t)fj, status); fd = nanbox_s(float32_round_to_int(fp, status)); set_float_rounding_mode(old_mode, status); +/* + * LoongArch host if fj == 0 or INT32_MIN , set the fcsr cause FP_DIV0 + * so we need set exception flags float_flag_divbyzero. + */ +if (((uint32_t)fj == 0) | ((uint32_t)fj == INT32_MIN )) { +set_float_exception_flags(float_flag_divbyzero, status); +} update_fcsr0_mask(env, GETPC(), float_flag_inexact); return fd; } @@ -336,6 +343,13 @@ uint64_t helper_flogb_d(CPULoongArchState *env, uint64_t fj) fd = float64_log2(fj, status); fd = float64_round_to_int(fd, status); set_float_rounding_mode(old_mode, status); +/* + * LoongArch host if fj == 0 or INT64_MIN , set the fcsr cause FP_DIV0 + * so we need set exception flags float_flag_divbyzero. + */ +if ( (fj == 0) | (fj == INT64_MIN)) { +set_float_exception_flags(float_flag_divbyzero, status); +} update_fcsr0_mask(env, GETPC(), float_flag_inexact); return fd; } -- 2.31.1
[PATCH 1/5] target/loongarch: ftint_xxx insns set the result high 32bit 0xffffffff
we just set high 32bit 0x as the other float instructions do. Signed-off-by: Song Gao --- target/loongarch/fpu_helper.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/target/loongarch/fpu_helper.c b/target/loongarch/fpu_helper.c index 4b9637210a..1a24667eaf 100644 --- a/target/loongarch/fpu_helper.c +++ b/target/loongarch/fpu_helper.c @@ -518,7 +518,7 @@ uint64_t helper_frint_s(CPULoongArchState *env, uint64_t fj) { uint64_t fd; -fd = (uint64_t)(float32_round_to_int((uint32_t)fj, &env->fp_status)); +fd = nanbox_s(float32_round_to_int((uint32_t)fj, &env->fp_status)); update_fcsr0(env, GETPC()); return fd; } @@ -574,7 +574,7 @@ uint64_t helper_ftintrm_w_d(CPULoongArchState *env, uint64_t fj) FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); set_float_rounding_mode(float_round_down, &env->fp_status); -fd = (uint64_t)float64_to_int32(fj, &env->fp_status); +fd = nanbox_s(float64_to_int32(fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -592,7 +592,7 @@ uint64_t helper_ftintrm_w_s(CPULoongArchState *env, uint64_t fj) FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); set_float_rounding_mode(float_round_down, &env->fp_status); -fd = (uint64_t)float32_to_int32((uint32_t)fj, &env->fp_status); +fd = nanbox_s(float32_to_int32((uint32_t)fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -646,7 +646,7 @@ uint64_t helper_ftintrp_w_d(CPULoongArchState *env, uint64_t fj) FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); set_float_rounding_mode(float_round_up, &env->fp_status); -fd = (uint64_t)float64_to_int32(fj, &env->fp_status); +fd = nanbox_s(float64_to_int32(fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -664,7 +664,7 @@ uint64_t helper_ftintrp_w_s(CPULoongArchState *env, uint64_t fj) FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); set_float_rounding_mode(float_round_up, &env->fp_status); -fd = (uint64_t)float32_to_int32((uint32_t)fj, &env->fp_status); +fd = nanbox_s(float32_to_int32((uint32_t)fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -715,7 +715,7 @@ uint64_t helper_ftintrz_w_d(CPULoongArchState *env, uint64_t fj) uint64_t fd; FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); -fd = (uint64_t)float64_to_int32_round_to_zero(fj, &env->fp_status); +fd = nanbox_s(float64_to_int32_round_to_zero(fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -786,7 +786,7 @@ uint64_t helper_ftintrne_w_d(CPULoongArchState *env, uint64_t fj) FloatRoundMode old_mode = get_float_rounding_mode(&env->fp_status); set_float_rounding_mode(float_round_nearest_even, &env->fp_status); -fd = (uint64_t)float64_to_int32(fj, &env->fp_status); +fd = nanbox_s(float64_to_int32(fj, &env->fp_status)); set_float_rounding_mode(old_mode, &env->fp_status); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { @@ -848,7 +848,7 @@ uint64_t helper_ftint_w_s(CPULoongArchState *env, uint64_t fj) { uint64_t fd; -fd = (uint64_t)float32_to_int32((uint32_t)fj, &env->fp_status); +fd = nanbox_s(float32_to_int32((uint32_t)fj, &env->fp_status)); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { if (float32_is_any_nan((uint32_t)fj)) { fd = 0; @@ -862,7 +862,7 @@ uint64_t helper_ftint_w_d(CPULoongArchState *env, uint64_t fj) { uint64_t fd; -fd = (uint64_t)float64_to_int32(fj, &env->fp_status); +fd = nanbox_s(float64_to_int32(fj, &env->fp_status)); if (get_float_exception_flags(&env->fp_status) & (float_flag_invalid)) { if (float64_is_any_nan(fj)) { fd = 0; -- 2.31.1
Re: [PATCH] RISC-V: Add support for Ztso
On 9/16/22 14:52, Palmer Dabbelt wrote: Though, honestly, I've had patches to add the required barriers sitting around for the last few releases, to better support things like x86 on aarch64. I should just finish that up. I can just do that for the RISC-V TSO support? Like the cover letter says that was my first thought, it's only when I found the comment saying not to do it that I went this way. My patches inject the barriers automatically by the tcg optimizer, rather than by hand, which is what the comment was trying to discourage. Last version was https://lore.kernel.org/qemu-devel/20210316220735.2048137-1-richard.hender...@linaro.org/ r~
[PATCH 0/5] Fix some bugs
hi, This series fix some bugs find from RISU test. Thanks. Song Gao Song Gao (5): target/loongarch: ftint_xxx insns set the result high 32bit 0x target/loongarch: bstrins.w need set dest register EXT_SIGN target/loongarch: Fix fnm{sub/add}_{s/d} set wrong flags target/loongarch: flogb_{s/d} add set float_flag_divbyzero target/loongarch: div if x/0 set dividend to 0 target/loongarch/fpu_helper.c | 32 - target/loongarch/insn_trans/trans_arith.c.inc | 34 +++ target/loongarch/insn_trans/trans_bit.c.inc | 4 +-- .../loongarch/insn_trans/trans_farith.c.inc | 12 +++ 4 files changed, 58 insertions(+), 24 deletions(-) -- 2.31.1
[PATCH 3/5] target/loongarch: Fix fnm{sub/add}_{s/d} set wrong flags
Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_farith.c.inc | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/loongarch/insn_trans/trans_farith.c.inc b/target/loongarch/insn_trans/trans_farith.c.inc index 65ad2ffab8..7bb3f41aee 100644 --- a/target/loongarch/insn_trans/trans_farith.c.inc +++ b/target/loongarch/insn_trans/trans_farith.c.inc @@ -97,9 +97,9 @@ TRANS(fmadd_s, gen_muladd, gen_helper_fmuladd_s, 0) TRANS(fmadd_d, gen_muladd, gen_helper_fmuladd_d, 0) TRANS(fmsub_s, gen_muladd, gen_helper_fmuladd_s, float_muladd_negate_c) TRANS(fmsub_d, gen_muladd, gen_helper_fmuladd_d, float_muladd_negate_c) -TRANS(fnmadd_s, gen_muladd, gen_helper_fmuladd_s, - float_muladd_negate_product | float_muladd_negate_c) -TRANS(fnmadd_d, gen_muladd, gen_helper_fmuladd_d, - float_muladd_negate_product | float_muladd_negate_c) -TRANS(fnmsub_s, gen_muladd, gen_helper_fmuladd_s, float_muladd_negate_product) -TRANS(fnmsub_d, gen_muladd, gen_helper_fmuladd_d, float_muladd_negate_product) +TRANS(fnmadd_s, gen_muladd, gen_helper_fmuladd_s, float_muladd_negate_result) +TRANS(fnmadd_d, gen_muladd, gen_helper_fmuladd_d, float_muladd_negate_result) +TRANS(fnmsub_s, gen_muladd, gen_helper_fmuladd_s, + float_muladd_negate_c | float_muladd_negate_result) +TRANS(fnmsub_d, gen_muladd, gen_helper_fmuladd_d, + float_muladd_negate_c | float_muladd_negate_result) -- 2.31.1
[PATCH v2] RISC-V: Add support for Ztso
The Ztso extension was recently frozen, this adds it as a CPU property and adds various fences throughout the port in order to allow TSO targets to function on weaker hosts. We need no fences for AMOs as they're already SC, the placess we need barriers are described. Signed-off-by: Palmer Dabbelt --- Like the v1 this has been pretty minimally tested, but I figured I'd just send it. This is what I would have done the first time had I not read that comment near TCG_GUEST_DEFAULT_MO, I think trying to describe that when I ran into Richard at the Cauldron was probably more confusing than just digging up the code and sending it along. --- target/riscv/cpu.c | 3 +++ target/riscv/cpu.h | 1 + target/riscv/insn_trans/trans_rva.c.inc | 11 --- target/riscv/insn_trans/trans_rvi.c.inc | 16 ++-- target/riscv/insn_trans/trans_rvv.c.inc | 20 target/riscv/translate.c| 3 +++ 6 files changed, 49 insertions(+), 5 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ac6f82ebd0..d66169efa5 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -919,6 +919,8 @@ static Property riscv_cpu_extensions[] = { DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false), DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false), +DEFINE_PROP_BOOL("ztso", RISCVCPU, cfg.ext_ztso, false), + /* Vendor-specific custom extensions */ DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, false), @@ -1094,6 +1096,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) ISA_EDATA_ENTRY(zksed, ext_zksed), ISA_EDATA_ENTRY(zksh, ext_zksh), ISA_EDATA_ENTRY(zkt, ext_zkt), +ISA_EDATA_ENTRY(ztso, ext_ztso), ISA_EDATA_ENTRY(zve32f, ext_zve32f), ISA_EDATA_ENTRY(zve64f, ext_zve64f), ISA_EDATA_ENTRY(zhinx, ext_zhinx), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 5c7acc055a..c64fd4e258 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -433,6 +433,7 @@ struct RISCVCPUConfig { bool ext_zve32f; bool ext_zve64f; bool ext_zmmul; +bool ext_ztso; bool rvv_ta_all_1s; uint32_t mvendorid; diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc index 45db82c9be..9066e1bde3 100644 --- a/target/riscv/insn_trans/trans_rva.c.inc +++ b/target/riscv/insn_trans/trans_rva.c.inc @@ -26,7 +26,11 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop) tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); } tcg_gen_qemu_ld_tl(load_val, src1, ctx->mem_idx, mop); -if (a->aq) { +/* + * TSO defines AMOs as acquire+release-RCsc, but does not define LR/SC as + * AMOs. Instead treat them like loads. + */ +if (a->aq || ctx->ztso) { tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } @@ -61,9 +65,10 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop) gen_set_label(l1); /* * Address comparison failure. However, we still need to - * provide the memory barrier implied by AQ/RL. + * provide the memory barrier implied by AQ/RL/TSO. */ -tcg_gen_mb(TCG_MO_ALL + a->aq * TCG_BAR_LDAQ + a->rl * TCG_BAR_STRL); +TCGBar bar_strl = (ctx->ztso || a->rl) ? TCG_BAR_STRL : 0; +tcg_gen_mb(TCG_MO_ALL + a->aq * TCG_BAR_LDAQ + bar_strl); gen_set_gpr(ctx, a->rd, tcg_constant_tl(1)); gen_set_label(l2); diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index ca8e3d1ea1..9bef42a3e5 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -261,11 +261,19 @@ static bool gen_load_i128(DisasContext *ctx, arg_lb *a, MemOp memop) static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop) { +bool out; + if (get_xl(ctx) == MXL_RV128) { -return gen_load_i128(ctx, a, memop); +out = gen_load_i128(ctx, a, memop); } else { -return gen_load_tl(ctx, a, memop); +out = gen_load_tl(ctx, a, memop); +} + +if (ctx->ztso) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } + +return out; } static bool trans_lb(DisasContext *ctx, arg_lb *a) @@ -322,6 +330,10 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a, MemOp memop) TCGv addr = get_address(ctx, a->rs1, a->imm); TCGv data = get_gpr(ctx, a->rs2, EXT_NONE); +if (ctx->ztso) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); +} + tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop); return true; } diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index 6c091824b6..1994b38035 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -671,8 +671,28 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data
[PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
div.d, div.du, div,w, div.wu, the LoongArch host if x/0 the result is 0. So we set the divisor to 1 and the dividend to 0. Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_arith.c.inc | 34 +++ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc index 8e45eadbc8..c97afb16f9 100644 --- a/target/loongarch/insn_trans/trans_arith.c.inc +++ b/target/loongarch/insn_trans/trans_arith.c.inc @@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2) tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2); } +static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv src2) +{ +TCGv zero = tcg_constant_tl(0); +TCGv one = tcg_constant_tl(1); + +/* + * If x / 0, set the diviend to 0 set the divisor to 1 + * this is the same with LoongArch host. + */ +tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1); +tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2); +} + static void gen_div_d(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -prep_divisor_d(t0, src1, src2); -tcg_gen_div_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); + +prep_div(t0, t1, src1, src2); +tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) @@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) static void gen_div_du(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -prep_divisor_du(t0, src2); -tcg_gen_divu_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); +prep_div(t0, t1, src1, src2); +tcg_gen_divu_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) @@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) static void gen_div_w(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -/* We need not check for integer overflow for div_w. */ -prep_divisor_du(t0, src2); -tcg_gen_div_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); + +prep_div(t0, t1, src1, src2); +tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2) -- 2.31.1
Re: [PATCH] RISC-V: Add support for Ztso
On Sat, 17 Sep 2022 01:02:46 PDT (-0700), Richard Henderson wrote: On 9/16/22 14:52, Palmer Dabbelt wrote: Though, honestly, I've had patches to add the required barriers sitting around for the last few releases, to better support things like x86 on aarch64. I should just finish that up. I can just do that for the RISC-V TSO support? Like the cover letter says that was my first thought, it's only when I found the comment saying not to do it that I went this way. My patches inject the barriers automatically by the tcg optimizer, rather than by hand, which is what the comment was trying to discourage. Last version was https://lore.kernel.org/qemu-devel/20210316220735.2048137-1-richard.hender...@linaro.org/ Thanks, I get it now.
Re: [RFC PATCH 3/4] hw/intc/gic: use MxTxAttrs to divine accessing CPU
On 9/16/22 17:28, Alex Bennée wrote: I pondered boolean but wasn't sure if that would blow up the size of MemTxAttrs so went for the bitfield. You can use bool with a bitfield. Though Phil's suggestion of .requestor_type has merit. That depends on what comes of Peter's hw modeling suggestion that might integrate the types on any given bus. r~
Re: [PATCH 2/5] target/loongarch: bstrins.w need set dest register EXT_SIGN
On 2022/9/17 15:59, Song Gao wrote: Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_bit.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc index 9337714ec4..33e94878fd 100644 --- a/target/loongarch/insn_trans/trans_bit.c.inc +++ b/target/loongarch/insn_trans/trans_bit.c.inc @@ -37,7 +37,7 @@ static bool gen_rr_ms_ls(DisasContext *ctx, arg_rr_ms_ls *a, DisasExtend src_ext, DisasExtend dst_ext, void (*func)(TCGv, TCGv, unsigned int, unsigned int)) { -TCGv dest = gpr_dst(ctx, a->rd, dst_ext); +TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE); I think this may not be correct. Maybe the code was used for debugging but forgot to modify? TCGv src1 = gpr_src(ctx, a->rj, src_ext); if (a->ls > a->ms) { @@ -206,7 +206,7 @@ TRANS(maskeqz, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_maskeqz) TRANS(masknez, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_masknez) TRANS(bytepick_w, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_w) TRANS(bytepick_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_d) -TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) +TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, gen_bstrins) TRANS(bstrins_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) TRANS(bstrpick_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, tcg_gen_extract_tl) TRANS(bstrpick_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, tcg_gen_extract_tl)
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
On 2022/9/17 15:59, Song Gao wrote: div.d, div.du, div,w, div.wu, the LoongArch host if x/0 the result is 0. The message has a typo: "div,w" => "div.w" Also I don't know why we need to do this, since the manual say: "When the divisor is 0, the result can be any value". So we set the divisor to 1 and the dividend to 0. Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_arith.c.inc | 34 +++ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc index 8e45eadbc8..c97afb16f9 100644 --- a/target/loongarch/insn_trans/trans_arith.c.inc +++ b/target/loongarch/insn_trans/trans_arith.c.inc @@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2) tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2); } +static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv src2) +{ +TCGv zero = tcg_constant_tl(0); +TCGv one = tcg_constant_tl(1); + +/* + * If x / 0, set the diviend to 0 set the divisor to 1 + * this is the same with LoongArch host. + */ +tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1); +tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2); +} + static void gen_div_d(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -prep_divisor_d(t0, src1, src2); -tcg_gen_div_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); + +prep_div(t0, t1, src1, src2); +tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) @@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) static void gen_div_du(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -prep_divisor_du(t0, src2); -tcg_gen_divu_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); +prep_div(t0, t1, src1, src2); +tcg_gen_divu_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) @@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) static void gen_div_w(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); -/* We need not check for integer overflow for div_w. */ -prep_divisor_du(t0, src2); -tcg_gen_div_tl(dest, src1, t0); +TCGv t1 = tcg_temp_new(); + +prep_div(t0, t1, src1, src2); +tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); +tcg_temp_free(t1); } static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2)
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
在 2022/9/17 下午4:59, Qi Hu 写道: On 2022/9/17 15:59, Song Gao wrote: div.d, div.du, div,w, div.wu, the LoongArch host if x/0 the result is 0. The message has a typo: "div,w" => "div.w" Also I don't know why we need to do this, since the manual say: "When the divisor is 0, the result can be any value". I tested on LoongArch host, the result is always 0. So we set the divisor to 1 and the dividend to 0. Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_arith.c.inc | 34 +++ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/target/loongarch/insn_trans/trans_arith.c.inc b/target/loongarch/insn_trans/trans_arith.c.inc index 8e45eadbc8..c97afb16f9 100644 --- a/target/loongarch/insn_trans/trans_arith.c.inc +++ b/target/loongarch/insn_trans/trans_arith.c.inc @@ -147,12 +147,28 @@ static void prep_divisor_du(TCGv ret, TCGv src2) tcg_gen_movcond_tl(TCG_COND_EQ, ret, src2, zero, one, src2); } +static void prep_div(TCGv divisor, TCGv dividend, TCGv src1, TCGv src2) +{ + TCGv zero = tcg_constant_tl(0); + TCGv one = tcg_constant_tl(1); + + /* + * If x / 0, set the diviend to 0 set the divisor to 1 + * this is the same with LoongArch host. + */ + tcg_gen_movcond_tl(TCG_COND_EQ, dividend, src2, zero, zero, src1); + tcg_gen_movcond_tl(TCG_COND_EQ, divisor, src2, zero, one, src2); +} + static void gen_div_d(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); - prep_divisor_d(t0, src1, src2); - tcg_gen_div_tl(dest, src1, t0); + TCGv t1 = tcg_temp_new(); + + prep_div(t0, t1, src1, src2); + tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); + tcg_temp_free(t1); } static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) @@ -166,9 +182,11 @@ static void gen_rem_d(TCGv dest, TCGv src1, TCGv src2) static void gen_div_du(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); - prep_divisor_du(t0, src2); - tcg_gen_divu_tl(dest, src1, t0); + TCGv t1 = tcg_temp_new(); + prep_div(t0, t1, src1, src2); + tcg_gen_divu_tl(dest, t1, t0); tcg_temp_free(t0); + tcg_temp_free(t1); } static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) @@ -182,10 +200,12 @@ static void gen_rem_du(TCGv dest, TCGv src1, TCGv src2) static void gen_div_w(TCGv dest, TCGv src1, TCGv src2) { TCGv t0 = tcg_temp_new(); - /* We need not check for integer overflow for div_w. */ - prep_divisor_du(t0, src2); - tcg_gen_div_tl(dest, src1, t0); + TCGv t1 = tcg_temp_new(); + + prep_div(t0, t1, src1, src2); + tcg_gen_div_tl(dest, t1, t0); tcg_temp_free(t0); + tcg_temp_free(t1); } static void gen_rem_w(TCGv dest, TCGv src1, TCGv src2)
Re: [PATCH 2/5] target/loongarch: bstrins.w need set dest register EXT_SIGN
在 2022/9/17 下午4:41, Qi Hu 写道: On 2022/9/17 15:59, Song Gao wrote: Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_bit.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc index 9337714ec4..33e94878fd 100644 --- a/target/loongarch/insn_trans/trans_bit.c.inc +++ b/target/loongarch/insn_trans/trans_bit.c.inc @@ -37,7 +37,7 @@ static bool gen_rr_ms_ls(DisasContext *ctx, arg_rr_ms_ls *a, DisasExtend src_ext, DisasExtend dst_ext, void (*func)(TCGv, TCGv, unsigned int, unsigned int)) { - TCGv dest = gpr_dst(ctx, a->rd, dst_ext); + TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE); I think this may not be correct. Maybe the code was used for debugging but forgot to modify? We just need EXT_SIGN the result. TCGv src1 = gpr_src(ctx, a->rj, src_ext); if (a->ls > a->ms) { @@ -206,7 +206,7 @@ TRANS(maskeqz, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_maskeqz) TRANS(masknez, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_masknez) TRANS(bytepick_w, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_w) TRANS(bytepick_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_d) -TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) +TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, gen_bstrins) TRANS(bstrins_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) TRANS(bstrpick_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, tcg_gen_extract_tl) TRANS(bstrpick_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, tcg_gen_extract_tl)
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
On 9/17/22 11:12, gaosong wrote: 在 2022/9/17 下午4:59, Qi Hu 写道: On 2022/9/17 15:59, Song Gao wrote: div.d, div.du, div,w, div.wu, the LoongArch host if x/0 the result is 0. The message has a typo: "div,w" => "div.w" Also I don't know why we need to do this, since the manual say: "When the divisor is 0, the result can be any value". I tested on LoongArch host, the result is always 0. But it is legal for a different loongarch host implementation to return some other value. Therefore the test itself is not correct. r~
Re: [PATCH 2/5] target/loongarch: bstrins.w need set dest register EXT_SIGN
On 2022/9/17 17:16, gaosong wrote: 在 2022/9/17 下午4:41, Qi Hu 写道: On 2022/9/17 15:59, Song Gao wrote: Signed-off-by: Song Gao --- target/loongarch/insn_trans/trans_bit.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc index 9337714ec4..33e94878fd 100644 --- a/target/loongarch/insn_trans/trans_bit.c.inc +++ b/target/loongarch/insn_trans/trans_bit.c.inc @@ -37,7 +37,7 @@ static bool gen_rr_ms_ls(DisasContext *ctx, arg_rr_ms_ls *a, DisasExtend src_ext, DisasExtend dst_ext, void (*func)(TCGv, TCGv, unsigned int, unsigned int)) { - TCGv dest = gpr_dst(ctx, a->rd, dst_ext); + TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE); I think this may not be correct. Maybe the code was used for debugging but forgot to modify? We just need EXT_SIGN the result. Got it. Thanks. Qi TCGv src1 = gpr_src(ctx, a->rj, src_ext); if (a->ls > a->ms) { @@ -206,7 +206,7 @@ TRANS(maskeqz, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_maskeqz) TRANS(masknez, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_masknez) TRANS(bytepick_w, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_w) TRANS(bytepick_d, gen_rrr_sa, EXT_NONE, EXT_NONE, gen_bytepick_d) -TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) +TRANS(bstrins_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, gen_bstrins) TRANS(bstrins_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, gen_bstrins) TRANS(bstrpick_w, gen_rr_ms_ls, EXT_NONE, EXT_SIGN, tcg_gen_extract_tl) TRANS(bstrpick_d, gen_rr_ms_ls, EXT_NONE, EXT_NONE, tcg_gen_extract_tl)
[PATCH 0/4] target/m68k: MacOS supervisor/user mode switch fixes
This series fixes a couple of bugs that were discovered when trying to boot MacOS on my github q800 branch with virtual memory enabled. Patch 1 renames M68K_FEATURE_M68000 to M68K_FEATURE_M68K in order to clarify that this feature indicates any Motorola 68K CPU rather than the 68000 specifically [1]. Patch 2 increases the size of the M68K features bitmap since there are already 32 features present, and we need to add one more. Patch 3 fixes up the MOVE-from-SR instruction which is privileged from the 68010 CPU onwards to use a newly introduced M68K_FEATURE_MOVEFROMSR_PRIV feature [2]. Patch 4 ensures that we always call gen_exit_tb() after writes to the SR register since any change of the S bit can change the security context. Signed-off-by: Mark Cave-Ayland Notes: [1] The m68k code currently contains a mix of real CPU features and pseudo features that represent each 680X0 CPU. In general QEMU maps features to CPUs which is why I've introduced the new M68K_FEATURE_MOVEFROMSR_PRIV feature, but there are still checks for specific 680X0 CPU models. This could do with a tidy-up, but without a specific set of test images across 68K and Coldfire I don't feel I'm confident enough to do this. [2] The existing code in MOVE-from-SR uses !m68k_feature(env, M68K_FEATURE_M68000) to suggest that the condition should match for any CPU that isn't a 68000 (i.e. 68010 and later) but as we see from this series, this is not the case according to the code. Some of the Mac 68K folk have suggested there are likely other cases in target/m68k where the same assumption has been used and the check logic is incorrect, but again without specific examples it's difficult for me to test. Mark Cave-Ayland (4): target/m68k: rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K target/m68k: increase size of m68k CPU features from uint32_t to uint64_t target/m68k: use M68K_FEATURE_MOVEFROMSR_PRIV feature for move_from_sr privilege check target/m68k: always call gen_exit_tb() after writes to SR target/m68k/cpu.c | 11 +++- target/m68k/cpu.h | 13 ++-- target/m68k/helper.c| 2 +- target/m68k/op_helper.c | 2 +- target/m68k/translate.c | 142 +--- 5 files changed, 91 insertions(+), 79 deletions(-) -- 2.30.2
[PATCH 1/4] target/m68k: rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K
The M68K_FEATURE_M68000 feature is misleading in that its name suggests the feature is defined just for Motorola 68000 CPUs, whilst in fact it is defined for all Motorola 680X0 CPUs. In order to avoid confusion with the other M68K_FEATURE_M680X0 constants which define the features available for specific Motorola CPU models, rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K and add comments to clarify its usage. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 2 +- target/m68k/cpu.h | 5 +- target/m68k/helper.c| 2 +- target/m68k/op_helper.c | 2 +- target/m68k/translate.c | 138 5 files changed, 75 insertions(+), 74 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 5bbefda575..f681be3a2a 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -110,7 +110,7 @@ static void m68000_cpu_initfn(Object *obj) M68kCPU *cpu = M68K_CPU(obj); CPUM68KState *env = &cpu->env; -m68k_set_feature(env, M68K_FEATURE_M68000); +m68k_set_feature(env, M68K_FEATURE_M68K); m68k_set_feature(env, M68K_FEATURE_USP); m68k_set_feature(env, M68K_FEATURE_WORD_INDEX); m68k_set_feature(env, M68K_FEATURE_MOVEP); diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 4d8f48e8c7..67b6c12c28 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -480,8 +480,9 @@ void do_m68k_semihosting(CPUM68KState *env, int nr); */ enum m68k_features { -/* Base m68k instruction set */ -M68K_FEATURE_M68000, +/* Base Motorola CPU set (not set for Coldfire CPUs) */ +M68K_FEATURE_M68K, +/* Motorola CPU feature sets */ M68K_FEATURE_M68010, M68K_FEATURE_M68020, M68K_FEATURE_M68030, diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 5728e48585..4621cf2402 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -460,7 +460,7 @@ void m68k_switch_sp(CPUM68KState *env) int new_sp; env->sp[env->current_sp] = env->aregs[7]; -if (m68k_feature(env, M68K_FEATURE_M68000)) { +if (m68k_feature(env, M68K_FEATURE_M68K)) { if (env->sr & SR_S) { /* SR:Master-Mode bit unimplemented then ISP is not available */ if (!m68k_feature(env, M68K_FEATURE_MSP) || env->sr & SR_M) { diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c index d9937ca8dc..99dc994fcb 100644 --- a/target/m68k/op_helper.c +++ b/target/m68k/op_helper.c @@ -433,7 +433,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw) static void do_interrupt_all(CPUM68KState *env, int is_hw) { -if (m68k_feature(env, M68K_FEATURE_M68000)) { +if (m68k_feature(env, M68K_FEATURE_M68K)) { m68k_interrupt_all(env, is_hw); return; } diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 5098f7e570..fad8af8f83 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -471,7 +471,7 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) if ((ext & 0x800) == 0 && !m68k_feature(s->env, M68K_FEATURE_WORD_INDEX)) return NULL_QREG; -if (m68k_feature(s->env, M68K_FEATURE_M68000) && +if (m68k_feature(s->env, M68K_FEATURE_M68K) && !m68k_feature(s->env, M68K_FEATURE_SCALED_INDEX)) { ext &= ~(3 << 9); } @@ -804,7 +804,7 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext *s, reg = get_areg(s, reg0); tmp = mark_to_release(s, tcg_temp_new()); if (reg0 == 7 && opsize == OS_BYTE && -m68k_feature(s->env, M68K_FEATURE_M68000)) { +m68k_feature(s->env, M68K_FEATURE_M68K)) { tcg_gen_subi_i32(tmp, reg, 2); } else { tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize)); @@ -888,7 +888,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, if (what == EA_STORE || !addrp) { TCGv tmp = tcg_temp_new(); if (reg0 == 7 && opsize == OS_BYTE && -m68k_feature(s->env, M68K_FEATURE_M68000)) { +m68k_feature(s->env, M68K_FEATURE_M68K)) { tcg_gen_addi_i32(tmp, reg, 2); } else { tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize)); @@ -2210,7 +2210,7 @@ DISAS_INSN(bitop_im) op = (insn >> 6) & 3; bitnum = read_im16(env, s); -if (m68k_feature(s->env, M68K_FEATURE_M68000)) { +if (m68k_feature(s->env, M68K_FEATURE_M68K)) { if (bitnum & 0xfe00) { disas_undef(env, s, insn); return; @@ -2875,7 +2875,7 @@ DISAS_INSN(mull) return; } SRC_EA(env, src1, OS_LONG, 0, NULL); -if (m68k_feature(s->env, M68K_FEATURE_M68000)) { +if (m68k_feature(s->env, M68K_FEATURE_M68K)) { tcg_gen_movi_i32(QREG_CC_C, 0); if (sign) { tcg_gen_muls2_i32(QREG_CC_N, QREG_CC_V, src1, DREG(ext, 12)); @@ -3470,7 +3470,7 @@ static inline void shift_im(DisasContext *s, uint16_t insn,
[PATCH 3/4] target/m68k: use M68K_FEATURE_MOVEFROMSR_PRIV feature for move_from_sr privilege check
Now that M68K_FEATURE_M68000 has been renamed to M68K_FEATURE_M68K it is easier to see that the privilege exception check is wrong: it is currently only generated for ColdFire CPUs when in fact it should also be generated for Motorola CPUs from the 68010 onwards. Introduce a new M68K_FEATURE_MOVEFROMSR_PRIV feature which is set for all non- Motorola CPUs, and for all Motorola CPUs from the 68010 onwards and use it to determine whether a privilege exception should be generated for the MOVE-from-SR instruction. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 5 + target/m68k/cpu.h | 2 ++ target/m68k/translate.c | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 7b4797e2f1..cc5311a4ac 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -102,6 +102,7 @@ static void m5206_cpu_initfn(Object *obj) CPUM68KState *env = &cpu->env; m68k_set_feature(env, M68K_FEATURE_CF_ISA_A); +m68k_set_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV); } /* Base feature set, including isns. for m68k family */ @@ -129,6 +130,7 @@ static void m68010_cpu_initfn(Object *obj) m68k_set_feature(env, M68K_FEATURE_RTD); m68k_set_feature(env, M68K_FEATURE_BKPT); m68k_set_feature(env, M68K_FEATURE_MOVEC); +m68k_set_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV); } /* @@ -241,6 +243,7 @@ static void m5208_cpu_initfn(Object *obj) m68k_set_feature(env, M68K_FEATURE_BRAL); m68k_set_feature(env, M68K_FEATURE_CF_EMAC); m68k_set_feature(env, M68K_FEATURE_USP); +m68k_set_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV); } static void cfv4e_cpu_initfn(Object *obj) @@ -254,6 +257,7 @@ static void cfv4e_cpu_initfn(Object *obj) m68k_set_feature(env, M68K_FEATURE_CF_FPU); m68k_set_feature(env, M68K_FEATURE_CF_EMAC); m68k_set_feature(env, M68K_FEATURE_USP); +m68k_set_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV); } static void any_cpu_initfn(Object *obj) @@ -275,6 +279,7 @@ static void any_cpu_initfn(Object *obj) m68k_set_feature(env, M68K_FEATURE_USP); m68k_set_feature(env, M68K_FEATURE_EXT_FULL); m68k_set_feature(env, M68K_FEATURE_WORD_INDEX); +m68k_set_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV); } static void m68k_cpu_realizefn(DeviceState *dev, Error **errp) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index d3384e5d98..57936ea780 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -537,6 +537,8 @@ enum m68k_features { M68K_FEATURE_UNALIGNED_DATA, /* TRAPcc insn. (680[2346]0, and CPU32) */ M68K_FEATURE_TRAPCC, +/* MOVE from SR privileged (from 68010) */ +M68K_FEATURE_MOVEFROMSR_PRIV, }; static inline uint64_t m68k_feature(CPUM68KState *env, int feature) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index fad8af8f83..be5561e1e9 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4598,7 +4598,7 @@ DISAS_INSN(move_from_sr) { TCGv sr; -if (IS_USER(s) && !m68k_feature(env, M68K_FEATURE_M68K)) { +if (IS_USER(s) && m68k_feature(env, M68K_FEATURE_MOVEFROMSR_PRIV)) { gen_exception(s, s->base.pc_next, EXCP_PRIVILEGE); return; } -- 2.30.2
[PATCH 2/4] target/m68k: increase size of m68k CPU features from uint32_t to uint64_t
There are already 32 feature bits in use, so change the size of the m68k CPU features to uint64_t (allong with the associated m68k_feature() functions) to allow up to 64 feature bits to be used. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 4 ++-- target/m68k/cpu.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index f681be3a2a..7b4797e2f1 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -38,12 +38,12 @@ static bool m68k_cpu_has_work(CPUState *cs) static void m68k_set_feature(CPUM68KState *env, int feature) { -env->features |= (1u << feature); +env->features |= (1ul << feature); } static void m68k_unset_feature(CPUM68KState *env, int feature) { -env->features &= (-1u - (1u << feature)); +env->features &= (-1ul - (1ul << feature)); } static void m68k_cpu_reset(DeviceState *dev) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 67b6c12c28..d3384e5d98 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -154,7 +154,7 @@ typedef struct CPUArchState { struct {} end_reset_fields; /* Fields from here on are preserved across CPU reset. */ -uint32_t features; +uint64_t features; } CPUM68KState; /* @@ -539,9 +539,9 @@ enum m68k_features { M68K_FEATURE_TRAPCC, }; -static inline int m68k_feature(CPUM68KState *env, int feature) +static inline uint64_t m68k_feature(CPUM68KState *env, int feature) { -return (env->features & (1u << feature)) != 0; +return (env->features & (1ul << feature)) != 0; } void m68k_cpu_list(void); -- 2.30.2
[PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Any write to SR can change the security state so always call gen_exit_tb() when this occurs. In particular MacOS makes use of andiw/oriw in a few places to handle the switch between user and supervisor mode. Signed-off-by: Mark Cave-Ayland --- target/m68k/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index be5561e1e9..892473d01f 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im) tcg_gen_or_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im) tcg_gen_and_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im) tcg_gen_xor_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr) } gen_push(s, gen_get_sr(s)); gen_set_sr_im(s, ext, 0); +gen_exit_tb(s); } DISAS_INSN(move_from_sr) -- 2.30.2
[PATCH] Remove unused MAX_IDE_BUS define
Several machines have an unused MAX_IDE_BUS define. Remove it from these machines that don't need it. Signed-off-by: BALATON Zoltan --- hw/alpha/dp264.c| 2 -- hw/hppa/machine.c | 2 -- hw/mips/fuloong2e.c | 1 - hw/mips/malta.c | 2 -- hw/ppc/prep.c | 2 -- hw/sparc64/sun4u.c | 1 - 6 files changed, 10 deletions(-) diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index f4349eba83..c502c8c62a 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -20,8 +20,6 @@ #include "qemu/datadir.h" #include "net/net.h" -#define MAX_IDE_BUS 2 - static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr) { if (((addr >> 41) & 3) == 2) { diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index e53d5f0fa7..355b3aac2e 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -30,8 +30,6 @@ #include "qemu/log.h" #include "net/net.h" -#define MAX_IDE_BUS 2 - #define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */ #define HPA_POWER_BUTTON (FIRMWARE_END - 0x10) diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c index 5ee546f5f6..8c747ac1a0 100644 --- a/hw/mips/fuloong2e.c +++ b/hw/mips/fuloong2e.c @@ -49,7 +49,6 @@ /* Fuloong 2e has a 512k flash: Winbond W39L040AP70Z */ #define BIOS_SIZE (512 * KiB) -#define MAX_IDE_BUS 2 /* * PMON is not part of qemu and released with BSD license, anyone diff --git a/hw/mips/malta.c b/hw/mips/malta.c index 0e932988e0..5099ed9592 100644 --- a/hw/mips/malta.c +++ b/hw/mips/malta.c @@ -69,8 +69,6 @@ #define FLASH_SIZE 0x40 -#define MAX_IDE_BUS 2 - typedef struct { MemoryRegion iomem; MemoryRegion iomem_lo; /* 0 - 0x900 */ diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index f08714f2ec..fcbe4c5837 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -50,8 +50,6 @@ /* SMP is not enabled, for now */ #define MAX_CPUS 1 -#define MAX_IDE_BUS 2 - #define CFG_ADDR 0xf510 #define KERNEL_LOAD_ADDR 0x0100 diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 0e27715ac4..387181ff77 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -66,7 +66,6 @@ #define PBM_PCI_IO_BASE (PBM_SPECIAL_BASE + 0x0200ULL) #define PROM_FILENAME"openbios-sparc64" #define NVRAM_SIZE 0x2000 -#define MAX_IDE_BUS 2 #define BIOS_CFG_IOPORT 0x510 #define FW_CFG_SPARC64_WIDTH (FW_CFG_ARCH_LOCAL + 0x00) #define FW_CFG_SPARC64_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01) -- 2.30.4
Re: [PATCH 2/4] target/m68k: increase size of m68k CPU features from uint32_t to uint64_t
On Sat, 17 Sep 2022, Mark Cave-Ayland wrote: There are already 32 feature bits in use, so change the size of the m68k CPU features to uint64_t (allong with the associated m68k_feature() functions) to allow up to 64 feature bits to be used. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 4 ++-- target/m68k/cpu.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index f681be3a2a..7b4797e2f1 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -38,12 +38,12 @@ static bool m68k_cpu_has_work(CPUState *cs) static void m68k_set_feature(CPUM68KState *env, int feature) { -env->features |= (1u << feature); +env->features |= (1ul << feature); } static void m68k_unset_feature(CPUM68KState *env, int feature) { -env->features &= (-1u - (1u << feature)); +env->features &= (-1ul - (1ul << feature)); Should these be ull instead of ul? Regards, BALATON Zoltan } static void m68k_cpu_reset(DeviceState *dev) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 67b6c12c28..d3384e5d98 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -154,7 +154,7 @@ typedef struct CPUArchState { struct {} end_reset_fields; /* Fields from here on are preserved across CPU reset. */ -uint32_t features; +uint64_t features; } CPUM68KState; /* @@ -539,9 +539,9 @@ enum m68k_features { M68K_FEATURE_TRAPCC, }; -static inline int m68k_feature(CPUM68KState *env, int feature) +static inline uint64_t m68k_feature(CPUM68KState *env, int feature) { -return (env->features & (1u << feature)) != 0; +return (env->features & (1ul << feature)) != 0; } void m68k_cpu_list(void);
[PATCH 0/2] audio: prevent a class of guest-triggered aborts
The issues with guest-triggered aborts started with commit ab32b78cd1 "audio: Simplify audio_bug() removing old code" which introduced an abort() in function audio_bug(). The abort() was there before, but it was only compiled in for debugging purposes. After this commit issue https://bugs.launchpad.net/bugs/1910603 showed up. This bug was mitigated with commits a2cd86a94a ("hw/audio/sb16: Avoid assertion by restricting I/O sampling rate range") and 60e543f5ce ("hw/audio/sb16: Restrict I/O sampling rate range for command 41h/42h"). The issue was only mitigated because I can still trigger the same abort. To reproduce start a FreeDOS QEMU guest with: ./qemu-system-i386 -machine pc,pcspk-audiodev=audio0 \ -device sb16,audiodev=audio0 \ -audiodev pa,id=audio0,timer-period=170,out.mixing-engine=on,out.buffer-length=181 \ -drive ... On the guest enter the out port sequence from launchpad bug #1910603: C:\> debug -o 22c 41 -o 22c 0 -o 22c 4 -o 22c 1c On the host: A bug was just triggered in audio_calloc Save all your work and restart without audio I am sorry Context: audio_pcm_sw_alloc_resources_out passed invalid arguments to audio_calloc nmemb=0 size=16 (len=0) Aborted (core dumped) Here is another example to trigger the same abort. Start a Linux guest with an AC97 audio device: ./qemu-system-x86_64 -machine q35,pcspk-audiodev=audio0 \ -device AC97,bus=pcie.0,addr=0x1b,audiodev=audio0 \ -audiodev pa,id=audio0 \ - ... Open a shell on the guest: ~>sudo lspci -s '00:1b.0' -nn -vv 00:1b.0 Multimedia audio controller [0401]: Intel Corporation 82801AA AC'97 Audio Controller [8086:2415] (rev 01) Subsystem: Red Hat, Inc. QEMU Virtual Machine [1af4:1100] Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0 Interrupt: pin A routed to IRQ 16 Region 0: I/O ports at c000 [size=1K] Region 1: I/O ports at c400 [size=256] Kernel driver in use: snd_intel8x0 Kernel modules: snd_intel8x0 ~># IOBAR0 + 0x2c ~>sudo outw 0xc02c 1 On the host: A bug was just triggered in audio_calloc Save all your work and restart without audio I am sorry Context: audio_pcm_sw_alloc_resources_out passed invalid arguments to audio_calloc nmemb=0 size=16 (len=0) Aborted (core dumped) Remove the abort() in audio_bug() to avoid this class of guest-triggered aborts. Volker Rümelin (2): Revert "audio: Log context for audio bug" audio: remove abort() in audio_bug() audio/audio.c | 24 audio/audio_template.h | 27 +++ 2 files changed, 27 insertions(+), 24 deletions(-) -- 2.35.3
[PATCH 1/2] Revert "audio: Log context for audio bug"
This reverts commit 8e30d39bade3010387177ca23dbc2244352ed4a3. Revert commit 8e30d39bad "audio: Log context for audio bug" to make error propagation work again. Signed-off-by: Volker Rümelin --- audio/audio.c | 25 + audio/audio_template.h | 27 +++ 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 76b8735b44..545ded6674 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -117,6 +117,7 @@ int audio_bug (const char *funcname, int cond) AUD_log (NULL, "I am sorry\n"); } AUD_log (NULL, "Context:\n"); +abort(); } return cond; @@ -137,7 +138,7 @@ static inline int audio_bits_to_index (int bits) default: audio_bug ("bits_to_index", 1); AUD_log (NULL, "invalid bits %d\n", bits); -abort(); +return 0; } } @@ -155,7 +156,7 @@ void *audio_calloc (const char *funcname, int nmemb, size_t size) AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n", funcname); AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len); -abort(); +return NULL; } return g_malloc0 (len); @@ -542,7 +543,7 @@ static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw) size_t live = hw->total_samples_captured - audio_pcm_hw_find_min_in (hw); if (audio_bug(__func__, live > hw->conv_buf->size)) { dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); -abort(); +return 0; } return live; } @@ -580,7 +581,7 @@ static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) } if (audio_bug(__func__, live > hw->conv_buf->size)) { dolog("live_in=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); -abort(); +return 0; } rpos = audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); @@ -655,7 +656,7 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live) if (audio_bug(__func__, live > hw->mix_buf->size)) { dolog("live=%zu hw->mix_buf->size=%zu\n", live, hw->mix_buf->size); -abort(); +return 0; } return live; } @@ -705,7 +706,7 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size) live = sw->total_hw_samples_mixed; if (audio_bug(__func__, live > hwsamples)) { dolog("live=%zu hw->mix_buf->size=%zu\n", live, hwsamples); -abort(); +return 0; } if (live == hwsamples) { @@ -997,7 +998,7 @@ static size_t audio_get_avail (SWVoiceIn *sw) if (audio_bug(__func__, live > sw->hw->conv_buf->size)) { dolog("live=%zu sw->hw->conv_buf->size=%zu\n", live, sw->hw->conv_buf->size); -abort(); +return 0; } ldebug ( @@ -1027,7 +1028,7 @@ static size_t audio_get_free(SWVoiceOut *sw) if (audio_bug(__func__, live > sw->hw->mix_buf->size)) { dolog("live=%zu sw->hw->mix_buf->size=%zu\n", live, sw->hw->mix_buf->size); -abort(); +return 0; } dead = sw->hw->mix_buf->size - live; @@ -1169,7 +1170,7 @@ static void audio_run_out (AudioState *s) if (audio_bug(__func__, live > hw->mix_buf->size)) { dolog("live=%zu hw->mix_buf->size=%zu\n", live, hw->mix_buf->size); -abort(); +continue; } if (hw->pending_disable && !nb_live) { @@ -1202,7 +1203,7 @@ static void audio_run_out (AudioState *s) if (audio_bug(__func__, hw->mix_buf->pos >= hw->mix_buf->size)) { dolog("hw->mix_buf->pos=%zu hw->mix_buf->size=%zu played=%zu\n", hw->mix_buf->pos, hw->mix_buf->size, played); -abort(); +hw->mix_buf->pos = 0; } #ifdef DEBUG_OUT @@ -1222,7 +1223,7 @@ static void audio_run_out (AudioState *s) if (audio_bug(__func__, played > sw->total_hw_samples_mixed)) { dolog("played=%zu sw->total_hw_samples_mixed=%zu\n", played, sw->total_hw_samples_mixed); -abort(); +played = sw->total_hw_samples_mixed; } sw->total_hw_samples_mixed -= played; @@ -1345,7 +1346,7 @@ static void audio_run_capture (AudioState *s) if (audio_bug(__func__, captured > sw->total_hw_samples_mixed)) { dolog("captured=%zu sw->total_hw_samples_mixed=%zu\n", captured, sw->total_hw_samples_mixed); -abort(); +captured = sw->total_hw_samples_mixed; } sw->total_hw_samples_mixed -= captured; diff --git a/audio/audio_template.h b/audio/audio_template.h index 7192b19e73..d2d348638b 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -59,13 +59,12 @@ static void glue(audio_init_nb_voices_, TYPE)(AudioStat
[PATCH 2/2] audio: remove abort() in audio_bug()
Commit ab32b78cd1 "audio: Simplify audio_bug() removing old code" introduced abort() in audio_bug() for regular builds. audio_bug() was never meant to abort QEMU for the following reasons. - There's code in audio_bug() that expects audio_bug() gets called more than once with error condition true. The variable 'shown' is only 0 on first error. - All call sites test the return code of audio_bug(), print an error context message and handle the errror. - The abort() in audio_bug() enables a class of guest-triggered aborts similar to the Launchpad Bug #1910603 at https://bugs.launchpad.net/bugs/1910603. Fixes: ab32b78cd1 "audio: Simplify audio_bug() removing old code" Buglink: https://bugs.launchpad.net/bugs/1910603 Signed-off-by: Volker Rümelin --- audio/audio.c | 1 - 1 file changed, 1 deletion(-) diff --git a/audio/audio.c b/audio/audio.c index 545ded6674..01c0cd8202 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -117,7 +117,6 @@ int audio_bug (const char *funcname, int cond) AUD_log (NULL, "I am sorry\n"); } AUD_log (NULL, "Context:\n"); -abort(); } return cond; -- 2.35.3
[PATCH] hw/display: load the correct ROM file for isa-vga device
Apparently we didn't load the correct ROM file when using the isa-vga device, which resulted in a display waiting to be initialized by a guest OS kernel. With this fix, SeaBIOS is able to print vital data to a text mode console during boot, which is useful in case of failing to continue booting. Signed-off-by: Liav Albani --- hw/display/vga-isa.c | 2 +- hw/display/vga_int.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c index 46abbc5653..bcf646d012 100644 --- a/hw/display/vga-isa.c +++ b/hw/display/vga-isa.c @@ -84,7 +84,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp) VBE_DISPI_LFB_PHYSICAL_ADDRESS, &s->vram); /* ROM BIOS */ -rom_add_vga(VGABIOS_FILENAME); +rom_add_vga(VGABIOS_ISAVGA_FILENAME); } static Property vga_isa_properties[] = { diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 305e700014..b5e803ac51 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -183,7 +183,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val); extern const uint8_t sr_mask[8]; extern const uint8_t gr_mask[16]; -#define VGABIOS_FILENAME "vgabios.bin" +#define VGABIOS_ISAVGA_FILENAME "vgabios-isa.bin" #define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin" extern const MemoryRegionOps vga_mem_ops; -- 2.37.3
Re: [PATCH] hw/display: load the correct ROM file for isa-vga device
On 9/17/22 17:06, Liav Albani wrote: Apparently we didn't load the correct ROM file when using the isa-vga device, which resulted in a display waiting to be initialized by a guest OS kernel. With this fix, SeaBIOS is able to print vital data to a text mode console during boot, which is useful in case of failing to continue booting. Signed-off-by: Liav Albani --- hw/display/vga-isa.c | 2 +- hw/display/vga_int.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c index 46abbc5653..bcf646d012 100644 --- a/hw/display/vga-isa.c +++ b/hw/display/vga-isa.c @@ -84,7 +84,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp) VBE_DISPI_LFB_PHYSICAL_ADDRESS, &s->vram); /* ROM BIOS */ -rom_add_vga(VGABIOS_FILENAME); +rom_add_vga(VGABIOS_ISAVGA_FILENAME); } static Property vga_isa_properties[] = { diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 305e700014..b5e803ac51 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -183,7 +183,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val); extern const uint8_t sr_mask[8]; extern const uint8_t gr_mask[16]; -#define VGABIOS_FILENAME "vgabios.bin" +#define VGABIOS_ISAVGA_FILENAME "vgabios-isa.bin" This should be an "vgabios-isavga.bin" actually, at least this is how it is named on my development machine. I'll send a v2 shortly. #define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin" extern const MemoryRegionOps vga_mem_ops;
Re: [PULL 00/12] linux-user patches
The close_range(2) man page says: close_range() first appeared in Linux 5.9. Library support was added in glibc in version 2.34. The qemu-user GitLab CI jobs are failing. For example, see https://gitlab.com/qemu-project/qemu/-/jobs/3043629417: ../linux-user/syscall.c:8734:26: error: implicit declaration of function 'close_range' is invalid in C99 [-Werror,-Wimplicit-function-declaration] return get_errno(close_range(arg1, arg2, arg3)); ^ There is a second issue with this pull request: ../linux-user/syscall.c:357:16: error: ‘pidfd_getfd’ defined but not used [-Werror=unused-function] 357 | _syscall3(int, pidfd_getfd, int, pidfd, int, targetfd, unsigned int, flags); | ^~~ ../linux-user/syscall.c:251:13: note: in definition of macro ‘_syscall3’ See https://gitlab.com/qemu-project/qemu/-/jobs/3043629434. Stefan
[PATCH v2] hw/display: load the correct ROM file for isa-vga device
Apparently QEMU didn't load the correct ROM file when using the isa-vga device on my development machine, which resulted in a display waiting to be initialized by a guest OS kernel. With this fix, SeaBIOS is able to print vital data to a text mode console during boot, which is useful in case of failing to continue booting. The build name of the vgabios.bin is changed too, to vgabios-isavga.bin to ensure we always have that file when QEMU is installed as a package or compiled from source. Signed-off-by: Liav Albani --- hw/display/vga-isa.c | 2 +- hw/display/vga_int.h | 2 +- pc-bios/meson.build | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c index 46abbc5653..bcf646d012 100644 --- a/hw/display/vga-isa.c +++ b/hw/display/vga-isa.c @@ -84,7 +84,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp) VBE_DISPI_LFB_PHYSICAL_ADDRESS, &s->vram); /* ROM BIOS */ -rom_add_vga(VGABIOS_FILENAME); +rom_add_vga(VGABIOS_ISAVGA_FILENAME); } static Property vga_isa_properties[] = { diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 305e700014..b63788e809 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -183,7 +183,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val); extern const uint8_t sr_mask[8]; extern const uint8_t gr_mask[16]; -#define VGABIOS_FILENAME "vgabios.bin" +#define VGABIOS_ISAVGA_FILENAME "vgabios-isavga.bin" #define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin" extern const MemoryRegionOps vga_mem_ops; diff --git a/pc-bios/meson.build b/pc-bios/meson.build index 388e0db6e4..6af94a4a0a 100644 --- a/pc-bios/meson.build +++ b/pc-bios/meson.build @@ -29,7 +29,7 @@ blobs = [ 'bios-microvm.bin', 'qboot.rom', 'sgabios.bin', - 'vgabios.bin', + 'vgabios-isavga.bin', 'vgabios-cirrus.bin', 'vgabios-stdvga.bin', 'vgabios-vmware.bin', -- 2.37.3
Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device
On 9/17/22 17:32, Liav Albani wrote: diff --git a/pc-bios/meson.build b/pc-bios/meson.build index 388e0db6e4..6af94a4a0a 100644 --- a/pc-bios/meson.build +++ b/pc-bios/meson.build @@ -29,7 +29,7 @@ blobs = [ 'bios-microvm.bin', 'qboot.rom', 'sgabios.bin', - 'vgabios.bin', + 'vgabios-isavga.bin', 'vgabios-cirrus.bin', Well, it seems like this one doesn't want to be compiled now, so I'll need to dive deeper to figure out how to ensure it always produces the requested file. 'vgabios-stdvga.bin', 'vgabios-vmware.bin',
Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device
On 9/17/22 17:40, Liav Albani wrote: On 9/17/22 17:32, Liav Albani wrote: diff --git a/pc-bios/meson.build b/pc-bios/meson.build index 388e0db6e4..6af94a4a0a 100644 --- a/pc-bios/meson.build +++ b/pc-bios/meson.build @@ -29,7 +29,7 @@ blobs = [ 'bios-microvm.bin', 'qboot.rom', 'sgabios.bin', - 'vgabios.bin', + 'vgabios-isavga.bin', 'vgabios-cirrus.bin', Well, it seems like this one doesn't want to be compiled now, so I'll need to dive deeper to figure out how to ensure it always produces the requested file. So apparently the problem on my development machine is that for some odd reason I don't have the "vgabios.bin" file, but I do have the "vgabios-isavga.bin" file. According to my package manager (pacman), it is owned by the SeaBIOS package: usr/share/qemu/vgabios-isavga.bin is owned by extra/seabios 1.15.0-1 Other files in that directory are owned by the same package as well, for example: usr/share/qemu/vgabios-stdvga.bin is owned by extra/seabios 1.15.0-1 usr/share/qemu/vgabios-virtio.bin is owned by extra/seabios 1.15.0-1 usr/share/qemu/vgabios-vmware.bin is owned by extra/seabios 1.15.0-1 So I'm not sure what is the best approach here to fix this. It is definitely not a problem in QEMU because when I compile it from source I do get a "vgabios.bin" file in the build directory, but I do think that QEMU should try to use the "vgabios-isavga.bin" file when using the isa-vga device, so in some way we could fix it in QEMU, which also makes sense to ensure the filename is "vgabios-isavga.bin" for the isa-vga device and not plain "vgabios.bin", which really doesn't say much about the type of the graphics device. 'vgabios-stdvga.bin', 'vgabios-vmware.bin',
Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device
+David On 17/9/22 17:06, Liav Albani wrote: On 9/17/22 17:40, Liav Albani wrote: On 9/17/22 17:32, Liav Albani wrote: diff --git a/pc-bios/meson.build b/pc-bios/meson.build index 388e0db6e4..6af94a4a0a 100644 --- a/pc-bios/meson.build +++ b/pc-bios/meson.build @@ -29,7 +29,7 @@ blobs = [ 'bios-microvm.bin', 'qboot.rom', 'sgabios.bin', - 'vgabios.bin', + 'vgabios-isavga.bin', 'vgabios-cirrus.bin', Well, it seems like this one doesn't want to be compiled now, so I'll need to dive deeper to figure out how to ensure it always produces the requested file. So apparently the problem on my development machine is that for some odd reason I don't have the "vgabios.bin" file, but I do have the "vgabios-isavga.bin" file. According to my package manager (pacman), it is owned by the SeaBIOS package: usr/share/qemu/vgabios-isavga.bin is owned by extra/seabios 1.15.0-1 Other files in that directory are owned by the same package as well, for example: usr/share/qemu/vgabios-stdvga.bin is owned by extra/seabios 1.15.0-1 usr/share/qemu/vgabios-virtio.bin is owned by extra/seabios 1.15.0-1 usr/share/qemu/vgabios-vmware.bin is owned by extra/seabios 1.15.0-1 So I'm not sure what is the best approach here to fix this. It is definitely not a problem in QEMU because when I compile it from source I do get a "vgabios.bin" file in the build directory, but I do think that QEMU should try to use the "vgabios-isavga.bin" file when using the isa-vga device, so in some way we could fix it in QEMU, which also makes sense to ensure the filename is "vgabios-isavga.bin" for the isa-vga device and not plain "vgabios.bin", which really doesn't say much about the type of the graphics device. This seems due to a recent Arch Linux change in SeaBIOS package: https://github.com/archlinux/svntogit-packages/commit/ef2a5da4f1aa3fa45eea88c83eee01e89974145f (so not related to mainstream QEMU project). Regards, Phil.
Re: [PULL 00/12] linux-user patches
On 17/9/22 16:26, Stefan Hajnoczi wrote: The close_range(2) man page says: close_range() first appeared in Linux 5.9. Library support was added in glibc in version 2.34. The qemu-user GitLab CI jobs are failing. For example, see https://gitlab.com/qemu-project/qemu/-/jobs/3043629417: ../linux-user/syscall.c:8734:26: error: implicit declaration of function 'close_range' is invalid in C99 [-Werror,-Wimplicit-function-declaration] return get_errno(close_range(arg1, arg2, arg3)); ^ There is a second issue with this pull request: ../linux-user/syscall.c:357:16: error: ‘pidfd_getfd’ defined but not used [-Werror=unused-function] 357 | _syscall3(int, pidfd_getfd, int, pidfd, int, targetfd, unsigned int, flags); | ^~~ ../linux-user/syscall.c:251:13: note: in definition of macro ‘_syscall3’ See https://gitlab.com/qemu-project/qemu/-/jobs/3043629434. Hmm apparently this PR hasn't been reviewed (although the patches were on the list for 2 weeks). The 'check DCO' job - looking for S-o-b tags - is green: https://gitlab.com/qemu-project/qemu/-/jobs/3043629425. Should we complete it by a R-b/A-b check over the commit range?
Re: [PULL 00/12] linux-user patches
On Sat, 17 Sept 2022 at 15:31, Philippe Mathieu-Daudé wrote: > > On 17/9/22 16:26, Stefan Hajnoczi wrote: > > The close_range(2) man page says: > > close_range() first appeared in Linux 5.9. Library support was added > > in glibc in version 2.34. > > > > The qemu-user GitLab CI jobs are failing. For example, see > > https://gitlab.com/qemu-project/qemu/-/jobs/3043629417: > > > > ../linux-user/syscall.c:8734:26: error: implicit declaration of > > function 'close_range' is invalid in C99 > > [-Werror,-Wimplicit-function-declaration] > > return get_errno(close_range(arg1, arg2, arg3)); > > ^ > > > > There is a second issue with this pull request: > > ../linux-user/syscall.c:357:16: error: ‘pidfd_getfd’ defined but not > > used [-Werror=unused-function] > > 357 | _syscall3(int, pidfd_getfd, int, pidfd, int, targetfd, unsigned > > int, flags); > > | ^~~ > > ../linux-user/syscall.c:251:13: note: in definition of macro ‘_syscall3’ > > > > See https://gitlab.com/qemu-project/qemu/-/jobs/3043629434. > > Hmm apparently this PR hasn't been reviewed (although the patches were > on the list for 2 weeks). > > The 'check DCO' job - looking for S-o-b tags - is green: > https://gitlab.com/qemu-project/qemu/-/jobs/3043629425. > Should we complete it by a R-b/A-b check over the commit range? In some areas there will be no R-b/A-b, so I don't think we can require those checks. Was this pull request supposed to go through Laurent instead of being applied directly by me? Stefan
Re: [PULL 00/11] semihosting patch queue
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v9 1/7] include: add zoned device structs
On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote: > Eric Blake 于2022年9月15日周四 16:05写道: > > > > On Sat, Sep 10, 2022 at 01:27:53PM +0800, Sam Li wrote: > > > Signed-off-by: Sam Li > > > Reviewed-by: Stefan Hajnoczi > > > Reviewed-by: Damien Le Moal > > > --- > > > include/block/block-common.h | 43 > > > 1 file changed, 43 insertions(+) > > > > > > diff --git a/include/block/block-common.h b/include/block/block-common.h > > > index fdb7306e78..36bd0e480e 100644 > > > --- a/include/block/block-common.h > > > +++ b/include/block/block-common.h > > > @@ -49,6 +49,49 @@ typedef struct BlockDriver BlockDriver; > > > typedef struct BdrvChild BdrvChild; > > > typedef struct BdrvChildClass BdrvChildClass; > > > > > > +typedef enum BlockZoneOp { > > > +BLK_ZO_OPEN, > > > +BLK_ZO_CLOSE, > > > +BLK_ZO_FINISH, > > > +BLK_ZO_RESET, > > > +} BlockZoneOp; > > > + > > > +typedef enum BlockZoneModel { > > > +BLK_Z_NONE = 0x0, /* Regular block device */ > > > +BLK_Z_HM = 0x1, /* Host-managed zoned block device */ > > > +BLK_Z_HA = 0x2, /* Host-aware zoned block device */ > > > +} BlockZoneModel; > > > + > > > +typedef enum BlockZoneCondition { > > > +BLK_ZS_NOT_WP = 0x0, > > > +BLK_ZS_EMPTY = 0x1, > > > +BLK_ZS_IOPEN = 0x2, > > > +BLK_ZS_EOPEN = 0x3, > > > +BLK_ZS_CLOSED = 0x4, > > > +BLK_ZS_RDONLY = 0xD, > > > +BLK_ZS_FULL = 0xE, > > > +BLK_ZS_OFFLINE = 0xF, > > > +} BlockZoneCondition; > > > + > > > +typedef enum BlockZoneType { > > > +BLK_ZT_CONV = 0x1, /* Conventional random writes supported */ > > > +BLK_ZT_SWR = 0x2, /* Sequential writes required */ > > > +BLK_ZT_SWP = 0x3, /* Sequential writes preferred */ > > > +} BlockZoneType; > > > + > > > +/* > > > + * Zone descriptor data structure. > > > + * Provides information on a zone with all position and size values in > > > bytes. > > > > I'm glad that you chose bytes here for use in qemu. But since the > > kernel struct blk_zone uses sectors instead of bytes, is it worth > > adding a sentence that we intentionally use bytes here, different from > > Linux, to make it easier for reviewers to realize that scaling when > > translating between qemu and kernel is necessary? > > Sorry about the unit mistake. The zone information is in sectors which > is the same as kernel struct blk_zone. I think adding a sentence to > inform the sector unit makes it clear what the zone descriptor is. I'd make the units bytes for consistency with the rest of the QEMU block layer. For example, the MapEntry structure that "qemu-img map" reports has names with similar fields and they are in bytes: struct MapEntry { int64_t start; int64_t length; Stefan signature.asc Description: PGP signature
Re: [PATCH v9 5/7] config: add check to block layer
On Sat, Sep 10, 2022 at 01:27:57PM +0800, Sam Li wrote: > diff --git a/block/file-posix.c b/block/file-posix.c > index 4edfa25d04..354de22860 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -779,6 +779,20 @@ static int raw_open_common(BlockDriverState *bs, QDict > *options, > goto fail; > } > } > +#ifdef CONFIG_BLKZONED > +/* > + * The kernel page chache does not reliably work for writes to SWR zones s/chache/cache/ signature.asc Description: PGP signature
Re: [PULL 00/20] target-arm.next patch queue
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v9 3/7] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls
On Sat, Sep 10, 2022 at 01:27:55PM +0800, Sam Li wrote: > Add a new zoned_host_device BlockDriver. The zoned_host_device option > accepts only zoned host block devices. By adding zone management > operations in this new BlockDriver, users can use the new block > layer APIs including Report Zone and four zone management operations > (open, close, finish, reset). > > Qemu-io uses the new APIs to perform zoned storage commands of the device: > zone_report(zrp), zone_open(zo), zone_close(zc), zone_reset(zrs), > zone_finish(zf). > > For example, to test zone_report, use following command: > $ ./build/qemu-io --image-opts -n driver=zoned_host_device, > filename=/dev/nullb0 > -c "zrp offset nr_zones" > > Signed-off-by: Sam Li > Reviewed-by: Hannes Reinecke > --- > block/block-backend.c | 145 ++ > block/file-posix.c| 323 +- > block/io.c| 41 > include/block/block-io.h | 7 + > include/block/block_int-common.h | 21 ++ > include/block/raw-aio.h | 6 +- > include/sysemu/block-backend-io.h | 17 ++ > meson.build | 1 + > qapi/block-core.json | 8 +- > qemu-io-cmds.c| 143 + > 10 files changed, 708 insertions(+), 4 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index d4a5df2ac2..ebe8d7bdf3 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -1431,6 +1431,15 @@ typedef struct BlkRwCo { > void *iobuf; > int ret; > BdrvRequestFlags flags; > +union { > +struct { > +unsigned int *nr_zones; > +BlockZoneDescriptor *zones; > +} zone_report; > +struct { > +BlockZoneOp op; > +} zone_mgmt; > +}; > } BlkRwCo; > > int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags) > @@ -1775,6 +1784,142 @@ int coroutine_fn blk_co_flush(BlockBackend *blk) > return ret; > } > > +static void blk_aio_zone_report_entry(void *opaque) { > +BlkAioEmAIOCB *acb = opaque; > +BlkRwCo *rwco = &acb->rwco; > + > +rwco->ret = blk_co_zone_report(rwco->blk, rwco->offset, > + rwco->zone_report.nr_zones, > + rwco->zone_report.zones); > +blk_aio_complete(acb); > +} > + > +BlockAIOCB *blk_aio_zone_report(BlockBackend *blk, int64_t offset, > +unsigned int *nr_zones, > +BlockZoneDescriptor *zones, > +BlockCompletionFunc *cb, void *opaque) > +{ > +BlkAioEmAIOCB *acb; > +Coroutine *co; > +IO_CODE(); > + > +blk_inc_in_flight(blk); > +acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque); > +acb->rwco = (BlkRwCo) { > +.blk= blk, > +.offset = offset, > +.ret= NOT_DONE, > +.zone_report = { > +.zones = zones, > +.nr_zones = nr_zones, Indentation is off here. QEMU uses 4-space indentation. > +}, > +}; > +acb->has_returned = false; > + > +co = qemu_coroutine_create(blk_aio_zone_report_entry, acb); > +bdrv_coroutine_enter(blk_bs(blk), co); > + > +acb->has_returned = true; > +if (acb->rwco.ret != NOT_DONE) { > +replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), > + blk_aio_complete_bh, acb); > +} > + > +return &acb->common; > +} > + > +static void blk_aio_zone_mgmt_entry(void *opaque) { > +BlkAioEmAIOCB *acb = opaque; > +BlkRwCo *rwco = &acb->rwco; > + > +rwco->ret = blk_co_zone_mgmt(rwco->blk, rwco->zone_mgmt.op, > + rwco->offset, acb->bytes); > +blk_aio_complete(acb); > +} > + > +BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk, BlockZoneOp op, > + int64_t offset, int64_t len, > + BlockCompletionFunc *cb, void *opaque) { > +BlkAioEmAIOCB *acb; > +Coroutine *co; > +IO_CODE(); > + > +blk_inc_in_flight(blk); > +acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque); > +acb->rwco = (BlkRwCo) { > +.blk= blk, > +.offset = offset, > +.ret= NOT_DONE, > +.zone_mgmt = { > +.op = op, Indentation is off here. QEMU uses 4-space indentation. > +}, > +}; > +acb->bytes = len; > +acb->has_returned = false; > + > +co = qemu_coroutine_create(blk_aio_zone_mgmt_entry, acb); > +bdrv_coroutine_enter(blk_bs(blk), co); > + > +acb->has_returned = true; > +if (acb->rwco.ret != NOT_DONE) { > +replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), > + blk_aio_complete_bh, acb); > +} > + > +return &acb->common; > +} > + > +/* > + * Send a zon
Re: [PULL 0/3] hmp queue
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v2 07/11] acpi/tests/bits: add python test that exercizes QEMU bios tables using biosbits
On Fri, Sep 16, 2022 at 09:30:42PM +0530, Ani Sinha wrote: > On Thu, Jul 28, 2022 at 12:08 AM Ani Sinha wrote: > > > > > > > > On Mon, 25 Jul 2022, Ani Sinha wrote: > > > > > > > > > > > On Sat, 16 Jul 2022, Michael S. Tsirkin wrote: > > > > > > > On Sat, Jul 16, 2022 at 12:06:00PM +0530, Ani Sinha wrote: > > > > > > > > > > > > > > > On Fri, Jul 15, 2022 at 11:20 Michael S. Tsirkin > > > > > wrote: > > > > > > > > > > On Fri, Jul 15, 2022 at 09:47:27AM +0530, Ani Sinha wrote: > > > > > > > Instead of all this mess, can't we just spawn e.g. "git clone > > > > > --depth > > > > > 1"? > > > > > > > And if the directory exists I would fetch and checkout. > > > > > > > > > > > > There are two reasons I can think of why I do not like this > > > > > idea: > > > > > > > > > > > > (a) a git clone of a whole directory would download all > > > > > versions of the > > > > > > binary whereas we want only a specific version. > > > > > > > > > > You mention shallow clone yourself, and I used --depth 1 above. > > > > > > > > > > > Downloading a single file > > > > > > by shallow cloning or creating a git archive is overkill IMHO > > > > > when a wget > > > > > > style retrieval works just fine. > > > > > > > > > > However, it does not provide for versioning, tagging etc so you > > > > > have > > > > > to implement your own schema. > > > > > > > > > > > > > > > Hmm I’m not sure if we need all that. Bits has its own versioning > > > > > mechanism and > > > > > I think all we need to do is maintain the same versioning logic and > > > > > maintain > > > > > binaries of different versions. Do we really need the power of > > > > > git/version > > > > > control here? Dunno. > > > > > > > > Well we need some schema. Given we are not using official bits releases > > > > I don't think we can reuse theirs. > > > > > > OK fine. Lets figuire out how to push bits somewhere in git.qemu.org and > > > the binaries in some other repo first. Everything else hinges on that. We > > > can fix the rest of the bits later incrementally. > > > > DanPB, any thoughts on putting bits on git.qemu.org or where and how to > > keep the binaries? > > Can we please conclude on this? > Peter, can you please fork the repo? I have tried many times to reach > you on IRC but failed. Probably because of travel around KVM forum. I think given our CI is under pressure again due to gitlab free tier limits, tying binaries to CI isn't a great idea at this stage. Can Ani just upload binaies to qemu.org for now? -- MST
Re: [PATCH] Remove unused MAX_IDE_BUS define
On 9/17/22 13:51, BALATON Zoltan wrote: Several machines have an unused MAX_IDE_BUS define. Remove it from these machines that don't need it. Signed-off-by: BALATON Zoltan --- hw/alpha/dp264.c| 2 -- hw/hppa/machine.c | 2 -- hw/mips/fuloong2e.c | 1 - hw/mips/malta.c | 2 -- hw/ppc/prep.c | 2 -- hw/sparc64/sun4u.c | 1 - 6 files changed, 10 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] Remove unused MAX_IDE_BUS define
On 17/9/22 13:51, BALATON Zoltan wrote: Several machines have an unused MAX_IDE_BUS define. Remove it from these machines that don't need it. Signed-off-by: BALATON Zoltan --- hw/alpha/dp264.c| 2 -- hw/hppa/machine.c | 2 -- hw/mips/fuloong2e.c | 1 - hw/mips/malta.c | 2 -- hw/ppc/prep.c | 2 -- hw/sparc64/sun4u.c | 1 - 6 files changed, 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé And queued to mips-next, thanks!
Re: [PATCH 1/3] target/riscv: Set the CPU resetvec directly
On 14/9/22 12:11, Alistair Francis via wrote: Instead of using our properties to set a config value which then might be used to set the resetvec (depending on your timing), let's instead just set the resetvec directly in the env struct. This allows us to set the reset vec from the command line with: -global driver=riscv.hart_array,property=resetvec,value=0x2400 Signed-off-by: Alistair Francis --- target/riscv/cpu.h | 3 +-- target/riscv/cpu.c | 13 +++-- target/riscv/machine.c | 6 +++--- 3 files changed, 7 insertions(+), 15 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/3] hw/riscv: opentitan: Expose the resetvec as a SoC property
On 14/9/22 12:11, Alistair Francis via wrote: On the OpenTitan hardware the resetvec is fixed at the start of ROM. In QEMU we don't run the ROM code and instead just jump to the next stage. This means we need to be a little more flexible about what the resetvec is. This patch allows us to set the resetvec from the command line with something like this: -global driver=riscv.lowrisc.ibex.soc,property=resetvec,value=0x2400 This way as the next stage changes we can update the resetvec. Signed-off-by: Alistair Francis --- include/hw/riscv/opentitan.h | 2 ++ hw/riscv/opentitan.c | 8 +++- 2 files changed, 9 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 01/10] mac_newworld: Drop some variables
On 17/9/22 01:07, BALATON Zoltan wrote: Values not used frequently enough may not worth putting in a local variable, especially with names almost as long as the original value because that does not improve readability, to the contrary it makes it harder to see what value is used. Drop a few such variables. This is the same clean up that was done for mac_oldworld in commit b8df32555ce5. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 65 +++ 1 file changed, 29 insertions(+), 36 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 02/10] mac_oldworld: Drop some more variables
On 17/9/22 01:07, BALATON Zoltan wrote: Drop some more local variables additionally to commit b8df32555ce5 to match clean ups done to mac_newwold in previous patch. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_oldworld.c | 43 +-- 1 file changed, 21 insertions(+), 22 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 05/10] mac_{old|new}world: Reduce number of QOM casts
On 17/9/22 01:07, BALATON Zoltan wrote: By storing the device pointers in a variable with the right type the number of QOM casts can be reduced which also makes the code more readable. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 60 --- hw/ppc/mac_oldworld.c | 26 --- 2 files changed, 39 insertions(+), 47 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 09/10] hw/ppc/mac.h: Move PROM and KERNEL defines to board code
On 17/9/22 01:07, BALATON Zoltan wrote: The PROM_FILENAME and KERNEL_* defines are used by mac_oldworld and mac_newworld but they don't have to be identical so these could be moved to the individual boards. The NVRAM_SIZE define is not used so it can be dropped. This further reduces the mac.h header. Signed-off-by: BALATON Zoltan --- hw/ppc/mac.h | 6 -- hw/ppc/mac_newworld.c | 4 hw/ppc/mac_oldworld.c | 7 ++- 3 files changed, 10 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 07/10] hw/ppc/mac.h: Move macio specific atuff out from shared header
On 17/9/22 01:07, BALATON Zoltan wrote: Move the parts specific to and only used by macio out from the shared mac.h into macio.c where they better belong. Signed-off-by: BALATON Zoltan --- hw/misc/macio/macio.c | 26 -- hw/ppc/mac.h | 23 --- 2 files changed, 24 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 07/10] hw/ppc/mac.h: Move macio specific atuff out from shared header
On 17/9/22 01:07, BALATON Zoltan wrote: Move the parts specific to and only used by macio out from the shared mac.h into macio.c where they better belong. Signed-off-by: BALATON Zoltan --- hw/misc/macio/macio.c | 26 -- hw/ppc/mac.h | 23 --- 2 files changed, 24 insertions(+), 25 deletions(-) BTW Subject: "Move macio specifics out of shared header"?
Re: [PATCH 1/7] scripts/nsis.py: Drop the unnecessary path separator
On 8/9/22 15:28, Bin Meng wrote: From: Bin Meng There is no need to append a path separator to the destination directory that is passed to "make install". Signed-off-by: Bin Meng --- scripts/nsis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/7] scripts/nsis.py: Fix destination directory name when invoked on Windows
On 8/9/22 15:28, Bin Meng wrote: From: Bin Meng "make installer" on Windows fails with the following message: Traceback (most recent call last): File "G:\msys64\home\foo\git\qemu\scripts\nsis.py", line 89, in main() File "G:\msys64\home\foo\git\qemu\scripts\nsis.py", line 34, in main with open( OSError: [Errno 22] Invalid argument: 'R:/Temp/tmpw83xhjquG:/msys64/qemu/system-emulations.nsh' ninja: build stopped: subcommand failed. Use os.path.splitdrive() to form a canonical path without the drive letter on Windows. This works with cross-build on Linux too. Fixes: 8adfeba953e0 ("meson: add NSIS building") Signed-off-by: Bin Meng --- scripts/nsis.py | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 4/7] .gitlab-ci.d/windows.yml: Drop the sed processing in the 64-bit build
On 8/9/22 15:28, Bin Meng wrote: From: Bin Meng The sed processing of build/config-host.mak seems to be no longer needed, and there is no such in the 32-bit build too. Drop it. Signed-off-by: Bin Meng --- .gitlab-ci.d/windows.yml | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 7/7] .gitlab-ci.d/windows.yml: Test 'make installer' in the CI
On 8/9/22 15:28, Bin Meng wrote: From: Bin Meng Now that we have supported packaging DLLs automatically, let's add the 'make installer' in the CI and publish the generated installer file as an artifact. Increase the job timeout to 90 minutes to accommodate to it. Signed-off-by: Bin Meng --- .gitlab-ci.d/windows.yml | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml index fffb202658..3a94d40e73 100644 --- a/.gitlab-ci.d/windows.yml +++ b/.gitlab-ci.d/windows.yml @@ -10,7 +10,7 @@ - ${CI_PROJECT_DIR}/msys64/var/cache needs: [] stage: build - timeout: 70m + timeout: 90m before_script: - If ( !(Test-Path -Path msys64\var\cache ) ) { mkdir msys64\var\cache @@ -28,6 +28,11 @@ - .\msys64\usr\bin\bash -lc 'pacman --noconfirm -Syuu' # Core update - .\msys64\usr\bin\bash -lc 'pacman --noconfirm -Syuu' # Normal update - taskkill /F /FI "MODULES eq msys-2.0.dll" + artifacts: +name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG" +expire_in: 7 days +paths: + - build/qemu-setup*.exe Do you really want to test this binary? I think the CI is only to test the installer. This is a stripped down version anyway (./configure options). If someone want to package/test, this should not be done here but locally. However I agree testing the installer doesn't bitrot is helpful, so *without* the "artifacts" section: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 5/7] block/nfs: Fix 32-bit Windows build
On 8/9/22 15:28, Bin Meng wrote: From: Bin Meng libnfs.h declares nfs_fstat() as the following for win32: int nfs_fstat(struct nfs_context *nfs, struct nfsfh *nfsfh, struct __stat64 *st); The 'st' parameter should be of type 'struct __stat64'. The codes happen to build successfully for 64-bit Windows, but it does not build for 32-bit Windows. Fixes: 6542aa9c75bc ("block: add native support for NFS") Fixes: 18a8056e0bc7 ("block/nfs: cache allocated filesize for read-only files") Signed-off-by: Bin Meng --- block/nfs.c | 8 1 file changed, 8 insertions(+) diff --git a/block/nfs.c b/block/nfs.c index 444c40b458..d5d67937dd 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -418,7 +418,11 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts, int flags, int open_flags, Error **errp) { int64_t ret = -EINVAL; +#ifdef _WIN32 +struct __stat64 st; +#else struct stat st; +#endif char *file = NULL, *strp = NULL; qemu_mutex_init(&client->mutex); @@ -781,7 +785,11 @@ static int nfs_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { NFSClient *client = state->bs->opaque; +#ifdef _WIN32 +struct __stat64 st; +#else struct stat st; +#endif int ret = 0; if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) { What about the field in struct NFSRPC?
Re: [PATCH] vfio/common: Fix vfio_iommu_type1_info use after free
On 15/9/22 19:18, Alex Williamson wrote: On error, vfio_get_iommu_info() frees and clears *info, but vfio_connect_container() continues to use the pointer regardless of the return value. Restructure the code such that a failure of this function triggers an error and clean up the remainder of the function, including updating an outdated comment that had drifted from its relevant line of code and using host page size for a default for better compatibility on non-4KB systems. Reported-by: Nicolin Chen Link: https://lore.kernel.org/all/20220910004245.2878-1-nicol...@nvidia.com/ Signed-off-by: Alex Williamson --- hw/vfio/common.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) Turns out I had the errno vs ret correct the first time from the thread in the above Link, vfio_get_iommu_info() returns -errno. diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ace9562a9ba1..6b5d8c0bf694 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, { struct vfio_iommu_type1_info *info; -/* - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit - * IOVA whatsoever. That's not actually true, but the current - * kernel interface doesn't tell us what it can map, and the - * existing Type1 IOMMUs generally support any IOVA we're - * going to actually try in practice. - */ ret = vfio_get_iommu_info(container, &info); +if (ret) { +error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info"); +goto enable_discards_exit; +} -if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) { -/* Assume 4k IOVA page size */ -info->iova_pgsizes = 4096; +if (info->flags & VFIO_IOMMU_INFO_PGSIZES) { +container->pgsizes = info->iova_pgsizes; +} else { +container->pgsizes = qemu_real_host_page_size(); } -vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes); -container->pgsizes = info->iova_pgsizes; -/* The default in the kernel ("dma_entry_limit") is 65535. */ -container->dma_max_mappings = 65535; -if (!ret) { -vfio_get_info_dma_avail(info, &container->dma_max_mappings); -vfio_get_iommu_info_migration(container, info); +if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) { +container->dma_max_mappings = 65535; } +vfio_get_iommu_info_migration(container, info); g_free(info); + +/* + * FIXME: We should parse VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE + * information to get the actual window extent rather than assume + * a 64-bit IOVA address space. + */ +vfio_host_win_add(container, 0, (hwaddr)-1, container->pgsizes); + break; } case VFIO_SPAPR_TCE_v2_IOMMU: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 2/4] scripts/ci/setup: Fix libxen requirements
On 14/9/22 14:41, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" XEN hypervisor is only available in ARM and x86, but the yaml only checked if the architecture is different from s390x, changed it to a more accurate test. Tested this change on a Ubuntu 20.04 ppc64le. Signed-off-by: Lucas Mateus Castro (alqotel) Reviewed-by: Alex Bennée --- scripts/ci/setup/build-environment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml index 6df3e61d94..7535228685 100644 --- a/scripts/ci/setup/build-environment.yml +++ b/scripts/ci/setup/build-environment.yml @@ -97,7 +97,7 @@ state: present when: - ansible_facts['distribution'] == 'Ubuntu' -- ansible_facts['architecture'] != 's390x' +- ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64' - name: Install basic packages to build QEMU on Ubuntu 20.04 package: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 3/4] scripts/ci/setup: spice-server only on x86 aarch64
On 14/9/22 14:41, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" Changed build-environment.yml to only install spice-server on x86_64 and aarch64 as this package is only available on those architectures. Signed-off-by: Lucas Mateus Castro (alqotel) --- scripts/ci/setup/build-environment.yml | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 1/1] monitor/hmp: print trace as option in help for log command
Hi Markus, On 2/9/22 14:24, Markus Armbruster wrote: Dongli Zhang writes: The below is printed when printing help information in qemu-system-x86_64 command line, and when CONFIG_TRACE_LOG is enabled: $ qemu-system-x86_64 -d help ... ... trace:PATTERN enable trace events Use "-d trace:help" to get a list of trace events. However, the options of "trace:PATTERN" are only printed by "qemu-system-x86_64 -d help", but missing in hmp "help log" command. Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"") Cc: Joe Jin Signed-off-by: Dongli Zhang --- Changed since v1: - change format for "none" as well. Changed since v2: - use "log trace:help" in help message. - add more clarification in commit message. - add 'Fixes' tag. --- monitor/hmp.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) Not this patch's fault: 1. "-d help" terminates with exit status 1, "-d trace:help" with 0. The former is wrong. 2. HMP "log trace:help" prints to stdout instead of the current monitor. 3. Output of HMP "log trace:help" sometimes is truncated for me. 4. Output of "log trace:help" and "info trace-events" is unwieldy. Sorted output could be a bit less unwieldy. 5. Could "log trace:help" and "info trace-events" share code? Do you mind opening issue(s) on our GitLab so we don't loose your analysis buried within the infinite mailing list?
Re: [PATCH v4 06/21] ppc4xx_sdram: Move size check to ppc4xx_sdram_init()
On 14/9/22 13:34, BALATON Zoltan wrote: Instead of checking if memory size is valid in board code move this check to ppc4xx_sdram_init() as this is a restriction imposed by the SDRAM controller. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc405.h | 2 -- hw/ppc/ppc405_boards.c | 10 -- hw/ppc/ppc405_uc.c | 11 ++- hw/ppc/ppc440_bamboo.c | 10 +- hw/ppc/ppc4xx_devs.c| 14 ++ include/hw/ppc/ppc4xx.h | 2 +- 6 files changed, 10 insertions(+), 39 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 08/21] ppc4xx_sdram: Drop extra zeros for readability
On 14/9/22 13:34, BALATON Zoltan wrote: Constants that are written zero padded for no good reason are hard to read, it's easier to see what is meant if it's just 0 or 1 instead. It would be nice to be able to use the single quote separator for integer literals, but they are only part of C++14, so C2x which doesn't seem stabilized yet :/ Ref: https://en.cppreference.com/w/cpp/language/integer_literal Meanwhile: Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: BALATON Zoltan --- hw/ppc/ppc4xx_devs.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c index 375834a52b..bfe7b2d3a6 100644 --- a/hw/ppc/ppc4xx_devs.c +++ b/hw/ppc/ppc4xx_devs.c @@ -49,31 +49,31 @@ static uint32_t sdram_ddr_bcr(hwaddr ram_base, hwaddr ram_size) switch (ram_size) { case 4 * MiB: -bcr = 0x; +bcr = 0; break; case 8 * MiB: -bcr = 0x0002; +bcr = 0x2; break; case 16 * MiB: -bcr = 0x0004; +bcr = 0x4; break; case 32 * MiB: -bcr = 0x0006; +bcr = 0x6; break; case 64 * MiB: -bcr = 0x0008; +bcr = 0x8; break; case 128 * MiB: -bcr = 0x000A; +bcr = 0xA; break; case 256 * MiB: -bcr = 0x000C; +bcr = 0xC; break; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid RAM size 0x%" HWADDR_PRIx "\n", __func__, ram_size); -return 0x; +return 0; } bcr |= ram_base & 0xFF80; bcr |= 1; @@ -104,7 +104,7 @@ static target_ulong sdram_size(uint32_t bcr) static void sdram_set_bcr(Ppc4xxSdramDdrState *sdram, int i, uint32_t bcr, int enabled) { -if (sdram->bank[i].bcr & 0x0001) { +if (sdram->bank[i].bcr & 1) { /* Unmap RAM */ trace_ppc4xx_sdram_unmap(sdram_base(sdram->bank[i].bcr), sdram_size(sdram->bank[i].bcr)); @@ -115,7 +115,7 @@ static void sdram_set_bcr(Ppc4xxSdramDdrState *sdram, int i, object_unparent(OBJECT(&sdram->bank[i].container)); } sdram->bank[i].bcr = bcr & 0xFFDEE001; -if (enabled && (bcr & 0x0001)) { +if (enabled && (bcr & 1)) { trace_ppc4xx_sdram_map(sdram_base(bcr), sdram_size(bcr)); memory_region_init(&sdram->bank[i].container, NULL, "sdram-container", sdram_size(bcr)); @@ -136,7 +136,7 @@ static void sdram_map_bcr(Ppc4xxSdramDdrState *sdram) sdram_set_bcr(sdram, i, sdram_ddr_bcr(sdram->bank[i].base, sdram->bank[i].size), 1); } else { -sdram_set_bcr(sdram, i, 0x, 0); +sdram_set_bcr(sdram, i, 0, 0); } } } @@ -213,7 +213,7 @@ static uint32_t sdram_ddr_dcr_read(void *opaque, int dcrn) break; default: /* Avoid gcc warning */ -ret = 0x; +ret = 0; break; } @@ -306,18 +306,18 @@ static void ppc4xx_sdram_ddr_reset(DeviceState *dev) { Ppc4xxSdramDdrState *sdram = PPC4xx_SDRAM_DDR(dev); -sdram->addr = 0x; -sdram->bear = 0x; -sdram->besr0 = 0x; /* No error */ -sdram->besr1 = 0x; /* No error */ -sdram->cfg = 0x; -sdram->ecccfg = 0x; /* No ECC */ -sdram->eccesr = 0x; /* No error */ +sdram->addr = 0; +sdram->bear = 0; +sdram->besr0 = 0; /* No error */ +sdram->besr1 = 0; /* No error */ +sdram->cfg = 0; +sdram->ecccfg = 0; /* No ECC */ +sdram->eccesr = 0; /* No error */ sdram->pmit = 0x07C0; sdram->rtr = 0x05F0; sdram->tr = 0x00854009; /* We pre-initialize RAM banks */ -sdram->status = 0x; +sdram->status = 0; sdram->cfg = 0x0080; }
Re: [PATCH v4 09/21] ppc440_sdram: Split off map/unmap of sdram banks for later reuse
On 14/9/22 13:34, BALATON Zoltan wrote: Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c index 5db59d1190..01184e717b 100644 --- a/hw/ppc/ppc440_uc.c +++ b/hw/ppc/ppc440_uc.c @@ -561,26 +561,33 @@ static uint64_t sdram_size(uint32_t bcr) return size; } +static void sdram_bank_map(Ppc4xxSdramBank *bank) +{ +memory_region_init(&bank->container, NULL, "sdram-container", bank->size); +memory_region_add_subregion(&bank->container, 0, &bank->ram); +memory_region_add_subregion(get_system_memory(), bank->base, +&bank->container); +} + +static void sdram_bank_unmap(Ppc4xxSdramBank *bank) +{ +memory_region_del_subregion(get_system_memory(), &bank->container); +memory_region_del_subregion(&bank->container, &bank->ram); +object_unparent(OBJECT(&bank->container)); +} + static void sdram_set_bcr(ppc440_sdram_t *sdram, int i, uint32_t bcr, int enabled) { if (sdram->bank[i].bcr & 1) { /* First unmap RAM if enabled */ -memory_region_del_subregion(get_system_memory(), -&sdram->bank[i].container); -memory_region_del_subregion(&sdram->bank[i].container, -&sdram->bank[i].ram); -object_unparent(OBJECT(&sdram->bank[i].container)); +sdram_bank_unmap(&sdram->bank[i]); } sdram->bank[i].bcr = bcr & 0xffe0ffc1; +sdram->bank[i].base = sdram_base(bcr); +sdram->bank[i].size = sdram_size(bcr); This part doesn't seem to belong the the same patch. if (enabled && (bcr & 1)) { -memory_region_init(&sdram->bank[i].container, NULL, "sdram-container", - sdram_size(bcr)); -memory_region_add_subregion(&sdram->bank[i].container, 0, -&sdram->bank[i].ram); -memory_region_add_subregion(get_system_memory(), -sdram_base(bcr), -&sdram->bank[i].container); +sdram_bank_map(&sdram->bank[i]); } }
Re: [PATCH v4 13/21] ppc4xx_sdram: Rename functions to prevent name clashes
On 14/9/22 13:34, BALATON Zoltan wrote: Rename functions to avoid name clashes when moving the DDR2 controller model currently called ppc440_sdram to ppc4xx_devs. This also more clearly shows which function belongs to which model. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 69 ++-- hw/ppc/ppc4xx_devs.c | 44 ++-- 2 files changed, 57 insertions(+), 56 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 21/21] ppc4xx_sdram: Add errp parameter to ppc4xx_sdram_banks()
On 14/9/22 13:34, BALATON Zoltan wrote: Do not exit from ppc4xx_sdram_banks() but report error via an errp parameter instead. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc4xx_sdram.c | 28 +++- 1 file changed, 19 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 14/21] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On 14/9/22 13:34, BALATON Zoltan wrote: Move the check for valid memory sizes from board to sdram controller init. Board now only checks for additional restrictions imposed by firmware then sdram init checks for valid sizes for SoC. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440.h| 4 ++-- hw/ppc/ppc440_uc.c | 15 +++ hw/ppc/sam460ex.c | 32 +--- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h index 01d76b8000..29f6f14ed7 100644 --- a/hw/ppc/ppc440.h +++ b/hw/ppc/ppc440.h @@ -11,13 +11,13 @@ #ifndef PPC440_H #define PPC440_H -#include "hw/ppc/ppc4xx.h" +#include "hw/ppc/ppc.h" void ppc4xx_l2sram_init(CPUPPCState *env); void ppc4xx_cpr_init(CPUPPCState *env); void ppc4xx_sdr_init(CPUPPCState *env); void ppc440_sdram_init(CPUPPCState *env, int nbanks, - Ppc4xxSdramBank *ram_banks); + MemoryRegion *ram); void ppc4xx_ahb_init(CPUPPCState *env); void ppc4xx_dma_init(CPUPPCState *env, int dcr_base); void ppc460ex_pcie_init(CPUPPCState *env); diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c index fa313f979d..bd2a489557 100644 --- a/hw/ppc/ppc440_uc.c +++ b/hw/ppc/ppc440_uc.c @@ -486,7 +486,7 @@ void ppc4xx_sdr_init(CPUPPCState *env) typedef struct ppc440_sdram_t { uint32_t addr; uint32_t mcopt2; -int nbanks; +int nbanks; /* Banks to use from the 4, e.g. when board has less slots */ Ppc4xxSdramBank bank[4]; } ppc440_sdram_t; @@ -732,18 +732,17 @@ static void sdram_ddr2_reset(void *opaque) } void ppc440_sdram_init(CPUPPCState *env, int nbanks, - Ppc4xxSdramBank *ram_banks) + MemoryRegion *ram) { ppc440_sdram_t *s; -int i; +const ram_addr_t valid_bank_sizes[] = { +4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB, +32 * MiB, 16 * MiB, 8 * MiB, 0 Where 16/8 come from? +}; s = g_malloc0(sizeof(*s)); s->nbanks = nbanks; -for (i = 0; i < nbanks; i++) { -s->bank[i].ram = ram_banks[i].ram; -s->bank[i].base = ram_banks[i].base; -s->bank[i].size = ram_banks[i].size; -} +ppc4xx_sdram_banks(ram, s->nbanks, s->bank, valid_bank_sizes); qemu_register_reset(&sdram_ddr2_reset, s); ppc_dcr_register(env, SDRAM0_CFGADDR, s, &sdram_ddr2_dcr_read, &sdram_ddr2_dcr_write); diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index dac329d482..9b850808a3 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -74,13 +74,6 @@ #define EBC_FREQ 11500 #define UART_FREQ 11059200 -/* The SoC could also handle 4 GiB but firmware does not work with that. */ -/* Maybe it overflows a signed 32 bit number somewhere? */ -static const ram_addr_t ppc460ex_sdram_bank_sizes[] = { -2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB, -32 * MiB, 0 -}; - struct boot_info { uint32_t dt_base; uint32_t dt_size; @@ -273,7 +266,6 @@ static void sam460ex_init(MachineState *machine) { MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *isa = g_new(MemoryRegion, 1); -Ppc4xxSdramBank *ram_banks = g_new0(Ppc4xxSdramBank, 1); MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1); DeviceState *uic[4]; int i; @@ -340,12 +332,22 @@ static void sam460ex_init(MachineState *machine) } /* SDRAM controller */ -/* put all RAM on first bank because board has one slot - * and firmware only checks that */ -ppc4xx_sdram_banks(machine->ram, 1, ram_banks, ppc460ex_sdram_bank_sizes); - +/* The SoC could also handle 4 GiB but firmware does not work with that. */ +if (machine->ram_size > 2 * GiB) { +error_report("Memory over 2 GiB is not supported"); +exit(1); +} +/* Firmware needs at least 64 MiB */ +if (machine->ram_size < 64 * MiB) { +error_report("Memory below 64 MiB is not supported"); +exit(1); +} +/* + * Put all RAM on first bank because board has one slot + * and firmware only checks that + */ +ppc440_sdram_init(env, 1, machine->ram); /* FIXME: does 460EX have ECC interrupts? */ -ppc440_sdram_init(env, 1, ram_banks); /* Enable SDRAM memory regions as we may boot without firmware */ if (ppc_dcr_write(env->dcr_env, SDRAM0_CFGADDR, 0x21) || ppc_dcr_write(env->dcr_env, SDRAM0_CFGDATA, 0x0800)) { @@ -358,8 +360,8 @@ static void sam460ex_init(MachineState *machine) qdev_get_gpio_in(uic[0], 2)); i2c = PPC4xx_I2C(dev)->bus; /* SPD EEPROM on RAM module */ -spd_data = spd_data_generate(ram_banks->size < 128 * MiB ? DDR : DDR2, - ram_banks->size); +spd_data = spd_data_generate(machine->ram_size < 128 * MiB ? DDR : DDR2, + machine->ram_size);
Re: [PATCH 1/4] target/m68k: rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K
On 17/9/22 13:25, Mark Cave-Ayland wrote: The M68K_FEATURE_M68000 feature is misleading in that its name suggests the feature is defined just for Motorola 68000 CPUs, whilst in fact it is defined for all Motorola 680X0 CPUs. In order to avoid confusion with the other M68K_FEATURE_M680X0 constants which define the features available for specific Motorola CPU models, rename M68K_FEATURE_M68000 to M68K_FEATURE_M68K and add comments to clarify its usage. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 2 +- target/m68k/cpu.h | 5 +- target/m68k/helper.c| 2 +- target/m68k/op_helper.c | 2 +- target/m68k/translate.c | 138 5 files changed, 75 insertions(+), 74 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/4] target/m68k: increase size of m68k CPU features from uint32_t to uint64_t
On 17/9/22 14:09, BALATON Zoltan wrote: On Sat, 17 Sep 2022, Mark Cave-Ayland wrote: There are already 32 feature bits in use, so change the size of the m68k CPU features to uint64_t (allong with the associated m68k_feature() functions) to allow up to 64 feature bits to be used. Signed-off-by: Mark Cave-Ayland --- target/m68k/cpu.c | 4 ++-- target/m68k/cpu.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index f681be3a2a..7b4797e2f1 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -38,12 +38,12 @@ static bool m68k_cpu_has_work(CPUState *cs) static void m68k_set_feature(CPUM68KState *env, int feature) { - env->features |= (1u << feature); + env->features |= (1ul << feature); env->features = deposit64(env->features, feature, 1, 1); } static void m68k_unset_feature(CPUM68KState *env, int feature) { - env->features &= (-1u - (1u << feature)); + env->features &= (-1ul - (1ul << feature)); env->features = deposit64(env->features, feature, 1, 0); Should these be ull instead of ul? Yes. Not needed if using the extract/deposit API. } static void m68k_cpu_reset(DeviceState *dev) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 67b6c12c28..d3384e5d98 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -154,7 +154,7 @@ typedef struct CPUArchState { struct {} end_reset_fields; /* Fields from here on are preserved across CPU reset. */ - uint32_t features; + uint64_t features; } CPUM68KState; /* @@ -539,9 +539,9 @@ enum m68k_features { M68K_FEATURE_TRAPCC, }; -static inline int m68k_feature(CPUM68KState *env, int feature) +static inline uint64_t m68k_feature(CPUM68KState *env, int feature) Why uint64_t? Can we simplify using a boolean? { - return (env->features & (1u << feature)) != 0; + return (env->features & (1ul << feature)) != 0; return extract64(env->features, feature, 1) == 1; } void m68k_cpu_list(void);
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
On 17/9/22 13:25, Mark Cave-Ayland wrote: Any write to SR can change the security state so always call gen_exit_tb() when this occurs. In particular MacOS makes use of andiw/oriw in a few places to handle the switch between user and supervisor mode. Shouldn't be safer to add the gen_exit_tb() call in gen_set_sr[_im]()? Signed-off-by: Mark Cave-Ayland --- target/m68k/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index be5561e1e9..892473d01f 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im) tcg_gen_or_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im) tcg_gen_and_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im) tcg_gen_xor_i32(dest, src1, im); if (with_SR) { gen_set_sr(s, dest, opsize == OS_BYTE); +gen_exit_tb(s); } else { DEST_EA(env, insn, opsize, dest, &addr); gen_logic_cc(s, dest, opsize); @@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr) } gen_push(s, gen_get_sr(s)); gen_set_sr_im(s, ext, 0); +gen_exit_tb(s); } DISAS_INSN(move_from_sr)