Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

[Thomas Huth]

On 24/04/2024 12.41, Prasad Pandit wrote:

On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.
  
In preparation for such a change, add MemReentrancyGuard * as a

parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different fix patch.
   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
   -> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the latter is 
a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

 https://gitlab.com/qemu-project/qemu/-/issues/556
 i.e.:
 https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3

 HTH,
  Thomas

Re: [PATCH v3 14/29] target/i386: Prefer fast cpu_env() over slower CPU QOM cast macro

[Thomas Huth]

On 30/01/2024 14.01, Igor Mammedov wrote:

On Mon, 29 Jan 2024 17:44:56 +0100
Philippe Mathieu-Daudé  wrote:


Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.



commenting here since, I'm not expert on coccinelle scripts.

On negative side we are permanently loosing type checking in this area.


Not really that much. Have a look at cpu_env(), it has a comment saying:

 "We validate that CPUArchState follows CPUState in cpu-all.h"

So instead of run-time checking, the check should have already been done 
during compile time, i.e. when you have a valid CPUState pointer, it should 
be possible to derive a valid CPUArchState pointer from it without much 
further checking during runtime.



Is it worth it, what gains do we get with this series?


It's a small optimization, but why not?


Side note,
QOM cast expenses you are replacing could be negated by disabling
CONFIG_QOM_CAST_DEBUG without killing type check code when it's enabled.
That way you will speed up not only cpuenv access but also all other casts
across the board.


Yes, but that checking is enabled by default and does not have such 
compile-time checks that could be used instead, so I think Philippe's series 
here is still a good idea.



Signed-off-by: Philippe Mathieu-Daudé 
---

...

  static inline void vmx_clear_nmi_blocking(CPUState *cpu)
  {
-X86CPU *x86_cpu = X86_CPU(cpu);
-CPUX86State *env = _cpu->env;
-
-env->hflags2 &= ~HF2_NMI_MASK;



+cpu_env(cpu)->hflags2 &= ~HF2_NMI_MASK;


this style of de-referencing return value of macro/function
was discouraged in past and preferred way was 'Foo f = CAST(me); f->some_access

(it's just imprint speaking, I don't recall where it comes from)


I agree, though the new code is perfectly valid, it looks nicer if we'd use 
a variable here instead.


 Thomas

Re: [PATCH v4 37/47] hw/net/lasi_i82596: Re-enable build

[Thomas Huth]

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

When converting to the shiny build-system-du-jour, a typo prevented the
last_i82596 driver from being built. Correct the config option name to
re-enable the build. And include "sysemu/sysemu.h" so it actually builds.

Fixes: b1419fa66558 ("meson: convert hw/net")
Signed-off-by: David Woodhouse 
---
  hw/net/lasi_i82596.c | 1 +
  hw/net/meson.build   | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c
index 6a3147fe2d..09e830ba5f 100644
--- a/hw/net/lasi_i82596.c
+++ b/hw/net/lasi_i82596.c
@@ -14,6 +14,7 @@
  #include "qapi/error.h"
  #include "qemu/timer.h"
  #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
  #include "net/eth.h"
  #include "hw/net/lasi_82596.h"
  #include "hw/net/i82596.h"
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 9afceb0619..2b426d3d5a 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -33,7 +33,7 @@ system_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: 
files('mv88w8618_eth.c'))
  system_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c'))
  system_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: 
files('stellaris_enet.c'))
  system_ss.add(when: 'CONFIG_LANCE', if_true: files('lance.c'))
-system_ss.add(when: 'CONFIG_LASI_I82596', if_true: files('lasi_i82596.c'))
+system_ss.add(when: 'CONFIG_LASI_82596', if_true: files('lasi_i82596.c'))
  system_ss.add(when: 'CONFIG_I82596_COMMON', if_true: files('i82596.c'))
  system_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
  system_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2144

Re: [PATCH 2/2] bulk: Prefer fast cpu_env() over slower CPU QOM cast macro

[Thomas Huth]

On 25/01/2024 17.56, Philippe Mathieu-Daudé wrote:

Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/hvf/vmx.h   |  9 +++
  hw/i386/vmmouse.c   |  6 ++---
  hw/i386/xen/xen-hvm.c   |  3 +--
  hw/intc/arm_gicv3_cpuif_common.c|  3 +--
  hw/ppc/mpc8544_guts.c   |  3 +--
  hw/ppc/pnv.c|  3 +--
  hw/ppc/pnv_xscom.c  |  3 +--
  hw/ppc/ppce500_spin.c   |  3 +--
  hw/ppc/spapr.c  |  3 +--
  hw/ppc/spapr_caps.c |  6 ++---
  target/alpha/cpu.c  | 21 +--
  target/alpha/gdbstub.c  |  6 ++---
  target/alpha/helper.c   | 12 +++--
  target/alpha/mem_helper.c   |  9 +++
  target/arm/cpu.c| 15 ---
  target/arm/debug_helper.c   |  6 ++---
  target/arm/gdbstub.c|  6 ++---
  target/arm/gdbstub64.c  |  6 ++---
  target/arm/helper.c |  9 +++
  target/arm/hvf/hvf.c| 12 +++--
  target/arm/kvm.c|  3 +--
  target/arm/ptw.c|  3 +--
  target/arm/tcg/cpu32.c  |  3 +--
  target/avr/cpu.c| 21 +--
  target/avr/gdbstub.c|  6 ++---
  target/avr/helper.c |  9 +++
  target/cris/cpu.c   |  3 +--
  target/cris/gdbstub.c   |  9 +++
  target/cris/helper.c| 12 +++--
  target/cris/translate.c |  3 +--
  target/hppa/cpu.c   |  6 ++---
  target/hppa/int_helper.c|  6 ++---
  target/hppa/mem_helper.c|  3 +--
  target/i386/arch_memory_mapping.c   |  3 +--
  target/i386/cpu-dump.c  |  3 +--
  target/i386/cpu.c   | 36 +
  target/i386/helper.c| 30 +++--
  target/i386/hvf/hvf.c   |  6 ++---
  target/i386/hvf/x86.c   |  3 +--
  target/i386/hvf/x86_emu.c   |  6 ++---
  target/i386/hvf/x86_task.c  | 10 +++
  target/i386/hvf/x86hvf.c|  6 ++---
  target/i386/kvm/kvm.c   |  6 ++---
  target/i386/kvm/xen-emu.c   | 30 +++--
  target/i386/tcg/sysemu/bpt_helper.c |  3 +--
  target/i386/tcg/tcg-cpu.c   | 12 +++--
  target/i386/tcg/user/excp_helper.c  |  3 +--
  target/i386/tcg/user/seg_helper.c   |  3 +--
  target/m68k/cpu.c   | 30 +++--
  target/m68k/gdbstub.c   |  6 ++---
  target/m68k/helper.c|  3 +--
  target/m68k/m68k-semi.c |  6 ++---
  target/m68k/op_helper.c |  9 +++
  target/m68k/translate.c |  3 +--
  target/microblaze/helper.c  |  3 +--
  target/microblaze/translate.c   |  3 +--
  target/mips/cpu.c   |  9 +++
  target/mips/gdbstub.c   |  6 ++---
  target/mips/kvm.c   | 27 +++
  target/mips/sysemu/physaddr.c   |  3 +--
  target/mips/tcg/exception.c |  3 +--
  target/mips/tcg/op_helper.c |  3 +--
  target/mips/tcg/sysemu/special_helper.c |  3 +--
  target/mips/tcg/sysemu/tlb_helper.c |  6 ++---
  target/mips/tcg/translate.c |  3 +--
  target/nios2/cpu.c  |  9 +++
  target/nios2/helper.c   |  3 +--
  target/nios2/nios2-semi.c   |  6 ++---
  target/openrisc/gdbstub.c   |  3 +--
  target/openrisc/interrupt.c |  6 ++---
  target/openrisc/translate.c |  3 +--
  target/ppc/cpu_init.c   |  9 +++
  target/ppc/excp_helper.c|  3 +--
  target/ppc/gdbstub.c| 12 +++--
  target/ppc/kvm.c|  6 ++---
  target/ppc/ppc-qmp-cmds.c   |  3 +--
  target/ppc/user_only_helper.c   |  3 +--
  target/riscv/arch_dump.c|  6 ++---
  target/riscv/cpu.c  | 15 ---
  target/riscv/cpu_helper.c   | 13 +++--
  target/riscv/debug.c|  9 +++
  target/riscv/gdbstub.c  |  6 ++---
  target/riscv/kvm/kvm-cpu.c  |  6 ++---
  target/riscv/tcg/tcg-cpu.c  |  9 +++
  target/riscv/translate.c|  3 +--
  target/rx/gdbstub.c |  6 ++---
  target/rx/helper.c  |  6 ++---
  target/rx/translate.c   |  3 +--
  target/s390x/cpu-dump.c 

Re: [PATCH v3 45/46] net: remove qemu_show_nic_models(), qemu_find_nic_model()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

These old functions can be removed now too. Let net_param_nic() print
the full set of network devices directly, and also make it note that a
list more specific to this platform/config will be available by using
'-nic model=help' instead.

Signed-off-by: David Woodhouse 
---
  include/net/net.h |  3 ---
  net/net.c | 39 ++-
  2 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 1be8b40074..19fb82833c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -203,9 +203,6 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
  int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
-int qemu_show_nic_models(const char *arg, const char *const *models);
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
  const char *alias);
  bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
diff --git a/net/net.c b/net/net.c
index ffd4b42d5a..09ab0889f5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -977,38 +977,6 @@ GPtrArray *qemu_get_nic_models(const char *device_type)
  return nic_models;
  }
  
-int qemu_show_nic_models(const char *arg, const char *const *models)

-{
-int i;
-
-if (!arg || !is_help_option(arg)) {
-return 0;
-}
-
-printf("Available NIC models:\n");
-for (i = 0 ; models[i]; i++) {
-printf("%s\n", models[i]);
-}
-return 1;
-}
-
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model)
-{
-int i;
-
-if (!nd->model)
-nd->model = g_strdup(default_model);
-
-for (i = 0 ; models[i]; i++) {
-if (strcmp(nd->model, models[i]) == 0)
-return i;
-}
-
-error_report("Unsupported NIC model: %s", nd->model);
-return -1;
-}
-
  static int net_init_nic(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
  {
@@ -1791,9 +1759,14 @@ static int net_param_nic(void *dummy, QemuOpts *opts, 
Error **errp)
  }
  if (is_help_option(type)) {
  GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
+int i;
  show_netdevs();
  printf("\n");
-qemu_show_nic_models(type, (const char **)nic_models->pdata);
+printf("Supported NIC models "


Can we please keep "Available" instead of "Supported" ? ... since not each 
NIC is supported on each machine type...


 Thomas



+   "(use -nic model=help for a filtered list):\n");
+for (i = 0 ; nic_models->pdata[i]; i++) {
+printf("%s\n", (char *)nic_models->pdata[i]);
+}
  g_ptr_array_free(nic_models, true);
  exit(0);
  }

Re: [PATCH v3 35/46] hw/mips/mipssim: use qemu_create_nic_device()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

The MIPS SIM platform instantiates its NIC only if a corresponding
configuration exists for it. Use qemu_create_nic_device() function for
that.

Signed-off-by: David Woodhouse 
---
  hw/mips/mipssim.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 01e323904d..16af31648e 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -118,13 +118,15 @@ static void main_cpu_reset(void *opaque)
  }
  }
  
-static void mipsnet_init(int base, qemu_irq irq, NICInfo *nd)

+static void mipsnet_init(int base, qemu_irq irq)
  {
  DeviceState *dev;
  SysBusDevice *s;
  
-dev = qdev_new("mipsnet");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("mipsnet", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -225,9 +227,8 @@ mips_mipssim_init(MachineState *machine)
sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
  }
  
-if (nd_table[0].used)

-/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
-mipsnet_init(0x4200, env->irq[2], _table[0]);
+/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
+mipsnet_init(0x4200, env->irq[2]);
  }
  
  static void mips_mipssim_machine_init(MachineClass *mc)


Reviewed-by: Thomas Huth 

Re: [PATCH v3 46/46] net: make nb_nics and nd_table[] static in net/net.c

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add: "Also remove the leftover definition of host_net_devices which 
has been forgotten to be removed in commit 7cc28cb061040cb089." (or so)


With that:
Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  include/net/net.h | 4 
  net/net.c | 3 +++
  system/globals.c  | 2 --
  3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 19fb82833c..766201c62c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -247,10 +247,6 @@ struct NICInfo {
  int nvectors;
  };
  
-extern int nb_nics;

-extern NICInfo nd_table[MAX_NICS];
-extern const char *host_net_devices[];
-
  /* from net.c */
  extern NetClientStateList net_clients;
  bool netdev_is_modern(const char *optstr);
diff --git a/net/net.c b/net/net.c
index 09ab0889f5..71cccb19da 100644
--- a/net/net.c
+++ b/net/net.c
@@ -77,6 +77,9 @@ static NetdevQueue nd_queue = 
QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
  static GHashTable *nic_model_help;
  
+static int nb_nics;

+static NICInfo nd_table[MAX_NICS];
+
  /***/
  /* network device redirectors */
  
diff --git a/system/globals.c b/system/globals.c

index e83b5428d1..b6d4e72530 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -36,8 +36,6 @@ int display_opengl;
  const char* keyboard_layout;
  bool enable_mlock;
  bool enable_cpu_pm;
-int nb_nics;
-NICInfo nd_table[MAX_NICS];
  int autostart = 1;
  int vga_interface_type = VGA_NONE;
  bool vga_interface_created;

Re: [PATCH v3 44/46] hw/pci: remove pci_nic_init_nofail()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

This function is no longer used.

Signed-off-by: David Woodhouse 
---
  hw/pci/pci.c | 72 
  include/hw/pci/pci.h |  3 --
  2 files changed, 75 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v3 43/46] net: remove qemu_check_nic_model()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add a short patch description à la "All callers have been converted 
in the previous patches, so this is not required anymore".


With that:
Reviewed-by: Thomas Huth 



Signed-off-by: David Woodhouse 
---
  include/net/net.h |  1 -
  net/net.c | 13 -
  2 files changed, 14 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 31e63d1f0d..1be8b40074 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -204,7 +204,6 @@ int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
  int qemu_show_nic_models(const char *arg, const char *const *models);
-void qemu_check_nic_model(NICInfo *nd, const char *model);
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
diff --git a/net/net.c b/net/net.c
index 4651b3f443..ffd4b42d5a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -992,19 +992,6 @@ int qemu_show_nic_models(const char *arg, const char 
*const *models)
  return 1;
  }
  
-void qemu_check_nic_model(NICInfo *nd, const char *model)

-{
-const char *models[2];
-
-models[0] = model;
-models[1] = NULL;
-
-if (qemu_show_nic_models(nd->model, models))
-exit(0);
-if (qemu_find_nic_model(nd, models, model) < 0)
-exit(1);
-}
-
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model)
  {

Re: [PATCH v3 34/46] hw/microblaze: use qemu_configure_nic_device()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/microblaze/petalogix_ml605_mmu.c  | 3 +--
  hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index fb7889cf67..0f5fabc32e 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -133,7 +133,6 @@ petalogix_ml605_init(MachineState *machine)
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
  /* axi ethernet and dma initialization. */

-qemu_check_nic_model(_table[0], "xlnx.axi-ethernet");
  eth0 = qdev_new("xlnx.axi-ethernet");
  dma = qdev_new("xlnx.axi-dma");
  
@@ -145,7 +144,7 @@ petalogix_ml605_init(MachineState *machine)

"axistream-connected-target", NULL);
  cs = object_property_get_link(OBJECT(dma),
"axistream-control-connected-target", NULL);
-qdev_set_nic_properties(eth0, _table[0]);
+qemu_configure_nic_device(eth0, true, NULL);
  qdev_prop_set_uint32(eth0, "rxmem", 0x1000);
  qdev_prop_set_uint32(eth0, "txmem", 0x1000);
  object_property_set_link(OBJECT(eth0), "axistream-connected", ds,
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 505639c298..dad46bd7f9 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -114,9 +114,8 @@ petalogix_s3adsp1800_init(MachineState *machine)
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
-qemu_check_nic_model(_table[0], "xlnx.xps-ethernetlite");

  dev = qdev_new("xlnx.xps-ethernetlite");
-qdev_set_nic_properties(dev, _table[0]);
+qemu_configure_nic_device(dev, true, NULL);
  qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
  qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);


Reviewed-by: Thomas Huth 

Re: [PATCH v3 32/46] hw/m68k/mcf5208: use qemu_create_nic_device()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/m68k/mcf5208.c | 19 ++-
  1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index d22d8536db..0cfb806c20 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -206,16 +206,16 @@ static void mcf5208_sys_init(MemoryRegion *address_space, 
qemu_irq *pic)
  }
  }
  
