Re: QEMU commit 0a923be2f642 broke my or1k image.

[Stafford Horne]

Hi Rob,

Sorry, I haven't had much time to sit down and work on this mail in the last two
weeks but wanted to get somethign back to you.  Here it goes.

On Wed, Sep 11, 2024 at 12:42:58AM -0500, Rob Landley wrote:
> Grab this tarball, extract it, and ./run-qemu.sh. It's a simple
> linux+initramfs image that boots to a shell prompt.
> 
>   https://landley.net/bin/mkroot/latest/or1k.tgz
> 
> QEMU 7.0.0 ran that linux-or1k image, but newer qemu does not. I besected the
> issue to qemu commit 0a923be2f642, and it's still broken in current tip of 
> tree.

Patch is:

 0a923be2f6 ("hw/openrisc: page-align FDT address")

> Rebuilding the image with current linux-git doesn't seem to make a difference?
> Either way I get serial output with old qemu and don't with current qemu.

The bisect looks strange as it's only moving a page boundary, it could be
correct but it seems harmeless.  There is another commit close by that was
causing issues with serial output for the barebox guys and this is a patch I am
working on to fix it.

https://lore.kernel.org/qemu-devel/20240908062756.70514-1-sho...@gmail.com/

I will try to get time today to look at your tarball and run it, but if not have
a look at this patch.

-Stafford

> Rob
> 
> P.S. Reproduction sequence for the tarball available upon request, kernel 
> config
> is in the docs/ directory, userspace is just a toybox mkroot build but "not
> getting kernel boot messages" happens way before userspace gets a vote.
> 

Re: [PATCH] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

On Fri, Sep 13, 2024 at 6:47 PM Peter Maydell  wrote:
>
> On Fri, 13 Sept 2024 at 16:55, Peter Xu  wrote:
> >
> > On Thu, Sep 12, 2024 at 03:27:55PM +0100, Peter Maydell wrote:
> > > Coverity is pretty unhappy about this trick, because it isn't able
> > > to recognise that we can figure out the address of 'bounce'
> > > from the address of 'bounce->buffer' and free it in the
> > > address_space_unmap() code, so it thinks that every use
> > > of address_space_map(), pci_dma_map(), etc, is a memory leak.
> > > We can mark all those as false positives, of course, but it got
> > > me wondering whether maybe we should have this function return
> > > a struct that has all the information address_space_unmap()
> > > needs rather than relying on it being able to figure it out
> > > from the host memory pointer...
> >
> > Indeed that sounds like a viable option.  Looks like we don't have a lot of
> > address_space_map() users.
>
> There's quite a few wrappers of it too, so it's a little hard to count.
> We might want to avoid the memory allocation in the common case
> by having the caller pass in an ASMapInfo struct to be filled
> in rather than having address_space_map() allocate-and-return one.

Hm, this would work, but not only does it complicate the code
consuming address_space_map, but it also increases memory footprint (a
pointer being replaced by a struct of sizeof(BounceBuffer) if done
naively), plus there's an additional pointer indirection (I'm doubtful
whether this can be optimized away by the compiler). I haven't done
any measurements of these effects, so can't say anything definitive,
but this seems pretty costly just to appease coverity...

Is there no way to inform coverity that a resource pointer is being
transmuted into a handle, so it can track that instead? Given that
pointer tricks like this and container_of usage is quite frequent, I
would expect coverity to have a better strategy to handle these rather
than suppressing false positive leak reports?

If coverity can be made to understand this, it seems we should at
least be able to mark the allocation in question as intentional leak
(and thus not to report) at the position in the code where we allocate
the bounce buffer, rather than having to create false positive
suppressions for every caller. Perhaps this would be simplified if we
extract out a function to allocate and initialize the bounce buffer,
and then tell coverity to assume that anything it returns is not a
leak? (My coverity experience is rather limited, just thinking out
loud based on what I've seen in other projects to help static analysis
tools in general)

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

Thanks for the report, and my apologies for the breakage.

On Fri, Sep 13, 2024 at 4:47 PM Peter Xu  wrote:
>
> On Fri, Sep 13, 2024 at 04:35:32PM +0200, Cédric Le Goater wrote:
> > Hello,
> >
> > +Mark (for the Mac devices)
> >
> > On 9/9/24 22:11, Peter Xu wrote:
> > > From: Mattias Nissler 
> > >
> > > When DMA memory can't be directly accessed, as is the case when
> > > running the device model in a separate process without shareable DMA
> > > file descriptors, bounce buffering is used.
> > >
> > > It is not uncommon for device models to request mapping of several DMA
> > > regions at the same time. Examples include:
> > >   * net devices, e.g. when transmitting a packet that is split across
> > > several TX descriptors (observed with igb)
> > >   * USB host controllers, when handling a packet with multiple data TRBs
> > > (observed with xhci)
> > >
> > > Previously, qemu only provided a single bounce buffer per AddressSpace
> > > and would fail DMA map requests while the buffer was already in use. In
> > > turn, this would cause DMA failures that ultimately manifest as hardware
> > > errors from the guest perspective.
> > >
> > > This change allocates DMA bounce buffers dynamically instead of
> > > supporting only a single buffer. Thus, multiple DMA mappings work
> > > correctly also when RAM can't be mmap()-ed.
> > >
> > > The total bounce buffer allocation size is limited individually for each
> > > AddressSpace. The default limit is 4096 bytes, matching the previous
> > > maximum buffer size. A new x-max-bounce-buffer-size parameter is
> > > provided to configure the limit for PCI devices.
> > >
> > > Signed-off-by: Mattias Nissler 
> > > Reviewed-by: Philippe Mathieu-Daudé 
> > > Acked-by: Peter Xu 
> > > Link: 
> > > https://lore.kernel.org/r/20240819135455.2957406-1-mniss...@rivosinc.com
> > > Signed-off-by: Peter Xu 
> > > ---
> > >   include/exec/memory.h   | 14 +++
> > >   include/hw/pci/pci_device.h |  3 ++
> > >   hw/pci/pci.c|  8 
> > >   system/memory.c |  5 ++-
> > >   system/physmem.c| 82 ++---
> > >   5 files changed, 76 insertions(+), 36 deletions(-)
> >
> > Here is a report of a segv of the ppc64 mac99+cpu970 machine booting debian.
> > See the stack trace below. Just wanted to let you know. I will digging 
> > further
> > next week.
> >
> > Thanks,
> >
> > C.
> >
> >
> >
> > Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
> > address_space_unmap (len=, access_len=0, is_write=false, 
> > buffer=0x0,
> > as=0x565d45c0 ) at ../system/physmem.c:
> >   assert(bounce->magic == BOUNCE_BUFFER_MAGIC);
> > (gdb) bt
> > #0  address_space_unmap
> > (len=, access_len=0, is_write=false, buffer=0x0, 
> > as=0x565d45c0 )
> > at ../system/physmem.c:
> > #1  address_space_unmap
> > (as=as@entry=0x565d45c0 , buffer=0x0, 
> > len=, is_write=, access_len=0) at 
> > ../system/physmem.c:3313
> > #2  0x5595ea48 in dma_memory_unmap
> > (access_len=, dir=, len=, 
> > buffer=, as=) at 
> > /home/legoater/work/qemu/qemu.git/include/sysemu/dma.h:236
> > #3  pmac_ide_atapi_transfer_cb (opaque=0x56c06470, ret=) 
> > at ../hw/ide/macio.c:122
> > #4  0x559861f3 in DBDMA_run (s=0x56c04c60) at 
> > ../hw/misc/macio/mac_dbdma.c:546
> > #5  DBDMA_run_bh (opaque=0x56c04c60) at ../hw/misc/macio/mac_dbdma.c:556
> > #6  0x55f19f33 in aio_bh_call (bh=bh@entry=0x56ab5570) at 
> > ../util/async.c:171
> > #7  0x55f1a0f5 in aio_bh_poll (ctx=ctx@entry=0x566af150) at 
> > ../util/async.c:218
> > #8  0x55f0269e in aio_dispatch (ctx=0x566af150) at 
> > ../util/aio-posix.c:423
> > #9  0x55f19d8e in aio_ctx_dispatch
> > (source=, callback=, user_data= > out>) at ../util/async.c:360
> > #10 0x77315f4f in g_main_context_dispatch () at 
> > /lib64/libglib-2.0.so.0
> > #11 0x55f1b488 in glib_pollfds_poll () at ../util/main-loop.c:287
> > #12 os_host_main_loop_wait (timeout=2143429) at ../util/main-loop.c:310
> > #13 main_loop_wait (nonblocking=nonblocking@entry=0) at 
> > ../util/main-loop.c:589
> > #14 0x55abeba3 in qemu_main_loop () at ../system/runstate.c:826
> > #15 0x55e63787 in qemu_default_main () at ../system/main.c:37
> > #16 0x76e29590 in __libc_start_call_main () at /lib64/libc.so.6
> > #17 0x76e29640 in __libc_start_main_impl () at /lib64/libc.so.6
> > #18 0x5588d4f5 in _start ()
>
> Thanks for the report!
>
> Mattias,
>
> Would you have time to take a look?

I noticed that the backtrace indicates address_space_unmap is called
with buffer=0x0, len=0. This wasn't really correct before my
concurrent bounce buffering change either, but it looks like the
previous code would have tolerated this to a certain extent (at least
no immediate crashes). Original code in question:

if (is_write) {
address_space_write(as, as->bounce.addr

Re: [PATCH 12/17] tests/tcg: ensure s390x-softmmu output redirected

[Alex Bennée]

Thomas Huth  writes:

> On 13/09/2024 19.26, Alex Bennée wrote:
>> The multiarch system tests output serial data which should be
>> redirected to the "output" chardev rather than echoed to the console.
>> Remove the unused EXTFLAGS variable while we are at it.
>> Signed-off-by: Alex Bennée 
>> ---
>>   tests/tcg/s390x/Makefile.softmmu-target | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/tests/tcg/s390x/Makefile.softmmu-target
>> b/tests/tcg/s390x/Makefile.softmmu-target
>> index f60f94b090..ad681bbe40 100644
>> --- a/tests/tcg/s390x/Makefile.softmmu-target
>> +++ b/tests/tcg/s390x/Makefile.softmmu-target
>> @@ -1,6 +1,6 @@
>>   S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
>>   VPATH+=$(S390X_SRC)
>> -QEMU_OPTS+=-action panic=exit-failure -nographic $(EXTFLAGS) -kernel
>> +QEMU_OPTS+=-action panic=exit-failure -nographic -serial chardev:output 
>> -kernel
>>   LINK_SCRIPT=$(S390X_SRC)/softmmu.ld
>>   CFLAGS+=-ggdb -O0
>>   LDFLAGS=-nostdlib -static
>
> EXTFLAGS has been added on purpose here, see commit
> 26a09ead7351f117ae780.

ahh missed that as I couldn't see it being used elsewhere. A comment
at the use site would have helped as git blame only gets you so far if
the line has changed multiple times.

>
>  Thomas

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 11/17] tests/tcg: only read/write 64 bit words on 64 bit systems

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> On 13/9/24 19:26, Alex Bennée wrote:
>> While the compilers will generally happily synthesise a 64 bit value
>> for you on 32 bit systems it doesn't exercise anything on QEMU. It
>> also makes it hard to accurately compare the accesses to test_data
>> when instrumenting.
>> Message-Id: <20240910140733.4007719-21-alex.ben...@linaro.org>
>> Reviewed-by: Pierrick Bouvier 
>> Signed-off-by: Alex Bennée 
>> ---
>>   tests/tcg/multiarch/system/memory.c | 26 +++---
>>   1 file changed, 19 insertions(+), 7 deletions(-)
>> diff --git a/tests/tcg/multiarch/system/memory.c
>> b/tests/tcg/multiarch/system/memory.c
>> index 8f2371975d..680dd4800b 100644
>> --- a/tests/tcg/multiarch/system/memory.c
>> +++ b/tests/tcg/multiarch/system/memory.c
>> @@ -163,6 +163,7 @@ static void init_test_data_u32(int offset)
>>   ml_printf("done %d @ %p\n", i, ptr);
>>   }
>>   +#if __SIZEOF_POINTER__ == 8
>
>>= ? :)

Ahh I was confused by the email quoting. Do we actually have any systems
with bigger pointers or is this just for future proofing?

>
>>   static void init_test_data_u64(int offset)
>>   {
>>   uint8_t count = 0;
>> @@ -187,6 +188,7 @@ static void init_test_data_u64(int offset)
>>   }
>>   ml_printf("done %d @ %p\n", i, ptr);
>>   }
>> +#endif
>> static bool read_test_data_u16(int offset)
>>   {
>> @@ -254,6 +256,7 @@ static bool read_test_data_u32(int offset)
>>   return true;
>>   }
>>   +#if __SIZEOF_POINTER__ == 8
>>   static bool read_test_data_u64(int offset)
>>   {
>>   uint64_t word, *ptr = (uint64_t *)&test_data[offset];
>> @@ -307,11 +310,16 @@ static bool read_test_data_u64(int offset)
>>   ml_printf("done %d @ %p\n", i, ptr);
>>   return true;
>>   }
>> +#endif
>> /* Read the test data and verify at various offsets */
>> -read_ufn read_ufns[] = { read_test_data_u16,
>> - read_test_data_u32,
>> - read_test_data_u64 };
>> +read_ufn read_ufns[] = {
>> +read_test_data_u16,
>> +read_test_data_u32,
>> +#if __SIZEOF_POINTER__ == 8
>> +read_test_data_u64
>> +#endif
>> +};
>> bool do_unsigned_reads(int start_off)
>>   {
>> @@ -476,10 +484,14 @@ bool do_signed_reads(bool neg_first)
>>   return ok;
>>   }
>>   -init_ufn init_ufns[] = { init_test_data_u8,
>> - init_test_data_u16,
>> - init_test_data_u32,
>> - init_test_data_u64 };
>> +init_ufn init_ufns[] = {
>> +init_test_data_u8,
>> +init_test_data_u16,
>> +init_test_data_u32,
>> +#if __SIZEOF_POINTER__ == 8
>> +init_test_data_u64
>> +#endif
>> +};
>> int main(void)
>>   {

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v2 08/18] tests/tcg/plugins/mem: add option to print memory accesses

[Alex Bennée]

From: Pierrick Bouvier 

By using "print-accesses=true" option, mem plugin will now print every
value accessed, with associated size, type (store vs load), symbol,
instruction address and phys/virt address accessed.

Reviewed-by: Richard Henderson 
Reviewed-by: Xingtao Yao 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240724194708.1843704-6-pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée 
---
 tests/tcg/plugins/mem.c | 69 -
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
index b650dddcce..086e6f5bdf 100644
--- a/tests/tcg/plugins/mem.c
+++ b/tests/tcg/plugins/mem.c
@@ -21,10 +21,15 @@ typedef struct {
 uint64_t io_count;
 } CPUCount;
 
+typedef struct {
+uint64_t vaddr;
+const char *sym;
+} InsnInfo;
+
 static struct qemu_plugin_scoreboard *counts;
 static qemu_plugin_u64 mem_count;
 static qemu_plugin_u64 io_count;
-static bool do_inline, do_callback;
+static bool do_inline, do_callback, do_print_accesses;
 static bool do_haddr;
 static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW;
 
@@ -60,6 +65,44 @@ static void vcpu_mem(unsigned int cpu_index, 
qemu_plugin_meminfo_t meminfo,
 }
 }
 
+static void print_access(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
+ uint64_t vaddr, void *udata)
+{
+InsnInfo *insn_info = udata;
+unsigned size = 8 << qemu_plugin_mem_size_shift(meminfo);
+const char *type = qemu_plugin_mem_is_store(meminfo) ? "store" : "load";
+qemu_plugin_mem_value value = qemu_plugin_mem_get_value(meminfo);
+uint64_t hwaddr =
+qemu_plugin_hwaddr_phys_addr(qemu_plugin_get_hwaddr(meminfo, vaddr));
+g_autoptr(GString) out = g_string_new("");
+g_string_printf(out,
+"0x%"PRIx64",%s,0x%"PRIx64",0x%"PRIx64",%d,%s,",
+insn_info->vaddr, insn_info->sym,
+vaddr, hwaddr, size, type);
+switch (value.type) {
+case QEMU_PLUGIN_MEM_VALUE_U8:
+g_string_append_printf(out, "0x%02"PRIx8, value.data.u8);
+break;
+case QEMU_PLUGIN_MEM_VALUE_U16:
+g_string_append_printf(out, "0x%04"PRIx16, value.data.u16);
+break;
+case QEMU_PLUGIN_MEM_VALUE_U32:
+g_string_append_printf(out, "0x%08"PRIx32, value.data.u32);
+break;
+case QEMU_PLUGIN_MEM_VALUE_U64:
+g_string_append_printf(out, "0x%016"PRIx64, value.data.u64);
+break;
+case QEMU_PLUGIN_MEM_VALUE_U128:
+g_string_append_printf(out, "0x%016"PRIx64"%016"PRIx64,
+   value.data.u128.high, value.data.u128.low);
+break;
+default:
+g_assert_not_reached();
+}
+g_string_append_printf(out, "\n");
+qemu_plugin_outs(out->str);
+}
+
 static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
 {
 size_t n = qemu_plugin_tb_n_insns(tb);
@@ -79,6 +122,16 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
qemu_plugin_tb *tb)
  QEMU_PLUGIN_CB_NO_REGS,
  rw, NULL);
 }
+if (do_print_accesses) {
+/* we leak this pointer, to avoid locking to keep track of it */
+InsnInfo *insn_info = g_malloc(sizeof(InsnInfo));
+const char *sym = qemu_plugin_insn_symbol(insn);
+insn_info->sym = sym ? sym : "";
+insn_info->vaddr = qemu_plugin_insn_vaddr(insn);
+qemu_plugin_register_vcpu_mem_cb(insn, print_access,
+ QEMU_PLUGIN_CB_NO_REGS,
+ rw, (void *) insn_info);
+}
 }
 }
 
@@ -117,6 +170,12 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
 return -1;
 }
+} else if (g_strcmp0(tokens[0], "print-accesses") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1],
+&do_print_accesses)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
 } else {
 fprintf(stderr, "option parsing failed: %s\n", opt);
 return -1;
@@ -129,6 +188,14 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 return -1;
 }
 
+if (do_print_accesses) {
+g_autoptr(GString) out = g_string_new("");
+g_string_printf(out,
+"insn_vaddr,insn_symbol,mem_vaddr,mem_hwaddr,"
+"access_size,access_type,mem_value\n");
+qemu_plugin_outs(out->str);
+}
+
 counts = qemu_plugin_scoreboard_new(sizeof(CPUCount));
 mem_count = qemu_plugin_scoreboard_u64_in_struct(
 counts, CPUCount, mem_count);
-- 
2.39.5

[PATCH v2 01/18] deprecation: don't enable TCG plugins by default on 32 bit hosts

[Alex Bennée]

The existing plugins already liberally use host pointer stuffing for
passing user data which will fail when doing 64 bit guests on 32 bit
hosts. We should discourage this by officially deprecating support and
adding another nail to the 32 bit host coffin.

Message-Id: <20240910140733.4007719-12-alex.ben...@linaro.org>
Reviewed-by: Pierrick Bouvier 
Signed-off-by: Alex Bennée 

---
v2
  - don't manually set based on CPU type, use __SIZEOF_POINTER__
---
 docs/about/deprecated.rst | 11 +++
 configure | 21 -
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 88f0f03786..f7c7c33d39 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -184,6 +184,17 @@ be an effective use of its limited resources, and thus 
intends to discontinue
 it. Since all recent x86 hardware from the past >10 years is capable of the
 64-bit x86 extensions, a corresponding 64-bit OS should be used instead.
 
+TCG Plugin support not enabled by default on 32-bit hosts (since 9.2)
+'
+
+While it is still possible to enable TCG plugin support for 32-bit
+hosts there are a number of potential pitfalls when instrumenting
+64-bit guests. The plugin APIs typically pass most addresses as
+uint64_t but practices like encoding that address in a host pointer
+for passing as user-data will lose data. As most software analysis
+benefits from having plenty of host memory it seems reasonable to
+encourage users to use 64 bit builds of QEMU for analysis work
+whatever targets they are instrumenting.
 
 System emulator CPUs
 
diff --git a/configure b/configure
index 7f6eb6b331..f376fa24b3 100755
--- a/configure
+++ b/configure
@@ -516,6 +516,25 @@ case "$cpu" in
 ;;
 esac
 
+# Now we have our CPU_CFLAGS we can check if we are targeting a 32 or
+# 64 bit host.
+
+check_64bit_host() {
+cat > $TMPC <

[PATCH v2 00/18] tcg plugins pre-PR (deprecations, mem apis, contrib plugins)

[Alex Bennée]

I think all these are ready to go having been mostly reviewed in previous
series. The following still need review:

  util/timer: avoid deadlock when shutting down
  tests/tcg: add a system test to check memory instrumentation
  tests/tcg: ensure s390x-softmmu output redirected
  tests/tcg/multiarch: add test for plugin memory access (0 acks, 1 sobs, 1 tbs)

v2
  - fix some nits
  - included fix to ips posted as an RFC before

Alex.

Akihiko Odaki (1):
  contrib/plugins: Add a plugin to generate basic block vectors

Alex Bennée (9):
  deprecation: don't enable TCG plugins by default on 32 bit hosts
  deprecation: don't enable TCG plugins by default with TCI
  contrib/plugins: control flow plugin
  tests/tcg: clean up output of memory system test
  tests/tcg: only read/write 64 bit words on 64 bit systems
  tests/tcg: ensure s390x-softmmu output redirected
  tests/tcg: add a system test to check memory instrumentation
  util/timer: avoid deadlock when shutting down
  contrib/plugins: avoid hanging program

Pierrick Bouvier (6):
  plugins: save value during memory accesses
  plugins: extend API to get latest memory value accessed
  tests/tcg: add mechanism to run specific tests with plugins
  tests/tcg: allow to check output of plugins
  tests/tcg/plugins/mem: add option to print memory accesses
  tests/tcg/multiarch: add test for plugin memory access

Rowan Hart (2):
  plugins: add plugin API to read guest memory
  plugins: add option to dump write argument to syscall plugin

 docs/about/deprecated.rst |  19 +
 docs/about/emulation.rst  |  44 +-
 configure |  32 +-
 accel/tcg/atomic_template.h   |  66 ++-
 include/hw/core/cpu.h |   4 +
 include/qemu/plugin.h |   4 +
 include/qemu/qemu-plugin.h|  64 ++-
 contrib/plugins/bbv.c | 158 +++
 contrib/plugins/cflow.c   | 384 ++
 contrib/plugins/ips.c |   5 +
 plugins/api.c |  53 +++
 plugins/core.c|   6 +
 tcg/tcg-op-ldst.c |  66 ++-
 tests/tcg/multiarch/system/memory.c   | 123 --
 tests/tcg/multiarch/test-plugin-mem-access.c  | 177 
 tests/tcg/plugins/mem.c   | 248 ++-
 tests/tcg/plugins/syscall.c   | 117 ++
 util/qemu-timer.c |  14 +-
 accel/tcg/atomic_common.c.inc |  13 +-
 accel/tcg/ldst_common.c.inc   |  38 +-
 contrib/plugins/Makefile  |   2 +
 plugins/qemu-plugins.symbols  |   2 +
 tests/tcg/Makefile.target |  12 +-
 tests/tcg/alpha/Makefile.softmmu-target   |   2 +-
 tests/tcg/alpha/Makefile.target   |   3 +
 tests/tcg/multiarch/Makefile.target   |  11 +
 tests/tcg/multiarch/check-plugin-output.sh|  36 ++
 .../multiarch/system/Makefile.softmmu-target  |   6 +
 .../system/validate-memory-counts.py  | 129 ++
 tests/tcg/ppc64/Makefile.target   |   5 +
 tests/tcg/s390x/Makefile.softmmu-target   |   8 +-
 31 files changed, 1768 insertions(+), 83 deletions(-)
 create mode 100644 contrib/plugins/bbv.c
 create mode 100644 contrib/plugins/cflow.c
 create mode 100644 tests/tcg/multiarch/test-plugin-mem-access.c
 create mode 100755 tests/tcg/multiarch/check-plugin-output.sh
 create mode 100755 tests/tcg/multiarch/system/validate-memory-counts.py

-- 
2.39.5

[PATCH v2 03/18] contrib/plugins: control flow plugin

[Alex Bennée]

This is a simple control flow tracking plugin that uses the latest
inline and conditional operations to detect and track control flow
changes. It is currently an exercise at seeing how useful the changes
are.

Reviewed-by: Pierrick Bouvier 
Message-Id: <20240910140733.4007719-14-alex.ben...@linaro.org>
Based-on: <20240312075428.244210-1-pierrick.bouv...@linaro.org>
Cc: Gustavo Romero 
Signed-off-by: Alex Bennée 

---
v2
  - only need a single call back
  - drop need for INSN_WIDTH
  - still don't understand the early exits

v3
  - move initial STORE ops to first instruction to avoid confusion
  with the conditional callback on the start
  - filter out non-branches before processing
  - fix off-by-one with accounting
  - display "sync fault" or "branch" instead of raw numbers
v4
  - rename hotdest to hottest (i.e. the hottest branch insn)
  - rename early to exception
  - WIP insn structure
v5
  - dropped #if 0 code and unused vars
---
 contrib/plugins/cflow.c  | 384 +++
 contrib/plugins/Makefile |   1 +
 2 files changed, 385 insertions(+)
 create mode 100644 contrib/plugins/cflow.c

diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
new file mode 100644
index 00..c0dc90b0fe
--- /dev/null
+++ b/contrib/plugins/cflow.c
@@ -0,0 +1,384 @@
+/*
+ * Control Flow plugin
+ *
+ * This plugin will track changes to control flow and detect where
+ * instructions fault.
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+typedef enum {
+SORT_HOTTEST,  /* hottest branch insn */
+SORT_EXCEPTION,/* most early exits */
+SORT_POPDEST,  /* most destinations (usually ret's) */
+} ReportType;
+
+ReportType report = SORT_HOTTEST;
+int topn = 10;
+
+typedef struct {
+uint64_t daddr;
+uint64_t dcount;
+} DestData;
+
+/* A node is an address where we can go to multiple places */
+typedef struct {
+GMutex lock;
+/* address of the branch point */
+uint64_t addr;
+/* array of DestData */
+GArray *dests;
+/* early exit/fault count */
+uint64_t early_exit;
+/* jump destination count */
+uint64_t dest_count;
+/* instruction data */
+char *insn_disas;
+/* symbol? */
+const char *symbol;
+/* times translated as last in block? */
+int last_count;
+/* times translated in the middle of block? */
+int mid_count;
+} NodeData;
+
+typedef enum {
+/* last insn in block, expected flow control */
+LAST_INSN = (1 << 0),
+/* mid-block insn, can only be an exception */
+EXCP_INSN = (1 << 1),
+/* multiple disassembly, may have changed */
+MULT_INSN = (1 << 2),
+} InsnTypes;
+
+typedef struct {
+/* address of the branch point */
+uint64_t addr;
+/* disassembly */
+char *insn_disas;
+/* symbol? */
+const char *symbol;
+/* types */
+InsnTypes type_flag;
+} InsnData;
+
+/* We use this to track the current execution state */
+typedef struct {
+/* address of end of block */
+uint64_t end_block;
+/* next pc after end of block */
+uint64_t pc_after_block;
+/* address of last executed PC */
+uint64_t last_pc;
+} VCPUScoreBoard;
+
+/* descriptors for accessing the above scoreboard */
+static qemu_plugin_u64 end_block;
+static qemu_plugin_u64 pc_after_block;
+static qemu_plugin_u64 last_pc;
+
+
+static GMutex node_lock;
+static GHashTable *nodes;
+struct qemu_plugin_scoreboard *state;
+
+/* SORT_HOTTEST */
+static gint hottest(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->dest_count > nb->dest_count ? -1 :
+na->dest_count == nb->dest_count ? 0 : 1;
+}
+
+static gint exception(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->early_exit > nb->early_exit ? -1 :
+na->early_exit == nb->early_exit ? 0 : 1;
+}
+
+static gint popular(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->dests->len > nb->dests->len ? -1 :
+na->dests->len == nb->dests->len ? 0 : 1;
+}
+
+/* Filter out non-branches - returns true to remove entry */
+static gboolean filter_non_branches(gpointer key, gpointer value, gpointer 
user_data)
+{
+NodeData *node = (NodeData *) value;
+
+return node->dest_count == 0;
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+g_autoptr(GString) result = g_string_new("collected ");
+GList *data;
+GCompareFunc sort = &hottest;
+int n = 0;
+
+g_mutex_lock(&node_lock);
+g_string_append_printf(result, "%d control flow nodes in the hash table\n",
+   g_hash_table_size(nodes));
+
+/* remove all nodes that didn't branch */
+g_has

[PATCH v2 07/18] tests/tcg: allow to check output of plugins

[Alex Bennée]

From: Pierrick Bouvier 

A specific plugin test can now read and check a plugin output, to ensure
it contains expected values.

Tested-by: Xingtao Yao 
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240724194708.1843704-5-pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée 
---
 tests/tcg/Makefile.target | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index c5b1c7a786..2da70b2fcf 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -90,6 +90,7 @@ CFLAGS=
 LDFLAGS=
 
 QEMU_OPTS=
+CHECK_PLUGIN_OUTPUT_COMMAND=
 
 
 # If TCG debugging, or TCI is enabled things are a lot slower
@@ -180,6 +181,10 @@ run-plugin-%:
-plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \
-d plugin -D $*.pout \
 $(call strip-plugin,$<))
+   $(if $(CHECK_PLUGIN_OUTPUT_COMMAND),  \
+   $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \
+  TEST, check plugin $(call extract-plugin,$@) output\
+  with $(call strip-plugin,$<)))
 else
 run-%: %
$(call run-test, $<, \
@@ -194,6 +199,10 @@ run-plugin-%:
  -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) 
\
  -d plugin -D $*.pout \
  $(QEMU_OPTS) $(call strip-plugin,$<))
+   $(if $(CHECK_PLUGIN_OUTPUT_COMMAND),  \
+   $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \
+  TEST, check plugin $(call extract-plugin,$@) output\
+  with $(call strip-plugin,$<)))
 endif
 
 gdb-%: %
-- 
2.39.5

[PATCH v2 05/18] plugins: extend API to get latest memory value accessed

[Alex Bennée]

From: Pierrick Bouvier 

This value can be accessed only during a memory callback, using
new qemu_plugin_mem_get_value function.

Returned value can be extended when QEMU will support accesses wider
than 128 bits.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1719
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2152
Reviewed-by: Richard Henderson 
Reviewed-by: Xingtao Yao 
Reviewed-by: Alex Bennée 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240724194708.1843704-3-pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée 
---
 include/qemu/qemu-plugin.h   | 32 
 plugins/api.c| 33 +
 plugins/qemu-plugins.symbols |  1 +
 3 files changed, 66 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c71c705b69..649ce89815 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
 QEMU_PLUGIN_MEM_RW,
 };
 
