Re: [RFC PATCH v1 0/2] vhost-vdpa: cache Virtio states

[Shao-Chien Chiang]

Hi,

I found a problem about cache.

If there are several devices operating the same backend device, the
cache might be inconsistent.

I think we could handle this by checking if a device is the first
device, but I'm not sure it will be feasible.

Is there any better approach to this problem?

Thank you!

Re: [PATCH 30/40] igb: Implement igb-specific oversize check

[Akihiko Odaki]

On 2023/04/16 20:22, Sriram Yagnaraman wrote:




-Original Message-
From: Akihiko Odaki 
Sent: Friday, 14 April 2023 13:37
Cc: Sriram Yagnaraman ; Jason Wang
; Dmitry Fleytman ;
Michael S. Tsirkin ; Alex Bennée ;
Philippe Mathieu-Daudé ; Thomas Huth
; Wainer dos Santos Moschetta
; Beraldo Leal ; Cleber Rosa
; Laurent Vivier ; Paolo Bonzini
; qemu-devel@nongnu.org; Akihiko Odaki

Subject: [PATCH 30/40] igb: Implement igb-specific oversize check

igb has a configurable size limit for LPE, and uses different limits depending 
on
whether the packet is treated as a VLAN packet.

Signed-off-by: Akihiko Odaki 
---
  hw/net/igb_core.c | 41 +++--
  1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
2013a9a53d..569897fb99 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -954,16 +954,21 @@ igb_rx_l4_cso_enabled(IGBCore *core)
  return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD);  }

-static bool


The convention in seems to be to declare return value in first line and then 
the function name in the next line.


There are already functions not following the convention, and it is more 
like exceptional in the entire QEMU code base. This patch prioritize the 
QEMU's common practice over e1000e's old convention.





-igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
+static bool igb_rx_is_oversized(IGBCore *core, const struct eth_header *ehdr,
+size_t size, bool lpe, uint16_t rlpml)
  {
-uint16_t pool = qn % IGB_NUM_VM_POOLS;
-bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
-int max_ethernet_lpe_size =
-core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
-int max_ethernet_vlan_size = 1522;
+size += 4;


Is the above 4 CRC bytes?


Yes. In v2, a new constant ETH_FCS_LEN is used to explictly state that.




+
+if (lpe) {
+return size > rlpml;
+}
+
+if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0x) &&
+e1000x_vlan_rx_filter_enabled(core->mac)) {
+return size > 1522;
+}


Should a check for 1526 bytes if extended VLAN is present be added?
Maybe in "igb: Strip the second VLAN tag for extended VLAN"?


In v2, I placed "igb: Strip the second VLAN tag for extended VLAN" 
earlier than this patch, and this patch is rewritten so it can handle 
the second VLAN tag too.






-return size > (lpe ? max_ethernet_lpe_size : max_ethernet_vlan_size);
+return size > 1518;
  }

  static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
@@ -976,6 +981,8 @@ static uint16_t igb_receive_assign(IGBCore *core,
const L2Header *l2_header,
  uint16_t queues = 0;
  uint16_t oversized = 0;
  uint16_t vid = be16_to_cpu(l2_header->vlan[0].h_tci) & VLAN_VID_MASK;
+bool lpe;
+uint16_t rlpml;
  int i;

  memset(rss_info, 0, sizeof(E1000E_RSSInfo)); @@ -984,6 +991,14 @@
static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
  *external_tx = true;
  }

+lpe = !!(core->mac[RCTL] & E1000_RCTL_LPE);
+rlpml = core->mac[RLPML];
+if (!(core->mac[RCTL] & E1000_RCTL_SBP) &&
+igb_rx_is_oversized(core, ehdr, size, lpe, rlpml)) {
+trace_e1000x_rx_oversized(size);
+return queues;
+}
+
  if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0x) &&
  !e1000x_rx_vlan_filter(core->mac, PKT_GET_VLAN_HDR(ehdr))) {
  return queues;
@@ -1067,7 +1082,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
const L2Header *l2_header,
  queues &= core->mac[VFRE];
  if (queues) {
  for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
-if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size)) {
+lpe = !!(core->mac[VMOLR0 + i] & E1000_VMOLR_LPE);
+rlpml = core->mac[VMOLR0 + i] & E1000_VMOLR_RLPML_MASK;
+if ((queues & BIT(i)) &&
+igb_rx_is_oversized(core, ehdr, size, lpe, rlpml))
+ {
  oversized |= BIT(i);
  }
  }
@@ -1609,11 +1627,6 @@ igb_receive_internal(IGBCore *core, const struct
iovec *iov, int iovcnt,
  iov_to_buf(iov, iovcnt, iov_ofs, &min_buf, sizeof(min_buf.l2_header));
  }

-/* Discard oversized packets if !LPE and !SBP. */
-if (e1000x_is_oversized(core->mac, size)) {
-return orig_size;
-}
-
  net_rx_pkt_set_packet_type(core->rx_pkt,
 get_eth_packet_type(&min_buf.l2_header.eth));
  net_rx_pkt_set_protocols(core->rx_pkt, iov, iovcnt, iov_ofs);
--
2.40.0

Re: [PULL 00/20] Migration 20230420 patches

[Richard Henderson]

On 4/20/23 14:17, Juan Quintela wrote:

The following changes since commit 2d82c32b2ceaca3dc3da5e36e10976f34bfcb598:

   Open 8.1 development tree (2023-04-20 10:05:25 +0100)

are available in the Git repository at:

   https://gitlab.com/juan.quintela/qemu.git  
tags/migration-20230420-pull-request

for you to fetch changes up to cdf07846e6fe07a2e20c93eed5902114dc1d3dcf:

   migration: Pass migrate_caps_check() the old and new caps (2023-04-20 
15:10:58 +0200)


Migration Pull request

This series include everything reviewed for migration:

- fix for disk stop/start (eric)
- detect filesystem of hostmem (peter)
- rename qatomic_mb_read (paolo)
- whitespace cleanup (李皆俊)
   I hope copy and paste work for the name O:-)
- atomic_counters series (juan)
- two first patches of capabilities (juan)

Please apply,


Fails CI:
https://gitlab.com/qemu-project/qemu/-/jobs/4159279870#L2896

/usr/lib/gcc-cross/mipsel-linux-gnu/10/../../../../mipsel-linux-gnu/bin/ld: 
libcommon.fa.p/migration_migration.c.o: undefined reference to symbol 
'__atomic_load_8@@LIBATOMIC_1.0'


You're using an atomic 8-byte operation on a host that doesn't support it.  Did you use 
qatomic_read__nocheck instead of qatomic_read to try and get around a build failure on 
i686?  The check is there for a reason...



r~

Re: [PULL 0/7] Merge tpm 2023/04/20 v1

[Richard Henderson]

On 4/20/23 13:32, Stefan Berger wrote:

Hello!

   This series provides TPM I2C device model support along with test cases.

Regards,
Stefan

The following changes since commit 2d82c32b2ceaca3dc3da5e36e10976f34bfcb598:

   Open 8.1 development tree (2023-04-20 10:05:25 +0100)

are available in the Git repository at:

   https://github.com/stefanberger/qemu-tpm.git  tags/pull-tpm-2023-04-20-1

for you to fetch changes up to 9d81aa3c0fe7480d722517f69e1bcb4aeaaf859c:

   qtest: Add a test case for TPM TIS I2C connected to Aspeed I2C controller 
(2023-04-20 08:17:15 -0400)


Joel Stanley (1):
   tests/avocado/aspeed: Add TPM TIS I2C test

Ninad Palsule (3):
   docs: Add support for TPM devices over I2C bus
   tpm: Extend common APIs to support TPM TIS I2C
   tpm: Add support for TPM device over I2C bus

Stefan Berger (3):
   qtest: Add functions for accessing devices on Aspeed I2C controller
   qtest: Move tpm_util_tis_transmit() into tpm-tis-utils.c and rename it
   qtest: Add a test case for TPM TIS I2C connected to Aspeed I2C
 controller


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as 
appropriate.


r~

[PATCH 1/2] tests/tcg/multiarch: Make the system memory test work on big-endian

[Ilya Leoshkevich]

Make sure values are stored in memory as little-endian regardless of
the host endianness.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/multiarch/system/memory.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/tests/tcg/multiarch/system/memory.c 
b/tests/tcg/multiarch/system/memory.c
index 214f7d4f54b..8efb440 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -121,6 +121,9 @@ static void init_test_data_u16(int offset)
 for (i = 0; i < max; i++) {
 uint8_t low = count++, high = count++;
 word = BYTE_SHIFT(high, 1) | BYTE_SHIFT(low, 0);
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap16(word);
+#endif
 *ptr++ = word;
 pdot(i);
 }
@@ -142,6 +145,9 @@ static void init_test_data_u32(int offset)
 uint8_t b4 = count++, b3 = count++;
 uint8_t b2 = count++, b1 = count++;
 word = BYTE_SHIFT(b1, 3) | BYTE_SHIFT(b2, 2) | BYTE_SHIFT(b3, 1) | b4;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap32(word);
+#endif
 *ptr++ = word;
 pdot(i);
 }
@@ -167,6 +173,9 @@ static void init_test_data_u64(int offset)
 word = BYTE_SHIFT(b1, 7) | BYTE_SHIFT(b2, 6) | BYTE_SHIFT(b3, 5) |
BYTE_SHIFT(b4, 4) | BYTE_SHIFT(b5, 3) | BYTE_SHIFT(b6, 2) |
BYTE_SHIFT(b7, 1) | b8;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap64(word);
+#endif
 *ptr++ = word;
 pdot(i);
 }
@@ -184,6 +193,9 @@ static bool read_test_data_u16(int offset)
 for (i = 0; i < max; i++) {
 uint8_t high, low;
 word = *ptr++;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap16(word);
+#endif
 high = (word >> 8) & 0xff;
 low = word & 0xff;
 if (high < low && high != 0) {
@@ -210,6 +222,9 @@ static bool read_test_data_u32(int offset)
 uint8_t b1, b2, b3, b4;
 int zeros = 0;
 word = *ptr++;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap32(word);
+#endif
 
 b1 = word >> 24 & 0xff;
 b2 = word >> 16 & 0xff;
@@ -251,6 +266,9 @@ static bool read_test_data_u64(int offset)
 uint8_t b1, b2, b3, b4, b5, b6, b7, b8;
 int zeros = 0;
 word = *ptr++;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+word = __builtin_bswap64(word);
+#endif
 
 b1 = ((uint64_t) (word >> 56)) & 0xff;
 b2 = ((uint64_t) (word >> 48)) & 0xff;
@@ -376,6 +394,9 @@ static bool read_test_data_s16(int offset, bool neg_first)
 
 for (i = 0; i < max; i++) {
 int32_t data = *ptr++;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+data = __builtin_bswap16(data);
+#endif
 
 if (neg_first && data < 0) {
 pdot(i);
@@ -401,6 +422,9 @@ static bool read_test_data_s32(int offset, bool neg_first)
 
 for (i = 0; i < max; i++) {
 int64_t data = *ptr++;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+data = __builtin_bswap32(data);
+#endif
 
 if (neg_first && data < 0) {
 pdot(i);
-- 
2.39.2

[PATCH 2/2] tests/tcg/s390x: Enable the multiarch system tests

[Ilya Leoshkevich]

Multiarch tests are written in C and need support for printing
characters. Instead of implementing the runtime from scratch, just
reuse the pc-bios/s390-ccw one.

Run tests with -nographic in order to enable SCLP (enable this for
the existing tests as well, since it does not hurt).

Use the default linker script for the new tests.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.softmmu-target | 37 +
 tests/tcg/s390x/console.c   | 12 
 tests/tcg/s390x/head64.S| 31 +
 3 files changed, 69 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/s390x/console.c
 create mode 100644 tests/tcg/s390x/head64.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target 
b/tests/tcg/s390x/Makefile.softmmu-target
index 3e7f72abcdc..2c42526e9cd 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -1,25 +1,40 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
-QEMU_OPTS=-action panic=exit-failure -kernel
+QEMU_OPTS=-action panic=exit-failure -nographic -kernel
 LINK_SCRIPT=$(S390X_SRC)/softmmu.ld
-LDFLAGS=-nostdlib -static -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none
+CFLAGS+=-ggdb -O0
+LDFLAGS=-nostdlib -static
 
 %.o: %.S
$(CC) -march=z13 -m64 -c $< -o $@
 
+%.o: %.c
+   $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -march=z13 -m64 -c $< -o $@
+
 %: %.o $(LINK_SCRIPT)
$(CC) $< -o $@ $(LDFLAGS)
 
-TESTS += unaligned-lowcore
-TESTS += bal
-TESTS += sam
-TESTS += lpsw
-TESTS += lpswe-early
-TESTS += ssm-early
-TESTS += stosm-early
-TESTS += exrl-ssm-early
+ASM_TESTS =
\
+bal
\
+exrl-ssm-early 
\
+sam
\
+lpsw   
\
+lpswe-early
\
+ssm-early  
\
+stosm-early
\
+unaligned-lowcore
 
 include $(S390X_SRC)/pgm-specification.mak
 $(PGM_SPECIFICATION_TESTS): pgm-specification-softmmu.o
 $(PGM_SPECIFICATION_TESTS): LDFLAGS+=pgm-specification-softmmu.o
-TESTS += $(PGM_SPECIFICATION_TESTS)
+ASM_TESTS += $(PGM_SPECIFICATION_TESTS)
+
+$(ASM_TESTS): LDFLAGS += -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none
+TESTS += $(ASM_TESTS)
+
+S390X_MULTIARCH_RUNTIME_OBJS = head64.o console.o $(MINILIB_OBJS)
+$(MULTIARCH_TESTS): $(S390X_MULTIARCH_RUNTIME_OBJS)
+$(MULTIARCH_TESTS): LDFLAGS += $(S390X_MULTIARCH_RUNTIME_OBJS)
+$(MULTIARCH_TESTS): CFLAGS += $(MINILIB_INC)
+memory: CFLAGS += -DCHECK_UNALIGNED=0
+TESTS += $(MULTIARCH_TESTS)
diff --git a/tests/tcg/s390x/console.c b/tests/tcg/s390x/console.c
new file mode 100644
index 000..d43ce3f44b4
--- /dev/null
+++ b/tests/tcg/s390x/console.c
@@ -0,0 +1,12 @@
+/*
+ * Console code for multiarch tests.
+ * Reuses the pc-bios/s390-ccw implementation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "../../../pc-bios/s390-ccw/sclp.c"
+
+void __sys_outc(char c)
+{
+write(1, &c, sizeof(c));
+}
diff --git a/tests/tcg/s390x/head64.S b/tests/tcg/s390x/head64.S
new file mode 100644
index 000..c6f36dfea4b
--- /dev/null
+++ b/tests/tcg/s390x/head64.S
@@ -0,0 +1,31 @@
+/*
+ * Startup code for multiarch tests.
+ * Reuses the pc-bios/s390-ccw implementation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#define main main_pre
+#include "../../../pc-bios/s390-ccw/start.S"
+#undef main
+
+main_pre:
+aghi %r15,-160 /* reserve stack for C code */
+brasl %r14,sclp_setup
+brasl %r14,main
+larl %r1,success_psw   /* check main() return code */
+ltgr %r2,%r2
+je 0f
+larl %r1,failure_psw
+0:
+lpswe 0(%r1)
+
+.align 8
+success_psw:
+.quad 0x200018000,0xfff/* see is_special_wait_psw() */
+failure_psw:
+.quad 0x200018000,0/* disabled wait */
+
+.section .bss
+.align 0x1000
+stack:
+.skip 0x8000
-- 
2.39.2

[PATCH 0/2] tests/tcg/s390x: Enable the multiarch system tests

[Ilya Leoshkevich]

Hi,

I noticed that Alex added "undefine MULTIARCH_TESTS" to
tests/tcg/s390x/Makefile.softmmu-target in the plugin patch, and
thought that it may better to just enable them, which this series
does.

Patch 1 fixes an endianness issue in the memory test.

Patch 2 enables the multiarch system test. The main difficulty is
outputting characters via SCLP, which is sidestepped by reusing the
pc-bios/s390-ccw implementation.

Best regards,
Ilya

Ilya Leoshkevich (2):
  tests/tcg/multiarch: Make the system memory test work on big-endian
  tests/tcg/s390x: Enable the multiarch system tests

 tests/tcg/multiarch/system/memory.c | 24 
 tests/tcg/s390x/Makefile.softmmu-target | 37 +
 tests/tcg/s390x/console.c   | 12 
 tests/tcg/s390x/head64.S| 31 +
 4 files changed, 93 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/s390x/console.c
 create mode 100644 tests/tcg/s390x/head64.S

-- 
2.39.2

Re: [RFC PATCH 00/13] gfxstream + rutabaga_gfx: a surprising delight or startling epiphany?

[Gurchetan Singh]

On Fri, Apr 21, 2023 at 9:02 AM Stefan Hajnoczi  wrote:
>
> On Thu, 20 Apr 2023 at 21:13, Gurchetan Singh
>  wrote:
> >
> > From: Gurchetan Singh 
> >
> > Rationale:
> >
> > - gfxstream [a] is good for the Android Emulator/upstream QEMU
> >   alignment
> > - Wayland passhthrough [b] via the cross-domain context type is good
> >   for Linux on Linux display virtualization
> > - rutabaga_gfx [c] sits on top of gfxstream, cross-domain and even
> >   virglrenderer
> > - This series ports rutabaga_gfx to QEMU
>
> What rutabaga_gfx and gfxstream? Can you explain where they sit in the
> stack and how they build on or complement virtio-gpu and
> virglrenderer?

rutabaga_gfx and gfxstream are both libraries that implement the
virtio-gpu protocol.  There's a document available in the Gitlab issue
to see where they fit in the stack [a].

gfxstream grew out of the Android Emulator's need to virtualize
graphics for app developers.  There's a short history of gfxstream in
patch 10.  It complements virglrenderer in that it's a bit more
cross-platform and targets different use cases -- more detail here
[b].  The ultimate goal is ditch out-of-tree kernel drivers in the
Android Emulator and adopt virtio, and porting gfxstream to QEMU would
speed up that transition.

rutabaga_gfx is a much smaller Rust library that sits on top of
gfxstream and even virglrenderer, but does a few extra things.  It
implements the cross-domain context type, which provides Wayland
passthrough.  This helps virtio-gpu by providing more modern display
virtualization.  For example, Microsoft for WSL2 also uses a similar
technique [c], but I believe it is not virtio-based nor open-source.
With this, we can have the same open-source Wayland passthrough
solution on crosvm, QEMU and even Fuchsia [d].  Also, there might be
an additional small Rust context type for security-sensitive use cases
in the future -- rutabaga_gfx wouldn't compile its gfxstream bindings
(since it's C++ based) in such cases.

Both gfxstream and rutabaga_gfx are a part of the virtio spec [e] now too.

[a] https://gitlab.com/qemu-project/qemu/-/issues/1611
[b] https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg04271.html
[c] https://www.youtube.com/watch?v=EkNBsBx501Q
[d] https://fuchsia-review.googlesource.com/c/fuchsia/+/778764
[e] 
https://github.com/oasis-tcs/virtio-spec/blob/master/device-types/gpu/description.tex#L533

>
> Stefan

Re: [RFC PATCH 13/13] HACK: schedule fence return on main AIO context

[Gurchetan Singh]

On Fri, Apr 21, 2023 at 9:00 AM Stefan Hajnoczi  wrote:
>
> On Thu, 20 Apr 2023 at 21:13, Gurchetan Singh
>  wrote:
> >
> > gfxstream and both cross-domain (and even newer versions
> > virglrenderer: see VIRGL_RENDERER_ASYNC_FENCE_CB) like to signal
> > fence completion on threads ("callback threads") that are
> > different from the thread that processes the command queue
> > ("main thread").
> >
> > This is generally possible with locking, and this what we do
> > in crosvm and other virtio-gpu1.1 implementations.  However, on
> > QEMU a deadlock is observed if virtio_gpu_ctrl_response_nodata(..)
> > [used in the fence callback] is used from a thread that is not the
> > main thread.
> >
> > The reason is the main thread takes the big QEMU lock (bql) somewhere
> > when processing the command queue, and virtio_gpu_ctrl_response_nodata(..)
> > needs that lock.  If you add in the lock needed to protect &g->fenceq
> > from concurrent access by the main thread and the callback threads,
> > you end can end up with deadlocks.
> >
> > It's possible to workaround this by scheduling the return of the fence
> > descriptors via aio_bh_schedule_oneshot_full(..), but that somewhat
> > negates the rationale for the asynchronous callbacks.
> >
> > I also played around with aio_context_acquire()/aio_context_release(),
> > doesn't seem to help.
> >
> > Is signaling the virtio_queue outside of the main thread possible?  If
> > so, how?
>
> Yes, if you want a specific thread to process a virtqueue, monitor the
> virtqueue's host_notifier (aka ioeventfd) from the thread. That's how
> --device virtio-blk,iothread= works. It attaches the host_notifier to
> the IOThread's AioContext. Whenever the guest kicks the virtqueue, the
> IOThread will respond instead of QEMU's main loop thread.
>
> That said, I don't know the threading model of your code. Could you
> explain which threads are involved? Do you want to process all buffers
> from virtqueue in a specific thread or only these fence buffers?

Only the fence callback would be signalled via these callback threads.
The virtqueue would not be processed by the callback thread.

There can be multiple callback threads: for example, one for the
gfxstream context and another for the Wayland context.  These threads
could be a C++ thread, a Rust thread or any other.

The strategy used by crosvm is to have a mutex around the fence state
to synchronize between multiple callback threads (which signal fences)
and main thread (which generates fences).

I tried to use aio_context_acquire(..)/aio_context_release(..) to
synchronize these threads, but that results in a deadlock.  I think
those functions may assume an IOThread and not necessarily any thread.
It seems aio_context_acquire(..) succeeds for the callback thread
though.

Here's what tried (rather than this patch which works, but is less
ideal than the solution below):

diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 196267aac2..0993b09090 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -798,6 +798,7 @@ virtio_gpu_rutabaga_process_cmd(VirtIOGPU *g,
 }

 if (cmd->finished) {
+g_free(cmd);
 return;
 }
 if (cmd->error) {
@@ -811,6 +812,8 @@ virtio_gpu_rutabaga_process_cmd(VirtIOGPU *g,
 return;
 }

+QTAILQ_INSERT_TAIL(&g->fenceq, cmd, next);
+
 fence.flags = cmd->cmd_hdr.flags;
 fence.ctx_id = cmd->cmd_hdr.ctx_id;
 fence.fence_id = cmd->cmd_hdr.fence_id;
@@ -827,9 +830,11 @@ virtio_gpu_rutabaga_fence_cb(uint64_t user_data,
  struct rutabaga_fence fence_data)
 {
 VirtIOGPU *g = (VirtIOGPU *)(void*)(uintptr_t)user_data;
+GET_VIRTIO_GPU_GL(g);
 struct virtio_gpu_ctrl_command *cmd, *tmp;

 bool signaled_ctx_specific = fence_data.flags &
RUTABAGA_FLAG_INFO_RING_IDX;
+aio_context_acquire(virtio_gpu->ctx);

 QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
 /*
@@ -856,6 +861,7 @@ virtio_gpu_rutabaga_fence_cb(uint64_t user_data,
 QTAILQ_REMOVE(&g->fenceq, cmd, next);
 g_free(cmd);
 }
+aio_context_release(virtio_gpu->ctx);
 }

 static int virtio_gpu_rutabaga_init(VirtIOGPU *g)
@@ -912,6 +918,7 @@ static int virtio_gpu_rutabaga_init(VirtIOGPU *g)
 free(channels.channels);
 }

+virtio_gpu->ctx = qemu_get_aio_context();
 return result;
 }

@@ -942,6 +949,30 @@ static int
virtio_gpu_rutabaga_get_num_capsets(VirtIOGPU *g)
 return (int)(num_capsets);
 }

+static void virtio_gpu_rutabaga_process_cmdq(VirtIOGPU *g)
+{
+struct virtio_gpu_ctrl_command *cmd;
+VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
+
+if (g->processing_cmdq) {
+return;
+}
+g->processing_cmdq = true;
+while (!QTAILQ_EMPTY(&g->cmdq)) {
+cmd = QTAILQ_FIRST(&g->cmdq);
+
+if (g->parent_obj.renderer_blocked) {
+break;
+}
+
+QTAILQ_REMOVE(&g->cmdq, cmd, next);
+
+/* pro

Re: [PATCH v2 17/54] tcg: Introduce tcg_out_xchg

[Philippe Mathieu-Daudé]

On 22/4/23 01:05, Philippe Mathieu-Daudé wrote:

On 11/4/23 03:04, Richard Henderson wrote:

We will want a backend interface for register swapping.
This is only properly defined for x86; all others get a
stub version that always indicates failure.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c    | 2 ++
  tcg/aarch64/tcg-target.c.inc | 5 +
  tcg/arm/tcg-target.c.inc | 5 +
  tcg/i386/tcg-target.c.inc    | 8 
  tcg/loongarch64/tcg-target.c.inc | 5 +
  tcg/mips/tcg-target.c.inc    | 5 +
  tcg/ppc/tcg-target.c.inc | 5 +
  tcg/riscv/tcg-target.c.inc   | 5 +
  tcg/s390x/tcg-target.c.inc   | 5 +
  tcg/sparc64/tcg-target.c.inc | 5 +
  tcg/tci/tcg-target.c.inc | 5 +
  11 files changed, 55 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 328e018a80..fde5ccc57c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -115,6 +115,8 @@ static void tcg_out_exts_i32_i64(TCGContext *s, 
TCGReg ret, TCGReg arg);
  static void tcg_out_extu_i32_i64(TCGContext *s, TCGReg ret, TCGReg 
arg);
  static void tcg_out_extrl_i64_i32(TCGContext *s, TCGReg ret, TCGReg 
arg);
  static void tcg_out_addi_ptr(TCGContext *s, TCGReg, TCGReg, 
tcg_target_long);
+static bool tcg_out_xchg(TCGContext *s, TCGType type, TCGReg r1, 
TCGReg r2)

+    __attribute__((unused));


Can you document this in docs/devel/tcg-ops.rst?


Oops this is the backend, thus not needed.


Otherwise,

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 17/54] tcg: Introduce tcg_out_xchg

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will want a backend interface for register swapping.
This is only properly defined for x86; all others get a
stub version that always indicates failure.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c| 2 ++
  tcg/aarch64/tcg-target.c.inc | 5 +
  tcg/arm/tcg-target.c.inc | 5 +
  tcg/i386/tcg-target.c.inc| 8 
  tcg/loongarch64/tcg-target.c.inc | 5 +
  tcg/mips/tcg-target.c.inc| 5 +
  tcg/ppc/tcg-target.c.inc | 5 +
  tcg/riscv/tcg-target.c.inc   | 5 +
  tcg/s390x/tcg-target.c.inc   | 5 +
  tcg/sparc64/tcg-target.c.inc | 5 +
  tcg/tci/tcg-target.c.inc | 5 +
  11 files changed, 55 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 328e018a80..fde5ccc57c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -115,6 +115,8 @@ static void tcg_out_exts_i32_i64(TCGContext *s, TCGReg ret, 
TCGReg arg);
  static void tcg_out_extu_i32_i64(TCGContext *s, TCGReg ret, TCGReg arg);
  static void tcg_out_extrl_i64_i32(TCGContext *s, TCGReg ret, TCGReg arg);
  static void tcg_out_addi_ptr(TCGContext *s, TCGReg, TCGReg, tcg_target_long);
+static bool tcg_out_xchg(TCGContext *s, TCGType type, TCGReg r1, TCGReg r2)
+__attribute__((unused));


Can you document this in docs/devel/tcg-ops.rst?

Otherwise,

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 16/54] tcg: Introduce tcg_out_movext

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

This is common code in most qemu_{ld,st} slow paths, extending the
input value for the store helper data argument or extending the
return value from the load helper.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c| 63 
  tcg/aarch64/tcg-target.c.inc |  8 +---
  tcg/arm/tcg-target.c.inc | 16 ++--
  tcg/i386/tcg-target.c.inc| 30 +++
  tcg/loongarch64/tcg-target.c.inc | 53 ---
  tcg/ppc/tcg-target.c.inc | 38 +--
  tcg/riscv/tcg-target.c.inc   | 13 +--
  tcg/s390x/tcg-target.c.inc   | 19 ++
  tcg/sparc64/tcg-target.c.inc | 32 
  9 files changed, 104 insertions(+), 168 deletions(-)




diff --git a/tcg/arm/tcg-target.c.inc b/tcg/arm/tcg-target.c.inc
index 1820655ee3..f865294861 100644
--- a/tcg/arm/tcg-target.c.inc
+++ b/tcg/arm/tcg-target.c.inc
@@ -1567,17 +1567,7 @@ static bool tcg_out_qemu_ld_slow_path(TCGContext *s, 
TCGLabelQemuLdst *lb)
  
  datalo = lb->datalo_reg;

  datahi = lb->datahi_reg;
-switch (opc & MO_SSIZE) {
-case MO_SB:
-tcg_out_ext8s(s, TCG_TYPE_I32, datalo, TCG_REG_R0);
-break;
-case MO_SW:
-tcg_out_ext16s(s, TCG_TYPE_I32, datalo, TCG_REG_R0);
-break;
-default:
-tcg_out_mov_reg(s, COND_AL, datalo, TCG_REG_R0);
-break;
-case MO_UQ:
+if ((opc & MO_SIZE) == MO_64) {
  if (datalo != TCG_REG_R1) {
  tcg_out_mov_reg(s, COND_AL, datalo, TCG_REG_R0);
  tcg_out_mov_reg(s, COND_AL, datahi, TCG_REG_R1);
@@ -1589,7 +1579,9 @@ static bool tcg_out_qemu_ld_slow_path(TCGContext *s, 
TCGLabelQemuLdst *lb)
  tcg_out_mov_reg(s, COND_AL, datahi, TCG_REG_R1);
  tcg_out_mov_reg(s, COND_AL, datalo, TCG_REG_TMP);
  }
-break;
+} else {
+tcg_out_movext(s, TCG_TYPE_I32, lb->datalo_reg,


Why not use 'datalo' like in i386?


+   TCG_TYPE_I32, opc & MO_SSIZE, TCG_REG_R0);
  }
  
  tcg_out_goto(s, COND_AL, lb->raddr);

diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index a166a195c4..4847da7e1a 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -1946,28 +1946,8 @@ static bool tcg_out_qemu_ld_slow_path(TCGContext *s, 
TCGLabelQemuLdst *l)
  tcg_out_branch(s, 1, qemu_ld_helpers[opc & (MO_BSWAP | MO_SIZE)]);
  
  data_reg = l->datalo_reg;

-switch (opc & MO_SSIZE) {
-case MO_SB:
-tcg_out_ext8s(s, l->type, data_reg, TCG_REG_EAX);
-break;
-case MO_SW:
-tcg_out_ext16s(s, l->type, data_reg, TCG_REG_EAX);
-break;
-#if TCG_TARGET_REG_BITS == 64
-case MO_SL:
-tcg_out_ext32s(s, data_reg, TCG_REG_EAX);
-break;
-#endif
-case MO_UB:
-case MO_UW:
-/* Note that the helpers have zero-extended to tcg_target_long.  */
-case MO_UL:
-tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
-break;
-case MO_UQ:
-if (TCG_TARGET_REG_BITS == 64) {
-tcg_out_mov(s, TCG_TYPE_I64, data_reg, TCG_REG_RAX);
-} else if (data_reg == TCG_REG_EDX) {
+if (TCG_TARGET_REG_BITS == 32 && (opc & MO_SIZE) == MO_64) {
+if (data_reg == TCG_REG_EDX) {
  /* xchg %edx, %eax */
  tcg_out_opc(s, OPC_XCHG_ax_r32 + TCG_REG_EDX, 0, 0, 0);
  tcg_out_mov(s, TCG_TYPE_I32, l->datahi_reg, TCG_REG_EAX);
@@ -1975,9 +1955,9 @@ static bool tcg_out_qemu_ld_slow_path(TCGContext *s, 
TCGLabelQemuLdst *l)
  tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
  tcg_out_mov(s, TCG_TYPE_I32, l->datahi_reg, TCG_REG_EDX);
  }
-break;
-default:
-g_assert_not_reached();
+} else {
+tcg_out_movext(s, l->type, data_reg,
+   TCG_TYPE_REG, opc & MO_SSIZE, TCG_REG_EAX);
  }
  
  /* Jump to the code corresponding to next IR of qemu_st */



[I'm skipping the ppc change hopping Daniel can review it]


diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 4c4178b700..b1d9c0bbe4 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -1971,10 +1971,6 @@ static const uint32_t qemu_stx_opc[(MO_SIZE + MO_BSWAP) 
+ 1] = {
  [MO_BSWAP | MO_UQ] = STDBRX,
  };
  
-static const uint32_t qemu_exts_opc[4] = {

-EXTSB, EXTSH, EXTSW, 0
-};
-
  #if defined (CONFIG_SOFTMMU)
  /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
   * int mmu_idx, uintptr_t ra)
@@ -2168,11 +2164,9 @@ static bool tcg_out_qemu_ld_slow_path(TCGContext *s, 
TCGLabelQemuLdst *lb)
  if (TCG_TARGET_REG_BITS == 32 && (opc & MO_SIZE) == MO_64) {
  tcg_out_mov(s, TCG_TYPE_I32, lo, TCG_REG_R4);
  tcg_out_mov(s, TCG_TYPE_I32, hi, TCG_REG_R3);
-} else if (opc & MO_SIGN) {
-uint32_t insn = qemu_exts_opc[

Re: [PATCH v2 15/54] tcg: Split out tcg_out_extrl_i64_i32

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for type truncation.  For those backends
that did not enable TCG_TARGET_HAS_extrl_i64_i32, use tcg_out_mov.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  4 
  tcg/aarch64/tcg-target.c.inc |  6 ++
  tcg/arm/tcg-target.c.inc |  5 +
  tcg/i386/tcg-target.c.inc|  9 ++---
  tcg/loongarch64/tcg-target.c.inc | 10 ++
  tcg/mips/tcg-target.c.inc|  9 ++---
  tcg/ppc/tcg-target.c.inc |  7 +++
  tcg/riscv/tcg-target.c.inc   | 10 ++
  tcg/s390x/tcg-target.c.inc   |  6 ++
  tcg/sparc64/tcg-target.c.inc |  9 ++---
  tcg/tci/tcg-target.c.inc |  7 +++
  11 files changed, 65 insertions(+), 17 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 13/54] tcg: Split out tcg_out_extu_i32_i64

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for type extension with zero.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  4 
  tcg/aarch64/tcg-target.c.inc | 10 ++
  tcg/arm/tcg-target.c.inc |  5 +
  tcg/i386/tcg-target.c.inc|  7 ++-
  tcg/loongarch64/tcg-target.c.inc | 10 ++
  tcg/mips/tcg-target.c.inc|  9 ++---
  tcg/ppc/tcg-target.c.inc | 10 ++
  tcg/riscv/tcg-target.c.inc   | 10 ++
  tcg/s390x/tcg-target.c.inc   | 10 ++
  tcg/sparc64/tcg-target.c.inc |  9 ++---
  tcg/tci/tcg-target.c.inc |  7 ++-
  11 files changed, 63 insertions(+), 28 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v4 11/11] Signed-off-by: Karim Taha

[Warner Losh]

Oh, I see you've uploaded an improved version of the patches, with the
fixes I'd
recommended. I'll queue that series instead.

Warner

On Fri, Apr 21, 2023 at 10:58 AM Karim Taha 
wrote:

> From: Warner Losh 
>
> Add the dispatching code of bind(2),connect(2), accpet(2), getpeername(2).
>
> Add the bind(2), connect(2), accept(2), getpeername(2) syscalls case
> statements to freebsd_syscall function defined in
> bsd-user/freebsd/os-syscall.c
>
> Signed-off-by: Warner Losh 
> Signed-off-by: Karim Taha 
> ---
>  bsd-user/freebsd/os-syscall.c | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index c8f998ecec..7f29196a05 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -44,6 +44,8 @@
>  #include "signal-common.h"
>  #include "user/syscall-trace.h"
>
> +/* BSD independent syscall shims */
> +#include "bsd-socket.h"
>  #include "bsd-file.h"
>  #include "bsd-proc.h"
>
> @@ -508,6 +510,25 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>  ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
>  break;
>
> +/*
> + * socket related system calls
> + */
> +case TARGET_FREEBSD_NR_accept: /* accept(2) */
> +ret = do_bsd_accept(arg1, arg2, arg3);
> +break;
> +
> +case TARGET_FREEBSD_NR_bind: /* bind(2) */
> +ret = do_bsd_bind(arg1, arg2, arg3);
> +break;
> +
> +case TARGET_FREEBSD_NR_connect: /* connect(2) */
> +ret = do_bsd_connect(arg1, arg2, arg3);
> +break;
> +
> +case TARGET_FREEBSD_NR_getpeername: /* getpeername(2) */
> +ret = do_bsd_getpeername(arg1, arg2, arg3);
> +break;
> +
>  default:
>  qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
>  ret = -TARGET_ENOSYS;
> --
> 2.40.0
>
>

Re: [PATCH v2 09/54] tcg: Split out tcg_out_exts_i32_i64

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for type extension with sign.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c| 4 
  tcg/aarch64/tcg-target.c.inc | 9 ++---
  tcg/arm/tcg-target.c.inc | 5 +
  tcg/i386/tcg-target.c.inc| 9 ++---
  tcg/loongarch64/tcg-target.c.inc | 7 ++-
  tcg/mips/tcg-target.c.inc| 7 ++-
  tcg/ppc/tcg-target.c.inc | 9 ++---
  tcg/riscv/tcg-target.c.inc   | 7 ++-
  tcg/s390x/tcg-target.c.inc   | 9 ++---
  tcg/sparc64/tcg-target.c.inc | 9 ++---
  tcg/tci/tcg-target.c.inc | 7 ++-
  11 files changed, 63 insertions(+), 19 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 01/11] Signed-off-by: Karim Taha

[Warner Losh]

On Fri, Apr 21, 2023 at 10:42 AM Karim Taha 
wrote:

> From: Warner Losh 
>
> Intialize guest_base in bsd-user/main.c.
>
> Allow guest_base to be initialized on 64-bit hosts, the initial value is
> used by g2h_untagged function defined in include/exec/cpu_ldst.h
>
> Signed-off-by: Karim Taha 
>

This is missing the 'Signed-off-by' line of the author (me in this case).
All the others suffer from this as well.

I'll see about downloading these patches and testing them locally. The
changes themselves look good
to me as far as breaking them up, at least initially. I'll take a closer
look when I queue them for upstreaming
to see what else I can recommend.

Warner

---
>  bsd-user/main.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index babc3b009b..afdc1b5f3c 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -50,8 +50,22 @@
>  #include "target_arch_cpu.h"
>
>  int singlestep;
> -uintptr_t guest_base;
> +
> +/*
> + * Going hand in hand with the va space needed (see below), we need
> + * to find a host address to map the guest to. Assume that qemu
> + * itself doesn't need memory above 32GB (or that we don't collide
> + * with anything interesting). This is selected rather arbitrarily,
> + * but seems to produce good results in tests to date.
> + */
> +# if HOST_LONG_BITS >= 64
> +uintptr_t guest_base = 0x8ul;/* at 32GB */
> +bool have_guest_base = true;
> +#else
> +uintptr_t guest_base;/* TODO: use sysctl to find big enough hole */
>  bool have_guest_base;
> +#endif
> +
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the
> possible
>   * guest address space into a contiguous chunk of virtual host memory.
> --
> 2.40.0
>
>

Re: [PATCH v2 07/54] tcg: Split out tcg_out_ext32s

[Philippe Mathieu-Daudé]

On 22/4/23 00:38, Philippe Mathieu-Daudé wrote:

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 32-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c    |  4 
  tcg/aarch64/tcg-target.c.inc |  9 +++--
  tcg/arm/tcg-target.c.inc |  5 +
  tcg/i386/tcg-target.c.inc    |  5 +++--
  tcg/loongarch64/tcg-target.c.inc |  2 +-
  tcg/mips/tcg-target.c.inc    | 12 +---
  tcg/ppc/tcg-target.c.inc |  5 +++--
  tcg/riscv/tcg-target.c.inc   |  2 +-
  tcg/s390x/tcg-target.c.inc   | 10 +-
  tcg/sparc64/tcg-target.c.inc | 11 ---
  tcg/tci/tcg-target.c.inc |  9 -
  11 files changed, 54 insertions(+), 20 deletions(-)





diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index f55829e9ce..d7964734c3 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -1429,6 +1429,11 @@ static void tcg_out_ext16s(TCGContext *s, 
TCGType type, TCGReg rd, TCGReg rn)

  tcg_out_sxt(s, type, MO_16, rd, rn);
  }
+static void tcg_out_ext32s(TCGContext *s, TCGReg rd, TCGReg rn)
+{
+    tcg_out_sxt(s, TCG_TYPE_I64, MO_32, rd, rn);
+}
+
  static inline void tcg_out_uxt(TCGContext *s, MemOp s_bits,
 TCGReg rd, TCGReg rn)
  {
@@ -2232,7 +2237,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode 
opc,

  case INDEX_op_bswap32_i64:
  tcg_out_rev(s, TCG_TYPE_I32, MO_32, a0, a1);
  if (a2 & TCG_BSWAP_OS) {
-    tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a0);
+    tcg_out_ext32s(s, a0, a0);
  }
  break;
  case INDEX_op_bswap32_i32:
@@ -2251,7 +2256,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode 
opc,

  break;
  case INDEX_op_ext_i32_i64:
-    case INDEX_op_ext32s_i64:
  tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a1);


While here, maybe reuse the new helper (easier to read):

     tcg_out_ext32s(s, a0, a1);


Now I see you do that in tcg_out_exts_i32_i64() in 2 patches :)


  break;
  case INDEX_op_extu_i32_i64:
@@ -2322,6 +2326,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode 
opc,

  case INDEX_op_ext16s_i32:
  case INDEX_op_ext16u_i64:
  case INDEX_op_ext16u_i32:
+    case INDEX_op_ext32s_i64:
  default:
  g_assert_not_reached();
  }


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 01/11] Signed-off-by: Karim Taha

[Karim Taha]

It was sent with git-publish, what do you mean by pointing to a branch?

On Fri, Apr 21, 2023 at 7:22 PM Alex Bennée  wrote:

>
> Karim Taha  writes:
>
> > On Fri, Apr 21, 2023 at 9:17 AM Daniel P. Berrangé 
> wrote:
> >
> >  On Fri, Apr 21, 2023 at 07:22:45AM +0200, Karim Taha wrote:
> >  > From: Warner Losh 
> >  >
> >  > Allow guest_base to be initialized on 64-bit hosts, the initial value
> is used by g2h_untagged function
> >  defined in include/exec/cpu_ldst.h
> >
> >  This commit message is all incorrectly structured I'm afraid.
> >
> >  There needs to a short 1 line summary, then a blank line,
> >  then the full commit description text, then a blank line,
> >  then the Signed-off-by tag(s).
> >
> >  Also if you're sending work done by Warner (as the From
> >  tag suggests), then we would expect to see Warner's own
> >  Signed-off-by tag, in addition to your own Signed-off-by.
> 
> >
> > Alright, thanks for the commit formatting tips, I resent the patch
> series, with my signed off by tag and the
> > author signed off by tags as well.
>
> Hmm something has gone wrong. Was this sent with a plain git-send-email
> or using a tool like git-publish?
>
> Can you point to a branch?
>
> >
> > Best regards,
> > Karim
>
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

Re: [PATCH v2 08/54] tcg: Split out tcg_out_ext32u

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 32-bit zero-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  4 
  tcg/aarch64/tcg-target.c.inc |  9 +++--
  tcg/arm/tcg-target.c.inc |  5 +
  tcg/i386/tcg-target.c.inc|  4 ++--
  tcg/loongarch64/tcg-target.c.inc |  2 +-
  tcg/mips/tcg-target.c.inc|  3 ++-
  tcg/ppc/tcg-target.c.inc |  4 +++-
  tcg/riscv/tcg-target.c.inc   |  2 +-
  tcg/s390x/tcg-target.c.inc   | 20 ++--
  tcg/sparc64/tcg-target.c.inc | 17 +++--
  tcg/tci/tcg-target.c.inc |  9 -
  11 files changed, 54 insertions(+), 25 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 07/54] tcg: Split out tcg_out_ext32s

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 32-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  4 
  tcg/aarch64/tcg-target.c.inc |  9 +++--
  tcg/arm/tcg-target.c.inc |  5 +
  tcg/i386/tcg-target.c.inc|  5 +++--
  tcg/loongarch64/tcg-target.c.inc |  2 +-
  tcg/mips/tcg-target.c.inc| 12 +---
  tcg/ppc/tcg-target.c.inc |  5 +++--
  tcg/riscv/tcg-target.c.inc   |  2 +-
  tcg/s390x/tcg-target.c.inc   | 10 +-
  tcg/sparc64/tcg-target.c.inc | 11 ---
  tcg/tci/tcg-target.c.inc |  9 -
  11 files changed, 54 insertions(+), 20 deletions(-)





diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index f55829e9ce..d7964734c3 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -1429,6 +1429,11 @@ static void tcg_out_ext16s(TCGContext *s, TCGType type, 
TCGReg rd, TCGReg rn)
  tcg_out_sxt(s, type, MO_16, rd, rn);
  }
  
