Re: [PATCH 0/1]

[Thomas Huth]

On 21/07/2021 00.13, Jose R. Ziviani wrote:

Hello!

This patch gives the ability to build TCG builtin even if
--enable-modules is selected. This is useful to have a base
QEMU with TCG native product but still using the benefits of
modules.


Could you please elaborate why this is required? Did you see a performance 
improvement? Or is there another problem?


 Thomas

[PATCH v3 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor

[Bin Meng]

From: Christina Wang 

In the legacy RX descriptor mode, VLAN tag was saved to d->special
by e1000e_build_rx_metadata() in e1000e_write_lgcy_rx_descr(), but
it was then zeroed out again at the end of the call, which is wrong.

Fixes: c89d416a2b0f ("e1000e: Don't zero out buffer address in rx descriptor")
Reported-by: Markus Carlstedt 
Signed-off-by: Christina Wang 
Signed-off-by: Bin Meng 

---

(no changes since v1)

 hw/net/e1000e_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index b4bf4ca2f1..8ae6fb7e14 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1285,7 +1285,6 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t 
*desc,
  >special);
 d->errors = (uint8_t) (le32_to_cpu(status_flags) >> 24);
 d->status = (uint8_t) le32_to_cpu(status_flags);
-d->special = 0;
 }
 
 static inline void
-- 
2.25.1

[PATCH v3 2/3] hw/net: e1000e: Correct the initial value of VET register

[Bin Meng]

From: Christina Wang 

The initial value of VLAN Ether Type (VET) register is 0x8100, as per
the manual and real hardware.

While Linux e1000e driver always writes VET register to 0x8100, it is
not always the case for everyone. Drivers relying on the reset value
of VET won't be able to transmit and receive VLAN frames in QEMU.

Unlike e1000 in QEMU, e1000e uses a field 'vet' in "struct E1000Core"
to cache the value of VET register, but the cache only gets updated
when VET register is written. To always get a consistent VET value
no matter VET is written or remains its reset value, drop the 'vet'
field and use 'core->mac[VET]' directly.

Reported-by: Markus Carlstedt 
Signed-off-by: Christina Wang 
Signed-off-by: Bin Meng 

---

Changes in v3:
- add a "init-vet" property for versioned machines