+enum qemu_plugin_mem_value_type {
+QEMU_PLUGIN_MEM_VALUE_U8,
+QEMU_PLUGIN_MEM_VALUE_U16,
+QEMU_PLUGIN_MEM_VALUE_U32,
+QEMU_PLUGIN_MEM_VALUE_U64,
+QEMU_PLUGIN_MEM_VALUE_U128,
+};
+
+/* typedef qemu_plugin_mem_value - value accessed during a load/store */
+typedef struct {
+enum qemu_plugin_mem_value_type type;
+union {
+uint8_t u8;
+uint16_t u16;
+uint32_t u32;
+uint64_t u64;
+struct {
+uint64_t low;
+uint64_t high;
+} u128;
+} data;
+} qemu_plugin_mem_value;
+
 /**
  * enum qemu_plugin_cond - condition to enable callback
  *
@@ -551,6 +574,15 @@ bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t 
info);
 QEMU_PLUGIN_API
 bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
 
+/**
+ * qemu_plugin_mem_get_mem_value() - return last value loaded/stored
+ * @info: opaque memory transaction handle
+ *
+ * Returns: memory value
+ */
+QEMU_PLUGIN_API
+qemu_plugin_mem_value qemu_plugin_mem_get_value(qemu_plugin_meminfo_t info);
+
 /**
  * qemu_plugin_get_hwaddr() - return handle for memory operation
  * @info: opaque memory info structure
diff --git a/plugins/api.c b/plugins/api.c
index 2ff13d09de..3316d4a04d 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -351,6 +351,39 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
 return get_plugin_meminfo_rw(info) & QEMU_PLUGIN_MEM_W;
 }
 
+qemu_plugin_mem_value qemu_plugin_mem_get_value(qemu_plugin_meminfo_t info)
+{
+uint64_t low = current_cpu->neg.plugin_mem_value_low;
+qemu_plugin_mem_value value;
+
+switch (qemu_plugin_mem_size_shift(info)) {
+case 0:
+value.type = QEMU_PLUGIN_MEM_VALUE_U8;
+value.data.u8 = (uint8_t)low;
+break;
+case 1:
+value.type = QEMU_PLUGIN_MEM_VALUE_U16;
+value.data.u16 = (uint16_t)low;
+break;
+case 2:
+value.type = QEMU_PLUGIN_MEM_VALUE_U32;
+value.data.u32 = (uint32_t)low;
+break;
+case 3:
+value.type = QEMU_PLUGIN_MEM_VALUE_U64;
+value.data.u64 = low;
+break;
+case 4:
+value.type = QEMU_PLUGIN_MEM_VALUE_U128;
+value.data.u128.low = low;
+value.data.u128.high = current_cpu->neg.plugin_mem_value_high;
+break;
+default:
+g_assert_not_reached();
+}
+return value;
+}
+
 /*
  * Virtual Memory queries
  */
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index ca773d8d9f..eed9d8abd9 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -13,6 +13,7 @@
   qemu_plugin_insn_size;
   qemu_plugin_insn_symbol;
   qemu_plugin_insn_vaddr;
+  qemu_plugin_mem_get_value;
   qemu_plugin_mem_is_big_endian;
   qemu_plugin_mem_is_sign_extended;
   qemu_plugin_mem_is_store;
-- 
2.39.5

[PATCH v2 02/18] deprecation: don't enable TCG plugins by default with TCI

[Alex Bennée]

The softmmu memory instrumentation test sees so many more accesses
than a normal translated host and its really not worth fixing up. Lets
deprecate this odd configuration and save on the CI cycles.

Message-Id: <20240910140733.4007719-13-alex.ben...@linaro.org>
Reviewed-by: Pierrick Bouvier 
Signed-off-by: Alex Bennée 
---
 docs/about/deprecated.rst |  8 
 configure | 11 +--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index f7c7c33d39..5aa2e35314 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -196,6 +196,14 @@ benefits from having plenty of host memory it seems 
reasonable to
 encourage users to use 64 bit builds of QEMU for analysis work
 whatever targets they are instrumenting.
 
+TCG Plugin support not enabled by default with TCI (since 9.2)
+''
+
+While the TCG interpreter can interpret the TCG ops used by plugins it
+is going to be so much slower it wouldn't make sense for any serious
+instrumentation. Due to implementation differences there will also be
+anomalies in things like memory instrumentation.
+
 System emulator CPUs
 
 
diff --git a/configure b/configure
index f376fa24b3..3778b61c40 100755
--- a/configure
+++ b/configure
@@ -629,6 +629,9 @@ meson_option_parse() {
 exit 1
   fi
 }
+has_meson_option() {
+test "${meson_options#*"$1"}" != "$meson_options"
+}
 
 meson_add_machine_file() {
   if test "$cross_compile" = "yes"; then
@@ -1048,8 +1051,12 @@ if test "$static" = "yes" ; then
   plugins="no"
 fi
 if test "$plugins" != "no" && test $host_bits -eq 64; then
-  plugins=yes
-  subdirs="$subdirs contrib/plugins"
+if has_meson_option "-Dtcg_interpreter=true"; then
+plugins="no"
+else
+plugins=yes
+subdirs="$subdirs contrib/plugins"
+fi
 fi
 
 cat > $TMPC << EOF
-- 
2.39.5

[PATCH v2 06/18] tests/tcg: add mechanism to run specific tests with plugins

[Alex Bennée]

From: Pierrick Bouvier 

Only multiarch tests are run with plugins, and we want to be able to run
per-arch test with plugins too.

Tested-by: Xingtao Yao 
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240724194708.1843704-4-pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée 
---
 tests/tcg/Makefile.target | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 452a2cde65..c5b1c7a786 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -152,10 +152,11 @@ PLUGINS=$(patsubst %.c, lib%.so, $(notdir $(wildcard 
$(PLUGIN_SRC)/*.c)))
 # only expand MULTIARCH_TESTS which are common on most of our targets
 # to avoid an exponential explosion as new tests are added. We also
 # add some special helpers the run-plugin- rules can use below.
+# In more, extra tests can be added using ADDITIONAL_PLUGINS_TESTS variable.
 
 ifneq ($(MULTIARCH_TESTS),)
 $(foreach p,$(PLUGINS), \
-   $(foreach t,$(MULTIARCH_TESTS),\
+   $(foreach t,$(MULTIARCH_TESTS) $(ADDITIONAL_PLUGINS_TESTS),\
$(eval run-plugin-$(t)-with-$(p): $t $p) \
$(eval RUN_TESTS+=run-plugin-$(t)-with-$(p
 endif # MULTIARCH_TESTS
-- 
2.39.5

[PATCH v2 04/18] plugins: save value during memory accesses

[Alex Bennée]

From: Pierrick Bouvier 

Different code paths handle memory accesses:
- tcg generated code
- load/store helpers
- atomic helpers

This value is saved in cpu->neg.plugin_mem_value_{high,low}. Values are
written only for accessed word size (upper bits are not set).

Atomic operations are doing read/write at the same time, so we generate
two memory callbacks instead of one, to allow plugins to access distinct
values.

For now, we can have access only up to 128 bits, thus split this in two
64 bits words. When QEMU will support wider operations, we'll be able to
reconsider this.

Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240724194708.1843704-2-pierrick.bouv...@linaro.org>
Signed-off-by: Alex Bennée 
---
 accel/tcg/atomic_template.h   | 66 ++-
 include/hw/core/cpu.h |  4 +++
 include/qemu/plugin.h |  4 +++
 plugins/core.c|  6 
 tcg/tcg-op-ldst.c | 66 +++
 accel/tcg/atomic_common.c.inc | 13 ++-
 accel/tcg/ldst_common.c.inc   | 38 
 7 files changed, 167 insertions(+), 30 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 1dc2151daf..89593b2502 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -53,6 +53,14 @@
 # error unsupported data size
 #endif
 
+#if DATA_SIZE == 16
+# define VALUE_LOW(val) int128_getlo(val)
+# define VALUE_HIGH(val) int128_gethi(val)
+#else
+# define VALUE_LOW(val) val
+# define VALUE_HIGH(val) 0
+#endif
+
 #if DATA_SIZE >= 4
 # define ABI_TYPE  DATA_TYPE
 #else
@@ -83,7 +91,12 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, abi_ptr 
addr,
 ret = qatomic_cmpxchg__nocheck(haddr, cmpv, newv);
 #endif
 ATOMIC_MMU_CLEANUP;
-atomic_trace_rmw_post(env, addr, oi);
+atomic_trace_rmw_post(env, addr,
+  VALUE_LOW(ret),
+  VALUE_HIGH(ret),
+  VALUE_LOW(newv),
+  VALUE_HIGH(newv),
+  oi);
 return ret;
 }
 
@@ -97,7 +110,12 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, abi_ptr addr, 
ABI_TYPE val,
 
 ret = qatomic_xchg__nocheck(haddr, val);
 ATOMIC_MMU_CLEANUP;
-atomic_trace_rmw_post(env, addr, oi);
+atomic_trace_rmw_post(env, addr,
+  VALUE_LOW(ret),
+  VALUE_HIGH(ret),
+  VALUE_LOW(val),
+  VALUE_HIGH(val),
+  oi);
 return ret;
 }
 
@@ -109,7 +127,12 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, abi_ptr addr,   
 \
 haddr = atomic_mmu_lookup(env_cpu(env), addr, oi, DATA_SIZE, retaddr);   \
 ret = qatomic_##X(haddr, val);  \
 ATOMIC_MMU_CLEANUP; \
-atomic_trace_rmw_post(env, addr, oi);   \
+atomic_trace_rmw_post(env, addr,\
+  VALUE_LOW(ret),   \
+  VALUE_HIGH(ret),  \
+  VALUE_LOW(val),   \
+  VALUE_HIGH(val),  \
+  oi);  \
 return ret; \
 }
 
@@ -145,7 +168,12 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, abi_ptr addr,   
 \
 cmp = qatomic_cmpxchg__nocheck(haddr, old, new);\
 } while (cmp != old);   \
 ATOMIC_MMU_CLEANUP; \
-atomic_trace_rmw_post(env, addr, oi);   \
+atomic_trace_rmw_post(env, addr,\
+  VALUE_LOW(old),   \
+  VALUE_HIGH(old),  \
+  VALUE_LOW(xval),  \
+  VALUE_HIGH(xval), \
+  oi);  \
 return RET; \
 }
 
@@ -188,7 +216,12 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, abi_ptr 
addr,
 ret = qatomic_cmpxchg__nocheck(haddr, BSWAP(cmpv), BSWAP(newv));
 #endif
 ATOMIC_MMU_CLEANUP;
-atomic_trace_rmw_post(env, addr, oi);
+atomic_trace_rmw_post(env, addr,
+  VALUE_LOW(ret),
+  VALUE_HIGH(ret),
+  VALUE_LOW(newv),
+  VALUE_HIGH(newv),
+  oi);
 return BSWAP(ret);
 }
 
@@ -202,7 +235,12 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, abi_ptr 
addr, ABI_TYPE val,
 

[PATCH v2 11/18] tests/tcg: only read/write 64 bit words on 64 bit systems

[Alex Bennée]

While the compilers will generally happily synthesise a 64 bit value
for you on 32 bit systems it doesn't exercise anything on QEMU. It
also makes it hard to accurately compare the accesses to test_data
when instrumenting.

Message-Id: <20240910140733.4007719-21-alex.ben...@linaro.org>
Reviewed-by: Pierrick Bouvier 
Signed-off-by: Alex Bennée 

---
v2
  - >= test of __SIZEOF_POINTER__
---
 tests/tcg/multiarch/system/memory.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/tests/tcg/multiarch/system/memory.c 
b/tests/tcg/multiarch/system/memory.c
index 8f2371975d..28080767b2 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -163,6 +163,7 @@ static void init_test_data_u32(int offset)
 ml_printf("done %d @ %p\n", i, ptr);
 }
 
+#if __SIZEOF_POINTER__ >= 8
 static void init_test_data_u64(int offset)
 {
 uint8_t count = 0;
@@ -187,6 +188,7 @@ static void init_test_data_u64(int offset)
 }
 ml_printf("done %d @ %p\n", i, ptr);
 }
+#endif
 
 static bool read_test_data_u16(int offset)
 {
@@ -254,6 +256,7 @@ static bool read_test_data_u32(int offset)
 return true;
 }
 
+#if __SIZEOF_POINTER__ >= 8
 static bool read_test_data_u64(int offset)
 {
 uint64_t word, *ptr = (uint64_t *)&test_data[offset];
@@ -307,11 +310,16 @@ static bool read_test_data_u64(int offset)
 ml_printf("done %d @ %p\n", i, ptr);
 return true;
 }
+#endif
 
 /* Read the test data and verify at various offsets */
-read_ufn read_ufns[] = { read_test_data_u16,
- read_test_data_u32,
- read_test_data_u64 };
+read_ufn read_ufns[] = {
+read_test_data_u16,
+read_test_data_u32,
+#if __SIZEOF_POINTER__ >= 8
+read_test_data_u64
+#endif
+};
 
 bool do_unsigned_reads(int start_off)
 {
@@ -476,10 +484,14 @@ bool do_signed_reads(bool neg_first)
 return ok;
 }
 
-init_ufn init_ufns[] = { init_test_data_u8,
- init_test_data_u16,
- init_test_data_u32,
- init_test_data_u64 };
+init_ufn init_ufns[] = {
+init_test_data_u8,
+init_test_data_u16,
+init_test_data_u32,
+#if __SIZEOF_POINTER__ >= 8
+init_test_data_u64
+#endif
+};
 
 int main(void)
 {
-- 
2.39.5

[PATCH v2 13/18] tests/tcg: add a system test to check memory instrumentation

[Alex Bennée]

At first I thought I could compile the user-mode test for system mode
however we already have a fairly comprehensive test case for system
mode in "memory" so lets use that.

As tracking every access will quickly build up with "print-access" we
add a new mode to track groups of reads and writes to regions. Because
the test_data is 16k aligned we can be sure all accesses to it are
ones we can count.

First we extend the test to report where the test_data region is. Then
we expand the pdot() function to track the total number of reads and
writes to the region. We have to add some addition pdot() calls to
take into account multiple reads/writes in the test loops.

Finally we add a python script to integrate the data from the plugin
and the output of the test and validate they both agree on the total
counts. As some boot codes clear the bss we also add a flag to add a
regions worth of writes to the expected total.

Signed-off-by: Alex Bennée 

---
v2
  - aggressively align test_data on "region size"
  - sort the regions in the final report
  - ensure alpha-softmmu uses byte access when it can
v3
  - fix thinko while iterating through the regions
  - fix the LE/BE storage of values in the mirror section
  - add --bss-cleared to script
  - clean-up some long lines in the script
---
 tests/tcg/multiarch/system/memory.c   |  50 +++--
 tests/tcg/plugins/mem.c   | 181 +-
 tests/tcg/alpha/Makefile.softmmu-target   |   2 +-
 .../multiarch/system/Makefile.softmmu-target  |   6 +
 .../system/validate-memory-counts.py  | 129 +
 tests/tcg/s390x/Makefile.softmmu-target   |   5 +
 6 files changed, 354 insertions(+), 19 deletions(-)
 create mode 100755 tests/tcg/multiarch/system/validate-memory-counts.py

diff --git a/tests/tcg/multiarch/system/memory.c 
b/tests/tcg/multiarch/system/memory.c
index 28080767b2..65a6038a24 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -14,26 +14,35 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef CHECK_UNALIGNED
 # error "Target does not specify CHECK_UNALIGNED"
 #endif
 
+uint32_t test_read_count;
+uint32_t test_write_count;
+
 #define MEM_PAGE_SIZE 4096 /* nominal 4k "pages" */
 #define TEST_SIZE (MEM_PAGE_SIZE * 4)  /* 4 pages */
 
 #define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])))
 
-__attribute__((aligned(MEM_PAGE_SIZE)))
+__attribute__((aligned(TEST_SIZE)))
 static uint8_t test_data[TEST_SIZE];
 
 typedef void (*init_ufn) (int offset);
 typedef bool (*read_ufn) (int offset);
 typedef bool (*read_sfn) (int offset, bool nf);
 
-static void pdot(int count)
+static void pdot(int count, bool write)
 {
+if (write) {
+test_write_count++;
+} else {
+test_read_count++;
+}
 if (count % 128 == 0) {
 ml_printf(".");
 }
@@ -67,7 +76,7 @@ static void init_test_data_u8(int unused_offset)
 
 for (i = 0; i < TEST_SIZE; i++) {
 *ptr++ = BYTE_NEXT(count);
-pdot(i);
+pdot(i, true);
 }
 
 ml_printf("done %d @ %p\n", i, ptr);
@@ -93,8 +102,9 @@ static void init_test_data_s8(bool neg_first)
   neg_first ? "neg first" : "pos first");
 for (i = 0; i < TEST_SIZE / 2; i++) {
 *ptr++ = get_byte(i, neg_first);
+pdot(i, true);
 *ptr++ = get_byte(i, !neg_first);
-pdot(i);
+pdot(i, true);
 }
 ml_printf("done %d @ %p\n", i * 2, ptr);
 }
@@ -116,6 +126,7 @@ static void reset_start_data(int offset)
 
 for (i = 0; i < offset; i++) {
 *ptr++ = 0;
+pdot(i, true);
 }
 
 ml_printf("done %d @ %p\n", i, ptr);
@@ -136,7 +147,7 @@ static void init_test_data_u16(int offset)
 uint16_t low = BYTE_NEXT(count), high = BYTE_NEXT(count);
 word = BYTE_SHIFT(high, 1) | BYTE_SHIFT(low, 0);
 *ptr++ = word;
-pdot(i);
+pdot(i, true);
 }
 ml_printf("done %d @ %p\n", i, ptr);
 }
@@ -158,7 +169,7 @@ static void init_test_data_u32(int offset)
 word = BYTE_SHIFT(b1, 3) | BYTE_SHIFT(b2, 2) | BYTE_SHIFT(b3, 1) |
BYTE_SHIFT(b4, 0);
 *ptr++ = word;
-pdot(i);
+pdot(i, true);
 }
 ml_printf("done %d @ %p\n", i, ptr);
 }
@@ -184,7 +195,7 @@ static void init_test_data_u64(int offset)
BYTE_SHIFT(b4, 4) | BYTE_SHIFT(b5, 3) | BYTE_SHIFT(b6, 2) |
BYTE_SHIFT(b7, 1) | BYTE_SHIFT(b8, 0);
 *ptr++ = word;
-pdot(i);
+pdot(i, true);
 }
 ml_printf("done %d @ %p\n", i, ptr);
 }
@@ -207,7 +218,7 @@ static bool read_test_data_u16(int offset)
 ml_printf("Error %d < %d\n", high, low);
 return false;
 } else {
-pdot(i);
+pdot(i, false);
 }
 
 }
@@ -249,7 +260,7 @@ static bool read_test_data_u32(int offset)
 ml_printf("Error %d, %d, %d, %d", b1, b2, b3, b4);
 return false;
 } else {
-pdot(i)

[PATCH v2 12/18] tests/tcg: ensure s390x-softmmu output redirected

[Alex Bennée]

The multiarch system tests output serial data which should be
redirected to the "output" chardev rather than echoed to the console.

Comment the use of EXTFLAGS variable while we are at it.

Signed-off-by: Alex Bennée 

---
v2
  - don't remove EXTFLAGS, add comment
---
 tests/tcg/s390x/Makefile.softmmu-target | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/s390x/Makefile.softmmu-target 
b/tests/tcg/s390x/Makefile.softmmu-target
index f60f94b090..be242ba8f1 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -1,6 +1,7 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
-QEMU_OPTS+=-action panic=exit-failure -nographic $(EXTFLAGS) -kernel
+# EXTFLAGS can be passed by the user, e.g. to override the --accel
+QEMU_OPTS+=-action panic=exit-failure -nographic -serial chardev:output 
$(EXTFLAGS) -kernel
 LINK_SCRIPT=$(S390X_SRC)/softmmu.ld
 CFLAGS+=-ggdb -O0
 LDFLAGS=-nostdlib -static
-- 
2.39.5

[PATCH v2 10/18] tests/tcg: clean up output of memory system test

[Alex Bennée]

This is useful information when debugging memory issues so lets
improve by:

  - include the ptr address for u8 fills (like the others)
  - indicate the number of operations for reads and writes
  - explicitly note when we are flushing
  - move the fill printf to after the reset

Message-Id: <20240910140733.4007719-20-alex.ben...@linaro.org>
Reviewed-by: Pierrick Bouvier 
Signed-off-by: Alex Bennée 
---
 tests/tcg/multiarch/system/memory.c | 47 ++---
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/tests/tcg/multiarch/system/memory.c 
b/tests/tcg/multiarch/system/memory.c
index 6eb2eb16f7..8f2371975d 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -63,12 +63,14 @@ static void init_test_data_u8(int unused_offset)
 int i;
 (void)(unused_offset);
 
-ml_printf("Filling test area with u8:");
+ml_printf("Filling test area with u8 (%p):", ptr);
+
 for (i = 0; i < TEST_SIZE; i++) {
 *ptr++ = BYTE_NEXT(count);
 pdot(i);
 }
-ml_printf("done\n");
+
+ml_printf("done %d @ %p\n", i, ptr);
 }
 
 /*
@@ -94,7 +96,7 @@ static void init_test_data_s8(bool neg_first)
 *ptr++ = get_byte(i, !neg_first);
 pdot(i);
 }
-ml_printf("done\n");
+ml_printf("done %d @ %p\n", i * 2, ptr);
 }
 
 /*
@@ -105,9 +107,18 @@ static void reset_start_data(int offset)
 {
 uint32_t *ptr = (uint32_t *) &test_data[0];
 int i;
+
+if (!offset) {
+return;
+}
+
+ml_printf("Flushing %d bytes from %p: ", offset, ptr);
+
 for (i = 0; i < offset; i++) {
 *ptr++ = 0;
 }
+
+ml_printf("done %d @ %p\n", i, ptr);
 }
 
 static void init_test_data_u16(int offset)
@@ -117,17 +128,17 @@ static void init_test_data_u16(int offset)
 const int max = (TEST_SIZE - offset) / sizeof(word);
 int i;
 
-ml_printf("Filling test area with u16 (offset %d, %p):", offset, ptr);
-
 reset_start_data(offset);
 
+ml_printf("Filling test area with u16 (offset %d, %p):", offset, ptr);
+
 for (i = 0; i < max; i++) {
 uint16_t low = BYTE_NEXT(count), high = BYTE_NEXT(count);
 word = BYTE_SHIFT(high, 1) | BYTE_SHIFT(low, 0);
 *ptr++ = word;
 pdot(i);
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 }
 
 static void init_test_data_u32(int offset)
@@ -137,10 +148,10 @@ static void init_test_data_u32(int offset)
 const int max = (TEST_SIZE - offset) / sizeof(word);
 int i;
 
-ml_printf("Filling test area with u32 (offset %d, %p):", offset, ptr);
-
 reset_start_data(offset);
 
+ml_printf("Filling test area with u32 (offset %d, %p):", offset, ptr);
+
 for (i = 0; i < max; i++) {
 uint32_t b4 = BYTE_NEXT(count), b3 = BYTE_NEXT(count);
 uint32_t b2 = BYTE_NEXT(count), b1 = BYTE_NEXT(count);
@@ -149,7 +160,7 @@ static void init_test_data_u32(int offset)
 *ptr++ = word;
 pdot(i);
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 }
 
 static void init_test_data_u64(int offset)
@@ -159,10 +170,10 @@ static void init_test_data_u64(int offset)
 const int max = (TEST_SIZE - offset) / sizeof(word);
 int i;
 
-ml_printf("Filling test area with u64 (offset %d, %p):", offset, ptr);
-
 reset_start_data(offset);
 
+ml_printf("Filling test area with u64 (offset %d, %p):", offset, ptr);
+
 for (i = 0; i < max; i++) {
 uint64_t b8 = BYTE_NEXT(count), b7 = BYTE_NEXT(count);
 uint64_t b6 = BYTE_NEXT(count), b5 = BYTE_NEXT(count);
@@ -174,7 +185,7 @@ static void init_test_data_u64(int offset)
 *ptr++ = word;
 pdot(i);
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 }
 
 static bool read_test_data_u16(int offset)
@@ -198,7 +209,7 @@ static bool read_test_data_u16(int offset)
 }
 
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 return true;
 }
 
@@ -239,7 +250,7 @@ static bool read_test_data_u32(int offset)
 pdot(i);
 }
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 return true;
 }
 
@@ -293,7 +304,7 @@ static bool read_test_data_u64(int offset)
 pdot(i);
 }
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 return true;
 }
 
@@ -365,7 +376,7 @@ static bool read_test_data_s8(int offset, bool neg_first)
 return false;
 }
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i * 2, ptr);
 return true;
 }
 
@@ -398,7 +409,7 @@ static bool read_test_data_s16(int offset, bool neg_first)
 return false;
 }
 }
-ml_printf("done @ %p\n", ptr);
+ml_printf("done %d @ %p\n", i, ptr);
 return true;
 }
 
@@ -431,7 +442,7 @@ static bool read_test_data_s32(int offset, bool neg_first)
 return fals

[PATCH v2 14/18] util/timer: avoid deadlock when shutting down

[Alex Bennée]

When we shut down a guest we disable the timers. However this can
cause deadlock if the guest has queued some async work that is trying
to advance system time and spins forever trying to wind time forward.
Pay attention to the return code and bail early if we can't wind time
forward.

Signed-off-by: Alex Bennée 
Reported-by: Elisha Hollander 
---
 util/qemu-timer.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 213114be68..6b1533bc2a 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -685,10 +685,17 @@ int64_t qemu_clock_advance_virtual_time(int64_t dest)
 {
 int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 AioContext *aio_context;
+int64_t deadline;
+
 aio_context = qemu_get_aio_context();
-while (clock < dest) {
-int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
+
+deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
   QEMU_TIMER_ATTR_ALL);
+/*
+ * A deadline of < 0 indicates this timer is not enabled, so we
+ * won't get far trying to run it forward.
+ */
+while (deadline >= 0 && clock < dest) {
 int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
 
 qemu_virtual_clock_set_ns(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 
warp);
@@ -696,6 +703,9 @@ int64_t qemu_clock_advance_virtual_time(int64_t dest)
 qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
 timerlist_run_timers(aio_context->tlg.tl[QEMU_CLOCK_VIRTUAL]);
 clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
+  QEMU_TIMER_ATTR_ALL);
 }
 qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
 
-- 
2.39.5

[PATCH v2 09/18] tests/tcg/multiarch: add test for plugin memory access

[Alex Bennée]

From: Pierrick Bouvier 

Add an explicit test to check expected memory values are read/written.
8,16,32 load/store are tested for all arch.
64,128 load/store are tested for aarch64/x64.
atomic operations (8,16,32,64) are tested for x64 only.

By default, atomic accesses are non atomic if a single cpu is running,
so we force creation of a second one by creating a new thread first.

load/store helpers code path can't be triggered easily in user mode (no
softmmu), so we can't test it here.

Output of test-plugin-mem-access.c is the list of expected patterns in
plugin output. By reading stdout, we can compare to plugins output and
have a multiarch test.

Can be run with:
make -C build/tests/tcg/$ARCH-linux-user 
run-plugin-test-plugin-mem-access-with-libmem.so

Tested-by: Xingtao Yao 
Signed-off-by: Pierrick Bouvier 
Message-Id: <20240910172033.1427812-7-pierrick.bouv...@linaro.org>
---
 tests/tcg/multiarch/test-plugin-mem-access.c | 177 +++
 tests/tcg/alpha/Makefile.target  |   3 +
 tests/tcg/multiarch/Makefile.target  |  11 ++
 tests/tcg/multiarch/check-plugin-output.sh   |  36 
 tests/tcg/ppc64/Makefile.target  |   5 +
 5 files changed, 232 insertions(+)
 create mode 100644 tests/tcg/multiarch/test-plugin-mem-access.c
 create mode 100755 tests/tcg/multiarch/check-plugin-output.sh