+static void tcg_out_ext32s(TCGContext *s, TCGReg rd, TCGReg rn)

+{
+tcg_out_sxt(s, TCG_TYPE_I64, MO_32, rd, rn);
+}
+
  static inline void tcg_out_uxt(TCGContext *s, MemOp s_bits,
 TCGReg rd, TCGReg rn)
  {
@@ -2232,7 +2237,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
  case INDEX_op_bswap32_i64:
  tcg_out_rev(s, TCG_TYPE_I32, MO_32, a0, a1);
  if (a2 & TCG_BSWAP_OS) {
-tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a0);
+tcg_out_ext32s(s, a0, a0);
  }
  break;
  case INDEX_op_bswap32_i32:
@@ -2251,7 +2256,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
  break;
  
  case INDEX_op_ext_i32_i64:

-case INDEX_op_ext32s_i64:
  tcg_out_sxt(s, TCG_TYPE_I64, MO_32, a0, a1);


While here, maybe reuse the new helper (easier to read):

tcg_out_ext32s(s, a0, a1);


  break;
  case INDEX_op_extu_i32_i64:
@@ -2322,6 +2326,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
  case INDEX_op_ext16s_i32:
  case INDEX_op_ext16u_i64:
  case INDEX_op_ext16u_i32:
+case INDEX_op_ext32s_i64:
  default:
  g_assert_not_reached();
  }


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 31/54] tcg: Move TCGLabelQemuLdst to tcg.c

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

This will shortly be used by sparc64 without also using
TCG_TARGET_NEED_LDST_LABELS.


Is that in this series?


Signed-off-by: Richard Henderson 
---
  tcg/tcg.c  | 13 +
  tcg/tcg-ldst.c.inc | 14 --
  2 files changed, 13 insertions(+), 14 deletions(-)

Re: [PATCH v2 30/54] tcg/sparc64: Pass TCGType to tcg_out_qemu_{ld,st}

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We need to set this in TCGLabelQemuLdst, so plumb this
all the way through from tcg_out_op.

Signed-off-by: Richard Henderson 
---
  tcg/sparc64/tcg-target.c.inc | 15 +++
  1 file changed, 7 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 29/54] tcg/sparc64: Drop is_64 test from tcg_out_qemu_ld data return

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

In tcg_canonicalize_memop, we remove MO_SIGN from MO_32 operations
with TCG_TYPE_I32.  Thus this is never set.  We already have an
identical test just above which does not include is_64

Signed-off-by: Richard Henderson 
---
  tcg/sparc64/tcg-target.c.inc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 19/54] tcg: Clear TCGLabelQemuLdst on allocation

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  tcg/tcg-ldst.c.inc | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 21/54] tcg/aarch64: Rationalize args to tcg_out_qemu_{ld, st}

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

Mark the argument registers const, because they must be passed to
add_qemu_ldst_label unmodified.  Rename the 'ext' parameter 'data_type' to
make the use clearer; pass it to tcg_out_qemu_st as well to even out the
interfaces.  Rename the 'otype' local 'addr_type' to make the use clearer.

Signed-off-by: Richard Henderson 
---
  tcg/aarch64/tcg-target.c.inc | 42 ++--
  1 file changed, 21 insertions(+), 21 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 5/6] tests/qtest: massively speed up migration-tet

[Juan Quintela]

Daniel P. Berrangé  wrote:
> The migration test cases that actually exercise live migration want to
> ensure there is a minimum of two iterations of pre-copy, in order to
> exercise the dirty tracking code.
>
> Historically we've queried the migration status, looking for the
> 'dirty-sync-count' value to increment to track iterations. This was
> not entirely reliable because often all the data would get transferred
> quickly enough that the migration would finish before we wanted it
> to. So we massively dropped the bandwidth and max downtime to
> guarantee non-convergance. This had the unfortunate side effect
> that every migration took at least 30 seconds to run (100 MB of
> dirty pages / 3 MB/sec).
>
> This optimization takes a different approach to ensuring that a
> mimimum of two iterations. Rather than waiting for dirty-sync-count
> to increment, directly look for an indication that the source VM
> has dirtied RAM that has already been transferred.
>
> On the source VM a magic marker is written just after the 3 MB
> offset. The destination VM is now montiored to detect when the
> magic marker is transferred. This gives a guarantee that the
> first 3 MB of memory have been transferred. Now the source VM
> memory is monitored at exactly the 3MB offset until we observe
> a flip in its value. This gives us a guaranteed that the guest
> workload has dirtied a byte that has already been transferred.
>
> Since we're looking at a place that is only 3 MB from the start
> of memory, with the 3 MB/sec bandwidth, this test should complete
> in 1 second, instead of 30 seconds.
>
> Once we've proved there is some dirty memory, migration can be
> set back to full speed for the remainder of the 1st iteration,
> and the entire of the second iteration at which point migration
> should be complete.
>
> Signed-off-by: Daniel P. Berrangé 

Hi

I think this is not enough.  As said before:
- xbzrle needs 3 iterations
- auto converge needs around 12 iterations (forgot) the exact number,
  but it is a lot.
- for (almost) all the rest of the tests, we don't really care, we just
  need the migration to finish.

One easy way to "test" it is: Change the "meaning" of ZERO downtime to
mean that we don't want to enter the completion stage, just continue
sending data.

Changig this in qemu:

modified   migration/migration.c
@@ -2726,6 +2726,9 @@ static MigIterateState 
migration_iteration_run(MigrationState *s)
 
 trace_migrate_pending_estimate(pending_size, must_precopy, can_postcopy);
 
+if (s->threshold_size == 0) {
+return MIG_ITERATE_RESUME;
+}
 if (must_precopy <= s->threshold_size) {
 qemu_savevm_state_pending_exact(&must_precopy, &can_postcopy);
 pending_size = must_precopy + can_postcopy;

And just setting the downtime to zero should be enough.

It is too late, so before I start with this, what do you think?

Later, Juan.

Re: [PATCH v2 26/54] tcg/s390x: Pass TCGType to tcg_out_qemu_{ld,st}

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We need to set this in TCGLabelQemuLdst, so plumb this
all the way through from tcg_out_op.

Signed-off-by: Richard Henderson 
---
  tcg/s390x/tcg-target.c.inc | 22 ++
  1 file changed, 14 insertions(+), 8 deletions(-)




@@ -1916,7 +1917,8 @@ static void tcg_out_qemu_ld(TCGContext* s, TCGReg 
data_reg, TCGReg addr_reg,
  
  tcg_out_qemu_ld_direct(s, opc, data_reg, base_reg, TCG_REG_R2, 0);
  
-add_qemu_ldst_label(s, 1, oi, data_reg, addr_reg, s->code_ptr, label_ptr);

+add_qemu_ldst_label(s, 1, oi, data_type, data_reg, addr_reg,


s/1/true/


+s->code_ptr, label_ptr);
  #else
  TCGReg index_reg;
  tcg_target_long disp;
@@ -1931,7 +1933,7 @@ static void tcg_out_qemu_ld(TCGContext* s, TCGReg 
data_reg, TCGReg addr_reg,
  }
  
  static void tcg_out_qemu_st(TCGContext* s, TCGReg data_reg, TCGReg addr_reg,

-MemOpIdx oi)
+MemOpIdx oi, TCGType data_type)
  {
  MemOp opc = get_memop(oi);
  #ifdef CONFIG_SOFTMMU
@@ -1947,7 +1949,8 @@ static void tcg_out_qemu_st(TCGContext* s, TCGReg 
data_reg, TCGReg addr_reg,
  
  tcg_out_qemu_st_direct(s, opc, data_reg, base_reg, TCG_REG_R2, 0);
  
-add_qemu_ldst_label(s, 0, oi, data_reg, addr_reg, s->code_ptr, label_ptr);

+add_qemu_ldst_label(s, 0, oi, data_type, data_reg, addr_reg,


s/0/false/

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 01/54] tcg: Replace if + tcg_abort with tcg_debug_assert

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c | 4 +---
  tcg/i386/tcg-target.c.inc | 8 +++-
  2 files changed, 4 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 06/54] tcg: Split out tcg_out_ext16u

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 16-bit zero-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  5 +
  tcg/aarch64/tcg-target.c.inc | 13 -
  tcg/arm/tcg-target.c.inc | 17 ++---
  tcg/i386/tcg-target.c.inc|  8 +++-
  tcg/loongarch64/tcg-target.c.inc |  7 ++-
  tcg/mips/tcg-target.c.inc|  5 +
  tcg/ppc/tcg-target.c.inc |  4 +++-
  tcg/riscv/tcg-target.c.inc   |  7 ++-
  tcg/s390x/tcg-target.c.inc   | 17 ++---
  tcg/sparc64/tcg-target.c.inc | 11 +--
  tcg/tci/tcg-target.c.inc | 14 +-
  11 files changed, 66 insertions(+), 42 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 05/54] tcg: Split out tcg_out_ext16s

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 16-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  7 +++
  tcg/aarch64/tcg-target.c.inc | 13 -
  tcg/arm/tcg-target.c.inc | 10 --
  tcg/i386/tcg-target.c.inc| 16 
  tcg/loongarch64/tcg-target.c.inc | 13 +
  tcg/mips/tcg-target.c.inc| 11 ---
  tcg/ppc/tcg-target.c.inc | 12 +---
  tcg/riscv/tcg-target.c.inc   |  9 +++--
  tcg/s390x/tcg-target.c.inc   | 12 
  tcg/sparc64/tcg-target.c.inc |  7 +++
  tcg/tci/tcg-target.c.inc | 21 -
  11 files changed, 79 insertions(+), 52 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 04/54] tcg: Split out tcg_out_ext8u

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 8-bit zero-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  5 +
  tcg/aarch64/tcg-target.c.inc | 11 +++
  tcg/arm/tcg-target.c.inc | 12 +---
  tcg/i386/tcg-target.c.inc|  7 +++
  tcg/loongarch64/tcg-target.c.inc |  7 ++-
  tcg/mips/tcg-target.c.inc|  9 -
  tcg/ppc/tcg-target.c.inc |  7 +++
  tcg/riscv/tcg-target.c.inc   |  7 ++-
  tcg/s390x/tcg-target.c.inc   | 14 +-
  tcg/sparc64/tcg-target.c.inc |  9 -
  tcg/tci/tcg-target.c.inc | 14 +-
  11 files changed, 69 insertions(+), 33 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 03/54] tcg: Split out tcg_out_ext8s

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

