Re: [PATCH] aio_wait_kick: add missing memory barrier

2022-06-04 Thread Paolo Bonzini 

On 6/4/22 14:51, Roman Kagan wrote:

On Tue, May 24, 2022 at 01:30:54PM -0400, Emanuele Giuseppe Esposito wrote:

It seems that aio_wait_kick always required a memory barrier
or atomic operation in the caller, but nobody actually
took care of doing it.

Let's put the barrier in the function instead, and pair it
with another one in AIO_WAIT_WHILE. Read aio_wait_kick()
comment for further explanation.

Suggested-by: Paolo Bonzini 
Signed-off-by: Emanuele Giuseppe Esposito 
---
  include/block/aio-wait.h |  2 ++
  util/aio-wait.c  | 16 +++-
  2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index b39eefb38d..54840f8622 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -81,6 +81,8 @@ extern AioWait global_aio_wait;
  AioContext *ctx_ = (ctx);  \
  /* Increment wait_->num_waiters before evaluating cond. */ \
  qatomic_inc(&wait_->num_waiters);  \
+/* Paired with smp_mb in aio_wait_kick(). */   \
+smp_mb();  \


IIRC qatomic_inc() ensures sequential consistency, isn't it enough here?


Nope, it only ensures sequential consistency with other SEQ_CST 
operations, i.e. not with qatomic_read or qatomic_set. :(


The smp_mb() is needed on ARM, in particular.


Paolo

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

2022-06-04 Thread Stafford Horne 
On Thu, Jun 02, 2022 at 11:42:30AM +, Joel Stanley wrote:
> Hi Stafford,
> 
> On Fri, 27 May 2022 at 17:27, Stafford Horne  wrote:
> >
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.
> 
> It's a good idea! I did some playing around with your patch today.
> 
> I'd suggest adding something to docs/system/target-openrsic.rst,
> including an example command lines.

Yeah, good idea, this is the command I am using:

qemu-system-or1k -cpu or1200 -M virt \
  -kernel /home/shorne/work/linux/vmlinux \
  -initrd /home/shorne/work/linux/initramfs.cpio.gz \
  -device virtio-net-device,netdev=user -netdev 
user,id=user,net=10.9.0.1/24,host=10.9.0.100 \
  -serial mon:stdio -nographic \
  -device virtio-blk-device,drive=d0 -drive 
file=/home/shorne/work/linux/virt.qcow2,id=d0,if=none,format=qcow2 \
  -gdb tcp::10001 -smp cpus=2 -m 64

I should have mentioned it but the config I am using is here:

  https://github.com/stffrdhrn/linux/commits/or1k-virt

> >
> > The platform provides:
> >  - OpenRISC SMP with up to 8 cpus
> 
> You have this:
> 
> #define VIRT_CPUS_MAX 4
> i
> I tried booting with -smp 4 and it locked up when starting userspace
> (or I stopped getting serial output?):
> 
> [0.06] smp: Brought up 1 node, 4 CPUs
> ...
> [0.96] Run /init as init process
> 
> Running with -smp 2 and 3 worked. It does make booting much much slower.

Right, it should be 4, I just write 8 from memory.  You are also, right I have
issues with running 4 CPU's.  I will try richard's suggestion.  I have some old
patches to configure MTTCG also, but it had some limitations.  I will dig those
up and get this fixed for this series.

> >  - Generated RTC to automatically configure the guest kernel
> 
> Did you mean device tree?

Yeah, thats what I meant.

> >
> > Signed-off-by: Stafford Horne 
> > ---
> >  configs/devices/or1k-softmmu/default.mak |   1 +
> >  hw/openrisc/Kconfig  |   9 +
> >  hw/openrisc/meson.build  |   1 +
> >  hw/openrisc/virt.c   | 429 +++
> >  4 files changed, 440 insertions(+)
> >  create mode 100644 hw/openrisc/virt.c
> >
> > diff --git a/configs/devices/or1k-softmmu/default.mak 
> > b/configs/devices/or1k-softmmu/default.mak
> > index 5b3ac89491..f3bf816067 100644
> > --- a/configs/devices/or1k-softmmu/default.mak
> > +++ b/configs/devices/or1k-softmmu/default.mak
> > @@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
> >  # Boards:
> >  #
> >  CONFIG_OR1K_SIM=y
> > +CONFIG_OR1K_VIRT=y
> > diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
> > index 8f284f3ba0..202134668e 100644
> > --- a/hw/openrisc/Kconfig
> > +++ b/hw/openrisc/Kconfig
> > @@ -4,3 +4,12 @@ config OR1K_SIM
> >  select OPENCORES_ETH
> >  select OMPIC
> >  select SPLIT_IRQ
> > +
> > +config OR1K_VIRT
> > +bool
> > +imply VIRTIO_VGA
> > +imply TEST_DEVICES
> > +select GOLDFISH_RTC
> > +select SERIAL
> > +select SIFIVE_TEST
> > +select VIRTIO_MMIO
> 
> You could include the liteeth device too if we merged that.

I think we could add that with a litex machine.  For that we would need at least
the litex UART and SoC for reset.

> > diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> > new file mode 100644
> > index 00..147196fda3
> > --- /dev/null
> > +++ b/hw/openrisc/virt.c
> > @@ -0,0 +1,429 @@
> > +/*
> > + * OpenRISC QEMU virtual machine.
> > + *
> > + * Copyright (c) 2022 Stafford Horne 
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see 
> > .
> 
> I think you can use the SPDX tag here instead of writing out the text.

Right.

> > +static void openrisc_virt_init(MachineState *machine)
> > +{
> > +ram_addr_t ram_size = machine->ram_size;
> > +const char *kernel_filename = machine->kernel_filename;
> > +OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
> > +OR1KVirtState *state = VIRT_MACHINE(machine);
> > +MemoryRegion *ram;
> > +hwaddr load_addr;
> > +int n;
> > +unsigned int smp_cpus = machine->smp.cpus;
> > +
> 
> > +openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
> > +

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

2022-06-04 Thread Stafford Horne 
On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> Hi Stafford,
> 
> On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne  wrote:
> > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley  wrote:
> > > > On Fri, 27 May 2022 at 17:27, Stafford Horne  wrote:
> > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > ports and the OpenRISC linux kernel port.
> > > > >
> > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > >
> > > > I enabled the options:
> > > >
> > > > CONFIG_RTC_CLASS=y
> > > > # CONFIG_RTC_SYSTOHC is not set
> > > > # CONFIG_RTC_NVMEM is not set
> > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > >
> > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > big endian guest running on my little endian host.
> > > >
> > > > Doing this fixes it:
> > > >
> > > > -.endianness = DEVICE_NATIVE_ENDIAN,
> > > > +.endianness = DEVICE_HOST_ENDIAN,
> > > >
> > > > [0.19] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > [0.19] goldfish_rtc 96005000.rtc: setting system clock to
> > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > >
> > > > But literally no other model in the tree does this, so I suspect it's
> > > > not the right fix.
> > >
> > > Goldfish devices are supposed to be little endian.
> > > Unfortunately m68k got this wrong, cfr.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > Please don't duplicate this bad behavior for new architectures
> >
> > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > play with it.  I was not attached to it. I can either remove it our find 
> > another
> > RTC.
> 
> Sorry for being too unclear: the mistake was not to use the Goldfish
> RTC, but to make its register accesses big-endian.
> Using Goldfish devices as little-endian devices should be fine.

OK, then I would think this patch would be needed on Goldfish.  I tested this
out and it seems to work:

Patch:

diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index 35e493be31..f1dc5af297 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
version_id)
 static const MemoryRegionOps goldfish_rtc_ops = {
 .read = goldfish_rtc_read,
 .write = goldfish_rtc_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
+.endianness = DEVICE_LITTLE_ENDIAN,
 .valid = {
 .min_access_size = 4,
 .max_access_size = 4

Boot Log:

io scheduler mq-deadline registered
io scheduler kyber registered
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
9000.serial: ttyS0 at MMIO 0x9000 (irq = 2, base_baud = 125) is 
a 16550A
printk: console [ttyS0] enabled
loop: module loaded
virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
Freeing initrd memory: 1696K
   *goldfish_rtc 96005000.rtc: registered as rtc0
   *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC 
(1654393797)
NET: Registered PF_PACKET protocol family
random: fast init done

-Stafford

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

2022-06-04 Thread Stafford Horne 
On Thu, Jun 02, 2022 at 08:39:21AM -0700, Richard Henderson wrote:
> On 5/27/22 10:27, Stafford Horne wrote:
> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
> ...
> > +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)
> 
> Just call the helper "semihosting" and be done with it.
> And the helper wants an ifdef for system mode.
> 
> > @@ -10,6 +10,7 @@ openrisc_ss.add(files(
> > 'fpu_helper.c',
> > 'gdbstub.c',
> > 'interrupt_helper.c',
> > +  'openrisc-semi.c',
> > 'sys_helper.c',
> > 'translate.c',
> >   ))
> 
> You want to add the new file for system mode only.
> Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

That's right, I'll update it.

> > +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
> > +{
> > +cpu_set_gpr(env, 11, ret);
> > +}
> 
> Let's drop this until you actually use it.  This appears to be attempting to
> mirror other, more complete semihosting, but missing the third "error"
> argument

Sure, I did mention I kept these here for future (real) semihosting support.
But I don't think that will happen.  So I can remove.

> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
> > +{
> > +uint32_t result;
> > +
> > +switch (k) {
> > +case HOSTED_EXIT:
> > +gdb_exit(cpu_get_gpr(env, 3));
> > +exit(cpu_get_gpr(env, 3));
> > +case HOSTED_RESET:
> > +#ifndef CONFIG_USER_ONLY
> > +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +return;
> > +#endif
> 
> Do you in fact want to exit to the main loop after asking for reset?
> That's the only way that "no return value" makes sense to me...

OK. I'll look at this more.
 
> > +default:
> > +qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
> > +  "semihosting syscall %d\n", k);
> 
> %u.

OK.

> >   static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
> >   {
> > +if (semihosting_enabled() &&
> > +a->k != 0) {
> > +gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
> > +}
> 
> Perhaps cleaner to move the semihosting dispatch switch here, instead of
> leaving it to the runtime?  The reason we have a runtime switch for other
> guests is that the semihosting syscall number is in a register.  This would
> then be
> 
> if (semihosting_enabled()) {
> switch (a->k) {
> case 0:
> break; /* normal nop */
> case HOSTED_EXIT:
> gen_helper_semihost_exit(cpu_R(dc, 3));
> break;
> case HOSTED_RESET:
> gen_helper_semihost_reset();
> tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
> 
> dc->base.is_jmp = DISAS_EXIT;
> break;
> ...
> }
> }