-static void mcf_fec_init(MemoryRegion *sysmem, NICInfo *nd, hwaddr base,

- qemu_irq *irqs)
+static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, qemu_irq *irqs)
  {
  DeviceState *dev;
  SysBusDevice *s;
  int i;
  
-qemu_check_nic_model(nd, TYPE_MCF_FEC_NET);

-dev = qdev_new(TYPE_MCF_FEC_NET);
-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device(TYPE_MCF_FEC_NET, true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -267,14 +267,7 @@ static void mcf5208evb_init(MachineState *machine)
  
  mcf5208_sys_init(address_space_mem, pic);
  
-if (nb_nics > 1) {

-error_report("Too many NICs");
-exit(1);
-}


I wonder whether we'd need a different mechanism to specify the maximum 
amount of on-board NICs now... Anyway, we can also think of that later, so:


Reviewed-by: Thomas Huth 

Re: [PATCH v3 03/46] net: add qemu_create_nic_bus_devices()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

This will instantiate any NICs which live on a given bus type. Each bus
is allowed *one* substitution (for PCI it's virtio → virtio-net-pci, for
Xen it's xen → xen-net-device; no point in overengineering it unless we
actually want more).

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  include/net/net.h |  3 +++
  net/net.c | 53 +++
  2 files changed, 56 insertions(+)


Reviewed-by: Thomas Huth 

Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

[Thomas Huth]

On 26/01/2024 15.34, David Woodhouse wrote:

On Fri, 2024-01-26 at 15:24 +0100, Thomas Huth wrote:

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

   if (!nd->model)
   nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so
that we've got it recorded somewhere that nd->model might be left at NULL
afterwards, but that there are no further consumers, so it should be fine.


Makes sense.

https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=694f82bc09080
now says:


net: add qemu_{configure,create}_nic_device(), qemu_find_nic_info()

Most code which directly accesses nd_table[] and nb_nics uses them for
one of two things. Either "I have created a NIC device and I'd like a
configuration for it", or "I will create a NIC device *if* there is a
configuration for it".  With some variants on the theme around whether
they actually *check* if the model specified in the configuration is
the right one.

Provide functions which perform both of those, allowing platforms to
be a little more consistent and as a step towards making nd_table[]
and nb_nics private to the net code.

One might argue that platforms ought to be consistent about whether
they create the unconfigured devices or not, but making significant
user-visible changes is explicitly *not* the intent right now.

The new functions leave the 'model' field of the NICInfo as NULL after
using it for the default NIC model, unlike the qemu_check_nic_model()
function which does set nd->model to match default_model explicitly.
This is acceptable because there is no code which consumes nd->model
except this NIC-matching code in net/net.c, and no reasonable excuse
for any code wanting to use nd->model in future.


With that feel free to add:
Reviewed-by: Thomas Huth 

Re: [PATCH v3 02/46] net: report list of available models according to platform

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

By noting the models for which a configuration was requested, we can give
the user an accurate list of which NIC models were actually available on
the platform/configuration that was otherwise chosen.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  net/net.c | 94 +++
  1 file changed, 94 insertions(+)

diff --git a/net/net.c b/net/net.c
index aeb7f573fc..962904eaef 100644
--- a/net/net.c
+++ b/net/net.c
@@ -75,6 +75,8 @@ typedef QSIMPLEQ_HEAD(, NetdevQueueEntry) NetdevQueue;
  
  static NetdevQueue nd_queue = QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
+static GHashTable *nic_model_help;

+
  /***/
  /* network device redirectors */
  
@@ -1087,12 +1089,94 @@ static int net_init_nic(const Netdev *netdev, const char *name,

  return idx;
  }
  
+static gboolean add_nic_result(gpointer key, gpointer value, gpointer user_data)

+{
+GPtrArray *results = user_data;
+GPtrArray *alias_list = value;
+const char *model = key;
+char *result;
+
+if (!alias_list) {
+result = g_strdup(model);
+} else {
+GString *result_str = g_string_new(model);
+int i;
+
+g_string_append(result_str, " (aka ");


It's an abbreviation, so I'd rather use "a.k.a." instead of "aka".

Apart from that, the patch looks reasonable to me.

 Thomas


+for (i = 0; i < alias_list->len; i++) {
+if (i) {
+g_string_append(result_str, ", ");
+}
+g_string_append(result_str, alias_list->pdata[i]);
+}
+g_string_append(result_str, ")");
+result = result_str->str;
+g_string_free(result_str, false);
+g_ptr_array_unref(alias_list);
+}
+g_ptr_array_add(results, result);
+return true;
+}

...

Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

[Thomas Huth]

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

  if (!nd->model)
  nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so 
that we've got it recorded somewhere that nd->model might be left at NULL 
afterwards, but that there are no further consumers, so it should be fine?


 Thomas

Re: [PATCH v3 14/46] hw/mips/loongson3_virt: use pci_init_nic_devices()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/mips/loongson3_virt.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 33eae01eca..caedde2df0 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -451,9 +451,7 @@ static inline void loongson3_virt_devices_init(MachineState 
*machine,
  usb_create_simple(usb_bus_find(-1), "usb-tablet");
  }
  
-for (i = 0; i < nb_nics; i++) {

-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  static void mips_loongson3_virt_init(MachineState *machine)


Reviewed-by: Thomas Huth 

Re: [PATCH v3 13/46] hw/mips/malta: use pci_init_nic_devices()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The Malta board setup code would previously place the first NIC into PCI
slot 11 if was a PCNet card, and the rest (including the first if it was
anything other than a PCNet card) would be dynamically assigned.

Now it will place any PCNet NIC into slot 11, and then anything else will
be dynamically assigned.

Signed-off-by: David Woodhouse 
---
  hw/mips/malta.c | 15 +++
  1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index d22bb1edef..af74008c82 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -612,18 +612,9 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion 
*address_space,
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "pcnet") == 0))
-/* The malta board has a PCNet card using PCI SLOT 11 */
-default_devaddr = "0b";
-
-pci_nic_init_nofail(nd, pci_bus, "pcnet", default_devaddr);
-}
+/* The malta board has a PCNet card using PCI SLOT 11 */
+pci_init_nic_in_slot(pci_bus, "pcnet", NULL, "0b");
+    pci_init_nic_devices(pci_bus, "pcnet");
  }


Reviewed-by: Thomas Huth 

Philippe, could you maybe have a look at this, too?

Re: [PATCH v3 12/46] hw/mips/fuloong2e: use pci_init_nic_devices()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The previous behaviour was: *if* the first NIC specified on the command
line was an RTL8139 (or unspecified model) then it gets assigned to PCI
slot 7, which is where the Fuloong board had an RTL8139. All other
devices (including the first, if it was specified a anything other then
an rtl8319) get dynamically assigned on the bus.

The new behaviour is subtly different: If the first NIC was given a
specific model *other* than rtl8139, and a subsequent NIC was not,
then the rtl8139 (or unspecified) NIC will go to slot 7 and the rest
will be dynamically assigned.


Sounds fine for me ... Philippe, what do you think?

Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  hw/mips/fuloong2e.c | 16 +++-
  1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 97b2c8ed8e..a45aac368c 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -201,19 +201,9 @@ static void main_cpu_reset(void *opaque)
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "rtl8139") == 0)) {
-/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
-default_devaddr = "07";
-}
-
-pci_nic_init_nofail(nd, pci_bus, "rtl8139", default_devaddr);
-}
+/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
+pci_init_nic_in_slot(pci_bus, "rtl8139", NULL, "07");
+pci_init_nic_devices(pci_bus, "rtl8139");
  }
  
  static void mips_fuloong2e_init(MachineState *machine)

Re: [PATCH v3 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

[Thomas Huth]

On 26/01/2024 12.25, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:20 +0100, Thomas Huth wrote:

On 26/01/2024 12.13, David Woodhouse wrote:

On Fri, 2024-01-26 at 11:43 +0100, Thomas Huth wrote:

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Eliminate direct access to nd_table[] and nb_nics by processing the the
Xen and ISA NICs first and then calling pci_init_nic_devices() for the
rest.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
    hw/i386/pc.c    | 26 --
    include/hw/net/ne2000-isa.h |  2 --
    2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 496498df3a..d80c536d88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -658,8 +658,11 @@ static void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
    {
    static int nb_ne2k = 0;

-    if (nb_ne2k == NE2000_NB_MAX)

+    if (nb_ne2k == NE2000_NB_MAX) {
+    error_setg(_fatal,
+   "maximum number of ISA NE2000 devices exceeded");
    return;
+    }


error_setg(_fatal, ...) quits QEMU, so the "return;" does not make
much sense anymore.
Now, according to include/qapi/error.h :

    * Please don't error_setg(_fatal, ...), use error_report() and
    * exit(), because that's more obvious.

So I'd suggest to do that instead.


It's going slightly in the opposite direction to what's requested in
https://lore.kernel.org/qemu-devel/34e2c0c6-4e04-486a-8e1f-4afdc461a...@linaro.org/

I was thinking that a future patch would let the _fatal be an
Error** passed in by the caller, and not actually hard-coded to be
fatal at all.

But sure, unless Philippe objects I'm happy to do it as you show above.


Now that you mention it, I'd also prefer having an Error** parameter to the
function instead, that's certainly cleaner. So if you don't mind, please
follow Philippe's suggestion instead!


Right. There's a whole bunch of functions to untangle, that take an
Error** but don't return success/failure independently as they should.
Or don't even take the Error**.

Rather than trying to fix that as part of this series, this was my
compromise — making it easy to switch that explicit _fatal out
for a function parameter, but not trying to shave that part of the yak
myself just yet.


I think the nicest compromise is to add the "Error **errp" to the 
pc_init_ne2k_isa() and change the caller to pass in _fatal there ... 
further clean-up (passing the error even up further in the stack) is out of 
scope of this series, indeed.


 Thomas

Re: [PATCH v3 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

[Thomas Huth]

On 26/01/2024 12.13, David Woodhouse wrote:

On Fri, 2024-01-26 at 11:43 +0100, Thomas Huth wrote:

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Eliminate direct access to nd_table[] and nb_nics by processing the the
Xen and ISA NICs first and then calling pci_init_nic_devices() for the
rest.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
   hw/i386/pc.c    | 26 --
   include/hw/net/ne2000-isa.h |  2 --
   2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 496498df3a..d80c536d88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -658,8 +658,11 @@ static void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
   {
   static int nb_ne2k = 0;
   
-    if (nb_ne2k == NE2000_NB_MAX)

+    if (nb_ne2k == NE2000_NB_MAX) {
+    error_setg(_fatal,
+   "maximum number of ISA NE2000 devices exceeded");
   return;
+    }


error_setg(_fatal, ...) quits QEMU, so the "return;" does not make
much sense anymore.
Now, according to include/qapi/error.h :

   * Please don't error_setg(_fatal, ...), use error_report() and
   * exit(), because that's more obvious.

So I'd suggest to do that instead.


It's going slightly in the opposite direction to what's requested in
https://lore.kernel.org/qemu-devel/34e2c0c6-4e04-486a-8e1f-4afdc461a...@linaro.org/

I was thinking that a future patch would let the _fatal be an
Error** passed in by the caller, and not actually hard-coded to be
fatal at all.

But sure, unless Philippe objects I'm happy to do it as you show above.


Now that you mention it, I'd also prefer having an Error** parameter to the 
function instead, that's certainly cleaner. So if you don't mind, please 
follow Philippe's suggestion instead!


 Thanks,
  Thomas

Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Most code which directly accesses nd_table[] and nb_nics uses them for
one of two things. Either "I have created a NIC device and I'd like a
configuration for it", or "I will create a NIC device *if* there is a
configuration for it".  With some variants on the theme around whether
they actually *check* if the model specified in the configuration is
the right one.

Provide functions which perform both of those, allowing platforms to
be a little more consistent and as a step towards making nd_table[]
and nb_nics private to the net code.

Also export the qemu_find_nic_info() helper, as some platforms have
special cases they need to handle.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  include/net/net.h |  7 ++-
  net/net.c | 51 +++
  2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/include/net/net.h b/include/net/net.h
index ffbd2c8d56..25ea83fd12 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -207,7 +207,12 @@ int qemu_show_nic_models(const char *arg, const char 
*const *models);
  void qemu_check_nic_model(NICInfo *nd, const char *model);
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model);
-
+NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
+const char *alias);
+bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
+   const char *alias);
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+const char *alias);
  void print_net_client(Monitor *mon, NetClientState *nc);
  void net_socket_rs_init(SocketReadState *rs,
  SocketReadStateFinalize *finalize,
diff --git a/net/net.c b/net/net.c
index 0520bc1681..aeb7f573fc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1087,6 +1087,57 @@ static int net_init_nic(const Netdev *netdev, const char 
*name,
  return idx;
  }
  
+NICInfo *qemu_find_nic_info(const char *typename, bool match_default,

+const char *alias)
+{
+NICInfo *nd;
+int i;
+
+for (i = 0; i < nb_nics; i++) {
+nd = _table[i];
+
+if (!nd->used || nd->instantiated) {
+continue;
+}
+
+if ((match_default && !nd->model) || !g_strcmp0(nd->model, typename)
+|| (alias && !g_strcmp0(nd->model, alias))) {
+return nd;
+}
+}
+return NULL;
+}
+
+
+/* "I have created a device. Please configure it if you can" */
+bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
+   const char *alias)
+{
+NICInfo *nd = qemu_find_nic_info(object_get_typename(OBJECT(dev)),
+ match_default, alias);
+
+if (nd) {
+qdev_set_nic_properties(dev, nd);
+return true;
+}
+return false;
+}
+
+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+const char *alias)
+{
+NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+DeviceState *dev;
+
+if (!nd) {
+return NULL;
+}


The qemu_check_nic_model() function that was used in some code that you 
turned into qemu_create_nic_device() used to set:


if (!nd->model)
nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by 
qemu_check_nic_model())


Should we do that also here to make sure that nd->model is not NULL afterwards?

(same question likely applies to qemu_configure_nic_device() )

Apart from that, the patch looks fine to me.

 Thomas