diff --git a/tests/tcg/multiarch/test-plugin-mem-access.c 
b/tests/tcg/multiarch/test-plugin-mem-access.c
new file mode 100644
index 00..057b9aac9f
--- /dev/null
+++ b/tests/tcg/multiarch/test-plugin-mem-access.c
@@ -0,0 +1,177 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Check if we detect all memory accesses expected using plugin API.
+ * Used in conjunction with ./check-plugin-mem-access.sh check script.
+ * Output of this program is the list of patterns expected in plugin output.
+ *
+ * 8,16,32 load/store are tested for all arch.
+ * 64,128 load/store are tested for aarch64/x64.
+ * atomic operations (8,16,32,64) are tested for x64 only.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#if defined(__x86_64__)
+#include 
+#elif defined(__aarch64__)
+#include 
+#endif /* __x86_64__ */
+
+static void *data;
+
+/* ,store_u8,.*,8,store,0xf1 */
+#define PRINT_EXPECTED(function, type, value, action) \
+do {  \
+printf(",%s,.*,%d,%s,%s\n",   \
+   #function, (int) sizeof(type) * 8, action, value); \
+} \
+while (0)
+
+#define DEFINE_STORE(name, type, value)  \
+ \
+static void print_expected_store_##name(void)\
+{\
+PRINT_EXPECTED(store_##name, type, #value, "store"); \
+}\
+ \
+static void store_##name(void)   \
+{\
+*((type *)data) = value; \
+print_expected_store_##name();   \
+}
+
+#define DEFINE_ATOMIC_OP(name, type, value)\
+   \
+static void print_expected_atomic_op_##name(void)  \
+{  \
+PRINT_EXPECTED(atomic_op_##name, type, "0x0*42", "load");  \
+PRINT_EXPECTED(atomic_op_##name, type, #value, "store");   \
+}  \
+   \
+static void atomic_op_##name(void) \
+{  \
+*((type *)data) = 0x42;\
+__sync_val_compare_and_swap((type *)data, 0x42, value);\
+print_expected_atomic_op_##name(); \
+}
+
+#define DEFINE_LOAD(name, type, value)  \
+\
+static void print_expected_load_##name(void)\
+{   \
+PRINT_EXPECTED(load_##name, type, #value, "load");  \
+}   \
+\
+static void load_##name(void)   \
+{   \
+\
+/* volatile forces load to be generated. */ \
+volatile type src = *((type *) data);   \
+volatile type dest = src;   \
+(void)src, (void)dest;  \
+print_expected_load_##na

[PATCH v2 18/18] contrib/plugins: avoid hanging program

[Alex Bennée]

Although we asks for instructions per second we work in quanta and
that cannot be 0. Fail to load the plugin instead and report the
minimum IPS we can handle.

Signed-off-by: Alex Bennée 
Reported-by: Elisha Hollander 
Reviewed-by: Richard Henderson 
---
 contrib/plugins/ips.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
index 29fa556d0f..6f078689dc 100644
--- a/contrib/plugins/ips.c
+++ b/contrib/plugins/ips.c
@@ -152,6 +152,11 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
 max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
 
+if (max_insn_per_quantum == 0) {
+fprintf(stderr, "minimum of %d instructions per second needed\n", 
NUM_TIME_UPDATE_PER_SEC);
+return -1;
+}
+
 time_handle = qemu_plugin_request_time_control();
 g_assert(time_handle);
 
-- 
2.39.5

[PATCH v2 15/18] contrib/plugins: Add a plugin to generate basic block vectors

[Alex Bennée]

From: Akihiko Odaki 

SimPoint is a widely used tool to find the ideal microarchitecture
simulation points so Valgrind[2] and Pin[3] support generating basic
block vectors for use with them. Let's add a corresponding plugin to
QEMU too.

Note that this plugin has a different goal with tests/plugin/bb.c.

This plugin creates a vector for each constant interval instead of
counting the execution of basic blocks for the entire run and able to
describe the change of execution behavior. Its output is also
syntactically simple and better suited for parsing, while the output of
tests/plugin/bb.c is more human-readable.

[1] https://cseweb.ucsd.edu/~calder/simpoint/
[2] https://valgrind.org/docs/manual/bbv-manual.html
[3] 
https://www.intel.com/content/www/us/en/developer/articles/tool/pin-a-dynamic-binary-instrumentation-tool.html

Signed-off-by: Yotaro Nada 
Signed-off-by: Akihiko Odaki 
Reviewed-by: Pierrick Bouvier 
Message-Id: <20240816-bb-v3-1-b9aa4a5c7...@daynix.com>
Signed-off-by: Alex Bennée 
---
 docs/about/emulation.rst |  30 
 contrib/plugins/bbv.c| 158 +++
 contrib/plugins/Makefile |   1 +
 3 files changed, 189 insertions(+)
 create mode 100644 contrib/plugins/bbv.c

diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index eea1261baa..a4470127c9 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -272,6 +272,36 @@ Behaviour can be tweaked with the following arguments:
   * - idle=true|false
 - Dump the current execution stats whenever the guest vCPU idles
 
+Basic Block Vectors
+...
+
+``contrib/plugins/bbv.c``
+
+The bbv plugin allows you to generate basic block vectors for use with the
+`SimPoint `__ analysis tool.
+
+.. list-table:: Basic block vectors arguments
+  :widths: 20 80
+  :header-rows: 1
+
+  * - Option
+- Description
+  * - interval=N
+- The interval to generate a basic block vector specified by the number of
+  instructions (Default: N = 1)
+  * - outfile=PATH
+- The path to output files.
+  It will be suffixed with ``.N.bb`` where ``N`` is a vCPU index.
+
+Example::
+
+  $ qemu-aarch64 \
+-plugin contrib/plugins/libbbv.so,interval=100,outfile=sha1 \
+tests/tcg/aarch64-linux-user/sha1
+  SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
+  $ du sha1.0.bb
+  23128   sha1.0.bb
+
 Instruction
 ...
 
diff --git a/contrib/plugins/bbv.c b/contrib/plugins/bbv.c
new file mode 100644
index 00..a5256517dd
--- /dev/null
+++ b/contrib/plugins/bbv.c
@@ -0,0 +1,158 @@
+/*
+ * Generate basic block vectors for use with the SimPoint analysis tool.
+ * SimPoint: https://cseweb.ucsd.edu/~calder/simpoint/
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include 
+#include 
+
+#include 
+
+typedef struct Bb {
+uint64_t vaddr;
+struct qemu_plugin_scoreboard *count;
+unsigned int index;
+} Bb;
+
+typedef struct Vcpu {
+uint64_t count;
+FILE *file;
+} Vcpu;
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+static GHashTable *bbs;
+static GRWLock bbs_lock;
+static char *filename;
+static struct qemu_plugin_scoreboard *vcpus;
+static uint64_t interval = 1;
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+for (int i = 0; i < qemu_plugin_num_vcpus(); i++) {
+fclose(((Vcpu *)qemu_plugin_scoreboard_find(vcpus, i))->file);
+}
+
+g_hash_table_unref(bbs);
+g_free(filename);
+qemu_plugin_scoreboard_free(vcpus);
+}
+
+static void free_bb(void *data)
+{
+qemu_plugin_scoreboard_free(((Bb *)data)->count);
+g_free(data);
+}
+
+static qemu_plugin_u64 count_u64(void)
+{
+return qemu_plugin_scoreboard_u64_in_struct(vcpus, Vcpu, count);
+}
+
+static qemu_plugin_u64 bb_count_u64(Bb *bb)
+{
+return qemu_plugin_scoreboard_u64(bb->count);
+}
+
+static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+g_autofree gchar *vcpu_filename = NULL;
+Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index);
+
+vcpu_filename = g_strdup_printf("%s.%u.bb", filename, vcpu_index);
+vcpu->file = fopen(vcpu_filename, "w");
+}
+
+static void vcpu_interval_exec(unsigned int vcpu_index, void *udata)
+{
+Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index);
+GHashTableIter iter;
+void *value;
+
+if (!vcpu->file) {
+return;
+}
+
+vcpu->count -= interval;
+
+fputc('T', vcpu->file);
+
+g_rw_lock_reader_lock(&bbs_lock);
+g_hash_table_iter_init(&iter, bbs);
+
+while (g_hash_table_iter_next(&iter, NULL, &value)) {
+Bb *bb = value;
+uint64_t bb_count = qemu_plugin_u64_get(bb_count_u64(bb), vcpu_index);
+
+if (!bb_count) {
+continue;
+}
+
+fprintf(vcpu->file, ":%u:%" PRIu64 " ", bb->index, bb_count);
+qemu_plugin_u64_set(bb_count_u64(bb), vcpu_index, 0);
+}
+
+g_rw_lock_reader_unlock(&bbs_lock);
+fputc

[PATCH v2 17/18] plugins: add option to dump write argument to syscall plugin

[Alex Bennée]

From: Rowan Hart 

Signed-off-by: Rowan Hart 
Reviewed-by: Pierrick Bouvier 
Tested-by: Pierrick Bouvier 
Message-Id: <20240827215329.248434-3-rowanbh...@gmail.com>
[AJB: tweak fmt string for vaddr]
Signed-off-by: Alex Bennée 

---
vAJB
  - tweak fmt string for PRIu64
v2
  - add static to arch_syscall_info
---
 docs/about/emulation.rst|  14 -
 tests/tcg/plugins/syscall.c | 117 
 2 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index a4470127c9..23e4949049 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -418,6 +418,19 @@ run::
   160  1  0
   135  1  0
 
+Behaviour can be tweaked with the following arguments:
+
+.. list-table:: Syscall plugin arguments
+  :widths: 20 80
+  :header-rows: 1
+
+  * - Option
+- Description
+  * - print=true|false
+- Print the number of times each syscall is called
+  * - log_writes=true|false
+- Log the buffer of each write syscall in hexdump format
+
 Test inline operations
 ..
 
@@ -807,4 +820,3 @@ Other emulation features
 When running system emulation you can also enable deterministic
 execution which allows for repeatable record/replay debugging. See
 :ref:`Record/Replay` for more details.
-
diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
index 72e1a5bf90..89dc7f49b1 100644
--- a/tests/tcg/plugins/syscall.c
+++ b/tests/tcg/plugins/syscall.c
@@ -22,8 +22,57 @@ typedef struct {
 int64_t errors;
 } SyscallStats;
 
+struct SyscallInfo {
+const char *name;
+int64_t write_sysno;
+};
+
+static const struct SyscallInfo arch_syscall_info[] = {
+{ "aarch64", 64 },
+{ "aarch64_be", 64 },
+{ "alpha", 4 },
+{ "arm", 4 },
+{ "armeb", 4 },
+{ "avr", -1 },
+{ "cris", -1 },
+{ "hexagon", 64 },
+{ "hppa", -1 },
+{ "i386", 4 },
+{ "loongarch64", -1 },
+{ "m68k", 4 },
+{ "microblaze", 4 },
+{ "microblazeel", 4 },
+{ "mips", 1 },
+{ "mips64", 1 },
+{ "mips64el", 1 },
+{ "mipsel", 1 },
+{ "mipsn32", 1 },
+{ "mipsn32el", 1 },
+{ "or1k", -1 },
+{ "ppc", 4 },
+{ "ppc64", 4 },
+{ "ppc64le", 4 },
+{ "riscv32", 64 },
+{ "riscv64", 64 },
+{ "rx", -1 },
+{ "s390x", -1 },
+{ "sh4", -1 },
+{ "sh4eb", -1 },
+{ "sparc", 4 },
+{ "sparc32plus", 4 },
+{ "sparc64", 4 },
+{ "tricore", -1 },
+{ "x86_64", 1 },
+{ "xtensa", 13 },
+{ "xtensaeb", 13 },
+{ NULL, -1 },
+};
+
 static GMutex lock;
 static GHashTable *statistics;
+static GByteArray *memory_buffer;
+static bool do_log_writes;
+static int64_t write_sysno = -1;
 
 static SyscallStats *get_or_create_entry(int64_t num)
 {
@@ -39,6 +88,44 @@ static SyscallStats *get_or_create_entry(int64_t num)
 return entry;
 }
 
+/*
+ * Hex-dump a GByteArray to the QEMU plugin output in the format:
+ * 61 63 63 65 6c 09 09 20 20 20 66 70 75 09 09 09  | accel.fpu...
+ * 20 6d 6f 64 75 6c 65 2d 63 6f 6d 6d 6f 6e 2e 63  | .module-common.c
+ */
+static void hexdump(const GByteArray *data)
+{
+g_autoptr(GString) out = g_string_new("");
+
+for (guint index = 0; index < data->len; index += 16) {
+for (guint col = 0; col < 16; col++) {
+if (index + col < data->len) {
+g_string_append_printf(out, "%02x ", data->data[index + col]);
+} else {
+g_string_append(out, "   ");
+}
+}
+
+g_string_append(out, " | ");
+
+for (guint col = 0; col < 16; col++) {
+if (index + col >= data->len) {
+break;
+}
+
+if (g_ascii_isgraph(data->data[index + col])) {
+g_string_append_printf(out, "%c", data->data[index + col]);
+} else {
+g_string_append(out, ".");
+}
+}
+
+g_string_append(out, "\n");
+}
+
+qemu_plugin_outs(out->str);
+}
+
 static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
  int64_t num, uint64_t a1, uint64_t a2,
  uint64_t a3, uint64_t a4, uint64_t a5,
@@ -54,6 +141,14 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int 
vcpu_index,
 g_autofree gchar *out = g_strdup_printf("syscall #%" PRIi64 "\n", num);
 qemu_plugin_outs(out);
 }
+
+if (do_log_writes && num == write_sysno) {
+if (qemu_plugin_read_memory_vaddr(a2, memory_buffer, a3)) {
+hexdump(memory_buffer);
+} else {
+fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
+}
+}
 }
 
 static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
@@ -127,6 +222,10 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_print)) {
 fprintf(stderr, "boolean argu

[PATCH v2 16/18] plugins: add plugin API to read guest memory

[Alex Bennée]

From: Rowan Hart 

Signed-off-by: Rowan Hart 
Reviewed-by: Pierrick Bouvier 
Message-Id: <20240827215329.248434-2-rowanbh...@gmail.com>
[AJB: tweaked cpu_memory_rw_debug call]
Signed-off-by: Alex Bennée 

---
vAJB:
  - explicit bool for cpu_memory_rw_debug
v2
  - fix alignment
---
 include/qemu/qemu-plugin.h   | 32 +++-
 plugins/api.c| 20 
 plugins/qemu-plugins.symbols |  1 +
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 649ce89815..622c9a0232 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -57,11 +57,19 @@ typedef uint64_t qemu_plugin_id_t;
  * - Remove qemu_plugin_register_vcpu_{tb, insn, mem}_exec_inline.
  *   Those functions are replaced by *_per_vcpu variants, which guarantee
  *   thread-safety for operations.
+ *
+ * version 3:
+ * - modified arguments and return value of qemu_plugin_insn_data to copy
+ *   the data into a user-provided buffer instead of returning a pointer
+ *   to the data.
+ *
+ * version 4:
+ * - added qemu_plugin_read_memory_vaddr
  */
 
 extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
 
-#define QEMU_PLUGIN_VERSION 3
+#define QEMU_PLUGIN_VERSION 4
 
 /**
  * struct qemu_info_t - system information for plugins
@@ -884,6 +892,28 @@ typedef struct {
 QEMU_PLUGIN_API
 GArray *qemu_plugin_get_registers(void);
 
+/**
+ * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
+ *
+ * @addr: A virtual address to read from
+ * @data: A byte array to store data into
+ * @len: The number of bytes to read, starting from @addr
+ *
+ * @len bytes of data is read starting at @addr and stored into @data. If @data
+ * is not large enough to hold @len bytes, it will be expanded to the necessary
+ * size, reallocating if necessary. @len must be greater than 0.
+ *
+ * This function does not ensure writes are flushed prior to reading, so
+ * callers should take care when calling this function in plugin callbacks to
+ * avoid attempting to read data which may not yet be written and should use
+ * the memory callback API instead.
+ *
+ * Returns true on success and false on failure.
+ */
+QEMU_PLUGIN_API
+bool qemu_plugin_read_memory_vaddr(uint64_t addr,
+   GByteArray *data, size_t len);
+
 /**
  * qemu_plugin_read_register() - read register for current vCPU
  *
diff --git a/plugins/api.c b/plugins/api.c
index 3316d4a04d..24ea64e2de 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -560,6 +560,26 @@ GArray *qemu_plugin_get_registers(void)
 return create_register_handles(regs);
 }
 
+bool qemu_plugin_read_memory_vaddr(vaddr addr, GByteArray *data, size_t len)
+{
+g_assert(current_cpu);
+
+if (len == 0) {
+return false;
+}
+
+g_byte_array_set_size(data, len);
+
+int result = cpu_memory_rw_debug(current_cpu, addr, data->data,
+ data->len, false);
+
+if (result < 0) {
+return false;
+}
+
+return true;
+}
+
 int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray 
*buf)
 {
 g_assert(current_cpu);
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index eed9d8abd9..032661f9ea 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -21,6 +21,7 @@
   qemu_plugin_num_vcpus;
   qemu_plugin_outs;
   qemu_plugin_path_to_binary;
+  qemu_plugin_read_memory_vaddr;
   qemu_plugin_read_register;
   qemu_plugin_register_atexit_cb;
   qemu_plugin_register_flush_cb;
-- 
2.39.5

Re: [PATCH] softmmu: Support concurrent bounce buffers

[Peter Maydell]

On Mon, 16 Sept 2024 at 08:35, Mattias Nissler  wrote:
>
> On Fri, Sep 13, 2024 at 6:47 PM Peter Maydell  
> wrote:
> >
> > On Fri, 13 Sept 2024 at 16:55, Peter Xu  wrote:
> > >
> > > On Thu, Sep 12, 2024 at 03:27:55PM +0100, Peter Maydell wrote:
> > > > Coverity is pretty unhappy about this trick, because it isn't able
> > > > to recognise that we can figure out the address of 'bounce'
> > > > from the address of 'bounce->buffer' and free it in the
> > > > address_space_unmap() code, so it thinks that every use
> > > > of address_space_map(), pci_dma_map(), etc, is a memory leak.
> > > > We can mark all those as false positives, of course, but it got
> > > > me wondering whether maybe we should have this function return
> > > > a struct that has all the information address_space_unmap()
> > > > needs rather than relying on it being able to figure it out
> > > > from the host memory pointer...
> > >
> > > Indeed that sounds like a viable option.  Looks like we don't have a lot 
> > > of
> > > address_space_map() users.
> >
> > There's quite a few wrappers of it too, so it's a little hard to count.
> > We might want to avoid the memory allocation in the common case
> > by having the caller pass in an ASMapInfo struct to be filled
> > in rather than having address_space_map() allocate-and-return one.
>
> Hm, this would work, but not only does it complicate the code
> consuming address_space_map, but it also increases memory footprint (a
> pointer being replaced by a struct of sizeof(BounceBuffer) if done
> naively), plus there's an additional pointer indirection (I'm doubtful
> whether this can be optimized away by the compiler). I haven't done
> any measurements of these effects, so can't say anything definitive,
> but this seems pretty costly just to appease coverity...
>
> Is there no way to inform coverity that a resource pointer is being
> transmuted into a handle, so it can track that instead? Given that
> pointer tricks like this and container_of usage is quite frequent, I
> would expect coverity to have a better strategy to handle these rather
> than suppressing false positive leak reports?

It's not purely that I want to appease Coverity. I also
think for human readers that the current trick with passing
back a pointer into host memory and relying on being able to
get back to either the MR or to the bounce-buffer struct
from that is pretty tricky. Would we have designed it that
way if we weren't starting with the pre-existing address_space_map()
function signature?

thanks
-- PMM

Re: [PATCH] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

On Mon, Sep 16, 2024 at 11:05 AM Peter Maydell  wrote:
>
> On Mon, 16 Sept 2024 at 08:35, Mattias Nissler  wrote:
> >
> > On Fri, Sep 13, 2024 at 6:47 PM Peter Maydell  
> > wrote:
> > >
> > > On Fri, 13 Sept 2024 at 16:55, Peter Xu  wrote:
> > > >
> > > > On Thu, Sep 12, 2024 at 03:27:55PM +0100, Peter Maydell wrote:
> > > > > Coverity is pretty unhappy about this trick, because it isn't able
> > > > > to recognise that we can figure out the address of 'bounce'
> > > > > from the address of 'bounce->buffer' and free it in the
> > > > > address_space_unmap() code, so it thinks that every use
> > > > > of address_space_map(), pci_dma_map(), etc, is a memory leak.
> > > > > We can mark all those as false positives, of course, but it got
> > > > > me wondering whether maybe we should have this function return
> > > > > a struct that has all the information address_space_unmap()
> > > > > needs rather than relying on it being able to figure it out
> > > > > from the host memory pointer...
> > > >
> > > > Indeed that sounds like a viable option.  Looks like we don't have a 
> > > > lot of
> > > > address_space_map() users.
> > >
> > > There's quite a few wrappers of it too, so it's a little hard to count.
> > > We might want to avoid the memory allocation in the common case
> > > by having the caller pass in an ASMapInfo struct to be filled
> > > in rather than having address_space_map() allocate-and-return one.
> >
> > Hm, this would work, but not only does it complicate the code
> > consuming address_space_map, but it also increases memory footprint (a
> > pointer being replaced by a struct of sizeof(BounceBuffer) if done
> > naively), plus there's an additional pointer indirection (I'm doubtful
> > whether this can be optimized away by the compiler). I haven't done
> > any measurements of these effects, so can't say anything definitive,
> > but this seems pretty costly just to appease coverity...
> >
> > Is there no way to inform coverity that a resource pointer is being
> > transmuted into a handle, so it can track that instead? Given that
> > pointer tricks like this and container_of usage is quite frequent, I
> > would expect coverity to have a better strategy to handle these rather
> > than suppressing false positive leak reports?
>
> It's not purely that I want to appease Coverity. I also
> think for human readers that the current trick with passing
> back a pointer into host memory and relying on being able to
> get back to either the MR or to the bounce-buffer struct
> from that is pretty tricky. Would we have designed it that
> way if we weren't starting with the pre-existing address_space_map()
> function signature?

Identifying a mapping by its address seems pretty natural to me for
callers of the API, at least as long as the address is all that the
caller ever needs. The solution I implemented didn't/doesn't seem
overly complicated to me, and I am hoping it wouldn't overwhelm anyone
else who has worked with container_of (which seems to be used
liberally in qemu). But at the end of the day it's a matter of
personal taste and the background of the person writing the code, so
I'll readily admit that this is personal opinion, and if qemu does
prefer a different style then by all means we should change it and
I'll be happy to help with that.

Re: [PATCH-for-9.1 v2 0/4] hw/ssi/pnv_spi: Fixes Coverity CID 1558831

[Chalapathi V]

On 13-09-2024 19:07, Cédric Le Goater wrote:

Hello,

On 9/13/24 15:24, Chalapathi V wrote:


On 12-09-2024 22:25, Cédric Le Goater wrote:

Chalapthi,

On 8/7/24 22:28, Philippe Mathieu-Daudé wrote:

v2:
- Cover PowerNV SSI in MAINTAINERS
- Use GLib API in pnv_spi_xfer_buffer_free()
- Simplify returning early

Supersedes: <20240806134829.351703-3-chalapath...@linux.ibm.com>


I was wondering where we were on this series. I see there were comments
on the initial one that would need some response at least. Do you have
plans for a respin ?

Thanks,

C.


Hello Cedric,

Thank You so much for reminding me. I apologize for not getting back 
on this sooner. 


That's fine. We have some spare time before the QEMU 9.2 cycle
closes. I'd say ~2 months. Still, it would be good to address
these issues before adding more models (Dan's TPM device model)
relying on it.

Sure. Thank You.


I am working on the review comments from initial v1 patchset and send 
the v2 patchset ASAP.


That would be a v3 ? Since Philippe sent a v2.

Thanks,

C.
Sure. Will send v3 for the series: hw/ssi/pnv_spi: Fixes Coverity CID 
1558831 and


a separate series for resolving Coverity CID 1558827.

Thank You,

Chalapathi

Re: [PULL v3 00/60] Misc HW & UI patches for 2024-09-13

[Peter Maydell]

On Fri, 13 Sept 2024 at 21:33, Philippe Mathieu-Daudé  wrote:
>
> v3: Fixed TMP105 tests
>
> The following changes since commit 28ae3179fc52d2e4d870b635c4a412aab99759e7:
>
>   Merge tag 'pull-target-arm-20240913' of 
> https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-09-13 
> 16:14:33 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/hw-misc-20240913
>
> for you to fetch changes up to b3372e0ec818d7747963a2ec7ae04fd1a8152afd:
>
>   ui: remove break after g_assert_not_reached() (2024-09-13 20:12:16 +0200)
>
> 
> Misc HW & UI patches
>
> - Remove deprecated SH4 SHIX machine TC58128 NAND EEPROM (Phil)
> - Remove deprecated CRIS target (Phil)
> - Remove deprecated RISC-V 'any' CPU type (Phil)
> - Add fifo8_peek_buf() to correctly handle FIFO wraparound (Mark)
> - Minor cleanups in Designware PCIe, PL011 and loongson IPI models (Phil)
> - Fixes in TI TMP105 temperature (Guenter)
> - Convert Sun ESCC and ADB mouses to QemuInputHandler (Mark)
> - Prevent heap overflow in VIRTIO sound device (Volker)
> - Cleanups around g_assert_not_reached() call (Pierrick)
> - Add Clément as VT-d reviewer (Clément)
> - Prevent stuck modifier keys and unexpected text input on Windows (Volker)
> - Explicitly set SDL2 swap interval when OpenGL is enabled (Gert)
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.2
for any user-visible changes.

-- PMM

Re: [PATCH for-9.2 00/53] arm: Drop deprecated boards

[Peter Maydell]

On Tue, 3 Sept 2024 at 17:07, Peter Maydell  wrote:
>
> This patchset removes the various Arm machines which we deprecated
> for the 9.0 release and are therefore allowed to remove for the 9.2
> release:
>  akita, borzoi, cheetah, connex, mainstone, n800, n810,
>  spitz, terrier, tosa, verdex, z2
> We get to drop over 30,000 lines of unmaintained code. So it's
> a big patchset but it's almost all deletions.

Hi -- ping for review on at least patches 06, 08, 18:

>   hw/display: Remove tc6393xb device
>   hw/arm: Remove 'cheetah' machine
>   hw/display: Remove pxa2xx_lcd.c

These are all straightforward removals of either
deprecated machines or devices that are definitely
exclusively used by those machines. That would be
enough for me to get the bulk of the uncontroversial
reviewed parts of this series upstream. I can then
roll a much smaller v2 with the parts still under
discussion.

Patches 22, 26 would also be helpful but they're a
bit further down the patchstack so less critical:

>   hw/arm: Remove pxa2xx_pic
>   hw/misc: Remove cbus

thanks
-- PMM

Re: -chardev with a JSON argument (was: [PATCH] chardev: introduce 'reconnect-ms' and deprecate 'reconnect')

[Kevin Wolf]

Am 14.09.2024 um 10:42 hat Markus Armbruster geschrieben:
> Peter Krempa  writes:
> 
> > This is a little off-topic:
> >
> > So I wanted to make libvirt use the new parameter to stay ahead
> > deprecation. I've applied this patch to qemu, dumped capabilities and
> > pretty much expected a bunch of test cases in libvirt fail as they'd be
> > using a deprecated field as libvirt is supposed to validate everything.
> >
> > And the test suite passed unexpectedly. I've dug further and noticed
> > that for some reason libvirt doesn't still use JSON parameters for
> > -chardev (which is the pre-requisite for validation).
> >
> > I've also noticed that at some point I attempted to convert it over
> > witnessed by having an (unused) capability named QEMU_CAPS_CHARDEV_JSON
> > that I've introduced.
> >
> > My questions are:
> > 1) Does '-chardev' accept JSON identical to 'chardev-add' QMP command?
> 
> Sadly, no.
> 
> How badly do you want it?

I suppose even my old patches to fully QAPIfy -chardev could be revived,
which would not only add JSON support, but also make sure that
everything that works in JSON also works with keyval syntax, both ways
stay in sync in the future and that you can access non-scalar options
without JSON:

https://repo.or.cz/qemu/kevin.git/shortlog/refs/tags/qapi-alias-chardev-v4

Without QAPI aliases, they will have to contain some ugly code to do the
compatibility conversion, but whatever can be generated can also be
written manually...

Kevin

Re: [PATCH v2 12/18] tests/tcg: ensure s390x-softmmu output redirected

[Ilya Leoshkevich]

On Mon, 2024-09-16 at 09:53 +0100, Alex Bennée wrote:
> The multiarch system tests output serial data which should be
> redirected to the "output" chardev rather than echoed to the console.
> 
> Comment the use of EXTFLAGS variable while we are at it.
> 
> Signed-off-by: Alex Bennée 

Acked-by: Ilya Leoshkevich 

Re: [PATCH v4 00/14] XIVE2 changes for TIMA operations

[Cédric Le Goater]

Hello Michael,

On 9/13/24 18:16, Michael Kowal wrote:

In XIVE Gen 2 there are many operations that were not modeled and are
needed for PowerVM.  These changes are associated with the following Thread
Interrupt Management Area subjects:
  - OS context
  - Thread context
  - Pulling contexts to 'cache lines'
  - Pool targets
  - Enhaced trace data for XIVE Gen2

version 4:
- fixed rebase error in patch set 9 xive2_cam_decode().  Complete changes
   for this are in patch set 10.
- version 3 reviewed tags:
   - added 10/14


Thanks for respining.

Would you be ok to become a XIVE reviewer ? I think Frederic has moved
to other tasks within IBM and doesn't have much time anymore.

C.

Re: [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5

[Cédric Le Goater]

Hello Saif,

On 3/21/24 11:04, Saif Abrar wrote:

Hello,

This series updates the existing PHB4 model to the latest spec:
"Power Systems Host Bridge 5 (PHB5) Functional Specification Version 0.5_00".

Updates include the following:
- implemented sticky reset logic
- implemented read-only, write-only, W1C and WxC logic
- return all 1's on read to unimplemented registers
- update PCIE registers for link status, speed and width
- implement IODA PCT debug table without any functionality
- set write-mask bits for PCIE Link-Control-2 register that is read/written by 
PHB4
- update LSI Source-ID register based on small/big PHB number of interrupts

Also, a new testbench for PHB4 model is added that does XSCOM read/writes
to various registers of interest and verifies the values.



Do you have any plans to send a v2 ?


Thanks,

C.

Re: [PULL 00/47] riscv-to-apply queue

[Daniel Henrique Barboza]

On 9/16/24 3:12 AM, Thomas Huth wrote:

On 15/09/2024 21.58, Daniel Henrique Barboza wrote:

Hi Peter, Alistair,

On 9/14/24 6:15 AM, Alistair Francis wrote:

On Fri, Sep 13, 2024 at 8:37 PM Peter Maydell  wrote:


On Thu, 12 Sept 2024 at 06:30, Alistair Francis  wrote:


The following changes since commit a4eb31c678400472de0b4915b9154a7c20d8332f:

   Merge tag 'pull-testing-gdbstub-oct-100924-1' of https://gitlab.com/ 
stsquad/qemu into staging (2024-09-11 13:17:29 +0100)

are available in the Git repository at:

   https://github.com/alistair23/qemu.git tags/pull-riscv-to- apply-20240912-1

for you to fetch changes up to 90d5d3c1115399d8e27621efd69dfa74a35a4932:

   hw/intc: riscv-imsic: Fix interrupt state updates. (2024-09-12 15:05:10 
+1000)


RISC-V PR for 9.2

* Add a property to set vl to ceil(AVL/2)
* Enable numamem testing for RISC-V
* Consider MISA bit choice in implied rule
* Fix the za64rs priv spec requirements
* Enable Bit Manip for OpenTitan Ibex CPU
* Fix the group bit setting of AIA with KVM
* Stop timer with infinite timecmp
* Add 'fcsr' register to QEMU log as a part of F extension
* Fix riscv64 build on musl libc
* Add preliminary textra trigger CSR functions
* RISC-V IOMMU support
* RISC-V bsd-user support
* Respect firmware ELF entry point
* Add Svvptc extension support
* Fix masking of rv32 physical address
* Fix linking problem with semihosting disabled
* Fix IMSIC interrupt state updates


Hi; this fails to build on FreeBSD:

https://gitlab.com/qemu-project/qemu/-/jobs/7817823771


Is this one of those jobs that are only available when running the main 
pipeline? I don't
have this x86-freebsd runner when triggering the gitlab pipeline. I ended up 
installing a
FreeBSD VM and using it to reproduce the problem.

Would be nice to have access to a FreeBSD runner as a regular user, even if 
just for x86_64,
to help detect these build problems before sending a PR.


You can enable this job for your pipelines, too, see 
.gitlab-ci.d/cirrus/README.rst for information how to configure it.


Nice! Thanks!



If you have a Linux host with KVM, you could alternatively also use "make 
vm-build-freebsd" on your local machine instead.



Way easier than what I did :D next time I'll grep the docs.


Daniel



  Thomas

Re: [PATCH v9 01/12] acpi/ghes: add a firmware file with HEST address

[Igor Mammedov]

On Sat, 14 Sep 2024 07:33:14 +0200
Mauro Carvalho Chehab  wrote:

> Hi Igor,
> 
> Em Fri, 13 Sep 2024 15:25:18 +0200
> Igor Mammedov  escreveu:
> 
> > > > in addition to this, it needs a patch on top to make sure
> > > > that we migrate hest_addr_le.
> > > > See a08a64627b6b 'ACPI: Record the Generic Error Status Block address'
> > > > and fixes on top of that for an example.  
> > > 
> > > Hmm... If I understood such change well, vmstate_ghes_state() will
> > > use this structure as basis to do migration:
> > > 
> > >   /* ghes.h */
> > >   typedef struct AcpiGhesState {
> > >   uint64_t hest_addr_le;
> > >   uint64_t ghes_addr_le;
> > >   bool present; /* True if GHES is present at all on this board */
> > >   } AcpiGhesState;
> > > 
> > >   /* generic_event_device.c */
> > >   static const VMStateDescription vmstate_ghes_state = {
> > >   .name = "acpi-ged/ghes",
> > >   .version_id = 1,
> > >   .minimum_version_id = 1,
> > >   .needed = ghes_needed,
> > >   .fields  = (VMStateField[]) {
> > >   VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > >  vmstate_ghes_state, AcpiGhesState),
> > >   VMSTATE_END_OF_LIST()
> > >   }
> > >   };
> > 
> > current code looks like that:
> > 
> >  
> > static const VMStateDescription vmstate_ghes = {
> >  
> > .name = "acpi-ghes",
> >  
> > .version_id = 1,
> >  
> > .minimum_version_id = 1,
> >  
> > .fields = (const VMStateField[]) {  
> >  
> > VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),   <<===
> >  
> > VMSTATE_END_OF_LIST()   
> >  
> > },  
> >  
> > };  
> >  
> > 
> >  
> > static bool ghes_needed(void *opaque)   
> >  
> > {   
> >  
> > AcpiGedState *s = opaque;   
> >  
> > return s->ghes_state.ghes_addr_le;  
> >  
> > }   
> >  
> > 
> >  
> > static const VMStateDescription vmstate_ghes_state = {  
> >  
> > .name = "acpi-ged/ghes",
> >  
> > .version_id = 1,
> >  
> > .minimum_version_id = 1,
> >  
> > .needed = ghes_needed,  
> >  
> > .fields = (const VMStateField[]) {  
> >  
> > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1, 
> >  
> >vmstate_ghes, AcpiGhesState),
> >  
> > VMSTATE_END_OF_LIST()   
> >  
> > }   
> >  
> > };  
> > 
> > where 
> > VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > explicitly defines field(s) within structure to be sent over wire.
> > 
> > we need to add a conditional field for ghes_addr_le
> > which will be sent only with new machine types, but not with old ones
> > to avoid migration breakage.
> > 
> > I don't know much about migration, but maybe we can get away with
> > similar condition as in ghes_needed(), or enabling QMP error injection
> > based on machine type version.
> > 
> > Or maybe adding a CLI option to enable QMP error injection in which
> > case the explicit option would serve as a trigger enable QMP command and
> > to migrate hest_addr_le.
> > It might be even better this way, since machine wouldn't need to
> > carry extra error source that will be used only for testing
> > and practically never in production VMs (aka reduced attack surface).
> > 
> > You can easily test it locally:
> >   new-qemu: with your patches
> >   old-qemu: qemu-9.1
> > 
> > and then try to do forth & back migration for following cases:
> >   1. (ping-pong case with OLD firmware/ACPI tables)
> >  start old-qemu with 9.1 machine type ->
> >migrate to file ->
> >start new-qemu with 9.1 machine type -> restore f

Re: [PATCH] hw/virtio/Kconfig: Include vhost-user-scmi only on arm targets

[Milan Zamazal]

Thomas Huth  writes:

> The System Control and Management Interface is specific to arm
> machines, so don't include this device in non-arm targets.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Milan Zamazal 

Thank you,
Milan

> ---
>  hw/virtio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
> index aa63ff7fd4..bea5be4d4a 100644
> --- a/hw/virtio/Kconfig
> +++ b/hw/virtio/Kconfig
> @@ -109,4 +109,4 @@ config VHOST_USER_SND
>  config VHOST_USER_SCMI
>  bool
>  default y
> -depends on VIRTIO && VHOST_USER
> +depends on VIRTIO && VHOST_USER && ARM

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Mark Cave-Ayland]