Yeah, that makes sense. I had written it in a way that would allow expanding for
real semi-hosting.  But I don't think we will do that with OpenRISC, so this is
good enough.

I am not sure if you saw the cover letter. I sent this RFC series to help
illustrate two options for providing OpenRISC targets that support poweroff and
reset.

One option being using these NOP's, the second is to create a virt target with
reset/poweroff hardware.

I am kind of leaning towards dropping the semi-hosting patches and only moving
forward with the virt patches.  The reason being that 1. we would not need to
expand the architecture spec to support the qemu virt platform, and we would
need to document the NOP's formally, and 2. OpenRISC doesn't really support the
full "semihosting" facilities for file open/close/write etc.

Any thoughts?  I guess this "semihosting" patch is pretty trivial.  But, maybe
it causes more confusion compared to just going with the virt route.  Also, if
we have virt I can't imagine anyone using the semihosting much.

-Stafford

[PATCH] docs/devel/*.txt: convert remaining files to restructuredText

2022-06-04 Thread oxr463 
From: Lucas Ramage 

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/527
Signed-off-by: Lucas Ramage 
---
 docs/devel/{blkdebug.txt => blkdebug.rst} |  8 +-
 docs/devel/{blkverify.txt => blkverify.rst}   | 12 ++---
 docs/devel/index-build.rst|  2 ++
 docs/devel/index-internals.rst|  5 
 docs/devel/{lockcnt.txt => lockcnt.rst}   | 25 ++-
 ...e-iothreads.txt => multiple-iothreads.rst} |  3 +++
 docs/devel/{rcu.txt => rcu.rst}   |  0
 docs/devel/{replay.txt => replay.rst} | 11 +---
 ...tio-migration.txt => virtio-migration.rst} |  6 ++---
 9 files changed, 48 insertions(+), 24 deletions(-)
 rename docs/devel/{blkdebug.txt => blkdebug.rst} (99%)
 rename docs/devel/{blkverify.txt => blkverify.rst} (94%)
 rename docs/devel/{lockcnt.txt => lockcnt.rst} (95%)
 rename docs/devel/{multiple-iothreads.txt => multiple-iothreads.rst} (99%)
 rename docs/devel/{rcu.txt => rcu.rst} (100%)
 rename docs/devel/{replay.txt => replay.rst} (96%)
 rename docs/devel/{virtio-migration.txt => virtio-migration.rst} (98%)

diff --git a/docs/devel/blkdebug.txt b/docs/devel/blkdebug.rst
similarity index 99%
rename from docs/devel/blkdebug.txt
rename to docs/devel/blkdebug.rst
index 0b0c128d35..660d35aaf8 100644
--- a/docs/devel/blkdebug.txt
+++ b/docs/devel/blkdebug.rst
@@ -1,5 +1,6 @@
 Block I/O error injection using blkdebug
-
+
+
 Copyright (C) 2014-2015 Red Hat Inc

 This work is licensed under the terms of the GNU GPL, version 2 or later.  See
@@ -13,6 +14,7 @@ This document gives an overview of the features available in 
blkdebug.

 Background
 --
+
 Block drivers have many error code paths that handle I/O errors.  Image formats
 are especially complex since metadata I/O errors during cluster allocation or
 while updating tables happen halfway through request processing and require
@@ -23,6 +25,7 @@ This way, all error paths can be tested to make sure they are 
correct.

 Rules
 -
+
 The blkdebug block driver takes a list of "rules" that tell the error injection
 engine when to fail an I/O request.

@@ -77,6 +80,7 @@ Rules support the following attributes:

 Events
 --
+
 Block drivers provide information about the type of I/O request they are about
 to make so rules can match specific types of requests.  For example, the qcow2
 block driver tells blkdebug when it accesses the L1 table so rules can match
@@ -98,6 +102,7 @@ meaning of specific events.

 State transitions
 -
+
 There are cases where more power is needed to match a particular I/O request in
 a longer sequence of requests.  For example:

@@ -149,6 +154,7 @@ State transition rules support the following attributes:

 Suspend and resume
 --
+
 Exercising code paths in block drivers may require specific ordering amongst
 concurrent requests.  The "breakpoint" feature allows requests to be halted on
 a blkdebug event and resumed later.  This makes it possible to achieve
diff --git a/docs/devel/blkverify.txt b/docs/devel/blkverify.rst
similarity index 94%
rename from docs/devel/blkverify.txt
rename to docs/devel/blkverify.rst
index aca826c51c..7e1b0fb500 100644
--- a/docs/devel/blkverify.txt
+++ b/docs/devel/blkverify.rst
@@ -1,6 +1,8 @@
-= Block driver correctness testing with blkverify =
+Block driver correctness testing with blkverify
+===

-== Introduction ==
+Introduction
+

 This document describes how to use the blkverify protocol to test that a block
 driver is operating correctly.
@@ -14,7 +16,8 @@ driver.
 Blkverify solves this problem by catching data corruption inside QEMU the first
 time bad data is read and reporting the disk sector that is corrupted.

-== How it works ==
+How it works
+

 The blkverify protocol has two child block devices, the "test" device and the
 "raw" device.  Read/write operations are mirrored to both devices so their
@@ -29,7 +32,8 @@ After a mirrored read operation completes, blkverify will 
compare the data and
 raise an error if it is not identical.  This makes it possible to catch the
 first instance where corrupt data is read.

-== Example ==
+Example
+---

 Imagine raw.img has 0xcd repeated throughout its first sector:

diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
index 1002a533a6..514ef308fc 100644
--- a/docs/devel/index-build.rst
+++ b/docs/devel/index-build.rst
@@ -11,6 +11,8 @@ the basics if you are adding new files and targets to the 
build.
build-system
kconfig
testing
+   blkdebug
+   blkverify
qtest
ci
qapi-code-gen
diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst
index e1a93df263..16f57b87da 100644
--- a/docs/devel/index-internals.rst
+++ b/docs/devel/index-internals.rst
@@ -11,11 +11,16 @@ Details about QEMU's various subsystems including how to 
ad

[PATCH 2/3] target/riscv: Remove generate_exception_mtval

2022-06-04 Thread Richard Henderson 
The function doesn't set mtval, it sets badaddr. Move the set
of badaddr directly into gen_exception_inst_addr_mis and use
generate_exception.

Signed-off-by: Richard Henderson 
---
 target/riscv/translate.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 9196aa71db..6e4bbea1cd 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -228,14 +228,6 @@ static void generate_exception(DisasContext *ctx, int excp)
 ctx->base.is_jmp = DISAS_NORETURN;
 }
 
-static void generate_exception_mtval(DisasContext *ctx, int excp)
-{
-gen_set_pc_imm(ctx, ctx->base.pc_next);
-tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr));
-gen_helper_raise_exception(cpu_env, tcg_constant_i32(excp));
-ctx->base.is_jmp = DISAS_NORETURN;
-}
-
 static void gen_exception_illegal(DisasContext *ctx)
 {
 tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
@@ -245,7 +237,8 @@ static void gen_exception_illegal(DisasContext *ctx)
 
 static void gen_exception_inst_addr_mis(DisasContext *ctx)
 {
-generate_exception_mtval(ctx, RISCV_EXCP_INST_ADDR_MIS);
+tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr));
+generate_exception(ctx, RISCV_EXCP_INST_ADDR_MIS);
 }
 
 static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
-- 
2.34.1

[PATCH 3/3] target/riscv: Minimize the calls to decode_save_opc

2022-06-04 Thread Richard Henderson 
The set of instructions that require decode_save_opc for
unwinding is really fairly small -- only insns that can
raise ILLEGAL_INSN at runtime.  This includes CSR, anything
that uses a *new* fp rounding mode, and many privileged insns.

Since unwind info is stored as the difference from the
previous insn, storing a 0 for most insns minimizes the
size of the unwind info.

Booting a debian kernel image to the missing rootfs panic yields

- gen code size   6819/1026886656
+ gen code size   21601907/1026886656

on 41k TranslationBlocks, a savings of 610kB or a bit less than 3%.

Signed-off-by: Richard Henderson 
---
 target/riscv/translate.c   | 18 +-
 target/riscv/insn_trans/trans_privileged.c.inc |  4 
 target/riscv/insn_trans/trans_rvh.c.inc|  2 ++
 target/riscv/insn_trans/trans_rvi.c.inc|  2 ++
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 6e4bbea1cd..b328a7b2ff 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -204,6 +204,13 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in)
 tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan);
 }
 
+static void decode_save_opc(DisasContext *ctx)
+{
+assert(ctx->insn_start != NULL);
+tcg_set_insn_start_param(ctx->insn_start, 1, ctx->opcode);
+ctx->insn_start = NULL;
+}
+
 static void gen_set_pc_imm(DisasContext *ctx, target_ulong dest)
 {
 if (get_xl(ctx) == MXL_RV32) {
@@ -633,6 +640,8 @@ static void gen_set_rm(DisasContext *ctx, int rm)
 return;
 }
 
+/* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
+decode_save_opc(ctx);
 gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm));
 }
 
@@ -1011,13 +1020,6 @@ static uint32_t opcode_at(DisasContextBase *dcbase, 
target_ulong pc)
 /* Include decoders for factored-out extensions */
 #include "decode-XVentanaCondOps.c.inc"
 