We will need a backend interface for performing 8-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c| 21 -
  tcg/aarch64/tcg-target.c.inc | 11 +++
  tcg/arm/tcg-target.c.inc | 10 --
  tcg/i386/tcg-target.c.inc| 10 +-
  tcg/loongarch64/tcg-target.c.inc | 11 ---
  tcg/mips/tcg-target.c.inc| 12 
  tcg/ppc/tcg-target.c.inc | 10 --
  tcg/riscv/tcg-target.c.inc   |  9 +++--
  tcg/s390x/tcg-target.c.inc   | 10 +++---
  tcg/sparc64/tcg-target.c.inc |  7 +++
  tcg/tci/tcg-target.c.inc | 21 -
  11 files changed, 81 insertions(+), 51 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 02/54] tcg: Replace tcg_abort with g_assert_not_reached

[Philippe Mathieu-Daudé]

On 11/4/23 03:04, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  include/tcg/tcg.h|  6 --
  target/i386/tcg/translate.c  | 20 ++--
  target/s390x/tcg/translate.c |  4 ++--
  tcg/optimize.c   | 10 --
  tcg/tcg.c|  8 
  tcg/aarch64/tcg-target.c.inc |  4 ++--
  tcg/arm/tcg-target.c.inc |  2 +-
  tcg/i386/tcg-target.c.inc| 14 +++---
  tcg/mips/tcg-target.c.inc| 14 +++---
  tcg/ppc/tcg-target.c.inc |  8 
  tcg/s390x/tcg-target.c.inc   |  8 
  tcg/sparc64/tcg-target.c.inc |  2 +-
  tcg/tci/tcg-target.c.inc |  2 +-
  13 files changed, 47 insertions(+), 55 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 4/6] tests/qtest: make more migration pre-copy scenarios run non-live

[Juan Quintela]

Daniel P. Berrangé  wrote:
> There are 27 pre-copy live migration scenarios being tested. In all of
> these we force non-convergance and run for one iteration, then let it
> converge and wait for completion during the second (or following)
> iterations. At 3 mbps bandwidth limit the first iteration takes a very
> long time (~30 seconds).
>
> While it is important to test the migration passes and convergance
> logic, it is overkill to do this for all 27 pre-copy scenarios. The
> TLS migration scenarios in particular are merely exercising different
> code paths during connection establishment.
>
> To optimize time taken, switch most of the test scenarios to run
> non-live (ie guest CPUs paused) with no bandwidth limits. This gives
> a massive speed up for most of the test scenarios.
>
> For test coverage the following scenarios are unchanged
>
>  * Precopy with UNIX sockets
>  * Precopy with UNIX sockets and dirty ring tracking
>  * Precopy with XBZRLE
>  * Precopy with multifd
>
> Signed-off-by: Daniel P. Berrangé 

Reviewed-by: Juan Quintela 

It is "infinitely" better that what we have.

But I wonder if we can do better.  We could just add a migration
parameter that says _don't_ complete, continue running.  We have
(almost) all of the functionality that we need for colo, just not an
easy way to set it up.

Just food for thought.

Later, Juan.

Re: [PATCH 06/42] tcg: Split out tcg_out_ext16u

[Philippe Mathieu-Daudé]

On 8/4/23 04:42, Richard Henderson wrote:

We will need a backend interface for performing 16-bit zero-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  5 +
  tcg/aarch64/tcg-target.c.inc | 13 -
  tcg/arm/tcg-target.c.inc | 17 ++---
  tcg/i386/tcg-target.c.inc|  8 +++-
  tcg/loongarch64/tcg-target.c.inc |  7 ++-
  tcg/mips/tcg-target.c.inc|  5 +
  tcg/ppc/tcg-target.c.inc |  4 +++-
  tcg/riscv/tcg-target.c.inc   |  7 ++-
  tcg/s390x/tcg-target.c.inc   | 17 ++---
  tcg/sparc64/tcg-target.c.inc | 11 +--
  tcg/tci/tcg-target.c.inc | 14 +-
  11 files changed, 66 insertions(+), 42 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 3/6] tests/qtest: capture RESUME events during migration

[Juan Quintela]

Daniel P. Berrangé  wrote:
> When running migration tests we monitor for a STOP event so we can skip
> redundant waits. This will be needed for the RESUME event too shortly.
>
> Signed-off-by: Daniel P. Berrangé 

Reviewed-by: Juan Quintela 

i.e. it is better that what we have now.

But


> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index f6f3c6680f..61396335cc 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -24,14 +24,20 @@
>  #define MIGRATION_STATUS_WAIT_TIMEOUT 120
>  
>  bool got_stop;
> +bool got_resume;
>  
> -static void check_stop_event(QTestState *who)
> +static void check_events(QTestState *who)
>  {
>  QDict *event = qtest_qmp_event_ref(who, "STOP");
>  if (event) {
>  got_stop = true;
>  qobject_unref(event);
>  }
> +event = qtest_qmp_event_ref(who, "RESUME");
> +if (event) {
> +got_resume = true;
> +qobject_unref(event);
> +}
>  }

What happens if we receive the events in the order RESUME/STOP (I mean
in the big scheme of things, not that it makes sense in this particular
case).

QDict *qtest_qmp_event_ref(QTestState *s, const char *event)
{
while (s->pending_events) {

GList *first = s->pending_events;
QDict *response = (QDict *)first->data;

s->pending_events = g_list_delete_link(s->pending_events, first);

if (!strcmp(qdict_get_str(response, "event"), event)) {
return response;
}
qobject_unref(response);
}
return NULL;
}

if we don't found the event that we are searching for, we just drop it.
Does this makes sense if we are searching only for more than one event?

Later, Juan.

Re: [PATCH v2 2/6] tests/qtests: remove migration test iterations config

[Juan Quintela]

Daniel P. Berrangé  wrote:
> The 'unsigned int interations' config for migration is somewhat
> overkill. Most tests don't set it, and a value of '0' is treated
> as equivalent to '1'. The only test that does set it, xbzrle,
> used a value of '2'.
>
> This setting, however, only relates to the migration iterations
> that take place prior to allowing convergence. IOW, on top of
> this iteration count, there is always at least 1 further migration
> iteration done to deal with pages that are dirtied during the
> previous iteration(s).
>
> IOW, even with iterations==1, the xbzrle test will be running for
> a minimum of 2 iterations. With this in mind we can simplify the
> code and just get rid of the special case.

Perhaps the old code was already wrong, but we need at least three
iterations for the xbzrle test:
- 1st iteration: xbzrle is not used, nothing is on cache.
- 2nd iteration: pages are put into cache, no xbzrle is used because
  there is no previous page.
- 3rd iteration: We really use xbzrle now against the copy of the
  previous iterations.

And yes, this should be commented somewhere.

Later, Juan.

Re: [PATCH 05/42] tcg: Split out tcg_out_ext16s

[Philippe Mathieu-Daudé]

On 8/4/23 04:42, Richard Henderson wrote:

We will need a backend interface for performing 16-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  7 +++
  tcg/aarch64/tcg-target.c.inc | 13 -
  tcg/arm/tcg-target.c.inc | 10 --
  tcg/i386/tcg-target.c.inc| 16 
  tcg/loongarch64/tcg-target.c.inc | 13 +
  tcg/mips/tcg-target.c.inc| 11 ---
  tcg/ppc/tcg-target.c.inc | 12 +---
  tcg/riscv/tcg-target.c.inc   |  9 +++--
  tcg/s390x/tcg-target.c.inc   | 12 
  tcg/sparc64/tcg-target.c.inc |  7 +++
  tcg/tci/tcg-target.c.inc | 21 -
  11 files changed, 79 insertions(+), 52 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 1/6] tests/qtest: replace qmp_discard_response with qtest_qmp_assert_success

[Juan Quintela]

Daniel P. Berrangé  wrote:
> The qmp_discard_response method simply ignores the result of the QMP
> command, merely unref'ing the object. This is a bad idea for tests
> as it leaves no trace if the QMP command unexpectedly failed. The
> qtest_qmp_assert_success method will validate that the QMP command
> returned without error, and if errors occur, it will print a message
> on the console aiding debugging.
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tests/qtest/ahci-test.c  | 31 ++--
>  tests/qtest/boot-order-test.c|  5 +
>  tests/qtest/fdc-test.c   | 15 +++---
>  tests/qtest/ide-test.c   |  5 +
>  tests/qtest/migration-test.c |  5 +
>  tests/qtest/test-filter-mirror.c |  5 +
>  tests/qtest/test-filter-redirector.c |  7 ++-
>  tests/qtest/virtio-blk-test.c| 24 ++---
>  8 files changed, 40 insertions(+), 57 deletions(-)

Reviewed-by: Juan Quintela 

> -/* TODO actually test the results and get rid of this */
> -#define qmp_discard_response(s, ...) qobject_unref(qtest_qmp(s, __VA_ARGS__))

As it couldn't be otherwise, all bad patterns are copied.

Re: [PATCH 04/42] tcg: Split out tcg_out_ext8u

[Philippe Mathieu-Daudé]

On 8/4/23 04:42, Richard Henderson wrote:

We will need a backend interface for performing 8-bit zero-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c|  5 +
  tcg/aarch64/tcg-target.c.inc | 11 +++
  tcg/arm/tcg-target.c.inc | 12 +---
  tcg/i386/tcg-target.c.inc|  7 +++
  tcg/loongarch64/tcg-target.c.inc |  7 ++-
  tcg/mips/tcg-target.c.inc|  9 -
  tcg/ppc/tcg-target.c.inc |  7 +++
  tcg/riscv/tcg-target.c.inc   |  7 ++-
  tcg/s390x/tcg-target.c.inc   | 14 +-
  tcg/sparc64/tcg-target.c.inc |  9 -
  tcg/tci/tcg-target.c.inc | 14 +-
  11 files changed, 69 insertions(+), 33 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

[PATCH v3 0/3] ACPI: i386: bump MADT to revision 5

[Eric DeVolder]

The following Linux kernel change broke CPU hotplug for MADT revision
less than 5.

 e2869bd7af60 ("x86/acpi/boot: Do not register processors that cannot be 
onlined for x2APIC")

Discussion on this topic can be located here:

 
https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devol...@oracle.com/T/#t

which resulted in the following fixes Linux in 6.3-rc5:

 a74fabfbd1b7: ("x86/ACPI/boot: Use FADT version to check support for online 
capable")
 fed8d8773b8e: ("x86/acpi/boot: Correct acpi_is_processor_usable() check")

However, as part of the investigation into resolving this breakage, I
learned that i386 QEMU reports revision 1, while technically it
generates revision 3. Aarch64 generates and reports revision 4.

ACPI 6.3 bumps MADT revision to 5 as it introduces an Online Capable
flag that the above Linux patch utilizes to denote hot pluggable CPUs.

So in order to bump MADT to the current revision of 5, need to
validate that all MADT table changes between 1 and 5 are present
in QEMU.

Below is a table summarizing the changes to the MADT. This information
gleamed from the ACPI specs on uefi.org.

ACPIMADTWhat
Version Version
1.0 MADT not present
2.0 1   Section 5.2.10.4
3.0 2   Section 5.2.11.4
 5.2.11.13 Local SAPIC Structure added two new fields:
  ACPI Processor UID Value
  ACPI Processor UID String
 5.2.10.14 Platform Interrupt Sources Structure:
  Reserved changed to Platform Interrupt Sources Flags
3.0b2   Section 5.2.11.4
 Added a section describing guidelines for the ordering of
 processors in the MADT to support proper boot processor
 and multi-threaded logical processor operation.
4.0 3   Section 5.2.12
 Adds Processor Local x2APIC structure type 9
 Adds Local x2APIC NMI structure type 0xA
5.0 3   Section 5.2.12
6.0 3   Section 5.2.12
6.0a4   Section 5.2.12
 Adds ARM GIC structure types 0xB-0xF
6.2a45  Section 5.2.12   <--- version 45, is indeed accurate!
6.2b5   Section 5.2.12
 GIC ITS last Reserved offset changed to 16 from 20 (typo)
6.3 5   Section 5.2.12
 Adds Local APIC Flags Online Capable!
 Adds GICC SPE Overflow Interrupt field
6.4 5   Section 5.2.12
 Adds Multiprocessor Wakeup Structure type 0x10
 (change notes says structure previously misplaced?)
6.5 5   Section 5.2.12

For the MADT revision change 1 -> 2, the spec has a change to the
SAPIC structure. In general, QEMU does not generate/support SAPIC.
So the QEMU i386 MADT revision can safely be moved to 2.

For the MADT revision change 2 -> 3, the spec adds Local x2APIC
structures. QEMU has long supported x2apic ACPI structures. A simple
search of x2apic within QEMU source and hw/i386/acpi-common.c
specifically reveals this. So the QEMU i386 MADT revision can safely
be moved to 3.

For the MADT revision change 3 -> 4, the spec adds support for the ARM
GIC structures. QEMU ARM does in fact generate and report revision 4.
As these will not be used by i386 QEMU, so then the QEMU i386 MADT
revision can safely be moved to 4 as well.

Now for the MADT revision change 4 -> 5, the spec adds the Online
Capable flag to the Local APIC structure, and the ARM GICC SPE
Overflow Interrupt field.

For i386, the ARM SPE is not applicable.

For the i386 Local APIC flag Online Capable, the spec has certain rules
about this value. And in particuar setting this value now explicitly
indicates a hotpluggable CPU.

So this patch makes the needed changes to move i386 MADT to
revision 5.

Without these changes, the information below shows "how" CPU hotplug
breaks with the current upstream Linux kernel 6.3.  For example, a Linux
guest started with:

 qemu-system-x86_64 -smp 30,maxcpus=32 ...

and then attempting to hotplug a CPU:

  (QEMU) device_add id=cpu30 driver=host-x86_64-cpu socket-id=0 core-id=30 
thread-id=0

fails with the following:

  APIC: NR_CPUS/possible_cpus limit of 30 reached. Processor 30/0x.
  ACPI: Unable to map lapic to logical cpu number
  acpi LNXCPU:1e: Enumeration failure

  # dmesg | grep smpboot
  smpboot: Allowing 30 CPUs, 0 hotplug CPUs
  smpboot: CPU0: Intel(R) Xeon(R) CPU D-1533 @ 2.10GHz (family: 0x)
  smpboot: Max logical packages: 1
  smpboot: Total of 30 processors activated (125708.76 BogoMIPS)

  # iasl -d /sys/firmware/tables/acpi/APIC
  [000h    4]Signature : "APIC"[Multiple APIC 
Descript
  [004h 0004   4] Table Length : 0170
  [008h 0008   1] Revision : 01  <=
  [009h 0009   1] Checksum : 9C
  [00Ah 0010   6]   Oem ID : "BOCHS "
  [010h 0016   8] Oem Table ID : "BXPC"
  [018h 0024   4] Oe

[PATCH v3 1/3] ACPI: bios-tables-test.c step 2 (allowed-diff entries)

[Eric DeVolder]

Following the guidelines in tests/qtest/bios-tables-test.c, this
change sets-up bios-tables-test-allowed-diff.h to exclude the
imminent changes to the APIC tables, per step 2.

Signed-off-by: Eric DeVolder 
---
 tests/qtest/bios-tables-test-allowed-diff.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..1e5e354ecf 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,5 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/APIC",
+"tests/data/acpi/q35/APIC",
+"tests/data/acpi/microvm/APIC",
+"tests/data/acpi/virt/APIC",
-- 
2.31.1

[PATCH v3 2/3] ACPI: i386: bump MADT to revision 5

[Eric DeVolder]

Currently i386 QEMU generates MADT revision 3, and reports
MADT revision 1. ACPI 6.3 introduces MADT revision 5.

For MADT revision 4, that introduces ARM GIC structures, which do
not apply to i386.

For MADT revision 5, the Local APIC flags introduces the Online
Capable bitfield.

Making MADT generate and report revision 5 will solve problems with
CPU hotplug (the Online Capable flag indicates hotpluggable CPUs).

Link: https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devolder@ora
cle.com/T/#t
Signed-off-by: Eric DeVolder 
---
 hw/i386/acpi-common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 52e5c1439a..5a5e73c399 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -39,7 +39,7 @@ void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids,
 uint32_t apic_id = apic_ids->cpus[uid].arch_id;
 /* Flags – Local APIC Flags */
 uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
- 1 /* Enabled */ : 0;
+ 1 /* Enabled */ : 2 /* Online Capable */;
 
 /* ACPI spec says that LAPIC entry for non present
  * CPU may be omitted from MADT or it must be marked
@@ -102,7 +102,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
 MachineClass *mc = MACHINE_GET_CLASS(x86ms);
 const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
 AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
-AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id,
+AcpiTable table = { .sig = "APIC", .rev = 5, .oem_id = oem_id,
 .oem_table_id = oem_table_id };
 
 acpi_table_begin(&table, table_data);
-- 
2.31.1

[PATCH v3 3/3] ACPI: bios-tables-test.c step 5 (updated expected table binaries)

[Eric DeVolder]

Following the guidelines in tests/qtest/bios-tables-test.c, this
is step 6.

For the cpuhp test case, it is started with:
 -smp 2,cores=3,sockets=2,maxcpus=6

So two of six CPUs are present, leaving 4 hot-pluggable CPUs. This
is what the disassembly diff below shows (two entries with Enabled=1
and the new Online Capable bit 0, and four entries with Enabled=0 and
Online Capable bit 1).

(NOTE: I'm only showing x86_64 .cphp as i386 is the same. And for
tests not involving hotplug, the diff shows just the corresponding
change to .Revision and .Checksum.)

  /*
   * Intel ACPI Component Architecture
   * AML/ASL+ Disassembler version 20230331 (64-bit version)
   * Copyright (c) 2000 - 2023 Intel Corporation
   *
 - * Disassembly of tests/data/acpi/pc/APIC.cphp, Fri Apr 21 16:50:07 2023
 + * Disassembly of /tmp/aml-9ON131, Fri Apr 21 16:50:07 2023
   *
   * ACPI Data Table [APIC]
   *
   * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in 
hex)
   */

  [000h  004h]   Signature : "APIC"[Multiple APIC 
Description Table (MADT)]
  [004h 0004 004h]Table Length : 00A0
 -[008h 0008 001h]Revision : 01
 -[009h 0009 001h]Checksum : 18
 +[008h 0008 001h]Revision : 05
 +[009h 0009 001h]Checksum : 0C
  [00Ah 0010 006h]  Oem ID : "BOCHS "
  [010h 0016 008h]Oem Table ID : "BXPC"
  [018h 0024 004h]Oem Revision : 0001
  [01Ch 0028 004h] Asl Compiler ID : "BXPC"
  [020h 0032 004h]   Asl Compiler Revision : 0001

  [024h 0036 004h]  Local Apic Address : FEE0
  [028h 0040 004h]   Flags (decoded below) : 0001
   PC-AT Compatibility : 1

  [02Ch 0044 001h]   Subtable Type : 00 [Processor Local APIC]
  [02Dh 0045 001h]  Length : 08
  [02Eh 0046 001h]Processor ID : 00
  [02Fh 0047 001h]   Local Apic ID : 00
  [030h 0048 004h]   Flags (decoded below) : 0001
 Processor Enabled : 1
Runtime Online Capable : 0

  [034h 0052 001h]   Subtable Type : 00 [Processor Local APIC]
  [035h 0053 001h]  Length : 08
  [036h 0054 001h]Processor ID : 01
  [037h 0055 001h]   Local Apic ID : 01
  [038h 0056 004h]   Flags (decoded below) : 0001
 Processor Enabled : 1
Runtime Online Capable : 0

  [03Ch 0060 001h]   Subtable Type : 00 [Processor Local APIC]
  [03Dh 0061 001h]  Length : 08
  [03Eh 0062 001h]Processor ID : 02
  [03Fh 0063 001h]   Local Apic ID : 02
 -[040h 0064 004h]   Flags (decoded below) : 
 +[040h 0064 004h]   Flags (decoded below) : 0002
 Processor Enabled : 0
 -  Runtime Online Capable : 0
 +  Runtime Online Capable : 1

  [044h 0068 001h]   Subtable Type : 00 [Processor Local APIC]
  [045h 0069 001h]  Length : 08
  [046h 0070 001h]Processor ID : 03
  [047h 0071 001h]   Local Apic ID : 04
 -[048h 0072 004h]   Flags (decoded below) : 
 +[048h 0072 004h]   Flags (decoded below) : 0002
 Processor Enabled : 0
 -  Runtime Online Capable : 0
 +  Runtime Online Capable : 1

  [04Ch 0076 001h]   Subtable Type : 00 [Processor Local APIC]
  [04Dh 0077 001h]  Length : 08
  [04Eh 0078 001h]Processor ID : 04
  [04Fh 0079 001h]   Local Apic ID : 05
 -[050h 0080 004h]   Flags (decoded below) : 
 +[050h 0080 004h]   Flags (decoded below) : 0002
 Processor Enabled : 0
 -  Runtime Online Capable : 0
 +  Runtime Online Capable : 1

  [054h 0084 001h]   Subtable Type : 00 [Processor Local APIC]
  [055h 0085 001h]  Length : 08
  [056h 0086 001h]Processor ID : 05
  [057h 0087 001h]   Local Apic ID : 06
 -[058h 0088 004h]   Flags (decoded below) : 
 +[058h 0088 004h]   Flags (decoded below) : 0002
 Processor Enabled : 0
 -  Runtime Online Capable : 0
 +  Runtime Online Capable : 1

  [05Ch 0092 001h]   Subtable Type : 01 [I/O APIC]
  [05Dh 0093 001h]  Length : 0C
  [05Eh 0094 001h] I/O Apic ID : 00
  [05Fh 0095 001h]Reserved : 00
  [060h 0096 004h] Address : FEC0
  [064h 0100 004h]   Interrupt : 

  [068h 0104 001h]   Subtable Type : 02 [Interrupt So

Re: [PATCH 03/42] tcg: Split out tcg_out_ext8s

[Philippe Mathieu-Daudé]

On 8/4/23 04:42, Richard Henderson wrote:

We will need a backend interface for performing 8-bit sign-extend.
Use it in tcg_reg_alloc_op in the meantime.

Signed-off-by: Richard Henderson 
---
  tcg/tcg.c| 21 -
  tcg/aarch64/tcg-target.c.inc | 11 +++
  tcg/arm/tcg-target.c.inc | 10 --
  tcg/i386/tcg-target.c.inc| 10 +-
  tcg/loongarch64/tcg-target.c.inc | 11 ---
  tcg/mips/tcg-target.c.inc| 12 
  tcg/ppc/tcg-target.c.inc | 10 --
  tcg/riscv/tcg-target.c.inc   |  9 +++--
  tcg/s390x/tcg-target.c.inc   | 10 +++---
  tcg/sparc64/tcg-target.c.inc |  7 +++
  tcg/tci/tcg-target.c.inc | 21 -
  11 files changed, 81 insertions(+), 51 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] hw/riscv: virt: Enable booting M-mode or S-mode FW from pflash0

[Philippe Mathieu-Daudé]

On 21/4/23 18:48, Andrea Bolognani wrote:

On Fri, Apr 21, 2023 at 04:36:15PM +0200, Heinrich Schuchardt wrote:

On 4/21/23 06:33, Sunil V L wrote:

Currently, virt machine supports two pflash instances each with
32MB size. However, the first pflash is always assumed to
contain M-mode firmware and reset vector is set to this if
enabled. Hence, for S-mode payloads like EDK2, only one pflash
instance is available for use. This means both code and NV variables
of EDK2 will need to use the same pflash.

The OS distros keep the EDK2 FW code as readonly. When non-volatile
variables also need to share the same pflash, it is not possible
to keep it as readonly since variables need write access.

To resolve this issue, the code and NV variables need to be separated.
But in that case we need an extra flash. Hence, modify the convention
such that pflash0 will contain the M-mode FW only when "-bios none"
option is used. Otherwise, pflash0 will contain the S-mode payload FW.
This enables both pflash instances available for EDK2 use.

Example usage:
1) pflash0 containing M-mode FW
qemu-system-riscv64 -bios none -pflash  -machine virt
or
qemu-system-riscv64 -bios none \
-drive file=,if=pflash,format=raw,unit=0 -machine virt

2) pflash0 containing S-mode payload like EDK2
qemu-system-riscv64 -pflash  -pflash  -machine  virt
or
qemu-system-riscv64 -bios  \
-pflash  \
-pflash  \
-machine  virt
or
qemu-system-riscv64 -bios  \
-drive file=,if=pflash,format=raw,unit=0,readonly=on \
-drive file=,if=pflash,format=raw,unit=1 \
-machine virt

Signed-off-by: Sunil V L 
Reported-by: Heinrich Schuchardt 


QEMU 7.2 (and possibly 8.0 to be released) contains the old behavior.

Changed use of command line parameters should depend on the version of
the virt machine, i.e. virt-7.2 should use the old behavior and virt as
alias for virt-8.0 should use the new behavior. Please, have a look at
the option handling in hw/arm/virt.c and macro DEFINE_VIRT_MACHINE().


I would normally agree with you, but note that RISC-V doesn't have
versioned machine types yet, so this kind of breakage is not
necessarily unexpected.

 From libvirt's point of view, being able to detect whether the new
behavior is implemented by looking for some machine type property
would be enough to handle the transition smoothly. That would of
course not help people running QEMU directly.

For what it's worth, this change seems to go in the right direction
by making things similar to other architectures (x86, Arm) so I'd
love to see it happen.


Unfortunately another arch that followed the bad example of using
a R/W device for the CODE region and not a simple ROM.

Re: [PULL 00/20] Block patches

[Richard Henderson]

On 4/20/23 13:09, Stefan Hajnoczi wrote:

The following changes since commit c1eb2ddf0f8075faddc5f7c3d39feae3e8e9d6b4:

   Update version for v8.0.0 release (2023-04-19 17:27:13 +0100)

are available in the Git repository at:

   https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 36e5e9b22abe56aa00ca067851555ad8127a7966:

   tracing: install trace events file only if necessary (2023-04-20 07:39:43 
-0400)


Pull request

Sam Li's zoned storage work and fixes I collected during the 8.0 freeze.



Carlos Santos (1):
   tracing: install trace events file only if necessary

Philippe Mathieu-Daudé (1):
   block/dmg: Declare a type definition for DMG uncompress function

Sam Li (17):
   block/block-common: add zoned device structs
   block/file-posix: introduce helper functions for sysfs attributes
   block/block-backend: add block layer APIs resembling Linux
 ZonedBlockDevice ioctls
   block/raw-format: add zone operations to pass through requests
   block: add zoned BlockDriver check to block layer
   iotests: test new zone operations
   block: add some trace events for new block layer APIs
   docs/zoned-storage: add zoned device documentation
   file-posix: add tracking of the zone write pointers
   block: introduce zone append write for zoned devices
   qemu-iotests: test zone append operation
   block: add some trace events for zone append
   include: update virtio_blk headers to v6.3-rc1
   virtio-blk: add zoned storage emulation for zoned devices
   block: add accounting for zone append operation
   virtio-blk: add some trace events for zoned emulation
   docs/zoned-storage:add zoned emulation use case

Thomas De Schampheleire (1):
   tracetool: use relative paths for '#line' preprocessor directives


32 failed CI jobs:
https://gitlab.com/qemu-project/qemu/-/pipelines/844927626/failures


r~

Re: [PATCH v10 06/11] target/arm: move cpu_tcg to tcg/cpu32.c

[Philippe Mathieu-Daudé]

On 12/4/23 14:18, Fabiano Rosas wrote:

From: Claudio Fontana 

move the module containing cpu models definitions
for 32bit TCG-only CPUs to tcg/ and rename it for clarity.

Signed-off-by: Claudio Fontana 
Signed-off-by: Fabiano Rosas 
Reviewed-by: Richard Henderson 
Acked-by: Thomas Huth 
---
  hw/arm/virt.c |  2 +-
  target/arm/meson.build|  1 -
  target/arm/{cpu_tcg.c => tcg/cpu32.c} | 13 +++--
  target/arm/tcg/cpu64.c|  2 +-
  target/arm/tcg/meson.build|  1 +
  tests/qtest/arm-cpu-features.c| 12 +---
  6 files changed, 15 insertions(+), 16 deletions(-)
  rename target/arm/{cpu_tcg.c => tcg/cpu32.c} (99%)




diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 1cb08138ad..1555b0bab8 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -506,9 +506,15 @@ static void test_query_cpu_model_expansion_kvm(const void 
*data)
  QDict *resp;
  char *error;
  
-assert_error(qts, "cortex-a15",

-"We cannot guarantee the CPU type 'cortex-a15' works "
-"with KVM on this host", NULL);
+if (qtest_has_accel("tcg")) {


Can we add a comment to explain this non trivial case?

I suppose the reason is "KVM is builtin but not available, so we don't
want to test the TCG fallback", is that correct?


+assert_error(qts, "cortex-a15",
+ "We cannot guarantee the CPU type 'cortex-a15' works "
+ "with KVM on this host", NULL);
+} else {
+assert_error(qts, "cortex-a15",
+ "The CPU type 'cortex-a15' is not a "
+ "recognized ARM CPU type", NULL);
+}
  
  assert_has_feature_enabled(qts, "host", "aarch64");
  

Re: [PATCH] target/riscv: Fix PMU node property for virt machine

[Conor Dooley]

On Fri, Apr 21, 2023 at 09:14:37PM +0800, Yu Chien Peter Lin wrote:
> The length of fdt_event_ctr_map[20] will add 5 dummy cells in
> "riscv,event-to-mhpmcounters" property, so directly initialize
> the array without an explicit size.
> 
> This patch also fixes the typo of PMU cache operation result ID
> of MISS (0x1) in the comments, and renames event idx 0x10021 to
> RISCV_PMU_EVENT_CACHE_ITLB_READ_MISS.
> 
> Signed-off-by: Yu Chien Peter Lin 
> ---
> 
>   $ ./build/qemu-system-riscv64 -M virt,dumpdtb=/tmp/virt.dtb -cpu 
> rv64,sscofpmf=on && dtc /tmp/virt.dtb | grep mhpmcounters
>   [...]
> riscv,event-to-mhpmcounters = <0x01 0x01 0x7fff9 
>0x02 0x02 0x7fffc
>0x10019 0x10019 0x7fff8
>0x1001b 0x1001b 0x7fff8
>0x10021 0x10021 0x7fff8
>dummy cells --->0x00 0x00 0x00 0x00 0x00>;
> 
> This won't break the OpenSBI, but will cause it to incorrectly increment
> num_hw_events [1] to 6, and DT validation failure in kernel [2].
> 
>   $ dt-validate -p Documentation/devicetree/bindings/processed-schema.json 
> virt.dtb
>   [...]
>   virt.dtb: soc: pmu: {'riscv,event-to-mhpmcounters': [[1, 1, 524281], [2, 2, 
> 524284], [65561, 65561, 524280], [65563, 65563, 524280], [65569, 65569, 
> 524280], [0, 0, 0], [0, 0]], 'compatible': ['riscv,pmu']} should not be valid 
> under {'type': 'object'}

I would note that this warning here does not go away with this patch ^^
It's still on my todo list, unless you want to fix it!

>   From schema: 
> /home/peterlin/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>   virt.dtb: pmu: riscv,event-to-mhpmcounters:6: [0, 0] is too short
>   [...]

And with the binding below there is a 3rd warning, but I think it is
incorrect and I submitted a patch for the binding to fix it.

That said, this doesn't seem to have been submitted on top of my naive
s/20/15/ patch that Alistair picked up. Is this intended to replace, or
for another branch? Replace works fine for me, don't have sentimental
feelings for that patch!

I think your approach here was better than my s/20/15/, but seems like a
bit of a fix and a bit of cleanup all-in-one.

Either way, warnings gone so WFM :) :)