On 16/09/2024 09:23, Mattias Nissler wrote:


Thanks for the report, and my apologies for the breakage.

On Fri, Sep 13, 2024 at 4:47 PM Peter Xu  wrote:


On Fri, Sep 13, 2024 at 04:35:32PM +0200, Cédric Le Goater wrote:

Hello,

+Mark (for the Mac devices)

On 9/9/24 22:11, Peter Xu wrote:

From: Mattias Nissler 

When DMA memory can't be directly accessed, as is the case when
running the device model in a separate process without shareable DMA
file descriptors, bounce buffering is used.

It is not uncommon for device models to request mapping of several DMA
regions at the same time. Examples include:
   * net devices, e.g. when transmitting a packet that is split across
 several TX descriptors (observed with igb)
   * USB host controllers, when handling a packet with multiple data TRBs
 (observed with xhci)

Previously, qemu only provided a single bounce buffer per AddressSpace
and would fail DMA map requests while the buffer was already in use. In
turn, this would cause DMA failures that ultimately manifest as hardware
errors from the guest perspective.

This change allocates DMA bounce buffers dynamically instead of
supporting only a single buffer. Thus, multiple DMA mappings work
correctly also when RAM can't be mmap()-ed.

The total bounce buffer allocation size is limited individually for each
AddressSpace. The default limit is 4096 bytes, matching the previous
maximum buffer size. A new x-max-bounce-buffer-size parameter is
provided to configure the limit for PCI devices.

Signed-off-by: Mattias Nissler 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Peter Xu 
Link: https://lore.kernel.org/r/20240819135455.2957406-1-mniss...@rivosinc.com
Signed-off-by: Peter Xu 
---
   include/exec/memory.h   | 14 +++
   include/hw/pci/pci_device.h |  3 ++
   hw/pci/pci.c|  8 
   system/memory.c |  5 ++-
   system/physmem.c| 82 ++---
   5 files changed, 76 insertions(+), 36 deletions(-)


Here is a report of a segv of the ppc64 mac99+cpu970 machine booting debian.
See the stack trace below. Just wanted to let you know. I will digging further
next week.

Thanks,

C.



Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
address_space_unmap (len=, access_len=0, is_write=false, 
buffer=0x0,
 as=0x565d45c0 ) at ../system/physmem.c:
  assert(bounce->magic == BOUNCE_BUFFER_MAGIC);
(gdb) bt
#0  address_space_unmap
 (len=, access_len=0, is_write=false, buffer=0x0, 
as=0x565d45c0 )
 at ../system/physmem.c:
#1  address_space_unmap
 (as=as@entry=0x565d45c0 , buffer=0x0, len=, is_write=, access_len=0) at ../system/physmem.c:3313
#2  0x5595ea48 in dma_memory_unmap
 (access_len=, dir=, len=, 
buffer=, as=) at 
/home/legoater/work/qemu/qemu.git/include/sysemu/dma.h:236
#3  pmac_ide_atapi_transfer_cb (opaque=0x56c06470, ret=) at 
../hw/ide/macio.c:122
#4  0x559861f3 in DBDMA_run (s=0x56c04c60) at 
../hw/misc/macio/mac_dbdma.c:546
#5  DBDMA_run_bh (opaque=0x56c04c60) at ../hw/misc/macio/mac_dbdma.c:556
#6  0x55f19f33 in aio_bh_call (bh=bh@entry=0x56ab5570) at 
../util/async.c:171
#7  0x55f1a0f5 in aio_bh_poll (ctx=ctx@entry=0x566af150) at 
../util/async.c:218
#8  0x55f0269e in aio_dispatch (ctx=0x566af150) at 
../util/aio-posix.c:423
#9  0x55f19d8e in aio_ctx_dispatch
 (source=, callback=, user_data=) at ../util/async.c:360
#10 0x77315f4f in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#11 0x55f1b488 in glib_pollfds_poll () at ../util/main-loop.c:287
#12 os_host_main_loop_wait (timeout=2143429) at ../util/main-loop.c:310
#13 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:589
#14 0x55abeba3 in qemu_main_loop () at ../system/runstate.c:826
#15 0x55e63787 in qemu_default_main () at ../system/main.c:37
#16 0x76e29590 in __libc_start_call_main () at /lib64/libc.so.6
#17 0x76e29640 in __libc_start_main_impl () at /lib64/libc.so.6
#18 0x5588d4f5 in _start ()


Thanks for the report!

Mattias,

Would you have time to take a look?


I noticed that the backtrace indicates address_space_unmap is called
with buffer=0x0, len=0. This wasn't really correct before my
concurrent bounce buffering change either, but it looks like the
previous code would have tolerated this to a certain extent (at least
no immediate crashes). Original code in question:

 if (is_write) {
 address_space_write(as, as->bounce.addr, MEMTXATTRS_UNSPECIFIED,
 as->bounce.buffer, access_len);
 }
 qemu_vfree(as->bounce.buffer);
 as->bounce.buffer = NULL;
 memory_region_unref(as->bounce.mr);
 /* Clear in_use before reading map_client_list.  */
 qatomic_set_mb(&as->bounce.in_use, false);
 address_space_notify_map_clients(as);

address_space_write and qemu_vfree are safe to call with NULL/0
p

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Peter Maydell]

On Mon, 16 Sept 2024 at 12:29, Mark Cave-Ayland
 wrote:
>
> On 16/09/2024 09:23, Mattias Nissler wrote:
> > Looking at the code, the dma_memory_unmap calls in hw/ide/macio.c seem
> > to be passing buffer=NULL unconditionally, since the dma_mem field in
> > struct DBDMA_io is never set to anything non-zero. In fact, I believe
> > after commit be1e343995ef81fc05d9a4e1ec263ca171d842e7 "macio: switch
> > over to new byte-aligned DMA helpers", the dma_memory_unmap calls in
> > hw/ide/macio.c aren't doing anything and should probably have been
> > removed together with the dma_mem, dma_len and dir fields in struct
> > DBDMA_io. Speculative patch:
> >
> > diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> > index e84bf2c9f6..15dd40138e 100644
> > --- a/hw/ide/macio.c
> > +++ b/hw/ide/macio.c
> > @@ -119,9 +119,6 @@ static void pmac_ide_atapi_transfer_cb(void
> > *opaque, int ret)
> >   return;
> >
> >   done:
> > -dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
> > - io->dir, io->dma_len);
> > -
> >   if (ret < 0) {
> >   block_acct_failed(blk_get_stats(s->blk), &s->acct);
> >   } else {
> > @@ -202,9 +199,6 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
> >   return;
> >
> >   done:
> > -dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
> > - io->dir, io->dma_len);
> > -
> >   if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
> >   if (ret < 0) {
> >   block_acct_failed(blk_get_stats(s->blk), &s->acct);
> > diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
> > index 4a3f644516..c774f6bf84 100644
> > --- a/include/hw/ppc/mac_dbdma.h
> > +++ b/include/hw/ppc/mac_dbdma.h
> > @@ -44,10 +44,6 @@ struct DBDMA_io {
> >   DBDMA_end dma_end;
> >   /* DMA is in progress, don't start another one */
> >   bool processing;
> > -/* DMA request */
> > -void *dma_mem;
> > -dma_addr_t dma_len;
> > -DMADirection dir;
> >   };
> >
> >   /*
> >
> > Cédric, can you try with the above patch and/or share more details of
> > your setup so I can verify (I tried booting a ppc64el-pseries dqib
> > image but didn't see the issue)?
>
> I'm fairly sure that this patch would break MacOS 9 which was the reason that
> dma_memory_unmap() was added here in the first place: what I was finding was 
> that
> without the dma_memory_unmap() the destination RAM wasn't being invalidated 
> (or
> marked dirty), causing random crashes during boot.

dma_memory_unmap() of something you never mapped is
definitely wrong. Whatever is going on here, leaving the unmap
call in after you removed the dma_memory_map() call is just
papering over the actual cause of the crashes.

> Would the issue be solved by adding a corresponding dma_memory_map() 
> beforehand at
> the relevant places in hw/ide/macio.c? If that's required as part of the 
> setup for
> bounce buffers then I can see how not having this present could cause 
> problems.

The only purpose of this API is sequences of:
  host_ptr = dma_memory_map(...);
  access the host_ptr directly;
  dma_memory_unmap(...);

The bounce-buffer stuff is an internal implementation detail
of making this API work when the DMA is going to a device.

We need to find whatever the actual cause of the macos failure is.
Mattias' suggested change looks right to me.

I do wonder if something needs the memory barrier than
unmap does as part of its operation, e.g. in the
implementation of the dma_blk_* functions.

-- PMM

Re: [PATCH v6 00/17] bsd-user: Comprehensive RISCV Support

[Daniel Henrique Barboza]

Hi,

Please CC the RISC-V maintainer (Alistair, that I just CCed in this reply) in 
all RISC-V
related patches. It would be nice to also CC qemu-ri...@nongnu.org to get more 
visibility
from the RISC-V developers too.


This series won't build in a FreeBSD x86_64 host:

In file included from ../bsd-user/main.c:53:
../bsd-user/riscv/target_arch_cpu.h:126:13: error: call to undeclared function 
'force_sig_fault'; ISO C99 and later do not support implicit function 
declarations [-Werror,-Wimplicit-function-declaration]
  126 | force_sig_fault(signo, code, env->pc);
  | ^
../bsd-user/riscv/target_arch_cpu.h:129:9: error: call to undeclared function 
'process_pending_signals'; ISO C99 and later do not support implicit function 
declarations [-Werror,-Wimplicit-function-declaration]
  129 | process_pending_signals(env);
  | ^
../bsd-user/main.c:608:5: error: call to undeclared function 'signal_init'; ISO 
C99 and later do not support implicit function declarations 
[-Werror,-Wimplicit-function-declaration]
  608 | signal_init();
  | ^
3 errors generated.


You're missing the following header in patch 2:


diff --git a/bsd-user/riscv/target_arch_cpu.h b/bsd-user/riscv/target_arch_cpu.h
index 57abfbd556..a93ea3915a 100644
--- a/bsd-user/riscv/target_arch_cpu.h
+++ b/bsd-user/riscv/target_arch_cpu.h
@@ -21,6 +21,7 @@
 #define TARGET_ARCH_CPU_H
 
 #include "target_arch.h"

+#include "signal-common.h"
 
 #define TARGET_DEFAULT_CPU_MODEL "max"



Please CC dbarb...@ventanamicro.com in the v7 because I'll compile test this 
series to avoid
another pipeline fail that will hold the upstreaming of everything else.



Thanks,


Daniel


On 9/15/24 12:25 PM, Ajeet Singh wrote:

Key Changes Compared to Version 5:
In target_arch_sigtramp.h removed static const,
as there was a compile-time constant issue

Mark Corbin (15):
   bsd-user: Implement RISC-V CPU initialization and main loop
   bsd-user: Add RISC-V CPU execution loop and syscall handling
   bsd-user: Implement RISC-V CPU register cloning and reset functions
   bsd-user: Implement RISC-V TLS register setup
   bsd-user: Add RISC-V ELF definitions and hardware capability detection
   bsd-user: Define RISC-V register structures and register copying
   bsd-user: Add RISC-V signal trampoline setup function
   bsd-user: Implement RISC-V sysarch system call emulation
   bsd-user: Add RISC-V thread setup and initialization support
   bsd-user: Define RISC-V VM parameters and helper functions
   bsd-user: Define RISC-V system call structures and constants
   bsd-user: Define RISC-V signal handling structures and constants
   bsd-user: Implement RISC-V signal trampoline setup functions
   bsd-user: Implement 'get_mcontext' for RISC-V
   bsd-user: Implement set_mcontext and get_ucontext_sigreturn for RISCV

Warner Losh (2):
   bsd-user: Add generic RISC-V64 target definitions
   bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files

  bsd-user/riscv/signal.c   | 170 ++
  bsd-user/riscv/target.h   |  20 +++
  bsd-user/riscv/target_arch.h  |  27 
  bsd-user/riscv/target_arch_cpu.c  |  29 +
  bsd-user/riscv/target_arch_cpu.h  | 147 ++
  bsd-user/riscv/target_arch_elf.h  |  42 +++
  bsd-user/riscv/target_arch_reg.h  |  88 +
  bsd-user/riscv/target_arch_signal.h   |  75 
  bsd-user/riscv/target_arch_sigtramp.h |  41 +++
  bsd-user/riscv/target_arch_sysarch.h  |  41 +++
  bsd-user/riscv/target_arch_thread.h   |  47 +++
  bsd-user/riscv/target_arch_vmparam.h  |  53 
  bsd-user/riscv/target_syscall.h   |  38 ++
  configs/targets/riscv64-bsd-user.mak  |   4 +
  14 files changed, 822 insertions(+)
  create mode 100644 bsd-user/riscv/signal.c
  create mode 100644 bsd-user/riscv/target.h
  create mode 100644 bsd-user/riscv/target_arch.h
  create mode 100644 bsd-user/riscv/target_arch_cpu.c
  create mode 100644 bsd-user/riscv/target_arch_cpu.h
  create mode 100644 bsd-user/riscv/target_arch_elf.h
  create mode 100644 bsd-user/riscv/target_arch_reg.h
  create mode 100644 bsd-user/riscv/target_arch_signal.h
  create mode 100644 bsd-user/riscv/target_arch_sigtramp.h
  create mode 100644 bsd-user/riscv/target_arch_sysarch.h
  create mode 100644 bsd-user/riscv/target_arch_thread.h
  create mode 100644 bsd-user/riscv/target_arch_vmparam.h
  create mode 100644 bsd-user/riscv/target_syscall.h
  create mode 100644 configs/targets/riscv64-bsd-user.mak

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Cédric Le Goater]

On 9/16/24 10:23, Mattias Nissler wrote:

Thanks for the report, and my apologies for the breakage.

On Fri, Sep 13, 2024 at 4:47 PM Peter Xu  wrote:


On Fri, Sep 13, 2024 at 04:35:32PM +0200, Cédric Le Goater wrote:

Hello,

+Mark (for the Mac devices)

On 9/9/24 22:11, Peter Xu wrote:

From: Mattias Nissler 

When DMA memory can't be directly accessed, as is the case when
running the device model in a separate process without shareable DMA
file descriptors, bounce buffering is used.

It is not uncommon for device models to request mapping of several DMA
regions at the same time. Examples include:
   * net devices, e.g. when transmitting a packet that is split across
 several TX descriptors (observed with igb)
   * USB host controllers, when handling a packet with multiple data TRBs
 (observed with xhci)

Previously, qemu only provided a single bounce buffer per AddressSpace
and would fail DMA map requests while the buffer was already in use. In
turn, this would cause DMA failures that ultimately manifest as hardware
errors from the guest perspective.

This change allocates DMA bounce buffers dynamically instead of
supporting only a single buffer. Thus, multiple DMA mappings work
correctly also when RAM can't be mmap()-ed.

The total bounce buffer allocation size is limited individually for each
AddressSpace. The default limit is 4096 bytes, matching the previous
maximum buffer size. A new x-max-bounce-buffer-size parameter is
provided to configure the limit for PCI devices.

Signed-off-by: Mattias Nissler 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Peter Xu 
Link: https://lore.kernel.org/r/20240819135455.2957406-1-mniss...@rivosinc.com
Signed-off-by: Peter Xu 
---
   include/exec/memory.h   | 14 +++
   include/hw/pci/pci_device.h |  3 ++
   hw/pci/pci.c|  8 
   system/memory.c |  5 ++-
   system/physmem.c| 82 ++---
   5 files changed, 76 insertions(+), 36 deletions(-)


Here is a report of a segv of the ppc64 mac99+cpu970 machine booting debian.
See the stack trace below. Just wanted to let you know. I will digging further
next week.

Thanks,

C.



Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
address_space_unmap (len=, access_len=0, is_write=false, 
buffer=0x0,
 as=0x565d45c0 ) at ../system/physmem.c:
  assert(bounce->magic == BOUNCE_BUFFER_MAGIC);
(gdb) bt
#0  address_space_unmap
 (len=, access_len=0, is_write=false, buffer=0x0, 
as=0x565d45c0 )
 at ../system/physmem.c:
#1  address_space_unmap
 (as=as@entry=0x565d45c0 , buffer=0x0, len=, is_write=, access_len=0) at ../system/physmem.c:3313
#2  0x5595ea48 in dma_memory_unmap
 (access_len=, dir=, len=, 
buffer=, as=) at 
/home/legoater/work/qemu/qemu.git/include/sysemu/dma.h:236
#3  pmac_ide_atapi_transfer_cb (opaque=0x56c06470, ret=) at 
../hw/ide/macio.c:122
#4  0x559861f3 in DBDMA_run (s=0x56c04c60) at 
../hw/misc/macio/mac_dbdma.c:546
#5  DBDMA_run_bh (opaque=0x56c04c60) at ../hw/misc/macio/mac_dbdma.c:556
#6  0x55f19f33 in aio_bh_call (bh=bh@entry=0x56ab5570) at 
../util/async.c:171
#7  0x55f1a0f5 in aio_bh_poll (ctx=ctx@entry=0x566af150) at 
../util/async.c:218
#8  0x55f0269e in aio_dispatch (ctx=0x566af150) at 
../util/aio-posix.c:423
#9  0x55f19d8e in aio_ctx_dispatch
 (source=, callback=, user_data=) at ../util/async.c:360
#10 0x77315f4f in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#11 0x55f1b488 in glib_pollfds_poll () at ../util/main-loop.c:287
#12 os_host_main_loop_wait (timeout=2143429) at ../util/main-loop.c:310
#13 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:589
#14 0x55abeba3 in qemu_main_loop () at ../system/runstate.c:826
#15 0x55e63787 in qemu_default_main () at ../system/main.c:37
#16 0x76e29590 in __libc_start_call_main () at /lib64/libc.so.6
#17 0x76e29640 in __libc_start_main_impl () at /lib64/libc.so.6
#18 0x5588d4f5 in _start ()


Thanks for the report!

Mattias,

Would you have time to take a look?


I noticed that the backtrace indicates address_space_unmap is called
with buffer=0x0, len=0. This wasn't really correct before my
concurrent bounce buffering change either, but it looks like the
previous code would have tolerated this to a certain extent (at least
no immediate crashes). Original code in question:

 if (is_write) {
 address_space_write(as, as->bounce.addr, MEMTXATTRS_UNSPECIFIED,
 as->bounce.buffer, access_len);
 }
 qemu_vfree(as->bounce.buffer);
 as->bounce.buffer = NULL;
 memory_region_unref(as->bounce.mr);
 /* Clear in_use before reading map_client_list.  */
 qatomic_set_mb(&as->bounce.in_use, false);
 address_space_notify_map_clients(as);

address_space_write and qemu_vfree are safe to call with NULL/0
param

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Mark Cave-Ayland]

On 16/09/2024 12:44, Peter Maydell wrote:


On Mon, 16 Sept 2024 at 12:29, Mark Cave-Ayland
 wrote:


On 16/09/2024 09:23, Mattias Nissler wrote:

Looking at the code, the dma_memory_unmap calls in hw/ide/macio.c seem
to be passing buffer=NULL unconditionally, since the dma_mem field in
struct DBDMA_io is never set to anything non-zero. In fact, I believe
after commit be1e343995ef81fc05d9a4e1ec263ca171d842e7 "macio: switch
over to new byte-aligned DMA helpers", the dma_memory_unmap calls in
hw/ide/macio.c aren't doing anything and should probably have been
removed together with the dma_mem, dma_len and dir fields in struct
DBDMA_io. Speculative patch:

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index e84bf2c9f6..15dd40138e 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -119,9 +119,6 @@ static void pmac_ide_atapi_transfer_cb(void
*opaque, int ret)
   return;

   done:
-dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
- io->dir, io->dma_len);
-
   if (ret < 0) {
   block_acct_failed(blk_get_stats(s->blk), &s->acct);
   } else {
@@ -202,9 +199,6 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
   return;

   done:
-dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
- io->dir, io->dma_len);
-
   if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
   if (ret < 0) {
   block_acct_failed(blk_get_stats(s->blk), &s->acct);
diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
index 4a3f644516..c774f6bf84 100644
--- a/include/hw/ppc/mac_dbdma.h
+++ b/include/hw/ppc/mac_dbdma.h
@@ -44,10 +44,6 @@ struct DBDMA_io {
   DBDMA_end dma_end;
   /* DMA is in progress, don't start another one */
   bool processing;
-/* DMA request */
-void *dma_mem;
-dma_addr_t dma_len;
-DMADirection dir;
   };

   /*

Cédric, can you try with the above patch and/or share more details of
your setup so I can verify (I tried booting a ppc64el-pseries dqib
image but didn't see the issue)?


I'm fairly sure that this patch would break MacOS 9 which was the reason that
dma_memory_unmap() was added here in the first place: what I was finding was 
that
without the dma_memory_unmap() the destination RAM wasn't being invalidated (or
marked dirty), causing random crashes during boot.


dma_memory_unmap() of something you never mapped is
definitely wrong. Whatever is going on here, leaving the unmap
call in after you removed the dma_memory_map() call is just
papering over the actual cause of the crashes.


Would the issue be solved by adding a corresponding dma_memory_map() beforehand 
at
the relevant places in hw/ide/macio.c? If that's required as part of the setup 
for
bounce buffers then I can see how not having this present could cause problems.


The only purpose of this API is sequences of:
   host_ptr = dma_memory_map(...);
   access the host_ptr directly;
   dma_memory_unmap(...);

The bounce-buffer stuff is an internal implementation detail
of making this API work when the DMA is going to a device.

We need to find whatever the actual cause of the macos failure is.
Mattias' suggested change looks right to me.

I do wonder if something needs the memory barrier than
unmap does as part of its operation, e.g. in the
implementation of the dma_blk_* functions.


It has been a few years now, but I'm fairly sure the issue was that dma_blk_read() 
didn't mark RAM containing code as dirty/invalid, and since MacOS 9 used overlays 
then it would crash randomly trying to execute stale memory. dma_memory_unmap() 
checks to see if the direction was to RAM, and then marks the memory dirty allowing 
the new code to get picked up after a MMU fault.


If the memory barriers are already in place for the dma_blk_*() functions then the 
analysis could be correct, in which case the bug is a misunderstanding I made in 
be1e343995 ("macio: switch over to new byte-aligned DMA helpers") back in 2016.



ATB,

Mark.

Re: [PATCH] hostmem: Apply merge property after the memory region is initialized

[Zhenyu Zhang]

[PATCH] hostmem: Apply merge property after the memory region is initialized

Test on 4k and 64k basic page size aarch64
The patches work well on my Ampere host.
The test results are as expected.

# /home/test/qemu.main/build/qemu-system-aarch64  \
   -accel kvm  -machine virt -cpu host   \
   -object memory-backend-ram,id=mem-memN0,size=4096M,merge=off

Tested-by: Zhenyu Zhang 


On Mon, Sep 16, 2024 at 7:31 AM Gavin Shan  wrote:
>
> The semantic change has been introduced by commit 5becdc0ab0 ("hostmem:
> simplify the code for merge and dump properties") even it clarifies that
> no senmatic change has been introduced. After the commit, the merge
> property can be applied even the corresponding memory region isn't
> initialized yet. This leads to crash dump by the following command
> lines.
>
>   # /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64  \
> -accel kvm  -machine virt -cpu host  \
> -object memory-backend-ram,id=mem-memN0,size=4096M,merge=off
> :
> qemu-system-aarch64: ../system/memory.c:2419: memory_region_get_ram_ptr: \
> Assertion `mr->ram_block' failed.
>
> Fix it by applying the merge property only when the memory region is
> initialized.
>
> Fixes: 5becdc0ab083 ("hostmem: simplify the code for merge and dump 
> properties")
> Reported-by: Zhenyu Zhang 
> Signed-off-by: Gavin Shan 
> ---
>  backends/hostmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 4e5576a4ad..181446626a 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -178,7 +178,7 @@ static void host_memory_backend_set_merge(Object *obj, 
> bool value, Error **errp)
>  return;
>  }
>
> -if (!host_memory_backend_mr_inited(backend) &&
> +if (host_memory_backend_mr_inited(backend) &&
>  value != backend->merge) {
>  void *ptr = memory_region_get_ram_ptr(&backend->mr);
>  uint64_t sz = memory_region_size(&backend->mr);
> --
> 2.45.2
>

[PULL 0/4] Edk2 stable202408 20240916 patches

[Gerd Hoffmann]

The following changes since commit ea9cdbcf3a0b8d5497cddf87990f1b39d8f3bb0a:

  Merge tag 'hw-misc-20240913' of https://github.com/philmd/qemu into staging 
(2024-09-15 18:27:40 +0100)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git 
tags/edk2-stable202408-20240916-pull-request

for you to fetch changes up to 2b759fbc9a70258a244f98da3415947dccc2702e:

  add loongarch binaries for edk2-stable202408 (2024-09-16 14:21:34 +0200)


edk2: update to 2024-08 stable tag



Gerd Hoffmann (3):
  update submodule and version file to edk2-stable202408
  update binaries to edk2-stable202408
  add loongarch binaries for edk2-stable202408

Xianglai Li (1):
  roms: Support compile the efi bios for loongarch

 docs/system/loongarch/virt.rst   |   2 +-
 meson.build  |   2 +-
 pc-bios/descriptors/60-edk2-loongarch64.json |  31 +++
 pc-bios/descriptors/meson.build  |   3 +-
 pc-bios/edk2-aarch64-code.fd.bz2 | Bin 1588976 -> 1565763 bytes
 pc-bios/edk2-arm-code.fd.bz2 | Bin 1571639 -> 1570311 bytes
 pc-bios/edk2-i386-code.fd.bz2| Bin 1775230 -> 1780004 bytes
 pc-bios/edk2-i386-secure-code.fd.bz2 | Bin 1877268 -> 1858666 bytes
 pc-bios/edk2-loongarch64-code.fd.bz2 | Bin 0 -> 1148383 bytes
 pc-bios/edk2-loongarch64-vars.fd.bz2 | Bin 0 -> 233 bytes
 pc-bios/edk2-riscv-code.fd.bz2   | Bin 1289337 -> 1296526 bytes
 pc-bios/edk2-x86_64-code.fd.bz2  | Bin 1892766 -> 1907255 bytes
 pc-bios/edk2-x86_64-microvm.fd.bz2   | Bin 1785290 -> 1787244 bytes
 pc-bios/edk2-x86_64-secure-code.fd.bz2   | Bin 1969096 -> 1962992 bytes
 pc-bios/meson.build  |   2 ++
 roms/edk2|   2 +-
 roms/edk2-build.config   |  13 
 roms/edk2-version|   4 +--
 18 files changed, 53 insertions(+), 6 deletions(-)
 create mode 100644 pc-bios/descriptors/60-edk2-loongarch64.json
 create mode 100644 pc-bios/edk2-loongarch64-code.fd.bz2
 create mode 100644 pc-bios/edk2-loongarch64-vars.fd.bz2

-- 
2.46.0

[PULL 3/4] roms: Support compile the efi bios for loongarch

[Gerd Hoffmann]

From: Xianglai Li 

Added loongarch UEFI BIOS support to compiled scripts.

  UEFI code images require 16M alignment, flash images require
16M alignment, under the loongarch architecture.This is agreed
upon when the firmware is loaded in QEMU under Loongarch.

  The naming of UEFI under loongarch refers to the x86 and arm naming methods,
and the UEFI image names in x86 and arm are:
edk2-i386-code.fd
edk2-i386-vars.fd
edk2-arm-code.fd
edk2-arm-vars.fd
So on loongarch, we named it:
edk2-loongarch64-code.fd
edk2-loongarch64-vars.fd

Signed-off-by: Xianglai Li 
Message-ID: <20240724022245.1317884-1-lixiang...@loongson.cn>
Signed-off-by: Gerd Hoffmann 
---
 docs/system/loongarch/virt.rst   |  2 +-
 meson.build  |  2 +-
 pc-bios/descriptors/60-edk2-loongarch64.json | 31 
 pc-bios/descriptors/meson.build  |  3 +-
 pc-bios/meson.build  |  2 ++
 roms/edk2-build.config   | 13 
 6 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 pc-bios/descriptors/60-edk2-loongarch64.json

diff --git a/docs/system/loongarch/virt.rst b/docs/system/loongarch/virt.rst
index 06d034b8ef28..172fba079e9d 100644
--- a/docs/system/loongarch/virt.rst
+++ b/docs/system/loongarch/virt.rst
@@ -64,7 +64,7 @@ Note: You need get the latest cross-tools at 
https://github.com/loongson/build-t
 
 (3) Build BIOS:
 
-See: 
https://github.com/tianocore/edk2-platforms/tree/master/Platform/Loongson/LoongArchQemuPkg#readme
+See: 
https://github.com/tianocore/edk2/tree/master/OvmfPkg/LoongArchVirt#readme
 
 Note: To build the release version of the bios,  set --buildtarget=RELEASE,
   the bios file path:  Build/LoongArchQemu/RELEASE_GCC5/FV/QEMU_EFI.fd
diff --git a/meson.build b/meson.build
index 2d4bf71b24ef..10464466ff39 100644
--- a/meson.build
+++ b/meson.build
@@ -93,7 +93,7 @@ else
   iasl = find_program(get_option('iasl'), required: true)
 endif
 
-edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 
'x86_64-softmmu', 'riscv64-softmmu' ]
+edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 
'x86_64-softmmu', 'riscv64-softmmu', 'loongarch64-softmmu' ]
 unpack_edk2_blobs = false
 foreach target : edk2_targets
   if target in target_dirs
diff --git a/pc-bios/descriptors/60-edk2-loongarch64.json 
b/pc-bios/descriptors/60-edk2-loongarch64.json
new file mode 100644
index ..f174a1fc9b27
--- /dev/null
+++ b/pc-bios/descriptors/60-edk2-loongarch64.json
@@ -0,0 +1,31 @@
+{
+"description": "UEFI firmware for loongarch64",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"executable": {
+"filename": "@DATADIR@/edk2-loongarch64-code.fd",
+"format": "raw"
+},
+"nvram-template": {
+"filename": "@DATADIR@/edk2-loongarch64-vars.fd",
+"format": "raw"
+}
+},
+"targets": [
+{
+"architecture": "loongarch64",
+"machines": [
+"virt*"
+]
+}
+],
+"features": [
+
+],
+"tags": [
+
+]
+}
diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
index 66f85d01c427..afb5a959ccf4 100644
--- a/pc-bios/descriptors/meson.build
+++ b/pc-bios/descriptors/meson.build
@@ -5,7 +5,8 @@ if unpack_edk2_blobs and get_option('install_blobs')
 '60-edk2-aarch64.json',
 '60-edk2-arm.json',
 '60-edk2-i386.json',
-'60-edk2-x86_64.json'
+'60-edk2-x86_64.json',
+'60-edk2-loongarch64.json'
   ]
 configure_file(input: files(f),
output: f,
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 8602b45b9b32..090379763e76 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -11,6 +11,8 @@ if unpack_edk2_blobs
 'edk2-i386-vars.fd',
 'edk2-x86_64-code.fd',
 'edk2-x86_64-secure-code.fd',
+'edk2-loongarch64-code.fd',
+'edk2-loongarch64-vars.fd',
   ]
 
   foreach f : fds
diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index cc9b21154205..9e45361fb445 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -131,3 +131,16 @@ cpy1 = FV/RISCV_VIRT_CODE.fd  edk2-riscv-code.fd
 cpy2 = FV/RISCV_VIRT_VARS.fd  edk2-riscv-vars.fd
 pad1 = edk2-riscv-code.fd 32m
 pad2 = edk2-riscv-vars.fd 32m
+
+
+# LoongArch64
+
+[build.loongarch64.qemu]
+conf = OvmfPkg/LoongArchVirt/LoongArchVirtQemu.dsc
+arch = LOONGARCH64
+plat = LoongArchVirtQemu
+dest = ../pc-bios
+cpy1 = FV/QEMU_EFI.fd  edk2-loongarch64-code.fd
+pad1 = edk2-loongarch64-code.fd 16m
+cpy2 = FV/QEMU_VARS.fd  edk2-loongarch64-vars.fd
+pad2 = edk2-loongarch64-vars.fd 16m
-- 
2.46.0

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Peter Maydell]

On Mon, 16 Sept 2024 at 13:14, Mark Cave-Ayland
 wrote:
>
> On 16/09/2024 12:44, Peter Maydell wrote:
>
> > On Mon, 16 Sept 2024 at 12:29, Mark Cave-Ayland
> >  wrote:
> >> I'm fairly sure that this patch would break MacOS 9 which was the reason 
> >> that
> >> dma_memory_unmap() was added here in the first place: what I was finding 
> >> was that
> >> without the dma_memory_unmap() the destination RAM wasn't being 
> >> invalidated (or
> >> marked dirty), causing random crashes during boot.
> >
> > dma_memory_unmap() of something you never mapped is
> > definitely wrong. Whatever is going on here, leaving the unmap
> > call in after you removed the dma_memory_map() call is just
> > papering over the actual cause of the crashes.
> >
> >> Would the issue be solved by adding a corresponding dma_memory_map() 
> >> beforehand at
> >> the relevant places in hw/ide/macio.c? If that's required as part of the 
> >> setup for
> >> bounce buffers then I can see how not having this present could cause 
> >> problems.
> >
> > The only purpose of this API is sequences of:
> >host_ptr = dma_memory_map(...);
> >access the host_ptr directly;
> >dma_memory_unmap(...);
> >
> > The bounce-buffer stuff is an internal implementation detail
> > of making this API work when the DMA is going to a device.
> >
> > We need to find whatever the actual cause of the macos failure is.
> > Mattias' suggested change looks right to me.
> >
> > I do wonder if something needs the memory barrier than
> > unmap does as part of its operation, e.g. in the
> > implementation of the dma_blk_* functions.
>
> It has been a few years now, but I'm fairly sure the issue was that 
> dma_blk_read()
> didn't mark RAM containing code as dirty/invalid, and since MacOS 9 used 
> overlays
> then it would crash randomly trying to execute stale memory. 
> dma_memory_unmap()
> checks to see if the direction was to RAM, and then marks the memory dirty 
> allowing
> the new code to get picked up after a MMU fault.

dma_blk_io() does its writes into guest memory by doing
a dma_memory_map()/write-to-host-pointer/dma_memory_unmap()
sequence, though (this is done in dma_blk_cb()).

More generally there should be *no* path for doing writes to
guest memory that does not handle the dirty-memory case:
so if there is one we need to find and fix it.

thanks
-- PMM

[PULL 1/4] update submodule and version file to edk2-stable202408

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 roms/edk2 | 2 +-
 roms/edk2-version | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/roms/edk2 b/roms/edk2
index edc6681206c1..b158dad150bf 16
--- a/roms/edk2
+++ b/roms/edk2
@@ -1 +1 @@
-Subproject commit edc6681206c1a8791981a2f911d2fb8b3d2f5768
+Subproject commit b158dad150bf02879668f72ce306445250838201
diff --git a/roms/edk2-version b/roms/edk2-version
index 1594ed8c4de9..069f19f8bf49 100644
--- a/roms/edk2-version
+++ b/roms/edk2-version
@@ -1,2 +1,2 @@
-EDK2_STABLE = edk2-stable202402
-EDK2_DATE = 02/14/2024
+EDK2_STABLE = edk2-stable202408
+EDK2_DATE = 08/13/2024
-- 
2.46.0

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Cédric Le Goater]

Mattias,


Cédric, can you try with the above patch and/or 


crash seems gone.


share more details of your setup so I can verify


You will need a Linnux powerpc or powerpc64 image for mac machines,
which are not common now days, or MacOS images. My debian images
are big. I will try to build you a small one for more tests.


Grab :

  
https://cdimage.debian.org/cdimage/ports/10.0/powerpc/iso-cd/debian-10.0-powerpc-NETINST-1.iso

and run :

  qemu-system-ppc -M mac99 -cpu g4 -cdrom debian-10.0.0-powerpc-NETINST-1.iso 
-nographic -boot d

Thanks,

C.

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

Thanks Cédric, I can reproduce now, and my proposed patch fixes avoids
the crash as expected.

On Mon, Sep 16, 2024 at 2:28 PM Cédric Le Goater  wrote:
>
> Mattias,
>
>
> > Cédric, can you try with the above patch and/or
> >
> > crash seems gone.
> >
> >> share more details of your setup so I can verify
> >
> > You will need a Linnux powerpc or powerpc64 image for mac machines,
> > which are not common now days, or MacOS images. My debian images
> > are big. I will try to build you a small one for more tests.
>
> Grab :
>
>
> https://cdimage.debian.org/cdimage/ports/10.0/powerpc/iso-cd/debian-10.0-powerpc-NETINST-1.iso
>
> and run :
>
>qemu-system-ppc -M mac99 -cpu g4 -cdrom 
> debian-10.0.0-powerpc-NETINST-1.iso -nographic -boot d
>
> Thanks,
>
> C.
>
>

Re: [PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs

[Brian Cain]

On 9/13/2024 9:30 PM, Brad Smith wrote:

On 2024-09-10 10:06 p.m., Brian Cain wrote:


On 9/10/2024 5:26 PM, Brad Smith wrote:

On 2024-09-10 4:17 p.m., Thomas Huth wrote:
Some systems (like OpenBSD) do not have the sha256sum or sha512sum 
programs
installed by default. Use the Python hashlib instead so we don't 
have to

rely on the external programs.


On OpenBSD they're named sha256 and sha512.

Rather than port the test to each OS's particular program names, we 
should use the portable solution that's included w/Python.


I wasn't trying to imply that the patch wasn't the right direction, it 
is when it comes portability. Just commenting
on what it says in the commit message. This isn't the first time such 
differences have come up.



I see.  Good point.

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

On Mon, Sep 16, 2024 at 2:28 PM Peter Maydell  wrote:
>
> On Mon, 16 Sept 2024 at 13:14, Mark Cave-Ayland
>  wrote:
> >
> > On 16/09/2024 12:44, Peter Maydell wrote:
> >
> > > On Mon, 16 Sept 2024 at 12:29, Mark Cave-Ayland
> > >  wrote:
> > >> I'm fairly sure that this patch would break MacOS 9 which was the reason 
> > >> that
> > >> dma_memory_unmap() was added here in the first place: what I was finding 
> > >> was that
> > >> without the dma_memory_unmap() the destination RAM wasn't being 
> > >> invalidated (or
> > >> marked dirty), causing random crashes during boot.
> > >
> > > dma_memory_unmap() of something you never mapped is
> > > definitely wrong. Whatever is going on here, leaving the unmap
> > > call in after you removed the dma_memory_map() call is just
> > > papering over the actual cause of the crashes.
> > >
> > >> Would the issue be solved by adding a corresponding dma_memory_map() 
> > >> beforehand at
> > >> the relevant places in hw/ide/macio.c? If that's required as part of the 
> > >> setup for
> > >> bounce buffers then I can see how not having this present could cause 
> > >> problems.
> > >
> > > The only purpose of this API is sequences of:
> > >host_ptr = dma_memory_map(...);
> > >access the host_ptr directly;
> > >dma_memory_unmap(...);
> > >
> > > The bounce-buffer stuff is an internal implementation detail
> > > of making this API work when the DMA is going to a device.
> > >
> > > We need to find whatever the actual cause of the macos failure is.
> > > Mattias' suggested change looks right to me.
> > >
> > > I do wonder if something needs the memory barrier than
> > > unmap does as part of its operation, e.g. in the
> > > implementation of the dma_blk_* functions.
> >
> > It has been a few years now, but I'm fairly sure the issue was that 
> > dma_blk_read()
> > didn't mark RAM containing code as dirty/invalid, and since MacOS 9 used 
> > overlays
> > then it would crash randomly trying to execute stale memory. 
> > dma_memory_unmap()
> > checks to see if the direction was to RAM, and then marks the memory dirty 
> > allowing
> > the new code to get picked up after a MMU fault.
>
> dma_blk_io() does its writes into guest memory by doing
> a dma_memory_map()/write-to-host-pointer/dma_memory_unmap()
> sequence, though (this is done in dma_blk_cb()).
>
> More generally there should be *no* path for doing writes to
> guest memory that does not handle the dirty-memory case:
> so if there is one we need to find and fix it.

I concur that it should be the responsibility of the code performing
the DMA write to make sure any invalidation side effects take place
rather than relying on ad-hoc calls taking place later.

Regardless, in the interest of reaching a conclusion here: Mark, can
you provide instructions on how to verify MacOS 9 or alternatively
kindly do a quick test?

Thanks,
Mattias

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Brian Cain]

On 9/6/2024 9:39 PM, Brian Cain wrote:

With newer clang builds (19.x), there's a warning for implicit function
declarations and it rejects linux-test.c.

glibc/musl's readdir64() declaration in dirent is guarded by
_LARGEFILE64_SOURCE, so we'll define it to fix the warning.

   BUILD   hexagon-linux-user guest-tests
 
/local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
 error: call to undeclared function 'readdir64'; ISO C99 and later do not 
support implicit function declarations [-Wimplicit-function-declaration]
   189 | de = readdir64(dir);
   |  ^

Signed-off-by: Brian Cain 
---
  tests/tcg/multiarch/linux/linux-test.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/tcg/multiarch/linux/linux-test.c 
b/tests/tcg/multiarch/linux/linux-test.c
index 64f57cb287..4e0e862ad9 100644
--- a/tests/tcg/multiarch/linux/linux-test.c
+++ b/tests/tcg/multiarch/linux/linux-test.c
@@ -17,6 +17,7 @@
   *  along with this program; if not, see .
   */
  #define _GNU_SOURCE
+#define _LARGEFILE64_SOURCE
  #include 
  #include 
  #include 



Alex -- what do you think about this one?

Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Cédric Le Goater]

On 9/16/24 14:41, Mattias Nissler wrote:

Thanks Cédric, I can reproduce now, and my proposed patch fixes avoids
the crash as expected.

disk images for macos9 and macosx10 all boot.

C.

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Alex Bennée]

Brian Cain  writes:

> On 9/6/2024 9:39 PM, Brian Cain wrote:
>> With newer clang builds (19.x), there's a warning for implicit function
>> declarations and it rejects linux-test.c.
>>
>> glibc/musl's readdir64() declaration in dirent is guarded by
>> _LARGEFILE64_SOURCE, so we'll define it to fix the warning.
>>
>>BUILD   hexagon-linux-user guest-tests
>>  
>> /local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
>>  error: call to undeclared function 'readdir64'; ISO C99 and later do not 
>> support implicit function declarations [-Wimplicit-function-declaration]
>>189 | de = readdir64(dir);
>>|  ^
>>
>> Signed-off-by: Brian Cain 
>> ---
>>   tests/tcg/multiarch/linux/linux-test.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/tcg/multiarch/linux/linux-test.c 
>> b/tests/tcg/multiarch/linux/linux-test.c
>> index 64f57cb287..4e0e862ad9 100644
>> --- a/tests/tcg/multiarch/linux/linux-test.c
>> +++ b/tests/tcg/multiarch/linux/linux-test.c
>> @@ -17,6 +17,7 @@
>>*  along with this program; if not, see .
>>*/
>>   #define _GNU_SOURCE
>> +#define _LARGEFILE64_SOURCE
>>   #include 
>>   #include 
>>   #include 
>
>
> Alex -- what do you think about this one?

Looking at the glibc headers the LARGEFILE stuff seems to be mainly
about cleanly mapping readdir64 to readdir. I don't think we are trying
to exercise 64 on 32 here so we could do:

modified   tests/tcg/multiarch/linux/linux-test.c
@@ -83,7 +83,7 @@ static void test_file(void)
 struct utimbuf tbuf;
 struct iovec vecs[2];
 DIR *dir;
-struct dirent64 *de;
+struct dirent *de;
 /* TODO: make common tempdir creation for tcg tests */
 char template[] = "/tmp/linux-test-XX";
 char *tmpdir = mkdtemp(template);
@@ -186,7 +186,7 @@ static void test_file(void)
 error("opendir");
 len = 0;
 for(;;) {
-de = readdir64(dir);
+de = readdir(dir);
 if (!de)
 break;
 if (strcmp(de->d_name, ".") != 0 &&

Does that work for your clang case?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Alex Bennée]

Brian Cain  writes:

> On 9/6/2024 9:39 PM, Brian Cain wrote:
>> With newer clang builds (19.x), there's a warning for implicit function
>> declarations and it rejects linux-test.c.
>>
>> glibc/musl's readdir64() declaration in dirent is guarded by
>> _LARGEFILE64_SOURCE, so we'll define it to fix the warning.
>>
>>BUILD   hexagon-linux-user guest-tests
>>  
>> /local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
>>  error: call to undeclared function 'readdir64'; ISO C99 and later do not 
>> support implicit function declarations [-Wimplicit-function-declaration]
>>189 | de = readdir64(dir);
>>|  ^
>>
>> Signed-off-by: Brian Cain 
>> ---
>>   tests/tcg/multiarch/linux/linux-test.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/tcg/multiarch/linux/linux-test.c 
>> b/tests/tcg/multiarch/linux/linux-test.c
>> index 64f57cb287..4e0e862ad9 100644
>> --- a/tests/tcg/multiarch/linux/linux-test.c
>> +++ b/tests/tcg/multiarch/linux/linux-test.c
>> @@ -17,6 +17,7 @@
>>*  along with this program; if not, see .
>>*/
>>   #define _GNU_SOURCE
>> +#define _LARGEFILE64_SOURCE
>>   #include 
>>   #include 
>>   #include 
>
>
> Alex -- what do you think about this one?

Actually scratch that, this is a 32 compat hack:

  1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit host)

Is the __USE_LARGEFILE64 symbol in the hexagon headers?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PULL 20/49] spapr: Tag pseries-2.1 - 2.11 machines as deprecated

[Cédric Le Goater]

Hello Harsh,

On 2/19/24 09:29, Nicholas Piggin wrote:

From: Cédric Le Goater 

pseries machines before version 2.11 have undergone many changes to
correct issues, mostly regarding migration compatibility. This is
obfuscating the code uselessly and makes maintenance more difficult.
Remove them and only keep the last version of the 2.x series, 2.12,
still in use by old distros.

Reviewed-by: Thomas Huth 
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Cédric Le Goater 
Signed-off-by: Nicholas Piggin 
---
  docs/about/deprecated.rst | 8 
  hw/ppc/spapr.c| 1 +
  roms/skiboot  | 2 +-
  3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 5a2305ccd6..36bd3e15ef 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -229,6 +229,14 @@ The Nios II architecture is orphan.
  The machine is no longer in existence and has been long unmaintained
  in QEMU. This also holds for the TC51828 16MiB flash that it uses.
  
+``pseries-2.1`` up to ``pseries-2.11`` (since 9.0)

+''
+
+Older pseries machines before version 2.12 have undergone many changes
+to correct issues, mostly regarding migration compatibility. These are
+no longer maintained and removing them will make the code easier to
+read and maintain. Use versions 2.12 and above as a replacement.
+


Would you have time, or a KVM PPC team member, to start removing
the now deprecated pseries machines in the QEMU 9.2 cycle ?

Thanks,

C.

Re: [PATCH v1 06/14] s390x: introduce s390_get_memory_limit()

[Nina Schoetterl-Glausch]

On Tue, 2024-09-10 at 19:58 +0200, David Hildenbrand wrote:
> Let's add s390_get_memory_limit(), to query what has been successfully
> set via s390_set_memory_limit(). Allow setting the limit only once.
> 
> Signed-off-by: David Hildenbrand 

Reviewed-by: Nina Schoetterl-Glausch 

Comment below.
> ---
>  target/s390x/cpu-sysemu.c | 19 +--
>  target/s390x/cpu.h|  1 +
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
> index 1cd30c1d84..1915567b3a 100644
> --- a/target/s390x/cpu-sysemu.c
> +++ b/target/s390x/cpu-sysemu.c
> @@ -255,12 +255,27 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, 
> S390CPU *cpu)
>  return s390_count_running_cpus();
>  }
>  
> +static uint64_t memory_limit;
> +
>  int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
>  {
> +int ret = 0;
> +
> +if (memory_limit) {
> +return -EBUSY;
> +}
>  if (kvm_enabled()) {
> -return kvm_s390_set_mem_limit(new_limit, hw_limit);
> +ret = kvm_s390_set_mem_limit(new_limit, hw_limit);
> +}
> +if (!ret) {
> +memory_limit = new_limit;
>  }
> -return 0;
> +return ret;
> +}
> +
> +uint64_t s390_get_memory_limit(void)
> +{

Might be nice to guard/warn against s390_set_memory_limit not having been 
called before.

> +return memory_limit;
>  }
>  
>  void s390_set_max_pagesize(uint64_t pagesize, Error **errp)
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index d6b75ad0e0..7a51b606ed 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -895,6 +895,7 @@ static inline void s390_do_cpu_load_normal(CPUState *cs, 
> run_on_cpu_data arg)
>  /* cpu.c */
>  void s390_crypto_reset(void);
>  int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
> +uint64_t s390_get_memory_limit(void);
>  void s390_set_max_pagesize(uint64_t pagesize, Error **errp);
>  void s390_cmma_reset(void);
>  void s390_enable_css_support(S390CPU *cpu);

[PATCH] .gitlab-ci.d/crossbuilds.yml: Force 'make check' to -j2 for cross-i686-tci

[Peter Maydell]

In commit 1374ed49e1453c300 we forced the cross-i686-tci job to -j1 to
see if this helped with test timeouts. It seems to help with that but
on the other hand we now sometimes run into the overall 60 minute
job timeout. Try -j2 instead.

Signed-off-by: Peter Maydell 
---
 .gitlab-ci.d/crossbuilds.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 1e21d082aa4..95dfc392244 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -62,11 +62,11 @@ cross-i686-tci:
 IMAGE: debian-i686-cross
 ACCEL: tcg-interpreter
 EXTRA_CONFIGURE_OPTS: 
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
 --disable-plugins --disable-kvm
-# Force tests to run in series, to see whether this
+# Force tests to run with reduced parallelism, to see whether this
 # reduces the flakiness of this CI job. The CI
 # environment by default shows us 8 CPUs and so we
 # would otherwise be using a parallelism of 9.
-MAKE_CHECK_ARGS: check check-tcg -j1
+MAKE_CHECK_ARGS: check check-tcg -j2
 
 cross-mipsel-system:
   extends: .cross_system_build_job
-- 
2.34.1

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Brian Cain]

On 9/16/2024 8:12 AM, Alex Bennée wrote:

Brian Cain  writes:


On 9/6/2024 9:39 PM, Brian Cain wrote:

With newer clang builds (19.x), there's a warning for implicit function
declarations and it rejects linux-test.c.

glibc/musl's readdir64() declaration in dirent is guarded by
_LARGEFILE64_SOURCE, so we'll define it to fix the warning.

BUILD   hexagon-linux-user guest-tests
  
/local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
 error: call to undeclared function 'readdir64'; ISO C99 and later do not 
support implicit function declarations [-Wimplicit-function-declaration]
189 | de = readdir64(dir);
|  ^

Signed-off-by: Brian Cain 
---
   tests/tcg/multiarch/linux/linux-test.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/tests/tcg/multiarch/linux/linux-test.c 
b/tests/tcg/multiarch/linux/linux-test.c
index 64f57cb287..4e0e862ad9 100644
--- a/tests/tcg/multiarch/linux/linux-test.c
+++ b/tests/tcg/multiarch/linux/linux-test.c
@@ -17,6 +17,7 @@
*  along with this program; if not, see .
*/
   #define _GNU_SOURCE
+#define _LARGEFILE64_SOURCE
   #include 
   #include 
   #include 


Alex -- what do you think about this one?

Actually scratch that, this is a 32 compat hack:

   1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit host)

Is the __USE_LARGEFILE64 symbol in the hexagon headers?

musl does not define/use __USE_LARGEFILE64, no.  If it's well defined I 
could examine whether it makes sense to add this feature to musl, 
though.  How does __USE_LARGEFILE64 differ from _LARGEFILE64_SOURCE?  Is 
it more appropriate to define that here?


-Brian

Re: [PATCH v3 00/29] target/arm: AdvSIMD decodetree conversion, part 4

[Peter Maydell]

On Thu, 12 Sept 2024 at 03:43, Richard Henderson
 wrote:
>
> Changes for v3:
>   - Zero-extend results in Widen NeonGenNarrowEnvFn return to 64 bits
>
> Only patch 26 needs review.
>



Applied to target-arm.next, thanks.

-- PMM

[PATCH v2 1/5] amd_iommu: Rename variable mmio to mr_mmio

[Santosh Shukla]

From: Suravee Suthikulpanit 

Rename the MMIO memory region variable 'mmio' to 'mr_mmio'
so to correctly name align with struct AMDVIState::variable type.

No functional change intended.

Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Santosh Shukla 
---
 hw/i386/acpi-build.c | 4 ++--
 hw/i386/amd_iommu.c  | 6 +++---
 hw/i386/amd_iommu.h  | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4967aa745902..5c6920f90d05 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2321,7 +2321,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, 
const char *oem_id,
 /* Capability offset */
 build_append_int_noprefix(table_data, s->pci.capab_offset, 2);
 /* IOMMU base address */
-build_append_int_noprefix(table_data, s->mmio.addr, 8);
+build_append_int_noprefix(table_data, s->mr_mmio.addr, 8);
 /* PCI Segment Group */
 build_append_int_noprefix(table_data, 0, 2);
 /* IOMMU info */
@@ -2356,7 +2356,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, 
const char *oem_id,
 /* Capability offset */
 build_append_int_noprefix(table_data, s->pci.capab_offset, 2);
 /* IOMMU base address */
-build_append_int_noprefix(table_data, s->mmio.addr, 8);
+build_append_int_noprefix(table_data, s->mr_mmio.addr, 8);
 /* PCI Segment Group */
 build_append_int_noprefix(table_data, 0, 2);
 /* IOMMU info */
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 464f0b666e69..abb64ea507be 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1598,10 +1598,10 @@ static void amdvi_sysbus_realize(DeviceState *dev, 
Error **errp)
 x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID);
 
 /* set up MMIO */
-memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
-  AMDVI_MMIO_SIZE);
+memory_region_init_io(&s->mr_mmio, OBJECT(s), &mmio_mem_ops, s,
+  "amdvi-mmio", AMDVI_MMIO_SIZE);
 memory_region_add_subregion(get_system_memory(), AMDVI_BASE_ADDR,
-&s->mmio);
+&s->mr_mmio);
 pci_setup_iommu(bus, &amdvi_iommu_ops, s);
 amdvi_init(s);
 }
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 73619fe9eaa7..e5c2ae94f243 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -353,7 +353,7 @@ struct AMDVIState {
 uint32_t pprlog_head;/* ppr log head */
 uint32_t pprlog_tail;/* ppr log tail */
 
-MemoryRegion mmio; /* MMIO region  */
+MemoryRegion mr_mmio;  /* MMIO region  */
 uint8_t mmior[AMDVI_MMIO_SIZE];/* read/write MMIO  */
 uint8_t w1cmask[AMDVI_MMIO_SIZE];  /* read/write 1 clear mask  */
 uint8_t romask[AMDVI_MMIO_SIZE];   /* MMIO read/only mask  */
-- 
2.43.5

[PATCH v2 2/5] amd_iommu: Add support for pass though mode

[Santosh Shukla]

From: Suravee Suthikulpanit 

Introduce 'nodma' shared memory region to support PT mode
so that for each device, we only create an alias to shared memory
region when DMA-remapping is disabled.

Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Santosh Shukla 
---
 hw/i386/amd_iommu.c | 49 -
 hw/i386/amd_iommu.h |  2 ++
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index abb64ea507be..c5f5103f4911 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -60,8 +60,9 @@ struct AMDVIAddressSpace {
 uint8_t bus_num;/* bus number   */
 uint8_t devfn;  /* device function  */
 AMDVIState *iommu_state;/* AMDVI - one per machine  */
-MemoryRegion root;  /* AMDVI Root memory map region */
+MemoryRegion root;  /* AMDVI Root memory map region */
 IOMMUMemoryRegion iommu;/* Device's address translation region  */
+MemoryRegion iommu_nodma;   /* Alias of shared nodma memory region  */
 MemoryRegion iommu_ir;  /* Device's interrupt remapping region  */
 AddressSpace as;/* device's corresponding address space */
 };
@@ -1412,6 +1413,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
 AMDVIState *s = opaque;
 AMDVIAddressSpace **iommu_as, *amdvi_dev_as;
 int bus_num = pci_bus_num(bus);
+X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
 iommu_as = s->address_spaces[bus_num];
 
@@ -1436,13 +1438,13 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
  * Memory region relationships looks like (Address range shows
  * only lower 32 bits to make it short in length...):
  *
- * |-+---+--|
- * | Name| Address range | Priority |
- * |-+---+--+
- * | amdvi_root  | - |0 |
- * |  amdvi_iommu| - |1 |
- * |  amdvi_iommu_ir | fee0-feef |   64 |
- * |-+---+--|
+ * |+---+--|
+ * | Name   | Address range | Priority |
+ * |+---+--+
+ * | amdvi-root | - |0 |
+ * | amdvi-iommu_nodma  | - |0 |
+ * | amdvi-iommu_ir | fee0-feef |   64 |
+ * |+---+--|
  */
 memory_region_init_iommu(&amdvi_dev_as->iommu,
  sizeof(amdvi_dev_as->iommu),
@@ -1461,7 +1463,25 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
 64);
 memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
 
MEMORY_REGION(&amdvi_dev_as->iommu),
-1);
+0);
+
+/* Build the DMA Disabled alias to shared memory */
+memory_region_init_alias(&amdvi_dev_as->iommu_nodma, OBJECT(s),
+ "amdvi-sys", &s->mr_sys, 0,
+ memory_region_size(&s->mr_sys));
+memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
+&amdvi_dev_as->iommu_nodma,
+0);
+
+if (!x86_iommu->pt_supported) {
+memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
+memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
+  true);
+} else {
+memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
+  false);
+memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, true);
+}
 }
 return &iommu_as[devfn]->as;
 }
@@ -1602,6 +1622,17 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error 
**errp)
   "amdvi-mmio", AMDVI_MMIO_SIZE);
 memory_region_add_subregion(get_system_memory(), AMDVI_BASE_ADDR,
 &s->mr_mmio);
+
+/* Create the share memory regions by all devices */
+memory_region_init(&s->mr_sys, OBJECT(s), "amdvi-sys", UINT64_MAX);
+
+/* set up the DMA disabled memory region */
+memory_region_init_alias(&s->mr_nodma, OBJECT(s),
+ "amdvi-nodma", get_system_memory(), 0,
+ memory_region_size(get_system_memory()));
+memory_region_add_subregion_overlap(&s->mr_sys, 0,
+   

[PATCH v2 5/5] amd_iommu: Check APIC ID > 255 for XTSup

[Santosh Shukla]

From: Suravee Suthikulpanit 

The XTSup mode enables x2APIC support for AMD IOMMU, which is needed
to support vcpu w/ APIC ID > 255.

Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Santosh Shukla 
---
v2:
- Fixed non-kvm build issue by adding a check for kvm_irqchip_is_split()

 hw/i386/amd_iommu.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 9095146525e6..24eebf053df0 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -32,6 +32,7 @@
 #include "trace.h"
 #include "hw/i386/apic-msidef.h"
 #include "hw/qdev-properties.h"
+#include "kvm/kvm_i386.h"
 
 /* used AMD-Vi MMIO registers */
 const char *amdvi_mmio_low[] = {
@@ -1651,6 +1652,16 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error 
**errp)
 memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
 &s->mr_ir, 1);
 