-static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
-{
-assert(ctx->insn_start != NULL);
-tcg_set_insn_start_param(ctx->insn_start, 1, opc);
-ctx->insn_start = NULL;
-}
-
 static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 {
 /*
@@ -1034,7 +1036,6 @@ static void decode_opc(CPURISCVState *env, DisasContext 
*ctx, uint16_t opcode)
 
 /* Check for compressed insn */
 if (extract16(opcode, 0, 2) != 3) {
-decode_save_opc(ctx, opcode);
 if (!has_ext(ctx, RVC)) {
 gen_exception_illegal(ctx);
 } else {
@@ -1049,7 +1050,6 @@ static void decode_opc(CPURISCVState *env, DisasContext 
*ctx, uint16_t opcode)
 opcode32 = deposit32(opcode32, 16, 16,
  translator_lduw(env, &ctx->base,
  ctx->base.pc_next + 2));
-decode_save_opc(ctx, opcode32);
 ctx->opcode = opcode32;
 ctx->pc_succ_insn = ctx->base.pc_next + 4;
 
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc 
b/target/riscv/insn_trans/trans_privileged.c.inc
index 53613682e8..46f96ad74d 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -75,6 +75,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 {
 #ifndef CONFIG_USER_ONLY
 if (has_ext(ctx, RVS)) {
+decode_save_opc(ctx);
 gen_helper_sret(cpu_pc, cpu_env);
 tcg_gen_exit_tb(NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
@@ -90,6 +91,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 static bool trans_mret(DisasContext *ctx, arg_mret *a)
 {
 #ifndef CONFIG_USER_ONLY
+decode_save_opc(ctx);
 gen_helper_mret(cpu_pc, cpu_env);
 tcg_gen_exit_tb(NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
@@ -102,6 +104,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 {
 #ifndef CONFIG_USER_ONLY
+decode_save_opc(ctx);
 gen_set_pc_imm(ctx, ctx->pc_succ_insn);
 gen_helper_wfi(cpu_env);
 return true;
@@ -113,6 +116,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 {
 #ifndef CONFIG_USER_ONLY
+decode_save_opc(ctx);
 gen_helper_tlb_flush(cpu_env);
 return true;
 #endif
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc 
b/target/riscv/insn_trans/trans_rvh.c.inc
index cebcb3f8f6..4f8aecddc7 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -169,6 +169,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, 
arg_sfence_vma *a)
 {
 REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+decode_save_opc(ctx);
 gen_helper_hyp_gvma_tlb_flush(cpu_env);
 return true;
 #endif
@@ -179,6 +180,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, 
arg_sfence_vma *a)
 {
 REQUIRE_EXT(ctx, RVH)

[PATCH 1/3] target/riscv: Set env->bins in gen_exception_illegal

2022-06-04 Thread Richard Henderson 
While we set env->bins when unwinding for ILLEGAL_INST,
from e.g. csrrw, we weren't setting it for immediately
illegal instructions.

Add a testcase for mtval via both exception paths.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1060
Signed-off-by: Richard Henderson 
---
 target/riscv/translate.c  |  2 +
 tests/tcg/riscv64/Makefile.softmmu-target | 21 +
 tests/tcg/riscv64/issue1060.S | 53 +++
 tests/tcg/riscv64/semihost.ld | 21 +
 4 files changed, 97 insertions(+)
 create mode 100644 tests/tcg/riscv64/Makefile.softmmu-target
 create mode 100644 tests/tcg/riscv64/issue1060.S
 create mode 100644 tests/tcg/riscv64/semihost.ld

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 55a4713af2..9196aa71db 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -238,6 +238,8 @@ static void generate_exception_mtval(DisasContext *ctx, int 
excp)
 
 static void gen_exception_illegal(DisasContext *ctx)
 {
+tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
+   offsetof(CPURISCVState, bins));
 generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
 }
 
diff --git a/tests/tcg/riscv64/Makefile.softmmu-target 
b/tests/tcg/riscv64/Makefile.softmmu-target
new file mode 100644
index 00..d51ece7023
--- /dev/null
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -0,0 +1,21 @@
+#
+# Aarch64 system tests
+#
+
+TEST_SRC = $(SRC_PATH)/tests/tcg/riscv64
+VPATH += $(TEST_SRC)
+
+LINK_SCRIPT = $(TEST_SRC)/semihost.ld
+LDFLAGS = -T $(LINK_SCRIPT)
+CFLAGS += -g -Og
+
+%.o: %.S
+   $(CC) $(CFLAGS) $< -c -o $@
+%: %.o $(LINK_SCRIPT)
+   $(LD) $(LDFLAGS) $< -o $@
+
+QEMU_OPTS += -M virt -display none -semihosting -device loader,file=
+
+EXTRA_RUNS += run-issue1060
+run-issue1060: issue1060
+   $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
diff --git a/tests/tcg/riscv64/issue1060.S b/tests/tcg/riscv64/issue1060.S
new file mode 100644
index 00..17b7fe1be2
--- /dev/null
+++ b/tests/tcg/riscv64/issue1060.S
@@ -0,0 +1,53 @@
+   .option norvc
+
+   .text
+   .global _start
+_start:
+   lla t0, trap
+   csrwmtvec, t0
+
+   # These are all illegal instructions
+   csrwtime, x0
+   .insn   i CUSTOM_0, 0, x0, x0, 0x321
+   csrwtime, x0
+   .insn   i CUSTOM_0, 0, x0, x0, 0x123
+   csrwcycle, x0
+
+   # Success!
+   li  a0, 0
+   j   _exit
+
+trap:
+   # When an instruction traps, compare it to the insn in memory.
+   csrrt0, mepc
+   csrrt1, mtval
+   lwu t2, 0(t0)
+   bne t1, t2, fail
+
+   # Skip the insn and continue.
+   addit0, t0, 4
+   csrwmepc, t0
+   mret
+
+fail:
+   li  a0, 1
+
+# Exit code in a0
+_exit:
+   lla a1, semiargs
+   li  t0, 0x20026 # ADP_Stopped_ApplicationExit
+   sd  t0, 0(a1)
+   sd  a0, 8(a1)
+   li  a0, 0x20# TARGET_SYS_EXIT_EXTENDED
+
+   # Semihosting call sequence
+   .balign 16
+   sllizero, zero, 0x1f
+   ebreak
+   sraizero, zero, 0x7
+   j   .
+
+   .data
+   .balign 16
+semiargs:
+   .space  16
diff --git a/tests/tcg/riscv64/semihost.ld b/tests/tcg/riscv64/semihost.ld
new file mode 100644
index 00..a59cc56b28
--- /dev/null
+++ b/tests/tcg/riscv64/semihost.ld
@@ -0,0 +1,21 @@
+ENTRY(_start)
+
+SECTIONS
+{
+/* virt machine, RAM starts at 2gb */
+. = 0x8000;
+.text : {
+*(.text)
+}
+.rodata : {
+*(.rodata)
+}
+/* align r/w section to next 2mb */
+. = ALIGN(1 << 21);
+.data : {
+*(.data)
+}
+.bss : {
+*(.bss)
+}
+}
-- 
2.34.1

[PATCH 0/3] target/riscv: Fix issue 1060

2022-06-04 Thread Richard Henderson 
This issue concerns the value of mtval for illegal
instruction exceptions, and came with a great test case.
The fix is just two lines, in the first patch, but
I noticed some cleanups on the way.


r~


Richard Henderson (3):
  target/riscv: Set env->bins in gen_exception_illegal
  target/riscv: Remove generate_exception_mtval
  target/riscv: Minimize the calls to decode_save_opc

 target/riscv/translate.c  | 31 +--
 .../riscv/insn_trans/trans_privileged.c.inc   |  4 ++
 target/riscv/insn_trans/trans_rvh.c.inc   |  2 +
 target/riscv/insn_trans/trans_rvi.c.inc   |  2 +
 tests/tcg/riscv64/Makefile.softmmu-target | 21 
 tests/tcg/riscv64/issue1060.S | 53 +++
 tests/tcg/riscv64/semihost.ld | 21 
 7 files changed, 116 insertions(+), 18 deletions(-)
 create mode 100644 tests/tcg/riscv64/Makefile.softmmu-target
 create mode 100644 tests/tcg/riscv64/issue1060.S
 create mode 100644 tests/tcg/riscv64/semihost.ld

-- 
2.34.1

Re: [PATCH 01/28] target/arm: Move stage_1_mmu_idx decl to internals.h

2022-06-04 Thread Richard Henderson 

On 6/4/22 03:40, Philippe Mathieu-Daudé wrote:

On 4/6/22 06:05, Richard Henderson wrote:

Move the decl from ptw.h to internals.h.  Provide an inline
version for user-only, just as we do for arm_stage1_mmu_idx.
Move an endif down to make the definition in helper.c be
system only.

Signed-off-by: Richard Henderson 
---
  target/arm/internals.h | 5 +
  target/arm/helper.c    | 5 ++---
  2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index b654bee468..72b6af5559 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -979,11 +979,16 @@ ARMMMUIdx arm_mmu_idx(CPUARMState *env);
   * Return the ARMMMUIdx for the stage1 traversal for the current regime.
   */
  #ifdef CONFIG_USER_ONLY
+static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
+{


Should we assert(mmu_idx == ARMMMUIdx_Stage1_E0)?


You mean ARMMMUIdx_EL10_0, the stage2 idx, but no, I don't think that's useful considering 
the ifdef.



r~

Re: [PATCH] linux-user/x86_64: Fix ELF_PLATFORM

2022-06-04 Thread Laurent Vivier 

Le 03/06/2022 à 23:38, Richard Henderson a écrit :

We had been using the i686 platform string for x86_64.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1041
Signed-off-by: Richard Henderson 
---
  linux-user/elfload.c | 30 +-
  1 file changed, 17 insertions(+), 13 deletions(-)



Reviewed-by: Laurent Vivier

Re: [PATCH] microvm: turn off io reservations for pcie root ports

2022-06-04 Thread Philippe Mathieu-Daudé via 

On 3/6/22 10:59, Gerd Hoffmann wrote:

The pcie host bridge has no io window on microvm,
so io reservations will not work.

Signed-off-by: Gerd Hoffmann 
---
  hw/i386/microvm.c | 6 ++
  1 file changed, 6 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH] test/tcg/arm: Use -mfloat-abi=soft for test-armv6m-undef

2022-06-04 Thread Philippe Mathieu-Daudé via 

On 4/6/22 05:27, Richard Henderson wrote:

GCC11 from crossbuild-essential-armhf from ubuntu 22.04 errors:
cc1: error: ‘-mfloat-abi=hard’: selected architecture lacks an FPU

Signed-off-by: Richard Henderson 
---
  tests/tcg/arm/Makefile.softmmu-target | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH] aio_wait_kick: add missing memory barrier

2022-06-04 Thread Roman Kagan 
On Tue, May 24, 2022 at 01:30:54PM -0400, Emanuele Giuseppe Esposito wrote:
> It seems that aio_wait_kick always required a memory barrier
> or atomic operation in the caller, but nobody actually
> took care of doing it.
> 
> Let's put the barrier in the function instead, and pair it
> with another one in AIO_WAIT_WHILE. Read aio_wait_kick()
> comment for further explanation.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  include/block/aio-wait.h |  2 ++
>  util/aio-wait.c  | 16 +++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
> index b39eefb38d..54840f8622 100644
> --- a/include/block/aio-wait.h
> +++ b/include/block/aio-wait.h
> @@ -81,6 +81,8 @@ extern AioWait global_aio_wait;
>  AioContext *ctx_ = (ctx);  \
>  /* Increment wait_->num_waiters before evaluating cond. */ \
>  qatomic_inc(&wait_->num_waiters);  \
> +/* Paired with smp_mb in aio_wait_kick(). */   \
> +smp_mb();  \

IIRC qatomic_inc() ensures sequential consistency, isn't it enough here?

>  if (ctx_ && in_aio_context_home_thread(ctx_)) {\
>  while ((cond)) {   \
>  aio_poll(ctx_, true);  \

Roman.

Re: [PATCH] linux-user/x86_64: Fix ELF_PLATFORM

2022-06-04 Thread Philippe Mathieu-Daudé via 

On 3/6/22 23:38, Richard Henderson wrote:

We had been using the i686 platform string for x86_64.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1041
Signed-off-by: Richard Henderson 
---
  linux-user/elfload.c | 30 +-
  1 file changed, 17 insertions(+), 13 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH 01/28] target/arm: Move stage_1_mmu_idx decl to internals.h

2022-06-04 Thread Philippe Mathieu-Daudé via 

On 4/6/22 06:05, Richard Henderson wrote:

Move the decl from ptw.h to internals.h.  Provide an inline
version for user-only, just as we do for arm_stage1_mmu_idx.
Move an endif down to make the definition in helper.c be
system only.

Signed-off-by: Richard Henderson 
---
  target/arm/internals.h | 5 +
  target/arm/helper.c| 5 ++---
  2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index b654bee468..72b6af5559 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -979,11 +979,16 @@ ARMMMUIdx arm_mmu_idx(CPUARMState *env);
   * Return the ARMMMUIdx for the stage1 traversal for the current regime.
   */
  #ifdef CONFIG_USER_ONLY
+static inline ARMMMUIdx stage_1_mmu_idx(ARMMMUIdx mmu_idx)
+{


Should we assert(mmu_idx == ARMMMUIdx_Stage1_E0)?


+return ARMMMUIdx_Stage1_E0;
+}

q: incorrect register emulation mask for Xen PCI passthrough?

2022-06-04 Thread Michael Tokarev 

There's a debian bugreport open - now against qemu - 
https://bugs.debian.org/988333 -
which initially said VGA Intel IGD Passthrough to Debian Xen HVM DomUs not 
working
but worked okay with windows DomUs.

The most interesting comment in there is the last one, 
https://bugs.debian.org/988333#146 ,
which sums it all up and provides a patch for the issue, at
https://github.com/QubesOS/qubes-vmm-xen-stubdom-linux/pull/3/commits/ab2b4c2ad02827a73c52ba561e9a921cc4bb227c
which is from 2017 (!).

I wonder if we should apply this one upstream, or if it is somehow incorrect
fix, should fix this particular issue the right way.  It's 5 years old already
and people are still suffering... :)

Thanks,

/mjt

Re: [PATCH v3 0/7] QOM'ify PIIX southbridge creation

2022-06-04 Thread Bernhard Beschow 
Am 30. Mai 2022 19:45:26 UTC schrieb "Philippe Mathieu-Daudé" :
>On 30/5/22 21:11, Mark Cave-Ayland wrote:
>> On 29/05/2022 14:02, Bernhard Beschow wrote:
>
>>>     Oh wait - I see now it's just the cover letter which is missing the 
>>> additional
>>>     maintainer addresses :)  If you could add them into the cover letter 
>>> for your next
>>>     revision that would be great, since it gives context for maintainers to 
>>> help with
>>>     the
>>>     review process.
>>> 
>>> 
>>> Hi Mark,
>>> 
>>> Thanks for your great work you put into reviews and the useful insights! It 
>>> seems to me that the time it takes for patches to be queued depends on the 
>>> subsystem - some are faster, some are slower...
>>> 
>>> I've automated my setup as described in [1]. However, it doesn't seem to 
>>> work for the cover letter which I'd expect to be sent to the union of all 
>>> reviewers of all patches. Any idea how to fix this?
>>> 
>>> Best regards,
>>> Bernhard
>>> 
>>> [1] 
>>> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer
>>>  
>>> 
>>>  
>> 
>> 
>> Good question. I tend to do "git format-patch -o /tmp/foo --cover-letter" to 
>> generate the series, fill in the cover letter, and then use "git send-email 
>> /tmp/foo" to send out the emails (entering in the results of 
>> get_maintainer.pl by hand). I'm not sure why the cover letter isn't being 
>> generated correctly in your case I'm afraid.
>
>Or try git-publish :) It does a first pass collecting Cc for each patch
>(calling get_maintainer.pl) then use that set on the cover.
>
>https://github.com/stefanha/git-publish

Yes, that worked. What an improvement!

Best regards,
Bernhard

Re: Changes for building bits on newer gcc 9.4 compiler

2022-06-04 Thread Ani Sinha 
On Fri, Jun 3, 2022 at 9:38 PM Ani Sinha  wrote:
>
> On an additional note, my changes are not backward compatible with
> older compiler. The build will break when built with a centos 7
> docker/vm/host:
>
> /home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:
> In function '_build_callargs':
> /home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:3707:6:
> error: empty declaration [-Werror]
>   __attribute__ ((fallthrough));
>   ^
> /home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:3707:6:
> error: ISO C90 forbids mixed declarations and code
> [-Werror=declaration-after-statement]
> /home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:
> At top level:
> cc1: error: unrecognized command line option
> "-Wno-shift-negative-value" [-Werror]
> cc1: error: unrecognized command line option "-Wno-cast-function-type" 
> [-Werror]
> cc1: error: unrecognized command line option
> "-Wno-address-of-packed-member" [-Werror]
> cc1: error: unrecognized command line option
> "-Wno-discarded-array-qualifiers" [-Werror]

I have fixed these. The code seems to build both on new and old compilers now.

>
> If fixing this is essential, we can ifdef some of these changes
> between compiler version checks.
>
> On Fri, Jun 3, 2022 at 2:06 PM Ani Sinha  wrote:
> >
> > Hi josh :
> > Here are the pull requests. Please feel free to review and merge:
> >
> > Main bits module:
> > https://github.com/biosbits/bits/pull/13
> >
> > Submodules:
> > https://github.com/biosbits/grub/pull/1
> > https://github.com/biosbits/python/pull/1
> > https://github.com/biosbits/libffi/pull/1
> > https://github.com/biosbits/fdlibm/pull/1
> >
> > Thanks
> > ani

Re: [PATCH] test/tcg/arm: Use -mfloat-abi=soft for test-armv6m-undef

2022-06-04 Thread Thomas Huth 

On 04/06/2022 05.27, Richard Henderson wrote:

GCC11 from crossbuild-essential-armhf from ubuntu 22.04 errors:
cc1: error: ‘-mfloat-abi=hard’: selected architecture lacks an FPU

Signed-off-by: Richard Henderson 
---
  tests/tcg/arm/Makefile.softmmu-target | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/arm/Makefile.softmmu-target 
b/tests/tcg/arm/Makefile.softmmu-target
index 3fe237ba39..7df88ddea8 100644
--- a/tests/tcg/arm/Makefile.softmmu-target
+++ b/tests/tcg/arm/Makefile.softmmu-target
@@ -20,7 +20,7 @@ LDFLAGS+=-nostdlib -N -static
  
  # Specific Test Rules
  
-test-armv6m-undef: EXTRA_CFLAGS+=-mcpu=cortex-m0

+test-armv6m-undef: EXTRA_CFLAGS+=-mcpu=cortex-m0 -mfloat-abi=soft
  
  run-test-armv6m-undef: QEMU_OPTS+=-semihosting -M microbit -kernel

  run-plugin-test-armv6m-undef-%: QEMU_OPTS+=-semihosting -M microbit -kernel


Reviewed-by: Thomas Huth