+dev = qdev_new(typename);
+qdev_set_nic_properties(dev, nd);
+return dev;
+}
  
  static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(

  const Netdev *netdev,

Re: [PATCH v3 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

[Thomas Huth]

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Eliminate direct access to nd_table[] and nb_nics by processing the the
Xen and ISA NICs first and then calling pci_init_nic_devices() for the
rest.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  hw/i386/pc.c| 26 --
  include/hw/net/ne2000-isa.h |  2 --
  2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 496498df3a..d80c536d88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -658,8 +658,11 @@ static void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
  {
  static int nb_ne2k = 0;
  
-if (nb_ne2k == NE2000_NB_MAX)

+if (nb_ne2k == NE2000_NB_MAX) {
+error_setg(_fatal,
+   "maximum number of ISA NE2000 devices exceeded");
  return;
+}


error_setg(_fatal, ...) quits QEMU, so the "return;" does not make 
much sense anymore.

Now, according to include/qapi/error.h :

 * Please don't error_setg(_fatal, ...), use error_report() and
 * exit(), because that's more obvious.

So I'd suggest to do that instead.

 Thanks,
  Thomas



  isa_ne2000_init(bus, ne2000_io[nb_ne2k],
  ne2000_irq[nb_ne2k], nd);
  nb_ne2k++;

Re: [PATCH v3 40/46] hw/s390x/s390-virtio-ccw: use qemu_create_nic_device()

[Thomas Huth]

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/s390x/s390-virtio-ccw.c | 11 ++-
  1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1169e20b94..202c378131 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -221,16 +221,9 @@ static void s390_init_ipl_dev(const char *kernel_filename,
  
  static void s390_create_virtio_net(BusState *bus, const char *name)

  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-DeviceState *dev;
-
-qemu_check_nic_model(nd, "virtio");
+DeviceState *dev;
  
-dev = qdev_new(name);

-qdev_set_nic_properties(dev, nd);
+while ((dev = qemu_create_nic_device(name, true, "virtio"))) {
  qdev_realize_and_unref(dev, bus, _fatal);
  }
  }


Acked-by: Thomas Huth 

Re: [PATCH] include/hw/xen: Use more inclusive language in comment

[Thomas Huth]

On 10/11/2023 10.30, Jan Beulich wrote:

On 09.11.2023 18:40, Thomas Huth wrote:

--- a/include/hw/xen/interface/hvm/params.h
+++ b/include/hw/xen/interface/hvm/params.h
@@ -255,7 +255,7 @@
   * Note that 'mixed' mode has not been evaluated for safety from a
   * security perspective.  Before using this mode in a
   * security-critical environment, each subop should be evaluated for
- * safety, with unsafe subops blacklisted in XSM.
+ * safety, with unsafe subops blocked in XSM.


To avoid another round trip when you send the patch against xen.git, as
already asked for by others, I'd like to point out that the wording
change isn't describing things sufficiently similarly: "blocked" reads
as if XSM would do so all by itself, whereas "blacklisted" has an
indication that something needs to be done for XSM to behave in the
intended way. Minimally I'd suggest "suitably blocked via", but perhaps
yet better wording can be thought of.


Ok, could then please someone from you Xen guys get this fixed with 
appropriate wording in the xen.git repo? I never checked out that repo 
before and before I now spend hours and hours to figure out how to 
contribute a patch to Xen, just to replace a single word, it's way easier if 
someone with pre-existing Xen experience is taking care of this.


 Thanks,
  Thomas

[PATCH] include/hw/xen: Use more inclusive language in comment

[Thomas Huth]

Let's improve the wording here.

Signed-off-by: Thomas Huth 
---
 include/hw/xen/interface/hvm/params.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/xen/interface/hvm/params.h 
b/include/hw/xen/interface/hvm/params.h
index a22b4ed45d..9bcb40284c 100644
--- a/include/hw/xen/interface/hvm/params.h
+++ b/include/hw/xen/interface/hvm/params.h
@@ -255,7 +255,7 @@
  * Note that 'mixed' mode has not been evaluated for safety from a
  * security perspective.  Before using this mode in a
  * security-critical environment, each subop should be evaluated for
- * safety, with unsafe subops blacklisted in XSM.
+ * safety, with unsafe subops blocked in XSM.
  */
 #define HVM_PARAM_ALTP2M   35
 #define XEN_ALTP2M_disabled  0
-- 
2.41.0

Re: [QEMU][PATCH] gitlab-ci.d/crossbuilds: Drop the '--disable-tcg' configuration for xen

[Thomas Huth]

On 11/04/2023 23.04, Vikram Garhwal wrote:

Xen is supported for aarch64 via xenpvh machine. disable-tcg option fails the
build for aarch64 target.

Link for xen on arm patch series: 
https://mail.gnu.org/archive/html/qemu-devel/2023-02/msg03979.html

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

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 61b8ac86ee..6867839248 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -186,7 +186,7 @@ cross-amd64-xen-only:
variables:
  IMAGE: debian-amd64-cross
  ACCEL: xen
-EXTRA_CONFIGURE_OPTS: --disable-tcg --disable-kvm
+EXTRA_CONFIGURE_OPTS: --disable-kvm
  
  cross-arm64-xen-only:

extends: .cross_accel_build_job
@@ -195,4 +195,4 @@ cross-arm64-xen-only:
variables:
  IMAGE: debian-arm64-cross
  ACCEL: xen
-EXTRA_CONFIGURE_OPTS: --disable-tcg --disable-kvm
+EXTRA_CONFIGURE_OPTS: --disable-kvm


This patch looks wrong. I'm pretty sure we wanted to test the build without 
TCG here. Building with TCG enabled is already done in other jobs. So 
instead of removing "--disable-tcg" here the question is rather: Why does it 
not build with this flag anymore? Can those problems be fixed instead?


 Thomas

Re: [PATCH v7 4/6] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded

[Thomas Huth]

On 13/03/2023 09.24, Alexander Bulekov wrote:

This protects devices from bh->mmio reentrancy issues.

Thanks: Thomas Huth  for diagnosing OS X test failure.
Reviewed-by: Darren Kenny 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Paul Durrant 
Signed-off-by: Alexander Bulekov 
---
  hw/9pfs/xen-9p-backend.c| 5 -
  hw/block/dataplane/virtio-blk.c | 3 ++-
  hw/block/dataplane/xen-block.c  | 5 +++--
  hw/char/virtio-serial-bus.c | 3 ++-
  hw/display/qxl.c| 9 ++---
  hw/display/virtio-gpu.c | 6 --
  hw/ide/ahci.c   | 3 ++-
  hw/ide/ahci_internal.h  | 1 +
  hw/ide/core.c   | 4 +++-
  hw/misc/imx_rngc.c  | 6 --
  hw/misc/macio/mac_dbdma.c   | 2 +-
  hw/net/virtio-net.c | 3 ++-
  hw/nvme/ctrl.c  | 6 --
  hw/scsi/mptsas.c| 3 ++-
  hw/scsi/scsi-bus.c  | 3 ++-
  hw/scsi/vmw_pvscsi.c| 3 ++-
  hw/usb/dev-uas.c| 3 ++-
  hw/usb/hcd-dwc2.c   | 3 ++-
  hw/usb/hcd-ehci.c   | 3 ++-
  hw/usb/hcd-uhci.c   | 2 +-
  hw/usb/host-libusb.c| 6 --
  hw/usb/redirect.c   | 6 --
  hw/usb/xen-usb.c| 3 ++-
  hw/virtio/virtio-balloon.c  | 5 +++--
  hw/virtio/virtio-crypto.c   | 3 ++-
  25 files changed, 66 insertions(+), 33 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v6 4/4] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded

[Thomas Huth]

On 05/02/2023 05.07, Alexander Bulekov wrote:

This protects devices from bh->mmio reentrancy issues.

Reviewed-by: Darren Kenny 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Alexander Bulekov 
---

...

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 65c4979c3c..f077c1b255 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -441,7 +441,9 @@ static int xen_9pfs_connect(struct XenLegacyDevice *xendev)
  xen_9pdev->rings[i].ring.out = xen_9pdev->rings[i].data +
 XEN_FLEX_RING_SIZE(ring_order);
  
-xen_9pdev->rings[i].bh = qemu_bh_new(xen_9pfs_bh, _9pdev->rings[i]);

+xen_9pdev->rings[i].bh = qemu_bh_new_guarded(xen_9pfs_bh,
+ _9pdev->rings[i],
+ 
(xen_9pdev)->mem_reentrancy_guard);


xen_9pdev is not derived from DeviceState, so you must not cast it with 
DEVICE().



diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 7ce001cacd..37091150cb 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1508,7 +1508,8 @@ static void ahci_cmd_done(const IDEDMA *dma)
  ahci_write_fis_d2h(ad);
  
  if (ad->port_regs.cmd_issue && !ad->check_bh) {

-ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad);
+ad->check_bh = qemu_bh_new_guarded(ahci_check_cmd_bh, ad,
+   (ad)->mem_reentrancy_guard);
  qemu_bh_schedule(ad->check_bh);
  }
  }


Dito - ad is not derived from DeviceState, so you cannot use DEVICE() here.

(This was causing the crash in the macOS CI job)


diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5d1039378f..8c8d1a8ec2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -519,7 +519,8 @@ BlockAIOCB *ide_issue_trim(
  
  iocb = blk_aio_get(_aiocb_info, s->blk, cb, cb_opaque);

  iocb->s = s;
-iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
+iocb->bh = qemu_bh_new_guarded(ide_trim_bh_cb, iocb,
+   (s)->mem_reentrancy_guard);


IDEState s is also not directly derived from DeviceState. Not sure, but 
maybe you can get to the device here in a similar way that is done in 
ide_identify() :


 IDEDevice *dev = s->unit ? s->bus->slave : s->bus->master;

?


diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 746f07c4d2..309cebacc6 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -908,8 +908,9 @@ static void virtio_balloon_device_realize(DeviceState *dev, 
Error **errp)
  precopy_add_notifier(>free_page_hint_notify);
  
  object_ref(OBJECT(s->iothread));

-s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
- virtio_ballloon_get_free_page_hints, s);
+s->free_page_bh = 
aio_bh_new_guarded(iothread_get_aio_context(s->iothread),
+ 
virtio_ballloon_get_free_page_hints, s,
+ (s)->mem_reentrancy_guard);


You could use "dev" instead of "s" here to get rid of the DEVICE() cast.

The remaining changes look fine to me.

 Thomas

Re: [PATCH v4 2/5] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

On 06/03/2023 15.06, Daniel P. Berrangé wrote:

On Mon, Mar 06, 2023 at 02:48:16PM +0100, Thomas Huth wrote:

On 06/03/2023 10.27, Daniel P. Berrangé wrote:

On Mon, Mar 06, 2023 at 09:46:55AM +0100, Thomas Huth wrote:

[...] If a 32-bit CPU guest
+environment should be enforced, you can switch off the "long mode" CPU
+flag, e.g. with ``-cpu max,lm=off``.


I had the idea to check this today and this is not quite sufficient,

[...]

A further difference is that qemy-system-i686 does not appear to enable
the 'syscall' flag, but I've not figured out where that difference is
coming from in the code.


I think I just spotted this by accident in target/i386/cpu.c
around line 637:

#ifdef TARGET_X86_64
#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
#else
#define TCG_EXT2_X86_64_FEATURES 0
#endif


Hmm, so right now the difference between qemu-system-i386 and
qemu-system-x86_64 is based on compile time conditionals. So we
have the burden of building everything twice and also a burden
of testing everything twice.

If we eliminate qemu-system-i386 we get rid of our own burden,
but users/mgmt apps need to adapt to force qemu-system-x86_64
to present a 32-bit system.

What about if we had qemu-system-i386 be a hardlink to
qemu-system-x86_64, and then changed behaviour based off the
executed binary name ?


We could also simply provide a shell script that runs:

 qemu-system-x86_64 -cpu qemu32 $*

... that'd sounds like the simplest solution to me.

 Thomas

Re: [PATCH v4 2/5] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

On 06/03/2023 10.27, Daniel P. Berrangé wrote:

On Mon, Mar 06, 2023 at 09:46:55AM +0100, Thomas Huth wrote:

[...] If a 32-bit CPU guest
+environment should be enforced, you can switch off the "long mode" CPU
+flag, e.g. with ``-cpu max,lm=off``.


I had the idea to check this today and this is not quite sufficient,

[...]

A further difference is that qemy-system-i686 does not appear to enable
the 'syscall' flag, but I've not figured out where that difference is
coming from in the code.


I think I just spotted this by accident in target/i386/cpu.c
around line 637:

#ifdef TARGET_X86_64
#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
#else
#define TCG_EXT2_X86_64_FEATURES 0
#endif

 Thomas

Re: [PATCH v4 2/5] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

On 06/03/2023 10.27, Daniel P. Berrangé wrote:

On Mon, Mar 06, 2023 at 09:46:55AM +0100, Thomas Huth wrote:

Aside from not supporting KVM on 32-bit hosts, the qemu-system-x86_64
binary is a proper superset of the qemu-system-i386 binary. With the
32-bit host support being deprecated, it is now also possible to
deprecate the qemu-system-i386 binary.

With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

  https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
  docs/about/deprecated.rst | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1ca9dc33d6..c4fcc6b33c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -34,6 +34,20 @@ deprecating the build option and no longer defend it in CI. 
The
  ``--enable-gcov`` build option remains for analysis test case
  coverage.
  
+``qemu-system-i386`` binary (since 8.0)

+'''
+
+The ``qemu-system-i386`` binary was mainly useful for running with KVM
+on 32-bit x86 hosts, but most Linux distributions already removed their
+support for 32-bit x86 kernels, so hardly anybody still needs this. The
+``qemu-system-x86_64`` binary is a proper superset and can be used to
+run 32-bit guests by selecting a 32-bit CPU model, including KVM support
+on x86_64 hosts. Thus users are recommended to reconfigure their systems
+to use the ``qemu-system-x86_64`` binary instead. If a 32-bit CPU guest
+environment should be enforced, you can switch off the "long mode" CPU
+flag, e.g. with ``-cpu max,lm=off``.


I had the idea to check this today and this is not quite sufficient,
because we have code that changes the family/model/stepping for
'max' which is target dependent:

#ifdef TARGET_X86_64
 object_property_set_int(OBJECT(cpu), "family", 15, _abort);
 object_property_set_int(OBJECT(cpu), "model", 107, _abort);
 object_property_set_int(OBJECT(cpu), "stepping", 1, _abort);
#else
 object_property_set_int(OBJECT(cpu), "family", 6, _abort);
 object_property_set_int(OBJECT(cpu), "model", 6, _abort);
 object_property_set_int(OBJECT(cpu), "stepping", 3, _abort);
#endif

The former is a 64-bit AMD model and the latter is a 32-bit model.

Seems LLVM was sensitive to this distinction to some extent:

https://gitlab.com/qemu-project/qemu/-/issues/191

A further difference is that qemy-system-i686 does not appear to enable
the 'syscall' flag, but I've not figured out where that difference is
coming from in the code.


Ugh, ok. I gave it a quick try with a patch like this:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4344,15 +4344,15 @@ static void max_x86_cpu_initfn(Object *obj)
  */
 object_property_set_str(OBJECT(cpu), "vendor", CPUID_VENDOR_AMD,
 _abort);
-#ifdef TARGET_X86_64
-object_property_set_int(OBJECT(cpu), "family", 15, _abort);
-object_property_set_int(OBJECT(cpu), "model", 107, _abort);
-object_property_set_int(OBJECT(cpu), "stepping", 1, _abort);
-#else
-object_property_set_int(OBJECT(cpu), "family", 6, _abort);
-object_property_set_int(OBJECT(cpu), "model", 6, _abort);
-object_property_set_int(OBJECT(cpu), "stepping", 3, _abort);
-#endif
+if (object_property_get_bool(obj, "lm", _abort)) {
+object_property_set_int(obj, "family", 15, _abort);
+object_property_set_int(obj, "model", 107, _abort);
+object_property_set_int(obj, "stepping", 1, _abort);
+} else {
+object_property_set_int(obj, "family", 6, _abort);
+object_property_set_int(obj, "model", 6, _abort);
+object_property_set_int(obj, "stepping", 3, _abort);
+}
 object_property_set_str(OBJECT(cpu), "model-id",
 "QEMU TCG CPU version " QEMU_HW_VERSION,
 _abort);

... but it seems like the "lm" property is not initialized
there yet, so this does not work... :-/

Giving that we have soft-freeze tomorrow, let's ignore this patch
for now and revisit this topic during the 8.1 cycle. But I'll
queue the other 4 patches to get some pressure out of our CI
during the freeze time.

 Thomas

[PATCH v4 5/5] gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

[Thomas Huth]

Hardly anybody still uses 32-bit arm environments for running QEMU,
so let's stop wasting our scarce CI minutes with these jobs.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 14 --
 1 file changed, 14 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index a25cb87ae4..61b8ac86ee 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -1,13 +1,6 @@
 include:
   - local: '/.gitlab-ci.d/crossbuild-template.yml'
 
-cross-armel-system:
-  extends: .cross_system_build_job
-  needs:
-job: armel-debian-cross-container
-  variables:
-IMAGE: debian-armel-cross
-
 cross-armel-user:
   extends: .cross_user_build_job
   needs:
@@ -15,13 +8,6 @@ cross-armel-user:
   variables:
 IMAGE: debian-armel-cross
 
-cross-armhf-system:
-  extends: .cross_system_build_job
-  needs:
-job: armhf-debian-cross-container
-  variables:
-IMAGE: debian-armhf-cross
-
 cross-armhf-user:
   extends: .cross_user_build_job
   needs:
-- 
2.31.1

[PATCH v4 0/5] Deprecate system emulation support for 32-bit x86 and arm hosts

[Thomas Huth]

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. Hardly anybody uses
QEMU system emulation on 32-bit x86 and arm hosts anymore, so it's
time to deprecate these environments now.

This is a follow-up patch series from the previous discussion here:

 https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for system emulation on 32-bit x86 host and 32-bit arm
hosts anymore, so it should be fine if we drop these host environments
soon (these are also the two architectures that contribute the most to
the long test times in our CI, so we would benefit a lot by dropping
those).

v4:
- Drop the patch that deprecated qemu-system-arm since it still
  might be required to enforce 32-bit guests with TCG
- Only deprecate system emulation on 32-bit x86 hosts since user-mode
  emulation might still be useful in certain scenarios
- Add a sentence how to enforce 32-bit mode with qemu-system-x86_64

v3:
- Update some commit descriptions according to the suggestions in v2
- Added the Reviewed-bys from v2

v2:
- Split binary and host deprecation into separate patches
- Added patches to immediately drop the jobs from the CI

Thomas Huth (5):
  docs/about/deprecated: Deprecate 32-bit x86 hosts for system emulation
  docs/about/deprecated: Deprecate the qemu-system-i386 binary
  gitlab-ci.d/crossbuilds: Drop the i386 system emulation job
  docs/about/deprecated: Deprecate 32-bit arm hosts for system emulation
  gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

 docs/about/deprecated.rst| 34 ++
 .gitlab-ci.d/crossbuilds.yml | 24 
 2 files changed, 34 insertions(+), 24 deletions(-)

-- 
2.31.1

[PATCH v4 4/5] docs/about/deprecated: Deprecate 32-bit arm hosts for system emulation

[Thomas Huth]

For running QEMU in system emulation mode, the user needs a rather
strong host system, i.e. not only an embedded low-frequency controller.
All recent beefy arm host machines should support 64-bit now, it's
unlikely that anybody is still seriously using QEMU on a 32-bit arm
CPU, so we deprecate the 32-bit arm hosts here to finally save use
some time and precious CI minutes.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index c4fcc6b33c..f0de517dc2 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -220,6 +220,15 @@ be an effective use of its limited resources, and thus 
intends to discontinue
 it. Since all recent x86 hardware from the past >10 years is capable of the
 64-bit x86 extensions, a corresponding 64-bit OS should be used instead.
 
+System emulation on 32-bit arm hosts (since 8.0)
+
+
+Since QEMU needs a strong host machine for running full system emulation, and
+all recent powerful arm hosts support 64-bit, the QEMU project deprecates the
+support for running any system emulation on 32-bit arm hosts in general. Use
+64-bit arm hosts for system emulation instead. (Note: "user" mode emulation
+continuous to be supported on 32-bit arm hosts, too)
+
 
 QEMU API (QAPI) events
 --
-- 
2.31.1

[PATCH v4 2/5] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

Aside from not supporting KVM on 32-bit hosts, the qemu-system-x86_64
binary is a proper superset of the qemu-system-i386 binary. With the
32-bit host support being deprecated, it is now also possible to
deprecate the qemu-system-i386 binary.

With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

 https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1ca9dc33d6..c4fcc6b33c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -34,6 +34,20 @@ deprecating the build option and no longer defend it in CI. 
The
 ``--enable-gcov`` build option remains for analysis test case
 coverage.
 
+``qemu-system-i386`` binary (since 8.0)
+'''
+
+The ``qemu-system-i386`` binary was mainly useful for running with KVM
+on 32-bit x86 hosts, but most Linux distributions already removed their
+support for 32-bit x86 kernels, so hardly anybody still needs this. The
+``qemu-system-x86_64`` binary is a proper superset and can be used to
+run 32-bit guests by selecting a 32-bit CPU model, including KVM support
+on x86_64 hosts. Thus users are recommended to reconfigure their systems
+to use the ``qemu-system-x86_64`` binary instead. If a 32-bit CPU guest
+environment should be enforced, you can switch off the "long mode" CPU
+flag, e.g. with ``-cpu max,lm=off``.
+
+
 System emulator command line arguments
 --
 