+/* AMD IOMMU with x2APIC mode requires xtsup=on */
+if (x86ms->apic_id_limit > 255 && !s->xtsup) {
+error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
+exit(EXIT_FAILURE);
+}
+if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
+error_report("AMD IOMMU xt=on requires support on the KVM side");
+exit(EXIT_FAILURE);
+}
+
 pci_setup_iommu(bus, &amdvi_iommu_ops, s);
 amdvi_init(s);
 }
-- 
2.43.5

[PATCH v2 4/5] amd_iommu: Send notification when invaldate interrupt entry cache

[Santosh Shukla]

From: Suravee Suthikulpanit 

In order to support AMD IOMMU interrupt remapping emulation with PCI
pass-through devices, QEMU needs to notify VFIO when guest IOMMU driver
updates and invalidate the guest interrupt remapping table (IRT), and
communicate information so that the host IOMMU driver can update
the shadowed interrupt remapping table in the host IOMMU.

Therefore, send notification when guet IOMMU emulates the IRT invalidation
commands.

Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Santosh Shukla 
---
 hw/i386/amd_iommu.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 24fcd561345c..9095146525e6 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -431,6 +431,12 @@ static void amdvi_complete_ppr(AMDVIState *s, uint64_t 
*cmd)
 trace_amdvi_ppr_exec();
 }
 
+static void amdvi_intremap_inval_notify_all(AMDVIState *s, bool global,
+   uint32_t index, uint32_t mask)
+{
+x86_iommu_iec_notify_all(X86_IOMMU_DEVICE(s), global, index, mask);
+}
+
 static void amdvi_inval_all(AMDVIState *s, uint64_t *cmd)
 {
 if (extract64(cmd[0], 0, 60) || cmd[1]) {
@@ -438,6 +444,9 @@ static void amdvi_inval_all(AMDVIState *s, uint64_t *cmd)
s->cmdbuf + s->cmdbuf_head);
 }
 
+/* Notify global invalidation */
+amdvi_intremap_inval_notify_all(s, true, 0, 0);
+
 amdvi_iotlb_reset(s);
 trace_amdvi_all_inval();
 }
@@ -486,6 +495,9 @@ static void amdvi_inval_inttable(AMDVIState *s, uint64_t 
*cmd)
 return;
 }
 
+/* Notify global invalidation */
+amdvi_intremap_inval_notify_all(s, true, 0, 0);
+
 trace_amdvi_intr_inval();
 }
 
-- 
2.43.5

[PATCH v2 3/5] amd_iommu: Use shared memory region for Interrupt Remapping

[Santosh Shukla]

From: Suravee Suthikulpanit 

Use shared memory region for interrupt remapping which can be
aliased by all devices.

Signed-off-by: Suravee Suthikulpanit 
Signed-off-by: Santosh Shukla 
---
 hw/i386/amd_iommu.c | 22 ++
 hw/i386/amd_iommu.h |  1 +
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index c5f5103f4911..24fcd561345c 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1443,7 +1443,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
  * |+---+--+
  * | amdvi-root | - |0 |
  * | amdvi-iommu_nodma  | - |0 |
- * | amdvi-iommu_ir | fee0-feef |   64 |
+ * | amdvi-iommu_ir | fee0-feef |1 |
  * |+---+--|
  */
 memory_region_init_iommu(&amdvi_dev_as->iommu,
@@ -1454,13 +1454,6 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
 memory_region_init(&amdvi_dev_as->root, OBJECT(s),
"amdvi_root", UINT64_MAX);
 address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name);
-memory_region_init_io(&amdvi_dev_as->iommu_ir, OBJECT(s),
-  &amdvi_ir_ops, s, "amd_iommu_ir",
-  AMDVI_INT_ADDR_SIZE);
-memory_region_add_subregion_overlap(&amdvi_dev_as->root,
-AMDVI_INT_ADDR_FIRST,
-&amdvi_dev_as->iommu_ir,
-64);
 memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
 
MEMORY_REGION(&amdvi_dev_as->iommu),
 0);
@@ -1472,6 +1465,13 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
 memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
 &amdvi_dev_as->iommu_nodma,
 0);
+/* Build the Interrupt Remapping alias to shared memory */
+memory_region_init_alias(&amdvi_dev_as->iommu_ir, OBJECT(s),
+ "amdvi-ir", &s->mr_ir, 0,
+ memory_region_size(&s->mr_ir));
+
memory_region_add_subregion_overlap(MEMORY_REGION(&amdvi_dev_as->iommu),
+AMDVI_INT_ADDR_FIRST,
+&amdvi_dev_as->iommu_ir, 1);
 
 if (!x86_iommu->pt_supported) {
 memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
@@ -1633,6 +1633,12 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error 
**errp)
 memory_region_add_subregion_overlap(&s->mr_sys, 0,
 &s->mr_nodma, 0);
 
+/* set up the Interrupt Remapping memory region */
+memory_region_init_io(&s->mr_ir, OBJECT(s), &amdvi_ir_ops,
+  s, "amdvi-ir", AMDVI_INT_ADDR_SIZE);
+memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
+&s->mr_ir, 1);
+
 pci_setup_iommu(bus, &amdvi_iommu_ops, s);
 amdvi_init(s);
 }
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index be417e51c4dc..e0dac4d9a96c 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -356,6 +356,7 @@ struct AMDVIState {
 MemoryRegion mr_mmio;  /* MMIO region  */
 MemoryRegion mr_sys;
 MemoryRegion mr_nodma;
+MemoryRegion mr_ir;
 uint8_t mmior[AMDVI_MMIO_SIZE];/* read/write MMIO  */
 uint8_t w1cmask[AMDVI_MMIO_SIZE];  /* read/write 1 clear mask  */
 uint8_t romask[AMDVI_MMIO_SIZE];   /* MMIO read/only mask  */
-- 
2.43.5

Re: qemu emulation for USB ports of Allwinner H3

[Guenter Roeck]

On 9/13/24 15:20, Niek Linnenbank wrote:

Hello Guenter, Gerd,

Thanks for bringing up the question. To be honest I do not know a lot about USB 
internals.
When adding the orangepi-pc board emulation, it seemed fairly easy to add it, 
but apart from a few basic tests, I did not use the USB functionality 
extensively.

I do own the actual Orange Pi PC board hardware, so I downloaded the 
'Orangepipc_2.0.8_ubuntu_bionic_server_linux5.4.65.7z' image from the official 
page to test:
http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/service-and-support/Orange-Pi-PC.html
 


After booting that image from an SD card with a serial console, this is the 
output from the same 'lsusb' command:

root@orangepipc:~# lsusb
Bus 008 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 007 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 006 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 009 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
root@orangepipc:~# uname -a
Linux orangepipc 5.4.65-sunxi #2.0.8 SMP Mon Oct 26 10:20:38 CST 2020 armv7l 
armv7l armv7l GNU/Linux
root@orangepipc:~# cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.04
DISTRIB_CODENAME=bionic
DISTRIB_DESCRIPTION="Ubuntu 18.04.5 LTS"

It does look like the same output compared to what we see under Qemu. But I'm 
not very sure if this confirms we are emulating correctly or not.



Yes, I found that as well, and Gerd has reported the same. The main difference,
as far as I can see, is that the host sees different register contents.
I am not sure if that is worth fixing. Either case it is not my highest 
priority.

Thanks,
Guenter


Regards,
Niek

On Mon, Sep 9, 2024 at 11:33 AM Gerd Hoffmann mailto:kra...@redhat.com>> wrote:

On Sun, Sep 08, 2024 at 11:36:18AM GMT, Guenter Roeck wrote:
 > Hi,
 >
 > the Allwinner H3 USB port qemu emulation creates separate USB ports
 > for its EHCI and OHCI controllers, resulting in a total of 8 USB ports.
 > From the orangepi-pc emulation:
 >
 > # lsusb
 > Bus 005 Device 001: ID 1d6b:0002
 > Bus 003 Device 001: ID 1d6b:0002
 > Bus 001 Device 001: ID 1d6b:0002
 > Bus 008 Device 001: ID 1d6b:0002
 > Bus 006 Device 001: ID 1d6b:0001
 > Bus 004 Device 001: ID 1d6b:0001
 > Bus 002 Device 001: ID 1d6b:0002
 > Bus 009 Device 001: ID 1d6b:0001
 > Bus 007 Device 001: ID 1d6b:0001
 >
 > The SoC supports EHCI companion interfaces, and my understanding is that
 > it only has four physical USB ports. Does the real hardware instantiate
 > separate EHCI and OHCI interfaces (for a total of 8 USB ports), or does 
it
 > use the companion functionality ?

Well, on the guest side you'll see 8 ports even when using the companion
functionality.  Each physical usb port has one ehci port (used when you
plug in usb 2.0+ devices) and one ohci port (used when you plug in usb
1.1 devices).

The main difference is on the qemu backend side.  When using the
companion functionality you have a single qemu usb bus accepting both
1.1 and 2.0+ devices.  When not using the companion functionality you
have one usb bus accepting 2.0+ devices and another usb bus accepting
usb 1.1 devices ...

The guest-visible difference is an per-port bit in ehci registers which
controls whenever ehci or the companion manages the device plugged in.
This bit exists for backward compatibility (guests without ehci driver
can manage all devices via ohci, with usb 2.0+ devices being downgraded
to 1.1 compatibility mode then).

 > If the real hardware only instantiates four USB ports (or, in other 
words,
 > if it utilizes EHCI companion functionality), would it make sense to
 > reflect that in qemu ?

Yes.

take care,
   Gerd



--
Niek Linnenbank

[PATCH v2 0/5] Interrupt Remap support for emulated amd viommu

[Santosh Shukla]

Series adds following feature support for emulated amd vIOMMU
1) Pass Through(PT) mode
2) Interrupt Remapping(IR) mode

1) PT mode
Introducing the shared 'nodma' memory region that can be aliased
by all the devices in the PT mode. Shared memory with aliasing
approach will help run VM faster when lot of devices attached to
VM.

2) IR mode
Shared IR memory region with aliasing approach proposed for the
reason mentioned in 1). Also add support to invalidate Interrupt
remaping table(IRT).

Series based on ea9cdbcf3a0b8d5497cddf87990f1b39d8f3bb0a

Testing:
1. nvme/fio testing for VM with > 255 vCPU with xtsup=on and x2apic
enabled
2. Windows Server 2022 VM testing for > 255 vCPU.

Change History:

V2:
- Fixed non-kvm build issue (Reported by Michael Tsirkin)

V1:
- https://lore.kernel.org/all/20240904100257.184851-3-santosh.shu...@amd.com/T/


Suravee Suthikulpanit (5):
  amd_iommu: Rename variable mmio to mr_mmio
  amd_iommu: Add support for pass though mode
  amd_iommu: Use shared memory region for Interrupt Remapping
  amd_iommu: Send notification when invaldate interrupt entry cache
  amd_iommu: Check APIC ID > 255 for XTSup

 hw/i386/acpi-build.c |  4 +-
 hw/i386/amd_iommu.c  | 98 +++-
 hw/i386/amd_iommu.h  |  5 ++-
 3 files changed, 85 insertions(+), 22 deletions(-)

-- 
2.43.5

Re: [PATCH] qemu/ui: set swap interval explicitly when OpenGL is enabled

[Michael Tokarev]

On 11.09.2024 12:14, gert.wol...@collabora.com wrote:

From: Gert Wollny 

Before 176e3783f2ab (ui/sdl2: OpenGL window context)
SDL_CreateRenderer was called unconditionally setting
the swap interval to 0. Since SDL_CreateRenderer is now no
longer called when OpenGL is enabled, the swap interval is
no longer set explicitly and vsync handling depends on
the environment settings which may lead to a performance
regression with virgl as reported in
https://gitlab.com/qemu-project/qemu/-/issues/2565

Restore the old vsync handling by explicitly calling
SDL_GL_SetSwapInterval if OpenGL is enabled.

Fixes: 176e3783f2ab (ui/sdl2: OpenGL window context)
Closes: https://gitlab.com/qemu-project/qemu/-/issues/2565


Is it a qemu-stable material (8.2, 9.0, 9.1) ?

Picked it up for now, please notify me if I should drop it.

Also please notify me if I should pick some other changes
for -stable.

Thanks,

/mjt

Re: [PULL 0/4] Edk2 stable202408 20240916 patches

[Peter Maydell]

On Mon, 16 Sept 2024 at 13:28, Gerd Hoffmann  wrote:
>
> The following changes since commit ea9cdbcf3a0b8d5497cddf87990f1b39d8f3bb0a:
>
>   Merge tag 'hw-misc-20240913' of https://github.com/philmd/qemu into staging 
> (2024-09-15 18:27:40 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/kraxel/qemu.git 
> tags/edk2-stable202408-20240916-pull-request
>
> for you to fetch changes up to 2b759fbc9a70258a244f98da3415947dccc2702e:
>
>   add loongarch binaries for edk2-stable202408 (2024-09-16 14:21:34 +0200)
>
> 
> edk2: update to 2024-08 stable tag
>
> 

Hi; this fails CI:

https://gitlab.com/qemu-project/qemu/-/jobs/7835418334

449/942 qemu:qtest+qtest-aarch64 / qtest-aarch64/bios-tables-test
ERROR 50.07s killed by signal 6 SIGABRT
>>> MALLOC_PERTURB_=179 QTEST_QEMU_BINARY=./qemu-system-aarch64 
>>> PYTHON=/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/pyvenv/bin/python3
>>>  
>>> G_TEST_DBUS_DAEMON=/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/tests/dbus-vmstate-daemon.sh
>>>  QTEST_QEMU_IMG=./qemu-img 
>>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
>>> /home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/tests/qtest/bios-tables-test
>>>  --tap -k
― ✀ ―
stderr:
acpi-test: Warning! SSDT binary file mismatch. Actual
[aml:/tmp/aml-9FJ0T2], Expected
[aml:tests/data/acpi/aarch64/virt/SSDT.memhp].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU
from scratch and re-run tests with V=1 environment variable set**
ERROR:../tests/qtest/bios-tables-test.c:553:test_acpi_asl: assertion
failed: (all_tables_match)
(test program exited with status code -6)

thanks
-- PMM

[PATCH v1 1/4] xen: Expose handle_bufioreq in xen_register_ioreq

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Expose handle_bufioreq in xen_register_ioreq().
This is to allow machines to enable or disable buffered ioreqs.

No functional change since all callers still set it to
HVM_IOREQSRV_BUFIOREQ_ATOMIC.

Signed-off-by: Edgar E. Iglesias 
---
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/xen/xen-hvm-common.c | 100 +++-
 hw/xen/xen-pvh-common.c |   4 +-
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 5 files changed, 70 insertions(+), 44 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 4f6446600c..d3df488c48 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -614,7 +614,9 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
**ram_memory)
 
 state = g_new0(XenIOState, 1);
 
-xen_register_ioreq(state, max_cpus, &xen_memory_listener);
+xen_register_ioreq(state, max_cpus,
+   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   &xen_memory_listener);
 
 xen_is_stubdomain = xen_check_stubdomain(state->xenstore);
 
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 3a9d6f981b..d8824cccf1 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -667,6 +667,8 @@ static int xen_map_ioreq_server(XenIOState *state)
 xen_pfn_t ioreq_pfn;
 xen_pfn_t bufioreq_pfn;
 evtchn_port_t bufioreq_evtchn;
+unsigned long num_frames = 1;
+unsigned long frame = 1;
 int rc;
 
 /*
@@ -675,59 +677,72 @@ static int xen_map_ioreq_server(XenIOState *state)
  */
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_bufioreq != 0);
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_ioreq(0) != 1);
+
+if (state->has_bufioreq) {
+frame = 0;
+num_frames = 2;
+}
 state->fres = xenforeignmemory_map_resource(xen_fmem, xen_domid,
  XENMEM_resource_ioreq_server,
- state->ioservid, 0, 2,
+ state->ioservid,
+ frame, num_frames,
  &addr,
  PROT_READ | PROT_WRITE, 0);
 if (state->fres != NULL) {
 trace_xen_map_resource_ioreq(state->ioservid, addr);
-state->buffered_io_page = addr;
-state->shared_page = addr + XC_PAGE_SIZE;
+state->shared_page = addr;
+if (state->has_bufioreq) {
+state->buffered_io_page = addr;
+state->shared_page = addr + XC_PAGE_SIZE;
+}
 } else if (errno != EOPNOTSUPP) {
 error_report("failed to map ioreq server resources: error %d 
handle=%p",
  errno, xen_xc);
 return -1;
 }
 
-rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
-   (state->shared_page == NULL) ?
-   &ioreq_pfn : NULL,
-   (state->buffered_io_page == NULL) ?
-   &bufioreq_pfn : NULL,
-   &bufioreq_evtchn);
-if (rc < 0) {
-error_report("failed to get ioreq server info: error %d handle=%p",
- errno, xen_xc);
-return rc;
-}
-
-if (state->shared_page == NULL) {
-trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
+if (state->has_bufioreq) {
+rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
+   (state->shared_page == NULL) ?
+   &ioreq_pfn : NULL,
+   (state->buffered_io_page == NULL) ?
+   &bufioreq_pfn : NULL,
+   &bufioreq_evtchn);
+if (rc < 0) {
+error_report("failed to get ioreq server info: error %d handle=%p",
+errno, xen_xc);
+return rc;
+}
 
-state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
-  PROT_READ | PROT_WRITE,
-  1, &ioreq_pfn, NULL);
 if (state->shared_page == NULL) {
-error_report("map shared IO page returned error %d handle=%p",
- errno, xen_xc);
+trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
+
+state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
+  PROT_READ | PROT_WRITE,
+  1, &ioreq_pfn, NULL);
+if (state->shared_page == NULL) {
+error_report("map shared IO page returned error %d handle=%p",
+errno, xen_xc);
+}
 }
-}
 
-if (state->buffered_io_page == N

[PATCH v1 2/4] hw/xen: xenpvh: Disable buffered IOREQs for ARM

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add a way to enable/disable buffered IOREQs for PVH machines
and disable them for ARM. ARM does not support buffered
IOREQ's nor the legacy way to map IOREQ info pages.

See the following for more details:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=2fbd7e609e1803ac5e5c26e22aa8e4b5a6cddbb1
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/ioreq.c;h=2e829d2e7f3760401b96fa7c930e2015fb1cf463;hb=HEAD#l138

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c| 3 +++
 hw/i386/xen/xen-pvh.c   | 3 +++
 hw/xen/xen-pvh-common.c | 2 +-
 include/hw/xen/xen-pvh-common.h | 3 +++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 04cb9855af..28af3910ea 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -66,6 +66,9 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
  */
 mc->max_cpus = GUEST_MAX_VCPUS;
 
+/* Xen/ARM does not use buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
+
 /* List of supported features known to work on PVH ARM.  */
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
index 45645667e9..f1f02d3311 100644
--- a/hw/i386/xen/xen-pvh.c
+++ b/hw/i386/xen/xen-pvh.c
@@ -89,6 +89,9 @@ static void xen_pvh_machine_class_init(ObjectClass *oc, void 
*data)
 /* We have an implementation specific init to create CPU objects.  */
 xpc->init = xen_pvh_init;
 
+/* Enable buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_ATOMIC;
+
 /*
  * PCI INTX routing.
  *
diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 08641fdcec..76a9b2b945 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -195,7 +195,7 @@ static void xen_pvh_init(MachineState *ms)
 
 xen_pvh_init_ram(s, sysmem);
 xen_register_ioreq(&s->ioreq, ms->smp.max_cpus,
-   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   xpc->handle_bufioreq,
&xen_memory_listener);
 
 if (s->cfg.virtio_mmio_num) {
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index bc09eea936..62c44a1ce7 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -43,6 +43,9 @@ struct XenPVHMachineClass {
  */
 int (*set_pci_link_route)(uint8_t line, uint8_t irq);
 
+/* Allow implementations to optionally enable buffered ioreqs.  */
+int handle_bufioreq;
+
 /*
  * Each implementation can optionally enable features that it
  * supports and are known to work.
-- 
2.43.0

[PATCH v1 0/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI on the ARM PVH machine. First we add a way to control the use
of buffered IOREQ's since those are not supported on Xen/ARM.
Finally we enable the PCI support.

I've published some instructions on how to try this including the work in
progress Xen side of the PVH PCI support:
https://github.com/edgarigl/docs/blob/master/xen/pvh/virtio-pci-dom0less.md

Cheers,
Edgar

Edgar E. Iglesias (4):
  xen: Expose handle_bufioreq in xen_register_ioreq
  hw/xen: xenpvh: Disable buffered IOREQs for ARM
  hw/xen: xenpvh: Add pci-intx-irq-base property
  hw/arm: xenpvh: Enable PCI for ARM PVH

 hw/arm/xen-pvh.c|  17 ++
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/i386/xen/xen-pvh.c   |   3 +
 hw/xen/xen-hvm-common.c | 100 +++-
 hw/xen/xen-pvh-common.c |  40 -
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen-pvh-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 8 files changed, 129 insertions(+), 44 deletions(-)

-- 
2.43.0

[PATCH v1 4/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI support for the ARM Xen PVH machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 28af3910ea..33f0dd5982 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -39,6 +39,16 @@ static void xen_arm_instance_init(Object *obj)
  VIRTIO_MMIO_DEV_SIZE };
 }
 
+static void xen_pvh_set_pci_intx_irq(void *opaque, int intx_irq, int level)
+{
+XenPVHMachineState *s = XEN_PVH_MACHINE(opaque);
+int irq = s->cfg.pci_intx_irq_base + intx_irq;
+
+if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
 static void xen_arm_machine_class_init(ObjectClass *oc, void *data)
 {
 XenPVHMachineClass *xpc = XEN_PVH_MACHINE_CLASS(oc);
@@ -69,7 +79,11 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
 /* Xen/ARM does not use buffered IOREQs.  */
 xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
 
+/* PCI INTX delivery.  */
+xpc->set_pci_intx_irq = xen_pvh_set_pci_intx_irq;
+
 /* List of supported features known to work on PVH ARM.  */
+xpc->has_pci = true;
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
 
-- 
2.43.0

[PATCH v1 3/4] hw/xen: xenpvh: Add pci-intx-irq-base property

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 76a9b2b945..218ac851cf 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -218,6 +218,11 @@ static void xen_pvh_init(MachineState *ms)
 error_report("pci-ecam-size only supports values 0 or 0x1000");
 exit(EXIT_FAILURE);
 }
+if (!s->cfg.pci_intx_irq_base) {
+error_report("PCI enabled but pci-intx-irq-base not set");
+exit(EXIT_FAILURE);
+}
+
 xenpvh_gpex_init(s, xpc, sysmem);
 }
 
@@ -273,6 +278,30 @@ XEN_PVH_PROP_MEMMAP(pci_ecam)
 XEN_PVH_PROP_MEMMAP(pci_mmio)
 XEN_PVH_PROP_MEMMAP(pci_mmio_high)
 
+static void xen_pvh_set_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value;
+
+if (!visit_type_uint32(v, name, &value, errp)) {
+return;
+}
+
+xp->cfg.pci_intx_irq_base = value;
+}
+
+static void xen_pvh_get_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value = xp->cfg.pci_intx_irq_base;
+
+visit_type_uint32(v, name, &value, errp);
+}
+
 void xen_pvh_class_setup_common_props(XenPVHMachineClass *xpc)
 {
 ObjectClass *oc = OBJECT_CLASS(xpc);
@@ -318,6 +347,13 @@ do {   
   \
 OC_MEMMAP_PROP(oc, "pci-ecam", pci_ecam);
 OC_MEMMAP_PROP(oc, "pci-mmio", pci_mmio);
 OC_MEMMAP_PROP(oc, "pci-mmio-high", pci_mmio_high);
+
+object_class_property_add(oc, "pci-intx-irq-base", "uint32_t",
+  xen_pvh_get_pci_intx_irq_base,
+  xen_pvh_set_pci_intx_irq_base,
+  NULL, NULL);
+object_class_property_set_description(oc, "pci-intx-irq-base",
+  "Set PCI INTX interrupt base line.");
 }
 
 #ifdef CONFIG_TPM
-- 
2.43.0

Re: [External] Re: [PATCH v5 08/13] migration/multifd: Add new migration option for multifd DSA offloading.

[Fabiano Rosas]

Yichen Wang  writes:

> On Wed, Jul 24, 2024 at 7:50 AM Markus Armbruster  wrote:
>>
>> Fabiano Rosas  writes:
>>
>> > Yichen Wang  writes:
>> >
>> >> On Thu, Jul 11, 2024 at 2:53 PM Yichen Wang  
>> >> wrote:
>> >>
>> >>> diff --git a/migration/options.c b/migration/options.c
>> >>> index 645f55003d..f839493016 100644
>> >>> --- a/migration/options.c
>> >>> +++ b/migration/options.c
>> >>> @@ -29,6 +29,7 @@
>> >>>  #include "ram.h"
>> >>>  #include "options.h"
>> >>>  #include "sysemu/kvm.h"
>> >>> +#include 
>> >>>
>> >>>  /* Maximum migrate downtime set to 2000 seconds */
>> >>>  #define MAX_MIGRATE_DOWNTIME_SECONDS 2000
>> >>> @@ -162,6 +163,10 @@ Property migration_properties[] = {
>> >>>  DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", 
>> >>> MigrationState,
>> >>> parameters.zero_page_detection,
>> >>> ZERO_PAGE_DETECTION_MULTIFD),
>> >>> +/* DEFINE_PROP_ARRAY("dsa-accel-path", MigrationState, x, */
>> >>> +/*parameters.dsa_accel_path, qdev_prop_string, 
>> >>> char *), */
>> >
>> > This is mostly correct, I think, you just need to create a field in
>> > MigrationState to keep the length (instead of x). However, I found out
>> > just now that this only works with QMP. Let me ask for other's
>> > opinions...
>> >
>> >>> +/* DEFINE_PROP_STRING("dsa-accel-path", MigrationState, */
>> >>> +/*parameters.dsa_accel_path), */
>> >>>
>> >>>  /* Migration capabilities */
>> >>>  DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
>> >>
>> >> I changed the dsa-accel-path to be a ['str'], i.e. strList* in C.
>> >> However, I am having a hard time about how to define the proper
>> >> properties here. I don't know what MACRO to use and I can't find good
>> >> examples... Need some guidance about how to proceed. Basically I will
>> >> need this to pass something like '-global
>> >> migration.dsa-accel-path="/dev/dsa/wq0.0"' in cmdline, or
>> >> "migrate_set_parameter dsa-accel-path" in QEMU CLI. Don't know how to
>> >> pass strList there.
>> >>
>> >> Thanks very much!
>> >
>> > @Daniel, @Markus, any idea here?
>> >
>> > If I'm reading this commit[1] right, it seems we decided to disallow
>> > passing of arrays without JSON, which affects -global on the
>> > command-line and HMP.
>> >
>> > 1- b06f8b500d (qdev: Rework array properties based on list visitor,
>> > 2023-11-09)
>> >
>> > QMP shell:
>> > (QEMU) migrate-set-parameters dsa-accel-path=['a','b']
>> > {"return": {}}
>> >
>> > HMP:
>> > (qemu) migrate_set_parameter dsa-accel-path "['a','b']"
>> > qemu-system-x86_64: ../qapi/string-input-visitor.c:343: parse_type_str:
>> > Assertion `siv->lm == LM_NONE' failed.
>>
>> HMP migrate_set_parameter doesn't support JSON.  It uses the string
>> input visitor to parse the value, which can only do lists of integers.
>>
>> The string visitors have been thorns in my side since forever.
>>
>> > Any recommendation? I believe all migration parameters so far can be set
>> > via those means, I don't think we can allow only this one to be
>> > QMP-only.
>> >
>> > Or am I just missing something?
>>
>> I don't think the string input visitor can be compatibly extended to
>> arbitrary lists.
>>
>> We could replace HMP migrate_set_parameter by migrate_set_parameters.
>> The new command parses its single argument into a struct
>> MigrateSetParameters with keyval_parse(),
>> qobject_input_visitor_new_keyval(), and
>> visit_type_MigrateSetParameters().
>>
>
> I tried Fabiano's suggestion, and put a unit32_t in MigrateState data
> structure. I got exactly the same: "qemu-system-x86_64.dsa:
> ../../../qapi/string-input-visitor.c:343: parse_type_str: Assertion
> `siv->lm == LM_NONE' failed.". Steve's patch is more to be a read-only
> field from HMP, so probably I can't do that.

What do you mean by read-only field? I thought his usage was the same as
what we want for dsa-accel-path:

(qemu) migrate_set_parameter cpr-exec-command abc def
(qemu) info migrate_parameters 
...
cpr-exec-command: abc def

(gdb) p valuestr
$3 = 0x5766a8d0 "abc def"
(gdb) p *p->cpr_exec_command 
$6 = {next = 0x5823d300, value = 0x5765f690 "abc"}
(gdb) p *p->cpr_exec_command.next
$7 = {next = 0x5805be20, value = 0x57fefc80 "def"}

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Alex Bennée]

Brian Cain  writes:

> On 9/16/2024 8:12 AM, Alex Bennée wrote:
>> Brian Cain  writes:
>>
>>> On 9/6/2024 9:39 PM, Brian Cain wrote:
 With newer clang builds (19.x), there's a warning for implicit function
 declarations and it rejects linux-test.c.

 glibc/musl's readdir64() declaration in dirent is guarded by
 _LARGEFILE64_SOURCE, so we'll define it to fix the warning.

 BUILD   hexagon-linux-user guest-tests
   
 /local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
  error: call to undeclared function 'readdir64'; ISO C99 and later do not 
 support implicit function declarations [-Wimplicit-function-declaration]
 189 | de = readdir64(dir);
 |  ^

 Signed-off-by: Brian Cain 
 ---
tests/tcg/multiarch/linux/linux-test.c | 1 +
1 file changed, 1 insertion(+)

 diff --git a/tests/tcg/multiarch/linux/linux-test.c 
 b/tests/tcg/multiarch/linux/linux-test.c
 index 64f57cb287..4e0e862ad9 100644
 --- a/tests/tcg/multiarch/linux/linux-test.c
 +++ b/tests/tcg/multiarch/linux/linux-test.c
 @@ -17,6 +17,7 @@
 *  along with this program; if not, see .
 */
#define _GNU_SOURCE
 +#define _LARGEFILE64_SOURCE
#include 
#include 
#include 
>>>
>>> Alex -- what do you think about this one?
>> Actually scratch that, this is a 32 compat hack:
>>
>>1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit host)
>>
>> Is the __USE_LARGEFILE64 symbol in the hexagon headers?
>>
> musl does not define/use __USE_LARGEFILE64, no.  If it's well defined
> I could examine whether it makes sense to add this feature to musl,
> though.  How does __USE_LARGEFILE64 differ from _LARGEFILE64_SOURCE? 
> Is it more appropriate to define that here?

Digging into the GNU source _LARGEFILE* is the correct define, the __USE
flags are internal. features.h says:

   _LARGEFILE_SOURCESome more functions for correct standard I/O.
   _LARGEFILE64_SOURCE  Additional functionality from LFS for large files.

although looking at _LARGEFILE64_SOURCE should be defined by
_GNU_SOURCE which is already set for linux-test.c

According to the musl WHATSNEW:

  compatibility:
  - make _GNU_SOURCE imply _LARGEFILE64_SOURCE

So is this a hexagon only thing?

>
> -Brian

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v7 05/17] bsd-user: Add RISC-V ELF definitions and hardware capability detection