Changes in v2:
- keep the 'vet' field in "struct E1000Core" for migration compatibility

 hw/core/machine.c| 21 +
 hw/net/e1000e.c  |  8 +++-
 hw/net/e1000e_core.c |  9 -
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 29982c1ef1..8355df69c7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -42,6 +42,7 @@ GlobalProperty hw_compat_6_0[] = {
 { "i8042", "extended-state", "false"},
 { "nvme-ns", "eui64-default", "off"},
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
 
@@ -51,6 +52,7 @@ GlobalProperty hw_compat_5_2[] = {
 { "virtio-blk-device", "report-discard-granularity", "off" },
 { "virtio-net-pci", "vectors", "3"},
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
@@ -65,6 +67,7 @@ GlobalProperty hw_compat_5_1[] = {
 { "pl011", "migrate-clk", "off" },
 { "virtio-pci", "x-ats-page-aligned", "off"},
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
 
@@ -77,6 +80,7 @@ GlobalProperty hw_compat_5_0[] = {
 { "vmport", "x-cmds-v2", "off" },
 { "virtio-device", "x-disable-legacy-check", "true" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
 
@@ -94,12 +98,14 @@ GlobalProperty hw_compat_4_2[] = {
 { "fw_cfg", "acpi-mr-restore", "false" },
 { "virtio-device", "use-disabled-flag", "false" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
 
 GlobalProperty hw_compat_4_1[] = {
 { "virtio-pci", "x-pcie-flr-init", "off" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
 
@@ -113,6 +119,7 @@ GlobalProperty hw_compat_4_0[] = {
 { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
 { "pl031", "migrate-tick-offset", "false" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -131,11 +138,13 @@ GlobalProperty hw_compat_3_1[] = {
 { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
 { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
 GlobalProperty hw_compat_3_0[] = {
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
 
@@ -147,6 +156,7 @@ GlobalProperty hw_compat_2_12[] = {
 { "vmware-svga", "global-vmstate", "true" },
 { "qxl-vga", "global-vmstate", "true" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
 
@@ -156,6 +166,7 @@ GlobalProperty hw_compat_2_11[] = {
 { "vhost-user-blk-pci", "vectors", "2" },
 { "e1000", "migrate_tso_props", "off" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
 
@@ -163,6 +174,7 @@ GlobalProperty hw_compat_2_10[] = {
 { "virtio-mouse-device", "wheel-axis", "false" },
 { "virtio-tablet-device", "wheel-axis", "false" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
 
@@ -172,6 +184,7 @@ GlobalProperty hw_compat_2_9[] = {
 { "virtio-net-device", "x-mtu-bypass-backend", "off" },
 { "pcie-root-port", "x-migrate-msix", "false" },
 { "e1000", "init-vet", "off" },
+{ "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
 
@@ -187,6 +200,7 @@ 

[PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register

[Bin Meng]

From: Christina Wang 

The initial value of VLAN Ether Type (VET) register is 0x8100, as per
the manual and real hardware.

While Linux e1000 driver always writes VET register to 0x8100, it is
not always the case for everyone. Drivers relying on the reset value
of VET won't be able to transmit and receive VLAN frames in QEMU.

Reported-by: Markus Carlstedt 
Signed-off-by: Christina Wang 
Signed-off-by: Bin Meng 

---

Changes in v3:
- add a "init-vet" property for versioned machines

 hw/core/machine.c | 27 +--
 hw/net/e1000.c| 26 ++
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 775add0795..29982c1ef1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -41,6 +41,7 @@ GlobalProperty hw_compat_6_0[] = {
 { "gpex-pcihost", "allow-unmapped-accesses", "false" },
 { "i8042", "extended-state", "false"},
 { "nvme-ns", "eui64-default", "off"},
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
 
@@ -49,6 +50,7 @@ GlobalProperty hw_compat_5_2[] = {
 { "PIIX4_PM", "smm-compat", "on"},
 { "virtio-blk-device", "report-discard-granularity", "off" },
 { "virtio-net-pci", "vectors", "3"},
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
@@ -62,6 +64,7 @@ GlobalProperty hw_compat_5_1[] = {
 { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */
 { "pl011", "migrate-clk", "off" },
 { "virtio-pci", "x-ats-page-aligned", "off"},
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
 
@@ -73,6 +76,7 @@ GlobalProperty hw_compat_5_0[] = {
 { "vmport", "x-report-vmx-type", "off" },
 { "vmport", "x-cmds-v2", "off" },
 { "virtio-device", "x-disable-legacy-check", "true" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
 
@@ -89,11 +93,13 @@ GlobalProperty hw_compat_4_2[] = {
 { "qxl-vga", "revision", "4" },
 { "fw_cfg", "acpi-mr-restore", "false" },
 { "virtio-device", "use-disabled-flag", "false" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
 
 GlobalProperty hw_compat_4_1[] = {
 { "virtio-pci", "x-pcie-flr-init", "off" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
 
@@ -106,6 +112,7 @@ GlobalProperty hw_compat_4_0[] = {
 { "virtio-device", "use-started", "false" },
 { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
 { "pl031", "migrate-tick-offset", "false" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -123,10 +130,13 @@ GlobalProperty hw_compat_3_1[] = {
 { "virtio-blk-device", "write-zeroes", "false" },
 { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
 { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
-GlobalProperty hw_compat_3_0[] = {};
+GlobalProperty hw_compat_3_0[] = {
+{ "e1000", "init-vet", "off" },
+};
 const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
 
 GlobalProperty hw_compat_2_12[] = {
@@ -136,6 +146,7 @@ GlobalProperty hw_compat_2_12[] = {
 { "VGA", "global-vmstate", "true" },
 { "vmware-svga", "global-vmstate", "true" },
 { "qxl-vga", "global-vmstate", "true" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
 
@@ -144,12 +155,14 @@ GlobalProperty hw_compat_2_11[] = {
 { "virtio-blk-pci", "vectors", "2" },
 { "vhost-user-blk-pci", "vectors", "2" },
 { "e1000", "migrate_tso_props", "off" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
 
 GlobalProperty hw_compat_2_10[] = {
 { "virtio-mouse-device", "wheel-axis", "false" },
 { "virtio-tablet-device", "wheel-axis", "false" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
 
@@ -158,6 +171,7 @@ GlobalProperty hw_compat_2_9[] = {
 { "intel-iommu", "pt", "off" },
 { "virtio-net-device", "x-mtu-bypass-backend", "off" },
 { "pcie-root-port", "x-migrate-msix", "false" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
 
@@ -172,6 +186,7 @@ GlobalProperty hw_compat_2_8[] = {
 { "virtio-pci", "x-pcie-pm-init", "off" },
 { "cirrus-vga", "vgamem_mb", "8" },
 { "isa-cirrus-vga", "vgamem_mb", "8" },
+{ "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_8_len = G_N_ELEMENTS(hw_compat_2_8);
 
@@ -181,6 +196,7 @@ GlobalProperty hw_compat_2_7[] = {
 { "ioapic", "version", "0x11" },
 { "intel-iommu", 

RE: [PATCH 1/2] ui/gtk: detach_all option for making all VCs detached upon starting

[Romli, Khairul Anuar]

I've tried and this patch is able to detach all the virtual console after we 
launch the qemu. However, I think we need to filter out other terminal that are 
not related to view such as compatmonitor(), serial and parallel. 

Also, I think we can have the detach specific to virtio-pci view without the 
need to have new parameters.

> -Original Message-
> From: Kim, Dongwon 
> Sent: Wednesday, July 21, 2021 6:17 AM
> To: Thomas Huth 
> Cc: qemu-devel@nongnu.org; Romli, Khairul Anuar
> 
> Subject: Re: [PATCH 1/2] ui/gtk: detach_all option for making all VCs
> detached upon starting
> 
> On Tue, Jul 20, 2021 at 03:42:16PM +0200, Thomas Huth wrote:
> > On 19/07/2021 23.41, Dongwon Kim wrote:
> > > With "detach-all=on" for display, all VCs are detached from the
> beginning.
> > > This is useful when there are multiple displays assigned to a guest OS.
> >
> > Can you elaborate? (i.e. why is it useful? Do you just want to avoid
> > having multiple things opened at startup? Or is there a different
> > reason?)
> Hi,
> 
> The original motivation is related to an use-case with a guest with multi-
> displays. In that use case, we wanted to have all guest displays placed side 
> by
> side from beginning. Virtual consoles other than guest displays (e.g. virtio-
> gpu-pci) are not actually needed but I found doing "detach-all" is the 
> simplest
> way.
> 
> >
> > > Signed-off-by: Dongwon Kim 
> > > Signed-off-by: Khairul Anuar Romli 
> > > ---
> > >   qapi/ui.json | 4 +++-
> > >   ui/gtk.c | 7 +++
> > >   2 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/qapi/ui.json b/qapi/ui.json index
> > > 1052ca9c38..ff14bb2f46 100644
> > > --- a/qapi/ui.json
> > > +++ b/qapi/ui.json
> > > @@ -1141,6 +1141,7 @@
> > >   # @show-cursor:   Force showing the mouse cursor (default: off).
> > >   # (since: 5.0)
> > >   # @gl:Enable OpenGL support (default: off).
> > > +# @detach-all:Detatch all VirtualConsoles from beginning (default:
> off).
> >
> > Needs a comment à la "(since: 6.2)" at the end (like the one after
> > "show-cursor" some lines earlier.
> >
> > >   #
> > >   # Since: 2.12
> > >   #
> > > @@ -1150,7 +1151,8 @@
> > >   '*full-screen'   : 'bool',
> > >   '*window-close'  : 'bool',
> > >   '*show-cursor'   : 'bool',
> > > -'*gl': 'DisplayGLMode' },
> > > +'*gl': 'DisplayGLMode',
> > > +'*detach-all': 'bool' },
> >
> > If this is for GTK only, shouldn't this rather go into DisplayGTK instead?
> > Or will this be also useful for other display types later?
> 
> This option might not be that useful for other use cases.. but at the same
> time, I'm pretty sure this will work universally (won't break
> anything..) but for now, I think it's good idea to limit this to GTK.
> 
> -DW
> 
> >
> >  Thomas
> >
> >
> > > 'discriminator' : 'type',
> > > 'data': { 'gtk': 'DisplayGTK',
> > >   'curses' : 'DisplayCurses',
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index ce885d2ca3..a07e5a049e 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -2211,6 +2211,7 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
> > >   GdkDisplay *window_display;
> > >   GtkIconTheme *theme;
> > >   char *dir;
> > > +int i;
> > >   if (!gtkinit) {
> > >   fprintf(stderr, "gtk initialization failed\n"); @@ -2290,6
> > > +2291,12 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
> > >   gtk_menu_item_activate(GTK_MENU_ITEM(s-
> >grab_on_hover_item));
> > >   }
> > >   gd_clipboard_init(s);
> > > +
> > > +if (opts->detach_all) {

> > > +for (i = 0; i < s->nb_vcs - 1; i++) {

[Romli, Khairul Anuar]  We can a conditional check here to only detech 
virtio-pci view rather than "everything". Also, we may want to consider not to 
detach the primary view and keep it remain in qemu primary window.

> > > +gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
> > > +}
> > > +}
> > >   }
> > >   static void early_gtk_display_init(DisplayOptions *opts)
> > >
> >

[PATCH 1/2 v5] Configure script for Haiku

[Richard Zak]

Signed-off-by: Richard Zak 
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 232c54dcc1..163af793e9 100755
--- a/configure
+++ b/configure
@@ -768,7 +768,8 @@ SunOS)
 ;;
 Haiku)
   haiku="yes"
-  QEMU_CFLAGS="-DB_USE_POSITIVE_POSIX_ERRORS -D_BSD_SOURCE $QEMU_CFLAGS"
+  pie="no"
+  QEMU_CFLAGS="-DB_USE_POSITIVE_POSIX_ERRORS -D_BSD_SOURCE -fPIC
$QEMU_CFLAGS"
 ;;
 Linux)
   audio_drv_list="try-pa oss"
-- 
2.25.1

v5: Proper formatting for patch (sorry)
v4:
This refers to the email from a few weeks ago, regarding TPM & Haiku. It
seems the assertion failure isn't really about the TPM, but about disabling
PIE and adding -fPIC. There's discussion on the Haiku forum[1] about the
incompatibility with PIE, and this fixes the assertion failure without
altering the TPM configuration variable.
[1] https://discuss.haiku-os.org/t/qemu-on-haiku-sdl-issue/10961/6?u=rjzak

Previously, the TPM option was causing an assertion error at
util/async.c:669 qemu_set_current_aio_context() !my_aiocontext. I suspect
it was because the TPM option may have implied PIE. This patch ensures PIE
doesn't get used, but -fPIC is used instead.



-- 
Regards,

Richard J. Zak
Professional Genius
PGP Key: https://keybase.io/rjzak/key.asc

[PATCH 4/5] migration: Teach QEMUFile to be QIOChannel-aware

[Peter Xu]

migration uses QIOChannel typed qemufiles.  In follow up patches, we'll need
the capability to identify this fact, so that we can get the backing QIOChannel
from a QEMUFile.

We can also define types for QEMUFile but so far since we only need to be able
to identify QIOChannel, introduce a boolean which is simpler.

No functional change.

Signed-off-by: Peter Xu 
---
 migration/qemu-file-channel.c | 4 ++--
 migration/qemu-file.c | 5 -
 migration/qemu-file.h | 2 +-
 migration/ram.c   | 2 +-
 migration/savevm.c| 4 ++--
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 867a5ed0c3..2f8b1fcd46 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -187,11 +187,11 @@ static const QEMUFileOps channel_output_ops = {
 QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
 {
 object_ref(OBJECT(ioc));
-return qemu_fopen_ops(ioc, _input_ops);
+return qemu_fopen_ops(ioc, _input_ops, true);
 }
 
 QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
 {
 object_ref(OBJECT(ioc));
-return qemu_fopen_ops(ioc, _output_ops);
+return qemu_fopen_ops(ioc, _output_ops, true);
 }
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 1eacf9e831..ada58c94dd 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -55,6 +55,8 @@ struct QEMUFile {
 Error *last_error_obj;
 /* has the file has been shutdown */
 bool shutdown;
+/* Whether opaque points to a QIOChannel */
+bool has_ioc;
 };
 
 /*
@@ -101,7 +103,7 @@ bool qemu_file_mode_is_not_valid(const char *mode)
 return false;
 }
 
-QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
+QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc)
 {
 QEMUFile *f;
 
@@ -109,6 +111,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps 
*ops)
 
 f->opaque = opaque;
 f->ops = ops;
+f->has_ioc = has_ioc;
 return f;
 }
 
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index a9b6d6ccb7..80d0e79fd1 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -119,7 +119,7 @@ typedef struct QEMUFileHooks {
 QEMURamSaveFunc *save_page;
 } QEMUFileHooks;
 
-QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
+QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
diff --git a/migration/ram.c b/migration/ram.c
index b5fc454b2f..f2a86f9971 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -550,7 +550,7 @@ static int compress_threads_save_setup(void)
 /* comp_param[i].file is just used as a dummy buffer to save data,
  * set its ops to empty.
  */
-comp_param[i].file = qemu_fopen_ops(NULL, _ops);
+comp_param[i].file = qemu_fopen_ops(NULL, _ops, false);
 comp_param[i].done = true;
 comp_param[i].quit = false;
 qemu_mutex_init(_param[i].mutex);
diff --git a/migration/savevm.c b/migration/savevm.c
index 72848b946c..96b5e5d639 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -168,9 +168,9 @@ static const QEMUFileOps bdrv_write_ops = {
 static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
 {
 if (is_writable) {
-return qemu_fopen_ops(bs, _write_ops);
+return qemu_fopen_ops(bs, _write_ops, false);
 }
-return qemu_fopen_ops(bs, _read_ops);
+return qemu_fopen_ops(bs, _read_ops, false);
 }
 
 
-- 
2.31.1

[PATCH 3/5] migration: Introduce migration_ioc_[un]register_yank()

[Peter Xu]

There're plenty of places in migration/* that checks against either socket or
tls typed ioc for yank operations.  Provide two helpers to hide all these
information.

Signed-off-by: Peter Xu 
---
 migration/channel.c   | 15 ++-
 migration/multifd.c   |  8 ++--
 migration/qemu-file-channel.c |  8 ++--
 migration/yank_functions.c| 28 
 migration/yank_functions.h|  2 ++
 5 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/migration/channel.c b/migration/channel.c
index 01275a9162..c4fc000a1a 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -44,13 +44,7 @@ void migration_channel_process_incoming(QIOChannel *ioc)
  TYPE_QIO_CHANNEL_TLS)) {
 migration_tls_channel_process_incoming(s, ioc, _err);
 } else {
-if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET) ||
-object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS)) {
-yank_register_function(MIGRATION_YANK_INSTANCE,
-   migration_yank_iochannel,
-   QIO_CHANNEL(ioc));
-}
-
+migration_ioc_register_yank(ioc);
 migration_ioc_process_incoming(ioc, _err);
 }
 
@@ -94,12 +88,7 @@ void migration_channel_connect(MigrationState *s,
 } else {
 QEMUFile *f = qemu_fopen_channel_output(ioc);
 
-if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET) ||
-object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS)) {
-yank_register_function(MIGRATION_YANK_INSTANCE,
-   migration_yank_iochannel,
-   QIO_CHANNEL(ioc));
-}
+migration_ioc_register_yank(ioc);
 
 qemu_mutex_lock(>qemu_file_lock);
 s->to_dst_file = f;
diff --git a/migration/multifd.c b/migration/multifd.c
index ab41590e71..377da78f5b 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -987,12 +987,8 @@ int multifd_load_cleanup(Error **errp)
 for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDRecvParams *p = _recv_state->params[i];
 
-if ((object_dynamic_cast(OBJECT(p->c), TYPE_QIO_CHANNEL_SOCKET) ||
- object_dynamic_cast(OBJECT(p->c), TYPE_QIO_CHANNEL_TLS))
-&& OBJECT(p->c)->ref == 1) {
-yank_unregister_function(MIGRATION_YANK_INSTANCE,
- migration_yank_iochannel,
- QIO_CHANNEL(p->c));
+if (OBJECT(p->c)->ref == 1) {
+migration_ioc_unregister_yank(p->c);
 }
 
 object_unref(OBJECT(p->c));
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index fad340ea7a..867a5ed0c3 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -107,12 +107,8 @@ static int channel_close(void *opaque, Error **errp)
 int ret;
 QIOChannel *ioc = QIO_CHANNEL(opaque);
 ret = qio_channel_close(ioc, errp);
-if ((object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET) ||
- object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS))
-&& OBJECT(ioc)->ref == 1) {
-yank_unregister_function(MIGRATION_YANK_INSTANCE,
- migration_yank_iochannel,
- QIO_CHANNEL(ioc));
+if (OBJECT(ioc)->ref == 1) {
+migration_ioc_unregister_yank(ioc);
 }
 object_unref(OBJECT(ioc));
 return ret;
diff --git a/migration/yank_functions.c b/migration/yank_functions.c
index 96c90e17dc..23697173ae 100644
--- a/migration/yank_functions.c
+++ b/migration/yank_functions.c
@@ -11,6 +11,9 @@
 #include "qapi/error.h"
 #include "io/channel.h"
 #include "yank_functions.h"
+#include "qemu/yank.h"
+#include "io/channel-socket.h"
+#include "io/channel-tls.h"
 
 void migration_yank_iochannel(void *opaque)
 {
@@ -18,3 +21,28 @@ void migration_yank_iochannel(void *opaque)
 
 qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
 }
+
+/* Return whether yank is supported on this ioc */
+static bool migration_ioc_yank_supported(QIOChannel *ioc)
+{
+return object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET) ||
+object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
+}
+
+void migration_ioc_register_yank(QIOChannel *ioc)
+{
+if (migration_ioc_yank_supported(ioc)) {
+yank_register_function(MIGRATION_YANK_INSTANCE,
+   migration_yank_iochannel,
+   QIO_CHANNEL(ioc));
+}
+}
+
+void migration_ioc_unregister_yank(QIOChannel *ioc)
+{
+if (migration_ioc_yank_supported(ioc)) {
+yank_unregister_function(MIGRATION_YANK_INSTANCE,
+ migration_yank_iochannel,
+ QIO_CHANNEL(ioc));
+}
+}
diff --git a/migration/yank_functions.h 

[PATCH 2/5] migration: Shutdown src in await_return_path_close_on_source()

[Peter Xu]

We have a logic in await_return_path_close_on_source() that we will explicitly
shutdown the socket when migration encounters errors.  However it could be racy
because from_dst_file could have been reset right after checking it but before
passing it to qemu_file_shutdown() by the rp_thread.

Fix it by shutdown() on the src file instead.  Since they must be a pair of
qemu files, shutdown on either of them will work the same.

Since at it, drop the check for from_dst_file directly, which makes the
behavior even more predictable.

Reported-by: Dr. David Alan Gilbert 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 21b94f75a3..4f48cde796 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2882,12 +2882,15 @@ static int 
await_return_path_close_on_source(MigrationState *ms)
  * rp_thread will exit, however if there's an error we need to cause
  * it to exit.
  */
-if (qemu_file_get_error(ms->to_dst_file) && ms->rp_state.from_dst_file) {
+if (qemu_file_get_error(ms->to_dst_file)) {
 /*
  * shutdown(2), if we have it, will cause it to unblock if it's stuck
- * waiting for the destination.
+ * waiting for the destination.  We do shutdown on to_dst_file should
+ * also shutdown the from_dst_file as they're in a pair. We explicilty
+ * don't operate on from_dst_file because it's potentially racy
+ * (rp_thread could have reset it in parallel).
  */
-qemu_file_shutdown(ms->rp_state.from_dst_file);
+qemu_file_shutdown(ms->to_dst_file);
 mark_source_rp_bad(ms);
 }
 trace_await_return_path_close_on_source_joining();
-- 
2.31.1

[PATCH 0/5] migrations: Fix potential rare race of migration-test after yank

[Peter Xu]

Patch 1 fixes a possible race that migration thread can accidentally skip
join() of rp_thread even if the return thread is enabled.  Patch 1 is suspected
to also be the root cause of the recent hard-to-reproduce migration-test
failure here reported by PMM:

https://lore.kernel.org/qemu-devel/YPamXAHwan%2FPPXLf@work-vm/

I didn't reproduce it myself; but after co-debugged with Dave it's suspected
that the race of rp_thread could be the cause.  It's not exposed before because
yank is soo strict on releasing instances, while we're not that strict before,
and didn't join() on rp_thread wasn't so dangerous after all when migration
succeeded before.

Patch 2 fixes another theoretical race on accessing from_dst_file spotted by
Dave.  I don't think there's known issues with it, but may still worth fixing.

Patch 3 should be a cleanup on yank that I think would be nice to have.

Patch 4-5 are further cleanups to remove the ref==1 check in channel_close(),
finally, as I always thought that's a bit hackish.  So I used explicit
unregister of the yank function at necessary places to replace that ref==1 one.

I still think having patch 3-5 altogether would be great, however I think patch
1 should still be the most important to be reviewed.  Also it would be great to
know whether patch 1 could fix the known yank crash.

Please review, thanks.

Peter Xu (5):
  migration: Fix missing join() of rp_thread
  migration: Shutdown src in await_return_path_close_on_source()
  migration: Introduce migration_ioc_[un]register_yank()
  migration: Teach QEMUFile to be QIOChannel-aware
  migration: Move the yank unregister of channel_close out

 migration/channel.c   | 15 ++---
 migration/migration.c | 18 +++
 migration/migration.h |  7 ++
 migration/multifd.c   |  8 ++-
 migration/qemu-file-channel.c | 11 ++---
 migration/qemu-file.c | 17 +-
 migration/qemu-file.h |  4 +++-
 migration/ram.c   |  2 +-
 migration/savevm.c| 11 +++--
 migration/yank_functions.c| 42 +++
 migration/yank_functions.h|  3 +++
 11 files changed, 101 insertions(+), 37 deletions(-)

-- 
2.31.1

[PATCH 5/5] migration: Move the yank unregister of channel_close out

[Peter Xu]

It's efficient, but hackish to call yank unregister calls in channel_close(),
especially it'll be hard to debug when qemu crashed with some yank function
leaked.

Remove that hack, but instead explicitly unregister yank functions at the
places where needed, they are:

  (on src)
  - migrate_fd_cleanup
  - postcopy_pause

  (on dst)
  - migration_incoming_state_destroy
  - postcopy_pause_incoming

Some small helpers are introduced to achieve this task.  One of them is called
migration_file_get_ioc(), which tries to fetch the ioc out of the qemu file.
It's a bit tricky because qemufile is also used for savevm/loadvm.  We need to
check for NULL to bypass those.  Please see comment above that helper for more
information.

Signed-off-by: Peter Xu 
---
 migration/migration.c |  5 +
 migration/qemu-file-channel.c |  3 ---
 migration/qemu-file.c | 12 
 migration/qemu-file.h |  2 ++
 migration/savevm.c|  7 +++
 migration/yank_functions.c| 14 ++
 migration/yank_functions.h|  1 +
 7 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 4f48cde796..65b8c2eb52 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -59,6 +59,7 @@
 #include "multifd.h"
 #include "qemu/yank.h"
 #include "sysemu/cpus.h"
+#include "yank_functions.h"
 
 #define MAX_THROTTLE  (128 << 20)  /* Migration transfer speed throttling 
*/
 
@@ -273,6 +274,7 @@ void migration_incoming_state_destroy(void)
 }
 
 if (mis->from_src_file) {
+migration_ioc_unregister_yank_from_file(mis->from_src_file);
 qemu_fclose(mis->from_src_file);
 mis->from_src_file = NULL;
 }
@@ -1811,6 +1813,7 @@ static void migrate_fd_cleanup(MigrationState *s)
  * Close the file handle without the lock to make sure the
  * critical section won't block for long.
  */
+migration_ioc_unregister_yank_from_file(tmp);
 qemu_fclose(tmp);
 }
 
@@ -3337,6 +3340,8 @@ static MigThrError postcopy_pause(MigrationState *s)
 
 /* Current channel is possibly broken. Release it. */
 assert(s->to_dst_file);
+/* Unregister yank for current channel */
+migration_ioc_unregister_yank_from_file(s->to_dst_file);
 qemu_mutex_lock(>qemu_file_lock);
 file = s->to_dst_file;
 s->to_dst_file = NULL;
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 2f8b1fcd46..bb5a5752df 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -107,9 +107,6 @@ static int channel_close(void *opaque, Error **errp)
 int ret;
 QIOChannel *ioc = QIO_CHANNEL(opaque);
 ret = qio_channel_close(ioc, errp);
-if (OBJECT(ioc)->ref == 1) {
-migration_ioc_unregister_yank(ioc);
-}
 object_unref(OBJECT(ioc));
 return ret;
 }
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index ada58c94dd..b32ff35e73 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -854,3 +854,15 @@ void qemu_file_set_blocking(QEMUFile *f, bool block)
 f->ops->set_blocking(f->opaque, block, NULL);
 }
 }
+
+/*
+ * Return the ioc object if it's a migration channel.  Note: it can return NULL
+ * for callers passing in a non-migration qemufile.  E.g. see qemu_fopen_bdrv()
+ * and its usage in e.g. load_snapshot().  So we need to check against NULL
+ * before using it.  If without the check, migration_incoming_state_destroy()
+ * could fail for load_snapshot().
+ */
+QIOChannel *migration_file_get_ioc(QEMUFile *file)
+{
+return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL;
+}
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 80d0e79fd1..59f3f78e8b 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -27,6 +27,7 @@
 
 #include 
 #include "exec/cpu-common.h"
+#include "io/channel.h"
 
 /* Read a chunk of data from a file at the given position.  The pos argument
  * can be ignored if the file is only be used for streaming.  The number of
@@ -179,5 +180,6 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, 
void *data);
 size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
  ram_addr_t offset, size_t size,
  uint64_t *bytes_sent);
+QIOChannel *migration_file_get_ioc(QEMUFile *file);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 96b5e5d639..7b7b64bd13 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -65,6 +65,7 @@
 #include "qemu/bitmap.h"
 #include "net/announce.h"
 #include "qemu/yank.h"
+#include "yank_functions.h"
 
 const unsigned int postcopy_ram_discard_version;
 
@@ -2568,6 +2569,12 @@ static bool 
postcopy_pause_incoming(MigrationIncomingState *mis)
 /* Clear the triggered bit to allow one recovery */
 mis->postcopy_recover_triggered = false;
 
+/*
+ * Unregister yank with either from/to src would work, 

[PATCH 1/5] migration: Fix missing join() of rp_thread

[Peter Xu]

It's possible that the migration thread skip the join() of the rp_thread in
below race and crash on src right at finishing migration:

   migration_thread rp_thread
    -
migration_completion()
(before rp_thread quits)
from_dst_file=NULL
[thread got scheduled out]
  s->rp_state.from_dst_file==NULL
(skip join() of rp_thread)
migrate_fd_cleanup()
  qemu_fclose(s->to_dst_file)
  yank_unregister_instance()
assert(yank_find_entry())  <--- crash

It could mostly happen with postcopy, but that shouldn't be required, e.g., I
think it could also trigger with MIGRATION_CAPABILITY_RETURN_PATH set.

It's suspected that above race could be the root cause of a recent (but rare)
migration-test break reported by either Dave or PMM:

https://lore.kernel.org/qemu-devel/YPamXAHwan%2FPPXLf@work-vm/

The issue is: from_dst_file is reset in the rp_thread, so if the thread reset
it to NULL fast enough then the migration thread will assume there's no
rp_thread at all.

This could potentially cause more severe issue (e.g. crash) after the yank code.

Fix it by using a boolean to keep "whether we've created rp_thread".

Cc: Dr. David Alan Gilbert 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 4 +++-
 migration/migration.h | 7 +++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 2d306582eb..21b94f75a3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2867,6 +2867,7 @@ static int open_return_path_on_source(MigrationState *ms,
 
 qemu_thread_create(>rp_state.rp_thread, "return path",
source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
+ms->rp_state.rp_thread_created = true;
 
 trace_open_return_path_on_source_continue();
 
@@ -2891,6 +2892,7 @@ static int 
await_return_path_close_on_source(MigrationState *ms)
 }
 trace_await_return_path_close_on_source_joining();
 qemu_thread_join(>rp_state.rp_thread);
+ms->rp_state.rp_thread_created = false;
 trace_await_return_path_close_on_source_close();
 return ms->rp_state.error;
 }
@@ -3170,7 +3172,7 @@ static void migration_completion(MigrationState *s)
  * it will wait for the destination to send it's status in
  * a SHUT command).
  */
-if (s->rp_state.from_dst_file) {
+if (s->rp_state.rp_thread_created) {
 int rp_error;
 trace_migration_return_path_end_before();
 rp_error = await_return_path_close_on_source(s);
diff --git a/migration/migration.h b/migration/migration.h
index 2ebb740dfa..c302879fad 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -195,6 +195,13 @@ struct MigrationState {
 QEMUFile *from_dst_file;
 QemuThreadrp_thread;
 bool  error;
+/*
+ * We can also check non-zero of rp_thread, but there's no "official"
+ * way to do this, so this bool makes it slightly more elegant.
+ * Checking from_dst_file for this is racy because from_dst_file will
+ * be cleared in the rp_thread!
+ */
+bool  rp_thread_created;
 QemuSemaphore rp_sem;
 } rp_state;
 
-- 
2.31.1

[PATCH 1/3] docs: convert qapi-code-gen.txt to ReST

[John Snow]

This is a very rudimentary conversion from .txt to .rst changing as
little as possible, but getting it to render somewhat nicely; without
using any Sphinx directives. (It is 'native' ReST.)

Further patches will add cross-references and Sphinx-specific extensions
to make it sparkle.

Signed-off-by: John Snow 
---
 docs/devel/index.rst  |   1 +
 .../{qapi-code-gen.txt => qapi-code-gen.rst}  | 559 ++
 2 files changed, 312 insertions(+), 248 deletions(-)
 rename docs/devel/{qapi-code-gen.txt => qapi-code-gen.rst} (86%)

diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index 153979caf4b..bfba3a8daa6 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -42,3 +42,4 @@ modifying QEMU's source code.
multi-process
ebpf_rss
vfio-migration
+   qapi-code-gen
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.rst
similarity index 86%
rename from docs/devel/qapi-code-gen.txt
rename to docs/devel/qapi-code-gen.rst
index c1cb6f987de..b79ecddb599 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.rst
@@ -1,12 +1,17 @@
-= How to use the QAPI code generator =
+==
+How to use the QAPI code generator
+==
 
-Copyright IBM Corp. 2011
-Copyright (C) 2012-2016 Red Hat, Inc.
+..
+   Copyright IBM Corp. 2011
+   Copyright (C) 2012-2016 Red Hat, Inc.
 
-This work is licensed under the terms of the GNU GPL, version 2 or
-later.  See the COPYING file in the top-level directory.
+   This work is licensed under the terms of the GNU GPL, version 2 or
+   later.  See the COPYING file in the top-level directory.
 
-== Introduction ==
+
+Introduction
+
 
 QAPI is a native C API within QEMU which provides management-level
 functionality to internal and external users.  For external
@@ -23,7 +28,8 @@ Protocol and to C.  It additionally provides guidance on 
maintaining
 Client JSON Protocol compatibility.
 
 
-== The QAPI schema language ==
+The QAPI schema language
+
 
 The QAPI schema defines the Client JSON Protocol's commands and
 events, as well as types used by them.  Forward references are
@@ -39,9 +45,10 @@ complex types (structs and two flavors of unions), and 
alternate types
 (a choice between other types).
 
 
-=== Schema syntax ===
+Schema syntax
+-
 
-Syntax is loosely based on JSON (http://www.ietf.org/rfc/rfc8259.txt).
+Syntax is loosely based on `JSON `_.
 Differences:
 
 * Comments: start with a hash character (#) that is not part of a
@@ -79,7 +86,7 @@ syntax in an EBNF-like notation:
 The order of members within JSON objects does not matter unless
 explicitly noted.
 
-A QAPI schema consists of a series of top-level expressions:
+A QAPI schema consists of a series of top-level expressions::
 
 SCHEMA = TOP-LEVEL-EXPR...
 
@@ -87,11 +94,11 @@ The top-level expressions are all JSON objects.  Code and
 documentation is generated in schema definition order.  Code order
 should not matter.
 
-A top-level expressions is either a directive or a definition:
+A top-level expressions is either a directive or a definition::
 
 TOP-LEVEL-EXPR = DIRECTIVE | DEFINITION
 
-There are two kinds of directives and six kinds of definitions:
+There are two kinds of directives and six kinds of definitions::
 
 DIRECTIVE = INCLUDE | PRAGMA
 DEFINITION = ENUM | STRUCT | UNION | ALTERNATE | COMMAND | EVENT
@@ -99,9 +106,10 @@ There are two kinds of directives and six kinds of 
definitions:
 These are discussed in detail below.
 
 
-=== Built-in Types ===
+Built-in Types
+--
 
-The following types are predefined, and map to C as follows:
+The following types are predefined, and map to C as follows::
 
   SchemaC  JSON
   str   char * any JSON string, UTF-8
@@ -124,12 +132,14 @@ The following types are predefined, and map to C as 
follows:
   QType QType  JSON string matching enum QType values
 
 
-=== Include directives ===
+Include directives
+--
+
+Syntax::
 
-Syntax:
 INCLUDE = { 'include': STRING }
 
-The QAPI schema definitions can be modularized using the 'include' directive:
+The QAPI schema definitions can be modularized using the 'include' directive::
 
  { 'include': 'path/to/file.json' }
 
@@ -144,9 +154,11 @@ an outer file.  The parser may be made stricter in the 
future to
 prevent incomplete include files.
 
 
-=== Pragma directives ===
+Pragma directives
+-
+
+Syntax::
 
-Syntax:
 PRAGMA = { 'pragma': {
'*doc-required': BOOL,
'*command-name-exceptions': [ STRING, ... ],
@@ -172,9 +184,11 @@ names may contain uppercase letters, and '_' instead of 
'-'.  Default
 is none.
 
 
-=== Enumeration types ===
+Enumeration types
+-
+
+Syntax::
 
-Syntax:
 ENUM = { 'enum': STRING,
  'data': [ ENUM-VALUE, ... ],
  

[PATCH v1 26/29] tests/tcg/configure.sh: add handling for assembler only builds

[Alex Bennée]

Up until this point we only handled local compilers or assumed we had
everything in the container. This falls down when we are building QEMU
inside the container.

This special handling only affects tricore for now but I put it in a
case just in case we add any other "special" targets. Setting
CROSS_CC_GUEST is a bit of a hack just to ensure the test runs as we
gate on a detected compiler even though the Makefile won't actually
use it. It also means we display something sane in the configure
output.

Signed-off-by: Alex Bennée 
Message-Id: <20210720114057.32053-3-alex.ben...@linaro.org>
---
 tests/tcg/configure.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index aa7c24328a..1f985ccfc0 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -72,6 +72,10 @@ fi
 : ${cross_cc_x86_64="x86_64-linux-gnu-gcc"}
 : ${cross_cc_cflags_x86_64="-m64"}
 
+# tricore is special as it doesn't have a compiler
+: ${cross_as_tricore="tricore-as"}
+: ${cross_ld_tricore="tricore-ld"}
+
 for target in $target_list; do
   arch=${target%%-*}
 
@@ -247,6 +251,20 @@ for target in $target_list; do
   fi
   fi
   fi
+
+  # Special handling for assembler only tests
+  eval "target_as=\"\${cross_as_$arch}\""
+  eval "target_ld=\"\${cross_ld_$arch}\""
+  if has $target_as && has $target_ld; then
+  case $target in
+  tricore-softmmu)
+  echo "CROSS_CC_GUEST=$target_as" >> $config_target_mak
+  echo "CROSS_AS_GUEST=$target_as" >> $config_target_mak
+  echo "CROSS_LD_GUEST=$target_ld" >> $config_target_mak
+  got_cross_cc=yes
+  ;;
+  esac
+  fi
   fi
 
   if test $got_cross_cc = yes; then
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 02/29] docs: collect the disparate device emulation docs into one section

[Alex Bennée]

While we are at it add a brief preamble that explains some of the
common concepts in QEMU's device emulation which will hopefully lead
to less confusing about our dizzying command line options.

Signed-off-by: Alex Bennée 
Message-Id: <20210714182056.25888-3-alex.ben...@linaro.org>
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Daniel P. Berrangé 
Cc: Eduardo Habkost 

---
v2
  - be a bit more precise about necessity of a buses
  - add an example showing id/bus relations
---
 docs/system/device-emulation.rst  | 89 +++
 docs/system/{ => devices}/ivshmem.rst |  0
 docs/system/{ => devices}/net.rst |  0
 docs/system/{ => devices}/nvme.rst|  0
 docs/system/{ => devices}/usb.rst |  0
 docs/system/{ => devices}/virtio-pmem.rst |  0
 docs/system/index.rst |  6 +-
 7 files changed, 90 insertions(+), 5 deletions(-)
 create mode 100644 docs/system/device-emulation.rst
 rename docs/system/{ => devices}/ivshmem.rst (100%)
 rename docs/system/{ => devices}/net.rst (100%)
 rename docs/system/{ => devices}/nvme.rst (100%)
 rename docs/system/{ => devices}/usb.rst (100%)
 rename docs/system/{ => devices}/virtio-pmem.rst (100%)

diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
new file mode 100644
index 00..7af5dbefab
--- /dev/null
+++ b/docs/system/device-emulation.rst
@@ -0,0 +1,89 @@
+.. _device-emulation:
+
+Device Emulation
+
+
+QEMU supports the emulation of a large number of devices from
+peripherals such network cards and USB devices to integrated systems
+on a chip (SoCs). Configuration of these is often a source of
+confusion so it helps to have an understanding of some of the terms
+used to describes devices within QEMU.
+
+Common Terms
+
+
+Device Front End
+
+
+A device front end is how a device is presented to the guest. The type
+of device presented should match the hardware that the guest operating
+system is expecting to see. All devices can be specified with the
+``--device`` command line option. Running QEMU with the command line
+options ``--device help`` will list all devices it is aware of. Using
+the command line ``--device foo,help`` will list the additional
+configuration options available for that device.
+
+A front end is often paired with a back end, which describes how the
+host's resources are used in the emulation.
+
+Device Buses
+
+
+Most devices will exist on a BUS of some sort. Depending on the
+machine model you choose (``-M foo``) a number of buses will have been
+automatically created. In most cases the BUS a device is attached to
+can be inferred, for example PCI devices are generally automatically
+allocated to the next free address of first PCI bus found. However in
+complicated configurations you can explicitly specify what bus
+(``bus=ID``) a device is attached to along with its address
+(``addr=N``).
+
+Some devices, for example a PCI SCSI host controller, will add an
+additional buses to the system that other devices can be attached to.
+A hypothetical chain of devices might look like:
+
+  --device foo,bus=pci.0,addr=0,id=foo
+  --device bar,bus=foo.0,addr=1,id=baz
+
+which would be a bar device (with the ID of baz) which is attached to
+the first foo bus (foo.0) at address 1. The foo device which provides
+that bus is itself is attached to the first PCI bus (pci.0).
+
+
+Device Back End
+===
+
+The back end describes how the data from the emulated device will be
+processed by QEMU. The configuration of the back end is usually
+specific to the class of device being emulated. For example serial
+devices will be backed by a ``--chardev`` which can redirect the data
+to a file or socket or some other system. Storage devices are handled
+by ``--blockdev`` which will specify how blocks are handled, for
+example being stored in a qcow2 file or accessing a raw host disk
+partition. Back ends can sometimes be stacked to implement features
+like snapshots.
+
+While the choice of back end is generally transparent to the guest
+there are cases where features will not be reported to the guest if
+the back end is unable to support it.
+
+Device Pass Through
+===
+
+Device pass through is where the device is actually given access to
+the underlying hardware. This can be as simple as exposing a single
+USB device on the host system to the guest or dedicating a video card
+in a PCI slot to the exclusive use of the guest.
+
+
+Emulated Devices
+
+
+.. toctree::
+   :maxdepth: 1
+
+   devices/ivshmem.rst
+   devices/net.rst
+   devices/nvme.rst
+   devices/usb.rst
+   devices/virtio-pmem.rst
diff --git a/docs/system/ivshmem.rst b/docs/system/devices/ivshmem.rst
similarity index 100%
rename from docs/system/ivshmem.rst
rename to docs/system/devices/ivshmem.rst
diff --git a/docs/system/net.rst b/docs/system/devices/net.rst
similarity index 100%
rename from docs/system/net.rst
rename to 

[PATCH 2/3] docs/qapi-code-gen: Beautify formatting

[John Snow]

Mostly, add ``literal`` markers to a lot of things like C types, add
code blocks, and fix the way a few things render.

Signed-off-by: John Snow 
---
 docs/devel/qapi-code-gen.rst | 172 ++-
 1 file changed, 90 insertions(+), 82 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index b79ecddb599..4a28118d951 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -40,7 +40,7 @@ by any commands or events, for the side effect of generated C 
code
 used internally.
 
 There are several kinds of types: simple types (a number of built-in
-types, such as 'int' and 'str'; as well as enumerations), arrays,
+types, such as ``int`` and ``str``; as well as enumerations), arrays,
 complex types (structs and two flavors of unions), and alternate types
 (a choice between other types).
 
@@ -51,37 +51,37 @@ Schema syntax
 Syntax is loosely based on `JSON `_.
 Differences:
 
-* Comments: start with a hash character (#) that is not part of a
+* Comments: start with a hash character (``#``) that is not part of a
   string, and extend to the end of the line.
 
-* Strings are enclosed in 'single quotes', not "double quotes".
+* Strings are enclosed in ``'single quotes'``, not ``"double quotes"``.
 
 * Strings are restricted to printable ASCII, and escape sequences to
-  just '\\'.
+  just ``\\``.
 
-* Numbers and null are not supported.
+* Numbers and ``null`` are not supported.
 
 A second layer of syntax defines the sequences of JSON texts that are
 a correctly structured QAPI schema.  We provide a grammar for this
 syntax in an EBNF-like notation:
 
-* Production rules look like non-terminal = expression
-* Concatenation: expression A B matches expression A, then B
-* Alternation: expression A | B matches expression A or B
-* Repetition: expression A... matches zero or more occurrences of
-  expression A
-* Repetition: expression A, ... matches zero or more occurrences of
-  expression A separated by ,
-* Grouping: expression ( A ) matches expression A
-* JSON's structural characters are terminals: { } [ ] : ,
-* JSON's literal names are terminals: false true
-* String literals enclosed in 'single quotes' are terminal, and match
-  this JSON string, with a leading '*' stripped off
-* When JSON object member's name starts with '*', the member is
+* Production rules look like ``non-terminal = expression``
+* Concatenation: expression ``A B`` matches expression ``A``, then ``B``
+* Alternation: expression ``A | B`` matches expression ``A`` or ``B``
+* Repetition: expression ``A...`` matches zero or more occurrences of
+  expression ``A``
+* Repetition: expression ``A, ...`` matches zero or more occurrences of
+  expression ``A`` separated by ``,``
+* Grouping: expression ``( A )`` matches expression ``A``
+* JSON's structural characters are terminals: ``{ } [ ] : ,``
+* JSON's literal names are terminals: ``false true``
+* String literals enclosed in ``'single quotes'`` are terminal, and match
+  this JSON string, with a leading ``*`` stripped off
+* When JSON object member's name starts with ``*``, the member is
   optional.
-* The symbol STRING is a terminal, and matches any JSON string
-* The symbol BOOL is a terminal, and matches JSON false or true
-* ALL-CAPS words other than STRING are non-terminals
+* The symbol ``STRING`` is a terminal, and matches any JSON string
+* The symbol ``BOOL`` is a terminal, and matches JSON ``false`` or ``true``
+* ALL-CAPS words other than ``STRING`` are non-terminals
 
 The order of members within JSON objects does not matter unless
 explicitly noted.
@@ -109,27 +109,30 @@ These are discussed in detail below.
 Built-in Types
 --
 
-The following types are predefined, and map to C as follows::
+The following types are predefined, and map to C as follows:
 
-  SchemaC  JSON
-  str   char * any JSON string, UTF-8
-  numberdouble any JSON number
-  int   int64_ta JSON number without fractional part
-   that fits into the C integer type
-  int8  int8_t likewise
-  int16 int16_tlikewise
-  int32 int32_tlikewise
-  int64 int64_tlikewise
-  uint8 uint8_tlikewise
-  uint16uint16_t   likewise
-  uint32uint32_t   likewise
-  uint64uint64_t   likewise
-  size  uint64_t   like uint64_t, except StringInputVisitor
-   accepts size suffixes
-  bool  bool   JSON true or false
-  null  QNull *JSON null
-  any   QObject *  any JSON value
-  QType QType  JSON string matching enum QType values
+  = == 
+  SchemaC  JSON
+  = == 
+  ``str``   ``char *`` any JSON string, UTF-8
+  ``number````double`` any JSON number
+  ``int``   ``int64_t``

[PATCH 3/3] docs/qapi-code-gen: add cross-references

[John Snow]

Add clickables to many places.

Signed-off-by: John Snow 
---
 docs/devel/qapi-code-gen.rst | 107 +++
 1 file changed, 58 insertions(+), 49 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 4a28118d951..8c77af2d076 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -156,6 +156,7 @@ from making a forward reference to a type that is only 
introduced by
 an outer file.  The parser may be made stricter in the future to
 prevent incomplete include files.
 
+.. _pragma:
 
 Pragma directives
 -
@@ -186,6 +187,7 @@ Pragma 'member-name-exceptions' takes a list of types whose 
member
 names may contain uppercase letters, and ``"_"`` instead of ``"-"``.
 Default is none.
 
+.. _ENUM-VALUE:
 
 Enumeration types
 -
@@ -228,13 +230,15 @@ additional enumeration constant PREFIX__MAX with value N.
 Do not use string or an integer type when an enumeration type can do
 the job satisfactorily.
 
-The optional 'if' member specifies a conditional.  See "Configuring
-the schema" below for more on this.
+The optional 'if' member specifies a conditional.  See `Configuring the
+schema`_ below for more on this.
 
-The optional 'features' member specifies features.  See "Features"
+The optional 'features' member specifies features.  See Features_
 below for more on this.
 
 
+.. _TYPE-REF:
+
 Type references and array types
 ---
 
@@ -269,11 +273,13 @@ Member 'struct' names the struct type.
 
 Each MEMBER of the 'data' object defines a member of the struct type.
 
+.. _MEMBERS:
+
 The MEMBER's STRING name consists of an optional ``*`` prefix and the
 struct member name.  If ``*`` is present, the member is optional.
 
 The MEMBER's value defines its properties, in particular its type.
-The form TYPE-REF is shorthand for :code:`{ 'type': TYPE-REF }`.
+The form TYPE-REF_ is shorthand for :code:`{ 'type': TYPE-REF }`.
 
 Example::
 
@@ -300,10 +306,10 @@ both members like this::
  { "file": "/some/place/my-image",
"backing": "/some/place/my-backing-file" }
 
-The optional 'if' member specifies a conditional.  See "Configuring
-the schema" below for more on this.
+The optional 'if' member specifies a conditional.  See `Configuring
+the schema`_ below for more on this.
 
-The optional 'features' member specifies features.  See "Features"
+The optional 'features' member specifies features.  See Features_
 below for more on this.
 
 
@@ -337,7 +343,7 @@ union must have at least one branch.
 The BRANCH's STRING name is the branch name.
 
 The BRANCH's value defines the branch's properties, in particular its
-type.  The form TYPE-REF is shorthand for :code:`{ 'type': TYPE-REF }`.
+type.  The form TYPE-REF_ is shorthand for :code:`{ 'type': TYPE-REF }`.
 
 A simple union type defines a mapping from automatic discriminator
 values to data types like in this example::
@@ -368,12 +374,12 @@ Flat unions permit arbitrary common members that occur in 
all variants
 of the union, not just a discriminator.  Their discriminators need not
 be named 'type'.  They also avoid nesting on the wire.
 
-The 'base' member defines the common members.  If it is a MEMBERS
+The 'base' member defines the common members.  If it is a MEMBERS_
 object, it defines common members just like a struct type's 'data'
 member defines struct type members.  If it is a STRING, it names a
 struct type whose members are the common members.
 
-All flat union branches must be of struct type.
+All flat union branches must be `Struct types`_.
 
 In the Client JSON Protocol, a flat union is represented by an object
 with the common members (from the base type) and the selected branch's
@@ -425,10 +431,10 @@ is identical on the wire to::
  { 'union': 'Flat', 'base': { 'type': 'Enum' }, 'discriminator': 'type',
'data': { 'one': 'Branch1', 'two': 'Branch2' } }
 
-The optional 'if' member specifies a conditional.  See "Configuring
-the schema" below for more on this.
+The optional 'if' member specifies a conditional.  See `Configuring
+the schema`_ below for more on this.
 
-The optional 'features' member specifies features.  See "Features"
+The optional 'features' member specifies features.  See Features_
 below for more on this.
 
 
@@ -481,10 +487,10 @@ following example objects::
  "read-only": false,
  "filename": "/tmp/mydisk.qcow2" } }
 
-The optional 'if' member specifies a conditional.  See "Configuring
-the schema" below for more on this.
+The optional 'if' member specifies a conditional.  See `Configuring
+the schema`_ below for more on this.
 
-The optional 'features' member specifies features.  See "Features"
+The optional 'features' member specifies features.  See Features_
 below for more on this.
 
 
@@ -511,10 +517,10 @@ Syntax::
 
 Member 'command' names the command.
 
-Member 'data' defines the arguments.  It defaults to an empty MEMBERS
+Member 'data' defines the arguments.  It 

[PATCH v1 28/29] gitlab-ci: Remove the second superfluous macos task

[Alex Bennée]

From: Thomas Huth 

While there might have been bigger differnces between the -base and
the -xcode images in the beginning, they almost vanished in the
current builds, e.g. when comparing the output of the "configure"
step after cleaning up the differences due to temporary path names,
I only get:

  $ diff -u /tmp/base.txt /tmp/xcode.txt
  --- /tmp/base.txt 2021-07-16 09:16:24.211427940 +0200
  +++ /tmp/xcode.txt2021-07-16 09:16:43.029684274 +0200
  @@ -19,14 +19,14 @@
   Build type: native build
   Project name: qemu
   Project version: 6.0.50
  -C compiler for the host machine: cc (clang 12.0.0 "Apple clang version 
12.0.0 (clang-1200.0.32.29)")
  +C compiler for the host machine: cc (clang 12.0.0 "Apple clang version 
12.0.0 (clang-1200.0.32.28)")
   C linker for the host machine: cc ld64 609.8
   Host machine cpu family: x86_64
   Host machine cpu: x86_64
   Program sh found: YES (/bin/sh)
   Program python3 found: YES (/usr/local/opt/python@3.9/bin/python3.9)
   Program bzip2 found: YES (/usr/bin/bzip2)
  -C++ compiler for the host machine: c++ (clang 12.0.0 "Apple clang version 
12.0.0 (clang-1200.0.32.29)")
  +C++ compiler for the host machine: c++ (clang 12.0.0 "Apple clang version 
12.0.0 (clang-1200.0.32.28)")
   C++ linker for the host machine: c++ ld64 609.8
   Objective-C compiler for the host machine: clang (clang 12.0.0)
   Objective-C linker for the host machine: clang ld64 609.8

Since we're not using Xcode itself at all, it seems like it does not
make much sense anymore to waste compute cycles with two images here.
Thus let's delete the -xcode job now.

Signed-off-by: Thomas Huth 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Daniel P. Berrangé 
Message-Id: <20210719073051.1559348-1-th...@redhat.com>
[AJB: fix up commit formatting which trips up b4]
Signed-off-by: Alex Bennée 
---
 .gitlab-ci.d/cirrus.yml | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index 60b13ed83f..675db69622 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -85,18 +85,3 @@ x64-macos-11-base-build:
 PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin
 PKG_CONFIG_PATH: 
/usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
 TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat 
check-qtest-x86_64
-
-x64-macos-11-xcode-build:
-  extends: .cirrus_build_job
-  variables:
-NAME: macos-11
-CIRRUS_VM_INSTANCE_TYPE: osx_instance
-CIRRUS_VM_IMAGE_SELECTOR: image
-CIRRUS_VM_IMAGE_NAME: big-sur-xcode
-CIRRUS_VM_CPUS: 12
-CIRRUS_VM_RAM: 24G
-UPDATE_COMMAND: brew update
-INSTALL_COMMAND: brew install
-PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin
-PKG_CONFIG_PATH: 
/usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
-TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat 
check-qtest-x86_64
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 17/29] contrib/gitdm: add an explicit academic entry for BU

[Alex Bennée]

For some reason Alexander's contributions were not getting grouped
from the plain "edu" mapping.

Signed-off-by: Alex Bennée 
Reviewed-by: Alexander Bulekov 
Message-Id: <20210714182056.25888-20-alex.ben...@linaro.org>
---
 contrib/gitdm/group-map-academics | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/gitdm/group-map-academics 
b/contrib/gitdm/group-map-academics
index bf3c894821..44745ca85b 100644
--- a/contrib/gitdm/group-map-academics
+++ b/contrib/gitdm/group-map-academics
@@ -16,3 +16,6 @@ c...@braap.org
 uni-paderborn.de
 edu
 edu.cn
+
+# Boston University
+bu.edu
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 16/29] contrib/gitdm: add group-map for Netflix

[Alex Bennée]

Warner confirmed he works for Netflix on IRC.

Signed-off-by: Alex Bennée 
Reviewed-by: Warner Losh 
Message-Id: <20210714182056.25888-19-alex.ben...@linaro.org>
---
 contrib/gitdm/group-map-netflix | 5 +
 gitdm.config| 1 +
 2 files changed, 6 insertions(+)
 create mode 100644 contrib/gitdm/group-map-netflix

diff --git a/contrib/gitdm/group-map-netflix b/contrib/gitdm/group-map-netflix
new file mode 100644
index 00..468f95dcb2
--- /dev/null
+++ b/contrib/gitdm/group-map-netflix
@@ -0,0 +1,5 @@
+#
+# Netflix contributors using their personal emails
+#
+
+i...@bsdimp.com
diff --git a/gitdm.config b/gitdm.config
index a3542d2fc7..5d5e70fe5f 100644
--- a/gitdm.config
+++ b/gitdm.config
@@ -35,6 +35,7 @@ GroupMap contrib/gitdm/group-map-cadence Cadence Design 
Systems
 GroupMap contrib/gitdm/group-map-codeweavers CodeWeavers
 GroupMap contrib/gitdm/group-map-ibm IBM
 GroupMap contrib/gitdm/group-map-janustech Janus Technologies
+GroupMap contrib/gitdm/group-map-netflix Netflix
 GroupMap contrib/gitdm/group-map-redhat Red Hat
 GroupMap contrib/gitdm/group-map-wavecomp Wave Computing
 
-- 
2.32.0.264.g75ae10bc75

[PATCH 0/3] docs: convert qapi-code-gen.txt to qapi-code-gen.rst

[John Snow]

Patch 1 does (roughly) the bare minimum, patch 2 adds some formatting,
and patch 3 adds cross-references.

John Snow (3):
  docs: convert qapi-code-gen.txt to ReST
  docs/qapi-code-gen: Beautify formatting
  docs/qapi-code-gen: add cross-references

 docs/devel/index.rst  |   1 +
 .../{qapi-code-gen.txt => qapi-code-gen.rst}  | 800 ++
 2 files changed, 441 insertions(+), 360 deletions(-)
 rename docs/devel/{qapi-code-gen.txt => qapi-code-gen.rst} (76%)

-- 
2.31.1

[PATCH v1 29/29] gitlab-ci: Extract OpenSBI job rules to reusable section

[Alex Bennée]

From: Philippe Mathieu-Daudé 

All jobs depending on 'docker-opensbi' job must use at most all
the rules that triggers it. The simplest way to ensure that
is to always use the same rules. Extract all the rules to a
reusable section, and include this section (with the 'extends'
keyword) in both 'docker-opensbi' and 'build-opensbi' jobs.

The problem was introduced in commit c6fc0fc1a71 ("gitlab-ci.yml:
Add jobs to build OpenSBI firmware binaries"), but was revealed in
commit 91e9c47e50a ("docker: OpenSBI build job depends on OpenSBI
container").

This fix is similar to the one used with the EDK2 firmware job in
commit ac0595cf6b3 ("gitlab-ci: Extract EDK2 job rules to reusable
section").

Reported-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210720164829.3949558-1-phi...@redhat.com>
Signed-off-by: Alex Bennée 
---
 .gitlab-ci.d/opensbi.yml | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
index f66cd1d908..d8a0456679 100644
--- a/.gitlab-ci.d/opensbi.yml
+++ b/.gitlab-ci.d/opensbi.yml
@@ -1,10 +1,23 @@
-docker-opensbi:
- stage: containers
- rules: # Only run this job when the Dockerfile is modified
+# All jobs needing docker-opensbi must use the same rules it uses.
+.opensbi_job_rules:
+ rules: # Only run this job when ...
  - changes:
+   # this file is modified
- .gitlab-ci.d/opensbi.yml
+   # or the Dockerfile is modified
- .gitlab-ci.d/opensbi/Dockerfile
when: always
+ - changes: # or roms/opensbi/ is modified (submodule updated)
+   - roms/opensbi/*
+   when: always
+ - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 
'opensbi'
+   when: always
+ - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description 
contains 'OpenSBI'
+   when: always
+
+docker-opensbi:
+ extends: .opensbi_job_rules
+ stage: containers
  image: docker:19.03.1
  services:
  - docker:19.03.1-dind
@@ -24,16 +37,9 @@ docker-opensbi:
  - docker push $IMAGE_TAG
 
 build-opensbi:
+ extends: .opensbi_job_rules
  stage: build
  needs: ['docker-opensbi']
- rules: # Only run this job when ...
- - changes: # ... roms/opensbi/ is modified (submodule updated)
-   - roms/opensbi/*
-   when: always
- - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 
'opensbi'
-   when: always
- - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description 
contains 'OpenSBI'
-   when: always
  artifacts:
paths: # 'artifacts.zip' will contains the following files:
- pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 20/29] tcg/plugins: implement a qemu_plugin_user_exit helper

[Alex Bennée]

In user-mode emulation there is a small race between preexit_cleanup
and exit_group() which means we may end up calling instrumented
instructions before the kernel reaps child threads. To solve this we
implement a new helper which ensures the callbacks are flushed along
with any translations before we let the host do it's a thing.

While we are at it make the documentation of
qemu_plugin_register_atexit_cb clearer as to what the user can expect.

Signed-off-by: Alex Bennée 
Reviewed-by: Mahmoud Mandour 
Acked-by: Warner Losh 
Message-Id: <20210719123732.24457-1-alex.ben...@linaro.org>

---
v2
  - included qemu_plugin_disable_mem_helpers
---
 include/qemu/plugin.h  | 12 
 include/qemu/qemu-plugin.h | 13 +
 bsd-user/syscall.c |  6 +++---
 linux-user/exit.c  |  2 +-
 plugins/core.c | 39 ++
 5 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 0fefbc6084..9a8438f683 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -190,6 +190,16 @@ void qemu_plugin_add_dyn_cb_arr(GArray *arr);
 
 void qemu_plugin_disable_mem_helpers(CPUState *cpu);
 
+/**
+ * qemu_plugin_user_exit(): clean-up callbacks before calling exit callbacks
+ *
+ * This is a user-mode only helper that ensure we have fully cleared
+ * callbacks from all threads before calling the exit callbacks. This
+ * is so the plugins themselves don't have to jump through hoops to
+ * guard against race conditions.
+ */
+void qemu_plugin_user_exit(void);
+
 #else /* !CONFIG_PLUGIN */
 
 static inline void qemu_plugin_add_opts(void)
@@ -250,6 +260,8 @@ void qemu_plugin_add_dyn_cb_arr(GArray *arr)
 static inline void qemu_plugin_disable_mem_helpers(CPUState *cpu)
 { }
 
+static inline void qemu_plugin_user_exit(void)
+{ }
 #endif /* !CONFIG_PLUGIN */
 
 #endif /* QEMU_PLUGIN_H */
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index dc3496f36c..e6e815abc5 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -549,6 +549,19 @@ void qemu_plugin_vcpu_for_each(qemu_plugin_id_t id,
 void qemu_plugin_register_flush_cb(qemu_plugin_id_t id,
qemu_plugin_simple_cb_t cb);
 
+/**
+ * qemu_plugin_register_atexit_cb() - register exit callback
+ * @id: plugin ID
+ * @cb: callback
+ * @userdata: user data for callback
+ *
+ * The @cb function is called once execution has finished. Plugins
+ * should be able to free all their resources at this point much like
+ * after a reset/uninstall callback is called.
+ *
+ * In user-mode it is possible a few un-instrumented instructions from
+ * child threads may run before the host kernel reaps the threads.
+ */
 void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
 qemu_plugin_udata_cb_t cb, void *userdata);
 
diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index 7d986e9700..3f44311396 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -335,7 +335,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 _mcleanup();
 #endif
 gdb_exit(arg1);
-qemu_plugin_atexit_cb();
+qemu_plugin_user_exit();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
 ret = 0; /* avoid warning */
@@ -437,7 +437,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long 
arg1,
 _mcleanup();
 #endif
 gdb_exit(arg1);
-qemu_plugin_atexit_cb();
+qemu_plugin_user_exit();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
 ret = 0; /* avoid warning */
@@ -516,7 +516,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 _mcleanup();
 #endif
 gdb_exit(arg1);
-qemu_plugin_atexit_cb();
+qemu_plugin_user_exit();
 /* XXX: should free thread stack and CPU env */
 _exit(arg1);
 ret = 0; /* avoid warning */
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 70b344048c..527e29cbc1 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -35,5 +35,5 @@ void preexit_cleanup(CPUArchState *env, int code)
 __gcov_dump();
 #endif
 gdb_exit(code);
-qemu_plugin_atexit_cb();
+qemu_plugin_user_exit();
 }
diff --git a/plugins/core.c b/plugins/core.c
index e1bcdb570d..7cf4f87e18 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -487,6 +487,45 @@ void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
 plugin_register_cb_udata(id, QEMU_PLUGIN_EV_ATEXIT, cb, udata);
 }
 
+/*
+ * Handle exit from linux-user. Unlike the normal atexit() mechanism
+ * we need to handle the clean-up manually as it's possible threads
+ * are still running. We need to remove all callbacks from code
+ * generation, flush the current translations and then we can safely
+ * trigger the exit callbacks.
+ */
+
+void qemu_plugin_user_exit(void)
+{
+

[PATCH v1 09/29] gitdm.config: sort the corporate GroupMap entries

[Alex Bennée]

Lets try and keep them that way.

Signed-off-by: Alex Bennée 
Message-Id: <20210714182056.25888-10-alex.ben...@linaro.org>
---
 gitdm.config | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitdm.config b/gitdm.config
index 7378238c20..a3542d2fc7 100644
--- a/gitdm.config
+++ b/gitdm.config
@@ -28,15 +28,15 @@ EmailMap contrib/gitdm/domain-map
 #
 # Use GroupMap to map a file full of addresses to the
 # same employer. This is used for people that don't post from easily
-# identifiable corporate emails.
+# identifiable corporate emails. Please keep this list sorted.
 #
 
-GroupMap contrib/gitdm/group-map-redhat Red Hat
-GroupMap contrib/gitdm/group-map-wavecomp Wave Computing
 GroupMap contrib/gitdm/group-map-cadence Cadence Design Systems
 GroupMap contrib/gitdm/group-map-codeweavers CodeWeavers
 GroupMap contrib/gitdm/group-map-ibm IBM
 GroupMap contrib/gitdm/group-map-janustech Janus Technologies
+GroupMap contrib/gitdm/group-map-redhat Red Hat
+GroupMap contrib/gitdm/group-map-wavecomp Wave Computing
 
 # Also group together our prolific individual contributors
 # and those working under academic auspices
-- 
2.32.0.264.g75ae10bc75

[PULL 4/7] tests/acceptance/virtio-gpu.py: combine kernel command line

[Cleber Rosa]

Both tests use the same kernel command line arguments, so there's no
need to have a common and then an additional set of arguments.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-5-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 20a59fabf3..fbde278705 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -34,7 +34,7 @@ class VirtioGPUx86(Test):
 :avocado: tags=cpu:host
 """
 
-KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
+KERNEL_COMMAND_LINE = "printk.time=0 console=ttyS0 rdinit=/bin/bash"
 KERNEL_URL = (
 "https://archives.fedoraproject.org/pub/fedora;
 "/linux/releases/33/Everything/x86_64/os/images"
@@ -58,9 +58,6 @@ def test_virtio_vga_virgl(self):
 """
 :avocado: tags=device:virtio-vga
 """
-kernel_command_line = (
-self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-)
 # FIXME: should check presence of virtio, virgl etc
 self.require_accelerator('kvm')
 
@@ -78,7 +75,7 @@ def test_virtio_vga_virgl(self):
 "-initrd",
 initrd_path,
 "-append",
-kernel_command_line,
+self.KERNEL_COMMAND_LINE,
 )
 try:
 self.vm.launch()
@@ -96,9 +93,6 @@ def test_vhost_user_vga_virgl(self):
 """
 :avocado: tags=device:vhost-user-vga
 """
-kernel_command_line = (
-self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-)
 # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
 self.require_accelerator('kvm')
 
@@ -145,7 +139,7 @@ def test_vhost_user_vga_virgl(self):
 "-initrd",
 initrd_path,
 "-append",
-kernel_command_line,
+self.KERNEL_COMMAND_LINE,
 )
 self.vm.launch()
 self.wait_for_console_pattern("as init process")
-- 
2.31.1

[PULL 3/7] tests/acceptance/virtio-gpu.py: combine CPU tags

[Cleber Rosa]

Like previously done with the arch tags, all tests use the same CPU
value so it's possible to combine them at the class level.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-4-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 729b99b2e5..20a59fabf3 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -31,6 +31,7 @@ class VirtioGPUx86(Test):
 """
 :avocado: tags=virtio-gpu
 :avocado: tags=arch:x86_64
+:avocado: tags=cpu:host
 """
 
 KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
@@ -56,7 +57,6 @@ def wait_for_console_pattern(self, success_message, vm=None):
 def test_virtio_vga_virgl(self):
 """
 :avocado: tags=device:virtio-vga
-:avocado: tags=cpu:host
 """
 kernel_command_line = (
 self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
@@ -95,7 +95,6 @@ def test_virtio_vga_virgl(self):
 def test_vhost_user_vga_virgl(self):
 """
 :avocado: tags=device:vhost-user-vga
-:avocado: tags=cpu:host
 """
 kernel_command_line = (
 self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-- 
2.31.1

[PULL 1/7] tests/acceptance/virtio-gpu.py: use require_accelerator()

[Cleber Rosa]

Since efe30d501 there's a shorthand for requiring specific
accelerators, and canceling the test if it's not available.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-2-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 589332c1b7..42602a240a 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -17,10 +17,6 @@
 import subprocess
 
 
-ACCEL_NOT_AVAILABLE_FMT = "%s accelerator does not seem to be available"
-KVM_NOT_AVAILABLE = ACCEL_NOT_AVAILABLE_FMT % "KVM"
-
-
 def pick_default_vug_bin():
 relative_path = "./contrib/vhost-user-gpu/vhost-user-gpu"
 if is_readable_executable_file(relative_path):
@@ -66,8 +62,7 @@ def test_virtio_vga_virgl(self):
 self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
 )
 # FIXME: should check presence of virtio, virgl etc
-if not kvm_available(self.arch, self.qemu_bin):
-self.cancel(KVM_NOT_AVAILABLE)
+self.require_accelerator('kvm')
 
 kernel_path = self.fetch_asset(self.KERNEL_URL)
 initrd_path = self.fetch_asset(self.INITRD_URL)
@@ -107,8 +102,7 @@ def test_vhost_user_vga_virgl(self):
 self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
 )
 # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
-if not kvm_available(self.arch, self.qemu_bin):
-self.cancel(KVM_NOT_AVAILABLE)
+self.require_accelerator('kvm')
 
 vug = pick_default_vug_bin()
 if not vug:
-- 
2.31.1

[PULL 2/7] tests/acceptance/virtio-gpu.py: combine x86_64 arch tags

[Cleber Rosa]

The test class in question is x86_64 specific, so it's possible to set
the tags at the class level.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-3-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 42602a240a..729b99b2e5 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -30,6 +30,7 @@ def pick_default_vug_bin():
 class VirtioGPUx86(Test):
 """
 :avocado: tags=virtio-gpu
+:avocado: tags=arch:x86_64
 """
 
 KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
@@ -54,7 +55,6 @@ def wait_for_console_pattern(self, success_message, vm=None):
 
 def test_virtio_vga_virgl(self):
 """
-:avocado: tags=arch:x86_64
 :avocado: tags=device:virtio-vga
 :avocado: tags=cpu:host
 """
@@ -94,7 +94,6 @@ def test_virtio_vga_virgl(self):
 
 def test_vhost_user_vga_virgl(self):
 """
-:avocado: tags=arch:x86_64
 :avocado: tags=device:vhost-user-vga
 :avocado: tags=cpu:host
 """
-- 
2.31.1

[PULL 5/7] tests/acceptance/virtio-gpu.py: use virtio-vga-gl

[Cleber Rosa]

Since 49afbca3b, the use of an optional virgl renderer is not
available anymore, and since b36eb8860f, the way to choose a GL based
rendered is to use the "virtio-vga-gl" device.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-6-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index fbde278705..0f84affe82 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -56,7 +56,7 @@ def wait_for_console_pattern(self, success_message, vm=None):
 
 def test_virtio_vga_virgl(self):
 """
-:avocado: tags=device:virtio-vga
+:avocado: tags=device:virtio-vga-gl
 """
 # FIXME: should check presence of virtio, virgl etc
 self.require_accelerator('kvm')
@@ -67,7 +67,7 @@ def test_virtio_vga_virgl(self):
 self.vm.set_console()
 self.vm.add_args("-m", "2G")
 self.vm.add_args("-machine", "pc,accel=kvm")
-self.vm.add_args("-device", "virtio-vga,virgl=on")
+self.vm.add_args("-device", "virtio-vga-gl")
 self.vm.add_args("-display", "egl-headless")
 self.vm.add_args(
 "-kernel",
-- 
2.31.1

[PATCH v1 19/29] contrib/gitdm: add more individual contributor entries.

[Alex Bennée]

Also ensure Li's canonical gmail address is used.

Signed-off-by: Alex Bennée 
Acked-by: Li Qiang 
Acked-by: Chetan Pant 
Acked-by: Akihiko Odaki 
Message-Id: <20210714182056.25888-22-alex.ben...@linaro.org>
---
 contrib/gitdm/aliases   | 3 +++
 contrib/gitdm/group-map-individuals | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/contrib/gitdm/aliases b/contrib/gitdm/aliases
index c6ed215e68..4792413ce7 100644
--- a/contrib/gitdm/aliases
+++ b/contrib/gitdm/aliases
@@ -31,6 +31,9 @@ pbrook@c046a42c-6fe2-441c-8c8c-71466251a162 
p...@codesourcery.com
 ths@c046a42c-6fe2-441c-8c8c-71466251a162 t...@networkno.de
 malc@c046a42c-6fe2-441c-8c8c-71466251a162 av1...@comtv.ru
 
+# canonical emails
+liq...@163.com liq...@gmail.com
+
 # some broken tags
 yuval.shaia.ml.gmail.com yuval.shaia...@gmail.com
 
diff --git a/contrib/gitdm/group-map-individuals 
b/contrib/gitdm/group-map-individuals
index 9b6406e624..f816aa8770 100644
--- a/contrib/gitdm/group-map-individuals
+++ b/contrib/gitdm/group-map-individuals
@@ -31,3 +31,6 @@ jho...@kernel.org
 atar4q...@gmail.com
 minwoo.im@gmail.com
 bmeng...@gmail.com
+liq...@gmail.com
+chetan4wind...@gmail.com
+akihiko.od...@gmail.com
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 21/29] plugins/cache: Fixed a bug with destroying FIFO metadata

[Alex Bennée]

From: Mahmoud Mandour 

This manifests itself when associativity degree is greater than the
number of sets and FIFO is used, otherwise it's also a memory leak
whenever FIFO was used.

Signed-off-by: Mahmoud Mandour 
Reviewed-by: Alex Bennée 
Message-Id: <20210714172151.8494-2-ma.mando...@gmail.com>
Signed-off-by: Alex Bennée 
---
 contrib/plugins/cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index bf0d2f6097..4a71602639 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -200,7 +200,7 @@ static void fifo_destroy(Cache *cache)
 {
 int i;
 
-for (i = 0; i < cache->assoc; i++) {
+for (i = 0; i < cache->num_sets; i++) {
 g_queue_free(cache->sets[i].fifo_queue);
 }
 }
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 24/29] plugins: Fix physical address calculation for IO regions

[Alex Bennée]

From: Aaron Lindsay 

The address calculation for IO regions introduced by

commit 787148bf928a54b5cc86f5b434f9399e9737679c
Author: Aaron Lindsay 
plugins: Expose physical addresses instead of device offsets

is not always accurate. Use the more correct
MemoryRegionSection.offset_within_address_space.

Signed-off-by: Aaron Lindsay 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210720195735.3934473-1-aa...@os.amperecomputing.com>
Signed-off-by: Alex Bennée 
---
 plugins/api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/api.c b/plugins/api.c
index 78b563c5c5..2d521e6ba8 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -319,7 +319,7 @@ uint64_t qemu_plugin_hwaddr_phys_addr(const struct 
qemu_plugin_hwaddr *haddr)
 return block->offset + offset + block->mr->addr;
 } else {
 MemoryRegionSection *mrs = haddr->v.io.section;
-return haddr->v.io.offset + mrs->mr->addr;
+return mrs->offset_within_address_space + haddr->v.io.offset;
 }
 }
 #endif
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 25/29] hw/tricore: fix inclusion of tricore_testboard

[Alex Bennée]

We inadvertently added a symbol clash causing the build not to include
the testboard needed for check-tcg.

Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig")
Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-Id: <20210720114057.32053-2-alex.ben...@linaro.org>
---
 configs/devices/tricore-softmmu/default.mak | 1 +
 hw/tricore/Kconfig  | 3 +--
 hw/tricore/meson.build  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configs/devices/tricore-softmmu/default.mak 
b/configs/devices/tricore-softmmu/default.mak
index 5cc91cebce..cb8fc286eb 100644
--- a/configs/devices/tricore-softmmu/default.mak
+++ b/configs/devices/tricore-softmmu/default.mak
@@ -1 +1,2 @@
+CONFIG_TRICORE_TESTBOARD=y
 CONFIG_TRIBOARD=y
diff --git a/hw/tricore/Kconfig b/hw/tricore/Kconfig
index 506e6183c1..33c1e852c3 100644
--- a/hw/tricore/Kconfig
+++ b/hw/tricore/Kconfig
@@ -1,9 +1,8 @@
-config TRICORE
+config TRICORE_TESTBOARD
 bool
 
 config TRIBOARD
 bool
-select TRICORE
 select TC27X_SOC
 
 config TC27X_SOC
diff --git a/hw/tricore/meson.build b/hw/tricore/meson.build
index 47e36bb077..7e3585daf8 100644
--- a/hw/tricore/meson.build
+++ b/hw/tricore/meson.build
@@ -1,6 +1,6 @@
 tricore_ss = ss.source_set()
-tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testboard.c'))
-tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testdevice.c'))
+tricore_ss.add(when: 'CONFIG_TRICORE_TESTBOARD', if_true: 
files('tricore_testboard.c'))
+tricore_ss.add(when: 'CONFIG_TRICORE_TESTBOARD', if_true: 
files('tricore_testdevice.c'))
 tricore_ss.add(when: 'CONFIG_TRIBOARD', if_true: files('triboard.c'))
 tricore_ss.add(when: 'CONFIG_TC27X_SOC', if_true: files('tc27x_soc.c'))
 
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 14/29] contrib/gitdm: add domain-map for Crudebyte

[Alex Bennée]

Signed-off-by: Alex Bennée 
Reviewed-by: Christian Schoenebeck 
Message-Id: <20210714182056.25888-15-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 5ac8288716..e42861cd11 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -9,6 +9,7 @@ baidu.com   Baidu
 bytedance.com   ByteDance
 cmss.chinamobile.com China Mobile
 citrix.com  Citrix
+crudebyte.com   Crudebyte
 eldorado.org.br Instituto de Pesquisas Eldorado
 fujitsu.com Fujitsu
 google.com  Google
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 27/29] gitlab: enable a very minimal build with the tricore container

[Alex Bennée]

Rather than base of the shared Debian 10 container which would require
us to bring in even more dependencies just bring in what is needed for
building tricore-softmmu in GitLab. We don't even remove the container
from the DOCKER_PARTIAL_IMAGES lest we cause more confusion.

Signed-off-by: Alex Bennée 
Message-Id: <20210720114057.32053-4-alex.ben...@linaro.org>
---
 .gitlab-ci.d/buildtest.yml| 11 ++
 .../dockerfiles/debian-tricore-cross.docker   | 34 ---
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 89df51517c..48cb45a783 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -354,6 +354,17 @@ build-some-softmmu:
 TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
 MAKE_CHECK_ARGS: check-tcg
 
+# We build tricore in a very minimal tricore only container
+build-tricore-softmmu:
+  extends: .native_build_job_template
+  needs:
+job: tricore-debian-cross-container
+  variables:
+IMAGE: debian-tricore-cross
+CONFIGURE_ARGS: --disable-tools --disable-fdt --enable-debug
+TARGETS: tricore-softmmu
+MAKE_CHECK_ARGS: check-tcg
+
 clang-system:
   extends: .native_build_job_template
   needs:
diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker 
b/tests/docker/dockerfiles/debian-tricore-cross.docker
index 985925134c..d8df2c6117 100644
--- a/tests/docker/dockerfiles/debian-tricore-cross.docker
+++ b/tests/docker/dockerfiles/debian-tricore-cross.docker
@@ -1,23 +1,47 @@
 #
 # Docker TriCore cross-compiler target
 #
-# This docker target builds on the debian Stretch base image.
+# This docker target builds on the Debian Buster base image but
+# doesn't inherit from the common one to avoid bringing in unneeded
+# dependencies.
 #
 # Copyright (c) 2018 Philippe Mathieu-Daudé
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
-FROM qemu/debian10
+FROM docker.io/library/debian:buster-slim
 
 MAINTAINER Philippe Mathieu-Daudé 
 
+RUN apt update && \
+DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
+DEBIAN_FRONTEND=noninteractive eatmydata apt install -yy \
+   bzip2 \
+   ca-certificates \
+   ccache \
+   g++ \
+   gcc \
+   git \
+   libglib2.0-dev \
+   libpixman-1-dev \
+   libtest-harness-perl \
+   locales \
+   make \
+   ninja-build \
+   perl-base \
+   pkgconf \
+   python3-pip \
+   python3-setuptools \
+   python3-wheel
+
 RUN git clone --single-branch \
 https://github.com/bkoppelmann/tricore-binutils.git \
 /usr/src/binutils && \
 cd /usr/src/binutils && chmod +x missing && \
-CFLAGS=-w ./configure --prefix=/usr --disable-nls --target=tricore && \
+CFLAGS=-w ./configure --prefix=/usr/local --disable-nls --target=tricore 
&& \
 make && make install && \
 rm -rf /usr/src/binutils
 
-# This image isn't designed for building QEMU but building tests
-ENV QEMU_CONFIGURE_OPTS --disable-system --disable-user
+# This image can only build a very minimal QEMU as well as the tests
+ENV DEF_TARGET_LIST tricore-softmmu
+ENV QEMU_CONFIGURE_OPTS --disable-user --disable-tools --disable-fdt
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 22/29] plugins/cache: limited the scope of a mutex lock

[Alex Bennée]

From: Mahmoud Mandour 

It's not necessary to lock the address translation portion of the
vcpu_mem_access callback.

Signed-off-by: Mahmoud Mandour 
Reviewed-by: Alex Bennée 
Message-Id: <20210714172151.8494-3-ma.mando...@gmail.com>
Signed-off-by: Alex Bennée 
---
 contrib/plugins/cache.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index 4a71602639..695fb969dc 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -355,15 +355,14 @@ static void vcpu_mem_access(unsigned int vcpu_index, 
qemu_plugin_meminfo_t info,
 struct qemu_plugin_hwaddr *hwaddr;
 InsnData *insn;
 
-g_mutex_lock();
 hwaddr = qemu_plugin_get_hwaddr(info, vaddr);
 if (hwaddr && qemu_plugin_hwaddr_is_io(hwaddr)) {
-g_mutex_unlock();
 return;
 }
 
 effective_addr = hwaddr ? qemu_plugin_hwaddr_phys_addr(hwaddr) : vaddr;
 
+g_mutex_lock();
 if (!access_cache(dcache, effective_addr)) {
 insn = (InsnData *) userdata;
 insn->dmisses++;
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 03/29] docs: add a section on the generalities of vhost-user

[Alex Bennée]

While we do mention some of this stuff in the various daemons and
manuals the subtleties of the socket and memory sharing are sometimes
missed. This document attempts to give some background on vhost-user
daemons in general terms.

Signed-off-by: Alex Bennée 
Reviewed-by: Stefan Hajnoczi 
Message-Id: <20210714182056.25888-4-alex.ben...@linaro.org>
---
 docs/interop/vhost-user.rst|  2 +
 docs/system/device-emulation.rst   |  1 +
 docs/system/devices/vhost-user.rst | 59 ++
 3 files changed, 62 insertions(+)
 create mode 100644 docs/system/devices/vhost-user.rst

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d6085f7045..7fc693521e 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1,3 +1,5 @@
+.. _vhost_user_proto:
+
 ===
 Vhost-user Protocol
 ===
diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
index 7af5dbefab..3bebb862b9 100644
--- a/docs/system/device-emulation.rst
+++ b/docs/system/device-emulation.rst
@@ -86,4 +86,5 @@ Emulated Devices
devices/net.rst
devices/nvme.rst
devices/usb.rst
+   devices/vhost-user.rst
devices/virtio-pmem.rst
diff --git a/docs/system/devices/vhost-user.rst 
b/docs/system/devices/vhost-user.rst
new file mode 100644
index 00..86128114fa
--- /dev/null
+++ b/docs/system/devices/vhost-user.rst
@@ -0,0 +1,59 @@
+.. _vhost_user:
+
+vhost-user back ends
+
+
+vhost-user back ends are way to service the request of VirtIO devices
+outside of QEMU itself. To do this there are a number of things
+required.
+
+vhost-user device
+===
+
+These are simple stub devices that ensure the VirtIO device is visible
+to the guest. The code is mostly boilerplate although each device has
+a ``chardev`` option which specifies the ID of the ``--chardev``
+device that connects via a socket to the vhost-user *daemon*.
+
+vhost-user daemon
+=
+
+This is a separate process that is connected to by QEMU via a socket
+following the :ref:`vhost_user_proto`. There are a number of daemons
+that can be built when enabled by the project although any daemon that
+meets the specification for a given device can be used.
+
+Shared memory object
+
+
+In order for the daemon to access the VirtIO queues to process the
+requests it needs access to the guest's address space. This is
+achieved via the ``memory-backend-file`` or ``memory-backend-memfd``
+objects. A reference to a file-descriptor which can access this object
+will be passed via the socket as part of the protocol negotiation.
+
+Currently the shared memory object needs to match the size of the main
+system memory as defined by the ``-m`` argument.
+
+Example
+===
+
+First start you daemon.
+
+.. parsed-literal::
+
+  $ virtio-foo --socket-path=/var/run/foo.sock $OTHER_ARGS
+
+The you start your QEMU instance specifying the device, chardev and
+memory objects.
+
+.. parsed-literal::
+
+  $ |qemu_system| \\
+  -m 4096 \\
+  -chardev socket,id=ba1,path=/var/run/foo.sock \\
+  -device vhost-user-foo,chardev=ba1,$OTHER_ARGS \\
+  -object memory-backend-memfd,id=mem,size=4G,share=on \\
+  -numa node,memdev=mem \\
+...
+
-- 
2.32.0.264.g75ae10bc75

[PULL 7/7] remote/memory: Replace share parameter with ram_flags

[Cleber Rosa]

From: Yang Zhong 

Fixes: d5015b801340 ("softmmu/memory: Pass ram_flags to
qemu_ram_alloc_from_fd()")

Signed-off-by: Yang Zhong 
Reviewed-by: David Hildenbrand 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Pankaj Gupta 
Reviewed-by: Peter Xu 
Message-Id: <20210709052800.63588-1-yang.zh...@intel.com>
Signed-off-by: Cleber Rosa 
---
 hw/remote/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index 472ed2a272..6e21ab1a45 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -46,7 +46,7 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
 subregion = g_new(MemoryRegion, 1);
 memory_region_init_ram_from_fd(subregion, NULL,
name, sysmem_info->sizes[region],
-   true, msg->fds[region],
+   RAM_SHARED, msg->fds[region],
sysmem_info->offsets[region],
errp);
 
-- 
2.31.1

[PULL 6/7] tests/acceptance/virtio-gpu.py: provide kernel and initrd hashes

[Cleber Rosa]

By providing kernel and initrd hashes, the test guarantees the
integrity of the images used and avoids the warnings set by
fetch_asset() when hashes are lacking.

Signed-off-by: Cleber Rosa 
Message-Id: <20210714174051.28164-7-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/virtio-gpu.py | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 0f84affe82..4acc1e6d5f 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -40,11 +40,13 @@ class VirtioGPUx86(Test):
 "/linux/releases/33/Everything/x86_64/os/images"
 "/pxeboot/vmlinuz"
 )
+KERNEL_HASH = '1433cfe3f2ffaa44de4ecfb57ec25dc2399cdecf'
 INITRD_URL = (
 "https://archives.fedoraproject.org/pub/fedora;
 "/linux/releases/33/Everything/x86_64/os/images"
 "/pxeboot/initrd.img"
 )
+INITRD_HASH = 'c828d68a027b53e5220536585efe03412332c2d9'
 
 def wait_for_console_pattern(self, success_message, vm=None):
 wait_for_console_pattern(
@@ -61,8 +63,8 @@ def test_virtio_vga_virgl(self):
 # FIXME: should check presence of virtio, virgl etc
 self.require_accelerator('kvm')
 
-kernel_path = self.fetch_asset(self.KERNEL_URL)
-initrd_path = self.fetch_asset(self.INITRD_URL)
+kernel_path = self.fetch_asset(self.KERNEL_URL, self.KERNEL_HASH)
+initrd_path = self.fetch_asset(self.INITRD_URL, self.INITRD_HASH)
 
 self.vm.set_console()
 self.vm.add_args("-m", "2G")
@@ -100,8 +102,8 @@ def test_vhost_user_vga_virgl(self):
 if not vug:
 self.cancel("Could not find vhost-user-gpu")
 
-kernel_path = self.fetch_asset(self.KERNEL_URL)
-initrd_path = self.fetch_asset(self.INITRD_URL)
+kernel_path = self.fetch_asset(self.KERNEL_URL, self.KERNEL_HASH)
+initrd_path = self.fetch_asset(self.INITRD_URL, self.INITRD_HASH)
 
 # Create socketpair to connect proxy and remote processes
 qemu_sock, vug_sock = socket.socketpair(
-- 
2.31.1

[PATCH v1 23/29] plugins/cache: Fixed "function decl. is not a prototype" warnings

[Alex Bennée]

From: Mahmoud Mandour 

Signed-off-by: Mahmoud Mandour 
Reviewed-by: Alex Bennée 
Message-Id: <20210714172151.8494-7-ma.mando...@gmail.com>
Signed-off-by: Alex Bennée 
---
 contrib/plugins/cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index 695fb969dc..066ea6d8ec 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -469,7 +469,7 @@ static int icmp(gconstpointer a, gconstpointer b)
 return insn_a->imisses < insn_b->imisses ? 1 : -1;
 }
 
-static void log_stats()
+static void log_stats(void)
 {
 g_autoptr(GString) rep = g_string_new("");
 g_string_append_printf(rep,
@@ -487,7 +487,7 @@ static void log_stats()
 qemu_plugin_outs(rep->str);
 }
 
-static void log_top_insns()
+static void log_top_insns(void)
 {
 int i;
 GList *curr, *miss_insns;
@@ -536,7 +536,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
 g_hash_table_destroy(miss_ht);
 }
 
-static void policy_init()
+static void policy_init(void)
 {
 switch (policy) {
 case LRU:
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 13/29] contrib/gitdm: un-ironically add a mapping for LWN

[Alex Bennée]

I think this mainly comes from kernel-doc stuff imported into the QEMU
tree.

Signed-off-by: Alex Bennée 
Cc: Jonathan Corbet 
Message-Id: <20210714182056.25888-14-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 27b8fbdf8a..5ac8288716 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -18,6 +18,7 @@ ibm.com IBM
 igalia.com  Igalia
 intel.com   Intel
 linaro.org  Linaro
+lwn.net LWN
 microsoft.com   Microsoft
 mvista.com  MontaVista
 nokia.com   Nokia
-- 
2.32.0.264.g75ae10bc75

[PULL for 6.1 0/7] Python and Acceptance Tests

[Cleber Rosa]

The following changes since commit c04b4d9e6b596ead3cf6046a9243fbfee068ef33:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2021-07-20 16:59:33 +0100)

are available in the Git repository at:

  https://gitlab.com/cleber.gnu/qemu.git/ tags/python-next-pull-request

for you to fetch changes up to f4a3fda43e389fa26d41ec9cd24f42c5fe20ba9d:

  remote/memory: Replace share parameter with ram_flags (2021-07-20 15:34:20 
-0400)


Acceptance Tests

- Fix for tests/acceptance/virtio-gpu.py to match the change in device
  name
- Fix for failure caught by tests/acceptance/multiprocess.py

PS: While not a maintainer for the subsystem in PATCH 7, I'm including
it as a one-off to facilitate the landing of the fix as discussed in
the mailing list.



Cleber Rosa (6):
  tests/acceptance/virtio-gpu.py: use require_accelerator()
  tests/acceptance/virtio-gpu.py: combine x86_64 arch tags
  tests/acceptance/virtio-gpu.py: combine CPU tags
  tests/acceptance/virtio-gpu.py: combine kernel command line
  tests/acceptance/virtio-gpu.py: use virtio-vga-gl
  tests/acceptance/virtio-gpu.py: provide kernel and initrd hashes

Yang Zhong (1):
  remote/memory: Replace share parameter with ram_flags

 hw/remote/memory.c |  2 +-
 tests/acceptance/virtio-gpu.py | 42 --
 2 files changed, 16 insertions(+), 28 deletions(-)

-- 
2.31.1

[PATCH v1 04/29] configure: remove needless if leg

[Alex Bennée]

It was pointed out in review of the previous patch that the if leg
isn't needed as the for loop will not enter on an empty $device_archs.

Fixes: d1d5e9eefd ("configure: allow the selection of alternate config in the 
build")
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Message-Id: <20210714182056.25888-5-alex.ben...@linaro.org>
---
 configure | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 232c54dcc1..05d96afc17 100755
--- a/configure
+++ b/configure
@@ -5120,12 +5120,10 @@ if test "$skip_meson" = no; then
   echo "[properties]" >> $cross
 
   # unroll any custom device configs
-  if test -n "$device_archs"; then
-  for a in $device_archs; do
-  eval "c=\$devices_${a}"
-  echo "${a}-softmmu = '$c'" >> $cross
-  done
-  fi
+  for a in $device_archs; do
+  eval "c=\$devices_${a}"
+  echo "${a}-softmmu = '$c'" >> $cross
+  done
 
   test -z "$cxx" && echo "link_language = 'c'" >> $cross
   echo "[built-in options]" >> $cross
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 15/29] contrib/gitdm: add domain-map for NVIDIA

[Alex Bennée]

Signed-off-by: Alex Bennée 
Reviewed-by: Kirti Wankhede 
Cc: Yishai Hadas 
Message-Id: <20210714182056.25888-18-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index e42861cd11..2800d9f986 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -24,6 +24,7 @@ microsoft.com   Microsoft
 mvista.com  MontaVista
 nokia.com   Nokia
 nuviainc.comNUVIA
+nvidia.com  NVIDIA
 oracle.com  Oracle
 proxmox.com Proxmox
 quicinc.com Qualcomm Innovation Center
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 18/29] contrib/gitdm: add a new interns group-map for GSoC/Outreachy work

[Alex Bennée]

It makes sense to put our various interns in a group so we can see the
overall impact of GSoC and Outreachy on the project.

Signed-off-by: Alex Bennée 
Reviewed-by: Mahmoud Mandour 
Cc: Ahmed Karaman 
Cc: César Belley 
Message-Id: <20210714182056.25888-21-alex.ben...@linaro.org>
---
 contrib/gitdm/group-map-interns | 13 +
 gitdm.config|  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 contrib/gitdm/group-map-interns

diff --git a/contrib/gitdm/group-map-interns b/contrib/gitdm/group-map-interns
new file mode 100644
index 00..fe33a3231e
--- /dev/null
+++ b/contrib/gitdm/group-map-interns
@@ -0,0 +1,13 @@
+#
+# Group together everyone working as an intern via one of the various
+# outreach programs.
+#
+
+# GSoC 2020 Virtual FIDO/U2F security key
+cesar.bel...@lse.epita.fr
+
+# GSoC 2020 TCG performance
+ahmedkhaledkara...@gmail.com
+
+# GSoC 2021 TCG plugins
+ma.mando...@gmail.com
diff --git a/gitdm.config b/gitdm.config
index 5d5e70fe5f..288b100d89 100644
--- a/gitdm.config
+++ b/gitdm.config
@@ -40,9 +40,10 @@ GroupMap contrib/gitdm/group-map-redhat Red Hat
 GroupMap contrib/gitdm/group-map-wavecomp Wave Computing
 
 # Also group together our prolific individual contributors
-# and those working under academic auspices
+# and those working under academic or intern auspices
 GroupMap contrib/gitdm/group-map-individuals (None)
 GroupMap contrib/gitdm/group-map-academics Academics (various)
+GroupMap contrib/gitdm/group-map-interns GSoC/Outreachy Interns
 
 # Group together robots and other auto-reporters
 GroupMap contrib/gitdm/group-map-robots Robots (various)
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 11/29] contrib/gitdm: add domain-map for Eldorado

[Alex Bennée]

Luis acked on IRC:

  #qemu@znc-oftc_2021-07-13.txt:[15:00:02]  stsquad: "eldorado.org.br 
Eldorado" is fine

Signed-off-by: Alex Bennée 
Acked-by: Luis Pires 
Cc: Bruno Larsen (billionai) 
Message-Id: <20210714182056.25888-12-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index beeb24341e..41875c9e75 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -9,6 +9,7 @@ baidu.com   Baidu
 bytedance.com   ByteDance
 cmss.chinamobile.com China Mobile
 citrix.com  Citrix
+eldorado.org.br Instituto de Pesquisas Eldorado
 fujitsu.com Fujitsu
 google.com  Google
 greensocs.com   GreenSocs
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 08/29] contrib/gitdm: add a group mapping for robot scanners

[Alex Bennée]

This mostly affects Reported-by: tags

Signed-off-by: Alex Bennée 
Message-Id: <20210714182056.25888-9-alex.ben...@linaro.org>
---
 contrib/gitdm/group-map-robots | 7 +++
 gitdm.config   | 3 +++
 2 files changed, 10 insertions(+)
 create mode 100644 contrib/gitdm/group-map-robots

diff --git a/contrib/gitdm/group-map-robots b/contrib/gitdm/group-map-robots
new file mode 100644
index 00..ffd956c2eb
--- /dev/null
+++ b/contrib/gitdm/group-map-robots
@@ -0,0 +1,7 @@
+#
+# There are various automatic robots that occasionally scan and report
+# bugs. Let's group them together here.
+#
+
+# Euler Robot
+euler.ro...@huawei.com
diff --git a/gitdm.config b/gitdm.config
index c01c219078..7378238c20 100644
--- a/gitdm.config
+++ b/gitdm.config
@@ -43,6 +43,9 @@ GroupMap contrib/gitdm/group-map-janustech Janus Technologies
 GroupMap contrib/gitdm/group-map-individuals (None)
 GroupMap contrib/gitdm/group-map-academics Academics (various)
 
+# Group together robots and other auto-reporters
+GroupMap contrib/gitdm/group-map-robots Robots (various)
+
 #
 #
 # Use FileTypeMap to map a file types to file names using regular
-- 
2.32.0.264.g75ae10bc75

Re: [PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface

[Dongwon Kim]

On Sun, Jul 18, 2021 at 11:35:35PM -0700, Kasireddy, Vivek wrote:
> Hi DW,
> 
> > eb_fb_blit needs more parameters which describe x and y offsets and width
> > and height of the actual scanout to specify the size and cordination of
> > partial image to blit in the guest fb in case the guest fb contains multiple
> > display outputs.
> > 
> > Signed-off-by: Dongwon Kim 
> > ---
> >  hw/display/virtio-gpu-udmabuf.c |  4 ++--
> >  include/ui/egl-helpers.h|  2 +-
> >  ui/egl-headless.c   |  2 +-
> >  ui/egl-helpers.c| 10 ++
> >  ui/gtk-egl.c|  7 ---
> >  ui/sdl2-gl.c|  2 +-
> >  6 files changed, 15 insertions(+), 12 deletions(-)
> > 
> > diff --git a/hw/display/virtio-gpu-udmabuf.c 
> > b/hw/display/virtio-gpu-udmabuf.c
> [Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the 
> same patch.
[DW] Yeah, I will split it.

> 
> > index a64194c6de..3ea6e76371 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -186,8 +186,8 @@ static VGPUDMABuf
> >  dmabuf->buf.stride = fb->stride;
> >  dmabuf->buf.x = r->x;
> >  dmabuf->buf.y = r->y;
> > -dmabuf->buf.scanout_width;
> > -dmabuf->buf.scanout_height;
> > +dmabuf->buf.scanout_width = r->width;
> > +dmabuf->buf.scanout_height = r->height;
> >  dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
> >  dmabuf->buf.fd = res->dmabuf_fd;
> > 
> > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> > index f1bf8f97fc..e21118501e 100644
> > --- a/include/ui/egl-helpers.h
> > +++ b/include/ui/egl-helpers.h
> > @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int 
> > height);
> >  void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
> >GLuint texture, bool delete);
> >  void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
> > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
> > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, 
> > bool flip);
> >  void egl_fb_read(DisplaySurface *dst, egl_fb *src);
> > 
> >  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool 
> > flip);
> > diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> > index da377a74af..bdf10fec84 100644
> > --- a/ui/egl-headless.c
> > +++ b/ui/egl-headless.c
> > @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener 
> > *dcl,
> >1.0, 1.0);
> >  } else {
> >  /* no cursor -> use simple framebuffer blit */
> > -egl_fb_blit(>blit_fb, >guest_fb, edpy->y_0_top);
> > +egl_fb_blit(>blit_fb, >guest_fb, x, y, w, h, 
> > edpy->y_0_top);
> >  }
> > 
> >  egl_fb_read(edpy->ds, >blit_fb);
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index 6d0cb2b5cb..2af3dcc0a6 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int 
> > height)
> >  egl_fb_setup_for_tex(fb, width, height, texture, true);
> >  }
> > 
> > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
> > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, 
> > bool flip)
> [Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, 
> would you be not
> be able to use the dmabuf member that would be added to egl_fb that would 
> contain x, y, w and h:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html

[DW] sounds like a valid idea but wouldn't it be making this function
too specific? I think it is reasonable to specify the offset and the
size of blitted area.. although I agree that having too many parameters
don't look good.

> 
> 
> >  {
> >  GLuint y1, y2;
> > 
> >  glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
> >  glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
> >  glViewport(0, 0, dst->width, dst->height);
> > -y1 = flip ? src->height : 0;
> > -y2 = flip ? 0 : src->height;
> > -glBlitFramebuffer(0, y1, src->width, y2,
> > +w = (x + w) > src->width ? src->width - x : w;
> > +h = (y + h) > src->height ? src->height - y : h;
> > +y1 = flip ? h + y : y;
> > +y2 = flip ? y : h + y;
> > +glBlitFramebuffer(x, y1, x + w, y2,
> [Kasireddy, Vivek] While you are at it, could you please create new local 
> variables x1, y1, x2, y2
> to store the above values and pass them to glBlitFramebuffer to improve the 
> readability of this code? 
[DW] I will think about making this look more undertandable.

> 
> Thanks,
> Vivek
> >0, 0, dst->width, dst->height,
> >GL_COLOR_BUFFER_BIT, GL_LINEAR);
> >  }
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > index 2a2e6d3a17..ceb52b1045 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc)
> >  wh = 

[PATCH v1 12/29] contrib/gitdm: add domain-map/group-map for Wind River

[Alex Bennée]

As per discussion at:
  
http://patchwork.ozlabs.org/project/qemu-devel/patch/20201004180443.2035359-19-f4...@amsat.org/

I've added Bin's personal email as an individual contributor.

Signed-off-by: Alex Bennée 
Acked-by: Bin Meng 
Cc: Ruimei Yan 
Cc: Xuzhou Cheng 
Message-Id: <20210714182056.25888-13-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map| 1 +
 contrib/gitdm/group-map-individuals | 1 +
 2 files changed, 2 insertions(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 41875c9e75..27b8fbdf8a 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -34,6 +34,7 @@ suse.comSUSE
 suse.de SUSE
 virtuozzo.com   Virtuozzo
 wdc.com Western Digital
+windriver.com   Wind River
 xilinx.com  Xilinx
 yadro.com   YADRO
 yandex-team.ru  Yandex
diff --git a/contrib/gitdm/group-map-individuals 
b/contrib/gitdm/group-map-individuals
index 4ac2f98823..9b6406e624 100644
--- a/contrib/gitdm/group-map-individuals
+++ b/contrib/gitdm/group-map-individuals
@@ -30,3 +30,4 @@ h...@tuxfamily.org
 jho...@kernel.org
 atar4q...@gmail.com
 minwoo.im@gmail.com
+bmeng...@gmail.com
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 07/29] contrib/gitdm: add domain-map for MontaVista

[Alex Bennée]

Signed-off-by: Alex Bennée 
Acked-by: Corey Minyard 
Message-Id: <20210714182056.25888-8-alex.ben...@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 0074da618f..efbbb15643 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -18,6 +18,7 @@ igalia.com  Igalia
 intel.com   Intel
 linaro.org  Linaro
 microsoft.com   Microsoft
+mvista.com  MontaVista
 nokia.com   Nokia
 nuviainc.comNUVIA
 oracle.com  Oracle
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 01/29] gitignore: Update with some filetypes

[Alex Bennée]

From: Viresh Kumar 

Update .gitignore to ignore .swp and .patch files.

Signed-off-by: Viresh Kumar 
Signed-off-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Message-Id: 
<79262dbe1f7888eb02e1911501eebafa6f2f6400.1616583806.git.viresh.ku...@linaro.org>
Message-Id: <20210714182056.25888-2-alex.ben...@linaro.org>
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 75a4be0724..eb2553026c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,3 +13,5 @@ GTAGS
 *~
 *.ast_raw
 *.depend_raw
+*.swp
+*.patch
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 06/29] .mailmap: fix up some broken commit authors

[Alex Bennée]

Fixes: 49a6f3bffb ("target/arm: Correct the encoding of MDCCSR_EL0 and 
DBGDSCRint")
Fixes: 5a07192a04 ("target/i386: Fix handling of k_gs_base register in 32-bit 
mode in gdbstub")
Signed-off-by: Alex Bennée 
Cc: Nick Hudson 
Cc: Marek Dolata 
Message-Id: <20210714182056.25888-7-alex.ben...@linaro.org>
---
 .mailmap | 4 
 1 file changed, 4 insertions(+)

diff --git a/.mailmap b/.mailmap
index a1bd659817..082ff893ab 100644
--- a/.mailmap
+++ b/.mailmap
@@ -27,6 +27,10 @@ Paul Brook  pbrook 
 ths 

 malc  malc 
 
+# Corrupted Author fields
+Marek Dolata  mkdol...@us.ibm.com 
+Nick Hudson  hn...@vmware.com 
+
 # There is also a:
 #(no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162>
 # for the cvs2svn initialization commit e63c3dc74bf.
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 05/29] contrib/gitdm: add some new aliases to fix up commits

[Alex Bennée]

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Cc: Yuval Shaia 
Message-Id: <20210714182056.25888-6-alex.ben...@linaro.org>
---
 contrib/gitdm/aliases | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/gitdm/aliases b/contrib/gitdm/aliases
index c1e744312f..c6ed215e68 100644
--- a/contrib/gitdm/aliases
+++ b/contrib/gitdm/aliases
@@ -31,6 +31,9 @@ pbrook@c046a42c-6fe2-441c-8c8c-71466251a162 
p...@codesourcery.com
 ths@c046a42c-6fe2-441c-8c8c-71466251a162 t...@networkno.de
 malc@c046a42c-6fe2-441c-8c8c-71466251a162 av1...@comtv.ru
 
+# some broken tags
+yuval.shaia.ml.gmail.com yuval.shaia...@gmail.com
+
 # There is also a:
 #(no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162>
 # for the cvs2svn initialization commit e63c3dc74bf.
-- 
2.32.0.264.g75ae10bc75

[PATCH v1 10/29] contrib/gitdm: add domain-map/group-map mappings for Samsung

[Alex Bennée]

Minwoo's work from their personal address are treated as personal
contributions.

Signed-off-by: Alex Bennée 
Acked-by: Klaus Jensen 
Cc: Gollu Appalanaidu 
Cc: Minwoo Im 
Message-Id: <20210714182056.25888-11-alex.ben...@linaro.org>

---
vPrePr
  - removed extraneous groupmap
---
 contrib/gitdm/domain-map| 1 +
 contrib/gitdm/group-map-individuals | 1 +
 2 files changed, 2 insertions(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index efbbb15643..beeb24341e 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -26,6 +26,7 @@ proxmox.com Proxmox
 quicinc.com Qualcomm Innovation Center
 redhat.com  Red Hat
 rt-rk.com   RT-RK
+samsung.com Samsung
 siemens.com Siemens
 sifive.com  SiFive
 suse.comSUSE
diff --git a/contrib/gitdm/group-map-individuals 
b/contrib/gitdm/group-map-individuals
index 36bbb77c39..4ac2f98823 100644
--- a/contrib/gitdm/group-map-individuals
+++ b/contrib/gitdm/group-map-individuals
@@ -29,3 +29,4 @@ mrol...@gmail.com
 h...@tuxfamily.org
 jho...@kernel.org
 atar4q...@gmail.com
+minwoo.im@gmail.com
-- 
2.32.0.264.g75ae10bc75

[PATCH for 6.1-rc1 v1 00/29] various fixes pre-PR (metadata, docs, plugins, testing)

[Alex Bennée]

Hi,

This is a roll-up of all the various patches I've been posting
targeting the 6.1 bug fixes. So far they include:

  - gitdm metadata updates (dropped un-acked mappings)
  - documentation on driver/device configuration
  - some miscellaneous plugin bug fixes
  - fix and CI test for Tricore (posted earlier today)
  - gitlab tweaks for MacOSX and OpenBSI

Phillipe may have merged his PR with some of the tricore patches by
the time this gets in.
  
The following are still need review:

  - gitlab: enable a very minimal build with the tricore container
  - tests/tcg/configure.sh: add handling for assembler only builds
  - contrib/gitdm: add more individual contributor entries. (3 acks, 1 sobs)
  - contrib/gitdm: un-ironically add a mapping for LWN
  - contrib/gitdm: add domain-map/group-map for Wind River
  - contrib/gitdm: add domain-map for Eldorado
  - contrib/gitdm: add domain-map/group-map mappings for Samsung
  - gitdm.config: sort the corporate GroupMap entries
  - contrib/gitdm: add a group mapping for robot scanners
  - contrib/gitdm: add domain-map for MontaVista
  - .mailmap: fix up some broken commit authors
  - docs: collect the disparate device emulation docs into one section


Aaron Lindsay (1):
  plugins: Fix physical address calculation for IO regions

Alex Bennée (22):
  docs: collect the disparate device emulation docs into one section
  docs: add a section on the generalities of vhost-user
  configure: remove needless if leg
  contrib/gitdm: add some new aliases to fix up commits
  .mailmap: fix up some broken commit authors
  contrib/gitdm: add domain-map for MontaVista
  contrib/gitdm: add a group mapping for robot scanners
  gitdm.config: sort the corporate GroupMap entries
  contrib/gitdm: add domain-map/group-map mappings for Samsung
  contrib/gitdm: add domain-map for Eldorado
  contrib/gitdm: add domain-map/group-map for Wind River
  contrib/gitdm: un-ironically add a mapping for LWN
  contrib/gitdm: add domain-map for Crudebyte
  contrib/gitdm: add domain-map for NVIDIA
  contrib/gitdm: add group-map for Netflix
  contrib/gitdm: add an explicit academic entry for BU
  contrib/gitdm: add a new interns group-map for GSoC/Outreachy work
  contrib/gitdm: add more individual contributor entries.
  tcg/plugins: implement a qemu_plugin_user_exit helper
  hw/tricore: fix inclusion of tricore_testboard
  tests/tcg/configure.sh: add handling for assembler only builds
  gitlab: enable a very minimal build with the tricore container

Mahmoud Mandour (3):
  plugins/cache: Fixed a bug with destroying FIFO metadata
  plugins/cache: limited the scope of a mutex lock
  plugins/cache: Fixed "function decl. is not a prototype" warnings

Philippe Mathieu-Daudé (1):
  gitlab-ci: Extract OpenSBI job rules to reusable section

Thomas Huth (1):
  gitlab-ci: Remove the second superfluous macos task

Viresh Kumar (1):
  gitignore: Update with some filetypes

 docs/interop/vhost-user.rst   |  2 +
 docs/system/device-emulation.rst  | 90 +++
 docs/system/{ => devices}/ivshmem.rst |  0
 docs/system/{ => devices}/net.rst |  0
 docs/system/{ => devices}/nvme.rst|  0
 docs/system/{ => devices}/usb.rst |  0
 docs/system/devices/vhost-user.rst| 59 
 docs/system/{ => devices}/virtio-pmem.rst |  0
 docs/system/index.rst |  6 +-
 configure | 10 +--
 configs/devices/tricore-softmmu/default.mak   |  1 +
 include/qemu/plugin.h | 12 +++
 include/qemu/qemu-plugin.h| 13 +++
 bsd-user/syscall.c|  6 +-
 contrib/plugins/cache.c   | 11 ++-
 linux-user/exit.c |  2 +-
 plugins/api.c |  2 +-
 plugins/core.c| 39 
 .gitignore|  2 +
 .gitlab-ci.d/buildtest.yml| 11 +++
 .gitlab-ci.d/cirrus.yml   | 15 
 .gitlab-ci.d/opensbi.yml  | 28 +++---
 .mailmap  |  4 +
 contrib/gitdm/aliases |  6 ++
 contrib/gitdm/domain-map  |  7 ++
 contrib/gitdm/group-map-academics |  3 +
 contrib/gitdm/group-map-individuals   |  5 ++
 contrib/gitdm/group-map-interns   | 13 +++
 contrib/gitdm/group-map-netflix   |  5 ++
 contrib/gitdm/group-map-robots|  7 ++
 gitdm.config  | 13 ++-
 hw/tricore/Kconfig|  3 +-
 hw/tricore/meson.build|  4 +-
 .../dockerfiles/debian-tricore-cross.docker   | 34 +--
 tests/tcg/configure.sh| 18 
 35 files changed, 370 insertions(+), 61 deletions(-)
 create mode 100644 docs/system/device-emulation.rst
 rename 

Re: [PATCH 1/2] virtio-gpu: splitting one extended mode guest fb into n-scanouts

[Dongwon Kim]

On Sun, Jul 18, 2021 at 11:17:00PM -0700, Kasireddy, Vivek wrote:
> Hi DW,
> 
> > When guest is running Linux/X11 with extended multiple displays mode 
> > enabled,
> > the guest shares one scanout resource each time containing whole surface
> > rather than sharing individual display output separately. This extended 
> > frame
> > is properly splited and rendered on the corresponding scanout surfaces but
> > not in case of blob-resource (zero copy).
> > 
> > This code change lets the qemu split this one large surface data into 
> > multiple
> > in case of blob-resource as well so that each sub frame then can be blitted
> > properly to each scanout.
> > 
> > Signed-off-by: Dongwon Kim 
> > ---
> >  hw/display/virtio-gpu-udmabuf.c | 19 +++
> >  hw/display/virtio-gpu.c |  5 +++--
> >  include/hw/virtio/virtio-gpu.h  |  5 +++--
> >  include/ui/console.h|  4 
> >  stubs/virtio-gpu-udmabuf.c  |  3 ++-
> >  5 files changed, 23 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/display/virtio-gpu-udmabuf.c 
> > b/hw/display/virtio-gpu-udmabuf.c
> > index 3c01a415e7..a64194c6de 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -171,7 +171,8 @@ static VGPUDMABuf
> >  *virtio_gpu_create_dmabuf(VirtIOGPU *g,
> >uint32_t scanout_id,
> >struct virtio_gpu_simple_resource *res,
> > -  struct virtio_gpu_framebuffer *fb)
> > +  struct virtio_gpu_framebuffer *fb,
> > +  struct virtio_gpu_rect *r)
> >  {
> >  VGPUDMABuf *dmabuf;
> > 
> > @@ -183,6 +184,10 @@ static VGPUDMABuf
> >  dmabuf->buf.width = fb->width;
> >  dmabuf->buf.height = fb->height;
> >  dmabuf->buf.stride = fb->stride;
> > +dmabuf->buf.x = r->x;
> > +dmabuf->buf.y = r->y;
> > +dmabuf->buf.scanout_width;
> > +dmabuf->buf.scanout_height;
> [Kasireddy, Vivek] Would you not be able to use buf.width and buf.height?
> What is the difference between these and scanout_width/height?
[DW] buf.width and buf.height is the combined width/height. For example,
if guest has two 1920x1080 w/ extended display setup, buf.width = 3840
and buf.height = 1080. buf.scanout_width and buf.scanout_height are
individual display's, so they are 1920 and 1080 respectively.

> 
> >  dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
> >  dmabuf->buf.fd = res->dmabuf_fd;
> > 
> > @@ -195,24 +200,22 @@ static VGPUDMABuf
> >  int virtio_gpu_update_dmabuf(VirtIOGPU *g,
> >   uint32_t scanout_id,
> >   struct virtio_gpu_simple_resource *res,
> > - struct virtio_gpu_framebuffer *fb)
> > + struct virtio_gpu_framebuffer *fb,
> > + struct virtio_gpu_rect *r)
> >  {
> >  struct virtio_gpu_scanout *scanout = 
> > >parent_obj.scanout[scanout_id];
> >  VGPUDMABuf *new_primary, *old_primary = NULL;
> > 
> > -new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb);
> > +new_primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb, r);
> >  if (!new_primary) {
> >  return -EINVAL;
> >  }
> > 
> >  if (g->dmabuf.primary) {
> > -old_primary = g->dmabuf.primary;
> > +old_primary = g->dmabuf.primary[scanout_id];
> >  }
> > 
> > -g->dmabuf.primary = new_primary;
> > -qemu_console_resize(scanout->con,
> > -new_primary->buf.width,
> > -new_primary->buf.height);
> > +g->dmabuf.primary[scanout_id] = new_primary;
> >  dpy_gl_scanout_dmabuf(scanout->con, _primary->buf);
> > 
> >  if (old_primary) {
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index e183f4ecda..11a87dad79 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -523,9 +523,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
> >  console_has_gl(scanout->con)) {
> >  dpy_gl_update(scanout->con, 0, 0, scanout->width,
> [Kasireddy, Vivek] Unrelated to your change but shouldn't 0,0 be replaced 
> with 
> scanout->x and scanout->y?
[DW] Each scanout's x and y are all 0. Actual offsets are r->s and r->y.

> 
> >scanout->height);
> > -return;
> >  }
> >  }
> > +return;
> >  }
> > 
> >  if (!res->blob &&
> > @@ -598,6 +598,7 @@ static void virtio_gpu_update_scanout(VirtIOGPU *g,
> >  scanout->y = r->y;
> >  scanout->width = r->width;
> >  scanout->height = r->height;
> > +qemu_console_resize(scanout->con, scanout->width, scanout->height);
> [Kasireddy, Vivek] IIRC, there was no qemu_console_resize for the non-blob 
> resources case.
> Moving this call to update_scanout means that it will also be called for 
> non-blob resources
> Path; not 

[PATCH 1/2] modules: Implement new helper functions

[Jose R. Ziviani]

The function module_load_one() fills a hash table with modules that
were successfuly loaded. However, that table is a static variable of
module_load_one(). This patch changes it and creates a function that
informs whether a given module was loaded or not.

It also creates a function that returns the module name given the
typename.

Signed-off-by: Jose R. Ziviani 
---
 include/qemu/module.h |  4 +++
 util/module.c | 57 +--
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 3deac0078b..f45773718d 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -14,6 +14,7 @@
 #ifndef QEMU_MODULE_H
 #define QEMU_MODULE_H
 
+#include 
 
 #define DSO_STAMP_FUN glue(qemu_stamp, CONFIG_STAMP)
 #define DSO_STAMP_FUN_STR stringify(DSO_STAMP_FUN)
@@ -74,6 +75,9 @@ void module_load_qom_one(const char *type);
 void module_load_qom_all(void);
 void module_allow_arch(const char *arch);
 
+bool module_is_loaded(const char *name);
+const char *module_get_name_from_obj(const char *obj);
+
 /**
  * DOC: module info annotation macros
  *
diff --git a/util/module.c b/util/module.c
index 6bb4ad915a..f23adc65bf 100644
--- a/util/module.c
+++ b/util/module.c
@@ -119,6 +119,8 @@ static const QemuModinfo module_info_stub[] = { {
 static const QemuModinfo *module_info = module_info_stub;
 static const char *module_arch;
 
+static GHashTable *loaded_modules;
+
 void module_init_info(const QemuModinfo *info)
 {
 module_info = info;
@@ -206,13 +208,10 @@ static int module_load_file(const char *fname, bool 
mayfail, bool export_symbols
 out:
 return ret;
 }
-#endif
 
 bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
 {
 bool success = false;
-
-#ifdef CONFIG_MODULES
 char *fname = NULL;
 #ifdef CONFIG_MODULE_UPGRADES
 char *version_dir;
@@ -223,7 +222,6 @@ bool module_load_one(const char *prefix, const char 
*lib_name, bool mayfail)
 int i = 0, n_dirs = 0;
 int ret;
 bool export_symbols = false;
-static GHashTable *loaded_modules;
 const QemuModinfo *modinfo;
 const char **sl;
 
@@ -307,12 +305,9 @@ bool module_load_one(const char *prefix, const char 
*lib_name, bool mayfail)
 g_free(dirs[i]);
 }
 
-#endif
 return success;
 }
 
-#ifdef CONFIG_MODULES
-
 static bool module_loaded_qom_all;
 
 void module_load_qom_one(const char *type)
@@ -377,6 +372,39 @@ void qemu_load_module_for_opts(const char *group)
 }
 }
 
+bool module_is_loaded(const char *name)
+{
+if (!loaded_modules || !g_hash_table_contains(loaded_modules, name)) {
+return false;
+}
+
+return true;
+}
+
+const char *module_get_name_from_obj(const char *obj)
+{
+const QemuModinfo *modinfo;
+const char **sl;
+
+if (!obj) {
+return NULL;
+}
+
+for (modinfo = module_info; modinfo->name != NULL; modinfo++) {
+if (!modinfo->objs) {
+continue;
+}
+
+for (sl = modinfo->objs; *sl != NULL; sl++) {
+if (strcmp(obj, *sl) == 0) {
+return modinfo->name;
+}
+}
+}
+
+return NULL;
+}
+
 #else
 
 void module_allow_arch(const char *arch) {}
@@ -384,4 +412,19 @@ void qemu_load_module_for_opts(const char *group) {}
 void module_load_qom_one(const char *type) {}
 void module_load_qom_all(void) {}
 
+bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
+{
+return false;
+}
+
+bool module_is_loaded(const char *name)
+{
+return false;
+}
+
+char *module_get_name_from_obj(const char *obj)
+{
+return NULL;
+}
+
 #endif
-- 
2.32.0

[PATCH 2/2] qom: Improve error message in module_object_class_by_name()

[Jose R. Ziviani]

module_object_class_by_name() calls module_load_qom_one if the object
is provided by a dynamically linked library. Such library might not be
available at this moment - for instance, it can be a package not yet
installed. Thus, instead of assert error messages, this patch outputs
more friendly messages.

Current error messages:
$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
...
ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: 
(ops != NULL)
Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: 
assertion failed: (ops != NULL)
[1]31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...

New error message:
$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
accel-tcg-x86_64 module is missing, install the package or config the library 
path correctly.

Or with other modules, when possible:
$ ./qemu-system-x86_64 -machine q35 -accel kvm -kernel /boot/vmlinuz -vga qxl   
  ✹
hw-display-qxl module is missing, install the package or config the library 
path correctly.
qemu-system-x86_64: QXL VGA not available

$ make check
...
Running test qtest-x86_64/test-filter-mirror
Running test qtest-x86_64/endianness-test
accel-qtest-x86_64 module is missing, install the package or config the library 
path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library 
path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library 
path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library 
path correctly.
accel-qtest-x86_64 module is missing, install the package or config the library 
path correctly.
accel-tcg-x86_64 module is missing, install the package or config the library 
path correctly.
...

Signed-off-by: Jose R. Ziviani 
---
 accel/accel-softmmu.c | 5 -
 qom/object.c  | 9 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f52..52449ac2d0 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -79,7 +79,10 @@ void accel_init_ops_interfaces(AccelClass *ac)
  * all accelerators need to define ops, providing at least a mandatory
  * non-NULL create_vcpu_thread operation.
  */
-g_assert(ops != NULL);
+if (ops == NULL) {
+exit(1);
+}
+
 if (ops->ops_init) {
 ops->ops_init(ops);
 }
diff --git a/qom/object.c b/qom/object.c
index 6a01d56546..3a170ea9df 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/module.h"
 #include "qemu/osdep.h"
 #include "hw/qdev-core.h"
 #include "qapi/error.h"
@@ -1031,8 +1032,16 @@ ObjectClass *module_object_class_by_name(const char 
*typename)
 oc = object_class_by_name(typename);
 #ifdef CONFIG_MODULES
 if (!oc) {
+const char *module_name = module_get_name_from_obj(typename);
 module_load_qom_one(typename);
 oc = object_class_by_name(typename);
+if (!oc && module_name) {
+if (!module_is_loaded(module_name)) {
+fprintf(stderr, "%s module is missing, install the "
+"package or config the library path "
+"correctly.\n", module_name);
+}
+}
 }
 #endif
 return oc;
-- 
2.32.0

[PATCH 0/2] Improve module accelerator error message

[Jose R. Ziviani]

The main objective here is to fix an user issue when trying to load TCG
that was built as module, but it's not installed or found in the library
path.

For example:

$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
...
ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: 
(ops != NULL)
Bail out! ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: 
assertion failed: (ops != NULL)
[1]31964 IOT instruction (core dumped)  ./qemu-system-x86_64 ...

The new error message becomes:

$ ./qemu-system-x86_64 -machine q35 -accel tcg -kernel /boot/vmlinuz
accel-tcg-x86_64 module is missing, install the package or config the library 
path correctly.

Jose R. Ziviani (2):
  modules: Implement new helper functions
  qom: Improve error message in module_object_class_by_name()

 accel/accel-softmmu.c |  5 +++-
 include/qemu/module.h |  4 +++
 qom/object.c  |  9 +++
 util/module.c | 57 +--
 4 files changed, 67 insertions(+), 8 deletions(-)

-- 
2.32.0

Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint

[Philippe Mathieu-Daudé]

On 7/20/21 11:53 PM, Philippe Mathieu-Daudé wrote:
> On 7/20/21 11:08 PM, Richard Henderson wrote:
>> On 7/20/21 10:56 AM, Peter Maydell wrote:
>>> On Tue, 20 Jul 2021 at 20:54, Richard Henderson
>>>  wrote:

 This will allow a breakpoint hack to move out of AVR's translator.

 Signed-off-by: Richard Henderson 
>>>
 diff --git a/cpu.c b/cpu.c
 index 83059537d7..91d9e38acb 100644
 --- a/cpu.c
 +++ b/cpu.c
 @@ -267,8 +267,13 @@ static void breakpoint_invalidate(CPUState *cpu,
 target_ulong pc)
   int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
     CPUBreakpoint **breakpoint)
   {
 +    CPUClass *cc = CPU_GET_CLASS(cpu);
   CPUBreakpoint *bp;

 +    if (cc->gdb_adjust_breakpoint) {
 +    pc = cc->gdb_adjust_breakpoint(cpu, pc);
 +    }
 +
   bp = g_malloc(sizeof(*bp));

   bp->pc = pc;
 @@ -294,8 +299,13 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr
 pc, int flags,
   /* Remove a specific breakpoint.  */
   int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
   {
 +    CPUClass *cc = CPU_GET_CLASS(cpu);
   CPUBreakpoint *bp;

 +    if (cc->gdb_adjust_breakpoint) {
 +    pc = cc->gdb_adjust_breakpoint(cpu, pc);
 +    }
 +
   QTAILQ_FOREACH(bp, >breakpoints, entry) {
   if (bp->pc == pc && bp->flags == flags) {
   cpu_breakpoint_remove_by_ref(cpu, bp);
 -- 
>>>
>>> So previously for AVR we would have considered the bp at 0x100
>>> and the one at 0x800100 as distinct (in the sense that the only way
>>> the gdb remote protocol distinguishes breakpoints is by "what address",
>>> and these have different addresses). After this change, they won't
>>> be distinct, because if you set a bp at 0x100 and 0x800100 and then
>>> try to remove the one at 0x100 we might remove the 0x800100 one,
>>> because we're storing only the adjusted-address, not the one gdb used.

Yes. Looks good.

>>>
>>> This might not matter in practice...
>>
>> I don't think it will matter.
>>
>> Currently, if it sets both 0x100 and 0x800100, then we'll record two
>> breakpoints, and with either we'll raise EXCP_DEBUG when pc == 0x100.
>>
>> Afterward, we'll have two CPUBreakpoint structures that both contain
>> 0x100, and when pc == 0x100 we'll raise EXCP_DEBUG.  If gdb removes the
>> breakpoint at 0x800100, we'll remove one of the two CPUBreakpoint.  But
>> we'll still stop at 0x100, as expected.  When it removes the breakpoint
>> at 0x100, both CPUBreakpoint structures will be gone.
>>
>> In principal, gdb could now add a breakpoint at 0x800100 and remove it
>> with 0x100, where it could not before.  But I don't expect that to
>> happen.  If we reported any kind of status to gdb re the breakpoint
>> insertion or removal (e.g. bp not found), then it might matter, but we
>> don't.

IIUC QEMU model is "hardware breakpoint". I don't know how gdb deals
if user add both soft/hard bp. Neither do I know how many soft/hard
bp are "annouced" via gdbstub monitor to gdb (Alex?). Amusingly
gdb-xml/avr-cpu.xml announces itself as a riscv cpu:



Maybe because there is no official XML declaration for AVR?
https://sourceware.org/gdb/current/onlinedocs/gdb/Standard-Target-Features.html#Standard-Target-Features

>> Practically, this is working around what I'd call a gdb bug wrt avr. 
>> Which may even have been fixed -- I haven't looked.

I agree this won't matter much (and this patch makes it cleaner) so:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/2] ui/gtk: detach_all option for making all VCs detached upon starting

[Dongwon Kim]

On Tue, Jul 20, 2021 at 03:42:16PM +0200, Thomas Huth wrote:
> On 19/07/2021 23.41, Dongwon Kim wrote:
> > With "detach-all=on" for display, all VCs are detached from the beginning.
> > This is useful when there are multiple displays assigned to a guest OS.
> 
> Can you elaborate? (i.e. why is it useful? Do you just want to avoid having
> multiple things opened at startup? Or is there a different reason?)
Hi,

The original motivation is related to an use-case with a guest with
multi-displays. In that use case, we wanted to have all guest displays
placed side by side from beginning. Virtual consoles other than guest
displays (e.g. virtio-gpu-pci) are not actually needed but I found doing
"detach-all" is the simplest way.

> 
> > Signed-off-by: Dongwon Kim 
> > Signed-off-by: Khairul Anuar Romli 
> > ---
> >   qapi/ui.json | 4 +++-
> >   ui/gtk.c | 7 +++
> >   2 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 1052ca9c38..ff14bb2f46 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1141,6 +1141,7 @@
> >   # @show-cursor:   Force showing the mouse cursor (default: off).
> >   # (since: 5.0)
> >   # @gl:Enable OpenGL support (default: off).
> > +# @detach-all:Detatch all VirtualConsoles from beginning (default: 
> > off).
> 
> Needs a comment à la "(since: 6.2)" at the end (like the one after
> "show-cursor" some lines earlier.
> 
> >   #
> >   # Since: 2.12
> >   #
> > @@ -1150,7 +1151,8 @@
> >   '*full-screen'   : 'bool',
> >   '*window-close'  : 'bool',
> >   '*show-cursor'   : 'bool',
> > -'*gl': 'DisplayGLMode' },
> > +'*gl': 'DisplayGLMode',
> > +'*detach-all': 'bool' },
> 
> If this is for GTK only, shouldn't this rather go into DisplayGTK instead?
> Or will this be also useful for other display types later?

This option might not be that useful for other use cases.. but at the
same time, I'm pretty sure this will work universally (won't break
anything..) but for now, I think it's good idea to limit this to GTK.

-DW

> 
>  Thomas
> 
> 
> > 'discriminator' : 'type',
> > 'data': { 'gtk': 'DisplayGTK',
> >   'curses' : 'DisplayCurses',
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index ce885d2ca3..a07e5a049e 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -2211,6 +2211,7 @@ static void gtk_display_init(DisplayState *ds, 
> > DisplayOptions *opts)
> >   GdkDisplay *window_display;
> >   GtkIconTheme *theme;
> >   char *dir;
> > +int i;
> >   if (!gtkinit) {
> >   fprintf(stderr, "gtk initialization failed\n");
> > @@ -2290,6 +2291,12 @@ static void gtk_display_init(DisplayState *ds, 
> > DisplayOptions *opts)
> >   gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
> >   }
> >   gd_clipboard_init(s);
> > +
> > +if (opts->detach_all) {
> > +for (i = 0; i < s->nb_vcs - 1; i++) {
> > +gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
> > +}
> > +}
> >   }
> >   static void early_gtk_display_init(DisplayOptions *opts)
> > 
> 

Re: [PATCH-for-6.1] gitlab-ci: Extract OpenSBI job rules to reusable section

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> All jobs depending on 'docker-opensbi' job must use at most all
> the rules that triggers it. The simplest way to ensure that
> is to always use the same rules. Extract all the rules to a
> reusable section, and include this section (with the 'extends'
> keyword) in both 'docker-opensbi' and 'build-opensbi' jobs.
>
> The problem was introduced in commit c6fc0fc1a71 ("gitlab-ci.yml:
> Add jobs to build OpenSBI firmware binaries"), but was revealed in
> commit 91e9c47e50a ("docker: OpenSBI build job depends on OpenSBI
> container").
>
> This fix is similar to the one used with the EDK2 firmware job in
> commit ac0595cf6b3 ("gitlab-ci: Extract EDK2 job rules to reusable
> section").
>
> Reported-by: Daniel P. Berrangé 
> Signed-off-by: Philippe Mathieu-Daudé 

Queued to for-6.1/fixes-for-rc1, thanks.

-- 
Alex Bennée

Re: [PATCH] plugins: Fix physical address calculation for IO regions

[Alex Bennée]

Aaron Lindsay  writes:

> The address calculation for IO regions introduced by
>
> commit 787148bf928a54b5cc86f5b434f9399e9737679c
> Author: Aaron Lindsay 
> plugins: Expose physical addresses instead of device offsets

Queued to for-6.1/fixes-for-rc1, thanks.

>
> is not always accurate. Use the more correct
> MemoryRegionSection.offset_within_address_space.
> ---
>  plugins/api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plugins/api.c b/plugins/api.c
> index 5c1a413928..ba14e6f2b2 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -319,7 +319,7 @@ uint64_t qemu_plugin_hwaddr_phys_addr(const struct 
> qemu_plugin_hwaddr *haddr)
>  return block->offset + offset + block->mr->addr;
>  } else {
>  MemoryRegionSection *mrs = haddr->v.io.section;
> -return haddr->v.io.offset + mrs->mr->addr;
> +return mrs->offset_within_address_space + haddr->v.io.offset;
>  }
>  }
>  #endif


-- 
Alex Bennée

Re: [PATCH] gitlab-ci: Remove the second superfluous macos task

[Alex Bennée]

Thomas Huth  writes:

> While there might have been bigger differnces between the -base and
> the -xcode images in the beginning, they almost vanished in the
> current builds, e.g. when comparing the output of the "configure"
> step after cleaning up the differences due to temporary path names,
> I only get:

Queued to for-6.1/fixes-for-rc1, thanks.

-- 
Alex Bennée

[PATCH 1/1] modules: Option to build native TCG with --enable-modules

[Jose R. Ziviani]

Adds an option (--enable-tcg-builtin) to build TCG natively when
--enable-modules argument is passed to the build system. It gives
the opportunity to have this important accelerator built-in and
still take advantage of the new modular system.

Signed-off-by: Jose R. Ziviani 
---
 configure | 12 ++--
 meson.build   | 11 ++-
 meson_options.txt |  2 ++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 232c54dcc1..64d7a909ce 100755
--- a/configure
+++ b/configure
@@ -345,6 +345,7 @@ tsan="no"
 fortify_source="$default_feature"
 strip_opt="yes"
 tcg_interpreter="false"
+tcg_builtin="false"
 bigendian="no"
 mingw32="no"
 gcov="no"
@@ -1120,6 +1121,8 @@ for opt do
   ;;
   --enable-tcg) tcg="enabled"
   ;;
+  --enable-tcg-builtin) tcg_builtin="true"
+  ;;
   --disable-malloc-trim) malloc_trim="disabled"
   ;;
   --enable-malloc-trim) malloc_trim="enabled"
@@ -1817,6 +1820,7 @@ Advanced options (experts only):
Default:trace-
   --disable-slirp  disable SLIRP userspace network connectivity
   --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, 
experimental and slow)
+  --enable-tcg-builtin force TCG builtin even with --enable-modules
   --enable-malloc-trim enable libc malloc_trim() for memory optimization
   --oss-libpath to OSS library
   --cpu=CPUBuild for host CPU [$cpu]
@@ -2318,7 +2322,11 @@ if test "$solaris" = "yes" ; then
   fi
 fi
 
-if test "$tcg" = "enabled"; then
+if test "$tcg" = "disabled"; then
+debug_tcg="no"
+tcg_interpreter="false"
+tcg_builtin="false"
+else
 git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
 git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
 fi
@@ -5229,7 +5237,7 @@ if test "$skip_meson" = no; then
 -Dvhost_user_blk_server=$vhost_user_blk_server 
-Dmultiprocess=$multiprocess \
 -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek 
-Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
 $(if test "$default_features" = no; then echo 
"-Dauto_features=disabled"; fi) \
-   -Dtcg_interpreter=$tcg_interpreter \
+-Dtcg_interpreter=$tcg_interpreter -Dtcg_builtin=$tcg_builtin \
 $cross_arg \
 "$PWD" "$source_path"
 
diff --git a/meson.build b/meson.build
index 2f377098d7..2909043aab 100644
--- a/meson.build
+++ b/meson.build
@@ -93,9 +93,13 @@ if cpu in ['x86', 'x86_64']
 endif
 
 modular_tcg = []
+is_tcg_modular = false
 # Darwin does not support references to thread-local variables in modules
 if targetos != 'darwin'
   modular_tcg = ['i386-softmmu', 'x86_64-softmmu']
+  is_tcg_modular = config_host.has_key('CONFIG_MODULES') \
+   and get_option('tcg').enabled() \
+   and not get_option('tcg_builtin')
 endif
 
 edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 
'x86_64-softmmu' ]
@@ -279,6 +283,9 @@ if not get_option('tcg').disabled()
 
   accelerators += 'CONFIG_TCG'
   config_host += { 'CONFIG_TCG': 'y' }
+  if is_tcg_modular
+config_host += { 'CONFIG_TCG_MODULAR': 'y' }
+  endif
 endif
 
 if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
@@ -1567,7 +1574,7 @@ foreach target : target_dirs
   elif sym == 'CONFIG_XEN' and have_xen_pci_passthrough
 config_target += { 'CONFIG_XEN_PCI_PASSTHROUGH': 'y' }
   endif
-  if target in modular_tcg
+  if target in modular_tcg and is_tcg_modular
 config_target += { 'CONFIG_TCG_MODULAR': 'y' }
   else
 config_target += { 'CONFIG_TCG_BUILTIN': 'y' }
@@ -2976,6 +2983,8 @@ summary_info += {'TCG support':   
config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
   if get_option('tcg_interpreter')
 summary_info += {'TCG backend':   'TCI (TCG with bytecode interpreter, 
experimental and slow)'}
+  elif is_tcg_modular
+summary_info += {'TCG backend':   'module (@0@)'.format(cpu)}
   else
 summary_info += {'TCG backend':   'native (@0@)'.format(cpu)}
   endif
diff --git a/meson_options.txt b/meson_options.txt
index a9a9b8f4c6..c27749b864 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -43,6 +43,8 @@ option('tcg', type: 'feature', value: 'auto',
description: 'TCG support')
 option('tcg_interpreter', type: 'boolean', value: false,
description: 'TCG with bytecode interpreter (experimental and slow)')
+option('tcg_builtin', type: 'boolean', value: 'false',
+   description: 'Force TCG builtin')
 option('cfi', type: 'boolean', value: 'false',
description: 'Control-Flow Integrity (CFI)')
 option('cfi_debug', type: 'boolean', value: 'false',
-- 
2.32.0

[PATCH 0/1]

[Jose R. Ziviani]

Hello!

This patch gives the ability to build TCG builtin even if
--enable-modules is selected. This is useful to have a base
QEMU with TCG native product but still using the benefits of
modules.

Thank you!

Jose R. Ziviani (1):
  modules: Option to build native TCG with --enable-modules

 configure | 12 ++--
 meson.build   | 11 ++-
 meson_options.txt |  2 ++
 3 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.32.0

Re: [PATCH for-6.1 v6 15/17] accel/tcg: Remove TranslatorOps.breakpoint_check

[Philippe Mathieu-Daudé]

On 7/20/21 9:54 PM, Richard Henderson wrote:
> The hook is now unused, with breakpoints checked outside translation.
> 
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/translator.h | 11 ---
>  target/arm/helper.h   |  2 --
>  target/alpha/translate.c  | 16 
>  target/arm/debug_helper.c |  7 ---
>  target/arm/translate-a64.c| 25 -
>  target/arm/translate.c| 29 -
>  target/avr/translate.c| 18 --
>  target/cris/translate.c   | 20 
>  target/hexagon/translate.c| 17 -
>  target/hppa/translate.c   | 11 ---
>  target/i386/tcg/translate.c   | 28 
>  target/m68k/translate.c   | 18 --
>  target/microblaze/translate.c | 18 --
>  target/mips/tcg/translate.c   | 19 ---
>  target/nios2/translate.c  | 27 ---
>  target/openrisc/translate.c   | 17 -
>  target/ppc/translate.c| 18 --
>  target/riscv/translate.c  | 17 -
>  target/rx/translate.c | 14 --
>  target/s390x/tcg/translate.c  | 24 
>  target/sh4/translate.c| 18 --
>  target/sparc/translate.c  | 17 -
>  target/tricore/translate.c| 16 
>  target/xtensa/translate.c | 17 -
>  24 files changed, 424 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH for-6.1 v6 12/17] target/avr: Implement gdb_adjust_breakpoint

[Philippe Mathieu-Daudé]

On 7/20/21 9:54 PM, Richard Henderson wrote:
> Ensure at registration that all breakpoints are in
> code space, not data space.
> 
> Signed-off-by: Richard Henderson 
> ---
>  target/avr/cpu.h   |  1 +
>  target/avr/cpu.c   |  1 +
>  target/avr/gdbstub.c   | 13 +
>  target/avr/translate.c | 14 --
>  4 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> index d148e8c75a..93e3faa0a9 100644
> --- a/target/avr/cpu.h
> +++ b/target/avr/cpu.h
> @@ -162,6 +162,7 @@ hwaddr avr_cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> addr);
>  int avr_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int avr_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  int avr_print_insn(bfd_vma addr, disassemble_info *info);
> +vaddr avr_cpu_gdb_adjust_breakpoint(CPUState *cpu, vaddr addr);
>  
>  static inline int avr_feature(CPUAVRState *env, AVRFeature feature)
>  {
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 57e3fab4a0..ea14175ca5 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -223,6 +223,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void 
> *data)
>  cc->disas_set_info = avr_cpu_disas_set_info;
>  cc->gdb_read_register = avr_cpu_gdb_read_register;
>  cc->gdb_write_register = avr_cpu_gdb_write_register;
> +cc->gdb_adjust_breakpoint = avr_cpu_gdb_adjust_breakpoint;
>  cc->gdb_num_core_regs = 35;
>  cc->gdb_core_xml_file = "avr-cpu.xml";
>  cc->tcg_ops = _tcg_ops;
> diff --git a/target/avr/gdbstub.c b/target/avr/gdbstub.c
> index c28ed67efe..1c1b908c92 100644
> --- a/target/avr/gdbstub.c
> +++ b/target/avr/gdbstub.c
> @@ -82,3 +82,16 @@ int avr_cpu_gdb_write_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>  
>  return 0;
>  }
> +
> +vaddr avr_cpu_gdb_adjust_breakpoint(CPUState *cpu, vaddr addr)
> +{
> +/*
> + * This is due to some strange GDB behavior
> + * Let's assume main has address 0x100:
> + * b main   - sets breakpoint at address 0x0100 (code)

I'd say hardware breakpoint is used here (using the Breakpoint
Unit via JTAG),

> + * b *0x100 - sets breakpoint at address 0x00800100 (data)

while software breakpoint is used here (insert a BREAK instruction
at that address).

> + *
> + * Force all breakpoints into code space.
> + */
> +return addr % OFFSET_DATA;
> +}

>From ATmega640 DS:

The Break Point Unit implements Break on Change of Program Flow,
Single Step Break, two Program Memory Break Points, and two combined
Break Points. Together, the four Break Points can be configured as
either:
  • 4 single Program Memory Break Points

  • 3 Single Program Memory Break Points
+ 1 single Data Memory Break Point

  • 2 single Program Memory Break Points
+ 2 single Data Memory Break Points

  • 2 single Program Memory Break Points
+ 1 Program Memory Break Point with mask (“range Break Point”)

  • 2 single Program Memory Break Points
+ 1 Data Memory Break Point with mask (“range Break Point”)

A debugger, like the AVR Studio, may however use one or more of
these resources for its internal purpose, leaving less flexibility
to the end-user.

[...]

All necessary execution commands are available in AVR Studio, both
on source level and on disassembly level.
The user can execute the program, single step through the code either
by tracing into or stepping over functions, step out of functions,
place the cursor on a statement and execute until the statement is
reached, stop the execution, and reset the execution target.
In addition, the user can have an unlimited number of code Break
Points (using the BREAK instruction) and up to two data memory Break
Points, alternatively combined as a mask (range) Break Point.

I wish we didn't have to add gdb_adjust_breakpoint() but we can
remove it later, so for this patch:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 11/16] microvm: Drop dead error handling in microvm_machine_state_init()

[Pankaj Gupta]

> Stillborn in commit 0ebf007dda "hw/i386: Introduce the microvm machine
> type".
>
> Cc: Sergio Lopez 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/i386/microvm.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index aba0c83219..f257ec5a0b 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -458,15 +458,10 @@ static void microvm_machine_state_init(MachineState 
> *machine)
>  {
>  MicrovmMachineState *mms = MICROVM_MACHINE(machine);
>  X86MachineState *x86ms = X86_MACHINE(machine);
> -Error *local_err = NULL;
>
>  microvm_memory_init(mms);
>
>  x86_cpus_init(x86ms, CPU_VERSION_LATEST);
> -if (local_err) {
> -error_report_err(local_err);
> -exit(1);
> -}
>
>  microvm_devices_init(mms);
>  }
> --
> 2.31.1

Reviewed-by: Pankaj Gupta 

Re: [PATCH 16/16] vl: Don't continue after -smp help.

[Pankaj Gupta]

> We continue after -smp help:
>
> $ qemu-system-x86_64 -smp help -display none -monitor stdio
> smp-opts options:
>   cores=
>   cpus=
>   dies=
>   maxcpus=
>   sockets=
>   threads=
> QEMU 6.0.50 monitor - type 'help' for more information
> (qemu)
>
> Other options, such as -object help and -device help, don't.
>
> Adjust -smp not to continue either.
>
> Cc: Paolo Bonzini 
> Signed-off-by: Markus Armbruster 
> ---
>  softmmu/vl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index ce0ecc736b..8f9d97635a 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1543,7 +1543,7 @@ machine_parse_property_opt(QemuOptsList *opts_list, 
> const char *propname,
>  prop = keyval_parse(arg, opts_list->implied_opt_name, , 
> _fatal);
>  if (help) {
>  qemu_opts_print_help(opts_list, true);
> -return;
> +exit(0);
>  }
>  opts = qdict_new();
>  qdict_put(opts, propname, prop);
> --

Reviewed-by: Pankaj Gupta 

Re: [PATCH 10/16] migration: Handle migration_incoming_setup() errors consistently

[Pankaj Gupta]

> Commit b673eab4e2 "multifd: Make multifd_load_setup() get an Error
> parameter" changed migration_incoming_setup() to take an Error **
> argument, and adjusted the callers accordingly.  It neglected to
> change adjust multifd_load_setup(): it still exit()s on error.  Clean
> that up.
>
> The error now gets propagated up two call chains: via
> migration_fd_process_incoming() to rdma_accept_incoming_migration(),
> and via migration_ioc_process_incoming() to
> migration_channel_process_incoming().  Both chain ends report the
> error with error_report_err(), but otherwise ignore it.  Behavioral
> change: we no longer exit() on this error.
>
> This is consistent with how we handle other errors here, e.g. from
> multifd_recv_new_channel() via migration_ioc_process_incoming() to
> migration_channel_process_incoming().  Wether it's consistently right
> or consistently wrong I can't tell.
>
> Also clean up the return value from the unusual 0 on success, 1 on
> error to the more common true on success, false on error.
>
> Cc: Juan Quintela 
> Cc: Dr. David Alan Gilbert 
> Signed-off-by: Markus Armbruster 
> ---
>  migration/migration.c | 27 +--
>  1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 231dc24414..c1c0a48647 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -609,30 +609,25 @@ fail:
>  }
>
>  /**
> - * @migration_incoming_setup: Setup incoming migration
> - *
> - * Returns 0 for no error or 1 for error
> - *
> + * migration_incoming_setup: Setup incoming migration
>   * @f: file for main migration channel
>   * @errp: where to put errors
> + *
> + * Returns: %true on success, %false on error.
>   */
> -static int migration_incoming_setup(QEMUFile *f, Error **errp)
> +static bool migration_incoming_setup(QEMUFile *f, Error **errp)
>  {
>  MigrationIncomingState *mis = migration_incoming_get_current();
> -Error *local_err = NULL;
>
> -if (multifd_load_setup(_err) != 0) {
> -/* We haven't been able to create multifd threads
> -   nothing better to do */
> -error_report_err(local_err);
> -exit(EXIT_FAILURE);
> +if (multifd_load_setup(errp) != 0) {
> +return false;
>  }
>
>  if (!mis->from_src_file) {
>  mis->from_src_file = f;
>  }
>  qemu_file_set_blocking(f, false);
> -return 0;
> +return true;
>  }
>
>  void migration_incoming_process(void)
> @@ -675,14 +670,11 @@ static bool postcopy_try_recover(QEMUFile *f)
>
>  void migration_fd_process_incoming(QEMUFile *f, Error **errp)
>  {
> -Error *local_err = NULL;
> -
>  if (postcopy_try_recover(f)) {
>  return;
>  }
>
> -if (migration_incoming_setup(f, _err)) {
> -error_propagate(errp, local_err);
> +if (!migration_incoming_setup(f, errp)) {
>  return;
>  }
>  migration_incoming_process();
> @@ -703,8 +695,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, 
> Error **errp)
>  return;
>  }
>
> -if (migration_incoming_setup(f, _err)) {
> -error_propagate(errp, local_err);
> +if (!migration_incoming_setup(f, errp)) {
>  return;
>  }
>

Reviewed-by: Pankaj Gupta 

Re: [RFC PATCH 2/6] i386/sev: extend sev-guest property to include SEV-SNP

[Daniel P . Berrangé]

On Tue, Jul 20, 2021 at 02:42:12PM -0500, Michael Roth wrote:
> On Tue, Jul 13, 2021 at 03:46:19PM +0200, Markus Armbruster wrote:
> > Brijesh Singh  writes:
> > 
> > > To launch the SEV-SNP guest, a user can specify up to 8 parameters.
> > > Passing all parameters through command line can be difficult. To simplify
> > > the launch parameter passing, introduce a .ini-like config file that can 
> > > be
> > > used for passing the parameters to the launch flow.
> > >
> > > The contents of the config file will look like this:
> > >
> > > $ cat snp-launch.init
> > >
> > > # SNP launch parameters
> > > [SEV-SNP]
> > > init_flags = 0
> > > policy = 0x1000
> > > id_block = "YWFhYWFhYWFhYWFhYWFhCg=="
> > >
> > >
> > > Add 'snp' property that can be used to indicate that SEV guest launch
> > > should enable the SNP support.
> > >
> > > SEV-SNP guest launch examples:
> > >
> > > 1) launch without additional parameters
> > >
> > >   $(QEMU_CLI) \
> > > -object sev-guest,id=sev0,snp=on
> > >
> > > 2) launch with optional parameters
> > >   $(QEMU_CLI) \
> > > -object sev-guest,id=sev0,snp=on,launch-config=
> > >
> > > Signed-off-by: Brijesh Singh 
> > 
> > I acknowledge doing complex configuration on the command line can be
> > awkward.  But if we added a separate configuration file for every
> > configurable thing where that's the case, we'd have too many already,
> > and we'd constantly grow more.  I don't think this is a viable solution.
> > 
> > In my opinion, much of what we do on the command line should be done in
> > configuration files instead.  Not in several different configuration
> > languages, mind, but using one common language for all our configuration
> > needs.
> > 
> > Some of us argue this language already exists: QMP.  It can't do
> > everything the command line can do, but that's a matter of putting in
> > the work.  However, JSON isn't a good configuration language[1].  To get
> > a decent one, we'd have to to extend JSON[2], or wrap another concrete
> > syntax around QMP's abstract syntax.
> > 
> > But this doesn't help you at all *now*.
> > 
> > I recommend to do exactly what we've done before for complex
> > configuration: define it in the QAPI schema, so we can use both dotted
> > keys and JSON on the command line, and can have QMP, too.  Examples:
> > -blockdev, -display, -compat.
> > 
> > Questions?
> 
> Hi Markus, Daniel,
> 
> I'm dealing with similar considerations with some SNP config options
> relating to CPUID enforcement, so I've started looking into this as
> well, but am still a little confused on the best way to proceed.
> 
> I see that -blockdev supports both JSON command-line arguments (via
> qobject_input_visitor_new) and dotted keys
> (via qobject_input_vistior_new_keyval).
> 
> We could introduce a new config group do the same, maybe something specific
> to ConfidentialGuestSupport objects, e.g.:
> 
>   -confidential-guest-support sev-guest,id=sev0,key_a.subkey_b=...

We don't wnt to be adding new CLI options anymore. -object with json
syntx should ultimately be able to cover everything we'll ever need
to do.

> 
> and use the same mechanisms to parse the options, but this seems to
> either involve adding a layer of option translations between command-line
> and the underlying object properties, or, if we keep the 1:1 mapping
> between QAPI-defined keys and object properties, it basically becomes a
> way to pass a different Visitor implementation into object_property_set(),
> in this case one created by object_input_visitor_new_keyval() instead of
> opts_visitor_new().
> 
> In either case, genericizing it beyond CGS/SEV would basically be
> introducing:
> 
>   -object2 sev-guest,id=sev0,key_a.subkey_b=...
>
> Which one seems preferable? Or is the answer neither?

Yep, neither is the answer.

> 
> I've also been looking at whether this could all be handled via -object,
> and it seems -object already supports JSON command-line arguments, and that
> switching it from using OptsVisitor to QObjectVisitor for non-JSON case
> would be enough to have it handle dotted keys, but I'm not sure what the
> fall-out would be compatibility-wise.
> 
> All lot of that falls under making sure the QObject/keyval parser is
> compatible with existing command-lines parsed via OptsVisitor. One example
> where there still seems to be a difference is lack of support for ranges
> such as "cpus=1-4" in keyval parser. Daniel had a series that addressed
> this:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg08248.html
> 
> but it doesn't seem to have made it into the tree, which is why I feel like
> maybe there are complications with this approach I haven't considered?

The core problem with QemuOpts is that it has hacked various hacks
grafted onto it to cope with non-scalar data. My patch was adding
yet another hack. It very hard to introduce new hacks without risk
of causing incompatibility with other existing hacks, or introducing
unexpected edge cases that we don't anticipate.

Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint

[Philippe Mathieu-Daudé]

On 7/20/21 11:08 PM, Richard Henderson wrote:
> On 7/20/21 10:56 AM, Peter Maydell wrote:
>> On Tue, 20 Jul 2021 at 20:54, Richard Henderson
>>  wrote:
>>>
>>> This will allow a breakpoint hack to move out of AVR's translator.
>>>
>>> Signed-off-by: Richard Henderson 
>>
>>> diff --git a/cpu.c b/cpu.c
>>> index 83059537d7..91d9e38acb 100644
>>> --- a/cpu.c
>>> +++ b/cpu.c
>>> @@ -267,8 +267,13 @@ static void breakpoint_invalidate(CPUState *cpu,
>>> target_ulong pc)
>>>   int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
>>>     CPUBreakpoint **breakpoint)
>>>   {
>>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>>>   CPUBreakpoint *bp;
>>>
>>> +    if (cc->gdb_adjust_breakpoint) {
>>> +    pc = cc->gdb_adjust_breakpoint(cpu, pc);
>>> +    }
>>> +
>>>   bp = g_malloc(sizeof(*bp));
>>>
>>>   bp->pc = pc;
>>> @@ -294,8 +299,13 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr
>>> pc, int flags,
>>>   /* Remove a specific breakpoint.  */
>>>   int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
>>>   {
>>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>>>   CPUBreakpoint *bp;
>>>
>>> +    if (cc->gdb_adjust_breakpoint) {
>>> +    pc = cc->gdb_adjust_breakpoint(cpu, pc);
>>> +    }
>>> +
>>>   QTAILQ_FOREACH(bp, >breakpoints, entry) {
>>>   if (bp->pc == pc && bp->flags == flags) {
>>>   cpu_breakpoint_remove_by_ref(cpu, bp);
>>> -- 
>>
>> So previously for AVR we would have considered the bp at 0x100
>> and the one at 0x800100 as distinct (in the sense that the only way
>> the gdb remote protocol distinguishes breakpoints is by "what address",
>> and these have different addresses). After this change, they won't
>> be distinct, because if you set a bp at 0x100 and 0x800100 and then
>> try to remove the one at 0x100 we might remove the 0x800100 one,
>> because we're storing only the adjusted-address, not the one gdb used.
>>
>> This might not matter in practice...
> 
> I don't think it will matter.
> 
> Currently, if it sets both 0x100 and 0x800100, then we'll record two
> breakpoints, and with either we'll raise EXCP_DEBUG when pc == 0x100.
> 
> Afterward, we'll have two CPUBreakpoint structures that both contain
> 0x100, and when pc == 0x100 we'll raise EXCP_DEBUG.  If gdb removes the
> breakpoint at 0x800100, we'll remove one of the two CPUBreakpoint.  But
> we'll still stop at 0x100, as expected.  When it removes the breakpoint
> at 0x100, both CPUBreakpoint structures will be gone.
> 
> In principal, gdb could now add a breakpoint at 0x800100 and remove it
> with 0x100, where it could not before.  But I don't expect that to
> happen.  If we reported any kind of status to gdb re the breakpoint
> insertion or removal (e.g. bp not found), then it might matter, but we
> don't.
> 
> Practically, this is working around what I'd call a gdb bug wrt avr. 
> Which may even have been fixed -- I haven't looked.

This is not a bug but a feature to deal with the Harvard architecture.
QEMU AVR model is based on GCC sources so uses the same "feature".

The AVR core has 2 address spaces: "CODE" and "DATA". An address space
is always zero-based (so both are). To avoid having to deal with
relocation of symbols from different AS but having same address, the
DATA space is mapped at 0x80 (bit 23 is "virtual" as inexistant
- masked - from the CODE AS).

The core can not execute from DATA, so CPUBreakpoint can only be
triggered from CODE.

I once implemented different AS but switched to smth else :/
It was working but for some reason I couldn't remove the
OFFSET_DATA / OFFSET_CODE definitions, I don't remember &
should respin... See
https://gitlab.com/philmd/qemu/-/compare/avr_gsoc_v1a...avr_gsoc_v1b

Extract of the patches to show the idea:

diff --git a/target/avr/cpu.h b/target/avr/cpu.h
+/* Indexes used when registering address spaces with
cpu_address_space_init */
+typedef enum AVRASIdx {
+AVRASIdx_CODE = 0,
+AVRASIdx_DATA = 1,
+} AVRASIdx;

diff --git a/target/avr/cpu.c b/target/avr/cpu.c
@@ -96,6 +98,13 @@ static void avr_cpu_realizefn(DeviceState *dev, Error
**errp)
 error_propagate(errp, local_err);
 return;
 }
+
+cs->num_ases = 2;
+cpu_address_space_init(cs, AVRASIdx_CODE, "cpu-program-bus",
+   get_program_memory());
+cpu_address_space_init(cs, AVRASIdx_DATA, "cpu-data-bus",
+   get_data_memory());
+
 qemu_init_vcpu(cs);
 cpu_reset(cs);

diff --git a/target/avr/helper.c b/target/avr/helper.c
-/*
- * This function implements IN instruction
- *
- * It does the following
- * a.  if an IO register belongs to CPU, its value is read and returned
- * b.  otherwise io address is translated to mem address and physical
memory
- * is read.
- * c.  it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
- *
- */
-target_ulong helper_inb(CPUAVRState *env, uint32_t port)
+static uint8_t data_read(CPUAVRState *env, uint32_t addr)
 {
-   

Re: [PATCH for-6.1 v6 00/17] tcg: breakpoint reorg

[Mark Cave-Ayland]

On 20/07/2021 20:54, Richard Henderson wrote:


This is fixing #404 ("windows xp boot takes much longer...")
and several other similar reports.

Changes for v6:
   * Reinstate accidental loss of singlestep overriding breakpoint check.
 Shows up in the record-replay avocado tests failing to make progress.
   * Add CPUState.gdb_adjust_breakpoint hook for AVR weirdness.

Changes for v5:
   * Include missing hunk in tb_gen_code, as noted in reply to v4.
   * Remove helper_check_breakpoints from target/arm/.
   * Reorg cflags_for_breakpoints into check_for_breakpoints;
 reorg cpu_exec to use a break instead of a longjmp.
   * Move singlestep_enabled check from cflags_for_breakpoints
 to curr_cflags, which makes cpu_exec_step_atomic cleaner.

Changes for v4:
   * Issue breakpoints directly from cflags_for_breakpoints.
 Do not generate code for a TB beginning with a BP at all.
   * Drop the problematic TranslatorOps.breakpoint_check hook entirely.

Changes for v3:
   * Map CF_COUNT_MASK == 0 -> TCG_MAX_INSNS.
   * Split out *_breakpoint_check fixes for avr, mips, riscv.

Changes for v2:
   * All prerequisites and 7 of the patches from v1 with are merged.

Patches lacking review:
   11-hw-core-Introduce-CPUClass.gdb_adjust_breakpoint.patch
   12-target-avr-Implement-gdb_adjust_breakpoint.patch
   15-accel-tcg-Remove-TranslatorOps.breakpoint_check.patch
   17-accel-tcg-Record-singlestep_enabled-in-tb-cflags.patch


r~


Richard Henderson (17):
   accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
   accel/tcg: Move curr_cflags into cpu-exec.c
   target/alpha: Drop goto_tb path in gen_call_pal
   accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR
   accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain
   accel/tcg: Handle -singlestep in curr_cflags
   accel/tcg: Use CF_NO_GOTO_{TB, PTR} in cpu_exec_step_atomic
   hw/core: Introduce TCGCPUOps.debug_check_breakpoint
   target/arm: Implement debug_check_breakpoint
   target/i386: Implement debug_check_breakpoint
   hw/core: Introduce CPUClass.gdb_adjust_breakpoint
   target/avr: Implement gdb_adjust_breakpoint
   accel/tcg: Merge tb_find into its only caller
   accel/tcg: Move breakpoint recognition outside translation
   accel/tcg: Remove TranslatorOps.breakpoint_check
   accel/tcg: Hoist tb_cflags to a local in translator_loop
   accel/tcg: Record singlestep_enabled in tb->cflags

  include/exec/exec-all.h   |  24 ++--
  include/exec/translator.h |  11 --
  include/hw/core/cpu.h |   4 +
  include/hw/core/tcg-cpu-ops.h |   6 +
  target/arm/helper.h   |   2 -
  target/arm/internals.h|   3 +
  target/avr/cpu.h  |   1 +
  accel/tcg/cpu-exec.c  | 205 ++
  accel/tcg/translate-all.c |   7 +-
  accel/tcg/translator.c|  39 ++-
  cpu.c |  34 ++
  target/alpha/translate.c  |  31 +
  target/arm/cpu.c  |   1 +
  target/arm/cpu_tcg.c  |   1 +
  target/arm/debug_helper.c |  12 +-
  target/arm/translate-a64.c|  25 -
  target/arm/translate.c|  29 -
  target/avr/cpu.c  |   1 +
  target/avr/gdbstub.c  |  13 +++
  target/avr/translate.c|  32 --
  target/cris/translate.c   |  20 
  target/hexagon/translate.c|  17 ---
  target/hppa/translate.c   |  11 --
  target/i386/tcg/tcg-cpu.c |  12 ++
  target/i386/tcg/translate.c   |  28 -
  target/m68k/translate.c   |  18 ---
  target/microblaze/translate.c |  18 ---
  target/mips/tcg/translate.c   |  19 
  target/nios2/translate.c  |  27 -
  target/openrisc/translate.c   |  17 ---
  target/ppc/translate.c|  18 ---
  target/riscv/translate.c  |  17 ---
  target/rx/translate.c |  14 ---
  target/s390x/tcg/translate.c  |  24 
  target/sh4/translate.c|  18 ---
  target/sparc/translate.c  |  17 ---
  target/tricore/translate.c|  16 ---
  target/xtensa/translate.c |  17 ---
  tcg/tcg-op.c  |  28 ++---
  39 files changed, 248 insertions(+), 589 deletions(-)


I spent a bit of time this evening testing this patchset with some typical OpenBIOS 
debugging patterns across SPARC32, SPARC64 and PPC and didn't spot any regressions, 
and the WinXP image still boots in 25s so:


Tested-by: Mark Cave-Ayland 

I did find the slow-down noticeable when testing a few OpenBIOS breakpoints e.g. a 
breakpoint on OpenBIOS SPARC64's init_memory() upped the boot time to the Forth 
prompt from 9s to 30s even though it hits only once early in boot (presumably due to 
its proximity to a hot routine). Having said that, the changes look like a good 
improvement and patch 14 suggests that there could be further optimisations to be 
made in future, so in general this feels like a net win.



ATB,

Mark.

[PATCH v4 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h

[Richard Henderson]

All instances of EXTRA_ARGS are now identical.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 36 
 accel/tcg/cputlb.c  |  1 -
 accel/tcg/user-exec.c   |  1 -
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index d347462af5..52fb26a274 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -71,7 +71,8 @@
 #endif
 
 ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
-  ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+  ABI_TYPE cmpv, ABI_TYPE newv,
+  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -92,7 +93,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 
 #if DATA_SIZE >= 16
 #if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -106,8 +108,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr EXTRA_ARGS)
 return val;
 }
 
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -121,8 +123,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
 }
 #endif
 #else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
-   ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+   TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -139,7 +141,7 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr,
 
 #define GEN_ATOMIC_HELPER(X)\
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
-ABI_TYPE val EXTRA_ARGS)\
+ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
 ATOMIC_MMU_DECLS;   \
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
@@ -173,7 +175,7 @@ GEN_ATOMIC_HELPER(xor_fetch)
  */
 #define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET)\
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
-ABI_TYPE xval EXTRA_ARGS)   \
+ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
 ATOMIC_MMU_DECLS;   \
 XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
@@ -218,7 +220,8 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX,  DATA_TYPE, new)
 #endif
 
 ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
-  ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+  ABI_TYPE cmpv, ABI_TYPE newv,
+  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -239,7 +242,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 
 #if DATA_SIZE >= 16
 #if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -253,8 +257,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr EXTRA_ARGS)
 return BSWAP(val);
 }
 
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -270,8 +274,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
 }
 #endif
 #else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
-   ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+   TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr 

[PATCH v4 10/10] accel/tcg: Push trace info building into atomic_common.c.inc

[Richard Henderson]

Use trace_mem_get_info instead of trace_mem_build_info,
using the TCGMemOpIdx that we already have.  Do this in
the atomic_trace_*_pre function as common subroutines.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h   | 48 +--
 accel/tcg/atomic_common.c.inc | 37 ++-
 2 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 6ee0158c5f..d89af4cc1e 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -77,10 +77,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-atomic_trace_rmw_pre(env, addr, info);
 #if DATA_SIZE == 16
 ret = atomic16_cmpxchg(haddr, cmpv, newv);
 #else
@@ -99,10 +97,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ, retaddr);
 DATA_TYPE val;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_ld_pre(env, addr, oi);
 
-atomic_trace_ld_pre(env, addr, info);
 val = atomic16_read(haddr);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_ld_post(env, addr, info);
@@ -114,10 +110,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 {
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_WRITE, retaddr);
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_st_pre(env, addr, oi);
 
-atomic_trace_st_pre(env, addr, info);
 atomic16_set(haddr, val);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_st_post(env, addr, info);
@@ -130,10 +124,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-atomic_trace_rmw_pre(env, addr, info);
 ret = qatomic_xchg__nocheck(haddr, val);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_rmw_post(env, addr, info);
@@ -147,9 +139,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong 
addr,   \
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,  \
  PAGE_READ | PAGE_WRITE, retaddr); \
 DATA_TYPE ret;  \
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
- ATOMIC_MMU_IDX);   \
-atomic_trace_rmw_pre(env, addr, info);  \
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);\
 ret = qatomic_##X(haddr, val);  \
 ATOMIC_MMU_CLEANUP; \
 atomic_trace_rmw_post(env, addr, info); \
@@ -182,9 +172,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong 
addr,   \
 XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
   PAGE_READ | PAGE_WRITE, retaddr); \
 XDATA_TYPE cmp, old, new, val = xval;   \
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
- ATOMIC_MMU_IDX);   \
-atomic_trace_rmw_pre(env, addr, info);  \
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);\
 smp_mb();   \
 cmp = qatomic_read__nocheck(haddr); \
 do {\
@@ -228,10 +216,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-

[PATCH v4 09/10] trace: Fold mem-internal.h into mem.h

[Richard Henderson]

Since the last thing that mem.h does is include mem-internal.h,
the symbols are not actually private.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 trace/mem-internal.h | 50 
 trace/mem.h  | 50 ++--
 plugins/core.c   |  2 +-
 3 files changed, 40 insertions(+), 62 deletions(-)
 delete mode 100644 trace/mem-internal.h

diff --git a/trace/mem-internal.h b/trace/mem-internal.h
deleted file mode 100644
index 8b72b678fa..00
--- a/trace/mem-internal.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Helper functions for guest memory tracing
- *
- * Copyright (C) 2016 Lluís Vilanova 
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-
-#ifndef TRACE__MEM_INTERNAL_H
-#define TRACE__MEM_INTERNAL_H
-
-#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
-#define TRACE_MEM_SE (1ULL << 4)/* sign extended (y/n) */
-#define TRACE_MEM_BE (1ULL << 5)/* big endian (y/n) */
-#define TRACE_MEM_ST (1ULL << 6)/* store (y/n) */
-#define TRACE_MEM_MMU_SHIFT 8   /* mmu idx */
-
-static inline uint16_t trace_mem_build_info(
-int size_shift, bool sign_extend, MemOp endianness,
-bool store, unsigned int mmu_idx)
-{
-uint16_t res;
-
-res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
-if (sign_extend) {
-res |= TRACE_MEM_SE;
-}
-if (endianness == MO_BE) {
-res |= TRACE_MEM_BE;
-}
-if (store) {
-res |= TRACE_MEM_ST;
-}
-#ifdef CONFIG_SOFTMMU
-res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
-#endif
-return res;
-}
-
-static inline uint16_t trace_mem_get_info(MemOp op,
-  unsigned int mmu_idx,
-  bool store)
-{
-return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
-op & MO_BSWAP, store,
-mmu_idx);
-}
-
-#endif /* TRACE__MEM_INTERNAL_H */
diff --git a/trace/mem.h b/trace/mem.h
index 9644f592b4..2f27e7bdf0 100644
--- a/trace/mem.h
+++ b/trace/mem.h
@@ -12,24 +12,52 @@
 
 #include "tcg/tcg.h"
 
-
-/**
- * trace_mem_get_info:
- *
- * Return a value for the 'info' argument in guest memory access traces.
- */
-static uint16_t trace_mem_get_info(MemOp op, unsigned int mmu_idx, bool store);
+#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
+#define TRACE_MEM_SE (1ULL << 4)/* sign extended (y/n) */
+#define TRACE_MEM_BE (1ULL << 5)/* big endian (y/n) */
+#define TRACE_MEM_ST (1ULL << 6)/* store (y/n) */
+#define TRACE_MEM_MMU_SHIFT 8   /* mmu idx */
 
 /**
  * trace_mem_build_info:
  *
  * Return a value for the 'info' argument in guest memory access traces.
  */
-static uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
- MemOp endianness, bool store,
- unsigned int mmuidx);
+static inline uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
+MemOp endianness, bool store,
+unsigned int mmu_idx)
+{
+uint16_t res;
+
+res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
+if (sign_extend) {
+res |= TRACE_MEM_SE;
+}
+if (endianness == MO_BE) {
+res |= TRACE_MEM_BE;
+}
+if (store) {
+res |= TRACE_MEM_ST;
+}
+#ifdef CONFIG_SOFTMMU
+res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
+#endif
+return res;
+}
 
 
-#include "trace/mem-internal.h"
+/**
+ * trace_mem_get_info:
+ *
+ * Return a value for the 'info' argument in guest memory access traces.
+ */
+static inline uint16_t trace_mem_get_info(MemOp op,
+  unsigned int mmu_idx,
+  bool store)
+{
+return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
+op & MO_BSWAP, store,
+mmu_idx);
+}
 
 #endif /* TRACE__MEM_H */
diff --git a/plugins/core.c b/plugins/core.c
index e1bcdb570d..474db287cb 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -27,7 +27,7 @@
 #include "exec/helper-proto.h"
 #include "tcg/tcg.h"
 #include "tcg/tcg-op.h"
-#include "trace/mem-internal.h" /* mem_info macros */
+#include "trace/mem.h" /* mem_info macros */
 #include "plugin.h"
 #include "qemu/compiler.h"
 
-- 
2.25.1

[PATCH v4 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_*

[Richard Henderson]

Unify the parameters of atomic_mmu_lookup between cputlb.c and
user-exec.c.  Call the function directly, and remove the macros.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 41 +
 accel/tcg/cputlb.c  |  7 +--
 accel/tcg/user-exec.c   | 12 ++-
 3 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index ae6b6a03be..6ee0158c5f 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -95,7 +96,9 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ, retaddr);
+DATA_TYPE val;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
 
@@ -109,7 +112,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_WRITE, retaddr);
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
  ATOMIC_MMU_IDX);
 
@@ -123,7 +127,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -139,7 +144,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,  \
+ PAGE_READ | PAGE_WRITE, retaddr); \
 DATA_TYPE ret;  \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
  ATOMIC_MMU_IDX);   \
@@ -161,7 +167,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
 
 #undef GEN_ATOMIC_HELPER
 
-/* These helpers are, as a whole, full barriers.  Within the helper,
+/*
+ * These helpers are, as a whole, full barriers.  Within the helper,
  * the leading barrier is explicit and the trailing barrier is within
  * cmpxchg primitive.
  *
@@ -172,7 +179,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
+XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+  PAGE_READ | PAGE_WRITE, retaddr); \
 XDATA_TYPE cmp, old, new, val = xval;   \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
  ATOMIC_MMU_IDX);   \
@@ -217,7 +225,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE 

[PATCH v4 05/10] accel/tcg: Standardize atomic helpers on softmmu api

[Richard Henderson]

Reduce the amount of code duplication by always passing
the TCGMemOpIdx argument to helper_atomic_*.  This is not
currently used for user-only, but it's easy to ignore.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/tcg-runtime.h   | 46 ---
 accel/tcg/cputlb.c| 32 
 accel/tcg/user-exec.c | 26 -
 tcg/tcg-op.c  | 51 ++---
 accel/tcg/atomic_common.c.inc | 70 +++
 5 files changed, 82 insertions(+), 143 deletions(-)

diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index 91a5b7e85f..37cbd722bf 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -39,8 +39,6 @@ DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env)
 DEF_HELPER_FLAGS_3(memset, TCG_CALL_NO_RWG, ptr, ptr, int, ptr)
 #endif /* IN_HELPER_PROTO */
 
-#ifdef CONFIG_SOFTMMU
-
 DEF_HELPER_FLAGS_5(atomic_cmpxchgb, TCG_CALL_NO_WG,
i32, env, tl, i32, i32, i32)
 DEF_HELPER_FLAGS_5(atomic_cmpxchgw_be, TCG_CALL_NO_WG,
@@ -88,50 +86,6 @@ DEF_HELPER_FLAGS_5(atomic_cmpxchgq_le, TCG_CALL_NO_WG,
TCG_CALL_NO_WG, i32, env, tl, i32, i32)
 #endif /* CONFIG_ATOMIC64 */
 
-#else
-
-DEF_HELPER_FLAGS_4(atomic_cmpxchgb, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-#ifdef CONFIG_ATOMIC64
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_be, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_le, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-#endif
-
-#ifdef CONFIG_ATOMIC64
-#define GEN_ATOMIC_HELPERS(NAME) \
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_le),  \
-   TCG_CALL_NO_WG, i64, env, tl, i64)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_be),  \
-   TCG_CALL_NO_WG, i64, env, tl, i64)
-#else
-#define GEN_ATOMIC_HELPERS(NAME) \
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)
-#endif /* CONFIG_ATOMIC64 */
-
-#endif /* CONFIG_SOFTMMU */
-
 GEN_ATOMIC_HELPERS(fetch_add)
 GEN_ATOMIC_HELPERS(fetch_and)
 GEN_ATOMIC_HELPERS(fetch_or)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 63da1cc96f..842cf4b572 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2725,38 +2725,6 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong 
ptr, uint64_t val)
 #include "atomic_template.h"
 #endif
 
-/* Second set of helpers are directly callable from TCG as helpers.  */
-
-#undef EXTRA_ARGS
-#undef ATOMIC_NAME
-#undef ATOMIC_MMU_LOOKUP_RW
-#undef ATOMIC_MMU_LOOKUP_R
-#undef ATOMIC_MMU_LOOKUP_W
-
-#define EXTRA_ARGS , TCGMemOpIdx oi
-#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
-#define ATOMIC_MMU_LOOKUP_RW \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, 
GETPC())
-#define ATOMIC_MMU_LOOKUP_R \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ, GETPC())
-#define ATOMIC_MMU_LOOKUP_W \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_WRITE, GETPC())
-
-#define DATA_SIZE 1
-#include "atomic_template.h"
-
-#define DATA_SIZE 2
-#include "atomic_template.h"
-
-#define DATA_SIZE 4
-#include "atomic_template.h"
-
-#ifdef CONFIG_ATOMIC64
-#define DATA_SIZE 8
-#include "atomic_template.h"
-#endif
-#undef ATOMIC_MMU_IDX
-
 /* Code access functions.  */
 
 static uint64_t 

[PATCH v4 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS

[Richard Henderson]

All definitions are now empty.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 12 
 accel/tcg/cputlb.c  |  1 -
 accel/tcg/user-exec.c   |  1 -
 3 files changed, 14 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 52fb26a274..ae6b6a03be 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -96,7 +95,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -111,7 +109,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
  ATOMIC_MMU_IDX);
@@ -126,7 +123,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -143,7 +139,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-ATOMIC_MMU_DECLS;   \
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
 DATA_TYPE ret;  \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
@@ -177,7 +172,6 @@ GEN_ATOMIC_HELPER(xor_fetch)
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-ATOMIC_MMU_DECLS;   \
 XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
 XDATA_TYPE cmp, old, new, val = xval;   \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
@@ -223,7 +217,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -245,7 +238,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
  ATOMIC_MMU_IDX);
@@ -260,7 +252,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, true,
  ATOMIC_MMU_IDX);
@@ -277,7 +268,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 ABI_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -294,7 +284,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, 

[PATCH v4 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only

[Richard Henderson]

Always provide the atomic interface using TCGMemOpIdx oi
and uintptr_t retaddr.  Rename from helper_* to cpu_* so
as to (mostly) match the exec/cpu_ldst.h functions, and
to emphasize that they are not callable from TCG directly.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 include/tcg/tcg.h | 78 ---
 accel/tcg/cputlb.c|  8 ++--
 accel/tcg/user-exec.c | 59 --
 target/arm/helper-a64.c   |  8 ++--
 target/i386/tcg/mem_helper.c  | 15 +--
 target/m68k/op_helper.c   | 19 +++--
 target/ppc/mem_helper.c   | 16 +++
 target/s390x/tcg/mem_helper.c | 19 -
 8 files changed, 104 insertions(+), 118 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 25dd19d6e1..44ccd86f3e 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -1341,31 +1341,32 @@ void helper_be_stq_mmu(CPUArchState *env, target_ulong 
addr, uint64_t val,
 # define helper_ret_stl_mmu   helper_le_stl_mmu
 # define helper_ret_stq_mmu   helper_le_stq_mmu
 #endif
+#endif /* CONFIG_SOFTMMU */
 
-uint32_t helper_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cpu_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
 uint32_t cmpv, uint32_t newv,
 TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
-   uint64_t cmpv, uint64_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
-   uint64_t cmpv, uint64_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
+uint64_t cmpv, uint64_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
+uint64_t cmpv, uint64_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
 
 #define GEN_ATOMIC_HELPER(NAME, TYPE, SUFFIX) \
-TYPE helper_atomic_ ## NAME ## SUFFIX ## _mmu \
+TYPE cpu_atomic_ ## NAME ## SUFFIX ## _mmu\
 (CPUArchState *env, target_ulong addr, TYPE val,  \
  TCGMemOpIdx oi, uintptr_t retaddr);
 
@@ -1411,31 +1412,22 @@ GEN_ATOMIC_HELPER_ALL(xchg)
 
 #undef GEN_ATOMIC_HELPER_ALL
 #undef GEN_ATOMIC_HELPER
-#endif /* CONFIG_SOFTMMU */
 
-/*
- * These aren't really a "proper" helpers because TCG cannot manage Int128.
- * However, use the same format as the others, for use by the backends.
- *
- * The cmpxchg functions are only defined if HAVE_CMPXCHG128;
- * the ld/st functions are only defined if HAVE_ATOMIC128,
- * as defined by .
- */
-Int128 helper_atomic_cmpxchgo_le_mmu(CPUArchState *env, target_ulong addr,
- Int128 cmpv, Int128 newv,
- TCGMemOpIdx oi, uintptr_t retaddr);

[PATCH v4 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types

[Richard Henderson]

Use it to avoid some clang-12 -Watomic-alignment errors,
forcing some structures to be aligned and as a pointer when
we have ensured that the address is aligned.

Tested-by: Cole Robinson 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h |  4 ++--
 include/qemu/atomic.h   | 14 +-
 include/qemu/stats64.h  |  2 +-
 softmmu/timers-state.h  |  2 +-
 linux-user/hppa/cpu_loop.c  |  2 +-
 util/qsp.c  |  4 ++--
 6 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index afa8a9daf3..d347462af5 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -28,8 +28,8 @@
 # define SHIFT  4
 #elif DATA_SIZE == 8
 # define SUFFIX q
-# define DATA_TYPE  uint64_t
-# define SDATA_TYPE int64_t
+# define DATA_TYPE  aligned_uint64_t
+# define SDATA_TYPE aligned_int64_t
 # define BSWAP  bswap64
 # define SHIFT  3
 #elif DATA_SIZE == 4
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index fe5467d193..112a29910b 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -271,7 +271,19 @@
 _oldn;  \
 })
 
-/* Abstractions to access atomically (i.e. "once") i64/u64 variables */
+/*
+ * Abstractions to access atomically (i.e. "once") i64/u64 variables.
+ *
+ * The i386 abi is odd in that by default members are only aligned to
+ * 4 bytes, which means that 8-byte types can wind up mis-aligned.
+ * Clang will then warn about this, and emit a call into libatomic.
+ *
+ * Use of these types in structures when they will be used with atomic
+ * operations can avoid this.
+ */
+typedef int64_t aligned_int64_t __attribute__((aligned(8)));
+typedef uint64_t aligned_uint64_t __attribute__((aligned(8)));
+
 #ifdef CONFIG_ATOMIC64
 /* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
 #define qatomic_read_i64(P) \
diff --git a/include/qemu/stats64.h b/include/qemu/stats64.h
index fdd3d1b8f9..802402254b 100644
--- a/include/qemu/stats64.h
+++ b/include/qemu/stats64.h
@@ -21,7 +21,7 @@
 
 typedef struct Stat64 {
 #ifdef CONFIG_ATOMIC64
-uint64_t value;
+aligned_uint64_t value;
 #else
 uint32_t low, high;
 uint32_t lock;
diff --git a/softmmu/timers-state.h b/softmmu/timers-state.h
index 8c262ce139..94bb7394c5 100644
--- a/softmmu/timers-state.h
+++ b/softmmu/timers-state.h
@@ -47,7 +47,7 @@ typedef struct TimersState {
 int64_t last_delta;
 
 /* Compensate for varying guest execution speed.  */
-int64_t qemu_icount_bias;
+aligned_int64_t qemu_icount_bias;
 
 int64_t vm_clock_warp_start;
 int64_t cpu_clock_offset;
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 3aaaf3337c..82d8183821 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -82,7 +82,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
 o64 = *(uint64_t *)g2h(cs, old);
 n64 = *(uint64_t *)g2h(cs, new);
 #ifdef CONFIG_ATOMIC64
-r64 = qatomic_cmpxchg__nocheck((uint64_t *)g2h(cs, addr),
+r64 = qatomic_cmpxchg__nocheck((aligned_uint64_t *)g2h(cs, 
addr),
o64, n64);
 ret = r64 != o64;
 #else
diff --git a/util/qsp.c b/util/qsp.c
index bacc5fa2f6..8562b14a87 100644
--- a/util/qsp.c
+++ b/util/qsp.c
@@ -83,8 +83,8 @@ typedef struct QSPCallSite QSPCallSite;
 struct QSPEntry {
 void *thread_ptr;
 const QSPCallSite *callsite;
-uint64_t n_acqs;
-uint64_t ns;
+aligned_uint64_t n_acqs;
+aligned_uint64_t ns;
 unsigned int n_objs; /* count of coalesced objs; only used for reporting */
 };
 typedef struct QSPEntry QSPEntry;
-- 
2.25.1

[PATCH v4 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks

[Richard Henderson]

We now require c11, so the fallbacks are now dead code

Tested-by: Cole Robinson 
Reviewed-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 configure |   7 --
 include/qemu/atomic.h | 204 +++---
 2 files changed, 10 insertions(+), 201 deletions(-)

diff --git a/configure b/configure
index 232c54dcc1..b5965b159f 100755
--- a/configure
+++ b/configure
@@ -3991,18 +3991,11 @@ cat > $TMPC << EOF
 int main(void)
 {
   uint64_t x = 0, y = 0;
-#ifdef __ATOMIC_RELAXED
   y = __atomic_load_n(, __ATOMIC_RELAXED);
   __atomic_store_n(, y, __ATOMIC_RELAXED);
   __atomic_compare_exchange_n(, , x, 0, __ATOMIC_RELAXED, 
__ATOMIC_RELAXED);
   __atomic_exchange_n(, y, __ATOMIC_RELAXED);
   __atomic_fetch_add(, y, __ATOMIC_RELAXED);
-#else
-  typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
-  __sync_lock_test_and_set(, y);
-  __sync_val_compare_and_swap(, y, 0);
-  __sync_fetch_and_add(, y);
-#endif
   return 0;
 }
 EOF
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index a7654d2a33..fe5467d193 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -60,8 +60,9 @@
 (unsigned short)1, 
\
   (expr)+0))
 
-#ifdef __ATOMIC_RELAXED
-/* For C11 atomic ops */
+#ifndef __ATOMIC_RELAXED
+#error "Expecting C11 atomic ops"
+#endif
 
 /* Manual memory barriers
  *
@@ -239,193 +240,8 @@
 #define qatomic_xor(ptr, n) \
 ((void) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST))
 
-#else /* __ATOMIC_RELAXED */
-
-#ifdef __alpha__
-#define smp_read_barrier_depends()   asm volatile("mb":::"memory")
-#endif
-
-#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
-
-/*
- * Because of the strongly ordered storage model, wmb() and rmb() are nops
- * here (a compiler barrier only).  QEMU doesn't do accesses to write-combining
- * qemu memory or non-temporal load/stores from C code.
- */
-#define smp_mb_release()   barrier()
-#define smp_mb_acquire()   barrier()
-
-/*
- * __sync_lock_test_and_set() is documented to be an acquire barrier only,
- * but it is a full barrier at the hardware level.  Add a compiler barrier
- * to make it a full barrier also at the compiler level.
- */
-#define qatomic_xchg(ptr, i)(barrier(), __sync_lock_test_and_set(ptr, i))
-
-#elif defined(_ARCH_PPC)
-
-/*
- * We use an eieio() for wmb() on powerpc.  This assumes we don't
- * need to order cacheable and non-cacheable stores with respect to
- * each other.
- *
- * smp_mb has the same problem as on x86 for not-very-new GCC
- * (http://patchwork.ozlabs.org/patch/126184/, Nov 2011).
- */
-#define smp_wmb()  ({ asm volatile("eieio" ::: "memory"); (void)0; })
-#if defined(__powerpc64__)
-#define smp_mb_release()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#define smp_mb_acquire()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#else
-#define smp_mb_release()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-#define smp_mb_acquire()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-#endif
-#define smp_mb()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-
-#endif /* _ARCH_PPC */
-
-/*
- * For (host) platforms we don't have explicit barrier definitions
- * for, we use the gcc __sync_synchronize() primitive to generate a
- * full barrier.  This should be safe on all platforms, though it may
- * be overkill for smp_mb_acquire() and smp_mb_release().
- */
-#ifndef smp_mb
-#define smp_mb()   __sync_synchronize()
-#endif
-
-#ifndef smp_mb_acquire
-#define smp_mb_acquire()   __sync_synchronize()
-#endif
-
-#ifndef smp_mb_release
-#define smp_mb_release()   __sync_synchronize()
-#endif
-
-#ifndef smp_read_barrier_depends
-#define smp_read_barrier_depends()   barrier()
-#endif
-
-#ifndef signal_barrier
-#define signal_barrier()barrier()
-#endif
-
-/* These will only be atomic if the processor does the fetch or store
- * in a single issue memory operation
- */
-#define qatomic_read__nocheck(p)   (*(__typeof__(*(p)) volatile*) (p))
-#define qatomic_set__nocheck(p, i) ((*(__typeof__(*(p)) volatile*) (p)) = (i))
-
-#define qatomic_read(ptr)   qatomic_read__nocheck(ptr)
-#define qatomic_set(ptr, i) qatomic_set__nocheck(ptr,i)
-
-/**
- * qatomic_rcu_read - reads a RCU-protected pointer to a local variable
- * into a RCU read-side critical section. The pointer can later be safely
- * dereferenced within the critical section.
- *
- * This ensures that the pointer copy is invariant thorough the whole critical
- * section.
- *
- * Inserts memory barriers on architectures that require them (currently only
- * Alpha) and documents which pointers are protected by RCU.
- *
- * qatomic_rcu_read also includes a compiler barrier to ensure that
- * value-speculative optimizations (e.g. VSS: Value Speculation
- * Scheduling) does not perform the data read before the pointer read
- * by speculating the value of the pointer.
- *

[PATCH v4 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64

[Richard Henderson]

Clang warnings about questionable atomic usage get localized
to the inline function in atomic.h.  By using a macro, we get
the full traceback to the original use that caused the warning.

Tested-by: Cole Robinson 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 include/qemu/atomic.h | 29 +
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 3ccf84fd46..a7654d2a33 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -457,26 +457,15 @@
 
 /* Abstractions to access atomically (i.e. "once") i64/u64 variables */
 #ifdef CONFIG_ATOMIC64
-static inline int64_t qatomic_read_i64(const int64_t *ptr)
-{
-/* use __nocheck because sizeof(void *) might be < sizeof(u64) */
-return qatomic_read__nocheck(ptr);
-}
-
-static inline uint64_t qatomic_read_u64(const uint64_t *ptr)
-{
-return qatomic_read__nocheck(ptr);
-}
-
-static inline void qatomic_set_i64(int64_t *ptr, int64_t val)
-{
-qatomic_set__nocheck(ptr, val);
-}
-
-static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
-{
-qatomic_set__nocheck(ptr, val);
-}
+/* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
+#define qatomic_read_i64(P) \
+_Generic(*(P), int64_t: qatomic_read__nocheck(P))
+#define qatomic_read_u64(P) \
+_Generic(*(P), uint64_t: qatomic_read__nocheck(P))
+#define qatomic_set_i64(P, V) \
+_Generic(*(P), int64_t: qatomic_set__nocheck(P, V))
+#define qatomic_set_u64(P, V) \
+_Generic(*(P), uint64_t: qatomic_set__nocheck(P, V))
 
 static inline void qatomic64_init(void)
 {
-- 
2.25.1

[PATCH v4 00/10] Atomic cleanup + clang-12 build fix

[Richard Henderson]

This is intended to fix building with clang-12 on i386.

In the process, I found bugs wrt handling of guest memory in target/
with respect to atomics, fixed by unifying the api between softmmu
and user-only and removing some ifdefs under target/.

Unification of the api allowed some further cleanups.

Changes for v4:
  * Use _Generic for typechecking of CONFIG_ATOMIC64 macros (pmm).
  * Style nits in tcg-op.c (pmm).

Changes for v3:
  * Dropped the typeof_strip_qual patch with broader testing.
  * Squashed an include fix for trace/mem.h, with plugins enabled.


All patches have at least one R-b; barring further comment
I plan to include this in a pull request tomorrow.


r~


Richard Henderson (10):
  qemu/atomic: Use macros for CONFIG_ATOMIC64
  qemu/atomic: Remove pre-C11 atomic fallbacks
  qemu/atomic: Add aligned_{int64,uint64}_t types
  tcg: Rename helper_atomic_*_mmu and provide for user-only
  accel/tcg: Standardize atomic helpers on softmmu api
  accel/tcg: Fold EXTRA_ARGS into atomic_template.h
  accel/tcg: Remove ATOMIC_MMU_DECLS
  accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
  trace: Fold mem-internal.h into mem.h
  accel/tcg: Push trace info building into atomic_common.c.inc

 configure |   7 -
 accel/tcg/atomic_template.h   | 141 +--
 accel/tcg/tcg-runtime.h   |  46 ---
 include/qemu/atomic.h | 247 +-
 include/qemu/stats64.h|   2 +-
 include/tcg/tcg.h |  78 +--
 softmmu/timers-state.h|   2 +-
 trace/mem-internal.h  |  50 ---
 trace/mem.h   |  50 +--
 accel/tcg/cputlb.c|  49 +--
 accel/tcg/user-exec.c |  41 +++---
 linux-user/hppa/cpu_loop.c|   2 +-
 plugins/core.c|   2 +-
 target/arm/helper-a64.c   |   8 +-
 target/i386/tcg/mem_helper.c  |  15 +--
 target/m68k/op_helper.c   |  19 +--
 target/ppc/mem_helper.c   |  16 +--
 target/s390x/tcg/mem_helper.c |  19 +--
 tcg/tcg-op.c  |  51 ++-
 util/qsp.c|   4 +-
 accel/tcg/atomic_common.c.inc | 107 +--
 21 files changed, 329 insertions(+), 627 deletions(-)
 delete mode 100644 trace/mem-internal.h

-- 
2.25.1

Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint

[Richard Henderson]

On 7/20/21 10:56 AM, Peter Maydell wrote:

On Tue, 20 Jul 2021 at 20:54, Richard Henderson
 wrote:


This will allow a breakpoint hack to move out of AVR's translator.

Signed-off-by: Richard Henderson 



diff --git a/cpu.c b/cpu.c
index 83059537d7..91d9e38acb 100644
--- a/cpu.c
+++ b/cpu.c
@@ -267,8 +267,13 @@ static void breakpoint_invalidate(CPUState *cpu, 
target_ulong pc)
  int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
CPUBreakpoint **breakpoint)
  {
+CPUClass *cc = CPU_GET_CLASS(cpu);
  CPUBreakpoint *bp;

+if (cc->gdb_adjust_breakpoint) {
+pc = cc->gdb_adjust_breakpoint(cpu, pc);
+}
+
  bp = g_malloc(sizeof(*bp));

  bp->pc = pc;
@@ -294,8 +299,13 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int 
flags,
  /* Remove a specific breakpoint.  */
  int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
  {
+CPUClass *cc = CPU_GET_CLASS(cpu);
  CPUBreakpoint *bp;

+if (cc->gdb_adjust_breakpoint) {
+pc = cc->gdb_adjust_breakpoint(cpu, pc);
+}
+
  QTAILQ_FOREACH(bp, >breakpoints, entry) {
  if (bp->pc == pc && bp->flags == flags) {
  cpu_breakpoint_remove_by_ref(cpu, bp);
--


So previously for AVR we would have considered the bp at 0x100
and the one at 0x800100 as distinct (in the sense that the only way
the gdb remote protocol distinguishes breakpoints is by "what address",
and these have different addresses). After this change, they won't
be distinct, because if you set a bp at 0x100 and 0x800100 and then
try to remove the one at 0x100 we might remove the 0x800100 one,
because we're storing only the adjusted-address, not the one gdb used.

This might not matter in practice...


I don't think it will matter.

Currently, if it sets both 0x100 and 0x800100, then we'll record two breakpoints, and with 
either we'll raise EXCP_DEBUG when pc == 0x100.


Afterward, we'll have two CPUBreakpoint structures that both contain 0x100, and when pc == 
0x100 we'll raise EXCP_DEBUG.  If gdb removes the breakpoint at 0x800100, we'll remove one 
of the two CPUBreakpoint.  But we'll still stop at 0x100, as expected.  When it removes 
the breakpoint at 0x100, both CPUBreakpoint structures will be gone.


In principal, gdb could now add a breakpoint at 0x800100 and remove it with 0x100, where 
it could not before.  But I don't expect that to happen.  If we reported any kind of 
status to gdb re the breakpoint insertion or removal (e.g. bp not found), then it might 
matter, but we don't.


Practically, this is working around what I'd call a gdb bug wrt avr.  Which may even have 
been fixed -- I haven't looked.



r~

Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint

[Peter Maydell]

On Tue, 20 Jul 2021 at 20:54, Richard Henderson
 wrote:
>
> This will allow a breakpoint hack to move out of AVR's translator.
>
> Signed-off-by: Richard Henderson 

> diff --git a/cpu.c b/cpu.c
> index 83059537d7..91d9e38acb 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -267,8 +267,13 @@ static void breakpoint_invalidate(CPUState *cpu, 
> target_ulong pc)
>  int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
>CPUBreakpoint **breakpoint)
>  {
> +CPUClass *cc = CPU_GET_CLASS(cpu);
>  CPUBreakpoint *bp;
>
> +if (cc->gdb_adjust_breakpoint) {
> +pc = cc->gdb_adjust_breakpoint(cpu, pc);
> +}
> +
>  bp = g_malloc(sizeof(*bp));
>
>  bp->pc = pc;
> @@ -294,8 +299,13 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int 
> flags,
>  /* Remove a specific breakpoint.  */
>  int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
>  {
> +CPUClass *cc = CPU_GET_CLASS(cpu);
>  CPUBreakpoint *bp;
>
> +if (cc->gdb_adjust_breakpoint) {
> +pc = cc->gdb_adjust_breakpoint(cpu, pc);
> +}
> +
>  QTAILQ_FOREACH(bp, >breakpoints, entry) {
>  if (bp->pc == pc && bp->flags == flags) {
>  cpu_breakpoint_remove_by_ref(cpu, bp);
> --

So previously for AVR we would have considered the bp at 0x100
and the one at 0x800100 as distinct (in the sense that the only way
the gdb remote protocol distinguishes breakpoints is by "what address",
and these have different addresses). After this change, they won't
be distinct, because if you set a bp at 0x100 and 0x800100 and then
try to remove the one at 0x100 we might remove the 0x800100 one,
because we're storing only the adjusted-address, not the one gdb used.

This might not matter in practice...

-- PMM

Re: [PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types

[Richard Henderson]

On 7/19/21 2:39 AM, Peter Maydell wrote:

This cast is OK, but it took me a while to verify that:
  * we check that 'addr' is 8-aligned further up in this function
  * we check that guest_base is at least page-aligned in
probe_guest_base(), and there's no way to avoid that function
getting called if you specify a guest-base value on the command line


Yep.


Is it worth asserting that the value we get back from g2h() really
is 8-aligned before casting ?


No, I don't think so.  I think the check on the guest address is sufficient, and Just Know 
that guest_base is not going to misalign anything.



r~

Re: [PATCH] block: drop BLK_PERM_GRAPH_MOD

[Eric Blake]

On Tue, Jul 20, 2021 at 05:22:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> First, this permission never protected node from being changed, as

a node

> generic child-replacing functions don't check it.
> 
> Second, it's a strange thing: it presents a permission of parent node
> to change its child. But generally, children are replaced by different
> mechanisms, like jobs or qmp commands, not by nodes.
> 
> Graph-mod permission is hard to understand. All other permissions
> describe operations which done by parent node on it child: read, write,

its child

> resize. Graph modification operations are something completely
> different.
> 
> The only place, where BLK_PERM_GRAPH_MOD is used as "perm" (not shared

s/place,/place/

> perm) is mirror_start_job, for s->target. Still modern code should use
> bdrv_freeze_backing_chain() to protect from graph modification, if we
> don't do it somewhere it may be considered as a bug. So, it's a bit
> risky to drop GRAPH_MOD, and analyzing of possible loss of protection
> is hard. But one day we should do it, let's do it now.
> 
> One more bit of information is that locking corresponding byte in

locking the

> file-posix doesn't make sense at all.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---

> +++ b/include/block/block.h
> @@ -269,12 +269,13 @@ enum {
>  BLK_PERM_RESIZE = 0x08,
>  
>  /**
> - * This permission is required to change the node that this BdrvChild
> - * points to.
> + * There was a removed now BLK_PERM_GRAPH_MOD, with value of 0x10. QEMU 
> 6.1

There was a now-removed bit BLK_PERM_GRAPH_MOD,

> + * and earlier still my lock corresponding byte in block/file-posix 
> locking.

s/still my lock/may still lock the/

> + * So, implementing some new permission should be very careful to not
> + * interfere with this old unused thing.
>   */
> -BLK_PERM_GRAPH_MOD  = 0x10,
>  
> -BLK_PERM_ALL= 0x1f,
> +BLK_PERM_ALL= 0x0f,
>  
>  DEFAULT_PERM_PASSTHROUGH= BLK_PERM_CONSISTENT_READ
>   | BLK_PERM_WRITE

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-6.1 v6 17/17] accel/tcg: Record singlestep_enabled in tb->cflags

[Peter Maydell]

On Tue, 20 Jul 2021 at 20:55, Richard Henderson
 wrote:
>
> Set CF_SINGLE_STEP when single-stepping is enabled.
> This avoids the need to flush all tb's when turning
> single-stepping on or off.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH for-6.1 v6 15/17] accel/tcg: Remove TranslatorOps.breakpoint_check

[Peter Maydell]

On Tue, 20 Jul 2021 at 20:55, Richard Henderson
 wrote:
>
> The hook is now unused, with breakpoints checked outside translation.
>
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/translator.h | 11 ---
>  target/arm/helper.h   |  2 --
>  target/alpha/translate.c  | 16 
>  target/arm/debug_helper.c |  7 ---
>  target/arm/translate-a64.c| 25 -
>  target/arm/translate.c| 29 -
>  target/avr/translate.c| 18 --
>  target/cris/translate.c   | 20 
>  target/hexagon/translate.c| 17 -
>  target/hppa/translate.c   | 11 ---
>  target/i386/tcg/translate.c   | 28 
>  target/m68k/translate.c   | 18 --
>  target/microblaze/translate.c | 18 --
>  target/mips/tcg/translate.c   | 19 ---
>  target/nios2/translate.c  | 27 ---
>  target/openrisc/translate.c   | 17 -
>  target/ppc/translate.c| 18 --
>  target/riscv/translate.c  | 17 -
>  target/rx/translate.c | 14 --
>  target/s390x/tcg/translate.c  | 24 
>  target/sh4/translate.c| 18 --
>  target/sparc/translate.c  | 17 -
>  target/tricore/translate.c| 16 
>  target/xtensa/translate.c | 17 -
>  24 files changed, 424 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64

[Richard Henderson]

On 7/19/21 2:01 AM, Peter Maydell wrote:

-static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
-{
-qatomic_set__nocheck(ptr, val);
-}

...

+#define qatomic_set_u64   qatomic_set__nocheck


Previously if you tried to do qatomic_set_i64() etc on something
that wasn't an int64_t*, the compiler would complain. Now it will
silently store a different-sized value, I think. Is there a way
to retain the typechecking in the macro versions?


I can add a trivial _Generic wrapper.


r~

Re: [PATCH v2 24/24] python/aqmp: add AsyncProtocol unit tests

[Beraldo Leal]

On Fri, Jul 16, 2021 at 08:32:53PM -0400, John Snow wrote:
> This tests most of protocol.py -- From a hacked up Coverage.py run, it's
> at about 86%. There's a few error cases that aren't very well tested
> yet, they're hard to induce artificially so far. I'm working on it.
> 
> Signed-off-by: John Snow 
> ---
>  python/tests/null_proto.py |  67 ++
>  python/tests/protocol.py   | 458 +
>  2 files changed, 525 insertions(+)
>  create mode 100644 python/tests/null_proto.py
>  create mode 100644 python/tests/protocol.py
> 
> diff --git a/python/tests/null_proto.py b/python/tests/null_proto.py
> new file mode 100644
> index 000..c697efc0001
> --- /dev/null
> +++ b/python/tests/null_proto.py
> @@ -0,0 +1,67 @@
> +import asyncio
> +
> +from qemu.aqmp.protocol import AsyncProtocol
> +
> +
> +class NullProtocol(AsyncProtocol[None]):
> +"""
> +NullProtocol is a test mockup of an AsyncProtocol implementation.
> +
> +It adds a fake_session instance variable that enables a code path
> +that bypasses the actual connection logic, but still allows the
> +reader/writers to start.
> +
> +Because the message type is defined as None, an asyncio.Event named
> +'trigger_input' is created that prohibits the reader from
> +incessantly being able to yield None; this input can be poked to
> +simulate an incoming message.
> +
> +For testing symmetry with do_recv, an interface is added to "send" a
> +Null message.
> +
> +For testing purposes, a "simulate_disconnection" method is also
> +added which allows us to trigger a bottom half disconnect without
> +injecting any real errors into the reader/writer loops; in essence
> +it performs exactly half of what disconnect() normally does.
> +"""
> +def __init__(self, name=None):
> +self.fake_session = False
> +self.trigger_input: asyncio.Event
> +super().__init__(name)
> +
> +async def _establish_session(self):
> +self.trigger_input = asyncio.Event()
> +await super()._establish_session()
> +
> +async def _do_accept(self, address, ssl=None):
> +if not self.fake_session:
> +await super()._do_accept(address, ssl)
> +
> +async def _do_connect(self, address, ssl=None):
> +if not self.fake_session:
> +await super()._do_connect(address, ssl)
> +
> +async def _do_recv(self) -> None:
> +await self.trigger_input.wait()
> +self.trigger_input.clear()
> +
> +def _do_send(self, msg: None) -> None:
> +pass
> +
> +async def send_msg(self) -> None:
> +await self._outgoing.put(None)
> +
> +async def simulate_disconnect(self) -> None:
> +# Simulates a bottom half disconnect, e.g. schedules a
> +# disconnection but does not wait for it to complete. This is
> +# used to put the loop into the DISCONNECTING state without
> +# fully quiescing it back to IDLE; this is normally something
> +# you cannot coax AsyncProtocol to do on purpose, but it will be
> +# similar to what happens with an unhandled Exception in the
> +# reader/writer.
> +#
> +# Under normal circumstances, the library design requires you to
> +# await on disconnect(), which awaits the disconnect task and
> +# returns bottom half errors as a pre-condition to allowing the
> +# loop to return back to IDLE.
> +self._schedule_disconnect()

Nitpick: Any reason for not using a docstring? I wouldn't mind if it was
a docstring instead. ;)

> diff --git a/python/tests/protocol.py b/python/tests/protocol.py
> new file mode 100644
> index 000..2374d01365e
> --- /dev/null
> +++ b/python/tests/protocol.py
> @@ -0,0 +1,458 @@
> +import asyncio
> +from contextlib import contextmanager
> +import os
> +import socket
> +from tempfile import TemporaryDirectory
> +
> +import avocado
> +
> +from qemu.aqmp import ConnectError, Runstate
> +from qemu.aqmp.protocol import StateError
> +from qemu.aqmp.util import asyncio_run, create_task

Nitpick: Maybe an isort?

> +# An Avocado bug prevents us from defining this testing class in-line here:
> +from null_proto import NullProtocol

Is this what you are looking for?

https://github.com/avocado-framework/avocado/pull/4764

If not, can you point to the right issue, please?

> +@contextmanager
> +def jammed_socket():
> +# This method opens up a random TCP port on localhost, then jams it.
> +socks = []
> +
> +try:
> +sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> +sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
> +sock.bind(('127.0.0.1', 0))
> +sock.listen(1)
> +address = sock.getsockname()
> +
> +socks.append(sock)
> +
> +# I don't *fully* understand why, but it takes *two* un-accepted
> +# connections to start jamming the socket.
> +for _ in range(2):
> +  

[Bug 1936977] Re: qemu-arm-static crashes "segmentation fault" when running "git clone"

[Peter Maydell]

This is the upstream QEMU bug tracker, not an Ubuntu specific tracker;
if you'd like Ubuntu to consider a backport of something, please file a
bug with them.


** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1936977

Title:
   qemu-arm-static crashes "segmentation fault" when running "git clone"

Status in QEMU:
  Invalid

Bug description:
  This is a reopen of #1869073 for `qemu-user-static/focal-
  updates,focal-security,now 1:4.2-3ubuntu6.17 amd64`.

  `git clone` reproducably segfaults in `qemu-arm-static` chroot.

  #1869073 mentions this should have been fixed for newer versions of
  QEMU, but for `focal` there's no newer version available, even in
  `focal-backports`.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1936977/+subscriptions

Re: tests/acceptance/multiprocess.py test failure

[Peter Maydell]

On Tue, 20 Jul 2021 at 21:18, Jag Raman  wrote:
>
>
>
> > On Jul 20, 2021, at 2:39 PM, Cleber Rosa  wrote:
> >
> >
> > Jag Raman writes:
> >>
> >> We presently don’t have permissions to send a PR to
> >> upstream (Peter Maydell).
> >>
> >> Presently, we are requesting someone else who has
> >> permissions to do PRs on our behalf. We will work
> >> on getting permissions to send PRs going forward.

> > I'm going to include that patch in an upcoming PR.  Please let me know
> > if this is not what you intended.
> >
> > PS: I'm not sure I follow what your specific permission problem is, if
> > it's technical or something else.  But, in either case, I'd recommend you
> > sync the MAINTAINERS file entries with your roles/abilities to maintain
> > those files listed.

> I have not registered a GPG keys to submit PR - please see following
> email for context:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg765788.html
>
> I’ll get started on this process as I can help with smaller patches.

This isn't a technical thing particularly -- I just prefer that
if you're not going to be submitting a lot of patches that they
go through some other submaintainer who can review and curate
them as they go past. I do not want us to have a structure
where we have 500 "submaintainers" all directly submitting PRs to me.

thanks
-- PMM

Re: [PATCH 04/16] multi-process: Fix pci_proxy_dev_realize() error handling

[Jag Raman]

> On Jul 20, 2021, at 8:53 AM, Markus Armbruster  wrote:
> 
> The Error ** argument must be NULL, _abort, _fatal, or a
> pointer to a variable containing NULL.  Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
> 
> pci_proxy_dev_realize() is wrong that way: it passes @errp to
> qio_channel_new_fd() without checking for failure.  If it runs into
> another failure, it trips error_setv()'s assertion.
> 
> Fix it to check for failure properly.
> 
> Fixes: 9f8112073aad8e485ac012ee18809457ab7f23a6
> Cc: Elena Ufimtseva 
> Cc: Jagannathan Raman 
> Cc: John G Johnson 
> Cc: Stefan Hajnoczi 
> Signed-off-by: Markus Armbruster 
> ---
> hw/remote/proxy.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)

Acked-by: Jagannathan Raman 

Re: tests/acceptance/multiprocess.py test failure

[Jag Raman]

> On Jul 20, 2021, at 2:39 PM, Cleber Rosa  wrote:
> 
> 
> Jag Raman writes:
> 
>> 
>> Hi Cleber,
>> 
>> We presently don’t have permissions to send a PR to
>> upstream (Peter Maydell).
>> 
>> Presently, we are requesting someone else who has
>> permissions to do PRs on our behalf. We will work
>> on getting permissions to send PRs going forward.
>> 
>> Thank you!
> 
> Hi Jag,
> 
> I'm going to include that patch in an upcoming PR.  Please let me know
> if this is not what you intended.
> 
> PS: I'm not sure I follow what your specific permission problem is, if
> it's technical or something else.  But, in either case, I'd recommend you
> sync the MAINTAINERS file entries with your roles/abilities to maintain
> those files listed.

Hi Cleber,

Thank you for including the patch in a PR.

I have not registered a GPG keys to submit PR - please see following
email for context:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg765788.html

I’ll get started on this process as I can help with smaller patches.

Thank you!
—
Jag

> 
> Best Regards,
> - Cleber.
> 

[Bug 1936977] [NEW] qemu-arm-static crashes "segmentation fault" when running "git clone"

[Zcien Dor]

Public bug reported:

This is a reopen of #1869073 for `qemu-user-static/focal-updates,focal-
security,now 1:4.2-3ubuntu6.17 amd64`.

`git clone` reproducably segfaults in `qemu-arm-static` chroot.

#1869073 mentions this should have been fixed for newer versions of
QEMU, but for `focal` there's no newer version available, even in
`focal-backports`.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1936977

Title:
   qemu-arm-static crashes "segmentation fault" when running "git clone"

Status in QEMU:
  New

Bug description:
  This is a reopen of #1869073 for `qemu-user-static/focal-
  updates,focal-security,now 1:4.2-3ubuntu6.17 amd64`.

  `git clone` reproducably segfaults in `qemu-arm-static` chroot.

  #1869073 mentions this should have been fixed for newer versions of
  QEMU, but for `focal` there's no newer version available, even in
  `focal-backports`.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1936977/+subscriptions

[PATCH for-6.1 v6 17/17] accel/tcg: Record singlestep_enabled in tb->cflags

[Richard Henderson]

Set CF_SINGLE_STEP when single-stepping is enabled.
This avoids the need to flush all tb's when turning
single-stepping on or off.

Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h   | 1 +
 accel/tcg/cpu-exec.c  | 7 ++-
 accel/tcg/translate-all.c | 4 
 accel/tcg/translator.c| 7 +--
 cpu.c | 4 
 5 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6873cce8df..5d1b6d80fb 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -497,6 +497,7 @@ struct TranslationBlock {
 #define CF_COUNT_MASK0x01ff
 #define CF_NO_GOTO_TB0x0200 /* Do not chain with goto_tb */
 #define CF_NO_GOTO_PTR   0x0400 /* Do not chain with goto_ptr */
+#define CF_SINGLE_STEP   0x0800 /* gdbstub single-step in effect */
 #define CF_LAST_IO   0x8000 /* Last insn may be an IO access.  */
 #define CF_MEMI_ONLY 0x0001 /* Only instrument memory ops */
 #define CF_USE_ICOUNT0x0002
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5cc6363f4c..fc895cf51e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -150,10 +150,15 @@ uint32_t curr_cflags(CPUState *cpu)
 uint32_t cflags = cpu->tcg_cflags;
 
 /*
+ * Record gdb single-step.  We should be exiting the TB by raising
+ * EXCP_DEBUG, but to simplify other tests, disable chaining too.
+ *
  * For singlestep and -d nochain, suppress goto_tb so that
  * we can log -d cpu,exec after every TB.
  */
-if (singlestep) {
+if (unlikely(cpu->singlestep_enabled)) {
+cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | CF_SINGLE_STEP | 1;
+} else if (singlestep) {
 cflags |= CF_NO_GOTO_TB | 1;
 } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
 cflags |= CF_NO_GOTO_TB;
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bf82c15aab..bbfcfb698c 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1432,10 +1432,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 }
 QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
 
-if (cpu->singlestep_enabled) {
-max_insns = 1;
-}
-
  buffer_overflow:
 tb = tcg_tb_alloc(tcg_ctx);
 if (unlikely(!tb)) {
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index b45337f3ba..c53a7f8e44 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -38,11 +38,6 @@ bool translator_use_goto_tb(DisasContextBase *db, 
target_ulong dest)
 return false;
 }
 
-/* Suppress goto_tb in the case of single-steping.  */
-if (db->singlestep_enabled) {
-return false;
-}
-
 /* Check for the dest on the same page as the start of the TB.  */
 return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
 }
@@ -60,7 +55,7 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 db->is_jmp = DISAS_NEXT;
 db->num_insns = 0;
 db->max_insns = max_insns;
-db->singlestep_enabled = cpu->singlestep_enabled;
+db->singlestep_enabled = cflags & CF_SINGLE_STEP;
 
 ops->init_disas_context(db, cpu);
 tcg_debug_assert(db->is_jmp == DISAS_NEXT);  /* no early exit */
diff --git a/cpu.c b/cpu.c
index d6ae5ae581..e1799a15bc 100644
--- a/cpu.c
+++ b/cpu.c
@@ -326,10 +326,6 @@ void cpu_single_step(CPUState *cpu, int enabled)
 cpu->singlestep_enabled = enabled;
 if (kvm_enabled()) {
 kvm_update_guest_debug(cpu, 0);
-} else {
-/* must flush all the translated code to avoid inconsistencies */
-/* XXX: only flush what is necessary */
-tb_flush(cpu);
 }
 trace_breakpoint_singlestep(cpu->cpu_index, enabled);
 }
-- 
2.25.1

[PATCH for-6.1 v6 15/17] accel/tcg: Remove TranslatorOps.breakpoint_check

[Richard Henderson]

The hook is now unused, with breakpoints checked outside translation.

Signed-off-by: Richard Henderson 
---
 include/exec/translator.h | 11 ---
 target/arm/helper.h   |  2 --
 target/alpha/translate.c  | 16 
 target/arm/debug_helper.c |  7 ---
 target/arm/translate-a64.c| 25 -
 target/arm/translate.c| 29 -
 target/avr/translate.c| 18 --
 target/cris/translate.c   | 20 
 target/hexagon/translate.c| 17 -
 target/hppa/translate.c   | 11 ---
 target/i386/tcg/translate.c   | 28 
 target/m68k/translate.c   | 18 --
 target/microblaze/translate.c | 18 --
 target/mips/tcg/translate.c   | 19 ---
 target/nios2/translate.c  | 27 ---
 target/openrisc/translate.c   | 17 -
 target/ppc/translate.c| 18 --
 target/riscv/translate.c  | 17 -
 target/rx/translate.c | 14 --
 target/s390x/tcg/translate.c  | 24 
 target/sh4/translate.c| 18 --
 target/sparc/translate.c  | 17 -
 target/tricore/translate.c| 16 
 target/xtensa/translate.c | 17 -
 24 files changed, 424 deletions(-)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index dd9c06d40d..d318803267 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -89,15 +89,6 @@ typedef struct DisasContextBase {
  * @insn_start:
  *  Emit the tcg_gen_insn_start opcode.
  *
- * @breakpoint_check:
- *  When called, the breakpoint has already been checked to match the PC,
- *  but the target may decide the breakpoint missed the address
- *  (e.g., due to conditions encoded in their flags).  Return true to
- *  indicate that the breakpoint did hit, in which case no more breakpoints
- *  are checked.  If the breakpoint did hit, emit any code required to
- *  signal the exception, and set db->is_jmp as necessary to terminate
- *  the main loop.
- *
  * @translate_insn:
  *  Disassemble one instruction and set db->pc_next for the start
  *  of the following instruction.  Set db->is_jmp as necessary to
@@ -113,8 +104,6 @@ typedef struct TranslatorOps {
 void (*init_disas_context)(DisasContextBase *db, CPUState *cpu);
 void (*tb_start)(DisasContextBase *db, CPUState *cpu);
 void (*insn_start)(DisasContextBase *db, CPUState *cpu);
-bool (*breakpoint_check)(DisasContextBase *db, CPUState *cpu,
- const CPUBreakpoint *bp);
 void (*translate_insn)(DisasContextBase *db, CPUState *cpu);
 void (*tb_stop)(DisasContextBase *db, CPUState *cpu);
 void (*disas_log)(const DisasContextBase *db, CPUState *cpu);
diff --git a/target/arm/helper.h b/target/arm/helper.h
index db87d7d537..248569b0cd 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -54,8 +54,6 @@ DEF_HELPER_1(yield, void, env)
 DEF_HELPER_1(pre_hvc, void, env)
 DEF_HELPER_2(pre_smc, void, env, i32)
 
-DEF_HELPER_1(check_breakpoints, void, env)
-
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_2(cpsr_write_eret, void, env, i32)
 DEF_HELPER_1(cpsr_read, i32, env)
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 949ba6ffde..de6c0a8439 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -2967,21 +2967,6 @@ static void alpha_tr_insn_start(DisasContextBase 
*dcbase, CPUState *cpu)
 tcg_gen_insn_start(dcbase->pc_next);
 }
 
-static bool alpha_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
-  const CPUBreakpoint *bp)
-{
-DisasContext *ctx = container_of(dcbase, DisasContext, base);
-
-ctx->base.is_jmp = gen_excp(ctx, EXCP_DEBUG, 0);
-
-/* The address covered by the breakpoint must be included in
-   [tb->pc, tb->pc + tb->size) in order to for it to be
-   properly cleared -- thus we increment the PC here so that
-   the logic setting tb->size below does the right thing.  */
-ctx->base.pc_next += 4;
-return true;
-}
-
 static void alpha_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
 {
 DisasContext *ctx = container_of(dcbase, DisasContext, base);
@@ -3040,7 +3025,6 @@ static const TranslatorOps alpha_tr_ops = {
 .init_disas_context = alpha_tr_init_disas_context,
 .tb_start   = alpha_tr_tb_start,
 .insn_start = alpha_tr_insn_start,
-.breakpoint_check   = alpha_tr_breakpoint_check,
 .translate_insn = alpha_tr_translate_insn,
 .tb_stop= alpha_tr_tb_stop,
 .disas_log  = alpha_tr_disas_log,
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 4a0c479527..2983e36dd3 100644
---