-- 
2.31.1

[PATCH v4 3/5] gitlab-ci.d/crossbuilds: Drop the i386 system emulation job

[Thomas Huth]

Hardly anybody still uses 32-bit x86 environments for running QEMU with
full system emulation, so let's stop wasting our scarce CI minutes with
this job.

(There are still the 32-bit MinGW and TCI jobs around for having
some compile test coverage on 32-bit)

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 10 --
 1 file changed, 10 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index d3a31a2112..a25cb87ae4 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -43,16 +43,6 @@ cross-arm64-user:
   variables:
 IMAGE: debian-arm64-cross
 
-cross-i386-system:
-  extends:
-- .cross_system_build_job
-- .cross_test_artifacts
-  needs:
-job: i386-fedora-cross-container
-  variables:
-IMAGE: fedora-i386-cross
-MAKE_CHECK_ARGS: check-qtest
-
 cross-i386-user:
   extends:
 - .cross_user_build_job
-- 
2.31.1

[PATCH v4 1/5] docs/about/deprecated: Deprecate 32-bit x86 hosts for system emulation

[Thomas Huth]

Hardly anybody still uses 32-bit x86 hosts today, so we should start
deprecating them to stop wasting our time and CI minutes here.
For example, there are also still some unresolved problems with these:
When emulating 64-bit binaries in user mode, TCG does not honor atomicity
for 64-bit accesses, which is "perhaps worse than not working at all"
(quoting Richard). Let's simply make it clear that people should use
64-bit x86 hosts nowadays and we do not intend to fix/maintain the old
32-bit stuff.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 15084f7bea..1ca9dc33d6 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -196,6 +196,17 @@ CI coverage support may bitrot away before the deprecation 
process
 completes. The little endian variants of MIPS (both 32 and 64 bit) are
 still a supported host architecture.
 
+System emulation on 32-bit x86 hosts (since 8.0)
+
+
+Support for 32-bit x86 host deployments is increasingly uncommon in mainstream
+OS distributions given the widespread availability of 64-bit x86 hardware.
+The QEMU project no longer considers 32-bit x86 support for system emulation to
+be an effective use of its limited resources, and thus intends to discontinue
+it. Since all recent x86 hardware from the past >10 years is capable of the
+64-bit x86 extensions, a corresponding 64-bit OS should be used instead.
+
+
 QEMU API (QAPI) events
 --
 
-- 
2.31.1

Re: [PATCH v2 4/6] docs/about/deprecated: Deprecate the qemu-system-arm binary

[Thomas Huth]

On 03/03/2023 12.16, Peter Maydell wrote:

On Thu, 2 Mar 2023 at 16:31, Thomas Huth  wrote:


qemu-system-aarch64 is a proper superset of qemu-system-arm,
and the latter was mainly still required for 32-bit KVM support.
But this 32-bit KVM arm support has been dropped in the Linux
kernel a couple of years ago already, so we don't really need
qemu-system-arm anymore, thus deprecated it now.

Signed-off-by: Thomas Huth 
---
  docs/about/deprecated.rst | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a30aa8dfdf..21ce70b5c9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -45,6 +45,16 @@ run 32-bit guests by selecting a 32-bit CPU model, including 
KVM support
  on x86_64 hosts. Thus users are recommended to reconfigure their systems
  to use the ``qemu-system-x86_64`` binary instead.

+``qemu-system-arm`` binary (since 8.0)
+''
+
+``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``.


I think this is not quite true -- at the moment if you want
"every feature we implement, 32-bit" the only way to get
that is 'qemu-system-arm -cpu max'. The '-cpu max' on
qemu-system-aarch64 is 64-bit, and we don't implement for TCG
the "-cpu max,aarch64=off" syntax that we do for KVM that would
let the user say "no 64-bit support".


Ok ... so what does that mean now? ... can we continue with this patch, e.g. 
after rephrasing the text a little bit, or do we need to implement "-cpu 
max,aarch64=off" for TCG first?


 Thomas

Re: [PATCH v2 0/6] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

On 03/03/2023 12.09, John Paul Adrian Glaubitz wrote:

Hello!

On Fri, 2023-03-03 at 10:48 +0100, Thomas Huth wrote:

x86 ==> deprecate both, user and system emulation support on
  32-bit hosts
arm ==> deprecate only system emulation on 32-bit hosts.


I would recommend against dropping support for 32-bit hosts for qemu-user
as there are some cases where the emulation of 32-bit user code on 64-bit
hosts does not work properly [1].

Adrian


[1] https://sourceware.org/bugzilla/show_bug.cgi?id=23960


The ticket is very long and hard to read, but ... oh my, does that mean you 
need to compile qemu-user in 32-bit mode on a 64-bit x86 host to properly 
run 32-bit binaries from other architectures? ... uh, that's ugly ... and 
sounds like bug in QEMU's user mode emulation ... and what if you're running 
a distro (or different 64-bit host architecutre) that does not support 
32-bit userspace libraries anymore? Then you're lost?


 Thomas

[PATCH v3 5/6] docs/about/deprecated: Deprecate the qemu-system-arm binary

[Thomas Huth]

qemu-system-aarch64 is a proper superset of qemu-system-arm,
and the latter was mainly still required for 32-bit KVM support.
But this 32-bit KVM arm support has been dropped in the Linux
kernel a couple of years ago already, so we don't really need
qemu-system-arm anymore, thus deprecated it now.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index e2e908f84d..1b7b3da309 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -45,6 +45,16 @@ run 32-bit guests by selecting a 32-bit CPU model, including 
KVM support
 on x86_64 hosts. Thus users are recommended to reconfigure their systems
 to use the ``qemu-system-x86_64`` binary instead.
 
+``qemu-system-arm`` binary (since 8.0)
+''
+
+``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``. The
+latter was mainly a requirement for running KVM on 32-bit arm hosts, but
+this 32-bit KVM support has been removed three years ago already (see:
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=541ad0150ca4
+). Thus the QEMU project will drop the ``qemu-system-arm`` binary in a
+future release. Use ``qemu-system-aarch64`` instead.
+
 
 System emulator command line arguments
 --
-- 
2.31.1

[PATCH v3 4/6] docs/about/deprecated: Deprecate 32-bit arm hosts for system emulation

[Thomas Huth]

For running QEMU in system emulation mode, the user needs a rather
strong host system, i.e. not only an embedded low-frequency controller.
All recent beefy arm host machines should support 64-bit now, it's
unlikely that anybody is still seriously using QEMU on a 32-bit arm
CPU, so we deprecate the 32-bit arm hosts here to finally save use
some time and precious CI minutes.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a30aa8dfdf..e2e908f84d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -219,6 +219,15 @@ discontinue it. Since all recent x86 hardware from the 
past >10 years
 is capable of the 64-bit x86 extensions, a corresponding 64-bit OS
 should be used instead.
 
+System emulation on 32-bit arm hosts (since 8.0)
+
+
+Since QEMU needs a strong host machine for running full system emulation, and
+all recent powerful arm hosts support 64-bit, the QEMU project deprecates the
+support for running any system emulation on 32-bit arm hosts in general. Use
+64-bit arm hosts for system emulation instead. (Note: "user" mode emulation
+continuous to be supported on 32-bit arm hosts, too)
+
 
 QEMU API (QAPI) events
 --
-- 
2.31.1

[PATCH v3 6/6] gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

[Thomas Huth]

Hardly anybody still uses 32-bit arm environments for running QEMU,
so let's stop wasting our scarce CI minutes with these jobs.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 14 --
 1 file changed, 14 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index b96439146f..d1feca9f9f 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -1,13 +1,6 @@
 include:
   - local: '/.gitlab-ci.d/crossbuild-template.yml'
 
-cross-armel-system:
-  extends: .cross_system_build_job
-  needs:
-job: armel-debian-cross-container
-  variables:
-IMAGE: debian-armel-cross
-
 cross-armel-user:
   extends: .cross_user_build_job
   needs:
@@ -15,13 +8,6 @@ cross-armel-user:
   variables:
 IMAGE: debian-armel-cross
 
-cross-armhf-system:
-  extends: .cross_system_build_job
-  needs:
-job: armhf-debian-cross-container
-  variables:
-IMAGE: debian-armhf-cross
-
 cross-armhf-user:
   extends: .cross_user_build_job
   needs:
-- 
2.31.1

[PATCH v3 0/6] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now (32-bit arm for system emulation, and 32-bit x86
completely).

This is a follow-up patch series from the previous discussion here:

 https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit x86 host support and for system emulation on
32-bit arm hosts anymore, so it should be fine if we drop these host
environments soon (these are also the two architectures that contribute
the most to the long test times in our CI, so we would benefit a lot by
dropping those).

v3:
- Update some commit descriptions according to the suggestions in v2
- Added the Reviewed-bys from v2

v2:
- Split binary and host deprecation into separate patches
- Added patches to immediately drop the jobs from the CI

Thomas Huth (6):
  docs/about/deprecated: Deprecate 32-bit x86 hosts
  docs/about/deprecated: Deprecate the qemu-system-i386 binary
  gitlab-ci.d/crossbuilds: Drop the i386 jobs
  docs/about/deprecated: Deprecate 32-bit arm hosts for system emulation
  docs/about/deprecated: Deprecate the qemu-system-arm binary
  gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

 docs/about/deprecated.rst| 43 
 .gitlab-ci.d/crossbuilds.yml | 34 
 2 files changed, 43 insertions(+), 34 deletions(-)

-- 
2.31.1

[PATCH v3 1/6] docs/about/deprecated: Deprecate 32-bit x86 hosts

[Thomas Huth]

Hardly anybody still uses 32-bit x86 hosts today, so we should start
deprecating them to stop wasting our time and CI minutes here.
For example, there are also still some unresolved problems with these:
When emulating 64-bit binaries in user mode, TCG does not honor atomicity
for 64-bit accesses, which is "perhaps worse than not working at all"
(quoting Richard). Let's simply make it clear that people should use
64-bit x86 hosts nowadays and we do not intend to fix/maintain the old
32-bit stuff.

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 15084f7bea..f0c1e6b545 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -196,6 +196,18 @@ CI coverage support may bitrot away before the deprecation 
process
 completes. The little endian variants of MIPS (both 32 and 64 bit) are
 still a supported host architecture.
 
+32-bit x86 hosts (since 8.0)
+
+
+Support for 32-bit x86 host deployments is increasingly uncommon in
+mainstream OS distributions given the widespread availability of 64-bit
+x86 hardware. The QEMU project no longer considers 32-bit x86 support
+to be an effective use of its limited resources, and thus intends to
+discontinue it. Since all recent x86 hardware from the past >10 years
+is capable of the 64-bit x86 extensions, a corresponding 64-bit OS
+should be used instead.
+
+
 QEMU API (QAPI) events
 --
 
-- 
2.31.1

[PATCH v3 2/6] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

Aside from not supporting KVM on 32-bit hosts, the qemu-system-x86_64
binary is a proper superset of the qemu-system-i386 binary. With the
32-bit host support being deprecated, it is now also possible to
deprecate the qemu-system-i386 binary.

With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

 https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index f0c1e6b545..a30aa8dfdf 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -34,6 +34,18 @@ deprecating the build option and no longer defend it in CI. 
The
 ``--enable-gcov`` build option remains for analysis test case
 coverage.
 