Thanks,
Conor.

> [1] 
> https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_pmu.c#L255
> [2] 
> https://github.com/torvalds/linux/blob/v6.3-rc7/Documentation/devicetree/bindings/perf/riscv%2Cpmu.yaml#L54-L66
> ---
>  target/riscv/cpu.h|  2 +-
>  target/riscv/cpu_helper.c |  2 +-
>  target/riscv/pmu.c| 88 +++
>  3 files changed, 45 insertions(+), 47 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 638e47c75a..eab518542c 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -812,7 +812,7 @@ enum riscv_pmu_event_idx {
>  RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
>  RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS = 0x10019,
>  RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS = 0x1001B,
> -RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
> +RISCV_PMU_EVENT_CACHE_ITLB_READ_MISS = 0x10021,
>  };
>  
>  /* CSR function table */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..5d3e032ec9 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1210,7 +1210,7 @@ static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu, 
> MMUAccessType access_type)
>  
>  switch (access_type) {
>  case MMU_INST_FETCH:
> -pmu_event_type = RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS;
> +pmu_event_type = RISCV_PMU_EVENT_CACHE_ITLB_READ_MISS;
>  break;
>  case MMU_DATA_LOAD:
>  pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS;
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index b8e56d2b7b..0b21c3fa38 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -35,51 +35,49 @@
>   */
>  void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
>  {
> -uint32_t fdt_event_ctr_map[20] = {};
> -uint32_t cmask;
> -
>  /* All the programmable counters can map to any event */
> -cmask = MAKE_32BIT_MASK(3, num_ctrs);
> -
> -   /*
> -* The event encoding is specified in the SBI specification
> -* Event idx is a 20bits wide number encoded as follows:
> -* event_idx[19:16] = type
> -* event_idx[15:0] = code
> -* The code field in cache events are encoded as follows:
> -* event_idx.code[15:3] = cache_id
> -* event_idx.code[2:1] = op_id
> -* event_idx.code[0:0] = result_id
> -*/
> -
> -   /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
> -   fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
> -   fdt_event_ctr_map[1] = cpu_to_be32(0x0001);
> -   fdt_event_ctr_map[2] = cpu_to_be32(cmask | 1 << 0);
> -
> -   /* SBI_PMU_HW_INSTRUCTIONS: 0x02 : type(0x00) */
> -   fdt_event_ctr_map[3] =

Re: [PATCH v10 04/11] target/arm: Do not expose all -cpu max features to qtests

[Philippe Mathieu-Daudé]

On 12/4/23 14:18, Fabiano Rosas wrote:

We're about to move the TCG-only -cpu max configuration code under
CONFIG_TCG. To be able to do that we need to make sure the qtests
still have some cpu configured even when no other accelerator is
available.

Delineate now what is used with TCG-only and what is also used with
qtests to make the subsequent patches cleaner.

Signed-off-by: Fabiano Rosas 
---
  target/arm/cpu64.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)


Nice.

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v10 03/11] target/arm: Extract TCG -cpu max code into a function

[Philippe Mathieu-Daudé]

On 12/4/23 14:18, Fabiano Rosas wrote:

Introduce aarch64_max_tcg_initfn that contains the TCG-only part of
-cpu max configuration. We'll need that to be able to restrict this
code to a TCG-only config in the next patches.

Signed-off-by: Fabiano Rosas 
---
  target/arm/cpu64.c | 32 ++--
  1 file changed, 18 insertions(+), 14 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v10 02/11] target/arm: Remove dead code from cpu_max_set_sve_max_vq

[Philippe Mathieu-Daudé]

On 12/4/23 14:18, Fabiano Rosas wrote:

The sve-max-vq property has been removed from the -cpu max used with
KVM, so code under kvm_enabled in cpu_max_set_sve_max_vq is not
reachable.

Fixes: 0baa21be49 ("target/arm: Make KVM -cpu max exactly like -cpu host")
Signed-off-by: Fabiano Rosas 
---
  target/arm/cpu64.c | 6 --
  1 file changed, 6 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v4 5/6] hw/cxl: Add poison injection via the mailbox.

[Jonathan Cameron via]

On Tue, 14 Mar 2023 07:27:52 +0100
Philippe Mathieu-Daudé  wrote:

> On 3/3/23 16:09, Jonathan Cameron wrote:
> > Very simple implementation to allow testing of corresponding
> > kernel code. Note that for now we track each 64 byte section
> > independently.  Whilst a valid implementation choice, it may
> > make sense to fuse entries so as to prove out more complex
> > corners of the kernel code.
> > 
> > Reviewed-by: Ira Weiny 
> > Signed-off-by: Jonathan Cameron 
> > ---
> > v4: No change
> > ---
> >   hw/cxl/cxl-mailbox-utils.c | 41 ++
> >   1 file changed, 41 insertions(+)  
> 
> 
> > +static CXLRetCode cmd_media_inject_poison(struct cxl_cmd *cmd,
> > +  CXLDeviceState *cxl_dstate,
> > +  uint16_t *len)
> > +{
> > +CXLType3Dev *ct3d = container_of(cxl_dstate, CXLType3Dev, cxl_dstate); 
> >  
> 
> This makes me wonder why CXLDeviceState isn't QDev based.

Interesting question that I'll look into, but I hope you don't mind if
I separate that question from this series.

Logically it's a one of a couple of different subsets of functionality and
different CXL components have a different mix of those. I'm not sure
that will map to a QDev based approach. I'll need to take more time to look into
this.

> 
> (Also, why include/hw/cxl/cxl_device.h is under GPL-2.0-only license?)

Not a clue.   Ben, any comment?

Re: [PULL 00/23] First batch of testing and misc patches for 8.1

[Richard Henderson]

On 4/20/23 11:11, Thomas Huth wrote:

  Hi!

The following changes since commit c1eb2ddf0f8075faddc5f7c3d39feae3e8e9d6b4:

   Update version for v8.0.0 release (2023-04-19 17:27:13 +0100)

are available in the Git repository at:

   https://gitlab.com/thuth/qemu.git tags/pull-request-2023-04-20

for you to fetch changes up to ec6fb1c8cd8d55c3d2a34cacfea6df0fe6c76057:

   tests/vm/freebsd: Update to FreeBSD 13.2 (2023-04-20 11:28:16 +0200)


* Compat machines for version 8.1
* Allow setting a chardev input file on the command line
* Fix .travis.yml to work with non-public Travis instances, too
* Move a lot of code from specifc_ss into softmmu_ss
* Add a test case for TPM TIS I2C connected to Aspeed I2C controller
* Update tests/vm/freebsd to version 13
* Some more misc minor fixes here and there


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as 
appropriate.


r~

Re: [PATCH 1/2] tests/migration: Make precopy fast

[Daniel P . Berrangé]

On Tue, Apr 18, 2023 at 02:20:27PM +0200, Juan Quintela wrote:
> Daniel P. Berrangé  wrote:
> > On Wed, Apr 12, 2023 at 04:20:00PM +0200, Juan Quintela wrote:
> >> Otherwise we do the 1st migration iteration at a too slow speed.
> >> 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  tests/qtest/migration-test.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >> 
> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> index 3b615b0da9..7b05b0b7dd 100644
> >> --- a/tests/qtest/migration-test.c
> >> +++ b/tests/qtest/migration-test.c
> >> @@ -1348,6 +1348,7 @@ static void test_precopy_common(MigrateCommon *args)
> >>  migrate_qmp(from, args->connect_uri, "{}");
> >>  }
> >>  
> >> +migrate_ensure_converge(from);
> >
> > This isn't right - it defeats the point of having the call to
> > migrate_ensure_non_converge() a few lines earlier.
> 
> Depends on what is the definiton or "right" O:-)
> 
> >>  if (args->result != MIG_TEST_SUCCEED) {
> >>  bool allow_active = args->result == MIG_TEST_FAIL;
> >> @@ -1365,8 +1366,6 @@ static void test_precopy_common(MigrateCommon *args)
> >>  wait_for_migration_pass(from);
> >>  }
> >>  
> >> -migrate_ensure_converge(from);
> >> -
> >
> > The reason why we had it here was to ensure that we test more than
> > 1 iteration of migration. With this change, migrate will succeed
> > on the first pass IIUC, and so we won't be exercising the more
> > complex code path of repeated iterations.
> 
> Aha.
> 
> If that is the definition of "right", then I agree that my changes are
> wrong.
> 
> But then I think we should change how we do the test.  We should split
> this function (then same for postcopy, multifd, etc) to have to
> versions, one that want to have multiple rounds, and another that can
> finish as fast as possible.
> 
> This way we need to setup the 3MB/s only for the tests that we want to
> loop, and for the others put something faster.
> 
> 
> >
> > I do agree with the overall idea though. We have many many migration
> > test scenarios and we don't need all of them to be testing multiple
> > iterations - a couple would be sufficient.
> >
> > In fact we don't even need to be testing live migration for most
> > of the cases. All the TLS test cases could be run with guest CPUs
> > paused entirely removing any dirtying, since they're only interested
> > in the initial network handshake/setup process testnig.
> >
> > I had some patches I was finishing off just before I went on vacation
> > a few weeks ago which do this kind of optimization, which I can send
> > out shortly.
> 
> I will wait for your patches before I sent anything different.
> 
> I have local patches for doing something different, changing
> 
>   "-serial file:%s/src_serial "
> 
> and other friends to:
> 
>   "-serial file:%s/src_serial%pid "
> 
> So we are sure that two tests never "reuse" the socket, as it can create
> problems for example when doing the cancel and relaunching the
> destination.
> 
> But as said, will wait until you send your series to send anything.

I've just sent a new series which has some more differences and
improvements

https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg03688.html


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH 01/11] Signed-off-by: Karim Taha

[Alex Bennée]

Karim Taha  writes:

> On Fri, Apr 21, 2023 at 9:17 AM Daniel P. Berrangé  
> wrote:
>
>  On Fri, Apr 21, 2023 at 07:22:45AM +0200, Karim Taha wrote:
>  > From: Warner Losh 
>  > 
>  > Allow guest_base to be initialized on 64-bit hosts, the initial value is 
> used by g2h_untagged function
>  defined in include/exec/cpu_ldst.h
>
>  This commit message is all incorrectly structured I'm afraid.
>
>  There needs to a short 1 line summary, then a blank line,
>  then the full commit description text, then a blank line,
>  then the Signed-off-by tag(s).
>
>  Also if you're sending work done by Warner (as the From
>  tag suggests), then we would expect to see Warner's own
>  Signed-off-by tag, in addition to your own Signed-off-by.

>
> Alright, thanks for the commit formatting tips, I resent the patch series, 
> with my signed off by tag and the
> author signed off by tags as well.

Hmm something has gone wrong. Was this sent with a plain git-send-email
or using a tool like git-publish?

Can you point to a branch?

>
> Best regards,
> Karim


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/2] tests/qtest: make more migration pre-copy scenarios run non-live

[Daniel P . Berrangé]

On Tue, Apr 18, 2023 at 04:52:32PM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé  writes:
> 
> > There are 27 pre-copy live migration scenarios being tested. In all of
> > these we force non-convergance and run for one iteration, then let it
> > converge and wait for completion during the second (or following)
> > iterations. At 3 mbps bandwidth limit the first iteration takes a very
> > long time (~30 seconds).
> >
> > While it is important to test the migration passes and convergance
> > logic, it is overkill to do this for all 27 pre-copy scenarios. The
> > TLS migration scenarios in particular are merely exercising different
> > code paths during connection establishment.
> >
> > To optimize time taken, switch most of the test scenarios to run
> > non-live (ie guest CPUs paused) with no bandwidth limits. This gives
> > a massive speed up for most of the test scenarios.
> >
> > For test coverage the following scenarios are unchanged
> >
> >  * Precopy with UNIX sockets
> >  * Precopy with UNIX sockets and dirty ring tracking
> >  * Precopy with XBZRLE
> >  * Precopy with multifd
> >
> > Signed-off-by: Daniel P. Berrangé 
> 
> ...
> 
> > -qtest_qmp_eventwait(to, "RESUME");
> > +if (!args->live) {
> > +qtest_qmp_discard_response(to, "{ 'execute' : 'cont'}");
> > +}
> > +if (!got_resume) {
> > +qtest_qmp_eventwait(to, "RESUME");
> > +}
> 
> Hi Daniel,
> 
> On an aarch64 host I'm sometimes (~30%) seeing a hang here on a TLS test:
> 
> ../configure --target-list=aarch64-softmmu --enable-gnutls
> 
> ... ./tests/qtest/migration-test --tap -k -p 
> /aarch64/migration/precopy/tcp/tls/psk/match

I never came to a satisfactory understanding of why this problem hits
you. I've just sent out a new version of this series, which has quite
a few differences, so possibly I've fixed it by luck.

So if you have time, I'd appreciate any testing you can try on

  https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg03688.html


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH v2 4/6] tests/qtest: make more migration pre-copy scenarios run non-live

[Daniel P . Berrangé]

There are 27 pre-copy live migration scenarios being tested. In all of
these we force non-convergance and run for one iteration, then let it
converge and wait for completion during the second (or following)
iterations. At 3 mbps bandwidth limit the first iteration takes a very
long time (~30 seconds).

While it is important to test the migration passes and convergance
logic, it is overkill to do this for all 27 pre-copy scenarios. The
TLS migration scenarios in particular are merely exercising different
code paths during connection establishment.

To optimize time taken, switch most of the test scenarios to run
non-live (ie guest CPUs paused) with no bandwidth limits. This gives
a massive speed up for most of the test scenarios.

For test coverage the following scenarios are unchanged

 * Precopy with UNIX sockets
 * Precopy with UNIX sockets and dirty ring tracking
 * Precopy with XBZRLE
 * Precopy with multifd

Signed-off-by: Daniel P. Berrangé 
---
 tests/qtest/migration-test.c | 60 ++--
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6492ffa7fe..40d0f75480 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -568,6 +568,9 @@ typedef struct {
 MIG_TEST_FAIL_DEST_QUIT_ERR,
 } result;
 
+/* Whether the guest CPUs should be running during migration */
+bool live;
+
 /* Postcopy specific fields */
 void *postcopy_data;
 bool postcopy_preempt;
@@ -1324,8 +1327,6 @@ static void test_precopy_common(MigrateCommon *args)
 return;
 }
 
-migrate_ensure_non_converge(from);
-
 if (args->start_hook) {
 data_hook = args->start_hook(from, to);
 }
@@ -1335,6 +1336,31 @@ static void test_precopy_common(MigrateCommon *args)
 wait_for_serial("src_serial");
 }
 
+if (args->live) {
+/*
+ * Testing live migration, we want to ensure that some
+ * memory is re-dirtied after being transferred, so that
+ * we exercise logic for dirty page handling. We achieve
+ * this with a ridiculosly low bandwidth that guarantees
+ * non-convergance.
+ */
+migrate_ensure_non_converge(from);
+} else {
+/*
+ * Testing non-live migration, we allow it to run at
+ * full speed to ensure short test case duration.
+ * For tests expected to fail, we don't need to
+ * change anything.
+ */
+if (args->result == MIG_TEST_SUCCEED) {
+qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+migrate_ensure_converge(from);
+}
+}
+
 if (!args->connect_uri) {
 g_autofree char *local_connect_uri =
 migrate_get_socket_address(to, "socket-address");
@@ -1352,19 +1378,29 @@ static void test_precopy_common(MigrateCommon *args)
 qtest_set_expected_status(to, EXIT_FAILURE);
 }
 } else {
-wait_for_migration_pass(from);
+if (args->live) {
+wait_for_migration_pass(from);
 
-migrate_ensure_converge(from);
+migrate_ensure_converge(from);
 
-/* We do this first, as it has a timeout to stop us
- * hanging forever if migration didn't converge */
-wait_for_migration_complete(from);
+/*
+ * We do this first, as it has a timeout to stop us
+ * hanging forever if migration didn't converge
+ */
+wait_for_migration_complete(from);
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+} else {
+wait_for_migration_complete(from);
 
-if (!got_stop) {
-qtest_qmp_eventwait(from, "STOP");
+qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
 }
 
-qtest_qmp_eventwait(to, "RESUME");
+if (!got_resume) {
+qtest_qmp_eventwait(to, "RESUME");
+}
 
 wait_for_serial("dest_serial");
 }
@@ -1382,6 +1418,7 @@ static void test_precopy_unix_plain(void)
 MigrateCommon args = {
 .listen_uri = uri,
 .connect_uri = uri,
+.live = true,
 };
 
 test_precopy_common(&args);
@@ -1397,6 +1434,7 @@ static void test_precopy_unix_dirty_ring(void)
 },
 .listen_uri = uri,
 .connect_uri = uri,
+.live = true,
 };
 
 test_precopy_common(&args);
@@ -1506,6 +1544,7 @@ static void test_precopy_unix_xbzrle(void)
 .listen_uri = uri,
 
 .start_hook = test_migrate_xbzrle_start,
+.live = true,
 };
 
 test_precopy_common(&args);
@@ -1906,6 +1945,7 @@ static void test_multifd_tcp_none(void)
 MigrateCommon args = {
 .listen_uri = "defer",
 .start_hook = test_migrate_precopy_tcp_multifd_start,
+.

[PATCH v2 3/6] tests/qtest: capture RESUME events during migration

[Daniel P . Berrangé]

When running migration tests we monitor for a STOP event so we can skip
redundant waits. This will be needed for the RESUME event too shortly.

Signed-off-by: Daniel P. Berrangé 
---
 tests/qtest/migration-helpers.c | 12 +---
 tests/qtest/migration-helpers.h |  1 +
 tests/qtest/migration-test.c|  1 +
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index f6f3c6680f..61396335cc 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -24,14 +24,20 @@
 #define MIGRATION_STATUS_WAIT_TIMEOUT 120
 
 bool got_stop;
+bool got_resume;
 
-static void check_stop_event(QTestState *who)
+static void check_events(QTestState *who)
 {
 QDict *event = qtest_qmp_event_ref(who, "STOP");
 if (event) {
 got_stop = true;
 qobject_unref(event);
 }
+event = qtest_qmp_event_ref(who, "RESUME");
+if (event) {
+got_resume = true;
+qobject_unref(event);
+}
 }
 
 #ifndef _WIN32
@@ -48,7 +54,7 @@ QDict *wait_command_fd(QTestState *who, int fd, const char 
*command, ...)
 va_end(ap);
 
 resp = qtest_qmp_receive(who);
-check_stop_event(who);
+check_events(who);
 
 g_assert(!qdict_haskey(resp, "error"));
 g_assert(qdict_haskey(resp, "return"));
@@ -73,7 +79,7 @@ QDict *wait_command(QTestState *who, const char *command, ...)
 resp = qtest_vqmp(who, command, ap);
 va_end(ap);
 
-check_stop_event(who);
+check_events(who);
 
 g_assert(!qdict_haskey(resp, "error"));
 g_assert(qdict_haskey(resp, "return"));
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index a188b62787..726a66cfc1 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -16,6 +16,7 @@
 #include "libqtest.h"
 
 extern bool got_stop;
+extern bool got_resume;
 
 #ifndef _WIN32
 G_GNUC_PRINTF(3, 4)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index e16120ff30..6492ffa7fe 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -596,6 +596,7 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
 }
 
 got_stop = false;
+got_resume = false;
 bootpath = g_strdup_printf("%s/bootsect", tmpfs);
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
 /* the assembled x86 boot sector should be exactly one sector large */
-- 
2.40.0

[PATCH v2 1/6] tests/qtest: replace qmp_discard_response with qtest_qmp_assert_success

[Daniel P . Berrangé]

The qmp_discard_response method simply ignores the result of the QMP
command, merely unref'ing the object. This is a bad idea for tests
as it leaves no trace if the QMP command unexpectedly failed. The
qtest_qmp_assert_success method will validate that the QMP command
returned without error, and if errors occur, it will print a message
on the console aiding debugging.

Signed-off-by: Daniel P. Berrangé 
---
 tests/qtest/ahci-test.c  | 31 ++--
 tests/qtest/boot-order-test.c|  5 +
 tests/qtest/fdc-test.c   | 15 +++---
 tests/qtest/ide-test.c   |  5 +
 tests/qtest/migration-test.c |  5 +
 tests/qtest/test-filter-mirror.c |  5 +
 tests/qtest/test-filter-redirector.c |  7 ++-
 tests/qtest/virtio-blk-test.c| 24 ++---
 8 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 1967cd5898..abab761c26 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -36,9 +36,6 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
-/* TODO actually test the results and get rid of this */
-#define qmp_discard_response(s, ...) qobject_unref(qtest_qmp(s, __VA_ARGS__))
-
 /* Test images sizes in MB */
 #define TEST_IMAGE_SIZE_MB_LARGE (200 * 1024)
 #define TEST_IMAGE_SIZE_MB_SMALL 64
@@ -1595,9 +1592,9 @@ static void test_atapi_tray(void)
 rsp = qtest_qmp_receive(ahci->parent->qts);
 qobject_unref(rsp);
 
-qmp_discard_response(ahci->parent->qts,
- "{'execute': 'blockdev-remove-medium', "
- "'arguments': {'id': 'cd0'}}");
+qtest_qmp_assert_success(ahci->parent->qts,
+ "{'execute': 'blockdev-remove-medium', "
+ "'arguments': {'id': 'cd0'}}");
 
 /* Test the tray without a medium */
 ahci_atapi_load(ahci, port);
@@ -1607,16 +1604,18 @@ static void test_atapi_tray(void)
 atapi_wait_tray(ahci, true);
 
 /* Re-insert media */
-qmp_discard_response(ahci->parent->qts,
- "{'execute': 'blockdev-add', "
- "'arguments': {'node-name': 'node0', "
-"'driver': 'raw', "
-"'file': { 'driver': 'file', "
-  "'filename': %s }}}", iso);
-qmp_discard_response(ahci->parent->qts,
- "{'execute': 'blockdev-insert-medium',"
- "'arguments': { 'id': 'cd0', "
- "'node-name': 'node0' }}");
+qtest_qmp_assert_success(
+ahci->parent->qts,
+"{'execute': 'blockdev-add', "
+"'arguments': {'node-name': 'node0', "
+  "'driver': 'raw', "
+  "'file': { 'driver': 'file', "
+"'filename': %s }}}", iso);
+qtest_qmp_assert_success(
+ahci->parent->qts,
+"{'execute': 'blockdev-insert-medium',"
+"'arguments': { 'id': 'cd0', "
+   "'node-name': 'node0' }}");
 
 /* Again, the event shows up first */
 qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-close-tray', "
diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
index 0680d79d6d..8f2b6ef05a 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -16,9 +16,6 @@
 #include "qapi/qmp/qdict.h"
 #include "standard-headers/linux/qemu_fw_cfg.h"
 