[Ajeet Singh]

From: Mark Corbin 

Introduced RISC-V specific ELF definitions and hardware capability
detection.
Additionally, a function to retrieve hardware capabilities
('get_elf_hwcap') is implemented, which returns the common bits set in
each CPU's ISA strings.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Kyle Evans 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_elf.h | 42 
 1 file changed, 42 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_elf.h

diff --git a/bsd-user/riscv/target_arch_elf.h b/bsd-user/riscv/target_arch_elf.h
new file mode 100644
index 00..4eb915e61e
--- /dev/null
+++ b/bsd-user/riscv/target_arch_elf.h
@@ -0,0 +1,42 @@
+/*
+ *  RISC-V ELF definitions
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_ELF_H
+#define TARGET_ARCH_ELF_H
+
+#define elf_check_arch(x) ((x) == EM_RISCV)
+#define ELF_START_MMAP 0x8000
+#define ELF_ET_DYN_LOAD_ADDR0x10
+#define ELF_CLASS   ELFCLASS64
+
+#define ELF_DATAELFDATA2LSB
+#define ELF_ARCHEM_RISCV
+
+#define ELF_HWCAP get_elf_hwcap()
+static uint32_t get_elf_hwcap(void)
+{
+RISCVCPU *cpu = RISCV_CPU(thread_cpu);
+
+return cpu->env.misa_ext_mask;
+}
+
+#define USE_ELF_CORE_DUMP
+#define ELF_EXEC_PAGESIZE4096
+
+#endif /* TARGET_ARCH_ELF_H */
-- 
2.34.1

[PATCH v7 02/17] bsd-user: Add RISC-V CPU execution loop and syscall handling

[Ajeet Singh]

From: Mark Corbin 

Implemented the RISC-V CPU execution loop, including handling various
exceptions and system calls. The loop continuously executes CPU
instructions,processes exceptions, and handles system calls by invoking
FreeBSD syscall handlers.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Jessica Clarke 
Co-authored-by: Kyle Evans 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_cpu.h | 94 
 1 file changed, 94 insertions(+)

diff --git a/bsd-user/riscv/target_arch_cpu.h b/bsd-user/riscv/target_arch_cpu.h
index f8d85e01ad..9c31d9dc4c 100644
--- a/bsd-user/riscv/target_arch_cpu.h
+++ b/bsd-user/riscv/target_arch_cpu.h
@@ -37,4 +37,98 @@ static inline void target_cpu_init(CPURISCVState *env,
 env->pc = regs->sepc;
 }
 
+static inline void target_cpu_loop(CPURISCVState *env)
+{
+CPUState *cs = env_cpu(env);
+int trapnr;
+abi_long ret;
+unsigned int syscall_num;
+int32_t signo, code;
+
+for (;;) {
+cpu_exec_start(cs);
+trapnr = cpu_exec(cs);
+cpu_exec_end(cs);
+process_queued_cpu_work(cs);
+
+signo = 0;
+
+switch (trapnr) {
+case EXCP_INTERRUPT:
+/* just indicate that signals should be handled asap */
+break;
+case EXCP_ATOMIC:
+cpu_exec_step_atomic(cs);
+break;
+case RISCV_EXCP_U_ECALL:
+syscall_num = env->gpr[xT0];
+env->pc += TARGET_INSN_SIZE;
+/* Compare to cpu_fetch_syscall_args() in riscv/riscv/trap.c */
+if (TARGET_FREEBSD_NR___syscall == syscall_num ||
+TARGET_FREEBSD_NR_syscall == syscall_num) {
+ret = do_freebsd_syscall(env,
+ env->gpr[xA0],
+ env->gpr[xA1],
+ env->gpr[xA2],
+ env->gpr[xA3],
+ env->gpr[xA4],
+ env->gpr[xA5],
+ env->gpr[xA6],
+ env->gpr[xA7],
+ 0);
+} else {
+ret = do_freebsd_syscall(env,
+ syscall_num,
+ env->gpr[xA0],
+ env->gpr[xA1],
+ env->gpr[xA2],
+ env->gpr[xA3],
+ env->gpr[xA4],
+ env->gpr[xA5],
+ env->gpr[xA6],
+ env->gpr[xA7]
+);
+}
+
+/*
+ * Compare to cpu_set_syscall_retval() in
+ * riscv/riscv/vm_machdep.c
+ */
+if (ret >= 0) {
+env->gpr[xA0] = ret;
+env->gpr[xT0] = 0;
+} else if (ret == -TARGET_ERESTART) {
+env->pc -= TARGET_INSN_SIZE;
+} else if (ret != -TARGET_EJUSTRETURN) {
+env->gpr[xA0] = -ret;
+env->gpr[xT0] = 1;
+}
+break;
+case RISCV_EXCP_ILLEGAL_INST:
+signo = TARGET_SIGILL;
+code = TARGET_ILL_ILLOPC;
+break;
+case RISCV_EXCP_BREAKPOINT:
+signo = TARGET_SIGTRAP;
+code = TARGET_TRAP_BRKPT;
+break;
+case EXCP_DEBUG:
+signo = TARGET_SIGTRAP;
+code = TARGET_TRAP_BRKPT;
+break;
+default:
+fprintf(stderr, "qemu: unhandled CPU exception "
+"0x%x - aborting\n", trapnr);
+cpu_dump_state(cs, stderr, 0);
+abort();
+}
+
+if (signo) {
+force_sig_fault(signo, code, env->pc);
+}
+
+process_pending_signals(env);
+}
+}
+
 #endif /* TARGET_ARCH_CPU_H */
-- 
2.34.1

[PATCH v7 06/17] bsd-user: Define RISC-V register structures and register copying

[Ajeet Singh]

From: Mark Corbin 

Added definitions for RISC-V register structures, including
general-purpose registers and floating-point registers, in
'target_arch_reg.h'. Implemented the 'target_copy_regs' function to
copy register values from the CPU state to the target register
structure, ensuring proper endianness handling using 'tswapreg'.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_reg.h | 88 
 1 file changed, 88 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_reg.h

diff --git a/bsd-user/riscv/target_arch_reg.h b/bsd-user/riscv/target_arch_reg.h
new file mode 100644
index 00..12b1c96b61
--- /dev/null
+++ b/bsd-user/riscv/target_arch_reg.h
@@ -0,0 +1,88 @@
+/*
+ *  RISC-V register structures
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_REG_H
+#define TARGET_ARCH_REG_H
+
+/* Compare with riscv/include/reg.h */
+typedef struct target_reg {
+uint64_t ra;/* return address */
+uint64_t sp;/* stack pointer */
+uint64_t gp;/* global pointer */
+uint64_t tp;/* thread pointer */
+uint64_t t[7];  /* temporaries */
+uint64_t s[12]; /* saved registers */
+uint64_t a[8];  /* function arguments */
+uint64_t sepc;  /* exception program counter */
+uint64_t sstatus;   /* status register */
+} target_reg_t;
+
+typedef struct target_fpreg {
+uint64_tfp_x[32][2];/* Floating point registers */
+uint64_tfp_fcsr;/* Floating point control reg */
+} target_fpreg_t;
+
+#define tswapreg(ptr)   tswapal(ptr)
+
+/* Compare with struct trapframe in riscv/include/frame.h */
+static inline void target_copy_regs(target_reg_t *regs,
+const CPURISCVState *env)
+{
+
+regs->ra = tswapreg(env->gpr[1]);
+regs->sp = tswapreg(env->gpr[2]);
+regs->gp = tswapreg(env->gpr[3]);
+regs->tp = tswapreg(env->gpr[4]);
+
+regs->t[0] = tswapreg(env->gpr[5]);
+regs->t[1] = tswapreg(env->gpr[6]);
+regs->t[2] = tswapreg(env->gpr[7]);
+regs->t[3] = tswapreg(env->gpr[28]);
+regs->t[4] = tswapreg(env->gpr[29]);
+regs->t[5] = tswapreg(env->gpr[30]);
+regs->t[6] = tswapreg(env->gpr[31]);
+
+regs->s[0] = tswapreg(env->gpr[8]);
+regs->s[1] = tswapreg(env->gpr[9]);
+regs->s[2] = tswapreg(env->gpr[18]);
+regs->s[3] = tswapreg(env->gpr[19]);
+regs->s[4] = tswapreg(env->gpr[20]);
+regs->s[5] = tswapreg(env->gpr[21]);
+regs->s[6] = tswapreg(env->gpr[22]);
+regs->s[7] = tswapreg(env->gpr[23]);
+regs->s[8] = tswapreg(env->gpr[24]);
+regs->s[9] = tswapreg(env->gpr[25]);
+regs->s[10] = tswapreg(env->gpr[26]);
+regs->s[11] = tswapreg(env->gpr[27]);
+
+regs->a[0] = tswapreg(env->gpr[10]);
+regs->a[1] = tswapreg(env->gpr[11]);
+regs->a[2] = tswapreg(env->gpr[12]);
+regs->a[3] = tswapreg(env->gpr[13]);
+regs->a[4] = tswapreg(env->gpr[14]);
+regs->a[5] = tswapreg(env->gpr[15]);
+regs->a[6] = tswapreg(env->gpr[16]);
+regs->a[7] = tswapreg(env->gpr[17]);
+
+regs->sepc = tswapreg(env->pc);
+}
+
+#undef tswapreg
+
+#endif /* TARGET_ARCH_REG_H */
-- 
2.34.1

[PATCH v7 03/17] bsd-user: Implement RISC-V CPU register cloning and reset functions

[Ajeet Singh]

From: Mark Corbin 

Added functions for cloning CPU registers and resetting the CPU state
for RISC-V architecture.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_cpu.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/bsd-user/riscv/target_arch_cpu.h b/bsd-user/riscv/target_arch_cpu.h
index 9c31d9dc4c..a93ea3915a 100644
--- a/bsd-user/riscv/target_arch_cpu.h
+++ b/bsd-user/riscv/target_arch_cpu.h
@@ -131,4 +131,18 @@ static inline void target_cpu_loop(CPURISCVState *env)
 }
 }
 
+static inline void target_cpu_clone_regs(CPURISCVState *env, target_ulong 
newsp)
+{
+if (newsp) {
+env->gpr[xSP] = newsp;
+}
+
+env->gpr[xA0] = 0;
+env->gpr[xT0] = 0;
+}
+
+static inline void target_cpu_reset(CPUArchState *env)
+{
+}
+
 #endif /* TARGET_ARCH_CPU_H */
-- 
2.34.1

[PATCH v7 09/17] bsd-user: Add RISC-V thread setup and initialization support

[Ajeet Singh]

From: Mark Corbin 

Implemented functions for setting up and initializing threads in the
RISC-V architecture.
The 'target_thread_set_upcall' function sets up the stack pointer,
program counter, and function argument for new threads.
The 'target_thread_init' function initializes thread registers based on
the provided image information.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Jessica Clarke 
Co-authored-by: Kyle Evans 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_thread.h | 47 +
 1 file changed, 47 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_thread.h

diff --git a/bsd-user/riscv/target_arch_thread.h 
b/bsd-user/riscv/target_arch_thread.h
new file mode 100644
index 00..95cd0b6ad7
--- /dev/null
+++ b/bsd-user/riscv/target_arch_thread.h
@@ -0,0 +1,47 @@
+/*
+ *  RISC-V thread support
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_THREAD_H
+#define TARGET_ARCH_THREAD_H
+
+/* Compare with cpu_set_upcall() in riscv/riscv/vm_machdep.c */
+static inline void target_thread_set_upcall(CPURISCVState *regs,
+abi_ulong entry, abi_ulong arg, abi_ulong stack_base,
+abi_ulong stack_size)
+{
+abi_ulong sp;
+
+sp = ROUND_DOWN(stack_base + stack_size, 16);
+
+regs->gpr[xSP] = sp;
+regs->pc = entry;
+regs->gpr[xA0] = arg;
+}
+
+/* Compare with exec_setregs() in riscv/riscv/machdep.c */
+static inline void target_thread_init(struct target_pt_regs *regs,
+struct image_info *infop)
+{
+regs->sepc = infop->entry;
+regs->regs[xRA] = infop->entry;
+regs->regs[xA0] = infop->start_stack;
+regs->regs[xSP] = ROUND_DOWN(infop->start_stack, 16);
+}
+
+#endif /* TARGET_ARCH_THREAD_H */
-- 
2.34.1

[PATCH v7 15/17] bsd-user: Implement 'get_mcontext' for RISC-V

[Ajeet Singh]

From: Mark Corbin 

Added the 'get_mcontext' function to extract and populate
the RISC-V machine context from the CPU state.
This function is used to gather the current state of the
general-purpose registers and store it in a 'target_mcontext_'
structure.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Signed-off-by: Warner Losh 
Co-authored-by: Warner Losh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/signal.c | 53 +
 1 file changed, 53 insertions(+)

diff --git a/bsd-user/riscv/signal.c b/bsd-user/riscv/signal.c
index 2597fec2fd..072ad821d2 100644
--- a/bsd-user/riscv/signal.c
+++ b/bsd-user/riscv/signal.c
@@ -61,3 +61,56 @@ abi_long setup_sigframe_arch(CPURISCVState *env, abi_ulong 
frame_addr,
 get_mcontext(env, mcp, flags);
 return 0;
 }
+
+/*
+ * Compare with get_mcontext() in riscv/riscv/machdep.c
+ * Assumes that the memory is locked if mcp points to user memory.
+ */
+abi_long get_mcontext(CPURISCVState *regs, target_mcontext_t *mcp,
+int flags)
+{
+
+mcp->mc_gpregs.gp_t[0] = tswap64(regs->gpr[5]);
+mcp->mc_gpregs.gp_t[1] = tswap64(regs->gpr[6]);
+mcp->mc_gpregs.gp_t[2] = tswap64(regs->gpr[7]);
+mcp->mc_gpregs.gp_t[3] = tswap64(regs->gpr[28]);
+mcp->mc_gpregs.gp_t[4] = tswap64(regs->gpr[29]);
+mcp->mc_gpregs.gp_t[5] = tswap64(regs->gpr[30]);
+mcp->mc_gpregs.gp_t[6] = tswap64(regs->gpr[31]);
+
+mcp->mc_gpregs.gp_s[0] = tswap64(regs->gpr[8]);
+mcp->mc_gpregs.gp_s[1] = tswap64(regs->gpr[9]);
+mcp->mc_gpregs.gp_s[2] = tswap64(regs->gpr[18]);
+mcp->mc_gpregs.gp_s[3] = tswap64(regs->gpr[19]);
+mcp->mc_gpregs.gp_s[4] = tswap64(regs->gpr[20]);
+mcp->mc_gpregs.gp_s[5] = tswap64(regs->gpr[21]);
+mcp->mc_gpregs.gp_s[6] = tswap64(regs->gpr[22]);
+mcp->mc_gpregs.gp_s[7] = tswap64(regs->gpr[23]);
+mcp->mc_gpregs.gp_s[8] = tswap64(regs->gpr[24]);
+mcp->mc_gpregs.gp_s[9] = tswap64(regs->gpr[25]);
+mcp->mc_gpregs.gp_s[10] = tswap64(regs->gpr[26]);
+mcp->mc_gpregs.gp_s[11] = tswap64(regs->gpr[27]);
+
+mcp->mc_gpregs.gp_a[0] = tswap64(regs->gpr[10]);
+mcp->mc_gpregs.gp_a[1] = tswap64(regs->gpr[11]);
+mcp->mc_gpregs.gp_a[2] = tswap64(regs->gpr[12]);
+mcp->mc_gpregs.gp_a[3] = tswap64(regs->gpr[13]);
+mcp->mc_gpregs.gp_a[4] = tswap64(regs->gpr[14]);
+mcp->mc_gpregs.gp_a[5] = tswap64(regs->gpr[15]);
+mcp->mc_gpregs.gp_a[6] = tswap64(regs->gpr[16]);
+mcp->mc_gpregs.gp_a[7] = tswap64(regs->gpr[17]);
+
+if (flags & TARGET_MC_GET_CLEAR_RET) {
+mcp->mc_gpregs.gp_a[0] = 0; /* a0 */
+mcp->mc_gpregs.gp_a[1] = 0; /* a1 */
+mcp->mc_gpregs.gp_t[0] = 0; /* clear syscall error */
+}
+
+mcp->mc_gpregs.gp_ra = tswap64(regs->gpr[1]);
+mcp->mc_gpregs.gp_sp = tswap64(regs->gpr[2]);
+mcp->mc_gpregs.gp_gp = tswap64(regs->gpr[3]);
+mcp->mc_gpregs.gp_tp = tswap64(regs->gpr[4]);
+mcp->mc_gpregs.gp_sepc = tswap64(regs->pc);
+
+return 0;
+}
-- 
2.34.1

[PATCH v7 04/17] bsd-user: Implement RISC-V TLS register setup

[Ajeet Singh]

From: Mark Corbin 

Included the prototype for the 'target_cpu_set_tls' function in the
'target_arch.h' header file. This function is responsible for setting
the Thread Local Storage (TLS) register for RISC-V architecture.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch.h | 27 +++
 bsd-user/riscv/target_arch_cpu.c | 29 +
 2 files changed, 56 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch.h
 create mode 100644 bsd-user/riscv/target_arch_cpu.c

diff --git a/bsd-user/riscv/target_arch.h b/bsd-user/riscv/target_arch.h
new file mode 100644
index 00..26ce07f343
--- /dev/null
+++ b/bsd-user/riscv/target_arch.h
@@ -0,0 +1,27 @@
+/*
+ * RISC-V specific prototypes
+ *
+ * Copyright (c) 2019 Mark Corbin 
+ *
+ * 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 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 .
+ */
+
+#ifndef TARGET_ARCH_H
+#define TARGET_ARCH_H
+
+#include "qemu.h"
+
+void target_cpu_set_tls(CPURISCVState *env, target_ulong newtls);
+
+#endif /* TARGET_ARCH_H */
diff --git a/bsd-user/riscv/target_arch_cpu.c b/bsd-user/riscv/target_arch_cpu.c
new file mode 100644
index 00..44e25d2ddf
--- /dev/null
+++ b/bsd-user/riscv/target_arch_cpu.c
@@ -0,0 +1,29 @@
+/*
+ *  RISC-V CPU related code
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+
+#include "target_arch.h"
+
+#define TP_OFFSET   16
+
+/* Compare with cpu_set_user_tls() in riscv/riscv/vm_machdep.c */
+void target_cpu_set_tls(CPURISCVState *env, target_ulong newtls)
+{
+env->gpr[xTP] = newtls + TP_OFFSET;
+}
-- 
2.34.1

[PATCH v7 11/17] bsd-user: Define RISC-V system call structures and constants

[Ajeet Singh]

From: Mark Corbin 

Introduced definitions for the RISC-V system call interface, including
the 'target_pt_regs' structure that outlines the register storage
layout during a system call.
Added constants for hardware machine identifiers.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Jessica Clarke 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_syscall.h | 38 +
 1 file changed, 38 insertions(+)
 create mode 100644 bsd-user/riscv/target_syscall.h

diff --git a/bsd-user/riscv/target_syscall.h b/bsd-user/riscv/target_syscall.h
new file mode 100644
index 00..e7e5231309
--- /dev/null
+++ b/bsd-user/riscv/target_syscall.h
@@ -0,0 +1,38 @@
+/*
+ *  RISC-V system call definitions
+ *
+ *  Copyright (c) Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef BSD_USER_RISCV_TARGET_SYSCALL_H
+#define BSD_USER_RISCV_TARGET_SYSCALL_H
+
+/*
+ * struct target_pt_regs defines the way the registers are stored on the stack
+ * during a system call.
+ */
+
+struct target_pt_regs {
+abi_ulong regs[32];
+abi_ulong sepc;
+};
+
+#define UNAME_MACHINE "riscv64"
+
+#define TARGET_HW_MACHINE   "riscv"
+#define TARGET_HW_MACHINE_ARCH  UNAME_MACHINE
+
+#endif /* BSD_USER_RISCV_TARGET_SYSCALL_H */
-- 
2.34.1

[PATCH v7 08/17] bsd-user: Implement RISC-V sysarch system call emulation

[Ajeet Singh]

From: Mark Corbin 

Added the 'do_freebsd_arch_sysarch' function to emulate the 'sysarch'
system call for the RISC-V architecture.
Currently, this function returns '-TARGET_EOPNOTSUPP' to indicate that
the operation is not supported.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_sysarch.h | 41 
 1 file changed, 41 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_sysarch.h

diff --git a/bsd-user/riscv/target_arch_sysarch.h 
b/bsd-user/riscv/target_arch_sysarch.h
new file mode 100644
index 00..9af42331b4
--- /dev/null
+++ b/bsd-user/riscv/target_arch_sysarch.h
@@ -0,0 +1,41 @@
+/*
+ *  RISC-V sysarch() system call emulation
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_SYSARCH_H
+#define TARGET_ARCH_SYSARCH_H
+
+#include "target_syscall.h"
+#include "target_arch.h"
+
+static inline abi_long do_freebsd_arch_sysarch(CPURISCVState *env, int op,
+abi_ulong parms)
+{
+
+return -TARGET_EOPNOTSUPP;
+}
+
+static inline void do_freebsd_arch_print_sysarch(
+const struct syscallname *name, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5, abi_long arg6)
+{
+
+gemu_log("UNKNOWN OP: %d, " TARGET_ABI_FMT_lx ")", (int)arg1, arg2);
+}
+
+#endif /* TARGET_ARCH_SYSARCH_H */
-- 
2.34.1

[PATCH v7 14/17] bsd-user: Implement RISC-V signal trampoline setup functions

[Ajeet Singh]

From: Mark Corbin 

Added functions for setting up the RISC-V signal trampoline and signal
frame:

'set_sigtramp_args()': Configures the RISC-V CPU state with arguments
for the signal handler. It sets up the registers with the signal
number,pointers to the signal info and user context, the signal handler
address, and the signal frame pointer.

'setup_sigframe_arch()': Initializes the signal frame with the current
machine context.This function copies the context from the CPU state to
the signal frame, preparing it for the signal handler.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Signed-off-by: Warner Losh 
Co-authored-by: Warner Losh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/signal.c | 63 +
 1 file changed, 63 insertions(+)
 create mode 100644 bsd-user/riscv/signal.c

diff --git a/bsd-user/riscv/signal.c b/bsd-user/riscv/signal.c
new file mode 100644
index 00..2597fec2fd
--- /dev/null
+++ b/bsd-user/riscv/signal.c
@@ -0,0 +1,63 @@
+/*
+ *  RISC-V signal definitions
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+
+#include "qemu.h"
+
+/*
+ * Compare with sendsig() in riscv/riscv/exec_machdep.c
+ * Assumes that target stack frame memory is locked.
+ */
+abi_long
+set_sigtramp_args(CPURISCVState *regs, int sig, struct target_sigframe *frame,
+abi_ulong frame_addr, struct target_sigaction *ka)
+{
+/*
+ * Arguments to signal handler:
+ *  a0 (10) = signal number
+ *  a1 (11) = siginfo pointer
+ *  a2 (12) = ucontext pointer
+ *  pc  = signal pointer handler
+ *  sp (2)  = sigframe pointer
+ *  ra (1)  = sigtramp at base of user stack
+ */
+
+ regs->gpr[xA0] = sig;
+ regs->gpr[xA1] = frame_addr +
+ offsetof(struct target_sigframe, sf_si);
+ regs->gpr[xA2] = frame_addr +
+ offsetof(struct target_sigframe, sf_uc);
+ regs->pc = ka->_sa_handler;
+ regs->gpr[xSP] = frame_addr;
+ regs->gpr[xRA] = TARGET_PS_STRINGS - TARGET_SZSIGCODE;
+ return 0;
+}
+
+/*
+ * Compare to riscv/riscv/exec_machdep.c sendsig()
+ * Assumes that the memory is locked if frame points to user memory.
+ */
+abi_long setup_sigframe_arch(CPURISCVState *env, abi_ulong frame_addr,
+ struct target_sigframe *frame, int flags)
+{
+target_mcontext_t *mcp = &frame->sf_uc.uc_mcontext;
+
+get_mcontext(env, mcp, flags);
+return 0;
+}
-- 
2.34.1

[PATCH v7 07/17] bsd-user: Add RISC-V signal trampoline setup function

[Ajeet Singh]

From: Mark Corbin 

Implemented the 'setup_sigtramp' function for setting up the signal
trampoline code in the RISC-V architecture.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_sigtramp.h | 41 +++
 1 file changed, 41 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_sigtramp.h

diff --git a/bsd-user/riscv/target_arch_sigtramp.h 
b/bsd-user/riscv/target_arch_sigtramp.h
new file mode 100644
index 00..dfe5076739
--- /dev/null
+++ b/bsd-user/riscv/target_arch_sigtramp.h
@@ -0,0 +1,41 @@
+/*
+ * RISC-V sigcode
+ *
+ * Copyright (c) 2019 Mark Corbin
+ *
+ * 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 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 .
+ */
+
+#ifndef TARGET_ARCH_SIGTRAMP_H
+#define TARGET_ARCH_SIGTRAMP_H
+
+/* Compare with sigcode() in riscv/riscv/locore.S */
+static inline abi_long setup_sigtramp(abi_ulong offset, unsigned sigf_uc,
+unsigned sys_sigreturn)
+{
+uint32_t sys_exit = TARGET_FREEBSD_NR_exit;
+
+uint32_t sigtramp_code[] = {
+/*1*/ const_le32(0x00010513),/*mv a0, sp*/
+/*2*/ const_le32(0x00050513 + (sigf_uc << 20)),  /*addi a0,a0,sigf_uc*/
+/*3*/ const_le32(0x0293 + (sys_sigreturn << 20)),/*li 
t0,sys_sigreturn*/
+/*4*/ const_le32(0x0073),/*ecall*/
+/*5*/ const_le32(0x0293 + (sys_exit << 20)), /*li t0,sys_exit*/
+/*6*/ const_le32(0x0073),/*ecall*/
+/*7*/ const_le32(0xFF1FF06F) /*b -16*/
+};
+
+return memcpy_to_target(offset, sigtramp_code, TARGET_SZSIGCODE);
+}
+#endif /* TARGET_ARCH_SIGTRAMP_H */
-- 
2.34.1

[PATCH v7 13/17] bsd-user: Define RISC-V signal handling structures and constants

[Ajeet Singh]

From: Mark Corbin 

Added definitions for RISC-V signal handling, including structures
and constants for managing signal frames and context

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Warner Losh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_signal.h | 75 +
 1 file changed, 75 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_signal.h

diff --git a/bsd-user/riscv/target_arch_signal.h 
b/bsd-user/riscv/target_arch_signal.h
new file mode 100644
index 00..1a634b865b
--- /dev/null
+++ b/bsd-user/riscv/target_arch_signal.h
@@ -0,0 +1,75 @@
+/*
+ *  RISC-V signal definitions
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_SIGNAL_H
+#define TARGET_ARCH_SIGNAL_H
+
+#include "cpu.h"
+
+
+#define TARGET_INSN_SIZE 4  /* riscv instruction size */
+
+/* Size of the signal trampoline code placed on the stack. */
+#define TARGET_SZSIGCODE((abi_ulong)(7 * TARGET_INSN_SIZE))
+
+/* Compare with riscv/include/_limits.h */
+#define TARGET_MINSIGSTKSZ  (1024 * 4)
+#define TARGET_SIGSTKSZ (TARGET_MINSIGSTKSZ + 32768)
+
+struct target_gpregs {
+uint64_tgp_ra;
+uint64_tgp_sp;
+uint64_tgp_gp;
+uint64_tgp_tp;
+uint64_tgp_t[7];
+uint64_tgp_s[12];
+uint64_tgp_a[8];
+uint64_tgp_sepc;
+uint64_tgp_sstatus;
+};
+
+struct target_fpregs {
+uint64_tfp_x[32][2];
+uint64_tfp_fcsr;
+uint32_tfp_flags;
+uint32_tpad;
+};
+
+typedef struct target_mcontext {
+struct target_gpregs   mc_gpregs;
+struct target_fpregs   mc_fpregs;
+uint32_t   mc_flags;
+#define TARGET_MC_FP_VALID 0x01
+uint32_t   mc_pad;
+uint64_t   mc_spare[8];
+} target_mcontext_t;
+
+#define TARGET_MCONTEXT_SIZE 864
+#define TARGET_UCONTEXT_SIZE 936
+
+#include "target_os_ucontext.h"
+
+struct target_sigframe {
+target_ucontext_t   sf_uc; /* = *sf_uncontext */
+target_siginfo_tsf_si; /* = *sf_siginfo (SA_SIGINFO case)*/
+};
+
+#define TARGET_SIGSTACK_ALIGN 16
+
+#endif /* TARGET_ARCH_SIGNAL_H */
-- 
2.34.1

[PATCH v7 12/17] bsd-user: Add generic RISC-V64 target definitions

[Ajeet Singh]

From: Warner Losh 

Added a generic definition for RISC-V64 target-specific details.
Implemented the 'regpairs_aligned' function,which returns 'false'
to indicate that register pairs are not aligned in the RISC-V64 ABI.

Signed-off-by: Warner Losh 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target.h | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 bsd-user/riscv/target.h

diff --git a/bsd-user/riscv/target.h b/bsd-user/riscv/target.h
new file mode 100644
index 00..036ddd185e
--- /dev/null
+++ b/bsd-user/riscv/target.h
@@ -0,0 +1,20 @@
+/*
+ * Riscv64 general target stuff that's common to all aarch details
+ *
+ * Copyright (c) 2022 M. Warner Losh 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef TARGET_H
+#define TARGET_H
+
+/*
+ * riscv64 ABI does not 'lump' the registers for 64-bit args.
+ */
+static inline bool regpairs_aligned(void *cpu_env)
+{
+return false;
+}
+
+#endif /* TARGET_H */
-- 
2.34.1

[PATCH v7 17/17] bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files

[Ajeet Singh]

From: Warner Losh 

Added configuration for RISC-V 64-bit target to the build system.

Signed-off-by: Warner Losh 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 configs/targets/riscv64-bsd-user.mak | 4 
 1 file changed, 4 insertions(+)
 create mode 100644 configs/targets/riscv64-bsd-user.mak

diff --git a/configs/targets/riscv64-bsd-user.mak 
b/configs/targets/riscv64-bsd-user.mak
new file mode 100644
index 00..191c2c483f
--- /dev/null
+++ b/configs/targets/riscv64-bsd-user.mak
@@ -0,0 +1,4 @@
+TARGET_ARCH=riscv64
+TARGET_BASE_ARCH=riscv
+TARGET_ABI_DIR=riscv
+TARGET_XML_FILES= gdb-xml/riscv-64bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml 
gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-64bit-virtual.xml
-- 
2.34.1

[PATCH v7 10/17] bsd-user: Define RISC-V VM parameters and helper functions

[Ajeet Singh]

From: Mark Corbin 

Added definitions for RISC-V VM parameters, including maximum and
default sizes for text, data, and stack, as well as address space
limits.
Implemented helper functions for retrieving and setting specific
values in the CPU state, such as stack pointer and return values.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_vmparam.h | 53 
 1 file changed, 53 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_vmparam.h

diff --git a/bsd-user/riscv/target_arch_vmparam.h 
b/bsd-user/riscv/target_arch_vmparam.h
new file mode 100644
index 00..0f2486def1
--- /dev/null
+++ b/bsd-user/riscv/target_arch_vmparam.h
@@ -0,0 +1,53 @@
+/*
+ *  RISC-V VM parameters definitions
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_VMPARAM_H
+#define TARGET_ARCH_VMPARAM_H
+
+#include "cpu.h"
+
+/* Compare with riscv/include/vmparam.h */
+#define TARGET_MAXTSIZ  (1 * GiB)   /* max text size */
+#define TARGET_DFLDSIZ  (128 * MiB) /* initial data size limit */
+#define TARGET_MAXDSIZ  (1 * GiB)   /* max data size */
+#define TARGET_DFLSSIZ  (128 * MiB) /* initial stack size limit */
+#define TARGET_MAXSSIZ  (1 * GiB)   /* max stack size */
+#define TARGET_SGROWSIZ (128 * KiB) /* amount to grow stack */
+
+#define TARGET_VM_MINUSER_ADDRESS   (0xUL)
+#define TARGET_VM_MAXUSER_ADDRESS   (0x0040UL)
+
+#define TARGET_USRSTACK (TARGET_VM_MAXUSER_ADDRESS - TARGET_PAGE_SIZE)
+
+static inline abi_ulong get_sp_from_cpustate(CPURISCVState *state)
+{
+return state->gpr[xSP];
+}
+
+static inline void set_second_rval(CPURISCVState *state, abi_ulong retval2)
+{
+state->gpr[xA1] = retval2;
+}
+
+static inline abi_ulong get_second_rval(CPURISCVState *state)
+{
+return state->gpr[xA1];
+}
+
+#endif /* TARGET_ARCH_VMPARAM_H */
-- 
2.34.1

[PATCH v7 00/17] bsd-user: Comprehensive RISCV Support

[Ajeet Singh]

Key Changes Compared to Version 6:
Included "signal-common.h" in target_arch_cpu.h

Mark Corbin (15):
  bsd-user: Implement RISC-V CPU initialization and main loop
  bsd-user: Add RISC-V CPU execution loop and syscall handling
  bsd-user: Implement RISC-V CPU register cloning and reset functions
  bsd-user: Implement RISC-V TLS register setup
  bsd-user: Add RISC-V ELF definitions and hardware capability detection
  bsd-user: Define RISC-V register structures and register copying
  bsd-user: Add RISC-V signal trampoline setup function
  bsd-user: Implement RISC-V sysarch system call emulation
  bsd-user: Add RISC-V thread setup and initialization support
  bsd-user: Define RISC-V VM parameters and helper functions
  bsd-user: Define RISC-V system call structures and constants
  bsd-user: Define RISC-V signal handling structures and constants
  bsd-user: Implement RISC-V signal trampoline setup functions
  bsd-user: Implement 'get_mcontext' for RISC-V
  bsd-user: Implement set_mcontext and get_ucontext_sigreturn for RISCV

Warner Losh (2):
  bsd-user: Add generic RISC-V64 target definitions
  bsd-user: Add RISC-V 64-bit Target Configuration and Debug XML Files

 bsd-user/riscv/signal.c   | 170 ++
 bsd-user/riscv/target.h   |  20 +++
 bsd-user/riscv/target_arch.h  |  27 
 bsd-user/riscv/target_arch_cpu.c  |  29 +
 bsd-user/riscv/target_arch_cpu.h  | 148 ++
 bsd-user/riscv/target_arch_elf.h  |  42 +++
 bsd-user/riscv/target_arch_reg.h  |  88 +
 bsd-user/riscv/target_arch_signal.h   |  75 
 bsd-user/riscv/target_arch_sigtramp.h |  41 +++
 bsd-user/riscv/target_arch_sysarch.h  |  41 +++
 bsd-user/riscv/target_arch_thread.h   |  47 +++
 bsd-user/riscv/target_arch_vmparam.h  |  53 
 bsd-user/riscv/target_syscall.h   |  38 ++
 configs/targets/riscv64-bsd-user.mak  |   4 +
 14 files changed, 823 insertions(+)
 create mode 100644 bsd-user/riscv/signal.c
 create mode 100644 bsd-user/riscv/target.h
 create mode 100644 bsd-user/riscv/target_arch.h
 create mode 100644 bsd-user/riscv/target_arch_cpu.c
 create mode 100644 bsd-user/riscv/target_arch_cpu.h
 create mode 100644 bsd-user/riscv/target_arch_elf.h
 create mode 100644 bsd-user/riscv/target_arch_reg.h
 create mode 100644 bsd-user/riscv/target_arch_signal.h
 create mode 100644 bsd-user/riscv/target_arch_sigtramp.h
 create mode 100644 bsd-user/riscv/target_arch_sysarch.h
 create mode 100644 bsd-user/riscv/target_arch_thread.h
 create mode 100644 bsd-user/riscv/target_arch_vmparam.h
 create mode 100644 bsd-user/riscv/target_syscall.h
 create mode 100644 configs/targets/riscv64-bsd-user.mak

-- 
2.34.1

[PATCH v7 01/17] bsd-user: Implement RISC-V CPU initialization and main loop

[Ajeet Singh]

From: Mark Corbin 

Added the initial implementation for RISC-V CPU initialization and main
loop. This includes setting up the general-purpose registers and
program counter based on the provided target architecture definitions.

Signed-off-by: Mark Corbin 
Signed-off-by: Ajeet Singh 
Co-authored-by: Jessica Clarke 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/target_arch_cpu.h | 40 
 1 file changed, 40 insertions(+)
 create mode 100644 bsd-user/riscv/target_arch_cpu.h

diff --git a/bsd-user/riscv/target_arch_cpu.h b/bsd-user/riscv/target_arch_cpu.h
new file mode 100644
index 00..f8d85e01ad
--- /dev/null
+++ b/bsd-user/riscv/target_arch_cpu.h
@@ -0,0 +1,40 @@
+/*
+ *  RISC-V CPU init and loop
+ *
+ *  Copyright (c) 2019 Mark Corbin
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef TARGET_ARCH_CPU_H
+#define TARGET_ARCH_CPU_H
+
+#include "target_arch.h"
+#include "signal-common.h"
+
+#define TARGET_DEFAULT_CPU_MODEL "max"
+
+static inline void target_cpu_init(CPURISCVState *env,
+struct target_pt_regs *regs)
+{
+int i;
+
+for (i = 1; i < 32; i++) {
+env->gpr[i] = regs->regs[i];
+}
+
+env->pc = regs->sepc;
+}
+
+#endif /* TARGET_ARCH_CPU_H */
-- 
2.34.1

[PATCH v7 16/17] bsd-user: Implement set_mcontext and get_ucontext_sigreturn for RISCV

[Ajeet Singh]

From: Mark Corbin 

Added implementations for 'set_mcontext' and 'get_ucontext_sigreturn'
functions for RISC-V architecture,
Both functions ensure that the CPU state and user context are properly
managed.

Signed-off-by: Mark Corbin 
Signed-off-by: Warner Losh 
Signed-off-by: Ajeet Singh 
Co-authored-by: Warner Losh 
Reviewed-by: Richard Henderson 
---
 bsd-user/riscv/signal.c | 54 +
 1 file changed, 54 insertions(+)

diff --git a/bsd-user/riscv/signal.c b/bsd-user/riscv/signal.c
index 072ad821d2..10c940cd49 100644
--- a/bsd-user/riscv/signal.c
+++ b/bsd-user/riscv/signal.c
@@ -114,3 +114,57 @@ abi_long get_mcontext(CPURISCVState *regs, 
target_mcontext_t *mcp,
 
 return 0;
 }
+
+/* Compare with set_mcontext() in riscv/riscv/exec_machdep.c */
+abi_long set_mcontext(CPURISCVState *regs, target_mcontext_t *mcp,
+int srflag)
+{
+
+regs->gpr[5] = tswap64(mcp->mc_gpregs.gp_t[0]);
+regs->gpr[6] = tswap64(mcp->mc_gpregs.gp_t[1]);
+regs->gpr[7] = tswap64(mcp->mc_gpregs.gp_t[2]);
+regs->gpr[28] = tswap64(mcp->mc_gpregs.gp_t[3]);
+regs->gpr[29] = tswap64(mcp->mc_gpregs.gp_t[4]);
+regs->gpr[30] = tswap64(mcp->mc_gpregs.gp_t[5]);
+regs->gpr[31] = tswap64(mcp->mc_gpregs.gp_t[6]);
+
+regs->gpr[8] = tswap64(mcp->mc_gpregs.gp_s[0]);
+regs->gpr[9] = tswap64(mcp->mc_gpregs.gp_s[1]);
+regs->gpr[18] = tswap64(mcp->mc_gpregs.gp_s[2]);
+regs->gpr[19] = tswap64(mcp->mc_gpregs.gp_s[3]);
+regs->gpr[20] = tswap64(mcp->mc_gpregs.gp_s[4]);
+regs->gpr[21] = tswap64(mcp->mc_gpregs.gp_s[5]);
+regs->gpr[22] = tswap64(mcp->mc_gpregs.gp_s[6]);
+regs->gpr[23] = tswap64(mcp->mc_gpregs.gp_s[7]);
+regs->gpr[24] = tswap64(mcp->mc_gpregs.gp_s[8]);
+regs->gpr[25] = tswap64(mcp->mc_gpregs.gp_s[9]);
+regs->gpr[26] = tswap64(mcp->mc_gpregs.gp_s[10]);
+regs->gpr[27] = tswap64(mcp->mc_gpregs.gp_s[11]);
+
+regs->gpr[10] = tswap64(mcp->mc_gpregs.gp_a[0]);
+regs->gpr[11] = tswap64(mcp->mc_gpregs.gp_a[1]);
+regs->gpr[12] = tswap64(mcp->mc_gpregs.gp_a[2]);
+regs->gpr[13] = tswap64(mcp->mc_gpregs.gp_a[3]);
+regs->gpr[14] = tswap64(mcp->mc_gpregs.gp_a[4]);
+regs->gpr[15] = tswap64(mcp->mc_gpregs.gp_a[5]);
+regs->gpr[16] = tswap64(mcp->mc_gpregs.gp_a[6]);
+regs->gpr[17] = tswap64(mcp->mc_gpregs.gp_a[7]);
+
+
+regs->gpr[1] = tswap64(mcp->mc_gpregs.gp_ra);
+regs->gpr[2] = tswap64(mcp->mc_gpregs.gp_sp);
+regs->gpr[3] = tswap64(mcp->mc_gpregs.gp_gp);
+regs->gpr[4] = tswap64(mcp->mc_gpregs.gp_tp);
+regs->pc = tswap64(mcp->mc_gpregs.gp_sepc);
+
+return 0;
+}
+
+/* Compare with sys_sigreturn() in riscv/riscv/machdep.c */
+abi_long get_ucontext_sigreturn(CPURISCVState *regs,
+abi_ulong target_sf, abi_ulong *target_uc)
+{
+
+*target_uc = target_sf;
+return 0;
+}
-- 
2.34.1

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Brian Cain]

On 9/16/2024 10:47 AM, Alex Bennée wrote:

Brian Cain  writes:


On 9/16/2024 8:12 AM, Alex Bennée wrote:

Brian Cain  writes:


On 9/6/2024 9:39 PM, Brian Cain wrote:

With newer clang builds (19.x), there's a warning for implicit function
declarations and it rejects linux-test.c.

glibc/musl's readdir64() declaration in dirent is guarded by
_LARGEFILE64_SOURCE, so we'll define it to fix the warning.

 BUILD   hexagon-linux-user guest-tests
   
/local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14:
 error: call to undeclared function 'readdir64'; ISO C99 and later do not 
support implicit function declarations [-Wimplicit-function-declaration]
 189 | de = readdir64(dir);
 |  ^

Signed-off-by: Brian Cain 
---
tests/tcg/multiarch/linux/linux-test.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tests/tcg/multiarch/linux/linux-test.c 
b/tests/tcg/multiarch/linux/linux-test.c
index 64f57cb287..4e0e862ad9 100644
--- a/tests/tcg/multiarch/linux/linux-test.c
+++ b/tests/tcg/multiarch/linux/linux-test.c
@@ -17,6 +17,7 @@
 *  along with this program; if not, see .
 */
#define _GNU_SOURCE
+#define _LARGEFILE64_SOURCE
#include 
#include 
#include 

Alex -- what do you think about this one?

Actually scratch that, this is a 32 compat hack:

1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit host)

Is the __USE_LARGEFILE64 symbol in the hexagon headers?


musl does not define/use __USE_LARGEFILE64, no.  If it's well defined
I could examine whether it makes sense to add this feature to musl,
though.  How does __USE_LARGEFILE64 differ from _LARGEFILE64_SOURCE?
Is it more appropriate to define that here?

Digging into the GNU source _LARGEFILE* is the correct define, the __USE
flags are internal. features.h says:

_LARGEFILE_SOURCESome more functions for correct standard I/O.
_LARGEFILE64_SOURCE  Additional functionality from LFS for large files.

although looking at _LARGEFILE64_SOURCE should be defined by
_GNU_SOURCE which is already set for linux-test.c

According to the musl WHATSNEW:

   compatibility:
   - make _GNU_SOURCE imply _LARGEFILE64_SOURCE


Yeah, I just noticed that myself. I guess I will look at how it's done 
and see if I can fix this so it's more general and can include this case.



So is this a hexagon only thing?

It's not - I expect it would impact any architecture using musl.

-Brian

Re: [PATCH 1/3] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Stefan Hajnoczi]

On Thu, Sep 12, 2024 at 04:44:30PM +0200, Albert Esteve wrote:
> Add SHMEM_MAP/_UNMAP request to the vhost-user
> spec documentation.
> 
> Signed-off-by: Albert Esteve 
> ---
>  docs/interop/vhost-user.rst | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d8419fd2f1..d680fea4a3 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -369,6 +369,7 @@ In QEMU the vhost-user message is implemented with the 
> following struct:
>VhostUserConfig config;
>VhostUserVringArea area;
>VhostUserInflight inflight;
> +  VhostUserMMap mmap;
>};
>} QEMU_PACKED VhostUserMsg;
>  
> @@ -1859,6 +1860,36 @@ is sent by the front-end.
>when the operation is successful, or non-zero otherwise. Note that if the
>operation fails, no fd is sent to the backend.
>  
> +``VHOST_USER_BACKEND_SHMEM_MAP``
> +  :id: 9
> +  :equivalent ioctl: N/A
> +  :request payload: fd and ``struct VhostUserMMap``

Where is struct VhostUserMMap defined? Please keep patches
self-contained and in a logical order so they can be reviewed linearly.

Otherwise:
Reviewed-by: Stefan Hajnoczi 

> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends to advertise a new mapping
> +  to be made in a given VIRTIO Shared Memory Region. Upon receiving the 
> message,
> +  The front-end will mmap the given fd into the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. A reply is generated indicating whether 
> mapping
> +  succeeded.
> +
> +  Mapping over an already existing map is not allowed and request shall fail.
> +  Therefore, the memory range in the request must correspond with a valid,
> +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> +  consume resources and that the request can fail when there are no resources
> +  available.
> +
> +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> +  :id: 10
> +  :equivalent ioctl: N/A
> +  :request payload: ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends so that the front-end un-mmap
> +  a given range (``shm_offset``, ``len``) in the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. Note that the given range shall correspond to
> +  the entirety of a valid mapped region.
> +  A reply is generated indicating whether unmapping succeeded.
> +
>  .. _reply_ack:
>  
>  VHOST_USER_PROTOCOL_F_REPLY_ACK
> -- 
> 2.45.2
> 


signature.asc
Description: PGP signature

Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Brian Cain]

On 9/16/2024 11:05 AM, Brian Cain wrote:


On 9/16/2024 10:47 AM, Alex Bennée wrote:

Brian Cain  writes:


On 9/16/2024 8:12 AM, Alex Bennée wrote:

Brian Cain  writes:


On 9/6/2024 9:39 PM, Brian Cain wrote:
With newer clang builds (19.x), there's a warning for implicit 
function

declarations and it rejects linux-test.c.

glibc/musl's readdir64() declaration in dirent is guarded by
_LARGEFILE64_SOURCE, so we'll define it to fix the warning.

 BUILD   hexagon-linux-user guest-tests
/local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14: 
error: call to undeclared function 'readdir64'; ISO C99 and later 
do not support implicit function declarations 
[-Wimplicit-function-declaration]

 189 | de = readdir64(dir);
 |  ^

Signed-off-by: Brian Cain 
---
    tests/tcg/multiarch/linux/linux-test.c | 1 +
    1 file changed, 1 insertion(+)

diff --git a/tests/tcg/multiarch/linux/linux-test.c 
b/tests/tcg/multiarch/linux/linux-test.c

index 64f57cb287..4e0e862ad9 100644
--- a/tests/tcg/multiarch/linux/linux-test.c
+++ b/tests/tcg/multiarch/linux/linux-test.c
@@ -17,6 +17,7 @@
 *  along with this program; if not, see 
.

 */
    #define _GNU_SOURCE
+#define _LARGEFILE64_SOURCE
    #include 
    #include 
    #include 

Alex -- what do you think about this one?

Actually scratch that, this is a 32 compat hack:

    1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit 
host)


Is the __USE_LARGEFILE64 symbol in the hexagon headers?


musl does not define/use __USE_LARGEFILE64, no.  If it's well defined
I could examine whether it makes sense to add this feature to musl,
though.  How does __USE_LARGEFILE64 differ from _LARGEFILE64_SOURCE?
Is it more appropriate to define that here?

Digging into the GNU source _LARGEFILE* is the correct define, the __USE
flags are internal. features.h says:

    _LARGEFILE_SOURCE    Some more functions for correct standard I/O.
    _LARGEFILE64_SOURCE  Additional functionality from LFS for large 
files.


although looking at _LARGEFILE64_SOURCE should be defined by
_GNU_SOURCE which is already set for linux-test.c

According to the musl WHATSNEW:

   compatibility:
   - make _GNU_SOURCE imply _LARGEFILE64_SOURCE


Yeah, I just noticed that myself. I guess I will look at how it's done 
and see if I can fix this so it's more general and can include this case.



So is this a hexagon only thing?

It's not - I expect it would impact any architecture using musl.



Hmm.  The _GNU_SOURCE guard was deliberately removed in 
25e6fee27f4a293728dd15b659170e7b9c7db9bc (see below).  IMO the relevant 
text is "portable software should be prepared for them not to exist" and 
" the intent is that this be a very short-term measure and that the 
macros be removed entirely in the next release cycle."  They're still 
there, guarded by only _LARGEFILE64_SOURCE.  I will bring up the 
question about what the future plan for this is on the musl list, but I 
also think that the appropriate, portable thing to do is to change the 
test case regardless of musl's plans.  If there were some other 
conformant C library it could implement it the same way.  IIUC the 
relevant specification is here 
https://unix.org/version2/whatsnew/lfs20mar.html



commit 25e6fee27f4a293728dd15b659170e7b9c7db9bc
Author: Rich Felker 
Date:   Tue Sep 27 15:04:05 2022 -0400

    remove LFS64 programming interfaces (macro-only) from _GNU_SOURCE

    these badly pollute the namespace with macros whenever _GNU_SOURCE is
    defined, which is always the case with g++, and especially tends to
    interfere with C++ constructs.

    as our implementation of these was macro-only, their removal cannot
    affect any existing binaries. at the source level, portable software
    should be prepared for them not to exist.

    for now, they are left in place with explicit _LARGEFILE64_SOURCE.
    this provides an easy temporary path for integrators/distributions to
    get packages building again right away if they break while working on
    a proper, upstreamable fix. the intent is that this be a very
    short-term measure and that the macros be removed entirely in the next
    release cycle.

Re: [PATCH 3/3] vhost_user.rst: Add GET_SHMEM_CONFIG message

[Stefan Hajnoczi]

On Thu, Sep 12, 2024 at 04:44:32PM +0200, Albert Esteve wrote:
> Add GET_SHMEM_CONFIG vhost-user frontend
> message to the spec documentation.
> 
> Signed-off-by: Albert Esteve 
> ---
>  docs/interop/vhost-user.rst | 39 +
>  1 file changed, 39 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index ec898d2440..fb7b27ce94 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -348,6 +348,19 @@ Device state transfer parameters
>In the future, additional phases might be added e.g. to allow
>iterative migration while the device is running.
>  
> +VIRTIO Shared Memory Region configuration
> +^
> +
> ++-+-+++--+
> +| num regions | padding | mem size 0 | .. | mem size 255 |
> ++-+-+++--+
> +
> +:num regions: a 32-bit number of regions

What is the purpose of this field? Does it indicate the number of mem
size elements (i.e. the array can be truncated if there are fewer than
256 elements) or the number of non-zero length elements?

> +
> +:padding: 32-bit
> +
> +:mem size: 64-bit size of VIRTIO Shared Memory Region
> +
>  C structure
>  ---
>  
> @@ -372,6 +385,7 @@ In QEMU the vhost-user message is implemented with the 
> following struct:
>VhostUserShared object;
>VhostUserTransferDeviceState transfer_state;
>VhostUserMMap mmap;
> +  VhostUserShMemConfig shmem;
>};
>} QEMU_PACKED VhostUserMsg;
>  
> @@ -1054,6 +1068,7 @@ Protocol features
>#define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
>#define VHOST_USER_PROTOCOL_F_SHARED_OBJECT18
>#define VHOST_USER_PROTOCOL_F_DEVICE_STATE 19
> +  #define VHOST_USER_PROTOCOL_F_SHMEM20
>  
>  Front-end message types
>  ---
> @@ -1728,6 +1743,30 @@ Front-end message types
>Using this function requires prior negotiation of the
>``VHOST_USER_PROTOCOL_F_DEVICE_STATE`` feature.
>  
> +``VHOST_USER_GET_SHMEM_CONFIG``
> +  :id: 44
> +  :equivalent ioctl: N/A
> +  :request payload: N/A
> +  :reply payload: ``struct VhostUserShMemConfig``
> +
> +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> +  successfully negotiated, this message can be submitted by the front-end
> +  to gather the VIRTIO Shared Memory Region configuration. Back-end will

"Back-end will" -> "The back-end will"

> +  respond with the number of VIRTIO Shared Memory Regions it requires, and
> +  each shared memory region size in an array. The shared memory IDs are
> +  represented by the index of the array. The information returned shall

"index of the array" -> either "array index" or "index of the array
element".

> +  comply with the following rules:
> +
> +  * The shared information will remain valid and unchanged for the entire
> +lifetime of the connection.
> +
> +  * The Shared Memory Region size must be a multiple of the page size
> +supported by mmap(2).
> +
> +  * The size may be 0 if the region is unused. This can happen when the
> +device does not support an optional feature but does support a feature
> +that uses a higher shmid.
> +
>  Back-end message types
>  --
>  
> -- 
> 2.45.2
> 


signature.asc
Description: PGP signature

Re: [PATCH 1/3] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Stefan Hajnoczi]

On Thu, Sep 12, 2024 at 04:44:30PM +0200, Albert Esteve wrote:
> Add SHMEM_MAP/_UNMAP request to the vhost-user
> spec documentation.
> 
> Signed-off-by: Albert Esteve 
> ---
>  docs/interop/vhost-user.rst | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d8419fd2f1..d680fea4a3 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -369,6 +369,7 @@ In QEMU the vhost-user message is implemented with the 
> following struct:
>VhostUserConfig config;
>VhostUserVringArea area;
>VhostUserInflight inflight;
> +  VhostUserMMap mmap;
>};
>} QEMU_PACKED VhostUserMsg;
>  
> @@ -1859,6 +1860,36 @@ is sent by the front-end.
>when the operation is successful, or non-zero otherwise. Note that if the
>operation fails, no fd is sent to the backend.
>  
> +``VHOST_USER_BACKEND_SHMEM_MAP``
> +  :id: 9
> +  :equivalent ioctl: N/A
> +  :request payload: fd and ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends to advertise a new mapping
> +  to be made in a given VIRTIO Shared Memory Region. Upon receiving the 
> message,
> +  The front-end will mmap the given fd into the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. A reply is generated indicating whether 
> mapping
> +  succeeded.
> +
> +  Mapping over an already existing map is not allowed and request shall fail.
> +  Therefore, the memory range in the request must correspond with a valid,
> +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> +  consume resources and that the request can fail when there are no resources
> +  available.
> +
> +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> +  :id: 10
> +  :equivalent ioctl: N/A
> +  :request payload: ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  This message can be submitted by the backends so that the front-end un-mmap
> +  a given range (``shm_offset``, ``len``) in the VIRTIO Shared Memory Region
> +  with the requested ``shmid``. Note that the given range shall correspond to
> +  the entirety of a valid mapped region.
> +  A reply is generated indicating whether unmapping succeeded.

There is no mention of VHOST_USER_PROTOCOL_F_SHMEM. I think it needs to
be negotiated in order to use VHOST_USER_BACKEND_SHMEM_MAP and
VHOST_USER_BACKEND_SHMEM_UNMAP.

> +
>  .. _reply_ack:
>  
>  VHOST_USER_PROTOCOL_F_REPLY_ACK
> -- 
> 2.45.2
> 


signature.asc
Description: PGP signature

Re: [PATCH 0/3] Document SHMEM vhost-user requests

[Stefan Hajnoczi]

On Thu, Sep 12, 2024 at 04:44:29PM +0200, Albert Esteve wrote:
> As a continuation of the
> "Add SHMEM_MAP/UNMAP requests" patch [1],
> I wanted to split vhost-user spec
> parts into a separate patch, so that
> it could be reviewed and integrated
> separately. Having the specs upstreamed
> would help others to add support for
> these new vhost-user requests on
> their systems.
> 
> This is such documentation-only patch.

The spec changes do not mention that exposing VIRTIO Shared Memory
Regions can lead to problems when other vhost-user devices attempt to
access those memory addresses. Please either introduce MEM_READ/WRITE
and explain its use or document that front-ends must forward mappings to
back-ends so that VIRTIO Shared Memory Regions can be accessed.

> 
> [1] - 
> https://lore.kernel.org/all/20240628145710.1516121-1-aest...@redhat.com/T/
> 
> Albert Esteve (3):
>   vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
>   vhost_user.rst: Align VhostUserMsg excerpt members
>   vhost_user.rst: Add GET_SHMEM_CONFIG message
> 
>  docs/interop/vhost-user.rst | 72 +
>  1 file changed, 72 insertions(+)
> 
> -- 
> 2.45.2
> 


signature.asc
Description: PGP signature

[PATCH v6 00/15] acpi: NUMA nodes for CXL HB as GP + complex NUMA test

[Jonathan Cameron via]

v6 changes:
- 2 new patches (11 and 12) to improve things in existing code after
  Igor pointed them out in the new code.
- More detailed example provided for docs for control of Generic Ports.
  This has proved a difficult concept to convey.
  Note there is one question Igor raised for Markus:
  - Is exit(1) ok for failure paths rather than error_fatal.
Markus has acked the patch (10) but maybe this part was not his
focus.
- Rebased.  Table data regenerated as other series touched DSDT.

Title becoming a little misleading as now this does a bunch of other
stuff as precursors, but I've kept it to maintain association with v3 and
before. A more accurate series title is probably
acpi: Rework GI affinity structure generation, add GPs + complex NUMA test.

ACPI 6.5 introduced Generic Port Affinity Structures to close a system
description gap that was a problem for CXL memory systems.
It defines an new SRAT Affinity structure (and hence allows creation of an
ACPI Proximity Node which can only be defined via an SRAT structure)
for the boundary between a discoverable fabric and a non discoverable
system interconnects etc.

The HMAT data on latency and bandwidth is combined with discoverable
information from the CXL bus (link speeds, lane counts) and CXL devices
(switch port to port characteristics and USP to memory, via CDAT tables
read from the device).  QEMU has supported the rest of the elements
of this chain for a while but now the kernel has caught up and we need
the missing element of Generic Ports (this code has been used extensively
in testing and debugging that kernel support, some resulting fixes
currently under review).

Generic Port Affinity Structures are very similar to the recently
added Generic Initiator Affinity Structures (GI) so this series
factors out and reuses much of that infrastructure for reuse
There are subtle differences (beyond the obvious structure ID change).

- The ACPI spec example (and linux kernel support) has a Generic
  Port not as associated with the CXL root port, but rather with
  the CXL Host bridge. As a result, an ACPI handle is used (rather
  than the PCI SBDF option for GIs). In QEMU the easiest way
  to get to this is to target the root bridge PCI Bus, and
  conveniently the root bridge bus number is used for the UID allowing
  us to construct an appropriate entry.

A key addition of this series is a complex NUMA topology example that
stretches the QEMU emulation code for GI, GP and nodes with just
CPUS, just memory, just hot pluggable memory, mixture of memory and CPUs.

A similar test showed up a few NUMA related bugs with fixes applied for
9.0 (note that one of these needs linux booted to identify that it
rejects the HMAT table and this test is a regression test for the
table generation only).

https://lore.kernel.org/qemu-devel/2eb6672cfdaea7dacd8e9bb0523887f13b9f85ce.1710282274.git@redhat.com/
https://lore.kernel.org/qemu-devel/74e2845c5f95b0c139c79233ddb65bb17f2dd679.1710282274.git@redhat.com/


Jonathan Cameron (15):
  hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle.
  hw/acpi/GI: Fix trivial parameter alignment issue.
  hw/acpi: Move AML building code for Generic Initiators to aml_build.c
  hw/acpi: Rename build_all_acpi_generic_initiators() to
build_acpi_generic_initiator()
  hw/pci: Add a busnr property to pci_props and use for acpi/gi
  acpi/pci: Move Generic Initiator object handling into acpi/pci.*
  hw/pci-bridge: Add acpi_uid property to TYPE_PXB_BUS
  hw/i386/acpi: Use TYPE_PXB_BUS property acpi_uid for DSDT
  hw/pci-host/gpex-acpi: Use acpi_uid property.
  hw/acpi: Generic Port Affinity Structure support
  hw/acpi: Make storage of node id uint32_t to reduce fragility
  hw/acpi: Generic Initiator - add missing object class property
descriptions.
  bios-tables-test: Allow for new acpihmat-generic-x test data.
  bios-tables-test: Add complex SRAT / HMAT test for GI GP
  bios-tables-test: Add data for complex numa test (GI, GP etc)

 qapi/qom.json |  41 +++
 include/hw/acpi/acpi_generic_initiator.h  |  47 
 include/hw/acpi/aml-build.h   |   7 +
 include/hw/acpi/pci.h |   3 +
 include/hw/pci/pci_bridge.h   |   1 +
 hw/acpi/acpi_generic_initiator.c  | 148 ---
 hw/acpi/aml-build.c   |  83 ++
 hw/acpi/pci.c | 242 ++
 hw/arm/virt-acpi-build.c  |   3 +-
 hw/i386/acpi-build.c  |   8 +-
 hw/pci-bridge/pci_expander_bridge.c   |  14 +-
 hw/pci-host/gpex-acpi.c   |   5 +-
 hw/pci/pci.c  |  14 +
 tests/qtest/bios-tables-test.c|  97 +++
 hw/acpi/meson.build   |   1 -
 .../data/acpi/x86/q35/APIC.acpihmat-generic-x | Bin 0 -> 136 bytes
 .../data/acpi/x86/q35/CEDT.acpihmat-generic-x | Bin 0 -> 6