+``qemu-system-i386`` binary (since 8.0)
+'''
+
+The ``qemu-system-i386`` binary was mainly useful for running with KVM
+on 32-bit x86 hosts, but most Linux distributions already removed their
+support for 32-bit x86 kernels, so hardly anybody still needs this. The
+``qemu-system-x86_64`` binary is a proper superset and can be used to
+run 32-bit guests by selecting a 32-bit CPU model, including KVM support
+on x86_64 hosts. Thus users are recommended to reconfigure their systems
+to use the ``qemu-system-x86_64`` binary instead.
+
+
 System emulator command line arguments
 --
 
-- 
2.31.1

[PATCH v3 3/6] gitlab-ci.d/crossbuilds: Drop the i386 jobs

[Thomas Huth]

Hardly anybody still uses 32-bit x86 environments for running QEMU,
so let's stop wasting our scarce CI minutes with these jobs.

(There are still the 32-bit MinGW and TCI jobs around for having
some compile test coverage on 32-bit, and the dockerfile can stay
in case someone wants to reproduce a flaw locally)

Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Wilfred Mallawa 
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 20 
 1 file changed, 20 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index d3a31a2112..b96439146f 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -43,26 +43,6 @@ cross-arm64-user:
   variables:
 IMAGE: debian-arm64-cross
 
-cross-i386-system:
-  extends:
-- .cross_system_build_job
-- .cross_test_artifacts
-  needs:
-job: i386-fedora-cross-container
-  variables:
-IMAGE: fedora-i386-cross
-MAKE_CHECK_ARGS: check-qtest
-
-cross-i386-user:
-  extends:
-- .cross_user_build_job
-- .cross_test_artifacts
-  needs:
-job: i386-fedora-cross-container
-  variables:
-IMAGE: fedora-i386-cross
-MAKE_CHECK_ARGS: check
-
 cross-i386-tci:
   extends:
 - .cross_accel_build_job
-- 
2.31.1

Re: [PATCH v2 0/6] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

On 02/03/2023 23.07, Philippe Mathieu-Daudé wrote:

On 2/3/23 17:31, Thomas Huth wrote:

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now.

This is a follow-up patch series from the previous discussion here:

  
https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/


where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit x86 host support and for system emulation on
32-bit arm hosts anymore, so it should be fine if we drop these host
environments soon (these are also the two architectures that contribute
the most to the long test times in our CI, so we would benefit a lot by
dropping those).


It is not clear from your cover that the deprecation only concern system
emulation on these hosts, not user emulation.


x86 ==> deprecate both, user and system emulation support on
32-bit hosts
arm ==> deprecate only system emulation on 32-bit hosts.

I tried to say it with the sentence "there is no real need for 32-bit x86 
host support and for system emulation on 32-bit arm hosts anymore, so it 
should be fine if we drop these host  environments soon" ... not sure why 
it's unclear, but if you have some better sentences, I'm open for suggestions.



I wonder about tools. Apparently they depend on sysemu now. I was
building a 'configure --enable-tools --disable-system' but now it
is empty.


I just did a try in a fresh build directory, and for me it was working: it 
builds qemu-img, qemu-io, qemu-edid etc. just fine.


What was missing in your case?

 Thomas

Re: [PATCH v2 4/6] docs/about/deprecated: Deprecate the qemu-system-arm binary

[Thomas Huth]

On 02/03/2023 23.16, Philippe Mathieu-Daudé wrote:

On 2/3/23 17:31, Thomas Huth wrote:

qemu-system-aarch64 is a proper superset of qemu-system-arm,
and the latter was mainly still required for 32-bit KVM support.
But this 32-bit KVM arm support has been dropped in the Linux
kernel a couple of years ago already, so we don't really need
qemu-system-arm anymore, thus deprecated it now.

Signed-off-by: Thomas Huth 
---
  docs/about/deprecated.rst | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a30aa8dfdf..21ce70b5c9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -45,6 +45,16 @@ run 32-bit guests by selecting a 32-bit CPU model, 
including KVM support

  on x86_64 hosts. Thus users are recommended to reconfigure their systems
  to use the ``qemu-system-x86_64`` binary instead.
+``qemu-system-arm`` binary (since 8.0)
+''
+
+``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``. The
+latter was mainly a requirement for running KVM on 32-bit arm hosts, but
+this 32-bit KVM support has been removed some years ago already (see:


s/some/few/?


I can also use "three years ago" since the patch had been merged in March 2020.

+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=541ad0150ca4 


+). Thus the QEMU project will drop the ``qemu-system-arm`` binary in a
+future release. Use ``qemu-system-aarch64`` instead.


If we unify, wouldn't it be simpler to name the single qemu-system
binary emulating various ARM architectures as 'qemu-system-arm'?


That would be more intuitive for people who are completely new to QEMU, but 
I guess it will cause a lot of "you broke my script that uses the -aarch64 
binary" troubles again. So I think it's likely better to not go down that road.


 Thomas

[PATCH v2 6/6] gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

[Thomas Huth]

Hardly anybody still uses 32-bit arm environments for running QEMU,
so let's stop wasting our scarce CI minutes with these jobs.

Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 14 --
 1 file changed, 14 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 3ce51adf77..419b0c2fe1 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -1,13 +1,6 @@
 include:
   - local: '/.gitlab-ci.d/crossbuild-template.yml'
 
-cross-armel-system:
-  extends: .cross_system_build_job
-  needs:
-job: armel-debian-cross-container
-  variables:
-IMAGE: debian-armel-cross
-
 cross-armel-user:
   extends: .cross_user_build_job
   needs:
@@ -15,13 +8,6 @@ cross-armel-user:
   variables:
 IMAGE: debian-armel-cross
 
-cross-armhf-system:
-  extends: .cross_system_build_job
-  needs:
-job: armhf-debian-cross-container
-  variables:
-IMAGE: debian-armhf-cross
-
 cross-armhf-user:
   extends: .cross_user_build_job
   needs:
-- 
2.31.1

[PATCH v2 5/6] docs/about/deprecated: Deprecate 32-bit arm hosts

[Thomas Huth]

For running QEMU in system emulation mode, the user needs a rather
strong host system, i.e. not only an embedded low-frequency controller.
All recent beefy arm host machines should support 64-bit now, it's
unlikely that anybody is still seriously using QEMU on a 32-bit arm
CPU, so we deprecate the 32-bit arm hosts here to finally save use
some time and precious CI minutes.

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 21ce70b5c9..c7113a7510 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -229,6 +229,15 @@ discontinue it. Since all recent x86 hardware from the 
past >10 years
 is capable of the 64-bit x86 extensions, a corresponding 64-bit OS
 should be used instead.
 
+System emulation on 32-bit arm hosts (since 8.0)
+
+
+Since QEMU needs a strong host machine for running full system emulation, and
+all recent powerful arm hosts support 64-bit, the QEMU project deprecates the
+support for running any system emulation on 32-bit arm hosts in general. Use
+64-bit arm hosts for system emulation instead. (Note: "user" mode emulation
+continuous to be supported on 32-bit arm hosts, too)
+
 
 QEMU API (QAPI) events
 --
-- 
2.31.1

[PATCH v2 4/6] docs/about/deprecated: Deprecate the qemu-system-arm binary

[Thomas Huth]

qemu-system-aarch64 is a proper superset of qemu-system-arm,
and the latter was mainly still required for 32-bit KVM support.
But this 32-bit KVM arm support has been dropped in the Linux
kernel a couple of years ago already, so we don't really need
qemu-system-arm anymore, thus deprecated it now.

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a30aa8dfdf..21ce70b5c9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -45,6 +45,16 @@ run 32-bit guests by selecting a 32-bit CPU model, including 
KVM support
 on x86_64 hosts. Thus users are recommended to reconfigure their systems
 to use the ``qemu-system-x86_64`` binary instead.
 
+``qemu-system-arm`` binary (since 8.0)
+''
+
+``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``. The
+latter was mainly a requirement for running KVM on 32-bit arm hosts, but
+this 32-bit KVM support has been removed some years ago already (see:
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=541ad0150ca4
+). Thus the QEMU project will drop the ``qemu-system-arm`` binary in a
+future release. Use ``qemu-system-aarch64`` instead.
+
 
 System emulator command line arguments
 --
-- 
2.31.1

[PATCH v2 3/6] gitlab-ci.d/crossbuilds: Drop the i386 jobs

[Thomas Huth]

Hardly anybody still uses 32-bit x86 environments for running QEMU,
so let's stop wasting our scarce CI minutes with these jobs.

Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/crossbuilds.yml | 16 
 1 file changed, 16 deletions(-)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 101416080c..3ce51adf77 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -43,22 +43,6 @@ cross-arm64-user:
   variables:
 IMAGE: debian-arm64-cross
 
-cross-i386-system:
-  extends: .cross_system_build_job
-  needs:
-job: i386-fedora-cross-container
-  variables:
-IMAGE: fedora-i386-cross
-MAKE_CHECK_ARGS: check-qtest
-
-cross-i386-user:
-  extends: .cross_user_build_job
-  needs:
-job: i386-fedora-cross-container
-  variables:
-IMAGE: fedora-i386-cross
-MAKE_CHECK_ARGS: check
-
 cross-i386-tci:
   extends: .cross_accel_build_job
   timeout: 60m
-- 
2.31.1

[PATCH v2 2/6] docs/about/deprecated: Deprecate 32-bit x86 hosts

[Thomas Huth]

Hardly anybody still uses 32-bit x86 hosts today, so we should start
deprecating them to stop wasting our time and CI minutes here.
For example, there are also still some unresolved problems with these:
When emulating 64-bit binaries in user mode, TCG does not honor atomicity
for 64-bit accesses, which is "perhaps worse than not working at all"
(quoting Richard). Let's simply make it clear that people should use
64-bit x86 hosts nowadays and we do not intend to fix/maintain the old
32-bit stuff.

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 11700adac9..a30aa8dfdf 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -208,6 +208,18 @@ CI coverage support may bitrot away before the deprecation 
process
 completes. The little endian variants of MIPS (both 32 and 64 bit) are
 still a supported host architecture.
 
+32-bit x86 hosts (since 8.0)
+
+
+Support for 32-bit x86 host deployments is increasingly uncommon in
+mainstream OS distributions given the widespread availability of 64-bit
+x86 hardware. The QEMU project no longer considers 32-bit x86 support
+to be an effective use of its limited resources, and thus intends to
+discontinue it. Since all recent x86 hardware from the past >10 years
+is capable of the 64-bit x86 extensions, a corresponding 64-bit OS
+should be used instead.
+
+
 QEMU API (QAPI) events
 --
 
-- 
2.31.1

[PATCH v2 1/6] docs/about/deprecated: Deprecate the qemu-system-i386 binary

[Thomas Huth]

Hardly anybody really requires the i386 binary anymore, since the
qemu-system-x86_64 binary is a proper superset. So let's deprecate
the 32-bit variant now, so that we can finally stop wasting our time
and CI minutes with this.

With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

 https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 15084f7bea..11700adac9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -34,6 +34,18 @@ deprecating the build option and no longer defend it in CI. 
The
 ``--enable-gcov`` build option remains for analysis test case
 coverage.
 
+``qemu-system-i386`` binary (since 8.0)
+'''
+
+The ``qemu-system-i386`` binary was mainly useful for running with KVM
+on 32-bit x86 hosts, but most Linux distributions already removed their
+support for 32-bit x86 kernels, so hardly anybody still needs this. The
+``qemu-system-x86_64`` binary is a proper superset and can be used to
+run 32-bit guests by selecting a 32-bit CPU model, including KVM support
+on x86_64 hosts. Thus users are recommended to reconfigure their systems
+to use the ``qemu-system-x86_64`` binary instead.
+
+
 System emulator command line arguments
 --
 
-- 
2.31.1

[PATCH v2 0/6] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now.

This is a follow-up patch series from the previous discussion here:

 https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit x86 host support and for system emulation on
32-bit arm hosts anymore, so it should be fine if we drop these host
environments soon (these are also the two architectures that contribute
the most to the long test times in our CI, so we would benefit a lot by
dropping those).

v2:
- Split binary and host deprecation into separate patches
- Added patches to immediately drop the jobs from the CI

Thomas Huth (6):
  docs/about/deprecated: Deprecate the qemu-system-i386 binary
  docs/about/deprecated: Deprecate 32-bit x86 hosts
  gitlab-ci.d/crossbuilds: Drop the i386 jobs
  docs/about/deprecated: Deprecate the qemu-system-arm binary
  docs/about/deprecated: Deprecate 32-bit arm hosts
  gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs

 docs/about/deprecated.rst| 43 
 .gitlab-ci.d/crossbuilds.yml | 30 -
 2 files changed, 43 insertions(+), 30 deletions(-)

-- 
2.31.1

Re: [PATCH 0/2] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

On 28/02/2023 22.32, Michael S. Tsirkin wrote:

On Tue, Feb 28, 2023 at 09:05:16PM +0100, Thomas Huth wrote:

Well, without CI, I assume that the code will bitrot quite fast (considering
that there are continuous improvements to TCG, for example).


We have lots of hosts which we don't test with CI.  They don't bitrot
because people do testing before release.


Other hosts don't bitrot completely since there are still people out there 
who are interested in those host systems. But are you aware of anybody who's 
still actively interested in 32-bit x86 host systems and thus makes sure 
that QEMU would still work fine there when we publish release candidates?


 Thomas

Re: [PATCH 0/2] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

On 28/02/2023 10.01, Daniel P. Berrangé wrote:

On Tue, Feb 28, 2023 at 08:39:49AM +0100, Thomas Huth wrote:

On 27/02/2023 19.38, Daniel P. Berrangé wrote:

On Mon, Feb 27, 2023 at 12:10:48PM +0100, Thomas Huth wrote:

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now.

This is a follow-up patch series from the previous discussion here:

   https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit host support for system emulation on x86 and
arm anymore, so it should be fine if we drop these host environments
now (these are also the two architectures that contribute the most to
the long test times in our CI, so we would benefit a lot by dropping
those).


Your description here is a little ambiguous about what's being
proposed. When you say dropping 32-bit host support do you mean
just for the system emulator binaries, or for QEMU entirely ?


Just for system emulation. Some people said that user emulation still might
be useful for some 32-bit environments.


And when the deprecation period is passed, are you proposing
to actively prevent 32-bit builds, or merely stopping CI testing
and leave 32-bit builds still working if people want them ?


CI is the main pain point, so that's the most important thing. So whether we
throw a warning or a hard error while configuring the build, I don't care
too much.


If we're merely wanting to drop CI support, we can do that any time and
deprecation is not required/expected.  We should only be using deprecation
where we're explicitly intending that the code will cease to work.


Well, without CI, I assume that the code will bitrot quite fast (considering 
that there are continuous improvements to TCG, for example). And who's then 
still volunteering to fix bugs that have crept in months ago, for a host 
architecture that nobody really uses anymore?
Clearly, 32-bit x86 host is pretty much dead nowadays, especially for 
programs like QEMU that need beefy host hardware. Why do we still waste our 
time with this?


 Thomas

Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

On 28/02/2023 11.51, Michael S. Tsirkin wrote:

On Tue, Feb 28, 2023 at 11:39:39AM +0100, Markus Armbruster wrote:

The question to answer: Is 32 bit x86 worth its upkeep?  Two
sub-questions: 1. Is it worth the human attention?  2. Is it worth
(scarce!) CI minutes?


3. Is it worth arguing about?


You are aware of the problems we're currently struggeling with, aren't you? 
Darn, we're having *SEVERE* problems with the CI, the QEMU project ran out 
of CI minutes for the second time this year, and you ask whether it's worth 
arguing about??? You're not serious with this question, are you?


 Thomas

Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

On 28/02/2023 10.03, Michael S. Tsirkin wrote:

On Tue, Feb 28, 2023 at 08:59:52AM +, Daniel P. Berrangé wrote:

On Tue, Feb 28, 2023 at 03:19:20AM -0500, Michael S. Tsirkin wrote:

On Tue, Feb 28, 2023 at 08:49:09AM +0100, Thomas Huth wrote:

On 27/02/2023 21.12, Michael S. Tsirkin wrote:

On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote:

I feel like we should have separate deprecation entries for the
i686 host support, and for qemu-system-i386 emulator binary, as
although they're related they are independant features with
differing impact. eg removing qemu-system-i386 affects all
host architectures, not merely 32-bit x86 host, so I think we
can explain the impact more clearly if we separate them.


Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is
a superset.

Removing support for building on 32 bit systems seems like a pity - it's
one of a small number of ways to run 64 bit binaries on 32 bit systems,
and the maintainance overhead is quite small.


Note: We're talking about 32-bit *x86* hosts here. Do you really think that
someone is still using QEMU usermode emulation
to run 64-bit binaries on a 32-bit x86 host?? ... If so, I'd be very surprised!


I don't know - why x86 specifically? One can build a 32 bit binary on any host.
I think 32 bit x86 environments are just more common in the cloud.


Can you point to anything that backs up that assertion. Clouds I've
seen always give you a 64-bit environment, and many OS no longer
even ship 32-bit installable media.


Sorry about being unclear. I meant that it seems easier to run CI in the
cloud in a 32 bit x64 environment than get a 32 bit ARM environment.


It's still doable ... but for how much longer? We're currently depending on 
Fedora, but they also slowly drop more and more support for this 
environment, see e.g.:


 https://www.theregister.com/2022/03/10/fedora_inches_closer_to_dropping/

 Thomas

Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

On 27/02/2023 21.25, Richard Henderson wrote:

On 2/27/23 01:50, Daniel P. Berrangé wrote:

On Mon, Feb 27, 2023 at 12:10:49PM +0100, Thomas Huth wrote:

Hardly anybody still uses 32-bit x86 hosts today, so we should
start deprecating them to finally have less test efforts.
With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

  https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Signed-off-by: Thomas Huth 
---
  docs/about/deprecated.rst | 13 +
  1 file changed, 13 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 15084f7bea..98517f5187 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -196,6 +196,19 @@ CI coverage support may bitrot away before the 
deprecation process

  completes. The little endian variants of MIPS (both 32 and 64 bit) are
  still a supported host architecture.
+32-bit x86 hosts and ``qemu-system-i386`` (since 8.0)
+'
+
+Testing 32-bit x86 host OS support takes a lot of precious time during the
+QEMU contiguous integration tests, and considering that most OS vendors
+stopped shipping 32-bit variants of their x86 OS distributions and most
+x86 hardware from the past >10 years is capable of 64-bit, keeping the
+32-bit support alive is an inadequate burden for the QEMU project. Thus
+QEMU will soon drop the support for 32-bit x86 host systems and the
+``qemu-system-i386`` binary. Use ``qemu-system-x86_64`` (which is a proper
+superset of ``qemu-system-i386``) on a 64-bit host machine instead.


I feel like we should have separate deprecation entries for the
i686 host support, and for qemu-system-i386 emulator binary, as
although they're related they are independant features with
differing impact.


Agreed.


OK, fair, I'll rework my patch according to your suggestion, Daniel.


32-bit x86 hosts


Support for 32-bit x86 host deployments is increasingly uncommon in
mainstream Linux distributions given the widespread availability of
64-bit x86 hardware. The QEMU project no longer considers 32-bit
x86 support to be an effective use of its limited resources, and
thus intends to discontinue it.

Current users of QEMU on 32-bit x86 hosts should either continue
using existing releases of QEMU, with the caveat that they will
no longer get security fixes, or migrate to a 64-bit platform
which remains capable of running 32-bit guests if needed.

Ack.