-/* TODO actually test the results and get rid of this */
-#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__))
-
 typedef struct {
 const char *args;
 uint64_t expected_boot;
@@ -43,7 +40,7 @@ static void test_a_boot_order(const char *machine,
   machine ?: "", test_args);
 actual = read_boot_order(qts);
 g_assert_cmphex(actual, ==, expected_boot);
-qmp_discard_response(qts, "{ 'execute': 'system_reset' }");
+qtest_qmp_assert_success(qts, "{ 'execute': 'system_reset' }");
 /*
  * system_reset only requests reset.  We get a RESET event after
  * the actual reset completes.  Need to wait for that.
diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 1f9b99ad6d..5e8fbda9df 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -28,9 +28,6 @@
 #include "libqtest-single.h"
 #include "qapi/qmp/qdict.h"
 
-/* TODO actually test the results and get rid of this */
-#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
-
 #define DRIVE_FLOPPY_BLANK \
 "-drive 
if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k"
 
@@ -304,9 +301,10 @@ static void test_media_insert(void)
 
 /* Insert media in drive. DSKCHK should not be reset until a step pulse
  * is sent. */
-qmp_discard_response("{'execute':'blockdev-change-medium', 'arguments

[PATCH v2 5/6] tests/qtest: massively speed up migration-tet

[Daniel P . Berrangé]

The migration test cases that actually exercise live migration want to
ensure there is a minimum of two iterations of pre-copy, in order to
exercise the dirty tracking code.

Historically we've queried the migration status, looking for the
'dirty-sync-count' value to increment to track iterations. This was
not entirely reliable because often all the data would get transferred
quickly enough that the migration would finish before we wanted it
to. So we massively dropped the bandwidth and max downtime to
guarantee non-convergance. This had the unfortunate side effect
that every migration took at least 30 seconds to run (100 MB of
dirty pages / 3 MB/sec).

This optimization takes a different approach to ensuring that a
mimimum of two iterations. Rather than waiting for dirty-sync-count
to increment, directly look for an indication that the source VM
has dirtied RAM that has already been transferred.

On the source VM a magic marker is written just after the 3 MB
offset. The destination VM is now montiored to detect when the
magic marker is transferred. This gives a guarantee that the
first 3 MB of memory have been transferred. Now the source VM
memory is monitored at exactly the 3MB offset until we observe
a flip in its value. This gives us a guaranteed that the guest
workload has dirtied a byte that has already been transferred.

Since we're looking at a place that is only 3 MB from the start
of memory, with the 3 MB/sec bandwidth, this test should complete
in 1 second, instead of 30 seconds.

Once we've proved there is some dirty memory, migration can be
set back to full speed for the remainder of the 1st iteration,
and the entire of the second iteration at which point migration
should be complete.

Signed-off-by: Daniel P. Berrangé 
---
 tests/qtest/migration-test.c | 170 +++
 1 file changed, 114 insertions(+), 56 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 40d0f75480..63bd8a1893 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -44,6 +44,20 @@ unsigned start_address;
 unsigned end_address;
 static bool uffd_feature_thread_id;
 
+/*
+ * An initial 3 MB offset is used as that corresponds
+ * to ~1 sec of data transfer with our bandwidth setting.
+ */
+#define MAGIC_OFFSET_BASE (3 * 1024 * 1024)
+/*
+ * A further 1k is added to ensure we're not a multiple
+ * of TEST_MEM_PAGE_SIZE, thus avoid clash with writes
+ * from the migration guest workload.
+ */
+#define MAGIC_OFFSET_SHUFFLE 1024
+#define MAGIC_OFFSET (MAGIC_OFFSET_BASE + MAGIC_OFFSET_SHUFFLE)
+#define MAGIC_MARKER 0xFEED12345678CAFEULL
+
 /*
  * Dirtylimit stop working if dirty page rate error
  * value less than DIRTYLIMIT_TOLERANCE_RANGE
@@ -172,28 +186,6 @@ static void wait_for_serial(const char *side)
 } while (true);
 }
 
-/*
- * It's tricky to use qemu's migration event capability with qtest,
- * events suddenly appearing confuse the qmp()/hmp() responses.
- */
-
-static int64_t read_ram_property_int(QTestState *who, const char *property)
-{
-QDict *rsp_return, *rsp_ram;
-int64_t result;
-
-rsp_return = migrate_query_not_failed(who);
-if (!qdict_haskey(rsp_return, "ram")) {
-/* Still in setup */
-result = 0;
-} else {
-rsp_ram = qdict_get_qdict(rsp_return, "ram");
-result = qdict_get_try_int(rsp_ram, property, 0);
-}
-qobject_unref(rsp_return);
-return result;
-}
-
 static int64_t read_migrate_property_int(QTestState *who, const char *property)
 {
 QDict *rsp_return;
@@ -205,10 +197,6 @@ static int64_t read_migrate_property_int(QTestState *who, 
const char *property)
 return result;
 }
 
-static uint64_t get_migration_pass(QTestState *who)
-{
-return read_ram_property_int(who, "dirty-sync-count");
-}
 
 static void read_blocktime(QTestState *who)
 {
@@ -219,23 +207,6 @@ static void read_blocktime(QTestState *who)
 qobject_unref(rsp_return);
 }
 
-static void wait_for_migration_pass(QTestState *who)
-{
-uint64_t initial_pass = get_migration_pass(who);
-uint64_t pass;
-
-/* Wait for the 1st sync */
-while (!got_stop && !initial_pass) {
-usleep(1000);
-initial_pass = get_migration_pass(who);
-}
-
-do {
-usleep(1000);
-pass = get_migration_pass(who);
-} while (pass == initial_pass && !got_stop);
-}
-
 static void check_guests_ram(QTestState *who)
 {
 /* Our ASM test will have been incrementing one byte from each page from
@@ -417,6 +388,91 @@ static void migrate_ensure_converge(QTestState *who)
 migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
 }
 
+/*
+ * Our goal is to ensure that we run a single full migration
+ * iteration, and also dirty memory, ensuring that at least
+ * one further iteration is required.
+ *
+ * We can't directly synchronize with the start of a migration
+ * so we have to apply some tricks monitoring memory that is
+ * transferred.
+ *
+ * Initially we s

[PATCH v2 6/6] tests/migration: Only run auto_converge in slow mode

[Daniel P . Berrangé]

From: Juan Quintela 

Signed-off-by: Juan Quintela 
---
 tests/qtest/migration-test.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 63bd8a1893..9ed178aa03 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1877,6 +1877,21 @@ static void test_validate_uuid_dst_not_set(void)
 do_test_validate_uuid(&args, false);
 }
 
+/*
+ * The way auto_converge works, we need to do too many passes to
+ * run this test.  Auto_converge logic is only run once every
+ * three iterations, so:
+ *
+ * - 3 iterations without auto_converge enabled
+ * - 3 iterations with pct = 5
+ * - 3 iterations with pct = 30
+ * - 3 iterations with pct = 55
+ * - 3 iterations with pct = 80
+ * - 3 iterations with pct = 95 (max(95, 80 + 25))
+ *
+ * To make things even worse, we need to run the initial stage at
+ * 3MB/s so we enter autoconverge even when host is (over)loaded.
+ */
 static void test_migrate_auto_converge(void)
 {
 g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -2660,8 +2675,12 @@ int main(int argc, char **argv)
test_validate_uuid_src_not_set);
 qtest_add_func("/migration/validate_uuid_dst_not_set",
test_validate_uuid_dst_not_set);
-
-qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
+/*
+ * See explanation why this test is slow on function definition
+ */
+if (g_test_slow()) {
+qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
+}
 qtest_add_func("/migration/multifd/tcp/plain/none",
test_multifd_tcp_none);
 /*
-- 
2.40.0

[PATCH v2 2/6] tests/qtests: remove migration test iterations config

[Daniel P . Berrangé]

The 'unsigned int interations' config for migration is somewhat
overkill. Most tests don't set it, and a value of '0' is treated
as equivalent to '1'. The only test that does set it, xbzrle,
used a value of '2'.

This setting, however, only relates to the migration iterations
that take place prior to allowing convergence. IOW, on top of
this iteration count, there is always at least 1 further migration
iteration done to deal with pages that are dirtied during the
previous iteration(s).

IOW, even with iterations==1, the xbzrle test will be running for
a minimum of 2 iterations. With this in mind we can simplify the
code and just get rid of the special case.

Signed-off-by: Daniel P. Berrangé 
---
 tests/qtest/migration-test.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index ac2e8ecac6..e16120ff30 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -568,9 +568,6 @@ typedef struct {
 MIG_TEST_FAIL_DEST_QUIT_ERR,
 } result;
 
-/* Optional: set number of migration passes to wait for */
-unsigned int iterations;
-
 /* Postcopy specific fields */
 void *postcopy_data;
 bool postcopy_preempt;
@@ -1354,13 +1351,7 @@ static void test_precopy_common(MigrateCommon *args)
 qtest_set_expected_status(to, EXIT_FAILURE);
 }
 } else {
-if (args->iterations) {
-while (args->iterations--) {
-wait_for_migration_pass(from);
-}
-} else {
-wait_for_migration_pass(from);
-}
+wait_for_migration_pass(from);
 
 migrate_ensure_converge(from);
 
@@ -1514,8 +1505,6 @@ static void test_precopy_unix_xbzrle(void)
 .listen_uri = uri,
 
 .start_hook = test_migrate_xbzrle_start,
-
-.iterations = 2,
 };
 
 test_precopy_common(&args);
-- 
2.40.0

[PATCH v2 0/6] tests/qtest: make migration-test massively faster

[Daniel P . Berrangé]

This makes migration-test faster by observing that most of the pre-copy
tests don't need to be doing a live migration. They get sufficient code
coverage with the guest CPUs paused.

On my machine this cuts the overall execution time of migration-test
by 50% from 15 minutes, down to 8 minutes, without sacrificing any
noticeable code coverage.

This is still quite slow though.

The tests which do still run in live mode all want to guarantee there
are at least 2 iterations of migration, to exercise the dirty page
handling code. This is achieved by running the 1 iteration with an
incredibly small bandwidth and max downtime to prevent convergance,
and watching query-migrate for the reported iteration to increment.
This guarantees that all the tests take at least 30 seconds to run.

Watching for the iteration counter to flip is inefficient and not
actually needed. Instead we merely need to prove that some amount
of already transferred data has been made dirty again. This in turn
will guarantee that a further iteration is required beyond the current
one. This proof is easy to achieve by monitoring the values at two
distinct addresses in guest RAM, and can cut the 30 second duration
down to 1 second.

After this optimization, and Juan's patch to disable autoconverge
testnig, the migration test runs in merely 1 minute which I think
it pretty impressive given the number of scenarios we're testing.

Daniel P. Berrangé (5):
  tests/qtest: replace qmp_discard_response with
qtest_qmp_assert_success
  tests/qtests: remove migration test iterations config
  tests/qtest: capture RESUME events during migration
  tests/qtest: make more migration pre-copy scenarios run non-live
  tests/qtest: massively speed up migration-tet

Juan Quintela (1):
  tests/migration: Only run auto_converge in slow mode

 tests/qtest/ahci-test.c  |  31 ++--
 tests/qtest/boot-order-test.c|   5 +-
 tests/qtest/fdc-test.c   |  15 +-
 tests/qtest/ide-test.c   |   5 +-
 tests/qtest/migration-helpers.c  |  12 +-
 tests/qtest/migration-helpers.h  |   1 +
 tests/qtest/migration-test.c | 252 +++
 tests/qtest/test-filter-mirror.c |   5 +-
 tests/qtest/test-filter-redirector.c |   7 +-
 tests/qtest/virtio-blk-test.c|  24 +--
 10 files changed, 227 insertions(+), 130 deletions(-)

-- 
2.40.0

Re: [PATCH v11 00/14] TCG code quality tracking

[Alex Bennée]

Fei Wu  writes:

> This patch series were done by Vanderson and Alex originally in 2019, I
> (Fei Wu) rebased them on latest upstream from:
> https://github.com/stsquad/qemu/tree/tcg/tbstats-and-perf-v10
> and send out this review per Alex's request, I will continue to address
> any future review comments here. As it's been a very long time and there
> are lots of conflicts during rebase, it's my fault if I introduce any
> problems during the process.

Hi Fei,

Thanks for picking this up. I can confirm that this applies cleanly to
master and I have kicked the tyres and things still seem to work. I'm
not sure if I can provide much review on code I wrote but a few things
to point out:

  - there are a number of CI failures, mainly qatomic on 32 bit guests
see https://gitlab.com/stsquad/qemu/-/pipelines/844857279/failures
maybe we just disable time accounting for 32 bit hosts?

  - we need a proper solution to the invalidation of TBs so we can
exclude them from lists (or at least not do the attempt
translation/fail dance). Alternatively we could page out a copy of
the TB data to a disk file when we hit a certain hotness? How would
this interact with the jitperf support already?

  - we should add some documentation to the manual so users don't have
to figure it all out by trail and error at the HMP command line.

  - there may be some exit cases missed because I saw some weird TB's
with very long IR generation times.

TB id:5 | phys:0xb5f21d00 virt:0xcf2f17721d00 flags:0x0051 1 inv/2
| exec:1889055/0 guest inst cov:1.05%
| trans:2 ints: g:4 op:32 op_opt:26 spills:0
| h/g (host bytes / guest insts): 56.00
| time to gen at 2.4GHz => code:6723.33(ns) IR:2378539.17(ns)

  - code motion in 9/14 should be folded into the first patch

Even if we can't find a solution for safely dumping TBs I think the
series without "tb-list" is still an improvement for getting rid of the
--enable-profiler and making info JIT useful by default.

Richard,

What do you think?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v4 10/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

getpeername(2) syscall.

Add the getpeername(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index f748266730..16fae3752a 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -112,4 +112,32 @@ static inline abi_long do_bsd_accept(int fd, abi_ulong 
target_addr,
 return ret;
 }
 
+/* getpeername(2) */
+static inline abi_long do_bsd_getpeername(int fd, abi_ulong target_addr,
+  abi_ulong target_addrlen_addr)
+{
+socklen_t addrlen;
+void *addr;
+abi_long ret;
+
+if (get_user_u32(addrlen, target_addrlen_addr)) {
+return -TARGET_EFAULT;
+}
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+return -TARGET_EFAULT;
+}
+addr = alloca(addrlen);
+ret = get_errno(getpeername(fd, addr, &addrlen));
+if (!is_error(ret)) {
+host_to_target_sockaddr(target_addr, addr, addrlen);
+if (put_user_u32(addrlen, target_addrlen_addr)) {
+ret = -TARGET_EFAULT;
+}
+}
+return ret;
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v4 08/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

connect(2) syscall.

Add the connect(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index 7da4cf11a0..f191f22d63 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -58,4 +58,25 @@ static inline abi_long do_bsd_bind(int sockfd, abi_ulong 
target_addr,
 return get_errno(bind(sockfd, addr, addrlen));
 }
 
+/* connect(2) */
+static inline abi_long do_bsd_connect(int sockfd, abi_ulong target_addr,
+  socklen_t addrlen)
+{
+abi_long ret;
+void *addr;
+
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+addr = alloca(addrlen + 1);
+
+ret = target_to_host_sockaddr(addr, target_addr, addrlen);
+
+if (is_error(ret)) {
+return ret;
+}
+
+return get_errno(connect(sockfd, addr, addrlen));
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v4 09/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

accept(2) syscall.

Add the accept(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 33 +
 1 file changed, 33 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index f191f22d63..f748266730 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -79,4 +79,37 @@ static inline abi_long do_bsd_connect(int sockfd, abi_ulong 
target_addr,
 return get_errno(connect(sockfd, addr, addrlen));
 }
 
+/* accept(2) */
+static inline abi_long do_bsd_accept(int fd, abi_ulong target_addr,
+ abi_ulong target_addrlen_addr)
+{
+socklen_t addrlen;
+void *addr;
+abi_long ret;
+
+if (target_addr == 0) {
+return get_errno(accept(fd, NULL, NULL));
+}
+/* return EINVAL if addrlen pointer is invalid */
+if (get_user_u32(addrlen, target_addrlen_addr)) {
+return -TARGET_EINVAL;
+}
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+return -TARGET_EINVAL;
+}
+addr = alloca(addrlen);
+
+ret = get_errno(accept(fd, addr, &addrlen));
+if (!is_error(ret)) {
+host_to_target_sockaddr(target_addr, addr, addrlen);
+if (put_user_u32(addrlen, target_addrlen_addr)) {
+ret = -TARGET_EFAULT;
+}
+}
+return ret;
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v4 11/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Warner Losh 

Add the dispatching code of bind(2),connect(2), accpet(2), getpeername(2).

Add the bind(2), connect(2), accept(2), getpeername(2) syscalls case
statements to freebsd_syscall function defined in bsd-user/freebsd/os-syscall.c

Signed-off-by: Warner Losh 
Signed-off-by: Karim Taha 
---
 bsd-user/freebsd/os-syscall.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index c8f998ecec..7f29196a05 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -44,6 +44,8 @@
 #include "signal-common.h"
 #include "user/syscall-trace.h"
 
+/* BSD independent syscall shims */
+#include "bsd-socket.h"
 #include "bsd-file.h"
 #include "bsd-proc.h"
 
@@ -508,6 +510,25 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
 break;
 
+/*
+ * socket related system calls
+ */
+case TARGET_FREEBSD_NR_accept: /* accept(2) */
+ret = do_bsd_accept(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_bind: /* bind(2) */
+ret = do_bsd_bind(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_connect: /* connect(2) */
+ret = do_bsd_connect(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_getpeername: /* getpeername(2) */
+ret = do_bsd_getpeername(arg1, arg2, arg3);
+break;
+
 default:
 qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
 ret = -TARGET_ENOSYS;
-- 
2.40.0

[PATCH v4 07/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

The implementation of bind(2) syscall and socket related syscalls.

Add bsd-user/bsd-socket.h, which contains the implementation of
bind(2), and the socket related system call shims.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 61 +++
 1 file changed, 61 insertions(+)
 create mode 100644 bsd-user/bsd-socket.h

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
new file mode 100644
index 00..7da4cf11a0
--- /dev/null
+++ b/bsd-user/bsd-socket.h
@@ -0,0 +1,61 @@
+/*
+ *  socket related system call shims
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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_SOCKET_H
+#define BSD_SOCKET_H
+
+#include 
+#include 
+#include 
+#include 
+
+#include "qemu-bsd.h"
+
+ssize_t safe_recvfrom(int s, void *buf, size_t len, int flags,
+struct sockaddr *restrict from, socklen_t *restrict fromlen);
+ssize_t safe_sendto(int s, const void *buf, size_t len, int flags,
+const struct sockaddr *to, socklen_t tolen);
+int safe_select(int nfds, fd_set *readfs, fd_set *writefds, fd_set *exceptfds,
+struct timeval *timeout);
+int safe_pselect(int nfds, fd_set *restrict readfds,
+fd_set *restrict writefds, fd_set *restrict exceptfds,
+const struct timespec *restrict timeout,
+const sigset_t *restrict newsigmask);
+
+/* bind(2) */
+static inline abi_long do_bsd_bind(int sockfd, abi_ulong target_addr,
+   socklen_t addrlen)
+{
+abi_long ret;
+void *addr;
+
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+
+addr = alloca(addrlen + 1);
+ret = target_to_host_sockaddr(addr, target_addr, addrlen);
+if (is_error(ret)) {
+return ret;
+}
+
+return get_errno(bind(sockfd, addr, addrlen));
+}
+
+#endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH] kvm: Merge kvm_check_extension() and kvm_vm_check_extension()

[Jean-Philippe Brucker]

The KVM_CHECK_EXTENSION ioctl can be issued either on the global fd
(/dev/kvm), or on the VM fd obtained with KVM_CREATE_VM. For most
extensions, KVM returns the same value with either method, but for some
of them it can refine the returned value depending on the VM type. The
KVM documentation [1] advises to use the VM fd:

  Based on their initialization different VMs may have different
  capabilities. It is thus encouraged to use the vm ioctl to query for
  capabilities (available with KVM_CAP_CHECK_EXTENSION_VM on the vm fd)

Ongoing work on Arm confidential VMs confirms this, as some capabilities
become unavailable to confidential VMs, requiring changes in QEMU to use
kvm_vm_check_extension() instead of kvm_check_extension() [2]. Rather
than changing each check one by one, change kvm_check_extension() to
always issue the ioctl on the VM fd when available, and remove
kvm_vm_check_extension().

Fall back to the global fd when the VM check is unavailable:

* Ancient kernels do not support KVM_CHECK_EXTENSION on the VM fd, since
  it was added by commit 92b591a4c46b ("KVM: Allow KVM_CHECK_EXTENSION
  on the vm fd") in Linux 3.17 [3]. Support for Linux 3.16 ended only in
  June 2020, but there may still be old images around.

* A couple of calls must be issued before the VM fd is available, since
  they determine the VM type: KVM_CAP_MIPS_VZ and KVM_CAP_ARM_VM_IPA_SIZE

Does any user actually depend on the check being done on the global fd
instead of the VM fd?  I surveyed all cases where KVM can return
different values depending on the query method. Luckily QEMU already
calls kvm_vm_check_extension() for most of those. Only three of them are
ambiguous, because currently done on the global fd:

* KVM_CAP_MAX_VCPUS and KVM_CAP_MAX_VCPU_ID on Arm, changes value if the
  user requests a vGIC different from the default. But QEMU queries this
  before vGIC configuration, so the reported value will be the same.

* KVM_CAP_SW_TLB on PPC. When issued on the global fd, returns false if
  the kvm-hv module is loaded; when issued on the VM fd, returns false
  only if the VM type is HV instead of PR. If this returns false, then
  QEMU will fail to initialize a BOOKE206 MMU model.

  So this patch supposedly improves things, as it allows to run this
  type of vCPU even when both KVM modules are loaded.

* KVM_CAP_PPC_SECURE_GUEST. Similarly, doing this check on a VM fd
  refines the returned value, and ensures that SVM is actually
  supported. Since QEMU follows the check with kvm_vm_enable_cap(), this
  patch should only provide better error reporting.

[1] https://www.kernel.org/doc/html/latest/virt/kvm/api.html#kvm-check-extension
[2] https://lore.kernel.org/kvm/875ybi0ytc@redhat.com/
[3] https://github.com/torvalds/linux/commit/92b591a4c46b

Suggested-by: Cornelia Huck 
Signed-off-by: Jean-Philippe Brucker 
---
 include/sysemu/kvm.h |  2 --
 include/sysemu/kvm_int.h |  1 +
 accel/kvm/kvm-all.c  | 26 +-
 target/i386/kvm/kvm.c|  6 +++---
 target/ppc/kvm.c | 34 +-
 5 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index c8281c07a7..d62054004e 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -438,8 +438,6 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
-int kvm_vm_check_extension(KVMState *s, unsigned int extension);
-
 #define kvm_vm_enable_cap(s, capability, cap_flags, ...) \
 ({   \
 struct kvm_enable_cap cap = {\
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index a641c974ea..f6aa22ea09 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -122,6 +122,7 @@ struct KVMState
 uint32_t xen_caps;
 uint16_t xen_gnttab_max_frames;
 uint16_t xen_evtchn_max_pirq;
+bool check_extension_vm;
 };
 
 void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index cf3a88d90e..eca815e763 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1109,22 +1109,13 @@ int kvm_check_extension(KVMState *s, unsigned int 
extension)
 {
 int ret;
 
-ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, extension);
-if (ret < 0) {
-ret = 0;
+if (!s->check_extension_vm) {
+ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, extension);
+} else {
+ret = kvm_vm_ioctl(s, KVM_CHECK_EXTENSION, extension);
 }
-
-return ret;
-}
-
-int kvm_vm_check_extension(KVMState *s, unsigned int extension)
-{
-int ret;
-
-ret = kvm_vm_ioctl(s, KVM_CHECK_EXTENSION, extension);
 if (ret < 0) {
-/* VM wide version not implemented, use global one instead */
-ret = kvm_check_extension(s, extension);
+ret = 0;
 }
 
 return ret;
@@ -2328

[PATCH v4 02/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Add the socket conversion related flags and structs.

Add the relevant definitions of struct target_sockaddr and struct
target_ip_mreq and the related flags, to be used in
bsd-user/bsd-socket.c for the socket conversion functions:
target_to_host_sockaddr, host_to_target_sockaddr, target_to_host_ip_mreq

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/syscall_defs.h | 110 
 1 file changed, 110 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index b6d113d24a..f041245792 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -179,6 +179,116 @@ struct target_freebsd__wrusage {
 struct target_freebsd_rusage wru_children;
 };
 
+/*
+ * sys/socket.h
+ */
+
+/*
+ * Types
+ */
+#define TARGET_SOCK_STREAM  1   /* stream socket */
+#define TARGET_SOCK_DGRAM   2   /* datagram socket */
+#define TARGET_SOCK_RAW 3   /* raw-protocol interface */
+#define TARGET_SOCK_RDM 4   /* reliably-delivered message */
+#define TARGET_SOCK_SEQPACKET   5   /* sequenced packet stream */
+
+
+/*
+ * Option flags per-socket.
+ */
+
+#define TARGET_SO_DEBUG 0x0001  /* turn on debugging info recording */
+#define TARGET_SO_ACCEPTCONN0x0002  /* socket has had listen() */
+#define TARGET_SO_REUSEADDR 0x0004  /* allow local address reuse */
+#define TARGET_SO_KEEPALIVE 0x0008  /* keep connections alive */
+#define TARGET_SO_DONTROUTE 0x0010  /* just use interface addresses */
+#define TARGET_SO_BROADCAST 0x0020  /* permit sending of broadcast msgs */
+#define TARGET_SO_USELOOPBACK   0x0040  /* bypass hardware when possible */
+#define TARGET_SO_LINGER0x0080  /* linger on close if data present */
+#define TARGET_SO_OOBINLINE 0x0100  /* leave received OOB data in line */
+#define TARGET_SO_REUSEPORT 0x0200  /* allow local address & port reuse */
+#define TARGET_SO_TIMESTAMP 0x0400  /* timestamp received dgram traffic */
+#define TARGET_SO_NOSIGPIPE 0x0800  /* no SIGPIPE from EPIPE */
+#define TARGET_SO_ACCEPTFILTER  0x1000  /* there is an accept filter */
+#define TARGET_SO_BINTIME   0x2000  /* timestamp received dgram traffic */
+#define TARGET_SO_NO_OFFLOAD0x4000  /* socket cannot be offloaded */
+#define TARGET_SO_NO_DDP0x8000  /* disable direct data placement */
+
+/*
+ * Additional options, not kept in so_options.
+ */
+#define TARGET_SO_SNDBUF0x1001  /* send buffer size */
+#define TARGET_SO_RCVBUF0x1002  /* receive buffer size */
+#define TARGET_SO_SNDLOWAT  0x1003  /* send low-water mark */
+#define TARGET_SO_RCVLOWAT  0x1004  /* receive low-water mark */
+#define TARGET_SO_SNDTIMEO  0x1005  /* send timeout */
+#define TARGET_SO_RCVTIMEO  0x1006  /* receive timeout */
+#define TARGET_SO_ERROR 0x1007  /* get error status and clear */
+#define TARGET_SO_TYPE  0x1008  /* get socket type */
+#define TARGET_SO_LABEL 0x1009  /* socket's MAC label */
+#define TARGET_SO_PEERLABEL 0x1010  /* socket's peer's MAC label */
+#define TARGET_SO_LISTENQLIMIT  0x1011  /* socket's backlog limit */
+#define TARGET_SO_LISTENQLEN0x1012  /* socket's complete queue length */
+#define TARGET_SO_LISTENINCQLEN 0x1013  /* socket's incomplete queue length */
+#define TARGET_SO_SETFIB0x1014  /* use this FIB to route */
+#define TARGET_SO_USER_COOKIE   0x1015  /* user cookie (dummynet etc.) */
+#define TARGET_SO_PROTOCOL  0x1016  /* get socket protocol (Linux name) */
+
+/* alias for SO_PROTOCOL (SunOS name) */
+#define TARGET_SO_PROTOTYPE TARGET_SO_PROTOCOL
+
+/*
+ * Level number for (get/set)sockopt() to apply to socket itself.
+ */
+#define TARGET_SOL_SOCKET   0x  /* options for socket level */
+
+#ifndef CMSG_ALIGN
+#define CMSG_ALIGN(len) (((len) + sizeof(long) - 1) & ~(sizeof(long) - 1))
+#endif
+
+/*
+ * sys/socket.h
+ */
+struct target_msghdr {
+abi_longmsg_name;   /* Socket name */
+int32_t msg_namelen;/* Length of name */
+abi_longmsg_iov;/* Data blocks */
+int32_t msg_iovlen; /* Number of blocks */
+abi_longmsg_control;/* Per protocol magic (eg BSD fd passing) */
+int32_t msg_controllen; /* Length of cmsg list */
+int32_t msg_flags;  /* flags on received message */
+};
+
+struct target_sockaddr {
+uint8_t sa_len;
+uint8_t sa_family;
+uint8_t sa_data[14];
+} QEMU_PACKED;
+
+struct target_in_addr {
+uint32_t s_addr; /* big endian */
+};
+
+struct target_cmsghdr {
+uint32_tcmsg_len;
+int32_t cmsg_level;
+int32_t cmsg_type;
+};
+
+/*
+ * netinet/in.h
+ */
+struct target_ip_mreq {
+struct target_in_addr   imr_multiaddr;
+struct target_in_addr   imr_interface;
+};
+
+struct target_ip_mreqn {
+struct target_in_addr   imr_multiaddr;
+struct target_in_addr   imr_address;
+int32_t imr_ifindex;
+};
+
 #defi

[PATCH v4 04/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Declaration of the socket conversion functions.

Add bsd-user/qemu-bsd.h, required by bsd-user/bsd-socket.h, contains
forward declarations of the socket conversion functions defined in 
bsd-user/bsd-socket.c.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/qemu-bsd.h | 36 
 1 file changed, 36 insertions(+)
 create mode 100644 bsd-user/qemu-bsd.h

diff --git a/bsd-user/qemu-bsd.h b/bsd-user/qemu-bsd.h
new file mode 100644
index 00..a052688596
--- /dev/null
+++ b/bsd-user/qemu-bsd.h
@@ -0,0 +1,36 @@
+/*
+ *  BSD conversion extern declarations
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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 QEMU_BSD_H
+#define QEMU_BSD_H
+
+#include 
+#include 
+#include 
+#include 
+
+/* bsd-socket.c */
+abi_long target_to_host_sockaddr(struct sockaddr *addr, abi_ulong target_addr,
+socklen_t len);
+abi_long host_to_target_sockaddr(abi_ulong target_addr, struct sockaddr *addr,
+socklen_t len);
+abi_long target_to_host_ip_mreq(struct ip_mreqn *mreqn, abi_ulong target_addr,
+socklen_t len);
+
+#endif /* QEMU_BSD_H */
-- 
2.40.0

[PATCH v4 06/11] Signed-off-by: Karim Taha

[Karim Taha]

Build bsd-user/bsd-socket.c.

Add bsd-user/bsd-socket.c to meson.build.

Signed-off-by: Karim Taha 
---
 bsd-user/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bsd-user/meson.build b/bsd-user/meson.build
index 5243122fc5..f648bd3554 100644
--- a/bsd-user/meson.build
+++ b/bsd-user/meson.build
@@ -7,6 +7,7 @@ bsd_user_ss = ss.source_set()
 common_user_inc += include_directories('include')
 
 bsd_user_ss.add(files(
+  'bsd-socket.c',
   'bsdload.c',
   'elfload.c',
   'main.c',
-- 
2.40.0

[PATCH v4 03/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Sean Bruno 

Target cmsghdr struct and flags.

Add the cmsghdr struct and alignment flags.

Co-authored-by: Kyle Evans 
Signed-off-by: Sean Bruno 
Signed-off-by: Kyle Evans 
Signed-off-by: Karim Taha 
---
 bsd-user/syscall_defs.h | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index f041245792..b594fafecc 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -275,6 +275,44 @@ struct target_cmsghdr {
 int32_t cmsg_type;
 };
 
+/*
+ * mips32 is the exception to the general rule of long-alignment; it
+ * unconditionally uses 64-bit alignment instead.
+ */
+#if defined(TARGET_MIPS) && TARGET_ABI_BITS == 32
+#define TARGET_ALIGNBYTES   (sizeof(abi_llong) - 1)
+#else
+#define TARGET_ALIGNBYTES   (sizeof(abi_long) - 1)
+#endif
+
+#define TARGET_CMSG_NXTHDR(mhdr, cmsg, cmsg_start) \
+   __target_cmsg_nxthdr(mhdr, cmsg, cmsg_start)
+#define TARGET_CMSG_ALIGN(len) (((len) + TARGET_ALIGNBYTES) \
+   & (size_t) ~TARGET_ALIGNBYTES)
+#define TARGET_CMSG_DATA(cmsg) \
+((unsigned char *)(cmsg) + TARGET_CMSG_ALIGN(sizeof(struct 
target_cmsghdr)))
+#define TARGET_CMSG_SPACE(len) \
+(TARGET_CMSG_ALIGN(sizeof(struct target_cmsghdr)) + TARGET_CMSG_ALIGN(len))
+#define TARGET_CMSG_LEN(len) \
+(TARGET_CMSG_ALIGN(sizeof(struct target_cmsghdr)) + (len))
+
+static inline struct target_cmsghdr *
+__target_cmsg_nxthdr(struct target_msghdr *__mhdr,
+ struct target_cmsghdr *__cmsg,
+ struct target_cmsghdr *__cmsg_start)
+{
+struct target_cmsghdr *__ptr;
+
+__ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg +
+TARGET_CMSG_ALIGN(tswap32(__cmsg->cmsg_len)));
+if ((unsigned long)((char *)(__ptr + 1) - (char *)__cmsg_start) >
+tswap32(__mhdr->msg_controllen)) {
+/* No more entries.  */
+return (struct target_cmsghdr *)0;
+}
+return __ptr;
+}
+
 /*
  * netinet/in.h
  */
-- 
2.40.0

[PATCH v4 01/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Warner Losh 

Intialize guest_base in bsd-user/main.c.

Allow guest_base to be initialized on 64-bit hosts, the initial value is used 
by g2h_untagged function defined in include/exec/cpu_ldst.h

Signed-off-by: Warner Losh 
Signed-off-by: Karim Taha 
---
 bsd-user/main.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index babc3b009b..afdc1b5f3c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -50,8 +50,22 @@
 #include "target_arch_cpu.h"
 
 int singlestep;
-uintptr_t guest_base;
+
+/*
+ * Going hand in hand with the va space needed (see below), we need
+ * to find a host address to map the guest to. Assume that qemu
+ * itself doesn't need memory above 32GB (or that we don't collide
+ * with anything interesting). This is selected rather arbitrarily,
+ * but seems to produce good results in tests to date.
+ */
+# if HOST_LONG_BITS >= 64
+uintptr_t guest_base = 0x8ul;/* at 32GB */
+bool have_guest_base = true;
+#else
+uintptr_t guest_base;/* TODO: use sysctl to find big enough hole */
 bool have_guest_base;
+#endif
+
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
  * guest address space into a contiguous chunk of virtual host memory.
-- 
2.40.0

[PATCH v4 05/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Definitions of the socket conversion functions.

Add bsd-user/bsd-socket.c, which contains the actual definitions of the
socket conversion functions.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.c | 108 ++
 1 file changed, 108 insertions(+)
 create mode 100644 bsd-user/bsd-socket.c

diff --git a/bsd-user/bsd-socket.c b/bsd-user/bsd-socket.c
new file mode 100644
index 00..8a5e4d
--- /dev/null
+++ b/bsd-user/bsd-socket.c
@@ -0,0 +1,108 @@
+/*
+ *  BSD socket system call related helpers
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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 
+#include 
+#include 
+#include 
+
+#include "qemu.h"
+#include "qemu-bsd.h"
+
+/*
+ * socket conversion
+ */
+abi_long target_to_host_sockaddr(struct sockaddr *addr, abi_ulong target_addr,
+ socklen_t len)
+{
+const socklen_t unix_maxlen = sizeof(struct sockaddr_un);
+sa_family_t sa_family;
+struct target_sockaddr *target_saddr;
+
+target_saddr = lock_user(VERIFY_READ, target_addr, len, 1);
+if (target_saddr == 0) {
+return -TARGET_EFAULT;
+}
+
+sa_family = target_saddr->sa_family;
+
+/*
+ * Oops. The caller might send a incomplete sun_path; sun_path
+ * must be terminated by \0 (see the manual page), but unfortunately
+ * it is quite common to specify sockaddr_un length as
+ * "strlen(x->sun_path)" while it should be "strlen(...) + 1". We will
+ * fix that here if needed.
+ */
+if (target_saddr->sa_family == AF_UNIX) {
+if (len < unix_maxlen && len > 0) {
+char *cp = (char *)target_saddr;
+
+if (cp[len - 1] && !cp[len]) {
+len++;
+}
+}
+if (len > unix_maxlen) {
+len = unix_maxlen;
+}
+}
+
+memcpy(addr, target_saddr, len);
+addr->sa_family = sa_family;/* type uint8_t */
+addr->sa_len = target_saddr->sa_len;/* type uint8_t */
+unlock_user(target_saddr, target_addr, 0);
+
+return 0;
+}
+
+abi_long host_to_target_sockaddr(abi_ulong target_addr, struct sockaddr *addr,
+ socklen_t len)
+{
+struct target_sockaddr *target_saddr;
+
+target_saddr = lock_user(VERIFY_WRITE, target_addr, len, 0);
+if (target_saddr == 0) {
+return -TARGET_EFAULT;
+}
+memcpy(target_saddr, addr, len);
+target_saddr->sa_family = addr->sa_family;  /* type uint8_t */
+target_saddr->sa_len = addr->sa_len;/* type uint8_t */
+unlock_user(target_saddr, target_addr, len);
+
+return 0;
+}
+
+abi_long target_to_host_ip_mreq(struct ip_mreqn *mreqn, abi_ulong target_addr,
+socklen_t len)
+{
+struct target_ip_mreqn *target_smreqn;
+
+target_smreqn = lock_user(VERIFY_READ, target_addr, len, 1);
+if (target_smreqn == 0) {
+return -TARGET_EFAULT;
+}
+mreqn->imr_multiaddr.s_addr = target_smreqn->imr_multiaddr.s_addr;
+mreqn->imr_address.s_addr = target_smreqn->imr_address.s_addr;
+if (len == sizeof(struct target_ip_mreqn)) {
+mreqn->imr_ifindex = tswapal(target_smreqn->imr_ifindex);
+}
+unlock_user(target_smreqn, target_addr, 0);
+
+return 0;
+}
-- 
2.40.0

[PATCH v4 00/11] Contribution task implementations, for the 'FreeBSD user emulation improvements' project.

[Karim Taha]

Upstream the implementations of bind(2), connect(2), accept(2) and
getpeername(2) system calls from the blitz branch of the bsd-user fork hosted at
https://github.com/qemu-bsd-user/qemu-bsd-user/tree/blitz.

Karim Taha (1):
  Signed-off-by: Karim Taha 

Sean Bruno (1):
  Signed-off-by: Karim Taha 

Stacey Son (7):
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 

Warner Losh (2):
  Signed-off-by: Karim Taha 
  Signed-off-by: Karim Taha 

 bsd-user/bsd-socket.c | 108 +
 bsd-user/bsd-socket.h | 143 
 bsd-user/freebsd/os-syscall.c |  21 +
 bsd-user/main.c   |  16 +++-
 bsd-user/meson.build  |   1 +
 bsd-user/qemu-bsd.h   |  36 +
 bsd-user/syscall_defs.h   | 148 ++
 7 files changed, 472 insertions(+), 1 deletion(-)
 create mode 100644 bsd-user/bsd-socket.c
 create mode 100644 bsd-user/bsd-socket.h
 create mode 100644 bsd-user/qemu-bsd.h

-- 
2.40.0

Re: [PATCH 01/11] Signed-off-by: Karim Taha

[Karim Taha]

On Fri, Apr 21, 2023 at 9:17 AM Daniel P. Berrangé 
wrote:

> On Fri, Apr 21, 2023 at 07:22:45AM +0200, Karim Taha wrote:
> > From: Warner Losh 
> >
> > Allow guest_base to be initialized on 64-bit hosts, the initial value is
> used by g2h_untagged function defined in include/exec/cpu_ldst.h
>
> This commit message is all incorrectly structured I'm afraid.
>
> There needs to a short 1 line summary, then a blank line,
> then the full commit description text, then a blank line,
> then the Signed-off-by tag(s).
>
> Also if you're sending work done by Warner (as the From
> tag suggests), then we would expect to see Warner's own
> Signed-off-by tag, in addition to your own Signed-off-by.
>
> > ---
> >  bsd-user/main.c | 16 +++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/bsd-user/main.c b/bsd-user/main.c
> > index babc3b009b..afdc1b5f3c 100644
> > --- a/bsd-user/main.c
> > +++ b/bsd-user/main.c
> > @@ -50,8 +50,22 @@
> >  #include "target_arch_cpu.h"
> >
> >  int singlestep;
> > -uintptr_t guest_base;
> > +
> > +/*
> > + * Going hand in hand with the va space needed (see below), we need
> > + * to find a host address to map the guest to. Assume that qemu
> > + * itself doesn't need memory above 32GB (or that we don't collide
> > + * with anything interesting). This is selected rather arbitrarily,
> > + * but seems to produce good results in tests to date.
> > + */
> > +# if HOST_LONG_BITS >= 64
> > +uintptr_t guest_base = 0x8ul;/* at 32GB */
> > +bool have_guest_base = true;
> > +#else
> > +uintptr_t guest_base;/* TODO: use sysctl to find big enough hole */
> >  bool have_guest_base;
> > +#endif
> > +
> >  /*
> >   * When running 32-on-64 we should make sure we can fit all of the
> possible
> >   * guest address space into a contiguous chunk of virtual host memory.
> > --
> > 2.40.0
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>
Alright, thanks for the commit formatting tips, I resent the patch series,
with my signed off by tag and the author signed off by tags as well.

Best regards,
Karim

[PULL v2 00/10] Hexagon queue

[Taylor Simpson]

The following changes since commit 60ca584b8af0de525656f959991a440f8c191f12:

  Merge tag 'pull-for-8.0-220323-1' of https://gitlab.com/stsquad/qemu into 
staging (2023-03-22 17:58:12 +)

are available in the Git repository at:

  https://github.com/quic/qemu tags/pull-hex-20230421

for you to fetch changes up to a305a170398d80c08e19c2ef4c8637a4f4de50e1:

  Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions 
(2023-04-21 09:32:52 -0700)


Hexagon update

 Changes in v2 
Remove patch that breaks clang CI



Marco Liebel (2):
  Use f-strings in python scripts
  Use black code style for python scripts

Matheus Tavares Bernardino (1):
  Hexagon (translate.c): avoid redundant PC updates on COF

Taylor Simpson (7):
  Hexagon (target/hexagon) Remove redundant/unused macros
  Hexagon (target/hexagon) Merge arguments to probe_pkt_scalar_hvx_stores
  Hexagon (target/hexagon) Add overrides for count trailing zeros/ones
  Hexagon (target/hexagon) Updates to USR should use get_result_gpr
  Hexagon (tests/tcg/hexagon) Move HVX test infra to header file
  Hexagon (target/hexagon) Remove unused slot variable in helpers
  Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions

 target/hexagon/gen_tcg.h|  52 +-
 target/hexagon/genptr.h |  10 +-
 target/hexagon/helper.h |   4 +-
 target/hexagon/macros.h |  91 +---
 target/hexagon/op_helper.h  |   2 +-
 target/hexagon/translate.h  |   1 +
 tests/tcg/hexagon/hvx_misc.h| 178 +++
 target/hexagon/genptr.c |  49 +-
 target/hexagon/idef-parser/parser-helpers.c |   5 +-
 target/hexagon/op_helper.c  |   6 +-
 target/hexagon/translate.c  |  31 +-
 tests/tcg/hexagon/hvx_misc.c| 160 +-
 tests/tcg/hexagon/misc.c|  56 ++-
 target/hexagon/dectree.py   | 396 ---
 target/hexagon/gen_analyze_funcs.py | 226 -
 target/hexagon/gen_helper_funcs.py  | 362 --
 target/hexagon/gen_helper_protos.py | 169 ---
 target/hexagon/gen_idef_parser_funcs.py |  85 ++--
 target/hexagon/gen_op_attribs.py|  12 +-
 target/hexagon/gen_op_regs.py   |  79 +--
 target/hexagon/gen_opcodes_def.py   |   8 +-
 target/hexagon/gen_printinsn.py |  84 ++--
 target/hexagon/gen_shortcode.py |  19 +-
 target/hexagon/gen_tcg_func_table.py|  18 +-
 target/hexagon/gen_tcg_funcs.py | 729 +++-
 target/hexagon/hex_common.py| 181 ---
 target/hexagon/idef-parser/idef-parser.y|   2 +-
 tests/tcg/hexagon/Makefile.target   |   1 +
 28 files changed, 1664 insertions(+), 1352 deletions(-)
 create mode 100644 tests/tcg/hexagon/hvx_misc.h

[PULL v2 07/10] Hexagon (target/hexagon) Updates to USR should use get_result_gpr

[Taylor Simpson]

Signed-off-by: Taylor Simpson 
Reviewed-by: Anton Johansson 
Message-Id: <20230405164211.30015-3-tsimp...@quicinc.com>
---
 target/hexagon/gen_tcg.h|  4 +-
 target/hexagon/genptr.h | 10 ++---
 target/hexagon/macros.h |  8 
 target/hexagon/genptr.c | 49 ++---
 target/hexagon/idef-parser/parser-helpers.c |  5 ++-
 target/hexagon/idef-parser/idef-parser.y|  2 +-
 6 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index 45f92adf6c..b189f725d7 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -1039,11 +1039,11 @@
 
 /* r0 = asr(r1, r2):sat */
 #define fGEN_TCG_S2_asr_r_r_sat(SHORTCODE) \
-gen_asr_r_r_sat(RdV, RsV, RtV)
+gen_asr_r_r_sat(ctx, RdV, RsV, RtV)
 
 /* r0 = asl(r1, r2):sat */
 #define fGEN_TCG_S2_asl_r_r_sat(SHORTCODE) \
-gen_asl_r_r_sat(RdV, RsV, RtV)
+gen_asl_r_r_sat(ctx, RdV, RsV, RtV)
 
 #define fGEN_TCG_SL2_jumpr31(SHORTCODE) \
 gen_jumpr(ctx, hex_gpr[HEX_REG_LR])
diff --git a/target/hexagon/genptr.h b/target/hexagon/genptr.h
index 591b059698..76e497aa48 100644
--- a/target/hexagon/genptr.h
+++ b/target/hexagon/genptr.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  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
@@ -37,9 +37,9 @@ TCGv gen_read_reg(TCGv result, int num);
 TCGv gen_read_preg(TCGv pred, uint8_t num);
 void gen_log_reg_write(int rnum, TCGv val);
 void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val);
-void gen_set_usr_field(int field, TCGv val);
-void gen_set_usr_fieldi(int field, int x);
-void gen_set_usr_field_if(int field, TCGv val);
+void gen_set_usr_field(DisasContext *ctx, int field, TCGv val);
+void gen_set_usr_fieldi(DisasContext *ctx, int field, int x);
+void gen_set_usr_field_if(DisasContext *ctx, int field, TCGv val);
 void gen_sat_i32(TCGv dest, TCGv source, int width);
 void gen_sat_i32_ovfl(TCGv ovfl, TCGv dest, TCGv source, int width);
 void gen_satu_i32(TCGv dest, TCGv source, int width);
@@ -48,7 +48,7 @@ void gen_sat_i64(TCGv_i64 dest, TCGv_i64 source, int width);
 void gen_sat_i64_ovfl(TCGv ovfl, TCGv_i64 dest, TCGv_i64 source, int width);
 void gen_satu_i64(TCGv_i64 dest, TCGv_i64 source, int width);
 void gen_satu_i64_ovfl(TCGv ovfl, TCGv_i64 dest, TCGv_i64 source, int width);
-void gen_add_sat_i64(TCGv_i64 ret, TCGv_i64 a, TCGv_i64 b);
+void gen_add_sat_i64(DisasContext *ctx, TCGv_i64 ret, TCGv_i64 a, TCGv_i64 b);
 TCGv gen_8bitsof(TCGv result, TCGv value);
 void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src);
 TCGv gen_get_byte(TCGv result, int N, TCGv src, bool sign);
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index f5f31b6930..21b5b5a06c 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -38,14 +38,6 @@
 #define TYPE_INT(X)  __builtin_types_compatible_p(typeof(X), int)
 #define TYPE_TCGV(X) __builtin_types_compatible_p(typeof(X), TCGv)
 #define TYPE_TCGV_I64(X) __builtin_types_compatible_p(typeof(X), TCGv_i64)
-
-#define SET_USR_FIELD_FUNC(X) \
-__builtin_choose_expr(TYPE_INT(X), \
-gen_set_usr_fieldi, \
-__builtin_choose_expr(TYPE_TCGV(X), \
-gen_set_usr_field, (void)0))
-#define SET_USR_FIELD(FIELD, VAL) \
-SET_USR_FIELD_FUNC(VAL)(FIELD, VAL)
 #else
 #define GET_USR_FIELD(FIELD) \
 fEXTRACTU_BITS(env->gpr[HEX_REG_USR], reg_field_info[FIELD].width, \
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index bb274d4a71..502c85ae35 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -486,30 +486,27 @@ static void gen_write_new_pc_pcrel(DisasContext *ctx, int 
pc_off,
 }
 }
 
-void gen_set_usr_field(int field, TCGv val)
+void gen_set_usr_field(DisasContext *ctx, int field, TCGv val)
 {
-tcg_gen_deposit_tl(hex_new_value[HEX_REG_USR], hex_new_value[HEX_REG_USR],
-   val,
+TCGv usr = get_result_gpr(ctx, HEX_REG_USR);
+tcg_gen_deposit_tl(usr, usr, val,
reg_field_info[field].offset,
reg_field_info[field].width);
 }
 
-void gen_set_usr_fieldi(int field, int x)
+void gen_set_usr_fieldi(DisasContext *ctx, int field, int x)
 {
 if (reg_field_info[field].width == 1) {
+TCGv usr = get_result_gpr(ctx, HEX_REG_USR);
 target_ulong bit = 1 << reg_field_info[field].offset;
 if ((x & 1) == 1) {
-tcg_gen_ori_tl(hex_new_value[HEX_REG_USR],
-   hex_new_value[HEX_REG_USR],
-   bit);
+tcg_gen_ori_tl(usr, usr, bit);
 } else {
-tcg_gen_andi_tl(hex_new_value[HEX_REG_USR],
-

[PULL v2 08/10] Hexagon (tests/tcg/hexagon) Move HVX test infra to header file

[Taylor Simpson]

This will facilitate adding additional tests in separate .c files

Signed-off-by: Taylor Simpson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230406174241.853296-1-tsimp...@quicinc.com>
---
 tests/tcg/hexagon/hvx_misc.h  | 178 ++
 tests/tcg/hexagon/hvx_misc.c  | 160 +--
 tests/tcg/hexagon/Makefile.target |   1 +
 3 files changed, 181 insertions(+), 158 deletions(-)
 create mode 100644 tests/tcg/hexagon/hvx_misc.h

diff --git a/tests/tcg/hexagon/hvx_misc.h b/tests/tcg/hexagon/hvx_misc.h
new file mode 100644
index 00..2e868340fd
--- /dev/null
+++ b/tests/tcg/hexagon/hvx_misc.h
@@ -0,0 +1,178 @@
+/*
+ *  Copyright(c) 2021-2023 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *
+ *  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 HVX_MISC_H
+#define HVX_MISC_H
+
+static inline void check(int line, int i, int j,
+ uint64_t result, uint64_t expect)
+{
+if (result != expect) {
+printf("ERROR at line %d: [%d][%d] 0x%016llx != 0x%016llx\n",
+   line, i, j, result, expect);
+err++;
+}
+}
+
+#define MAX_VEC_SIZE_BYTES 128
+
+typedef union {
+uint64_t ud[MAX_VEC_SIZE_BYTES / 8];
+int64_t   d[MAX_VEC_SIZE_BYTES / 8];
+uint32_t uw[MAX_VEC_SIZE_BYTES / 4];
+int32_t   w[MAX_VEC_SIZE_BYTES / 4];
+uint16_t uh[MAX_VEC_SIZE_BYTES / 2];
+int16_t   h[MAX_VEC_SIZE_BYTES / 2];
+uint8_t  ub[MAX_VEC_SIZE_BYTES / 1];
+int8_tb[MAX_VEC_SIZE_BYTES / 1];
+} MMVector;
+
+#define BUFSIZE  16
+#define OUTSIZE  16
+#define MASKMOD  3
+
+MMVector buffer0[BUFSIZE] __attribute__((aligned(MAX_VEC_SIZE_BYTES)));
+MMVector buffer1[BUFSIZE] __attribute__((aligned(MAX_VEC_SIZE_BYTES)));
+MMVector mask[BUFSIZE] __attribute__((aligned(MAX_VEC_SIZE_BYTES)));
+MMVector output[OUTSIZE] __attribute__((aligned(MAX_VEC_SIZE_BYTES)));
+MMVector expect[OUTSIZE] __attribute__((aligned(MAX_VEC_SIZE_BYTES)));
+
+#define CHECK_OUTPUT_FUNC(FIELD, FIELDSZ) \
+static inline void check_output_##FIELD(int line, size_t num_vectors) \
+{ \
+for (int i = 0; i < num_vectors; i++) { \
+for (int j = 0; j < MAX_VEC_SIZE_BYTES / FIELDSZ; j++) { \
+check(line, i, j, output[i].FIELD[j], expect[i].FIELD[j]); \
+} \
+} \
+}
+
+CHECK_OUTPUT_FUNC(d,  8)
+CHECK_OUTPUT_FUNC(w,  4)
+CHECK_OUTPUT_FUNC(h,  2)
+CHECK_OUTPUT_FUNC(b,  1)
+
+static inline void init_buffers(void)
+{
+int counter0 = 0;
+int counter1 = 17;
+for (int i = 0; i < BUFSIZE; i++) {
+for (int j = 0; j < MAX_VEC_SIZE_BYTES; j++) {
+buffer0[i].b[j] = counter0++;
+buffer1[i].b[j] = counter1++;
+}
+for (int j = 0; j < MAX_VEC_SIZE_BYTES / 4; j++) {
+mask[i].w[j] = (i + j % MASKMOD == 0) ? 0 : 1;
+}
+}
+}
+
+#define VEC_OP1(ASM, EL, IN, OUT) \
+asm("v2 = vmem(%0 + #0)\n\t" \
+"v2" #EL " = " #ASM "(v2" #EL ")\n\t" \
+"vmem(%1 + #0) = v2\n\t" \
+: : "r"(IN), "r"(OUT) : "v2", "memory")
+
+#define VEC_OP2(ASM, EL, IN0, IN1, OUT) \
+asm("v2 = vmem(%0 + #0)\n\t" \
+"v3 = vmem(%1 + #0)\n\t" \
+"v2" #EL " = " #ASM "(v2" #EL ", v3" #EL ")\n\t" \
+"vmem(%2 + #0) = v2\n\t" \
+: : "r"(IN0), "r"(IN1), "r"(OUT) : "v2", "v3", "memory")
+
+#define TEST_VEC_OP1(NAME, ASM, EL, FIELD, FIELDSZ, OP) \
+static inline void test_##NAME(void) \
+{ \
+void *pin = buffer0; \
+void *pout = output; \
+for (int i = 0; i < BUFSIZE; i++) { \
+VEC_OP1(ASM, EL, pin, pout); \
+pin += sizeof(MMVector); \
+pout += sizeof(MMVector); \
+} \
+for (int i = 0; i < BUFSIZE; i++) { \
+for (int j = 0; j < MAX_VEC_SIZE_BYTES / FIELDSZ; j++) { \
+expect[i].FIELD[j] = OP buffer0[i].FIELD[j]; \
+} \
+} \
+check_output_##FIELD(__LINE__, BUFSIZE); \
+}
+
+#define TEST_VEC_OP2(NAME, ASM, EL, FIELD, FIELDSZ, OP) \
+static inline void test_##NAME(void) \
+{ \
+void *p0 = buffer0; \
+void *p1 = buffer1; \
+void *pout = output; \
+for (int i = 0; i < BUFSIZE; i++) { \
+VEC_OP2(ASM, EL, p0, p1, pout); \
+p0 += sizeof(MMVector); \
+p1 += sizeof(MMVector); \
+pout += sizeof(MMVector); \
+} \
+for (int i = 0; i < BUFSIZE; i++) { \
+for (int j =

Re: [PATCH] hw/riscv: virt: Enable booting M-mode or S-mode FW from pflash0

[Andrea Bolognani]

On Fri, Apr 21, 2023 at 04:36:15PM +0200, Heinrich Schuchardt wrote:
> On 4/21/23 06:33, Sunil V L wrote:
> > Currently, virt machine supports two pflash instances each with
> > 32MB size. However, the first pflash is always assumed to
> > contain M-mode firmware and reset vector is set to this if
> > enabled. Hence, for S-mode payloads like EDK2, only one pflash
> > instance is available for use. This means both code and NV variables
> > of EDK2 will need to use the same pflash.
> >
> > The OS distros keep the EDK2 FW code as readonly. When non-volatile
> > variables also need to share the same pflash, it is not possible
> > to keep it as readonly since variables need write access.
> >
> > To resolve this issue, the code and NV variables need to be separated.
> > But in that case we need an extra flash. Hence, modify the convention
> > such that pflash0 will contain the M-mode FW only when "-bios none"
> > option is used. Otherwise, pflash0 will contain the S-mode payload FW.
> > This enables both pflash instances available for EDK2 use.
> >
> > Example usage:
> > 1) pflash0 containing M-mode FW
> > qemu-system-riscv64 -bios none -pflash  -machine virt
> > or
> > qemu-system-riscv64 -bios none \
> > -drive file=,if=pflash,format=raw,unit=0 -machine virt
> >
> > 2) pflash0 containing S-mode payload like EDK2
> > qemu-system-riscv64 -pflash  -pflash  -machine  
> > virt
> > or
> > qemu-system-riscv64 -bios  \
> > -pflash  \
> > -pflash  \
> > -machine  virt
> > or
> > qemu-system-riscv64 -bios  \
> > -drive file=,if=pflash,format=raw,unit=0,readonly=on \
> > -drive file=,if=pflash,format=raw,unit=1 \
> > -machine virt
> >
> > Signed-off-by: Sunil V L 
> > Reported-by: Heinrich Schuchardt 
>
> QEMU 7.2 (and possibly 8.0 to be released) contains the old behavior.
>
> Changed use of command line parameters should depend on the version of
> the virt machine, i.e. virt-7.2 should use the old behavior and virt as
> alias for virt-8.0 should use the new behavior. Please, have a look at
> the option handling in hw/arm/virt.c and macro DEFINE_VIRT_MACHINE().

I would normally agree with you, but note that RISC-V doesn't have
versioned machine types yet, so this kind of breakage is not
necessarily unexpected.

>From libvirt's point of view, being able to detect whether the new
behavior is implemented by looking for some machine type property
would be enough to handle the transition smoothly. That would of
course not help people running QEMU directly.

For what it's worth, this change seems to go in the right direction
by making things similar to other architectures (x86, Arm) so I'd
love to see it happen.

-- 
Andrea Bolognani / Red Hat / Virtualization

[RFC] hw/arm/virt: Provide DT binding generation for PCI eXpander Bridges

[Jonathan Cameron via]

This patch and the problem it is trying to resolve will form part of a talk
I will be giving next week at Linaro Connect. https://sched.co/1K85p

So in the spirit of giving people almost no warning... Plus being able to
say the discussion has kicked off here is the simplest solution I could
come up with. If we can agree on an approach to the sizing of memory
windows question by Thursday even better (I'll update the slides!) ;)

This is a precursor for CXL on arm-virt (for which DT support was
requested). CXL QEMU emulation uses pxb-cxl which inherits from the
slightly simpler pxb-pcie. ACPI support for these additional host bridges
has been available for some time but relies an interesting dance with
EDK2:
* During initial board creation the PXB buses are largely ignored
  and ACPI tables + DT passed to EDK2 only expose the root bus on
  which the section of the PXB instance that exists as a normal PCI EP
  may be discovered.
* EDK2 then carries out full PCI bus enumeration, including the buses
  below the PXB host bridges.
* Finally EDK2 calls back into QEMU to rebuild the tables via
  virt_acpi_build_update(), at which point the correct DSDT for the
  PXB host bridges is generated and an adjust DSDT section is generated
  for the primary host bridge (holes are bunched in the _CRS).

For device tree bindings there is no such dance with the firmware and as
such we need QEMU to directly present device tree entries for the primary
PCIe bus and the PXB instances.

This cannot be fully done either at initial PCIe instantiation, (PXB
instances not yet instantiated) or at in virt_machine_done() as it is
for ACPI (virtio-mem hot plug routines rely on presence of primary PCIe bus).
Thus the proposed solution is to set things up initially without being
careful and later update it with additional checks that we don't get
overlapping bus numbers.

It 'might' be possible to use hotplug handlers to update the DT as we
go along, but that would be rather challenging as each additional PXB
introduction would sometimes require an update of the dt for all other
instances.

Note: The linux,pci-domain dt element is more restrictive than the
equivalent in ACPI, so for now just put each PXB in it's own domain. I'll
look to address relaxing that on the kernel side, but then this code won't
initially work with older kernels - so this lack of alignment with ACPI
systems may have to persist (you end up with a single segment for ACPI
systems, and multiple for DT).

Another question is how to allocate the resources. A PXB instance uses
a section of the ECAM space (which is fine as that is defined by bus
numbers) and also part of each of the PCI windows. When EDK2 is involved,
the allocation of the windows is done after enumeration of connected
PCI devices / switches etc with heuristics adding extra space for
hotplug.  Thus the windows configured for the bridges can be easily
established and written into the ACPI _CRS entries.

I've considered two options:
1) (More or less) enumerate the full PCI topology in QEMU, just don't
   actually write the registers for routing. Apply similar heuristics
   (note that EDK2 and the kernel may have different ones and they
   have changed over time) to establish how much memory is needed
   in each window. This is a complex solution that may still provide
   allocations that aren't big enough for the kernel to then
   enumerate the PCI topology using it's algorithms.
2) Use the one thing we definitely control for PXB instances, the base
   bus number.  Based on how many buses are available to each PXB
   instance allocate a proportion of the available memory windows. Again,
   it's more than possible that this won't leave enough room to provide
   all of the necessary space to accommodate BARs, but is fairly simple to
   implement and fairly simple to understand when the allocation doesn't
   work. This option is what the current code does.

Note that either way any regressions should be limited to systems using
DT with PXB instances which would be odd given they don't currently work.

RFC being posted to get feedback on the general approach we should take.
One option would be to ignore pxb-pcie and only deal with pxb-cxl on
the basis that removes risk of regressions because we don't support
CXL on arm-virt yet.  However, code will end up much the same so we
can discuss this without the added fun that CXL will bring.

Other more minor nasties:
* EDK2 breaks if we pass it a DT that includes the PXB instances.
* Need to look closer at the interrupt mapping - or potentially make this
  MSI/MSIX only. Equivalent is broken on x86 / q35 :)

Signed-off-by: Jonathan Cameron 
---
 hw/arm/virt.c| 214 +++
 hw/pci/pci.c |  21 +
 include/hw/pci/pci.h |   1 +
 3 files changed, 236 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef..4648437bba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -55,6 +55,8 @@
 #include "qemu/bitops.

[PULL v2 10/10] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions

[Taylor Simpson]

Most of these are not modelled in QEMU, so save the overhead of
calling a helper.

The only exception is dczeroa.  It assigns to hex_dczero_addr, which
is handled during packet commit.

Signed-off-by: Taylor Simpson 
Reviewed-by: Richard Henderson 
Message-Id: <20230410202402.2856852-1-tsimp...@quicinc.com>
---
 target/hexagon/gen_tcg.h | 24 
 target/hexagon/macros.h  | 18 --
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index b189f725d7..329e7a1024 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -487,6 +487,19 @@
 #define fGEN_TCG_S2_storerinew_pcr(SHORTCODE) \
 fGEN_TCG_STORE_pcr(2, fSTORE(1, 4, EA, NtN))
 
+/* dczeroa clears the 32 byte cache line at the address given */
+#define fGEN_TCG_Y2_dczeroa(SHORTCODE) SHORTCODE
+
+/* In linux-user mode, these are not modelled, suppress compiler warning */
+#define fGEN_TCG_Y2_dcinva(SHORTCODE) \
+do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_dccleaninva(SHORTCODE) \
+do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_dccleana(SHORTCODE) \
+do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_icinva(SHORTCODE) \
+do { RsV = RsV; } while (0)
+
 /*
  * dealloc_return
  * Assembler mapped to
@@ -1211,6 +1224,17 @@
 do { \
 RsV = RsV; \
 } while (0)
+#define fGEN_TCG_Y2_isync(SHORTCODE) \
+do { } while (0)
+#define fGEN_TCG_Y2_barrier(SHORTCODE) \
+do { } while (0)
+#define fGEN_TCG_Y2_syncht(SHORTCODE) \
+do { } while (0)
+#define fGEN_TCG_Y2_dcfetchbo(SHORTCODE) \
+do { \
+RsV = RsV; \
+uiV = uiV; \
+} while (0)
 
 #define fGEN_TCG_J2_trap0(SHORTCODE) \
 do { \
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 9ddfc91b1d..3e162de3a7 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -659,20 +659,10 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, 
int shift)
 fEXTRACTU_BITS(env->gpr[HEX_REG_##REG], \
reg_field_info[FIELD].width, \
reg_field_info[FIELD].offset)
-#define fBARRIER()
-#define fSYNCH()
-#define fISYNC()
-#define fDCFETCH(REG) \
-do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fICINVA(REG) \
-do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fL2FETCH(ADDR, HEIGHT, WIDTH, STRIDE, FLAGS)
-#define fDCCLEANA(REG) \
-do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fDCCLEANINVA(REG) \
-do { (void)REG; } while (0) /* Nothing to do in qemu */
-
-#define fDCZEROA(REG) do { env->dczero_addr = (REG); } while (0)
+
+#ifdef QEMU_GENERATE
+#define fDCZEROA(REG) tcg_gen_mov_tl(hex_dczero_addr, (REG))
+#endif
 
 #define fBRANCH_SPECULATE_STALL(DOTNEWVAL, JUMP_COND, SPEC_DIR, HINTBITNUM, \
 STRBITNUM) /* Nothing */
-- 
2.25.1

[PULL v2 09/10] Hexagon (target/hexagon) Remove unused slot variable in helpers

[Taylor Simpson]

The slot variable in helpers was only passed to log_reg_write function
where the argument is unused.
- Remove declaration from generated helper functions
- Remove slot argument from log_reg_write

Signed-off-by: Taylor Simpson 
Reviewed-by: Richard Henderson 
Message-Id: <20230407204521.357244-1-tsimp...@quicinc.com>
---
 target/hexagon/macros.h| 14 +++---
 target/hexagon/op_helper.h |  2 +-
 target/hexagon/op_helper.c |  2 +-
 target/hexagon/gen_helper_funcs.py |  2 --
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 21b5b5a06c..9ddfc91b1d 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -347,9 +347,9 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int 
shift)
 
 #define fREAD_LR() (env->gpr[HEX_REG_LR])
 
-#define fWRITE_LR(A) log_reg_write(env, HEX_REG_LR, A, slot)
-#define fWRITE_FP(A) log_reg_write(env, HEX_REG_FP, A, slot)
-#define fWRITE_SP(A) log_reg_write(env, HEX_REG_SP, A, slot)
+#define fWRITE_LR(A) log_reg_write(env, HEX_REG_LR, A)
+#define fWRITE_FP(A) log_reg_write(env, HEX_REG_FP, A)
+#define fWRITE_SP(A) log_reg_write(env, HEX_REG_SP, A)
 
 #define fREAD_SP() (env->gpr[HEX_REG_SP])
 #define fREAD_LC0 (env->gpr[HEX_REG_LC0])
@@ -377,13 +377,13 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, 
int shift)
 #define fHINTJR(TARGET) { /* Not modelled in qemu */}
 #define fWRITE_LOOP_REGS0(START, COUNT) \
 do { \
-log_reg_write(env, HEX_REG_LC0, COUNT, slot);  \
-log_reg_write(env, HEX_REG_SA0, START, slot); \
+log_reg_write(env, HEX_REG_LC0, COUNT);  \
+log_reg_write(env, HEX_REG_SA0, START); \
 } while (0)
 #define fWRITE_LOOP_REGS1(START, COUNT) \
 do { \
-log_reg_write(env, HEX_REG_LC1, COUNT, slot);  \
-log_reg_write(env, HEX_REG_SA1, START, slot);\
+log_reg_write(env, HEX_REG_LC1, COUNT);  \
+log_reg_write(env, HEX_REG_SA1, START);\
 } while (0)
 
 #define fSET_OVERFLOW() SET_USR_FIELD(USR_OVF, 1)
diff --git a/target/hexagon/op_helper.h b/target/hexagon/op_helper.h
index 34b3a53975..db22b54401 100644
--- a/target/hexagon/op_helper.h
+++ b/target/hexagon/op_helper.h
@@ -27,7 +27,7 @@ uint32_t mem_load4(CPUHexagonState *env, uint32_t slot, 
target_ulong vaddr);
 uint64_t mem_load8(CPUHexagonState *env, uint32_t slot, target_ulong vaddr);
 
 void log_reg_write(CPUHexagonState *env, int rnum,
-   target_ulong val, uint32_t slot);
+   target_ulong val);
 void log_store64(CPUHexagonState *env, target_ulong addr,
  int64_t val, int width, int slot);
 void log_store32(CPUHexagonState *env, target_ulong addr,
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 099c111a8c..3cc71b69d9 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -53,7 +53,7 @@ G_NORETURN void HELPER(raise_exception)(CPUHexagonState *env, 
uint32_t excp)
 }
 
 void log_reg_write(CPUHexagonState *env, int rnum,
-   target_ulong val, uint32_t slot)
+   target_ulong val)
 {
 HEX_DEBUG_LOG("log_reg_write[%d] = " TARGET_FMT_ld " (0x" TARGET_FMT_lx 
")",
   rnum, val, val);
diff --git a/target/hexagon/gen_helper_funcs.py 
b/target/hexagon/gen_helper_funcs.py
index c4e04508f8..c73d792580 100755
--- a/target/hexagon/gen_helper_funcs.py
+++ b/target/hexagon/gen_helper_funcs.py
@@ -308,8 +308,6 @@ def gen_helper_function(f, tag, tagregs, tagimms):
 f.write(", ")
 f.write("uint32_t part1")
 f.write(")\n{\n")
-if not hex_common.need_slot(tag):
-f.write("uint32_t slot __attribute__((unused)) = 4;\n")
 if hex_common.need_ea(tag):
 gen_decl_ea(f)
 ## Declare the return variable
-- 
2.25.1

[PULL v2 06/10] Hexagon (target/hexagon) Add overrides for count trailing zeros/ones

[Taylor Simpson]

The following instructions are overriden
S2_ct0Count trailing zeros
S2_ct1Count trailing ones
S2_ct0p   Count trailing zeros (register pair)
S2_ct1p   Count trailing ones (register pair)

These instructions are not handled by idef-parser because the
imported semantics uses bit-reverse.  However, they are
straightforward to implement in TCG with tcg_gen_ctzi_*

Test cases added to tests/tcg/hexagon/misc.c

Signed-off-by: Taylor Simpson 
Reviewed-by: Richard Henderson 
Message-Id: <20230405164211.30015-1-tsimp...@quicinc.com>
---
 target/hexagon/gen_tcg.h | 24 +
 tests/tcg/hexagon/misc.c | 56 +++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index bcf0cf466a..45f92adf6c 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -1058,6 +1058,30 @@
 #define fGEN_TCG_SL2_jumpr31_fnew(SHORTCODE) \
 gen_cond_jumpr31(ctx, TCG_COND_NE, hex_new_pred_value[0])
 
+/* Count trailing zeros/ones */
+#define fGEN_TCG_S2_ct0(SHORTCODE) \
+do { \
+tcg_gen_ctzi_tl(RdV, RsV, 32); \
+} while (0)
+#define fGEN_TCG_S2_ct1(SHORTCODE) \
+do { \
+tcg_gen_not_tl(RdV, RsV); \
+tcg_gen_ctzi_tl(RdV, RdV, 32); \
+} while (0)
+#define fGEN_TCG_S2_ct0p(SHORTCODE) \
+do { \
+TCGv_i64 tmp = tcg_temp_new_i64(); \
+tcg_gen_ctzi_i64(tmp, RssV, 64); \
+tcg_gen_extrl_i64_i32(RdV, tmp); \
+} while (0)
+#define fGEN_TCG_S2_ct1p(SHORTCODE) \
+do { \
+TCGv_i64 tmp = tcg_temp_new_i64(); \
+tcg_gen_not_i64(tmp, RssV); \
+tcg_gen_ctzi_i64(tmp, tmp, 64); \
+tcg_gen_extrl_i64_i32(RdV, tmp); \
+} while (0)
+
 /* Floating point */
 #define fGEN_TCG_F2_conv_sf2df(SHORTCODE) \
 gen_helper_conv_sf2df(RddV, cpu_env, RsV)
diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index e73ab57334..e126751e3a 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  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
@@ -21,6 +21,7 @@
 typedef unsigned char uint8_t;
 typedef unsigned short uint16_t;
 typedef unsigned int uint32_t;
+typedef unsigned long long uint64_t;
 
 
 static inline void S4_storerhnew_rr(void *p, int index, uint16_t v)
@@ -333,6 +334,57 @@ void test_l2fetch(void)
   "l2fetch(r0, r3:2)\n\t");
 }
 
+static inline int ct0(uint32_t x)
+{
+int res;
+asm("%0 = ct0(%1)\n\t" : "=r"(res) : "r"(x));
+return res;
+}
+
+static inline int ct1(uint32_t x)
+{
+int res;
+asm("%0 = ct1(%1)\n\t" : "=r"(res) : "r"(x));
+return res;
+}
+
+static inline int ct0p(uint64_t x)
+{
+int res;
+asm("%0 = ct0(%1)\n\t" : "=r"(res) : "r"(x));
+return res;
+}
+
+static inline int ct1p(uint64_t x)
+{
+int res;
+asm("%0 = ct1(%1)\n\t" : "=r"(res) : "r"(x));
+return res;
+}
+
+void test_count_trailing_zeros_ones(void)
+{
+check(ct0(0x000f), 0);
+check(ct0(0x), 32);
+check(ct0(0x00f0), 4);
+
+check(ct1(0x00f0), 0);
+check(ct1(0x000f), 4);
+check(ct1(0x), 0);
+check(ct1(0x), 32);
+
+check(ct0p(0x000fULL), 0);
+check(ct0p(0xULL), 64);
+check(ct0p(0x00f0ULL), 4);
+
+check(ct1p(0x00f0ULL), 0);
+check(ct1p(0x000fULL), 4);
+check(ct1p(0xULL), 0);
+check(ct1p(0xULL), 64);
+check(ct1p(0xff0fULL), 20);
+check(ct1p(0xff0fULL), 36);
+}
+
 int main()
 {
 int res;
@@ -468,6 +520,8 @@ int main()
 
 test_l2fetch();
 
+test_count_trailing_zeros_ones();
+
 puts(err ? "FAIL" : "PASS");
 return err;
 }
-- 
2.25.1

[PULL v2 02/10] Use f-strings in python scripts

[Taylor Simpson]

From: Marco Liebel 

Replace python 2 format string with f-strings

Signed-off-by: Marco Liebel 
Signed-off-by: Taylor Simpson 
Reviewed-by: Taylor Simpson 
Tested-by: Taylor Simpson 
Message-Id: <20230320092533.2859433-2-quic_mlie...@quicinc.com>
---
 target/hexagon/gen_analyze_funcs.py | 115 -
 target/hexagon/gen_helper_funcs.py  |  54 ++--
 target/hexagon/gen_helper_protos.py |  10 +-
 target/hexagon/gen_idef_parser_funcs.py |  10 +-
 target/hexagon/gen_op_attribs.py|   6 +-
 target/hexagon/gen_op_regs.py   |  12 +-
 target/hexagon/gen_opcodes_def.py   |   4 +-
 target/hexagon/gen_printinsn.py |  16 +-
 target/hexagon/gen_shortcode.py |   4 +-
 target/hexagon/gen_tcg_func_table.py|   4 +-
 target/hexagon/gen_tcg_funcs.py | 317 +++-
 target/hexagon/hex_common.py|   4 +-
 12 files changed, 250 insertions(+), 306 deletions(-)

diff --git a/target/hexagon/gen_analyze_funcs.py 
b/target/hexagon/gen_analyze_funcs.py
index ebd3e7afb9..1e246209e8 100755
--- a/target/hexagon/gen_analyze_funcs.py
+++ b/target/hexagon/gen_analyze_funcs.py
@@ -29,57 +29,49 @@ def is_predicated(tag):
 return 'A_CONDEXEC' in hex_common.attribdict[tag]
 
 def analyze_opn_old(f, tag, regtype, regid, regno):
-regN = "%s%sN" % (regtype, regid)
+regN = f"{regtype}{regid}N"
 predicated = "true" if is_predicated(tag) else "false"
 if (regtype == "R"):
 if (regid in {"ss", "tt"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"dd", "ee", "xx", "yy"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
 elif (regid in {"s", "t", "u", "v"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"d", "e", "x", "y"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_reg_write(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "P"):
 if (regid in {"s", "t", "u", "v"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"d", "e", "x"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_pred_write(ctx, %s);\n" % (regN))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_pred_write(ctx, {regN});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "C"):
 if (regid == "ss"):
-f.write("//const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
 elif (regid == "dd"):
-f.write("const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
-f.write("ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
+f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
 elif (regid == "s"):
-f.write("//const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
 elif (regid == "d"):
-f.write("const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
-f.write("ctx_log_reg_write(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
+f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "M"):
 if (regid == "u"):
-f.write("//const int %s = insn->regno[%d];\n"% \
-   

[PULL v2 01/10] Hexagon (translate.c): avoid redundant PC updates on COF

[Taylor Simpson]

From: Matheus Tavares Bernardino 

When there is a conditional change of flow or an endloop instruction, we
preload HEX_REG_PC with ctx->next_PC at gen_start_packet(). Nonetheless,
we still generate TCG code to do this update again at gen_goto_tb() when
the condition for the COF is not met, thus producing redundant
instructions. This can be seen with the following packet:

 0x004002e4:  0x5c20d000 {   if (!P0) jump:t PC+0 }

Which generates this TCG code:

    004002e4
-> mov_i32 pc,$0x4002e8
   and_i32 loc9,p0,$0x1
   mov_i32 branch_taken,loc9
   add_i32 pkt_cnt,pkt_cnt,$0x2
   add_i32 insn_cnt,insn_cnt,$0x2
   brcond_i32 branch_taken,$0x0,ne,$L1
   goto_tb $0x0
   mov_i32 pc,$0x4002e4
   exit_tb $0x7fb0c36e5200
   set_label $L1
   goto_tb $0x1
-> mov_i32 pc,$0x4002e8
   exit_tb $0x7fb0c36e5201
   set_label $L0
   exit_tb $0x7fb0c36e5203

Note that even after optimizations, the redundant PC update is still
present:

    004002e4
-> mov_i32 pc,$0x4002e8 sync: 0  dead: 0 1  pref=0x
   mov_i32 branch_taken,$0x1sync: 0  dead: 0 1  pref=0x
   add_i32 pkt_cnt,pkt_cnt,$0x2 sync: 0  dead: 0 1  pref=0x
   add_i32 insn_cnt,insn_cnt,$0x2   sync: 0  dead: 0 1 2  pref=0x
   goto_tb $0x1
-> mov_i32 pc,$0x4002e8 sync: 0  dead: 0 1  pref=0x
   exit_tb $0x7fb0c36e5201
   set_label $L0
   exit_tb $0x7fb0c36e5203

With this patch, the second redundant update is properly discarded.

Note that we need the additional "move_to_pc" flag instead of just
avoiding the update whenever `dest == ctx->next_PC`, as that could
potentially skip updates from a COF with met condition, whose
ctx->branch_dest just happens to be equal to ctx->next_PC.

Signed-off-by: Matheus Tavares Bernardino 
Signed-off-by: Taylor Simpson 
Reviewed-by: Anton Johansson 
Reviewed-by: Taylor Simpson 
Message-Id: 

---
 target/hexagon/translate.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 665476ab48..58d638f734 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -128,14 +128,19 @@ static bool use_goto_tb(DisasContext *ctx, target_ulong 
dest)
 return translator_use_goto_tb(&ctx->base, dest);
 }
 
-static void gen_goto_tb(DisasContext *ctx, int idx, target_ulong dest)
+static void gen_goto_tb(DisasContext *ctx, int idx, target_ulong dest, bool
+move_to_pc)
 {
 if (use_goto_tb(ctx, dest)) {
 tcg_gen_goto_tb(idx);
-tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], dest);
+if (move_to_pc) {
+tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], dest);
+}
 tcg_gen_exit_tb(ctx->base.tb, idx);
 } else {
-tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], dest);
+if (move_to_pc) {
+tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], dest);
+}
 tcg_gen_lookup_and_goto_ptr();
 }
 }
@@ -150,11 +155,11 @@ static void gen_end_tb(DisasContext *ctx)
 if (ctx->branch_cond != TCG_COND_ALWAYS) {
 TCGLabel *skip = gen_new_label();
 tcg_gen_brcondi_tl(ctx->branch_cond, hex_branch_taken, 0, skip);
-gen_goto_tb(ctx, 0, ctx->branch_dest);
+gen_goto_tb(ctx, 0, ctx->branch_dest, true);
 gen_set_label(skip);
-gen_goto_tb(ctx, 1, ctx->next_PC);
+gen_goto_tb(ctx, 1, ctx->next_PC, false);
 } else {
-gen_goto_tb(ctx, 0, ctx->branch_dest);
+gen_goto_tb(ctx, 0, ctx->branch_dest, true);
 }
 } else if (ctx->is_tight_loop &&
pkt->insn[pkt->num_insns - 1].opcode == J2_endloop0) {
@@ -165,9 +170,9 @@ static void gen_end_tb(DisasContext *ctx)
 TCGLabel *skip = gen_new_label();
 tcg_gen_brcondi_tl(TCG_COND_LEU, hex_gpr[HEX_REG_LC0], 1, skip);
 tcg_gen_subi_tl(hex_gpr[HEX_REG_LC0], hex_gpr[HEX_REG_LC0], 1);
-gen_goto_tb(ctx, 0, ctx->base.tb->pc);
+gen_goto_tb(ctx, 0, ctx->base.tb->pc, true);
 gen_set_label(skip);
-gen_goto_tb(ctx, 1, ctx->next_PC);
+gen_goto_tb(ctx, 1, ctx->next_PC, false);
 } else {
 tcg_gen_lookup_and_goto_ptr();
 }
-- 
2.25.1

[PULL v2 04/10] Hexagon (target/hexagon) Remove redundant/unused macros

[Taylor Simpson]

Remove the following macros (remnants of the old generator design)
READ_REG
READ_PREG
WRITE_RREG
WRITE_PREG
Modify macros that rely on the above

The following are unused
READ_IREG
fGET_FIELD
fSET_FIELD
fREAD_P3
fREAD_NPC
fWRITE_LC0
fWRITE_LC1

Signed-off-by: Taylor Simpson 
Reviewed-by: Richard Henderson 
Message-Id: <20230405183048.147767-1-tsimp...@quicinc.com>
---
 target/hexagon/macros.h | 65 ++---
 1 file changed, 22 insertions(+), 43 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 482a9c787f..f5f31b6930 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -22,16 +22,6 @@
 #include "hex_regs.h"
 #include "reg_fields.h"
 
-#ifdef QEMU_GENERATE
-#define READ_REG(dest, NUM)  gen_read_reg(dest, NUM)
-#else
-#define READ_REG(NUM)(env->gpr[(NUM)])
-#define READ_PREG(NUM)   (env->pred[NUM])
-
-#define WRITE_RREG(NUM, VAL) log_reg_write(env, NUM, VAL, slot)
-#define WRITE_PREG(NUM, VAL) log_pred_write(env, NUM, VAL)
-#endif
-
 #define PCALIGN 4
 #define PCALIGN_MASK (PCALIGN - 1)
 
@@ -361,37 +351,30 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, 
int shift)
 tcg_gen_shli_tl(result, result, shift);
 return result;
 }
-#define fREAD_IREG(VAL, SHIFT) gen_read_ireg(ireg, (VAL), (SHIFT))
-#else
-#define fREAD_IREG(VAL) \
-(fSXTN(11, 64, (((VAL) & 0xf000) >> 21) | ((VAL >> 17) & 0x7f)))
 #endif
 
-#define fREAD_LR() (READ_REG(HEX_REG_LR))
+#define fREAD_LR() (env->gpr[HEX_REG_LR])
 
-#define fWRITE_LR(A) WRITE_RREG(HEX_REG_LR, A)
-#define fWRITE_FP(A) WRITE_RREG(HEX_REG_FP, A)
-#define fWRITE_SP(A) WRITE_RREG(HEX_REG_SP, A)
+#define fWRITE_LR(A) log_reg_write(env, HEX_REG_LR, A, slot)
+#define fWRITE_FP(A) log_reg_write(env, HEX_REG_FP, A, slot)
+#define fWRITE_SP(A) log_reg_write(env, HEX_REG_SP, A, slot)
 
-#define fREAD_SP() (READ_REG(HEX_REG_SP))
-#define fREAD_LC0 (READ_REG(HEX_REG_LC0))
-#define fREAD_LC1 (READ_REG(HEX_REG_LC1))
-#define fREAD_SA0 (READ_REG(HEX_REG_SA0))
-#define fREAD_SA1 (READ_REG(HEX_REG_SA1))
-#define fREAD_FP() (READ_REG(HEX_REG_FP))
+#define fREAD_SP() (env->gpr[HEX_REG_SP])
+#define fREAD_LC0 (env->gpr[HEX_REG_LC0])
+#define fREAD_LC1 (env->gpr[HEX_REG_LC1])
+#define fREAD_SA0 (env->gpr[HEX_REG_SA0])
+#define fREAD_SA1 (env->gpr[HEX_REG_SA1])
+#define fREAD_FP() (env->gpr[HEX_REG_FP])
 #ifdef FIXME
 /* Figure out how to get insn->extension_valid to helper */
 #define fREAD_GP() \
-(insn->extension_valid ? 0 : READ_REG(HEX_REG_GP))
+(insn->extension_valid ? 0 : env->gpr[HEX_REG_GP])
 #else
-#define fREAD_GP() READ_REG(HEX_REG_GP)
+#define fREAD_GP() (env->gpr[HEX_REG_GP])
 #endif
 #define fREAD_PC() (PC)
 
-#define fREAD_NPC() (next_PC & (0xfffe))
-
-#define fREAD_P0() (READ_PREG(0))
-#define fREAD_P3() (READ_PREG(3))
+#define fREAD_P0() (env->pred[0])
 
 #define fCHECK_PCALIGN(A)
 
@@ -402,24 +385,22 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, 
int shift)
 #define fHINTJR(TARGET) { /* Not modelled in qemu */}
 #define fWRITE_LOOP_REGS0(START, COUNT) \
 do { \
-WRITE_RREG(HEX_REG_LC0, COUNT);  \
-WRITE_RREG(HEX_REG_SA0, START); \
+log_reg_write(env, HEX_REG_LC0, COUNT, slot);  \
+log_reg_write(env, HEX_REG_SA0, START, slot); \
 } while (0)
 #define fWRITE_LOOP_REGS1(START, COUNT) \
 do { \
-WRITE_RREG(HEX_REG_LC1, COUNT);  \
-WRITE_RREG(HEX_REG_SA1, START);\
+log_reg_write(env, HEX_REG_LC1, COUNT, slot);  \
+log_reg_write(env, HEX_REG_SA1, START, slot);\
 } while (0)
-#define fWRITE_LC0(VAL) WRITE_RREG(HEX_REG_LC0, VAL)
-#define fWRITE_LC1(VAL) WRITE_RREG(HEX_REG_LC1, VAL)
 
 #define fSET_OVERFLOW() SET_USR_FIELD(USR_OVF, 1)
 #define fSET_LPCFG(VAL) SET_USR_FIELD(USR_LPCFG, (VAL))
 #define fGET_LPCFG (GET_USR_FIELD(USR_LPCFG))
-#define fWRITE_P0(VAL) WRITE_PREG(0, VAL)
-#define fWRITE_P1(VAL) WRITE_PREG(1, VAL)
-#define fWRITE_P2(VAL) WRITE_PREG(2, VAL)
-#define fWRITE_P3(VAL) WRITE_PREG(3, VAL)
+#define fWRITE_P0(VAL) log_pred_write(env, 0, VAL)
+#define fWRITE_P1(VAL) log_pred_write(env, 1, VAL)
+#define fWRITE_P2(VAL) log_pred_write(env, 2, VAL)
+#define fWRITE_P3(VAL) log_pred_write(env, 3, VAL)
 #define fPART1(WORK) if (part1) { WORK; return; }
 #define fCAST4u(A) ((uint32_t)(A))
 #define fCAST4s(A) ((int32_t)(A))
@@ -576,7 +557,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int 
shift)
 
 #define fMEMOP(NUM, SIZE, SIGN, EA, FNTYPE, VALUE)
 
-#define fGET_FRAMEKEY() READ_REG(HEX_REG_FRAMEKEY)
+#define fGET_FRAMEKEY() (env->gpr[HEX_REG_FRAMEKEY])
 #define fFRAME_SCRAMBLE(VAL) ((VAL) ^ (fCAST8u(fGET_FRAMEKEY()) << 32))
 #define fFRAME_UNSCRAMBLE(VAL) fFRAME_SCRAMBLE(VAL)
 
@@ -686,8 +667,6 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int 
shift)
 fEXTRACTU_BITS(env->gpr[HEX_REG_##REG], \
reg_field_

[PULL v2 05/10] Hexagon (target/hexagon) Merge arguments to probe_pkt_scalar_hvx_stores

[Taylor Simpson]

Reducing the number of arguments reduces the overhead of the helper
call

Signed-off-by: Taylor Simpson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230405164211.30015-2-tsimp...@quicinc.com>
---
 target/hexagon/helper.h|  4 ++--
 target/hexagon/translate.h |  1 +
 target/hexagon/op_helper.c |  4 ++--
 target/hexagon/translate.c | 10 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/target/hexagon/helper.h b/target/hexagon/helper.h
index 368f0b5708..ed7f9842f6 100644
--- a/target/hexagon/helper.h
+++ b/target/hexagon/helper.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *  Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
  *
  *  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
@@ -107,4 +107,4 @@ DEF_HELPER_2(vwhist128qm, void, env, s32)
 DEF_HELPER_4(probe_noshuf_load, void, env, i32, int, int)
 DEF_HELPER_2(probe_pkt_scalar_store_s0, void, env, int)
 DEF_HELPER_2(probe_hvx_stores, void, env, int)
-DEF_HELPER_3(probe_pkt_scalar_hvx_stores, void, env, int, int)
+DEF_HELPER_2(probe_pkt_scalar_hvx_stores, void, env, int)
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index db832b0f88..4b9f21c41d 100644
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -178,5 +178,6 @@ FIELD(PROBE_PKT_SCALAR_HVX_STORES, HAS_ST1,1, 1)
 FIELD(PROBE_PKT_SCALAR_HVX_STORES, HAS_HVX_STORES, 2, 1)
 FIELD(PROBE_PKT_SCALAR_HVX_STORES, S0_IS_PRED, 3, 1)
 FIELD(PROBE_PKT_SCALAR_HVX_STORES, S1_IS_PRED, 4, 1)
+FIELD(PROBE_PKT_SCALAR_HVX_STORES, MMU_IDX,5, 2)
 
 #endif
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index c9a156030e..099c111a8c 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -488,8 +488,7 @@ void HELPER(probe_hvx_stores)(CPUHexagonState *env, int 
mmu_idx)
 }
 }
 
-void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState *env, int mask,
- int mmu_idx)
+void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState *env, int mask)
 {
 bool has_st0 = FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, HAS_ST0);
 bool has_st1 = FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, HAS_ST1);
@@ -497,6 +496,7 @@ void HELPER(probe_pkt_scalar_hvx_stores)(CPUHexagonState 
*env, int mask,
 FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, HAS_HVX_STORES);
 bool s0_is_pred = FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, 
S0_IS_PRED);
 bool s1_is_pred = FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, 
S1_IS_PRED);
+int mmu_idx = FIELD_EX32(mask, PROBE_PKT_SCALAR_HVX_STORES, MMU_IDX);
 
 if (has_st0) {
 probe_store(env, 0, mmu_idx, s0_is_pred);
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 58d638f734..c087f183d0 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -808,13 +808,11 @@ static void gen_commit_packet(DisasContext *ctx)
 g_assert(!has_store_s1 && !has_hvx_store);
 process_dczeroa(ctx);
 } else if (has_hvx_store) {
-TCGv mem_idx = tcg_constant_tl(ctx->mem_idx);
-
 if (!has_store_s0 && !has_store_s1) {
+TCGv mem_idx = tcg_constant_tl(ctx->mem_idx);
 gen_helper_probe_hvx_stores(cpu_env, mem_idx);
 } else {
 int mask = 0;
-TCGv mask_tcgv;
 
 if (has_store_s0) {
 mask =
@@ -839,8 +837,10 @@ static void gen_commit_packet(DisasContext *ctx)
 FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES,
S1_IS_PRED, 1);
 }
-mask_tcgv = tcg_constant_tl(mask);
-gen_helper_probe_pkt_scalar_hvx_stores(cpu_env, mask_tcgv, 
mem_idx);
+mask = FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES, MMU_IDX,
+  ctx->mem_idx);
+gen_helper_probe_pkt_scalar_hvx_stores(cpu_env,
+   tcg_constant_tl(mask));
 }
 } else if (has_store_s0 && has_store_s1) {
 /*
-- 
2.25.1

[PATCH v3 09/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

accept(2) syscall.

Add the accept(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 33 +
 1 file changed, 33 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index f191f22d63..f748266730 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -79,4 +79,37 @@ static inline abi_long do_bsd_connect(int sockfd, abi_ulong 
target_addr,
 return get_errno(connect(sockfd, addr, addrlen));
 }
 
+/* accept(2) */
+static inline abi_long do_bsd_accept(int fd, abi_ulong target_addr,
+ abi_ulong target_addrlen_addr)
+{
+socklen_t addrlen;
+void *addr;
+abi_long ret;
+
+if (target_addr == 0) {
+return get_errno(accept(fd, NULL, NULL));
+}
+/* return EINVAL if addrlen pointer is invalid */
+if (get_user_u32(addrlen, target_addrlen_addr)) {
+return -TARGET_EINVAL;
+}
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+return -TARGET_EINVAL;
+}
+addr = alloca(addrlen);
+
+ret = get_errno(accept(fd, addr, &addrlen));
+if (!is_error(ret)) {
+host_to_target_sockaddr(target_addr, addr, addrlen);
+if (put_user_u32(addrlen, target_addrlen_addr)) {
+ret = -TARGET_EFAULT;
+}
+}
+return ret;
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v3 02/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Add the socket conversion related flags and structs.

Add the relevant definitions of struct target_sockaddr and struct
target_ip_mreq and the related flags, to be used in
bsd-user/bsd-socket.c for the socket conversion functions:
target_to_host_sockaddr, host_to_target_sockaddr, target_to_host_ip_mreq

Signed-off-by: Karim Taha 
---
 bsd-user/syscall_defs.h | 110 
 1 file changed, 110 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index b6d113d24a..f041245792 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -179,6 +179,116 @@ struct target_freebsd__wrusage {
 struct target_freebsd_rusage wru_children;
 };
 
+/*
+ * sys/socket.h
+ */
+
+/*
+ * Types
+ */
+#define TARGET_SOCK_STREAM  1   /* stream socket */
+#define TARGET_SOCK_DGRAM   2   /* datagram socket */
+#define TARGET_SOCK_RAW 3   /* raw-protocol interface */
+#define TARGET_SOCK_RDM 4   /* reliably-delivered message */
+#define TARGET_SOCK_SEQPACKET   5   /* sequenced packet stream */
+
+
+/*
+ * Option flags per-socket.
+ */
+
+#define TARGET_SO_DEBUG 0x0001  /* turn on debugging info recording */
+#define TARGET_SO_ACCEPTCONN0x0002  /* socket has had listen() */
+#define TARGET_SO_REUSEADDR 0x0004  /* allow local address reuse */
+#define TARGET_SO_KEEPALIVE 0x0008  /* keep connections alive */
+#define TARGET_SO_DONTROUTE 0x0010  /* just use interface addresses */
+#define TARGET_SO_BROADCAST 0x0020  /* permit sending of broadcast msgs */
+#define TARGET_SO_USELOOPBACK   0x0040  /* bypass hardware when possible */
+#define TARGET_SO_LINGER0x0080  /* linger on close if data present */
+#define TARGET_SO_OOBINLINE 0x0100  /* leave received OOB data in line */
+#define TARGET_SO_REUSEPORT 0x0200  /* allow local address & port reuse */
+#define TARGET_SO_TIMESTAMP 0x0400  /* timestamp received dgram traffic */
+#define TARGET_SO_NOSIGPIPE 0x0800  /* no SIGPIPE from EPIPE */
+#define TARGET_SO_ACCEPTFILTER  0x1000  /* there is an accept filter */
+#define TARGET_SO_BINTIME   0x2000  /* timestamp received dgram traffic */
+#define TARGET_SO_NO_OFFLOAD0x4000  /* socket cannot be offloaded */
+#define TARGET_SO_NO_DDP0x8000  /* disable direct data placement */
+
+/*
+ * Additional options, not kept in so_options.
+ */
+#define TARGET_SO_SNDBUF0x1001  /* send buffer size */
+#define TARGET_SO_RCVBUF0x1002  /* receive buffer size */
+#define TARGET_SO_SNDLOWAT  0x1003  /* send low-water mark */
+#define TARGET_SO_RCVLOWAT  0x1004  /* receive low-water mark */
+#define TARGET_SO_SNDTIMEO  0x1005  /* send timeout */
+#define TARGET_SO_RCVTIMEO  0x1006  /* receive timeout */
+#define TARGET_SO_ERROR 0x1007  /* get error status and clear */
+#define TARGET_SO_TYPE  0x1008  /* get socket type */
+#define TARGET_SO_LABEL 0x1009  /* socket's MAC label */
+#define TARGET_SO_PEERLABEL 0x1010  /* socket's peer's MAC label */
+#define TARGET_SO_LISTENQLIMIT  0x1011  /* socket's backlog limit */
+#define TARGET_SO_LISTENQLEN0x1012  /* socket's complete queue length */
+#define TARGET_SO_LISTENINCQLEN 0x1013  /* socket's incomplete queue length */
+#define TARGET_SO_SETFIB0x1014  /* use this FIB to route */
+#define TARGET_SO_USER_COOKIE   0x1015  /* user cookie (dummynet etc.) */
+#define TARGET_SO_PROTOCOL  0x1016  /* get socket protocol (Linux name) */
+
+/* alias for SO_PROTOCOL (SunOS name) */
+#define TARGET_SO_PROTOTYPE TARGET_SO_PROTOCOL
+
+/*
+ * Level number for (get/set)sockopt() to apply to socket itself.
+ */
+#define TARGET_SOL_SOCKET   0x  /* options for socket level */
+
+#ifndef CMSG_ALIGN
+#define CMSG_ALIGN(len) (((len) + sizeof(long) - 1) & ~(sizeof(long) - 1))
+#endif
+
+/*
+ * sys/socket.h
+ */
+struct target_msghdr {
+abi_longmsg_name;   /* Socket name */
+int32_t msg_namelen;/* Length of name */
+abi_longmsg_iov;/* Data blocks */
+int32_t msg_iovlen; /* Number of blocks */
+abi_longmsg_control;/* Per protocol magic (eg BSD fd passing) */
+int32_t msg_controllen; /* Length of cmsg list */
+int32_t msg_flags;  /* flags on received message */
+};
+
+struct target_sockaddr {
+uint8_t sa_len;
+uint8_t sa_family;
+uint8_t sa_data[14];
+} QEMU_PACKED;
+
+struct target_in_addr {
+uint32_t s_addr; /* big endian */
+};
+
+struct target_cmsghdr {
+uint32_tcmsg_len;
+int32_t cmsg_level;
+int32_t cmsg_type;
+};
+
+/*
+ * netinet/in.h
+ */
+struct target_ip_mreq {
+struct target_in_addr   imr_multiaddr;
+struct target_in_addr   imr_interface;
+};
+
+struct target_ip_mreqn {
+struct target_in_addr   imr_multiaddr;
+struct target_in_addr   imr_address;
+int32_t imr_ifindex;
+};
+
 #define safe_syscall0(type, name

[PATCH v3 04/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Declaration of the socket conversion functions.

Add bsd-user/qemu-bsd.h, required by bsd-user/bsd-socket.h, contains
forward declarations of the socket conversion functions defined in 
bsd-user/bsd-socket.c.

Signed-off-by: Karim Taha 
---
 bsd-user/qemu-bsd.h | 36 
 1 file changed, 36 insertions(+)
 create mode 100644 bsd-user/qemu-bsd.h

diff --git a/bsd-user/qemu-bsd.h b/bsd-user/qemu-bsd.h
new file mode 100644
index 00..a052688596
--- /dev/null
+++ b/bsd-user/qemu-bsd.h
@@ -0,0 +1,36 @@
+/*
+ *  BSD conversion extern declarations
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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 QEMU_BSD_H
+#define QEMU_BSD_H
+
+#include 
+#include 
+#include 
+#include 
+
+/* bsd-socket.c */
+abi_long target_to_host_sockaddr(struct sockaddr *addr, abi_ulong target_addr,
+socklen_t len);
+abi_long host_to_target_sockaddr(abi_ulong target_addr, struct sockaddr *addr,
+socklen_t len);
+abi_long target_to_host_ip_mreq(struct ip_mreqn *mreqn, abi_ulong target_addr,
+socklen_t len);
+
+#endif /* QEMU_BSD_H */
-- 
2.40.0

[PATCH v3 07/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

The implementation of bind(2) syscall and socket related syscalls.

Add bsd-user/bsd-socket.h, which contains the implementation of
bind(2), and the socket related system call shims.

Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 61 +++
 1 file changed, 61 insertions(+)
 create mode 100644 bsd-user/bsd-socket.h

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
new file mode 100644
index 00..7da4cf11a0
--- /dev/null
+++ b/bsd-user/bsd-socket.h
@@ -0,0 +1,61 @@
+/*
+ *  socket related system call shims
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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_SOCKET_H
+#define BSD_SOCKET_H
+
+#include 
+#include 
+#include 
+#include 
+
+#include "qemu-bsd.h"
+
+ssize_t safe_recvfrom(int s, void *buf, size_t len, int flags,
+struct sockaddr *restrict from, socklen_t *restrict fromlen);
+ssize_t safe_sendto(int s, const void *buf, size_t len, int flags,
+const struct sockaddr *to, socklen_t tolen);
+int safe_select(int nfds, fd_set *readfs, fd_set *writefds, fd_set *exceptfds,
+struct timeval *timeout);
+int safe_pselect(int nfds, fd_set *restrict readfds,
+fd_set *restrict writefds, fd_set *restrict exceptfds,
+const struct timespec *restrict timeout,
+const sigset_t *restrict newsigmask);
+
+/* bind(2) */
+static inline abi_long do_bsd_bind(int sockfd, abi_ulong target_addr,
+   socklen_t addrlen)
+{
+abi_long ret;
+void *addr;
+
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+
+addr = alloca(addrlen + 1);
+ret = target_to_host_sockaddr(addr, target_addr, addrlen);
+if (is_error(ret)) {
+return ret;
+}
+
+return get_errno(bind(sockfd, addr, addrlen));
+}
+
+#endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v3 05/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

Definitions of the socket conversion functions.

Add bsd-user/bsd-socket.c, which contains the actual definitions of the
socket conversion functions.

Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.c | 108 ++
 1 file changed, 108 insertions(+)
 create mode 100644 bsd-user/bsd-socket.c

diff --git a/bsd-user/bsd-socket.c b/bsd-user/bsd-socket.c
new file mode 100644
index 00..8a5e4d
--- /dev/null
+++ b/bsd-user/bsd-socket.c
@@ -0,0 +1,108 @@
+/*
+ *  BSD socket system call related helpers
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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 
+#include 
+#include 
+#include 
+
+#include "qemu.h"
+#include "qemu-bsd.h"
+
+/*
+ * socket conversion
+ */
+abi_long target_to_host_sockaddr(struct sockaddr *addr, abi_ulong target_addr,
+ socklen_t len)
+{
+const socklen_t unix_maxlen = sizeof(struct sockaddr_un);
+sa_family_t sa_family;
+struct target_sockaddr *target_saddr;
+
+target_saddr = lock_user(VERIFY_READ, target_addr, len, 1);
+if (target_saddr == 0) {
+return -TARGET_EFAULT;
+}
+
+sa_family = target_saddr->sa_family;
+
+/*
+ * Oops. The caller might send a incomplete sun_path; sun_path
+ * must be terminated by \0 (see the manual page), but unfortunately
+ * it is quite common to specify sockaddr_un length as
+ * "strlen(x->sun_path)" while it should be "strlen(...) + 1". We will
+ * fix that here if needed.
+ */
+if (target_saddr->sa_family == AF_UNIX) {
+if (len < unix_maxlen && len > 0) {
+char *cp = (char *)target_saddr;
+
+if (cp[len - 1] && !cp[len]) {
+len++;
+}
+}
+if (len > unix_maxlen) {
+len = unix_maxlen;
+}
+}
+
+memcpy(addr, target_saddr, len);
+addr->sa_family = sa_family;/* type uint8_t */
+addr->sa_len = target_saddr->sa_len;/* type uint8_t */
+unlock_user(target_saddr, target_addr, 0);
+
+return 0;
+}
+
+abi_long host_to_target_sockaddr(abi_ulong target_addr, struct sockaddr *addr,
+ socklen_t len)
+{
+struct target_sockaddr *target_saddr;
+
+target_saddr = lock_user(VERIFY_WRITE, target_addr, len, 0);
+if (target_saddr == 0) {
+return -TARGET_EFAULT;
+}
+memcpy(target_saddr, addr, len);
+target_saddr->sa_family = addr->sa_family;  /* type uint8_t */
+target_saddr->sa_len = addr->sa_len;/* type uint8_t */
+unlock_user(target_saddr, target_addr, len);
+
+return 0;
+}
+
+abi_long target_to_host_ip_mreq(struct ip_mreqn *mreqn, abi_ulong target_addr,
+socklen_t len)
+{
+struct target_ip_mreqn *target_smreqn;
+
+target_smreqn = lock_user(VERIFY_READ, target_addr, len, 1);
+if (target_smreqn == 0) {
+return -TARGET_EFAULT;
+}
+mreqn->imr_multiaddr.s_addr = target_smreqn->imr_multiaddr.s_addr;
+mreqn->imr_address.s_addr = target_smreqn->imr_address.s_addr;
+if (len == sizeof(struct target_ip_mreqn)) {
+mreqn->imr_ifindex = tswapal(target_smreqn->imr_ifindex);
+}
+unlock_user(target_smreqn, target_addr, 0);
+
+return 0;
+}
-- 
2.40.0

[PATCH v3 10/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

getpeername(2) syscall.

Add the getpeername(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index f748266730..16fae3752a 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -112,4 +112,32 @@ static inline abi_long do_bsd_accept(int fd, abi_ulong 
target_addr,
 return ret;
 }
 
+/* getpeername(2) */
+static inline abi_long do_bsd_getpeername(int fd, abi_ulong target_addr,
+  abi_ulong target_addrlen_addr)
+{
+socklen_t addrlen;
+void *addr;
+abi_long ret;
+
+if (get_user_u32(addrlen, target_addrlen_addr)) {
+return -TARGET_EFAULT;
+}
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+return -TARGET_EFAULT;
+}
+addr = alloca(addrlen);
+ret = get_errno(getpeername(fd, addr, &addrlen));
+if (!is_error(ret)) {
+host_to_target_sockaddr(target_addr, addr, addrlen);
+if (put_user_u32(addrlen, target_addrlen_addr)) {
+ret = -TARGET_EFAULT;
+}
+}
+return ret;
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0

[PATCH v3 11/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Warner Losh 

Add the dispatching code of bind(2),connect(2), accpet(2), getpeername(2).

Add the bind(2), connect(2), accept(2), getpeername(2) syscalls case
statements to freebsd_syscall function defined in bsd-user/freebsd/os-syscall.c

Signed-off-by: Karim Taha 
---
 bsd-user/freebsd/os-syscall.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index c8f998ecec..7f29196a05 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -44,6 +44,8 @@
 #include "signal-common.h"
 #include "user/syscall-trace.h"
 
+/* BSD independent syscall shims */
+#include "bsd-socket.h"
 #include "bsd-file.h"
 #include "bsd-proc.h"
 
@@ -508,6 +510,25 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
 break;
 
+/*
+ * socket related system calls
+ */
+case TARGET_FREEBSD_NR_accept: /* accept(2) */
+ret = do_bsd_accept(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_bind: /* bind(2) */
+ret = do_bsd_bind(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_connect: /* connect(2) */
+ret = do_bsd_connect(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_getpeername: /* getpeername(2) */
+ret = do_bsd_getpeername(arg1, arg2, arg3);
+break;
+
 default:
 qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
 ret = -TARGET_ENOSYS;
-- 
2.40.0

[PATCH v3 08/11] Signed-off-by: Karim Taha

[Karim Taha]

From: Stacey Son 

connect(2) syscall.

Add the connect(2) syscall to bsd-user/bsd-socket.h.

Signed-off-by: Karim Taha 
---
 bsd-user/bsd-socket.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/bsd-user/bsd-socket.h b/bsd-user/bsd-socket.h
index 7da4cf11a0..f191f22d63 100644
--- a/bsd-user/bsd-socket.h
+++ b/bsd-user/bsd-socket.h
@@ -58,4 +58,25 @@ static inline abi_long do_bsd_bind(int sockfd, abi_ulong 
target_addr,
 return get_errno(bind(sockfd, addr, addrlen));
 }
 
+/* connect(2) */
+static inline abi_long do_bsd_connect(int sockfd, abi_ulong target_addr,
+  socklen_t addrlen)
+{
+abi_long ret;
+void *addr;
+
+if ((int)addrlen < 0) {
+return -TARGET_EINVAL;
+}
+addr = alloca(addrlen + 1);
+
+ret = target_to_host_sockaddr(addr, target_addr, addrlen);
+
+if (is_error(ret)) {
+return ret;
+}
+
+return get_errno(connect(sockfd, addr, addrlen));
+}
+
 #endif /* BSD_SOCKET_H */
-- 
2.40.0