``qemu-system-i386`` binary removal
'''

The ``qemu-system-x86_64`` binary can be used to run 32-bit guests
by selecting a 32-bit CPU model, including KVM support on x86_64
hosts. Once support for the 32-bit x86 host platform is discontinued,
the ``qemu-system-i386`` binary will be redundant.


Missing "kvm" in this last sentence?  It is otherwise untrue for tcg.


I assume that Daniel only thought of 32-bit x86 hosts here, but indeed, it's 
untrue for non-x86 32-bit hosts. So this really should refer to KVM on 
32-bit x86 hosts instead. I'll rephrase it in v2.


 Thomas

Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

On 27/02/2023 21.12, Michael S. Tsirkin wrote:

On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote:

I feel like we should have separate deprecation entries for the
i686 host support, and for qemu-system-i386 emulator binary, as
although they're related they are independant features with
differing impact. eg removing qemu-system-i386 affects all
host architectures, not merely 32-bit x86 host, so I think we
can explain the impact more clearly if we separate them.


Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is
a superset.

Removing support for building on 32 bit systems seems like a pity - it's
one of a small number of ways to run 64 bit binaries on 32 bit systems,
and the maintainance overhead is quite small.


Note: We're talking about 32-bit *x86* hosts here. Do you really think that 
someone is still using QEMU usermode emulation

to run 64-bit binaries on a 32-bit x86 host?? ... If so, I'd be very surprised!


In fact, keeping this support around forces correct use of
posix APIs such as e.g. PRIx64 which makes the code base
more future-proof.


If you're concerned about PRIx64 and friends: We still continue to do 
compile testing with 32-bit MIPS cross-compilers and Windows 32-bit 
cross-compilers for now. The only thing we'd lose is the 32-bit "make check" 
run in the CI.


 Thomas

Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

On 27/02/2023 23.32, Philippe Mathieu-Daudé wrote:

On 27/2/23 21:12, Michael S. Tsirkin wrote:

On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote:

I feel like we should have separate deprecation entries for the
i686 host support, and for qemu-system-i386 emulator binary, as
although they're related they are independant features with
differing impact. eg removing qemu-system-i386 affects all
host architectures, not merely 32-bit x86 host, so I think we
can explain the impact more clearly if we separate them.


Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is
a superset.


Doesn't qemu-system-i386 start the CPU in a different mode that
qemu-system-x86_64? Last time we discussed it, we mention adding
-32 and -64 CLI flags to maintain compat, and IIRC this flag would
add boot code to switch the CPU in 32-b. But then maybe I misunderstood.
Thomas said, "CPUs must start in the same mode they start in HW".


No, I think you misunderstood something here. x86 CPUs always start in 
16-bit mode, as far as I know, and the firmware / OS then has to switch to 
32-bit or 64-bit mode as desired.


 Thomas

Re: [PATCH 0/2] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

On 27/02/2023 19.38, Daniel P. Berrangé wrote:

On Mon, Feb 27, 2023 at 12:10:48PM +0100, Thomas Huth wrote:

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now.

This is a follow-up patch series from the previous discussion here:

  https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit host support for system emulation on x86 and
arm anymore, so it should be fine if we drop these host environments
now (these are also the two architectures that contribute the most to
the long test times in our CI, so we would benefit a lot by dropping
those).


Your description here is a little ambiguous about what's being
proposed. When you say dropping 32-bit host support do you mean
just for the system emulator binaries, or for QEMU entirely ?


Just for system emulation. Some people said that user emulation still might 
be useful for some 32-bit environments.



And when the deprecation period is passed, are you proposing
to actively prevent 32-bit builds, or merely stopping CI testing
and leave 32-bit builds still working if people want them ?


CI is the main pain point, so that's the most important thing. So whether we 
throw a warning or a hard error while configuring the build, I don't care 
too much.


 Thomas

[PATCH 0/2] Deprecate support for 32-bit x86 and arm hosts

[Thomas Huth]

We're struggling quite badly with our CI minutes on the shared
gitlab runners, so we urgently need to think of ways to cut down
our supported build and target environments. qemu-system-i386 and
qemu-system-arm are not really required anymore, since nobody uses
KVM on the corresponding systems for production anymore, and the
-x86_64 and -arch64 variants are a proper superset of those binaries.
So it's time to deprecate them and the corresponding 32-bit host
environments now.

This is a follow-up patch series from the previous discussion here:

 https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/

where people still mentioned that there is still interest in certain
support for 32-bit host hardware. But as far as I could see, there is
no real need for 32-bit host support for system emulation on x86 and
arm anymore, so it should be fine if we drop these host environments
now (these are also the two architectures that contribute the most to
the long test times in our CI, so we would benefit a lot by dropping
those).

Thomas Huth (2):
  docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386
  docs/about: Deprecate 32-bit arm hosts and qemu-system-arm

 docs/about/deprecated.rst | 30 ++
 1 file changed, 30 insertions(+)

-- 
2.31.1

[PATCH 2/2] docs/about: Deprecate 32-bit arm hosts and qemu-system-arm

[Thomas Huth]

qemu-system-aarch64 is a proper superset of qemu-system-arm,
and the latter was mainly still required for 32-bit KVM support.
But this 32-bit KVM arm support has been dropped in the Linux
kernel a couple of years ago already,so we don't really need
qemu-system-arm anymore, thus deprecated it now.

Additionally, it's quite unlikely that anybody is still running
full system emulation on a 32-bit arm host nowadays. All recent
strong arm host machines should support 64-bit now, so we also
deprecate the 32-bit hosts here to finally save some precious
minutes in our CI.

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 17 +
 1 file changed, 17 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 98517f5187..a52e45b570 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -208,6 +208,23 @@ QEMU will soon drop the support for 32-bit x86 host 
systems and the
 ``qemu-system-i386`` binary. Use ``qemu-system-x86_64`` (which is a proper
 superset of ``qemu-system-i386``) on a 64-bit host machine instead.
 
+System emulation on 32-bit arm hosts and ``qemu-system-arm`` (since 8.0)
+
+
+``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``. The
+latter was mainly a requirement for running KVM on 32-bit arm hosts, but
+this 32-bit KVM support has been removed some years ago already (see:
+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=541ad0150ca4
+). Thus the QEMU project will drop the ``qemu-system-arm`` binary in a
+future release -- use ``qemu-system-aarch64`` instead.
+
+Since you need a strong host machine for running full system emulation,
+and all recent strong hosts support 64-bit anyway, the QEMU project
+also deprecates the support for running any system emulation on 32-bit
+arm hosts in general. Use 64-bit arm hosts for system emulation instead.
+(Note: "user" mode emulation continuous to be supported on 32-bit arm
+hosts, too)
+
 
 QEMU API (QAPI) events
 --
-- 
2.31.1

[PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386

[Thomas Huth]

Hardly anybody still uses 32-bit x86 hosts today, so we should
start deprecating them to finally have less test efforts.
With regards to 32-bit KVM support in the x86 Linux kernel,
the developers confirmed that they do not need a recent
qemu-system-i386 binary here:

 https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/

Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 15084f7bea..98517f5187 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -196,6 +196,19 @@ CI coverage support may bitrot away before the deprecation 
process
 completes. The little endian variants of MIPS (both 32 and 64 bit) are
 still a supported host architecture.
 
+32-bit x86 hosts and ``qemu-system-i386`` (since 8.0)
+'
+
+Testing 32-bit x86 host OS support takes a lot of precious time during the
+QEMU contiguous integration tests, and considering that most OS vendors
+stopped shipping 32-bit variants of their x86 OS distributions and most
+x86 hardware from the past >10 years is capable of 64-bit, keeping the
+32-bit support alive is an inadequate burden for the QEMU project. Thus
+QEMU will soon drop the support for 32-bit x86 host systems and the
+``qemu-system-i386`` binary. Use ``qemu-system-x86_64`` (which is a proper
+superset of ``qemu-system-i386``) on a 64-bit host machine instead.
+
+
 QEMU API (QAPI) events
 --
 
-- 
2.31.1

Re: [PATCH 5/6] tests: add G_GNUC_PRINTF for various functions

[Thomas Huth]

On 19/12/2022 14.02, Daniel P. Berrangé wrote:

Signed-off-by: Daniel P. Berrangé 
---
  tests/qtest/ahci-test.c   |  3 +++
  tests/qtest/arm-cpu-features.c|  1 +
  tests/qtest/erst-test.c   |  2 +-
  tests/qtest/ide-test.c|  3 ++-
  tests/qtest/ivshmem-test.c|  4 ++--
  tests/qtest/libqmp.c  |  2 +-
  tests/qtest/libqos/libqos-pc.h|  6 --
  tests/qtest/libqos/libqos-spapr.h |  6 --
  tests/qtest/libqos/libqos.h   |  6 --
  tests/qtest/libqos/virtio-9p.c|  1 +
  tests/qtest/migration-helpers.h   |  1 +
  tests/qtest/rtas-test.c   |  2 +-
  tests/qtest/usb-hcd-uhci-test.c   |  4 ++--
  tests/unit/test-qmp-cmds.c| 13 +
  14 files changed, 36 insertions(+), 18 deletions(-)

...

diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 2373cd64cb..6d52b4e5d8 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -138,6 +138,7 @@ void qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
  }
  
  
+G_GNUC_PRINTF(2, 3)

  static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...)
  {
  va_list ap;
@@ -160,6 +161,7 @@ static QObject *do_qmp_dispatch(bool allow_oob, const char 
*template, ...)
  return ret;
  }
  
+G_GNUC_PRINTF(3, 4)

  static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls,
const char *template, ...)
  {
@@ -269,7 +271,7 @@ static void test_dispatch_cmd_io(void)
  
  static void test_dispatch_cmd_deprecated(void)

  {
-const char *cmd = "{ 'execute': 'test-command-features1' }";
+#define cmd "{ 'execute': 'test-command-features1' }"
  QDict *ret;


That looks weird, why is this required?

 Thomas

Re: [PATCH 4/6] util/error: add G_GNUC_PRINTF for various functions

[Thomas Huth]

On 19/12/2022 14.02, Daniel P. Berrangé wrote:

Signed-off-by: Daniel P. Berrangé 
---
  util/error-report.c | 1 +
  util/error.c| 1 +
  2 files changed, 2 insertions(+)

diff --git a/util/error-report.c b/util/error-report.c
index 5edb2e6040..6e44a55732 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -193,6 +193,7 @@ real_time_iso8601(void)
   * a single phrase, with no newline or trailing punctuation.
   * Prepend the current location and append a newline.
   */
+G_GNUC_PRINTF(2, 0)
  static void vreport(report_type type, const char *fmt, va_list ap)
  {
  gchar *timestr;
diff --git a/util/error.c b/util/error.c
index b6c89d1412..1e7af665b8 100644
--- a/util/error.c
+++ b/util/error.c
@@ -45,6 +45,7 @@ static void error_handle_fatal(Error **errp, Error *err)
  }
  }
  
+G_GNUC_PRINTF(6, 0)

  static void error_setv(Error **errp,
 const char *src, int line, const char *func,
 ErrorClass err_class, const char *fmt, va_list ap,


Reviewed-by: Thomas Huth 

[PATCH] qemu-options: Limit the -xen options to x86 and arm

[Thomas Huth]

The Xen hypervisor is only available on x86 and arm - thus let's
limit the related options to these targets.

Signed-off-by: Thomas Huth 
---
 qemu-options.hx | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 1764eebfaf..cc3a39d21a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4184,16 +4184,17 @@ SRST
 ERST
 
 DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
-"-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
+"-xen-domid id   specify xen guest domain id\n",
+QEMU_ARCH_ARM | QEMU_ARCH_I386)
 DEF("xen-attach", 0, QEMU_OPTION_xen_attach,
 "-xen-attach attach to existing xen domain\n"
 "libxl will use this when starting QEMU\n",
-QEMU_ARCH_ALL)
+QEMU_ARCH_ARM | QEMU_ARCH_I386)
 DEF("xen-domid-restrict", 0, QEMU_OPTION_xen_domid_restrict,
 "-xen-domid-restrict restrict set of available xen operations\n"
 "to specified domain id. (Does not affect\n"
 "xenpv machine type).\n",
-QEMU_ARCH_ALL)
+QEMU_ARCH_ARM | QEMU_ARCH_I386)
 SRST
 ``-xen-domid id``
 Specify xen guest domain id (XEN only).
-- 
2.27.0

[PATCH] softmmu/vl: Fence 'xenfb' if Xen support is not compiled in

[Thomas Huth]

The 'xenfb' parameter for the '-vga' command line option is currently
always enabled unconditionally (since the xenfb is not a proper QOM
device that could be tested via its class name). That means it also
shows up if Xen is not enabled at all, e.g. like this:

 $ ./qemu-system-sparc -vga help
 none no graphic card
 xenfbXen paravirtualized framebuffer
 tcx  TCX framebuffer (default)
 cg3  CG3 framebuffer

Let's avoid this situation by fencing the parameter with the
CONFIG_XEN_BACKEND switch.

Signed-off-by: Thomas Huth 
---
 softmmu/vl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 06a0e342fe..e26421b815 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -934,10 +934,12 @@ static const VGAInterfaceInfo 
vga_interfaces[VGA_TYPE_MAX] = {
 .name = "CG3 framebuffer",
 .class_names = { "cgthree" },
 },
+#ifdef CONFIG_XEN_BACKEND
 [VGA_XENFB] = {
 .opt_name = "xenfb",
 .name = "Xen paravirtualized framebuffer",
 },
+#endif
 };
 
 static bool vga_interface_available(VGAInterfaceType t)
-- 
2.27.0

Re: [PATCH 07/27] Replace GCC_FMT_ATTR with G_GNUC_PRINTF

[Thomas Huth]

On 16/03/2022 14.16, Philippe Mathieu-Daudé wrote:

On 16/3/22 10:52, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.

Signed-off-by: Marc-André Lureau 
---
  audio/audio.h   |  4 +--
  block/qcow2.h   |  2 +-
  bsd-user/qemu.h |  2 +-
  hw/display/qxl.h    |  2 +-
  hw/net/rocker/rocker.h  |  2 +-
  hw/xen/xen_pt.h |  2 +-
  include/chardev/char-fe.h   |  2 +-
  include/disas/dis-asm.h |  2 +-
  include/hw/acpi/aml-build.h | 12 +++
  include/hw/core/cpu.h   |  2 +-
  include/hw/hw.h |  2 +-
  include/hw/virtio/virtio.h  |  2 +-
  include/hw/xen/xen-bus-helper.h |  4 +--
  include/hw/xen/xen-bus.h    |  4 +--
  include/hw/xen/xen_common.h |  2 +-
  include/hw/xen/xen_pvdev.h  |  2 +-
  include/monitor/monitor.h   |  4 +--
  include/qapi/error.h    | 20 ++--
  include/qapi/qmp/qjson.h    |  8 ++---
  include/qemu/buffer.h   |  2 +-
  include/qemu/compiler.h | 11 ++-
  include/qemu/error-report.h | 24 +++---
  include/qemu/log-for-trace.h    |  2 +-
  include/qemu/log.h  |  2 +-
  include/qemu/qemu-print.h   |  8 ++---
  include/qemu/readline.h |  2 +-
  qga/guest-agent-core.h  |  2 +-
  qga/vss-win32/requester.h   |  2 +-
  scripts/cocci-macro-file.h  |  2 +-
  tests/qtest/libqos/libqtest.h   | 42 -
  tests/qtest/libqtest-single.h   |  2 +-
  tests/qtest/migration-helpers.h |  6 ++--
  audio/alsaaudio.c   |  4 +--
  audio/dsoundaudio.c |  4 +--
  audio/ossaudio.c    |  4 +--
  audio/paaudio.c |  2 +-
  audio/sdlaudio.c    |  2 +-
  block/blkverify.c   |  2 +-
  block/ssh.c |  4 +--
  fsdev/9p-marshal.c  |  2 +-
  fsdev/virtfs-proxy-helper.c |  2 +-
  hw/9pfs/9p.c    |  2 +-
  hw/acpi/aml-build.c |  4 +--
  hw/mips/fuloong2e.c |  2 +-
  hw/mips/malta.c |  2 +-
  hw/net/rtl8139.c    |  2 +-
  hw/virtio/virtio.c  |  2 +-
  io/channel-websock.c    |  2 +-
  monitor/hmp.c   |  4 +--
  nbd/server.c    | 10 +++---
  qemu-img.c  |  4 +--
  qemu-io.c   |  2 +-
  qobject/json-parser.c   |  2 +-
  softmmu/qtest.c |  4 +--
  tests/qtest/libqtest.c  |  2 +-
  tests/unit/test-qobject-input-visitor.c |  4 +--
  audio/coreaudio.m   |  4 +--
  scripts/checkpatch.pl   |  2 +-
  58 files changed, 130 insertions(+), 137 deletions(-)



diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 3baa5e3790f7..f2bd050e3b9a 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -79,19 +79,12 @@
  #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
 sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
-#if defined(__clang__)
-/* clang doesn't support gnu_printf, so use printf. */
-# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
-#else
-/* Use gnu_printf (qemu uses standard format strings). */
-# define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
-# if defined(_WIN32)
+#if !defined(__clang__) && defined(_WIN32)
  /*
   * Map __printf__ to __gnu_printf__ because we want standard format 
strings even

   * when MinGW or GLib include files use __printf__.
   */
-#  define __printf__ __gnu_printf__
-# endif
+# define __printf__ __gnu_printf__
  #endif


Can we also poison GCC_FMT_ATTR? Maybe split in 2 patches, 1 converting
and another removing unused & poisoning?


I don't think that poisoning is required here since this macro is not used 
in "#ifdef" statements - so the compiler will complain to you if you still 
try to use it after the removal.


 Thomas

Re: [PATCH 2/2] Remove leading underscores from Xen defines

[Thomas Huth]

UBLIC_IO_USBIF_H__

-#define __XEN_PUBLIC_IO_USBIF_H__
+#ifndef XEN_PUBLIC_IO_USBIF_H
+#define XEN_PUBLIC_IO_USBIF_H
  
  #include "ring.h"

  #include "../grant_table.h"



I hope the Xen people can comment on whether the underscores had a purpose 
here or whether it's ok to remove them, thus:


Cc: xen-devel@lists.xenproject.org

From my QEMU-developer's side of view:

Reviewed-by: Thomas Huth 

Re: [PATCH v3 5/5] gitlab-ci: Add Xen cross-build jobs

[Thomas Huth]

On 07/12/2020 14.15, Philippe Mathieu-Daudé wrote:
> Cross-build ARM and X86 targets with only Xen accelerator enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds.yml | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index 51896bbc9fb..bd6473a75a7 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -134,3 +134,17 @@ cross-win64-system:
>extends: .cross_system_build_job
>variables:
>  IMAGE: fedora-win64-cross
> +
> +cross-amd64-xen-only:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-amd64-cross
> +ACCEL: xen
> +ACCEL_CONFIGURE_OPTS: --disable-tcg --disable-kvm
> +
> +cross-arm64-xen-only:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-arm64-cross
> +    ACCEL: xen
> +ACCEL_CONFIGURE_OPTS: --disable-tcg --disable-kvm

Reviewed-by: Thomas Huth 

Re: [PATCH v2 5/5] gitlab-ci: Add Xen cross-build jobs

[Thomas Huth]

On 07/12/2020 12.23, Philippe Mathieu-Daudé wrote:
> Cross-build ARM and X86 targets with only Xen accelerator enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds.yml | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index 7a94a66b4b3..31f10f1e145 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -135,3 +135,18 @@ cross-win64-system:
>extends: .cross_system_build_job
>variables:
>  IMAGE: fedora-win64-cross
> +
> +cross-amd64-xen:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-amd64-cross
> +ACCEL: xen
> +TARGETS: i386-softmmu,x86_64-softmmu
> +ACCEL_CONFIGURE_OPTS: --disable-tcg --disable-kvm
> +
> +cross-arm64-xen:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-arm64-cross
> +ACCEL: xen
> +TARGETS: aarch64-softmmu
Could you please simply replace aarch64-softmmu by arm-softmmu in the
target-list-exclude statement in this file instead of adding a new job for
arm64? That should have the same results and will spare us one job...

 Thanks,
  Thomas

Re: [PATCH v2 4/5] gitlab-ci: Add KVM s390x cross-build jobs

[Thomas Huth]

On 07/12/2020 12.23, Philippe Mathieu-Daudé wrote:
> Cross-build s390x target with only KVM accelerator enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds.yml | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index d8685ade376..7a94a66b4b3 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -1,4 +1,3 @@
> -
>  .cross_system_build_job:
>stage: build
>image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> @@ -120,6 +119,13 @@ cross-s390x-user:
>variables:
>  IMAGE: debian-s390x-cross
>  
> +cross-s390x-kvm:

I'd still prefer "-no-tcg" or maybe "-kvm-only" ... but that's just a matter
of taste, so:

Reviewed-by: Thomas Huth 

Re: [PATCH v2 3/5] gitlab-ci: Introduce 'cross_accel_build_job' template

[Thomas Huth]

On 07/12/2020 12.23, Philippe Mathieu-Daudé wrote:
> Introduce a job template to cross-build accelerator specific
> jobs (enable a specific accelerator, disabling the others).
> 
> The specific accelerator is selected by the $ACCEL environment
> variable (default to KVM).
> 
> Extra options such disabling other accelerators are passed
> via the $ACCEL_CONFIGURE_OPTS environment variable.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds.yml | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index 099949aaef3..d8685ade376 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -13,6 +13,23 @@
>xtensa-softmmu"
>  - make -j$(expr $(nproc) + 1) all check-build
>  
> +# Job to cross-build specific accelerators.
> +#
> +# Set the $ACCEL variable to select the specific accelerator (default to
> +# KVM), and set extra options (such disabling other accelerators) via the
> +# $ACCEL_CONFIGURE_OPTS variable.
> +.cross_accel_build_job:
> +  stage: build
> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> +  timeout: 30m
> +  script:
> +- mkdir build
> +- cd build
> +- PKG_CONFIG_PATH=$PKG_CONFIG_PATH
> +  ../configure --enable-werror $QEMU_CONFIGURE_OPTS --disable-tools
> +--enable-${ACCEL:-kvm} --target-list="$TARGETS" $ACCEL_CONFIGURE_OPTS
> +- make -j$(expr $(nproc) + 1) all check-build
> +
>  .cross_user_build_job:
>stage: build
>image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest

I wonder whether we could also simply use the .cross_user_build_job - e.g.
by adding a $EXTRA_CONFIGURE_OPTS variable in the "../configure ..." line so
that the accel-jobs could use that for their --enable... and --disable...
settings?

Anyway, I've got no strong opinion on that one, and I'm also fine if we add
this new template, so:

Reviewed-by: Thomas Huth 

Re: [PATCH v2 1/5] gitlab-ci: Document 'build-tcg-disabled' is a KVM X86 job

[Thomas Huth]

On 07/12/2020 12.23, Philippe Mathieu-Daudé wrote:
> Document what this job cover (build X86 targets with
> KVM being the single accelerator available).
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.yml | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index d0173e82b16..ee31b1020fe 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -220,6 +220,11 @@ build-disabled:
>s390x-softmmu i386-linux-user
>  MAKE_CHECK_ARGS: check-qtest SPEED=slow
>  
> +# This jobs explicitly disable TCG (--disable-tcg), KVM is detected by
> +# the configure script. The container doesn't contain Xen headers so
> +# Xen accelerator is not detected / selected. As result it build the
> +# i386-softmmu and x86_64-softmmu with KVM being the single accelerator
> +# available.
>  build-tcg-disabled:
>    <<: *native_build_job_definition
>variables:
> 

Reviewed-by: Thomas Huth 

Re: [PATCH 5/8] gitlab-ci: Add KVM s390x cross-build jobs

[Thomas Huth]

On 07/12/2020 11.26, Philippe Mathieu-Daudé wrote:
> On 12/7/20 11:00 AM, Philippe Mathieu-Daudé wrote:
>> On 12/7/20 6:46 AM, Thomas Huth wrote:
>>> On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
>>>> Cross-build s390x target with only KVM accelerator enabled.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé 
>>>> ---
>>>>  .gitlab-ci.d/crossbuilds-kvm-s390x.yml | 6 ++
>>>>  .gitlab-ci.yml | 1 +
>>>>  MAINTAINERS| 1 +
>>>>  3 files changed, 8 insertions(+)
>>>>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-s390x.yml
>>>>
>>>> diff --git a/.gitlab-ci.d/crossbuilds-kvm-s390x.yml 
>>>> b/.gitlab-ci.d/crossbuilds-kvm-s390x.yml
>>>> new file mode 100644
>>>> index 000..1731af62056
>>>> --- /dev/null
>>>> +++ b/.gitlab-ci.d/crossbuilds-kvm-s390x.yml
>>>> @@ -0,0 +1,6 @@
>>>> +cross-s390x-kvm:
>>>> +  extends: .cross_accel_build_job
>>>> +  variables:
>>>> +IMAGE: debian-s390x-cross
>>>> +TARGETS: s390x-softmmu
>>>> +ACCEL_CONFIGURE_OPTS: --disable-tcg
>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>>>> index 573afceb3c7..a69619d7319 100644
>>>> --- a/.gitlab-ci.yml
>>>> +++ b/.gitlab-ci.yml
>>>> @@ -14,6 +14,7 @@ include:
>>>>- local: '/.gitlab-ci.d/crossbuilds.yml'
>>>>- local: '/.gitlab-ci.d/crossbuilds-kvm-x86.yml'
>>>>- local: '/.gitlab-ci.d/crossbuilds-kvm-arm.yml'
>>>> +  - local: '/.gitlab-ci.d/crossbuilds-kvm-s390x.yml'
>>>
>>> KVM code is already covered by the "cross-s390x-system" job, but an
>>> additional compilation test with --disable-tcg makes sense here. I'd then
>>> rather name it "cross-s390x-no-tcg" or so instead of "cross-s390x-kvm".
> 
> What other accelerators are available on 390?

It's only TCG and KVM.

 Thomas

Re: [PATCH 7/8] gitlab-ci: Add KVM MIPS cross-build jobs

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> Cross-build mips target with KVM and TCG accelerators enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> later we'll build KVM-only.
> ---
>  .gitlab-ci.d/crossbuilds-kvm-mips.yml | 5 +
>  .gitlab-ci.yml| 1 +
>  MAINTAINERS   | 1 +
>  3 files changed, 7 insertions(+)
>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-mips.yml
> 
> diff --git a/.gitlab-ci.d/crossbuilds-kvm-mips.yml 
> b/.gitlab-ci.d/crossbuilds-kvm-mips.yml
> new file mode 100644
> index 000..81eeeb315bb
> --- /dev/null
> +++ b/.gitlab-ci.d/crossbuilds-kvm-mips.yml
> @@ -0,0 +1,5 @@
> +cross-mips64el-kvm:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-mips64el-cross
> +TARGETS: mips64el-softmmu

That's already covered, see:

https://gitlab.com/qemu-project/qemu/-/jobs/883985068#L296

 Thomas

Re: [PATCH 6/8] gitlab-ci: Add KVM PPC cross-build jobs

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> Cross-build PPC target with KVM and TCG accelerators enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> later this job build KVM-only.
> ---
>  .gitlab-ci.d/crossbuilds-kvm-ppc.yml | 5 +
>  .gitlab-ci.yml   | 1 +
>  MAINTAINERS  | 1 +
>  3 files changed, 7 insertions(+)
>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-ppc.yml
> 
> diff --git a/.gitlab-ci.d/crossbuilds-kvm-ppc.yml 
> b/.gitlab-ci.d/crossbuilds-kvm-ppc.yml
> new file mode 100644
> index 000..9df8bcf5a73
> --- /dev/null
> +++ b/.gitlab-ci.d/crossbuilds-kvm-ppc.yml
> @@ -0,0 +1,5 @@
> +cross-ppc64el-kvm:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-ppc64el-cross
> +TARGETS: ppc64-softmmu

Compilation of the ppc KVM code should already be covered by the
cross-ppc64el-system job, see e.g.:

https://gitlab.com/qemu-project/qemu/-/jobs/883985074#L297

Thus there is no need to add a new job for this here. It might be a good
idea to remove ppc64-softmmu from the exclude list in crossbuilds.yml,
though, so that we also check the 64-bit builds and not only the 32-bit ones.

 Thomas

Re: [PATCH 5/8] gitlab-ci: Add KVM s390x cross-build jobs

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> Cross-build s390x target with only KVM accelerator enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds-kvm-s390x.yml | 6 ++
>  .gitlab-ci.yml | 1 +
>  MAINTAINERS| 1 +
>  3 files changed, 8 insertions(+)
>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-s390x.yml
> 
> diff --git a/.gitlab-ci.d/crossbuilds-kvm-s390x.yml 
> b/.gitlab-ci.d/crossbuilds-kvm-s390x.yml
> new file mode 100644
> index 000..1731af62056
> --- /dev/null
> +++ b/.gitlab-ci.d/crossbuilds-kvm-s390x.yml
> @@ -0,0 +1,6 @@
> +cross-s390x-kvm:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-s390x-cross
> +TARGETS: s390x-softmmu
> +ACCEL_CONFIGURE_OPTS: --disable-tcg
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 573afceb3c7..a69619d7319 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -14,6 +14,7 @@ include:
>- local: '/.gitlab-ci.d/crossbuilds.yml'
>- local: '/.gitlab-ci.d/crossbuilds-kvm-x86.yml'
>- local: '/.gitlab-ci.d/crossbuilds-kvm-arm.yml'
> +  - local: '/.gitlab-ci.d/crossbuilds-kvm-s390x.yml'

KVM code is already covered by the "cross-s390x-system" job, but an
additional compilation test with --disable-tcg makes sense here. I'd then
rather name it "cross-s390x-no-tcg" or so instead of "cross-s390x-kvm".

And while you're at it, I'd maybe rather name the new file just
crossbuilds-s390x.yml and also move the other s390x related jobs into it?

 Thomas

Re: [PATCH 4/8] gitlab-ci: Add KVM ARM cross-build jobs

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> Cross-build ARM aarch64 target with KVM and TCG accelerators enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> later this job will build KVM-only.
> ---
>  .gitlab-ci.d/crossbuilds-kvm-arm.yml | 5 +
>  .gitlab-ci.yml   | 1 +
>  MAINTAINERS  | 1 +
>  3 files changed, 7 insertions(+)
>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-arm.yml
> 
> diff --git a/.gitlab-ci.d/crossbuilds-kvm-arm.yml 
> b/.gitlab-ci.d/crossbuilds-kvm-arm.yml
> new file mode 100644
> index 000..c74c6fdc9fb
> --- /dev/null
> +++ b/.gitlab-ci.d/crossbuilds-kvm-arm.yml
> @@ -0,0 +1,5 @@
> +cross-arm64-kvm:
> +  extends: .cross_accel_build_job
> +  variables:
> +IMAGE: debian-arm64-cross
> +TARGETS: aarch64-softmmu

Now that's a little bit surprising, I had expected that the KVM code is
already compiled by the "cross-arm64-system" job ... but looking at the
output of a corresponding pipeline, it says "KVM support: NO", see e.g.:

https://gitlab.com/qemu-project/qemu/-/jobs/883985039#L298

What's going wrong there? ... ah, well, it's because of the
"--target-list-exclude=aarch64-softmmu" in the template :-(
That was stupid. So instead of adding a new job, could you please simply
replace the aarch64-softmmu there by arm-softmmu?

 Thanks,
  Thomas

Re: [PATCH 3/8] gitlab-ci: Add KVM X86 cross-build jobs

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> Cross-build x86 target with only KVM accelerator enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds-kvm-x86.yml | 6 ++
>  .gitlab-ci.yml   | 1 +
>  MAINTAINERS  | 1 +
>  3 files changed, 8 insertions(+)
>  create mode 100644 .gitlab-ci.d/crossbuilds-kvm-x86.yml

We already have a job that tests with KVM enabled and TCG disabled in the
main .gitlab-ci.yml file, the "build-tcg-disabled" job. So I don't quite see
the point in adding yet another job that does pretty much the same? Did I
miss something?

 Thomas

Re: [PATCH 1/8] gitlab-ci: Replace YAML anchors by extends (cross_system_build_job)

[Thomas Huth]

On 06/12/2020 19.55, Philippe Mathieu-Daudé wrote:
> 'extends' is an alternative to using YAML anchors
> and is a little more flexible and readable. See:
> https://docs.gitlab.com/ee/ci/yaml/#extends
> 
> More importantly it allows exploding YAML jobs.
> 
> Reviewed-by: Wainer dos Santos Moschetta 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/crossbuilds.yml | 40 ++--
>  1 file changed, 20 insertions(+), 20 deletions(-)

Reviewed-by: Thomas Huth 

Re: [PATCH-for-5.2 2/3] gitlab-ci: Add a job to cover the --without-default-devices config

[Thomas Huth]

On 04/11/2020 03.27, Stefano Stabellini wrote:
[...]
> Actually I care about Xen and 9pfs support, it is one of the few
> combinations that I use regularly and it is even enabled in the Xilinx
> product I look after. But admittedly I don't test QEMU master as much as
> I should. With the recent changes to the build system it is not very
> suprising that there are some issues. It would be great to have a Xen
> and 9pfs test in the gitlab CI-loop.
> 
> 
> FYI I tried to build the latest QEMU on Alpine Linux 3.12 ARM64 and I
> get:
> 
>   ninja: unknown tool 'query'
> 
> Even after rebuilding ninja master by hand. Any ideas? I don't know much
> about ninja.
> 
> 
> So I gave up on that and I spinned up a Debian Buster x86 container for
> this build. That one got past the "ninja: unknown tool 'query'" error.
> The build completed without problems to the end.
> 
> Either way I can't reproduce the build error above.

Did you run "configure" with "--without-default-devices" ?

 Thomas

Re: --enable-xen on gitlab CI? (was Re: [PATCH 09/36] qdev: Make qdev_get_prop_ptr() get Object* arg)

[Thomas Huth]

On 30/10/2020 18.13, Paolo Bonzini wrote:
> On 30/10/20 12:35, Eduardo Habkost wrote:
>>
>> What is necessary to make sure we have a CONFIG_XEN=y job in
>> gitlab CI?  Maybe just including xen-devel in some of the
>> container images is enough?
> 
> Fedora already has it, but build-system-fedora does not include
> x86_64-softmmu.

Eduardo, could you try to add xen-devel to the centos8 container? If that
does not work, we can still move the x86_64-softmmu target to the fedora
pipeline instead.

 Thomas

Re: [PATCH v2 0/3] Add Xen CpusAccel

[Thomas Huth]

On 22/10/2020 17.29, Paolo Bonzini wrote:
> On 22/10/20 17:17, Jason Andryuk wrote:
>> On Tue, Oct 13, 2020 at 1:16 PM Paolo Bonzini  wrote:
>>>
>>> On 13/10/20 16:05, Jason Andryuk wrote:
 Xen was left behind when CpusAccel became mandatory and fails the assert
 in qemu_init_vcpu().  It relied on the same dummy cpu threads as qtest.
 Move the qtest cpu functions to a common location and reuse them for
 Xen.

 v2:
   New patch "accel: Remove _WIN32 ifdef from qtest-cpus.c"
   Use accel/dummy-cpus.c for filename
   Put prototype in include/sysemu/cpus.h

 Jason Andryuk (3):
   accel: Remove _WIN32 ifdef from qtest-cpus.c
   accel: move qtest CpusAccel functions to a common location
   accel: Add xen CpusAccel using dummy-cpus

  accel/{qtest/qtest-cpus.c => dummy-cpus.c} | 27 --
  accel/meson.build  |  8 +++
  accel/qtest/meson.build|  1 -
  accel/qtest/qtest-cpus.h   | 17 --
  accel/qtest/qtest.c|  5 +++-
  accel/xen/xen-all.c|  8 +++
  include/sysemu/cpus.h  |  3 +++
  7 files changed, 27 insertions(+), 42 deletions(-)
  rename accel/{qtest/qtest-cpus.c => dummy-cpus.c} (71%)
  delete mode 100644 accel/qtest/qtest-cpus.h

>>>
>>> Acked-by: Paolo Bonzini 
>>
>> Thank you, Paolo.  Also Anthony Acked and Claudio Reviewed patch 3.
>> How can we get this into the tree?
> 
> I think Anthony should send a pull request?

Since Anthony acked patch 3, I think I can also take it through the qtest tree.

 Thomas

Re: [PATCH 3/5] qom: Remove module_obj_name parameter from OBJECT_DECLARE* macros

[Thomas Huth]

On 16/09/2020 20.25, Eduardo Habkost wrote:
> One of the goals of having less boilerplate on QOM declarations
> is to avoid human error.  Requiring an extra argument that is
> never used is an opportunity for mistakes.
> 
> Remove the unused argument from OBJECT_DECLARE_TYPE and
> OBJECT_DECLARE_SIMPLE_TYPE.
> 
> Coccinelle patch used to convert all users of the macros:
> 
>   @@
>   declarer name OBJECT_DECLARE_TYPE;
>   identifier InstanceType, ClassType, lowercase, UPPERCASE;
>   @@
>OBJECT_DECLARE_TYPE(InstanceType, ClassType,
>   -lowercase,
>UPPERCASE);
> 
>   @@
>   declarer name OBJECT_DECLARE_SIMPLE_TYPE;
>   identifier InstanceType, lowercase, UPPERCASE;
>   @@
>OBJECT_DECLARE_SIMPLE_TYPE(InstanceType,
>   -lowercase,
>    UPPERCASE);
> 
> Signed-off-by: Eduardo Habkost 
> ---

Acked-by: Thomas Huth 

Re: [RFC PATCH 22/35] hw/m68k/mcf520x: Emit warning when old code is used

[Thomas Huth]

Am Mon,  8 Jun 2020 18:00:31 +0200
schrieb Philippe Mathieu-Daudé :

> This code hasn't been QOM'ified yet. Warn the user.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/m68k/mcf5206.c | 5 +
>  hw/m68k/mcf5208.c | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
> index a2fef04f8e..ec0d176674 100644
> --- a/hw/m68k/mcf5206.c
> +++ b/hw/m68k/mcf5206.c
> @@ -16,6 +16,7 @@
>  #include "qemu/timer.h"
>  #include "hw/ptimer.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-deprecated.h"
>  
>  /* General purpose timer module.  */
>  typedef struct {
> @@ -144,6 +145,8 @@ static m5206_timer_state
> *m5206_timer_init(qemu_irq irq) {
>  m5206_timer_state *s;
>  
> +qdev_warn_deprecated_function_used();
> +
>  s = g_new0(m5206_timer_state, 1);
>  s->timer = ptimer_init(m5206_timer_trigger, s,
> PTIMER_POLICY_DEFAULT); s->irq = irq;
> @@ -566,6 +569,8 @@ qemu_irq *mcf5206_init(MemoryRegion *sysmem,
> uint32_t base, M68kCPU *cpu) m5206_mbar_state *s;
>  qemu_irq *pic;
>  
> +qdev_warn_deprecated_function_used();
> +
>  s = g_new0(m5206_mbar_state, 1);

Ok, it's quite obvious what you refer to here...

>  memory_region_init_io(>iomem, NULL, _mbar_ops, s,
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 2ab9701ad6..72627f6833 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -26,6 +26,7 @@
>  #include "hw/sysbus.h"
>  #include "elf.h"
>  #include "exec/address-spaces.h"
> +#include "hw/qdev-deprecated.h"
>  
>  #define SYS_FREQ 1
>  
> @@ -191,6 +192,8 @@ static void mcf5208_sys_init(MemoryRegion
> *address_space, qemu_irq *pic) m5208_timer_state *s;
>  int i;
>  
> +qdev_warn_deprecated_function_used();
> +
>  /* SDRAMC.  */
>  memory_region_init_io(iomem, NULL, _sys_ops, NULL,
> "m5208-sys", 0x4000); memory_region_add_subregion(address_space,
> 0xfc0a8000, iomem);

... but it is not so obvious what you refer to here. I think that new
function should maybe have a "char *what" parameter that contains the
name of the struct you refer to. Or at least add a comment in front of
the function with a short description?

 Thomas

[Xen-devel] [Qemu-devel] [PATCH] trivial: Remove xenfb_enabled from sysemu.h

[Thomas Huth]

The define is only used in one other place. Move the code there
instead of keeping this xen-specific define in sysemu.h.

Signed-off-by: Thomas Huth 
---
 hw/xenpv/xen_machine_pv.c | 2 +-
 include/sysemu/sysemu.h   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 3a8af1a1e0..8df575a457 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -60,7 +60,7 @@ static void xen_init_pv(MachineState *machine)
 xen_be_register("qnic", _netdev_ops);
 
 /* configure framebuffer */
-if (xenfb_enabled) {
+if (vga_interface_type == VGA_XENFB) {
 xen_config_dev_vfb(0, "vnc");
 xen_config_dev_vkbd(0);
 }
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 80c57fdc4e..2ccf216158 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -32,7 +32,6 @@ typedef enum {
 } VGAInterfaceType;
 
 extern int vga_interface_type;
-#define xenfb_enabled (vga_interface_type == VGA_XENFB)
 
 extern int graphic_width;
 extern int graphic_height;
-- 
2.18.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH-for-5.0 v3 4/6] hw/pci-host/i440fx: Use definitions instead of magic values

[Thomas Huth]

On 09/12/2019 10.50, Philippe Mathieu-Daudé wrote:
> Use definitions from "hw/pci/pci_regs.h".
> This also helps when using git-grep.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/i440fx.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 0cc80b276d..414138595b 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -376,13 +376,13 @@ typedef struct {
>  
>  /* Here we just expose minimal host bridge offset subset. */
>  static const IGDHostInfo igd_host_bridge_infos[] = {
> -{0x08, 2},  /* revision id */
> -{0x2c, 2},  /* sybsystem vendor id */
> -{0x2e, 2},  /* sybsystem id */
> -{0x50, 2},  /* SNB: processor graphics control register */
> -{0x52, 2},  /* processor graphics control register */
> -{0xa4, 4},  /* SNB: graphics base of stolen memory */
> -{0xa8, 4},  /* SNB: base of GTT stolen memory */
> +{PCI_REVISION_ID, 2},
> +{PCI_SUBSYSTEM_VENDOR_ID, 2},
> +{PCI_SUBSYSTEM_ID,2},
> +{0x50,2}, /* SNB: processor graphics control 
> register */
> +{0x52,2}, /* processor graphics control register */
> +{0xa4,4}, /* SNB: graphics base of stolen memory */
> +{0xa8,4}, /* SNB: base of GTT stolen memory */
>  };
>  
>  static void host_pci_config_read(int pos, int len, uint32_t *val, Error 
> **errp)
> 

Reviewed-by: Thomas Huth 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH-for-5.0 v3 3/6] hw/pci-host/i440fx: Use size_t to iterate over ARRAY_SIZE()

[Thomas Huth]

On 09/12/2019 10.49, Philippe Mathieu-Daudé wrote:
> We don't enforce the -Wsign-conversion CPPFLAG, but it doesn't hurt
> to avoid this warning:
> 
>   warning: implicit conversion changes signedness: 'int' to 'size_t' (aka 
> 'unsigned long') [-Wsign-conversion]
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/i440fx.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index fbdc563599..0cc80b276d 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -419,12 +419,11 @@ out:
>  static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
>  {
>  uint32_t val = 0;
> -int i, num;
> +size_t i;
>  int pos, len;
>  Error *local_err = NULL;
>  
> -num = ARRAY_SIZE(igd_host_bridge_infos);
> -for (i = 0; i < num; i++) {
> +for (i = 0; i < ARRAY_SIZE(igd_host_bridge_infos); i++) {
>  pos = igd_host_bridge_infos[i].offset;
>  len = igd_host_bridge_infos[i].len;
>  host_pci_config_read(pos, len, , _err);
> 

Reviewed-by: Thomas Huth 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH-for-5.0 v3 2/6] hw/pci-host/i440fx: Extract PCII440FXState to "hw/pci-host/i440fx.h"

[Thomas Huth]

On 09/12/2019 10.49, Philippe Mathieu-Daudé wrote:
> Make the PCII440FXState structure public, so it can be used out of
> this source file. This will allow us to extract the IGD Passthrough
> Host Bridge, which is a children of the TYPE_I440FX_PCI_DEVICE.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/pci-host/i440fx.h | 19 +--
>  hw/pci-host/i440fx.c | 18 --
>  2 files changed, 17 insertions(+), 20 deletions(-)

Reviewed-by: Thomas Huth 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH-for-5.0 v3 1/6] hw/pci-host/i440fx: Correct the header description

[Thomas Huth]

On 09/12/2019 10.49, Philippe Mathieu-Daudé wrote:
> Missed during the refactor in commits 14a026dd58 and 0f25d865a,
> this file is now only about the i440FX chipset.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/i440fx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index f27131102d..3fc94426ea 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -1,5 +1,5 @@
>  /*
> - * QEMU i440FX/PIIX3 PCI Bridge Emulation
> + * QEMU i440FX PCI Bridge Emulation
>   *
>   * Copyright (c) 2006 Fabrice Bellard
>   *
> 

Reviewed-by: Thomas Huth 


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 21/32] hw/i386/pc: Reduce gsi_handler scope

[Thomas Huth]

On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
> pc_gsi_create() is the single function that uses gsi_handler.
> Make it a static variable.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/i386/pc.c | 2 +-
>  include/hw/i386/pc.h | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a7597c6c44..59de0c8a1f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -346,7 +346,7 @@ GlobalProperty pc_compat_1_4[] = {
>  };
>  const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
>  
> -void gsi_handler(void *opaque, int n, int level)
> +static void gsi_handler(void *opaque, int n, int level)
>  {
>  GSIState *s = opaque;
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index d0c6b9d469..75b44e156c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -172,8 +172,6 @@ typedef struct GSIState {
>  qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
>  } GSIState;
>  
> -void gsi_handler(void *opaque, int n, int level);
> -
>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
>  
>  /* vmport.c */
> 

Reviewed-by: Thomas Huth 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 02/32] hw/i386/pc: Move kvm_i8259_init() declaration to sysemu/kvm.h

[Thomas Huth]

On 17/10/2019 17.31, Philippe Mathieu-Daudé wrote:
> On 10/17/19 5:04 PM, Thomas Huth wrote:
>> On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
>>> Move the KVM-related call to "sysemu/kvm.h".
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>>   include/hw/i386/pc.h | 1 -
>>>   include/sysemu/kvm.h | 1 +
>>>   2 files changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>> index 6df4f4b6fb..09e74e7764 100644
>>> --- a/include/hw/i386/pc.h
>>> +++ b/include/hw/i386/pc.h
>>> @@ -158,7 +158,6 @@ typedef struct PCMachineClass {
>>>     extern DeviceState *isa_pic;
>>>   qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
>>> -qemu_irq *kvm_i8259_init(ISABus *bus);
>>>   int pic_read_irq(DeviceState *d);
>>>   int pic_get_output(DeviceState *d);
>>>   diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>>> index 9d143282bc..da8aa9f5a8 100644
>>> --- a/include/sysemu/kvm.h
>>> +++ b/include/sysemu/kvm.h
>>> @@ -513,6 +513,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s,
>>> qemu_irq irq, int gsi);
>>>   void kvm_pc_gsi_handler(void *opaque, int n, int level);
>>>   void kvm_pc_setup_irq_routing(bool pci_enabled);
>>>   void kvm_init_irq_routing(KVMState *s);
>>> +qemu_irq *kvm_i8259_init(ISABus *bus);
>>
>> Why? The function is defined in hw/i386/kvm/ - so moving its prototype
>> to a generic header sounds wrong to me.
> 
> This function is declared when compiling without KVM, and is available
> on the Alpha/HPPA/MIPS which don't have it.

Sorry, I failed to parse your last sentence. It's only used by hw/i386
code as far as I can see.

> You'd rather move the kvm_pc_* declarations to hw/i386/kvm/?

Maybe, but that's certainly something for a different patch series.

This series here should focus on what you've mentioned in the cover
letter, I think. It's already big enough.

 Thomas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 20/32] hw/i386/pc: Extract pc_gsi_create()

[Thomas Huth]

On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
> The GSI creation code is common to all PC machines, extract the
> common code.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/i386/pc.c | 15 +++
>  hw/i386/pc_piix.c|  9 +
>  hw/i386/pc_q35.c |  9 +
>  include/hw/i386/pc.h |  2 ++
>  4 files changed, 19 insertions(+), 16 deletions(-)

Is this really needed for this series here, or should this and the
following patches maybe rather be handled seperately?

Anyway, it looks like a good modification, so:
Reviewed-by: Thomas Huth 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 14/32] piix4: add a i8257 dma controller as specified in datasheet

[Thomas Huth]

On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
> 
> Remove i8257 instanciated in malta board, to not have it twice.

s/instanciated/instantiated/

 Thomas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 11/32] Revert "irq: introduce qemu_irq_proxy()"

[Thomas Huth]

On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé 
> 
> This function isn't used anymore.
> 
> This reverts commit 22ec3283efba9ba0792790da786d6776d83f2a92.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/core/irq.c| 14 --
>  include/hw/irq.h |  5 -
>  2 files changed, 19 deletions(-)
>
Reviewed-by: Thomas Huth 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 08/32] piix4: rename some variables in realize function

[Thomas Huth]

On 15/10/2019 18.26, Philippe Mathieu-Daudé wrote:
> From: Hervé Poussineau 
> 
> PIIX4 structure is now 's'
> PCI device is now 'pci_dev'
> DeviceState is now 'dev'

Why? Just for the sake of it?

> Acked-by: Michael S. Tsirkin 
> Acked-by: Paolo Bonzini 
> Signed-off-by: Hervé Poussineau 
> Message-Id: <20171216090228.28505-6-hpous...@reactos.org>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/isa/piix4.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 3294056cd5..4202243e41 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -88,16 +88,17 @@ static const VMStateDescription vmstate_piix4 = {
>  }
>  };
>  
> -static void piix4_realize(PCIDevice *dev, Error **errp)
> +static void piix4_realize(PCIDevice *pci_dev, Error **errp)
>  {
> -PIIX4State *d = PIIX4_PCI_DEVICE(dev);
> +DeviceState *dev = DEVICE(pci_dev);
> +PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci_dev);

AFAIK we rather want to get rid of DO_UPCAST in the long run, so please
don't introduce new ones!

See:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg05244.html

Unless there is a real need for the rename, I'd suggest to rather drop
this patch.

 Thomas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel