Re: [PATCH-for-8.2? v2 4/4] hw/arm/stm32f100: Report error when incorrect CPU is used

2023-11-19 Thread Gavin Shan

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:

The 'stm32vldiscovery' machine ignores the CPU type requested by
the command line. This might confuse users, since the following
will create a machine with a Cortex-M3 CPU:

   $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

   $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1
   qemu-system-aarch64: Invalid CPU type: neoverse-n1-arm-cpu
   The valid types are: cortex-m3-arm-cpu

Since the SoC family can only use Cortex-M3 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/stm32f100_soc.h | 4 
  hw/arm/stm32f100_soc.c | 9 ++---
  hw/arm/stm32vldiscovery.c  | 7 ++-
  3 files changed, 8 insertions(+), 12 deletions(-)



Reviewed-by: Gavin Shan 




Re: [PATCH-for-8.2? v2 3/4] hw/arm/stm32f205: Report error when incorrect CPU is used

2023-11-19 Thread Gavin Shan

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:

The 'netduino2' machine ignores the CPU type requested by the
command line. This might confuse users, since the following will
create a machine with a Cortex-M3 CPU:

   $ qemu-system-arm -M netduino2 -cpu cortex-a9

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

   $ qemu-system-arm -M netduino2 -cpu cortex-a9
   qemu-system-arm: Invalid CPU type: cortex-a9-arm-cpu
   The valid types are: cortex-m3-arm-cpu

Since the SoC family can only use Cortex-M3 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/stm32f205_soc.h | 4 
  hw/arm/netduino2.c | 7 ++-
  hw/arm/stm32f205_soc.c | 9 ++---
  3 files changed, 8 insertions(+), 12 deletions(-)



Reviewed-by: Gavin Shan 




Re: [PATCH-for-8.2? v2 2/4] hw/arm/stm32f405: Report error when incorrect CPU is used

2023-11-19 Thread Gavin Shan

On 11/17/23 17:17, Philippe Mathieu-Daudé wrote:

Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
CPU type requested by the command line. This might confuse users,
since the following will create a machine with a Cortex-M4 CPU:

   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
   qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
   The valid types are: cortex-m4-arm-cpu

Since the SoC family can only use Cortex-M4 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/stm32f405_soc.h | 4 
  hw/arm/netduinoplus2.c | 7 ++-
  hw/arm/olimex-stm32-h405.c | 8 ++--
  hw/arm/stm32f405_soc.c | 8 +---
  4 files changed, 13 insertions(+), 14 deletions(-)



Reviewed-by: Gavin Shan 




[PATCH 1/1] tcg/loongarch64: Fix tcg_out_mov() Aborted

2023-11-19 Thread Song Gao
On LoongArch host,  we got an Aborted from tcg_out_mov().

qemu-x86_64 configure with '--enable-debug'.

> (gdb) b /home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc:312
> Breakpoint 1 at 0x2576f0: file 
> /home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc, line 312.
> (gdb) run hello
[...]
> Thread 1 "qemu-x86_64" hit Breakpoint 1, tcg_out_mov (s=0xe91760 
> , type=TCG_TYPE_V128, ret=TCG_REG_V2,
> arg=TCG_REG_V0) at 
> /home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc:312
> 312   g_assert_not_reached();
> (gdb) bt
> #0  tcg_out_mov (s=0xe91760 , type=TCG_TYPE_V128, 
> ret=TCG_REG_V2, arg=TCG_REG_V0)
> at /home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc:312
> #1  0x00d0fee0 in tcg_reg_alloc_mov (s=0xe91760 , 
> op=0xf67c20) at ../tcg/tcg.c:4632
> #2  0x00d142f4 in tcg_gen_code (s=0xe91760 , 
> tb=0xffe8030340 ,
> pc_start=4346094) at ../tcg/tcg.c:6135
[...]
> (gdb) c
> Continuing.
> **
> ERROR:/home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc:312:tcg_out_mov:
>  code should not be reached
> Bail out! 
> ERROR:/home1/gaosong/code/qemu/tcg/loongarch64/tcg-target.c.inc:312:tcg_out_mov:
>  code should not be reached
>
> Thread 1 "qemu-x86_64" received signal SIGABRT, Aborted.
> 0x00fff7b1c390 in raise () from /lib64/libc.so.6
> (gdb) q

Signed-off-by: Song Gao 
---
 tcg/loongarch64/tcg-target.c.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index a588fb3085..5f68040230 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -308,6 +308,9 @@ static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg 
ret, TCGReg arg)
  */
 tcg_out_opc_or(s, ret, arg, TCG_REG_ZERO);
 break;
+case TCG_TYPE_V128:
+tcg_out_opc_vaddi_du(s, ret, arg, 0);
+break;
 default:
 g_assert_not_reached();
 }
-- 
2.25.1




Re: [PATCH] audio: Free consumed default audio devices

2023-11-19 Thread Marc-André Lureau
Hi

On Sun, Nov 19, 2023 at 12:40 PM Akihiko Odaki  wrote:
>
> Failed default audio devices were removed from the list but not freed,
> and that made LeakSanitizer sad. Free default audio devices as they are
> consumed.
>
> Signed-off-by: Akihiko Odaki 
> ---
>  audio/audio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index f91e05b72c..becf6cdf46 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1758,12 +1758,14 @@ static AudioState *audio_init(Audiodev *dev, Error 
> **errp)
>  goto out;
>  }
>  s->dev = dev = e->dev;
> +QSIMPLEQ_REMOVE_HEAD(_audiodevs, next);
> +g_free(e);
>  drvname = AudiodevDriver_str(dev->driver);
>  driver = audio_driver_lookup(drvname);
>  if (!audio_driver_init(s, driver, dev, NULL)) {
>  break;
>  }
> -QSIMPLEQ_REMOVE_HEAD(_audiodevs, next);
> +qapi_free_Audiodev(dev);

I'd set s->dev to NULL, to avoid potential dangling pointers.

(it looks like like this could already hit a double-free on error path)

>  }
>  }
>
> --
> 2.42.1
>




Re: [PATCH v3 4/4] hw/virtio: derive vhost-user-input from vhost-user-base

2023-11-19 Thread Manos Pitsidianakis

On Mon, 20 Nov 2023 06:37, Leo Yan  wrote:

This patch derives vhost-user-input from vhost-user-base class, so make
the input stub as a simpler boilerplate wrapper.

With the refactoring, vhost-user-input adds the property 'chardev', this
leads to conflict with the vhost-user-input-pci adds the same property.
To resolve the error, remove the duplicate property from
vhost-user-input-pci.

Signed-off-by: Leo Yan 
---


Reviewed-by: Manos Pitsidianakis 



Re: [PATCH v3 3/4] hw/virtio: Move vhost-user-input into virtio folder

2023-11-19 Thread Manos Pitsidianakis

On Mon, 20 Nov 2023 06:37, Leo Yan  wrote:

vhost-user-input is in the input folder.  On the other hand, the folder
'hw/virtio' maintains other virtio stubs (e.g. I2C, RNG, GPIO, etc).

This patch moves vhost-user-input into the virtio folder for better code
organization.  No functionality change.

Signed-off-by: Leo Yan 


Reviewed-by: Manos Pitsidianakis 



[PATCH v3 1/4] hw/virtio: Support set_config() callback in vhost-user-base

2023-11-19 Thread Leo Yan
The Virtio input device invokes set_config() callback for retrieving
the event configuration info, but the callback is not supported in
vhost-user-base.

This patch adds support set_config() callback in vhost-user-base.

Signed-off-by: Leo Yan 
Reviewed-by: Marc-André Lureau 
---
 hw/virtio/vhost-user-base.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 78cfa9a5bb..a83167191e 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -140,6 +140,22 @@ static void vub_get_config(VirtIODevice *vdev, uint8_t 
*config)
 }
 }
 
+static void vub_set_config(VirtIODevice *vdev, const uint8_t *config_data)
+{
+VHostUserBase *vub = VHOST_USER_BASE(vdev);
+int ret;
+
+g_assert(vub->config_size && vub->vhost_user.supports_config == true);
+
+ret = vhost_dev_set_config(>vhost_dev, config_data,
+   0, vub->config_size,
+   VHOST_SET_CONFIG_TYPE_FRONTEND);
+if (ret) {
+error_report("vhost guest set device config space failed: %d", ret);
+return;
+}
+}
+
 /*
  * When the daemon signals an update to the config we just need to
  * signal the guest as we re-read the config on demand above.
@@ -337,6 +353,7 @@ static void vub_class_init(ObjectClass *klass, void *data)
 vdc->unrealize = vub_device_unrealize;
 vdc->get_features = vub_get_features;
 vdc->get_config = vub_get_config;
+vdc->set_config = vub_set_config;
 vdc->set_status = vub_set_status;
 }
 
-- 
2.39.2




[PATCH v3 2/4] docs/system: Add vhost-user-input documentation

2023-11-19 Thread Leo Yan
This adds basic documentation for vhost-user-input.

Signed-off-by: Leo Yan 
---
 MAINTAINERS  |  1 +
 docs/system/device-emulation.rst |  1 +
 docs/system/devices/vhost-user-input.rst | 45 
 docs/system/devices/vhost-user.rst   |  4 ++-
 4 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 docs/system/devices/vhost-user-input.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index 0624d67932..8a26fe9493 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2262,6 +2262,7 @@ L: virtio...@lists.linux.dev
 virtio-input
 M: Gerd Hoffmann 
 S: Odd Fixes
+F: docs/system/devices/vhost-user-input.rst
 F: hw/input/vhost-user-input.c
 F: hw/input/virtio-input*.c
 F: include/hw/virtio/virtio-input.h
diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
index d1f3277cb0..f19777411c 100644
--- a/docs/system/device-emulation.rst
+++ b/docs/system/device-emulation.rst
@@ -94,6 +94,7 @@ Emulated Devices
devices/virtio-gpu.rst
devices/virtio-pmem.rst
devices/virtio-snd.rst
+   devices/vhost-user-input.rst
devices/vhost-user-rng.rst
devices/canokey.rst
devices/usb-u2f.rst
diff --git a/docs/system/devices/vhost-user-input.rst 
b/docs/system/devices/vhost-user-input.rst
new file mode 100644
index 00..118eb78101
--- /dev/null
+++ b/docs/system/devices/vhost-user-input.rst
@@ -0,0 +1,45 @@
+.. _vhost_user_input:
+
+QEMU vhost-user-input - Input emulation
+===
+
+This document describes the setup and usage of the Virtio input device.
+The Virtio input device is a paravirtualized device for input events.
+
+Description
+---
+
+The vhost-user-input device implementation was designed to work with a daemon
+polling on input devices and passes input events to the guest.
+
+QEMU provides a backend implementation in contrib/vhost-user-input.
+
+Linux kernel support
+
+
+Virtio input requires a guest Linux kernel built with the
+``CONFIG_VIRTIO_INPUT`` option.
+
+Examples
+
+
+The backend daemon should be started first:
+
+::
+
+  host# vhost-user-input --socket-path=input.sock  \
+  --evdev-path=/dev/input/event17
+
+The QEMU invocation needs to create a chardev socket to communicate with the
+backend daemon and access the VirtIO queues with the guest over the
+:ref:`shared memory `.
+
+::
+
+  host# qemu-system
\
+  -chardev socket,path=/tmp/input.sock,id=mouse0   
\
+  -device vhost-user-input-pci,chardev=mouse0  
\
+  -m 4096  
\
+  -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on
\
+  -numa node,memdev=mem
\
+  ...
diff --git a/docs/system/devices/vhost-user.rst 
b/docs/system/devices/vhost-user.rst
index c6afc4836f..9b2da106ce 100644
--- a/docs/system/devices/vhost-user.rst
+++ b/docs/system/devices/vhost-user.rst
@@ -42,7 +42,7 @@ platform details for what sort of virtio bus to use.
 - See https://github.com/rust-vmm/vhost-device
   * - vhost-user-input
 - Generic input driver
-- See contrib/vhost-user-input
+- :ref:`vhost_user_input`
   * - vhost-user-rng
 - Entropy driver
 - :ref:`vhost_user_rng`
@@ -91,6 +91,8 @@ following the :ref:`vhost_user_proto`. There are a number of 
daemons
 that can be built when enabled by the project although any daemon that
 meets the specification for a given device can be used.
 
+.. _shared_memory_object:
+
 Shared memory object
 
 
-- 
2.39.2




[PATCH v3 0/4] virtio: Refactor vhost input stub

2023-11-19 Thread Leo Yan
This series is to refactor vhost stub vhost-user-input.

Since vhost input stub requires set_config() callback for communication
event configurations between the backend and the guest, patch 01 is a
preparison for support set_config() callback in vhost-user-base.

The patch 02 is to add documentation for vhost-user-input.

The patch 03 is to move virtio input stub from the input folder to the
virtio folder.

The patch 04 derives vhost-user-input from vhost-user-base.  We reuse
the common code from vhhost-user-base as possible and the input stub is
simplized significantly.

This patch set has been tested with the backend daemon:

  # ./build/contrib/vhost-user-input/vhost-user-input \
 -p /dev/input/event20 -s /tmp/input.sock

The series is based on "[PATCH v8 0/7] virtio: cleanup
vhost-user-generic and reduce c" which introduces vhost-user-base.
Based-on: <20231107180752.3458672-1-alex.ben...@linaro.org>

Changes from v2:
- Created reference for shared memory object and updated
  vhost-user-input.rst respectively. (Marc-André)

Changes from v1:
- Fixed typo in vhost-user-input.rst.
- Updated MAINTAINERS for new added input document and
  changing folder for vhost-user-input.c. (Manos)


Leo Yan (4):
  hw/virtio: Support set_config() callback in vhost-user-base
  docs/system: Add vhost-user-input documentation
  hw/virtio: Move vhost-user-input into virtio folder
  hw/virtio: derive vhost-user-input from vhost-user-base

 MAINTAINERS  |   3 +-
 docs/system/device-emulation.rst |   1 +
 docs/system/devices/vhost-user-input.rst |  45 
 docs/system/devices/vhost-user.rst   |   4 +-
 hw/input/meson.build |   1 -
 hw/input/vhost-user-input.c  | 136 ---
 hw/virtio/meson.build|   4 +-
 hw/virtio/vhost-user-base.c  |  17 +++
 hw/virtio/vhost-user-input-pci.c |   3 -
 hw/virtio/vhost-user-input.c |  58 ++
 include/hw/virtio/virtio-input.h |   6 +-
 11 files changed, 132 insertions(+), 146 deletions(-)
 create mode 100644 docs/system/devices/vhost-user-input.rst
 delete mode 100644 hw/input/vhost-user-input.c
 create mode 100644 hw/virtio/vhost-user-input.c

-- 
2.39.2




[PATCH v3 3/4] hw/virtio: Move vhost-user-input into virtio folder

2023-11-19 Thread Leo Yan
vhost-user-input is in the input folder.  On the other hand, the folder
'hw/virtio' maintains other virtio stubs (e.g. I2C, RNG, GPIO, etc).

This patch moves vhost-user-input into the virtio folder for better code
organization.  No functionality change.

Signed-off-by: Leo Yan 
---
 MAINTAINERS | 2 +-
 hw/input/meson.build| 1 -
 hw/virtio/meson.build   | 4 +++-
 hw/{input => virtio}/vhost-user-input.c | 0
 4 files changed, 4 insertions(+), 3 deletions(-)
 rename hw/{input => virtio}/vhost-user-input.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8a26fe9493..fdc3edc6cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2263,8 +2263,8 @@ virtio-input
 M: Gerd Hoffmann 
 S: Odd Fixes
 F: docs/system/devices/vhost-user-input.rst
-F: hw/input/vhost-user-input.c
 F: hw/input/virtio-input*.c
+F: hw/virtio/vhost-user-input.c
 F: include/hw/virtio/virtio-input.h
 F: contrib/vhost-user-input/*
 
diff --git a/hw/input/meson.build b/hw/input/meson.build
index 640556bbbc..3cc8ab85f0 100644
--- a/hw/input/meson.build
+++ b/hw/input/meson.build
@@ -11,7 +11,6 @@ system_ss.add(when: 'CONFIG_TSC2005', if_true: 
files('tsc2005.c'))
 system_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: files('virtio-input.c'))
 system_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: 
files('virtio-input-hid.c'))
 system_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: 
files('virtio-input-host.c'))
-system_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: 
files('vhost-user-input.c'))
 
 system_ss.add(when: 'CONFIG_PXA2XX', if_true: files('pxa2xx_keypad.c'))
 system_ss.add(when: 'CONFIG_TSC210X', if_true: files('tsc210x.c'))
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 118d4d4da7..c924afcafc 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -25,6 +25,7 @@ if have_vhost
 system_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: 
files('vhost-user-i2c.c'))
 system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: 
files('vhost-user-rng.c'))
 system_virtio_ss.add(when: 'CONFIG_VHOST_USER_SND', if_true: 
files('vhost-user-snd.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: 
files('vhost-user-input.c'))
 
 # PCI Stubs
 system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: 
files('vhost-user-device-pci.c'))
@@ -36,6 +37,8 @@ if have_vhost
  if_true: files('vhost-user-rng-pci.c'))
 system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SND'],
  if_true: files('vhost-user-snd-pci.c'))
+system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 
'CONFIG_VHOST_USER_INPUT'],
+ if_true: files('vhost-user-input-pci.c'))
   endif
   if have_vhost_vdpa
 system_virtio_ss.add(files('vhost-vdpa.c'))
@@ -59,7 +62,6 @@ virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: 
files('vhost-vsock-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: 
files('vhost-user-vsock-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: 
files('vhost-user-blk-pci.c'))
-virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: 
files('vhost-user-input-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: 
files('vhost-user-scsi-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: 
files('vhost-scsi-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: 
files('vhost-user-fs-pci.c'))
diff --git a/hw/input/vhost-user-input.c b/hw/virtio/vhost-user-input.c
similarity index 100%
rename from hw/input/vhost-user-input.c
rename to hw/virtio/vhost-user-input.c
-- 
2.39.2




[PATCH v3 4/4] hw/virtio: derive vhost-user-input from vhost-user-base

2023-11-19 Thread Leo Yan
This patch derives vhost-user-input from vhost-user-base class, so make
the input stub as a simpler boilerplate wrapper.

With the refactoring, vhost-user-input adds the property 'chardev', this
leads to conflict with the vhost-user-input-pci adds the same property.
To resolve the error, remove the duplicate property from
vhost-user-input-pci.

Signed-off-by: Leo Yan 
---
 hw/virtio/vhost-user-input-pci.c |   3 -
 hw/virtio/vhost-user-input.c | 114 +--
 include/hw/virtio/virtio-input.h |   6 +-
 3 files changed, 21 insertions(+), 102 deletions(-)

diff --git a/hw/virtio/vhost-user-input-pci.c b/hw/virtio/vhost-user-input-pci.c
index b858898a36..3f4761ce88 100644
--- a/hw/virtio/vhost-user-input-pci.c
+++ b/hw/virtio/vhost-user-input-pci.c
@@ -30,9 +30,6 @@ static void vhost_user_input_pci_instance_init(Object *obj)
 
 virtio_instance_init_common(obj, >vhi, sizeof(dev->vhi),
 TYPE_VHOST_USER_INPUT);
-
-object_property_add_alias(obj, "chardev",
-  OBJECT(>vhi), "chardev");
 }
 
 static const VirtioPCIDeviceTypeInfo vhost_user_input_pci_info = {
diff --git a/hw/virtio/vhost-user-input.c b/hw/virtio/vhost-user-input.c
index 4ee3542106..bedec0468c 100644
--- a/hw/virtio/vhost-user-input.c
+++ b/hw/virtio/vhost-user-input.c
@@ -5,83 +5,25 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/error-report.h"
-#include "qapi/error.h"
-
 #include "hw/virtio/virtio-input.h"
 
-static int vhost_input_config_change(struct vhost_dev *dev)
-{
-error_report("vhost-user-input: unhandled backend config change");
-return -1;
-}
-
-static const VhostDevConfigOps config_ops = {
-.vhost_dev_config_notifier = vhost_input_config_change,
+static Property vinput_properties[] = {
+DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+DEFINE_PROP_END_OF_LIST(),
 };
 
-static void vhost_input_realize(DeviceState *dev, Error **errp)
-{
-VHostUserInput *vhi = VHOST_USER_INPUT(dev);
-VirtIOInput *vinput = VIRTIO_INPUT(dev);
-VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-
-vhost_dev_set_config_notifier(>vhost->dev, _ops);
-vinput->cfg_size = sizeof_field(virtio_input_config, u);
-if (vhost_user_backend_dev_init(vhi->vhost, vdev, 2, errp) == -1) {
-return;
-}
-}
-
-static void vhost_input_change_active(VirtIOInput *vinput)
-{
-VHostUserInput *vhi = VHOST_USER_INPUT(vinput);
-
-if (vinput->active) {
-vhost_user_backend_start(vhi->vhost);
-} else {
-vhost_user_backend_stop(vhi->vhost);
-}
-}
-
-static void vhost_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
-{
-VirtIOInput *vinput = VIRTIO_INPUT(vdev);
-VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
-Error *local_err = NULL;
-int ret;
-
-memset(config_data, 0, vinput->cfg_size);
-
-ret = vhost_dev_get_config(>vhost->dev, config_data, vinput->cfg_size,
-   _err);
-if (ret) {
-error_report_err(local_err);
-return;
-}
-}
-
-static void vhost_input_set_config(VirtIODevice *vdev,
-   const uint8_t *config_data)
+static void vinput_realize(DeviceState *dev, Error **errp)
 {
-VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
-int ret;
+VHostUserBase *vub = VHOST_USER_BASE(dev);
+VHostUserBaseClass *vubc = VHOST_USER_BASE_GET_CLASS(dev);
 
-ret = vhost_dev_set_config(>vhost->dev, config_data,
-   0, sizeof(virtio_input_config),
-   VHOST_SET_CONFIG_TYPE_FRONTEND);
-if (ret) {
-error_report("vhost-user-input: set device config space failed");
-return;
-}
+/* Fixed for input device */
+vub->virtio_id = VIRTIO_ID_INPUT;
+vub->num_vqs = 2;
+vub->vq_size = 4;
+vub->config_size = sizeof(virtio_input_config);
 
-virtio_notify_config(vdev);
-}
-
-static struct vhost_dev *vhost_input_get_vhost(VirtIODevice *vdev)
-{
-VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
-return >vhost->dev;
+vubc->parent_realize(dev, errp);
 }
 
 static const VMStateDescription vmstate_vhost_input = {
@@ -91,40 +33,20 @@ static const VMStateDescription vmstate_vhost_input = {
 
 static void vhost_input_class_init(ObjectClass *klass, void *data)
 {
-VirtIOInputClass *vic = VIRTIO_INPUT_CLASS(klass);
-VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->vmsd = _vhost_input;
-vdc->get_config = vhost_input_get_config;
-vdc->set_config = vhost_input_set_config;
-vdc->get_vhost = vhost_input_get_vhost;
-vic->realize = vhost_input_realize;
-vic->change_active = vhost_input_change_active;
-}
-
-static void vhost_input_init(Object *obj)
-{
-VHostUserInput *vhi = VHOST_USER_INPUT(obj);
-
-vhi->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND));
-

RE: [PATCH v6 09/21] vfio/iommufd: Enable pci hot reset through iommufd cdev interface

2023-11-19 Thread Duan, Zhenzhong


>-Original Message-
>From: Eric Auger 
>Sent: Friday, November 17, 2023 9:54 PM
>Subject: Re: [PATCH v6 09/21] vfio/iommufd: Enable pci hot reset through
>iommufd cdev interface
>
>
>
>On 11/14/23 11:09, Zhenzhong Duan wrote:
>> Add a new callback iommufd_cdev_pci_hot_reset to do iommufd specific
>> check and reset operation.
>
>nit: Implement the newly introduced pci_hot_reset callback?

Yes

>>
>> Signed-off-by: Zhenzhong Duan 
>> ---
>> v6: pci_hot_reset return -errno if fails
>>
>>  hw/vfio/iommufd.c| 145
>+++
>>  hw/vfio/trace-events |   1 +
>>  2 files changed, 146 insertions(+)
>>
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index e5bf528e89..3eec428162 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -24,6 +24,7 @@
>>  #include "sysemu/reset.h"
>>  #include "qemu/cutils.h"
>>  #include "qemu/chardev_open.h"
>> +#include "pci.h"
>>
>>  static int iommufd_cdev_map(VFIOContainerBase *bcontainer, hwaddr iova,
>>  ram_addr_t size, void *vaddr, bool readonly)
>> @@ -473,9 +474,153 @@ static void iommufd_cdev_detach(VFIODevice
>*vbasedev)
>>  close(vbasedev->fd);
>>  }
>>
>> +static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
>> +{
>> +VFIODevice *vbasedev_iter;
>> +
>> +QLIST_FOREACH(vbasedev_iter, _device_list, global_next) {
>> +if (vbasedev_iter->bcontainer->ops != _iommufd_ops) {
>> +continue;
>> +}
>> +if (devid == vbasedev_iter->devid) {
>> +return vbasedev_iter;
>> +}
>> +}
>> +return NULL;
>> +}
>> +
>> +static int iommufd_cdev_pci_hot_reset(VFIODevice *vbasedev, bool single)
>> +{
>> +VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
>> +struct vfio_pci_hot_reset_info *info = NULL;
>> +struct vfio_pci_dependent_device *devices;
>> +struct vfio_pci_hot_reset *reset;
>> +int ret, i;
>> +bool multi = false;
>> +
>> +trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
>> +
>> +if (!single) {
>> +vfio_pci_pre_reset(vdev);
>> +}
>> +vdev->vbasedev.needs_reset = false;
>> +
>> +ret = vfio_pci_get_pci_hot_reset_info(vdev, );
>> +
>> +if (ret) {
>> +goto out_single;
>> +}
>> +
>> +assert(info->flags & VFIO_PCI_HOT_RESET_FLAG_DEV_ID);
>> +
>> +devices = >devices[0];
>> +
>> +if (!(info->flags & VFIO_PCI_HOT_RESET_FLAG_DEV_ID_OWNED)) {
>> +if (!vdev->has_pm_reset) {
>> +for (i = 0; i < info->count; i++) {
>> +if (devices[i].devid == VFIO_PCI_DEVID_NOT_OWNED) {
>> +error_report("vfio: Cannot reset device %s, "
>> + "depends on device %04x:%02x:%02x.%x "
>> + "which is not owned.",
>> + vdev->vbasedev.name, devices[i].segment,
>> + devices[i].bus, PCI_SLOT(devices[i].devfn),
>> + PCI_FUNC(devices[i].devfn));
>> +}
>> +}
>> +}
>> +ret = -EPERM;
>> +goto out_single;
>> +}
>> +
>> +trace_vfio_pci_hot_reset_has_dep_devices(vdev->vbasedev.name);
>> +
>> +for (i = 0; i < info->count; i++) {
>> +VFIOPCIDevice *tmp;
>> +VFIODevice *vbasedev_iter;
>> +
>> +trace_iommufd_cdev_pci_hot_reset_dep_devices(devices[i].segment,
>> + devices[i].bus,
>> + 
>> PCI_SLOT(devices[i].devfn),
>> + 
>> PCI_FUNC(devices[i].devfn),
>> + devices[i].devid);
>> +
>> +/*
>> + * If a VFIO cdev device is resettable, all the dependent devices
>> + * are either bound to same iommufd or within same iommu_groups as
>> + * one of the iommufd bound devices.
>> + */
>> +assert(devices[i].devid != VFIO_PCI_DEVID_NOT_OWNED);
>> +
>> +if (devices[i].devid == vdev->vbasedev.devid ||
>> +devices[i].devid == VFIO_PCI_DEVID_OWNED) {
>> +continue;
>> +}
>> +
>> +vbasedev_iter = iommufd_cdev_pci_find_by_devid(devices[i].devid);
>> +if (!vbasedev_iter || !vbasedev_iter->dev->realized ||
>> +vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
>> +continue;
>> +}
>> +tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
>> +if (single) {
>> +ret = -EINVAL;
>> +goto out_single;
>> +}
>> +vfio_pci_pre_reset(tmp);
>> +tmp->vbasedev.needs_reset = false;
>> +multi = true;
>> +}
>> +
>> +if (!single && !multi) {
>> +ret = -EINVAL;
>> +goto out_single;
>> +}
>> +
>> +/* Use zero length array for hot reset with 

RE: [PATCH] docs/devel: Add VFIO iommufd backend documentation

2023-11-19 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, November 17, 2023 10:28 PM
>To: Duan, Zhenzhong ; qemu-devel@nongnu.org
>Cc: alex.william...@redhat.com; j...@nvidia.com; nicol...@nvidia.com;
>joao.m.mart...@oracle.com; eric.au...@redhat.com; pet...@redhat.com;
>jasow...@redhat.com; Tian, Kevin ; Liu, Yi L
>; Sun, Yi Y ; Peng, Chao P
>
>Subject: Re: [PATCH] docs/devel: Add VFIO iommufd backend documentation
>
>On 11/17/23 13:58, Cédric Le Goater wrote:
>> On 11/17/23 10:35, Zhenzhong Duan wrote:
>>> Suggested-by: Cédric Le Goater 
>>> Signed-off-by: Zhenzhong Duan 
>>
>> The content looks good but it lacks formatting. Please try to generate
>> the docs.
>
>Please check my vfio-8.2 branch.

Got it, thanks for help.

BRs.
Zhenzhong


[PATCH v3] migration: free 'saddr' since be no longer used

2023-11-19 Thread Zongmin Zhou
Since socket_parse() will allocate memory for 'saddr',and its value
will pass to 'addr' that allocated by migrate_uri_parse(),
then 'saddr' will no longer used,need to free.
But due to 'saddr->u' is shallow copying the contents of the union,
the members of this union containing allocated strings,and will be used after 
that.
So just free 'saddr' itself without doing a deep free on the contents of the 
SocketAddress.

Fixes: 72a8192e225c ("migration: convert migration 'uri' into 'MigrateAddress'")
Signed-off-by: Zongmin Zhou
---
 migration/migration.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 28a34c9068..1832dad618 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -462,7 +462,6 @@ bool migrate_uri_parse(const char *uri, MigrationChannel 
**channel,
 {
 g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
 g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
-SocketAddress *saddr = NULL;
 InetSocketAddress *isock = >u.rdma;
 strList **tail = >u.exec.args;
 
@@ -487,12 +486,14 @@ bool migrate_uri_parse(const char *uri, MigrationChannel 
**channel,
 strstart(uri, "vsock:", NULL) ||
 strstart(uri, "fd:", NULL)) {
 addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
-saddr = socket_parse(uri, errp);
+SocketAddress *saddr = socket_parse(uri, errp);
 if (!saddr) {
 return false;
 }
 addr->u.socket.type = saddr->type;
 addr->u.socket.u = saddr->u;
+/* Don't free the objects inside; their ownership moved to "addr" */
+g_free(saddr);
 } else if (strstart(uri, "file:", NULL)) {
 addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
 addr->u.file.filename = g_strdup(uri + strlen("file:"));
-- 
2.34.1




RE: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object

2023-11-19 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, November 17, 2023 9:56 PM
>Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
>
>On 11/17/23 14:29, Eric Auger wrote:
>> Hi Cédric,
>>
>> On 11/17/23 12:39, Duan, Zhenzhong wrote:
>>> Hi Cédric,
>>>
 -Original Message-
 From: Cédric Le Goater 
 Sent: Friday, November 17, 2023 7:10 PM
 Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd
>object

 Hello,

> +int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
 hwaddr iova,
> +ram_addr_t size, void *vaddr, bool readonly)
> +{
> +int ret, fd = be->fd;
> +struct iommu_ioas_map map = {
> +.size = sizeof(map),
> +.flags = IOMMU_IOAS_MAP_READABLE |
> + IOMMU_IOAS_MAP_FIXED_IOVA,
> +.ioas_id = ioas_id,
> +.__reserved = 0,
> +.user_va = (uintptr_t)vaddr,
> +.iova = iova,
> +.length = size,
> +};
> +
> +if (!readonly) {
> +map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
> +}
> +
> +ret = ioctl(fd, IOMMU_IOAS_MAP, );
> +trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
> +  vaddr, readonly, ret);
> +if (ret) {
> +ret = -errno;
> +error_report("IOMMU_IOAS_MAP failed: %m");
> +}
> +return ret;
> +}
 When using a UEFI guest, QEMU reports errors when mapping regions
 in the top PCI space :

iommufd_backend_map_dma  iommufd=10 ioas=2 iova=0x38001000
 size=0x3000 addr=0x7fce2c28b000 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
 0x38001000, 0x3000, 0x7fce2c28b000) = -22 (Invalid argument)

iommufd_backend_map_dma  iommufd=10 ioas=2 iova=0x38004000
 size=0x4000 addr=0x7fce2c98 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
 0x38004000, 0x4000, 0x7fce2c98) = -22 (Invalid argument)

 This is because IOMMUFD reserved IOVAs areas are :

   [ fee0 - feef ]
   [ 80 -  ] (39 bits address space)

 which were allocated when the device was initially attached.
 The topology is basic. Something is wrong.
>>>
>>> Thanks for your report. This looks a hardware limit of
>>> host IOMMU address width(39) < guest physical address width.
>>>
>>> A similar issue with a fix submitted below, ccing related people.
>>> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg02937.html
>>> It looks the fix will not work for hotplug.
>>>
>>> Or below qemu cmdline may help:
>>> "-cpu host,host-phys-bits-limit=39"
>>
>> don't you have the same issue with legacy VFIO code, you should?
>
>I tend to be lazy and use seabios for guests on the command line.
>I do see the error with legacy VFIO and uefi.
>
>However, with the address space size work-around and iommufd, the
>error is different, an EFAULT now. Some page pinning issue it seems.

Yes, this reminds me of iommufd not supporting p2p mapping yet.
So EFAULT is expected. Maybe I should add a comment in 
docs/devel/vfio-iommufd.rst

Thanks
Zhenzhong


Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Dan Hoffman
As far as I can tell, yes. Any optimization level above O0 does not have
this issue (on this version of Clang, at least)

On Sun, Nov 19, 2023 at 4:54 PM Philippe Mathieu-Daudé 
wrote:

> Hi,
>
> On 19/11/23 21:31, Daniel Hoffman wrote:
> > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > used to remove references to undefined symbols at the compile stage.
> > Some build configurations with some compilers don't attempt to
> > simplify this logic down in some cases (the pattern appears to be
> > that the literal false must be the first term) and this was causing
> > some builds to emit references to undefined symbols.
> >
> > An example of such a configuration is clang 16.0.6 with the following
> > configure: ./configure --enable-debug --without-default-features
> > --target-list=x86_64-softmmu --enable-tcg-interpreter
>
> Is the '--enable-debug' option triggering this?
>
> I'm surprised the order of conditions matters for code elision...
>
> > Signed-off-by: Daniel Hoffman 
> > ---
> >   hw/i386/x86.c | 15 ---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index b3d054889bb..2b6291ad8d5 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int
> default_cpu_version)
> >   /*
> >* Can we support APIC ID 255 or higher?  With KVM, that requires
> >* both in-kernel lapic and X2APIC userspace API.
> > + *
> > + * kvm_enabled() must go first to ensure that kvm_* references are
> > + * not emitted for the linker to consume (kvm_enabled() is
> > + * a literal `0` in configurations where kvm_* aren't defined)
> >*/
> > -if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
> > +if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
> >   (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
> >   error_report("current -smp configuration requires kernel "
> >"irqchip and X2APIC API support.");
> > @@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
> >   }
> >   cpu->thread_id = topo_ids.smt_id;
> >
> > -if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
> > -kvm_enabled() && !kvm_hv_vpindex_settable()) {
> > +/*
> > +* kvm_enabled() must go first to ensure that kvm_* references are
> > +* not emitted for the linker to consume (kvm_enabled() is
> > +* a literal `0` in configurations where kvm_* aren't defined)
> > +*/
> > +if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)
> &&
> > +!kvm_hv_vpindex_settable()) {
> >   error_setg(errp, "kernel doesn't allow setting HyperV
> VP_INDEX");
> >   return;
> >   }
>
>


[PATCH v6 6/8] hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()

2023-11-19 Thread Gavin Shan
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.

Signed-off-by: Gavin Shan 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Leif Lindholm 
Reviewed-by: Richard Henderson 
---
v6: Move valid_cpu_types[] to sbsa_ref_class_init()   (Phil)
---
 hw/arm/sbsa-ref.c | 36 ++--
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f3c9704693..477dca0637 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -145,27 +145,6 @@ static const int sbsa_ref_irqmap[] = {
 [SBSA_GWDT_WS0] = 16,
 };
 
-static const char * const valid_cpus[] = {
-ARM_CPU_TYPE_NAME("cortex-a57"),
-ARM_CPU_TYPE_NAME("cortex-a72"),
-ARM_CPU_TYPE_NAME("neoverse-n1"),
-ARM_CPU_TYPE_NAME("neoverse-v1"),
-ARM_CPU_TYPE_NAME("neoverse-n2"),
-ARM_CPU_TYPE_NAME("max"),
-};
-
-static bool cpu_type_valid(const char *cpu)
-{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
-if (strcmp(cpu, valid_cpus[i]) == 0) {
-return true;
-}
-}
-return false;
-}
-
 static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
 {
 uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
@@ -733,11 +712,6 @@ static void sbsa_ref_init(MachineState *machine)
 const CPUArchIdList *possible_cpus;
 int n, sbsa_max_cpus;
 
-if (!cpu_type_valid(machine->cpu_type)) {
-error_report("sbsa-ref: CPU type %s not supported", machine->cpu_type);
-exit(1);
-}
-
 if (kvm_enabled()) {
 error_report("sbsa-ref: KVM is not supported for this machine");
 exit(1);
@@ -898,10 +872,20 @@ static void sbsa_ref_instance_init(Object *obj)
 static void sbsa_ref_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+static const char * const valid_cpu_types[] = {
+ARM_CPU_TYPE_NAME("cortex-a57"),
+ARM_CPU_TYPE_NAME("cortex-a72"),
+ARM_CPU_TYPE_NAME("neoverse-n1"),
+ARM_CPU_TYPE_NAME("neoverse-v1"),
+ARM_CPU_TYPE_NAME("neoverse-n2"),
+ARM_CPU_TYPE_NAME("max"),
+NULL,
+};
 
 mc->init = sbsa_ref_init;
 mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("neoverse-n1");
+mc->valid_cpu_types = valid_cpu_types;
 mc->max_cpus = 512;
 mc->pci_allow_0_address = true;
 mc->minimum_page_bits = 12;
-- 
2.41.0




[PATCH v6 4/8] hw/arm/virt: Hide host CPU model for tcg

2023-11-19 Thread Gavin Shan
The 'host' CPU model isn't available until KVM or HVF is enabled.
For example, the following error messages are seen when the guest
is started with option '-cpu cortex-a8' on tcg after the next commit
is applied to check the CPU type in machine_run_board_init().

  ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
  assertion failed: (model != NULL)
  Bail out! ERROR:../hw/core/machine.c:1423:is_cpu_type_supported: \
  assertion failed: (model != NULL)
  Aborted (core dumped)

Hide 'host' CPU model until KVM or HVF is enabled. With this applied,
the valid CPU models can be shown.

  qemu-system-aarch64: Invalid CPU type: cortex-a8
  The valid types are: cortex-a7, cortex-a15, cortex-a35, \
  cortex-a55, cortex-a72, cortex-a76, cortex-a710, a64fx, \
  neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53,  \
  cortex-a57, max

Signed-off-by: Gavin Shan 
Reviewed-by: Richard Henderson 
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index be2856c018..668c0d3194 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -220,7 +220,9 @@ static const char *valid_cpus[] = {
 #endif
 ARM_CPU_TYPE_NAME("cortex-a53"),
 ARM_CPU_TYPE_NAME("cortex-a57"),
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
 ARM_CPU_TYPE_NAME("host"),
+#endif
 ARM_CPU_TYPE_NAME("max"),
 };
 
-- 
2.41.0




[PATCH v6 3/8] machine: Print CPU model name instead of CPU type

2023-11-19 Thread Gavin Shan
The names of supported CPU models instead of CPU types should be
printed when the user specified CPU type isn't supported, to be
consistent with the output from '-cpu ?'.

Correct the error messages to print CPU model names instead of CPU
type names.

Signed-off-by: Gavin Shan 
---
 hw/core/machine.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d19aec11a3..2cdebcffaf 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1392,6 +1392,7 @@ static void is_cpu_type_supported(const MachineState 
*machine, Error **errp)
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 ObjectClass *oc = object_class_by_name(machine->cpu_type);
 CPUClass *cc;
+char *model;
 int i;
 
 /*
@@ -1408,17 +1409,25 @@ static void is_cpu_type_supported(const MachineState 
*machine, Error **errp)
 
 /* The user specified CPU type isn't valid */
 if (!mc->valid_cpu_types[i]) {
-error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+model = cpu_model_from_type(machine->cpu_type);
+g_assert(model != NULL);
+error_setg(errp, "Invalid CPU type: %s", model);
+g_free(model);
+
+model = cpu_model_from_type(mc->valid_cpu_types[0]);
+g_assert(model != NULL);
 if (!mc->valid_cpu_types[1]) {
-error_append_hint(errp, "The only valid type is: %s",
-  mc->valid_cpu_types[0]);
+error_append_hint(errp, "The only valid type is: %s", model);
 } else {
-error_append_hint(errp, "The valid types are: %s",
-  mc->valid_cpu_types[0]);
+error_append_hint(errp, "The valid types are: %s", model);
 }
+g_free(model);
 
 for (i = 1; mc->valid_cpu_types[i]; i++) {
-error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+model = cpu_model_from_type(mc->valid_cpu_types[i]);
+g_assert(model != NULL);
+error_append_hint(errp, ", %s", model);
+g_free(model);
 }
 
 error_append_hint(errp, "\n");
-- 
2.41.0




[PATCH v6 7/8] hw/arm: Check CPU type in machine_run_board_init()

2023-11-19 Thread Gavin Shan
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it by
ourselves.

Signed-off-by: Gavin Shan 
Reviewed-by: Richard Henderson 
---
v6: To have unified valid_cpu_types[] and move it to board's
class_init() function(Phil)
---
 hw/arm/bananapi_m2u.c   | 12 ++--
 hw/arm/cubieboard.c | 12 ++--
 hw/arm/mps2-tz.c| 26 --
 hw/arm/mps2.c   | 26 --
 hw/arm/msf2-som.c   | 12 ++--
 hw/arm/musca.c  | 12 +---
 hw/arm/npcm7xx_boards.c | 12 +---
 hw/arm/orangepi.c   | 12 ++--
 8 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/hw/arm/bananapi_m2u.c b/hw/arm/bananapi_m2u.c
index 8f24b18d8c..0a4b6f29b1 100644
--- a/hw/arm/bananapi_m2u.c
+++ b/hw/arm/bananapi_m2u.c
@@ -71,12 +71,6 @@ static void bpim2u_init(MachineState *machine)
 exit(1);
 }
 
-/* Only allow Cortex-A7 for this board */
-if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
-error_report("This board can only be used with cortex-a7 CPU");
-exit(1);
-}
-
 r40 = AW_R40(object_new(TYPE_AW_R40));
 object_property_add_child(OBJECT(machine), "soc", OBJECT(r40));
 object_unref(OBJECT(r40));
@@ -133,12 +127,18 @@ static void bpim2u_init(MachineState *machine)
 
 static void bpim2u_machine_init(MachineClass *mc)
 {
+static const char * const valid_cpu_types[] = {
+ARM_CPU_TYPE_NAME("cortex-a7"),
+NULL
+};
+
 mc->desc = "Bananapi M2U (Cortex-A7)";
 mc->init = bpim2u_init;
 mc->min_cpus = AW_R40_NUM_CPUS;
 mc->max_cpus = AW_R40_NUM_CPUS;
 mc->default_cpus = AW_R40_NUM_CPUS;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
+mc->valid_cpu_types = valid_cpu_types;
 mc->default_ram_size = 1 * GiB;
 mc->default_ram_id = "bpim2u.ram";
 }
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 29146f5018..b976727eef 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -52,12 +52,6 @@ static void cubieboard_init(MachineState *machine)
 exit(1);
 }
 
-/* Only allow Cortex-A8 for this board */
-if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
-error_report("This board can only be used with cortex-a8 CPU");
-exit(1);
-}
-
 a10 = AW_A10(object_new(TYPE_AW_A10));
 object_property_add_child(OBJECT(machine), "soc", OBJECT(a10));
 object_unref(OBJECT(a10));
@@ -114,8 +108,14 @@ static void cubieboard_init(MachineState *machine)
 
 static void cubieboard_machine_init(MachineClass *mc)
 {
+static const char * const valid_cpu_types[] = {
+ARM_CPU_TYPE_NAME("cortex-a8"),
+NULL
+};
+
 mc->desc = "cubietech cubieboard (Cortex-A8)";
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+mc->valid_cpu_types = valid_cpu_types;
 mc->default_ram_size = 1 * GiB;
 mc->init = cubieboard_init;
 mc->block_default_type = IF_IDE;
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 668db5ed61..5d8cdc1a4c 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -813,12 +813,6 @@ static void mps2tz_common_init(MachineState *machine)
 int num_ppcs;
 int i;
 
-if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
-error_report("This board can only be used with CPU %s",
- mc->default_cpu_type);
-exit(1);
-}
-
 if (machine->ram_size != mc->default_ram_size) {
 char *sz = size_to_str(mc->default_ram_size);
 error_report("Invalid RAM size, should be %s", sz);
@@ -1318,6 +1312,10 @@ static void mps2tz_an505_class_init(ObjectClass *oc, 
void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+static const char * const valid_cpu_types[] = {
+ARM_CPU_TYPE_NAME("cortex-m33"),
+NULL
+};
 
 mc->desc = "ARM MPS2 with AN505 FPGA image for Cortex-M33";
 mc->default_cpus = 1;
@@ -1325,6 +1323,7 @@ static void mps2tz_an505_class_init(ObjectClass *oc, void 
*data)
 mc->max_cpus = mc->default_cpus;
 mmc->fpga_type = FPGA_AN505;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m33");
+mc->valid_cpu_types = valid_cpu_types;
 mmc->scc_id = 0x41045050;
 mmc->sysclk_frq = 20 * 1000 * 1000; /* 20MHz */
 mmc->apb_periph_frq = mmc->sysclk_frq;
@@ -1347,6 +1346,10 @@ static void mps2tz_an521_class_init(ObjectClass *oc, 
void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_CLASS(oc);
+static const char * const valid_cpu_types[] = {
+ARM_CPU_TYPE_NAME("cortex-m33"),
+NULL
+};
 
 mc->desc = "ARM MPS2 with AN521 FPGA image for dual Cortex-M33";
 mc->default_cpus = 2;
@@ -1354,6 +1357,7 @@ static void mps2tz_an521_class_init(ObjectClass *oc, 

[PATCH v6 8/8] hw/riscv/shakti_c: Check CPU type in machine_run_board_init()

2023-11-19 Thread Gavin Shan
Set mc->valid_cpu_types so that the user specified CPU type can
be validated in machine_run_board_init(). We needn't to do it
by ourselves.

Signed-off-by: Gavin Shan 
Reviewed-by: Philippe Mathieu-Daudé 
---
v6: Move valid_cpu_types[] to shakti_c_machine_class_init()   (Phil)
---
 hw/riscv/shakti_c.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 12ea74b032..3888034c2b 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -28,7 +28,6 @@
 #include "exec/address-spaces.h"
 #include "hw/riscv/boot.h"
 
-
 static const struct MemmapEntry {
 hwaddr base;
 hwaddr size;
@@ -47,12 +46,6 @@ static void shakti_c_machine_state_init(MachineState *mstate)
 ShaktiCMachineState *sms = RISCV_SHAKTI_MACHINE(mstate);
 MemoryRegion *system_memory = get_system_memory();
 
-/* Allow only Shakti C CPU for this platform */
-if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) {
-error_report("This board can only be used with Shakti C CPU");
-exit(1);
-}
-
 /* Initialize SoC */
 object_initialize_child(OBJECT(mstate), "soc", >soc,
 TYPE_RISCV_SHAKTI_SOC);
@@ -82,9 +75,15 @@ static void shakti_c_machine_instance_init(Object *obj)
 static void shakti_c_machine_class_init(ObjectClass *klass, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(klass);
+static const char * const valid_cpu_types[] = {
+RISCV_CPU_TYPE_NAME("shakti-c"),
+NULL
+};
+
 mc->desc = "RISC-V Board compatible with Shakti SDK";
 mc->init = shakti_c_machine_state_init;
 mc->default_cpu_type = TYPE_RISCV_CPU_SHAKTI_C;
+mc->valid_cpu_types = valid_cpu_types;
 mc->default_ram_id = "riscv.shakti.c.ram";
 }
 
-- 
2.41.0




[PATCH v6 5/8] hw/arm/virt: Check CPU type in machine_run_board_init()

2023-11-19 Thread Gavin Shan
Set mc->valid_cpu_types so that the user specified CPU type can be
validated in machine_run_board_init(). We needn't to do the check
by ourselves.

Signed-off-by: Gavin Shan 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
v6: Move valid_cpu_types[] to virt_machine_class_init()(Phil)
---
 hw/arm/virt.c | 62 +++
 1 file changed, 23 insertions(+), 39 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 668c0d3194..04f9f5fa56 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -204,40 +204,6 @@ static const int a15irqmap[] = {
 [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
 };
 
-static const char *valid_cpus[] = {
-#ifdef CONFIG_TCG
-ARM_CPU_TYPE_NAME("cortex-a7"),
-ARM_CPU_TYPE_NAME("cortex-a15"),
-ARM_CPU_TYPE_NAME("cortex-a35"),
-ARM_CPU_TYPE_NAME("cortex-a55"),
-ARM_CPU_TYPE_NAME("cortex-a72"),
-ARM_CPU_TYPE_NAME("cortex-a76"),
-ARM_CPU_TYPE_NAME("cortex-a710"),
-ARM_CPU_TYPE_NAME("a64fx"),
-ARM_CPU_TYPE_NAME("neoverse-n1"),
-ARM_CPU_TYPE_NAME("neoverse-v1"),
-ARM_CPU_TYPE_NAME("neoverse-n2"),
-#endif
-ARM_CPU_TYPE_NAME("cortex-a53"),
-ARM_CPU_TYPE_NAME("cortex-a57"),
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-ARM_CPU_TYPE_NAME("host"),
-#endif
-ARM_CPU_TYPE_NAME("max"),
-};
-
-static bool cpu_type_valid(const char *cpu)
-{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
-if (strcmp(cpu, valid_cpus[i]) == 0) {
-return true;
-}
-}
-return false;
-}
-
 static void create_randomness(MachineState *ms, const char *node)
 {
 struct {
@@ -2041,11 +2007,6 @@ static void machvirt_init(MachineState *machine)
 unsigned int smp_cpus = machine->smp.cpus;
 unsigned int max_cpus = machine->smp.max_cpus;
 
-if (!cpu_type_valid(machine->cpu_type)) {
-error_report("mach-virt: CPU type %s not supported", 
machine->cpu_type);
-exit(1);
-}
-
 possible_cpus = mc->possible_cpu_arch_ids(machine);
 
 /*
@@ -2939,6 +2900,28 @@ static void virt_machine_class_init(ObjectClass *oc, 
void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+static const char * const valid_cpu_types[] = {
+#ifdef CONFIG_TCG
+ARM_CPU_TYPE_NAME("cortex-a7"),
+ARM_CPU_TYPE_NAME("cortex-a15"),
+ARM_CPU_TYPE_NAME("cortex-a35"),
+ARM_CPU_TYPE_NAME("cortex-a55"),
+ARM_CPU_TYPE_NAME("cortex-a72"),
+ARM_CPU_TYPE_NAME("cortex-a76"),
+ARM_CPU_TYPE_NAME("cortex-a710"),
+ARM_CPU_TYPE_NAME("a64fx"),
+ARM_CPU_TYPE_NAME("neoverse-n1"),
+ARM_CPU_TYPE_NAME("neoverse-v1"),
+ARM_CPU_TYPE_NAME("neoverse-n2"),
+#endif
+ARM_CPU_TYPE_NAME("cortex-a53"),
+ARM_CPU_TYPE_NAME("cortex-a57"),
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+ARM_CPU_TYPE_NAME("host"),
+#endif
+ARM_CPU_TYPE_NAME("max"),
+NULL
+};
 
 mc->init = machvirt_init;
 /* Start with max_cpus set to 512, which is the maximum supported by KVM.
@@ -2965,6 +2948,7 @@ static void virt_machine_class_init(ObjectClass *oc, void 
*data)
 #else
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
 #endif
+mc->valid_cpu_types = valid_cpu_types;
 mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
 mc->kvm_type = virt_kvm_type;
 assert(!mc->get_hotplug_handler);
-- 
2.41.0




[PATCH v6 1/8] machine: Use error handling when CPU type is checked

2023-11-19 Thread Gavin Shan
QEMU will be terminated if the specified CPU type isn't supported
in machine_run_board_init(). The list of supported CPU type names
is tracked by mc->valid_cpu_types.

The error handling can be used to propagate error messages, to be
consistent how the errors are handled for other situations in the
same function.

No functional change intended.

Suggested-by: Igor Mammedov 
Signed-off-by: Gavin Shan 
---
 hw/core/machine.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c17398141..5b45dbbbd5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const 
char *mem_path, Error *
 MachineClass *machine_class = MACHINE_GET_CLASS(machine);
 ObjectClass *oc = object_class_by_name(machine->cpu_type);
 CPUClass *cc;
+Error *local_err = NULL;
 
 /* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
@@ -1466,15 +1467,16 @@ void machine_run_board_init(MachineState *machine, 
const char *mem_path, Error *
 
 if (!machine_class->valid_cpu_types[i]) {
 /* The user specified CPU is not valid */
-error_report("Invalid CPU type: %s", machine->cpu_type);
-error_printf("The valid types are: %s",
- machine_class->valid_cpu_types[0]);
+error_setg(_err, "Invalid CPU type: %s", machine->cpu_type);
+error_append_hint(_err, "The valid types are: %s",
+  machine_class->valid_cpu_types[0]);
 for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-error_printf(", %s", machine_class->valid_cpu_types[i]);
+error_append_hint(_err, ", %s",
+  machine_class->valid_cpu_types[i]);
 }
-error_printf("\n");
+error_append_hint(_err, "\n");
 
-exit(1);
+error_propagate(errp, local_err);
 }
 }
 
-- 
2.41.0




[PATCH v6 0/8] Unified CPU type check

2023-11-19 Thread Gavin Shan
This series bases on Phil's repository because the prepatory commits
have been queued to the branch.

  https://gitlab.com/philmd/qemu.git (branch: cpus-next)

There are two places where the user specified CPU type is checked to see
if it's supported or allowed by the board: machine_run_board_init() and
mc->init(). We don't have to maintain two duplicate sets of logic. This
series intends to move the check to machine_run_board_init() so that we
have unified CPU type check.

PATCH[1-3] refactors the logic to validate CPU type in machine_run_board_init()
PATCH[4-8] validates the CPU type in machine_run_board_init() for the
   individual boards

v1: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00302.html
v2: https://lists.nongnu.org/archive/html/qemu-arm/2023-07/msg00528.html
v3: https://lists.nongnu.org/archive/html/qemu-arm/2023-09/msg00157.html
v4: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg5.html
v5: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00611.html

Testing
===

With the following command lines, the output messages are varied before
and after the series is applied.

  ./build/qemu-system-aarch64\
  -accel tcg -machine virt,gic-version=3 \
  -cpu cortex-a8 -smp maxcpus=2,cpus=1

Before the series is applied:

  qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported

After the series is applied:

  qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu
  The valid models are: cortex-a7, cortex-a15, cortex-a35, cortex-a55,
cortex-a72, cortex-a76, a64fx, neoverse-n1,
neoverse-v1, cortex-a53, cortex-a57, max

Changelog
=
v6:
  * Drop PATCH[v5 01-23], queued by Phil (Phil)
  * Clearer hint if only one CPU type is supported and have
'const MachineState *' in is_cpu_type_supported()(Phil)
  * Move valid_cpu_types[] to board's class_init() function  (Phil)
v5:
  * PATCH[v5 01] to remove CPU class 'ev67' for alpha(Ricard/Igor)
  * PATCH[v5 02] to remove object_class_is_abstract() for hppa   (Gavin)
  * Don't move cpu_class_by_name()   (Richard)
  * PATCH[v5 04] to remove 'oc == NULL' since the check has
been covered in object_class_dynamic_cast()  (Igor)
  * Introduce generic cpu_list(), shared by most of the targets  (Richard)
  * Use g_str_has_suffix and g_auto_free (Richard)
  * Collect r-bs from Igor and Richard   (Gavin)
v4:
  * Integrate Philippe's patches where cpu_class_by_name()
is consolidated and my duplicate code is dropped(Philippe)
  * Simplified changelog and improvements   (Thomas)
  * g_assert() on the return value from cpu_model_from_type()
in is_cpu_type_supported()  (Philippe)
  * Collected r-bs from Philippe Mathieu-Daudé, Leif Lindholm,
Bastian Koppelmann, Daniel Henrique Barboza, Cédric Le Goater,
Gavin Shan  (Gavin)
v3:
  * Generic helper cpu_model_from_type()(Igor)
  * Apply cpu_model_from_type() to the individual targets   (Igor)
  * Implement cpu_list() for the missed targets (Gavin)
  * Remove mc->valid_cpu_models (Richard)
  * Separate patch to constify mc->validate_cpu_types   (Gavin)
v2:
  * Constify mc->valid_cpu_types(Richard)
  * Print the supported CPU models, instead of typenames(Peter)
  * Misc improvements for the hleper to do the check(Igor)
  * More patches to move the check  (Marcin)

Gavin Shan (8):
  machine: Use error handling when CPU type is checked
  machine: Introduce helper is_cpu_type_supported()
  machine: Print CPU model name instead of CPU type
  hw/arm/virt: Hide host CPU model for tcg
  hw/arm/virt: Check CPU type in machine_run_board_init()
  hw/arm/sbsa-ref: Check CPU type in machine_run_board_init()
  hw/arm: Check CPU type in machine_run_board_init()
  hw/riscv/shakti_c: Check CPU type in machine_run_board_init()

 hw/arm/bananapi_m2u.c   | 12 ++---
 hw/arm/cubieboard.c | 12 ++---
 hw/arm/mps2-tz.c| 26 ---
 hw/arm/mps2.c   | 26 ---
 hw/arm/msf2-som.c   | 12 ++---
 hw/arm/musca.c  | 12 +++--
 hw/arm/npcm7xx_boards.c | 12 +++--
 hw/arm/orangepi.c   | 12 ++---
 hw/arm/sbsa-ref.c   | 36 +--
 hw/arm/virt.c   | 60 ++---
 hw/core/machine.c   | 97 +
 hw/riscv/shakti_c.c | 13 +++---
 12 files changed, 173 insertions(+), 157 deletions(-)

-- 
2.41.0




[PATCH v6 2/8] machine: Introduce helper is_cpu_type_supported()

2023-11-19 Thread Gavin Shan
The logic, to check if the specified CPU type is supported in
machine_run_board_init(), is independent enough. Factor it out into
helper is_cpu_type_supported(). machine_run_board_init() looks a bit
clean with this. Since we're here, @machine_class is renamed to @mc
to avoid multiple line spanning of code. The error messages and comments
are tweaked a bit either.

No functional change intended.

Signed-off-by: Gavin Shan 
---
v6: 'const MachineState *' in is_cpu_type_supported() (Phil)
Clearer hint if only one valid CPU is supported   (Phil)
---
 hw/core/machine.c | 88 +++
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5b45dbbbd5..d19aec11a3 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1387,13 +1387,57 @@ out:
 return r;
 }
 
+static void is_cpu_type_supported(const MachineState *machine, Error **errp)
+{
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+ObjectClass *oc = object_class_by_name(machine->cpu_type);
+CPUClass *cc;
+int i;
+
+/*
+ * Check if the user specified CPU type is supported when the valid
+ * CPU types have been determined. Note that the user specified CPU
+ * type is provided through '-cpu' option.
+ */
+if (mc->valid_cpu_types && machine->cpu_type) {
+for (i = 0; mc->valid_cpu_types[i]; i++) {
+if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+break;
+}
+}
+
+/* The user specified CPU type isn't valid */
+if (!mc->valid_cpu_types[i]) {
+error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+if (!mc->valid_cpu_types[1]) {
+error_append_hint(errp, "The only valid type is: %s",
+  mc->valid_cpu_types[0]);
+} else {
+error_append_hint(errp, "The valid types are: %s",
+  mc->valid_cpu_types[0]);
+}
+
+for (i = 1; mc->valid_cpu_types[i]; i++) {
+error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+}
+
+error_append_hint(errp, "\n");
+return;
+}
+}
+
+/* Check if CPU type is deprecated and warn if so */
+cc = CPU_CLASS(oc);
+if (cc && cc->deprecation_note) {
+warn_report("CPU model %s is deprecated -- %s",
+machine->cpu_type, cc->deprecation_note);
+}
+}
 
 void machine_run_board_init(MachineState *machine, const char *mem_path, Error 
**errp)
 {
 ERRP_GUARD();
 MachineClass *machine_class = MACHINE_GET_CLASS(machine);
-ObjectClass *oc = object_class_by_name(machine->cpu_type);
-CPUClass *cc;
 Error *local_err = NULL;
 
 /* This checkpoint is required by replay to separate prior clock
@@ -1449,42 +1493,10 @@ void machine_run_board_init(MachineState *machine, 
const char *mem_path, Error *
 machine->ram = machine_consume_memdev(machine, machine->memdev);
 }
 
-/* If the machine supports the valid_cpu_types check and the user
- * specified a CPU with -cpu check here that the user CPU is supported.
- */
-if (machine_class->valid_cpu_types && machine->cpu_type) {
-int i;
-
-for (i = 0; machine_class->valid_cpu_types[i]; i++) {
-if (object_class_dynamic_cast(oc,
-  machine_class->valid_cpu_types[i])) {
-/* The user specified CPU is in the valid field, we are
- * good to go.
- */
-break;
-}
-}
-
-if (!machine_class->valid_cpu_types[i]) {
-/* The user specified CPU is not valid */
-error_setg(_err, "Invalid CPU type: %s", machine->cpu_type);
-error_append_hint(_err, "The valid types are: %s",
-  machine_class->valid_cpu_types[0]);
-for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-error_append_hint(_err, ", %s",
-  machine_class->valid_cpu_types[i]);
-}
-error_append_hint(_err, "\n");
-
-error_propagate(errp, local_err);
-}
-}
-
-/* Check if CPU type is deprecated and warn if so */
-cc = CPU_CLASS(oc);
-if (cc && cc->deprecation_note) {
-warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
-cc->deprecation_note);
+/* Check if the CPU type is supported */
+is_cpu_type_supported(machine, _err);
+if (local_err) {
+error_propagate(errp, local_err);
 }
 
 if (machine->cgs) {
-- 
2.41.0




Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Philippe Mathieu-Daudé

Hi,

On 19/11/23 21:31, Daniel Hoffman wrote:

`kvm_enabled()` is compiled down to `0` and short-circuit logic is
used to remove references to undefined symbols at the compile stage.
Some build configurations with some compilers don't attempt to
simplify this logic down in some cases (the pattern appears to be
that the literal false must be the first term) and this was causing
some builds to emit references to undefined symbols.

An example of such a configuration is clang 16.0.6 with the following
configure: ./configure --enable-debug --without-default-features
--target-list=x86_64-softmmu --enable-tcg-interpreter


Is the '--enable-debug' option triggering this?

I'm surprised the order of conditions matters for code elision...


Signed-off-by: Daniel Hoffman 
---
  hw/i386/x86.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b3d054889bb..2b6291ad8d5 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
  /*
   * Can we support APIC ID 255 or higher?  With KVM, that requires
   * both in-kernel lapic and X2APIC userspace API.
+ *
+ * kvm_enabled() must go first to ensure that kvm_* references are
+ * not emitted for the linker to consume (kvm_enabled() is
+ * a literal `0` in configurations where kvm_* aren't defined)
   */
-if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
+if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
  (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
  error_report("current -smp configuration requires kernel "
   "irqchip and X2APIC API support.");
@@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
  }
  cpu->thread_id = topo_ids.smt_id;
  
-if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&

-kvm_enabled() && !kvm_hv_vpindex_settable()) {
+/*
+* kvm_enabled() must go first to ensure that kvm_* references are
+* not emitted for the linker to consume (kvm_enabled() is
+* a literal `0` in configurations where kvm_* aren't defined)
+*/
+if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
+!kvm_hv_vpindex_settable()) {
  error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
  return;
  }





[PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

2023-11-19 Thread Philippe Mathieu-Daudé
Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format

  Message Format

  The same message format is used for RXFIFO, TXFIFO, and TXHPB.
  Each message includes four words (16 bytes). Software must read
  and write all four words regardless of the actual number of data
  bytes and valid fields in the message.

There is no mention in this reference manual about what the
hardware does when not all four words are read. To fix the
reported underflow behavior, I choose to fill the 4 frame data
registers when the first register (ID) is accessed, which is how
I expect hardware would do.

Reported-by: Qiang Liu 
Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/net/can/xlnx-zynqmp-can.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
index 58938b574e..c63fb4a83c 100644
--- a/hw/net/can/xlnx-zynqmp-can.c
+++ b/hw/net/can/xlnx-zynqmp-can.c
@@ -777,14 +777,18 @@ static void update_rx_fifo(XlnxZynqMPCANState *s, const 
qemu_can_frame *frame)
 }
 }
 
-static uint64_t can_rxfifo_pre_read(RegisterInfo *reg, uint64_t val)
+static uint64_t can_rxfifo_post_read_id(RegisterInfo *reg, uint64_t val)
 {
 XlnxZynqMPCANState *s = XLNX_ZYNQMP_CAN(reg->opaque);
+unsigned used = fifo32_num_used(>rx_fifo);
 
-if (!fifo32_is_empty(>rx_fifo)) {
-val = fifo32_pop(>rx_fifo);
-} else {
+if (used < CAN_FRAME_SIZE) {
 ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXUFLW, 1);
+} else {
+val = s->regs[R_RXFIFO_ID] = fifo32_pop(>rx_fifo);
+s->regs[R_RXFIFO_DLC] = fifo32_pop(>rx_fifo);
+s->regs[R_RXFIFO_DATA1] = fifo32_pop(>rx_fifo);
+s->regs[R_RXFIFO_DATA2] = fifo32_pop(>rx_fifo);
 }
 
 can_update_irq(s);
@@ -945,14 +949,11 @@ static const RegisterAccessInfo can_regs_info[] = {
 .post_write = can_tx_post_write,
 },{ .name = "RXFIFO_ID",  .addr = A_RXFIFO_ID,
 .ro = 0x,
-.post_read = can_rxfifo_pre_read,
+.post_read = can_rxfifo_post_read_id,
 },{ .name = "RXFIFO_DLC",  .addr = A_RXFIFO_DLC,
 .rsvd = 0xfff,
-.post_read = can_rxfifo_pre_read,
 },{ .name = "RXFIFO_DATA1",  .addr = A_RXFIFO_DATA1,
-.post_read = can_rxfifo_pre_read,
 },{ .name = "RXFIFO_DATA2",  .addr = A_RXFIFO_DATA2,
-.post_read = can_rxfifo_pre_read,
 },{ .name = "AFR",  .addr = A_AFR,
 .rsvd = 0xfff0,
 .post_write = can_filter_enable_post_write,
-- 
2.41.0




[PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs

2023-11-19 Thread Philippe Mathieu-Daudé
Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format

  Message Format

  The same message format is used for RXFIFO, TXFIFO, and TXHPB.
  Each message includes four words (16 bytes). Software must read
  and write all four words regardless of the actual number of data
  bytes and valid fields in the message.

There is no mention in this reference manual about what the
hardware does when not all four words are written. To fix the
reported underflow behavior when DATA2 register is written,
I choose to fill the data with the previous content of the
ID / DLC / DATA1 registers, which is how I expect hardware
would do.

Note there is no hardware flag raised under such condition.

Reported-by: Qiang Liu 
Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1425
Signed-off-by: Philippe Mathieu-Daudé 
Francisco Iglesias 
---
 hw/net/can/xlnx-zynqmp-can.c | 49 +---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
index e93e6c5e19..58938b574e 100644
--- a/hw/net/can/xlnx-zynqmp-can.c
+++ b/hw/net/can/xlnx-zynqmp-can.c
@@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
 return true;
 }
 
+static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t *data)
+{
+unsigned used = fifo32_num_used(fifo);
+bool is_txhpb = fifo == >txhpb_fifo;
+
+assert(used > 0);
+used %= CAN_FRAME_SIZE;
+
+/*
+ * Frame Message Format
+ *
+ * Each frame includes four words (16 bytes). Software must read and write
+ * all four words regardless of the actual number of data bytes and valid
+ * fields in the message.
+ * If software misbehave (not writting all four words), we use the previous
+ * registers content to initialize each missing word.
+ */
+if (used > 0) {
+/* ID, DLC, DATA1 missing */
+data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
+} else {
+data[0] = fifo32_pop(fifo);
+}
+if (used == 1 || used == 2) {
+/* DLC, DATA1 missing */
+data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
+} else {
+data[1] = fifo32_pop(fifo);
+}
+if (used == 1) {
+/* DATA1 missing */
+data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
+} else {
+data[2] = fifo32_pop(fifo);
+}
+/* DATA2 triggered the transfer thus is always available */
+data[3] = fifo32_pop(fifo);
+
+if (used) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Incomplete CAN frame (only %u/%u slots used)\n",
+  TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
+}
+}
+
 static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
 {
 qemu_can_frame frame;
@@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 
*fifo)
 }
 
 while (!fifo32_is_empty(fifo)) {
-for (i = 0; i < CAN_FRAME_SIZE; i++) {
-data[i] = fifo32_pop(fifo);
-}
+read_tx_frame(s, fifo, data);
 
 if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
 /*
-- 
2.41.0




[PATCH-for-8.2 v2 0/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping FIFOs

2023-11-19 Thread Philippe Mathieu-Daudé
Fix a pair of fuzzed bugs.

Patch #1 is reviewed, #2 is new.

Tested with the CAN tests from 'make check-qtest-aarch64'.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs
  hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

 hw/net/can/xlnx-zynqmp-can.c | 66 ++--
 1 file changed, 55 insertions(+), 11 deletions(-)

-- 
2.41.0




Re: [PATCH v3 0/4] ide: implement simple legacy/native mode switching for PCI IDE controllers

2023-11-19 Thread BALATON Zoltan

On Sun, 19 Nov 2023, BALATON Zoltan wrote:

On Thu, 16 Nov 2023, Mark Cave-Ayland wrote:
This series adds a simple implementation of legacy/native mode switching 
for PCI

IDE controllers and updates the via-ide device to use it.

The approach I take here is to add a new pci_ide_update_mode() function 
which handles
management of the PCI BARs and legacy IDE ioports for each mode to avoid 
exposing

details of the internal logic to individual PCI IDE controllers.

As noted in [1] this is extracted from a local WIP branch I have which 
contains

further work in this area. However for the moment I've kept it simple (and
restricted it to the via-ide device) which is good enough for Zoltan's PPC
images whilst paving the way for future improvements after 8.2.

Signed-off-by: Mark Cave-Ayland 

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg05403.html

v3:
- Rebase onto master
- Move ide_portio_list[] and ide_portio_list2[] to IDE core to prevent 
duplication in

 hw/ide/pci.c
- Don't zero BARs when switching from native mode to legacy mode, instead 
always force
 them to read zero as suggested in the PCI IDE specification (note: this 
also appears

 to fix the fuloong2e machine booting from IDE)


Not sure you're getting this, see also:
https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg04167.html
but this seems to break latest version of the AmigaOS driver for some reason. 
I assume this is the BAR zeroing that causes this as it works with v2 series 
and nothing else changed in v3 that could cause this. Testing was done by 
Rene Engel, cc'd so maybe he can add more info. It seems to work with my 
patch that sets BARs to legacy values and with v2 that sets them to 0 but not 
with v3 which should also read 0 but maybe something is off here.


The change log of the AmigaOS driver version which only works with v2 of 
this series says:


"Replaced ReadConfigLong(PCI_BASE_ADDRESS_x) calls with their
PCI interface variant GetResourceRange(x)->BaseAddress to get
the correct base address instead of the *bus* base address."

but not sure what that means. Apparently it reads BAR values differently 
and crashes before finding the IDE ports with v3 but not with v2. As it 
crashes it does not even print debug logs to check what it read.


(Additionally would also need the default value for BMDMA BAR4 as I wrote 
before but that's disabled by default so only an issue if one tries to 
enable it. Would be nics to have though as UDMA is faster.)


Regards,
BALATON Zoltan



- Add comments in pci_ide_update_mode() suggested by Kevin
- Drop the existing R-B and T-B tags: whilst this passes my local tests, 
the behaviour

 around zero BARs feels different enough here

v2:
- Rebase onto master
- Mask the bottom 4 bits of PCI_CLASS_PROG in pci_ide_update_mode() in 
patch 1

- Add patch 2 to remove the default BAR addresses to avoid confusion
- Don't set PCI_INTERRUPT_PIN directly in via_ide_reset() as it is already 
set
 by pci_ide_update_mode() in patch 3, and reword the commit message 
accordingly

- Add Tested-By tags from Zoltan and Bernhard


Mark Cave-Ayland (4):
 ide/ioport: move ide_portio_list[] and ide_portio_list2[] definitions
   to IDE core
 ide/pci: introduce pci_ide_update_mode() function
 ide/via: don't attempt to set default BAR addresses
 hw/ide/via: implement legacy/native mode switching

hw/ide/core.c | 12 ++
hw/ide/ioport.c   | 12 --
hw/ide/pci.c  | 84 +++
hw/ide/via.c  | 44 
include/hw/ide/internal.h |  3 ++
include/hw/ide/pci.h  |  1 +
6 files changed, 137 insertions(+), 19 deletions(-)









Re: [PATCH v3 0/4] ide: implement simple legacy/native mode switching for PCI IDE controllers

2023-11-19 Thread BALATON Zoltan

On Thu, 16 Nov 2023, Mark Cave-Ayland wrote:

This series adds a simple implementation of legacy/native mode switching for PCI
IDE controllers and updates the via-ide device to use it.

The approach I take here is to add a new pci_ide_update_mode() function which 
handles
management of the PCI BARs and legacy IDE ioports for each mode to avoid 
exposing
details of the internal logic to individual PCI IDE controllers.

As noted in [1] this is extracted from a local WIP branch I have which contains
further work in this area. However for the moment I've kept it simple (and
restricted it to the via-ide device) which is good enough for Zoltan's PPC
images whilst paving the way for future improvements after 8.2.

Signed-off-by: Mark Cave-Ayland 

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg05403.html

v3:
- Rebase onto master
- Move ide_portio_list[] and ide_portio_list2[] to IDE core to prevent 
duplication in
 hw/ide/pci.c
- Don't zero BARs when switching from native mode to legacy mode, instead 
always force
 them to read zero as suggested in the PCI IDE specification (note: this also 
appears
 to fix the fuloong2e machine booting from IDE)


Not sure you're getting this, see also:
https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg04167.html
but this seems to break latest version of the AmigaOS driver for some 
reason. I assume this is the BAR zeroing that causes this as it works with 
v2 series and nothing else changed in v3 that could cause this. Testing 
was done by Rene Engel, cc'd so maybe he can add more info. It seems to 
work with my patch that sets BARs to legacy values and with v2 that sets 
them to 0 but not with v3 which should also read 0 but maybe something is 
off here.


Regards,
BALATON Zoltan


- Add comments in pci_ide_update_mode() suggested by Kevin
- Drop the existing R-B and T-B tags: whilst this passes my local tests, the 
behaviour
 around zero BARs feels different enough here

v2:
- Rebase onto master
- Mask the bottom 4 bits of PCI_CLASS_PROG in pci_ide_update_mode() in patch 1
- Add patch 2 to remove the default BAR addresses to avoid confusion
- Don't set PCI_INTERRUPT_PIN directly in via_ide_reset() as it is already set
 by pci_ide_update_mode() in patch 3, and reword the commit message accordingly
- Add Tested-By tags from Zoltan and Bernhard


Mark Cave-Ayland (4):
 ide/ioport: move ide_portio_list[] and ide_portio_list2[] definitions
   to IDE core
 ide/pci: introduce pci_ide_update_mode() function
 ide/via: don't attempt to set default BAR addresses
 hw/ide/via: implement legacy/native mode switching

hw/ide/core.c | 12 ++
hw/ide/ioport.c   | 12 --
hw/ide/pci.c  | 84 +++
hw/ide/via.c  | 44 
include/hw/ide/internal.h |  3 ++
include/hw/ide/pci.h  |  1 +
6 files changed, 137 insertions(+), 19 deletions(-)






Re: [PATCH] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Dan Hoffman
Submitted a v3 with the minimum reproducible build configuration

On Sun, Nov 19, 2023 at 2:25 PM Michael S. Tsirkin  wrote:
>
> On Sun, Nov 19, 2023 at 02:19:25PM -0600, Dan Hoffman wrote:
> > Clang 16.0.6
> >
> > I can re-submit with the compiler and version if that helps.
>
> Worth mentioning this and the flags used I think.
>
> > On Sun, Nov 19, 2023 at 2:02 PM Michael S. Tsirkin  wrote:
> > >
> > > On Sun, Nov 19, 2023 at 11:03:54AM -0600, Dan Hoffman wrote:
> > > > On Sun, Nov 19, 2023 at 1:23 AM Michael S. Tsirkin  
> > > > wrote:
> > > > >
> > > > > On Sat, Nov 18, 2023 at 10:25:31AM -0800, Daniel Hoffman wrote:
> > > > > > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > > > > > used to remmove references to undefined symbols at the compile 
> > > > > > stage.
> > > > > > Some build configurations with some compilers don't attempt to
> > > > > > simplify this logic down in some cases (the pattern appears to be
> > > > > > that the literal false must be the first term) and this was causing
> > > > > > some builds to emit references to undefined symbols.
> > > > > >
> > > > > > Signed-off-by: Daniel Hoffman 
> > > > >
> > > > > Could we add a bit more detail here? Will help make sure
> > > > > this does not break again in the future.
> > > >
> > > > The configuration script was ran as such:  ../configure
> > > > --without-default-features --target-list=x86_64-softmmu,i386-softmmu
> > > > --enable-debug --enable-tcg-interpreter --enable-debug-tcg
> > > > --enable-debug-mutex
> > > >
> > > > I'm pretty sure the only relevant flags here are
> > > > --without-default-features, --target-list including x86_64-softmmu and
> > > > --enable-debug
> > > >
> > > > The only error I see is this: [...]/hw/i386/x86.c:422:(.text+0x1004):
> > > > undefined reference to `kvm_hv_vpindex_settable' (the other
> > > > kvm_enabled() was moved for the sake of consistency). My compiler is
> > > > clang (16.0.6).
> > > >
> > > > I haven't looked into the heuristics or logic for how the compile-time
> > > > short-circuit logic works, but I assumed only the first parameter is
> > > > "guaranteed" to be checked for a literal false (guaranteed is in
> > > > quotes because that's just how clang works, not because it's a feature
> > > > of the language IIRC).
> > > >
> > > > This pattern relies on somes subtle behavior with the compiler, so my
> > > > suggestion going forward would be to not rely on code optimizations
> > > > removing undefined references based on short-circuit logic (instead
> > > > have some configuration macro defined that disables all relevant
> > > > code). I'm a new contributor, so I submitted the minimum to make it
> > > > work on my machine.
> > > >
> > > > If you have any other questions, please let me know.
> > > >
> > > > Thanks!
> > >
> > > which compiler is this?
> > >
> > > --
> > > MST
> > >
>



[PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Daniel Hoffman
`kvm_enabled()` is compiled down to `0` and short-circuit logic is
used to remove references to undefined symbols at the compile stage.
Some build configurations with some compilers don't attempt to
simplify this logic down in some cases (the pattern appears to be
that the literal false must be the first term) and this was causing
some builds to emit references to undefined symbols.

An example of such a configuration is clang 16.0.6 with the following
configure: ./configure --enable-debug --without-default-features
--target-list=x86_64-softmmu --enable-tcg-interpreter

Signed-off-by: Daniel Hoffman 
---
 hw/i386/x86.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b3d054889bb..2b6291ad8d5 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
 /*
  * Can we support APIC ID 255 or higher?  With KVM, that requires
  * both in-kernel lapic and X2APIC userspace API.
+ *
+ * kvm_enabled() must go first to ensure that kvm_* references are
+ * not emitted for the linker to consume (kvm_enabled() is
+ * a literal `0` in configurations where kvm_* aren't defined)
  */
-if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
+if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
 (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
 error_report("current -smp configuration requires kernel "
  "irqchip and X2APIC API support.");
@@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
 }
 cpu->thread_id = topo_ids.smt_id;
 
-if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
-kvm_enabled() && !kvm_hv_vpindex_settable()) {
+/*
+* kvm_enabled() must go first to ensure that kvm_* references are
+* not emitted for the linker to consume (kvm_enabled() is
+* a literal `0` in configurations where kvm_* aren't defined)
+*/
+if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
+!kvm_hv_vpindex_settable()) {
 error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
 return;
 }
-- 
2.40.1




Re: [PATCH] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Michael S. Tsirkin
On Sun, Nov 19, 2023 at 02:19:25PM -0600, Dan Hoffman wrote:
> Clang 16.0.6
> 
> I can re-submit with the compiler and version if that helps.

Worth mentioning this and the flags used I think.

> On Sun, Nov 19, 2023 at 2:02 PM Michael S. Tsirkin  wrote:
> >
> > On Sun, Nov 19, 2023 at 11:03:54AM -0600, Dan Hoffman wrote:
> > > On Sun, Nov 19, 2023 at 1:23 AM Michael S. Tsirkin  
> > > wrote:
> > > >
> > > > On Sat, Nov 18, 2023 at 10:25:31AM -0800, Daniel Hoffman wrote:
> > > > > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > > > > used to remmove references to undefined symbols at the compile stage.
> > > > > Some build configurations with some compilers don't attempt to
> > > > > simplify this logic down in some cases (the pattern appears to be
> > > > > that the literal false must be the first term) and this was causing
> > > > > some builds to emit references to undefined symbols.
> > > > >
> > > > > Signed-off-by: Daniel Hoffman 
> > > >
> > > > Could we add a bit more detail here? Will help make sure
> > > > this does not break again in the future.
> > >
> > > The configuration script was ran as such:  ../configure
> > > --without-default-features --target-list=x86_64-softmmu,i386-softmmu
> > > --enable-debug --enable-tcg-interpreter --enable-debug-tcg
> > > --enable-debug-mutex
> > >
> > > I'm pretty sure the only relevant flags here are
> > > --without-default-features, --target-list including x86_64-softmmu and
> > > --enable-debug
> > >
> > > The only error I see is this: [...]/hw/i386/x86.c:422:(.text+0x1004):
> > > undefined reference to `kvm_hv_vpindex_settable' (the other
> > > kvm_enabled() was moved for the sake of consistency). My compiler is
> > > clang (16.0.6).
> > >
> > > I haven't looked into the heuristics or logic for how the compile-time
> > > short-circuit logic works, but I assumed only the first parameter is
> > > "guaranteed" to be checked for a literal false (guaranteed is in
> > > quotes because that's just how clang works, not because it's a feature
> > > of the language IIRC).
> > >
> > > This pattern relies on somes subtle behavior with the compiler, so my
> > > suggestion going forward would be to not rely on code optimizations
> > > removing undefined references based on short-circuit logic (instead
> > > have some configuration macro defined that disables all relevant
> > > code). I'm a new contributor, so I submitted the minimum to make it
> > > work on my machine.
> > >
> > > If you have any other questions, please let me know.
> > >
> > > Thanks!
> >
> > which compiler is this?
> >
> > --
> > MST
> >




Re: [PATCH] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Dan Hoffman
Clang 16.0.6

I can re-submit with the compiler and version if that helps.

On Sun, Nov 19, 2023 at 2:02 PM Michael S. Tsirkin  wrote:
>
> On Sun, Nov 19, 2023 at 11:03:54AM -0600, Dan Hoffman wrote:
> > On Sun, Nov 19, 2023 at 1:23 AM Michael S. Tsirkin  wrote:
> > >
> > > On Sat, Nov 18, 2023 at 10:25:31AM -0800, Daniel Hoffman wrote:
> > > > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > > > used to remmove references to undefined symbols at the compile stage.
> > > > Some build configurations with some compilers don't attempt to
> > > > simplify this logic down in some cases (the pattern appears to be
> > > > that the literal false must be the first term) and this was causing
> > > > some builds to emit references to undefined symbols.
> > > >
> > > > Signed-off-by: Daniel Hoffman 
> > >
> > > Could we add a bit more detail here? Will help make sure
> > > this does not break again in the future.
> >
> > The configuration script was ran as such:  ../configure
> > --without-default-features --target-list=x86_64-softmmu,i386-softmmu
> > --enable-debug --enable-tcg-interpreter --enable-debug-tcg
> > --enable-debug-mutex
> >
> > I'm pretty sure the only relevant flags here are
> > --without-default-features, --target-list including x86_64-softmmu and
> > --enable-debug
> >
> > The only error I see is this: [...]/hw/i386/x86.c:422:(.text+0x1004):
> > undefined reference to `kvm_hv_vpindex_settable' (the other
> > kvm_enabled() was moved for the sake of consistency). My compiler is
> > clang (16.0.6).
> >
> > I haven't looked into the heuristics or logic for how the compile-time
> > short-circuit logic works, but I assumed only the first parameter is
> > "guaranteed" to be checked for a literal false (guaranteed is in
> > quotes because that's just how clang works, not because it's a feature
> > of the language IIRC).
> >
> > This pattern relies on somes subtle behavior with the compiler, so my
> > suggestion going forward would be to not rely on code optimizations
> > removing undefined references based on short-circuit logic (instead
> > have some configuration macro defined that disables all relevant
> > code). I'm a new contributor, so I submitted the minimum to make it
> > work on my machine.
> >
> > If you have any other questions, please let me know.
> >
> > Thanks!
>
> which compiler is this?
>
> --
> MST
>



Re: [PATCH] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Michael S. Tsirkin
On Sun, Nov 19, 2023 at 11:03:54AM -0600, Dan Hoffman wrote:
> On Sun, Nov 19, 2023 at 1:23 AM Michael S. Tsirkin  wrote:
> >
> > On Sat, Nov 18, 2023 at 10:25:31AM -0800, Daniel Hoffman wrote:
> > > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > > used to remmove references to undefined symbols at the compile stage.
> > > Some build configurations with some compilers don't attempt to
> > > simplify this logic down in some cases (the pattern appears to be
> > > that the literal false must be the first term) and this was causing
> > > some builds to emit references to undefined symbols.
> > >
> > > Signed-off-by: Daniel Hoffman 
> >
> > Could we add a bit more detail here? Will help make sure
> > this does not break again in the future.
> 
> The configuration script was ran as such:  ../configure
> --without-default-features --target-list=x86_64-softmmu,i386-softmmu
> --enable-debug --enable-tcg-interpreter --enable-debug-tcg
> --enable-debug-mutex
> 
> I'm pretty sure the only relevant flags here are
> --without-default-features, --target-list including x86_64-softmmu and
> --enable-debug
> 
> The only error I see is this: [...]/hw/i386/x86.c:422:(.text+0x1004):
> undefined reference to `kvm_hv_vpindex_settable' (the other
> kvm_enabled() was moved for the sake of consistency). My compiler is
> clang (16.0.6).
> 
> I haven't looked into the heuristics or logic for how the compile-time
> short-circuit logic works, but I assumed only the first parameter is
> "guaranteed" to be checked for a literal false (guaranteed is in
> quotes because that's just how clang works, not because it's a feature
> of the language IIRC).
> 
> This pattern relies on somes subtle behavior with the compiler, so my
> suggestion going forward would be to not rely on code optimizations
> removing undefined references based on short-circuit logic (instead
> have some configuration macro defined that disables all relevant
> code). I'm a new contributor, so I submitted the minimum to make it
> work on my machine.
> 
> If you have any other questions, please let me know.
> 
> Thanks!

which compiler is this?

-- 
MST




[PATCH v2] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Daniel Hoffman
`kvm_enabled()` is compiled down to `0` and short-circuit logic is
used to remmove references to undefined symbols at the compile stage.
Some build configurations with some compilers don't attempt to
simplify this logic down in some cases (the pattern appears to be
that the literal false must be the first term) and this was causing
some builds to emit references to undefined symbols.

Signed-off-by: Daniel Hoffman 
---
 hw/i386/x86.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b3d054889bb..2b6291ad8d5 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
 /*
  * Can we support APIC ID 255 or higher?  With KVM, that requires
  * both in-kernel lapic and X2APIC userspace API.
+ *
+ * kvm_enabled() must go first to ensure that kvm_* references are
+ * not emitted for the linker to consume (kvm_enabled() is
+ * a literal `0` in configurations where kvm_* aren't defined)
  */
-if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
+if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
 (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
 error_report("current -smp configuration requires kernel "
  "irqchip and X2APIC API support.");
@@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
 }
 cpu->thread_id = topo_ids.smt_id;
 
-if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
-kvm_enabled() && !kvm_hv_vpindex_settable()) {
+/*
+* kvm_enabled() must go first to ensure that kvm_* references are
+* not emitted for the linker to consume (kvm_enabled() is
+* a literal `0` in configurations where kvm_* aren't defined)
+*/
+if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
+!kvm_hv_vpindex_settable()) {
 error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
 return;
 }
-- 
2.40.1




[PULL 0/1] target/xtensa fixes for v8.2

2023-11-19 Thread Max Filippov
The following changes since commit 9c673a41eefc50f1cb2fe3c083e7de842c7d276a:

  Update version for v8.2.0-rc0 release (2023-11-14 12:35:47 -0500)

are available in the Git repository at:

  https://github.com/OSLL/qemu-xtensa.git tags/20231119-xtensa-1

for you to fetch changes up to 1b173d06068c4a4e93fad88205399232925967a4:

  linux-user: xtensa: fix signal delivery in FDPIC (2023-11-19 10:56:26 -0800)


target/xtensa fixes for v8.2:

- fix signal delivery in FDPIC


Max Filippov (1):
  linux-user: xtensa: fix signal delivery in FDPIC

 linux-user/xtensa/signal.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)



Re: [PULL] target/xtensa fixes for v8.2

2023-11-19 Thread Max Filippov
On Sun, Nov 19, 2023 at 10:52 AM Max Filippov  wrote:
>
> The following changes since commit b411438aa4ecaf4bbde90e20283e5899fec10f58:
>
>   target/xtensa: Use tcg_gen_sextract_i32 (2023-10-21 19:17:28 -0700)
>
> are available in the Git repository at:
>
>   https://github.com/OSLL/qemu-xtensa.git tags/20231119-xtensa
>
> for you to fetch changes up to 79cc6538fba73b3c071d76d912486e96540df98f:
>
>   linux-user: xtensa: fix signal delivery in FDPIC (2023-11-19 10:38:07 -0800)
>
> 
> target/xtensa updates for v8.2:
>
> - fix signal delivery in FDPIC
>
> 
> Max Filippov (1):
>   linux-user: xtensa: fix signal delivery in FDPIC
>
>  linux-user/xtensa/signal.c | 28 ++--
>  1 file changed, 26 insertions(+), 2 deletions(-)

Sorry, wrong tag, please ignore.

-- 
Thanks.
-- Max



[PULL] target/xtensa fixes for v8.2

2023-11-19 Thread Max Filippov
The following changes since commit b411438aa4ecaf4bbde90e20283e5899fec10f58:

  target/xtensa: Use tcg_gen_sextract_i32 (2023-10-21 19:17:28 -0700)

are available in the Git repository at:

  https://github.com/OSLL/qemu-xtensa.git tags/20231119-xtensa

for you to fetch changes up to 79cc6538fba73b3c071d76d912486e96540df98f:

  linux-user: xtensa: fix signal delivery in FDPIC (2023-11-19 10:38:07 -0800)


target/xtensa updates for v8.2:

- fix signal delivery in FDPIC


Max Filippov (1):
  linux-user: xtensa: fix signal delivery in FDPIC

 linux-user/xtensa/signal.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)



[Stable-7.2.7 v2 00/62] Patch Round-up for stable 7.2.7, frozen on 2023-11-19

2023-11-19 Thread Michael Tokarev
The following patches are queued for QEMU stable v7.2.7:

  https://gitlab.com/qemu-project/qemu/-/commits/staging-7.2

Patch freeze is 2023-11-19 (frozen), and the release is planned for 2023-11-21:

  https://wiki.qemu.org/Planning/7.2

Please respond here or CC qemu-sta...@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.

v2: added more changes,
removed 6f189a08c1 ("ui/gtk-egl: Check EGLSurface before doing scanout"),
since this one caused a regression and the fix hasn't been found its way
to master still. Hopefully this one can be included (together with the fix)
in the next stable release.

The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.

Thanks!

/mjt

--
01* 7798f5c576d8 Nicholas Piggin:
   hw/ppc: Introduce functions for conversion between timebase and 
   nanoseconds
02* 47de6c4c2870 Nicholas Piggin:
   host-utils: Add muldiv64_round_up
03* eab0888418ab Nicholas Piggin:
   hw/ppc: Round up the decrementer interval when converting to ns
04* 8e0a5ac87800 Nicholas Piggin:
   hw/ppc: Avoid decrementer rounding errors
05* c8fbc6b9f2f3 Nicholas Piggin:
   target/ppc: Sign-extend large decrementer to 64-bits
06* 17dd1354c1d1 Nicholas Piggin:
   target/ppc: Decrementer fix BookE semantics
07* febb71d543a8 Nicholas Piggin:
   hw/ppc: Always store the decrementer value
08* 2529497cb6b2 Mikulas Patocka:
   linux-user/hppa: clear the PSW 'N' bit when delivering signals
09* 5b1270ef1477 Mikulas Patocka:
   linux-user/hppa: lock both words of function descriptor
10* 7b165fa16402 Li Zhijian:
   hw/cxl: Fix CFMW config memory leak
11* 903dbefc2b69 Peter Maydell:
   target/arm: Don't skip MTE checks for LDRT/STRT at EL0
12* 0e5903436de7 Nicholas Piggin:
   accel/tcg: mttcg remove false-negative halted assertion
13* 7cfcc79b0ab8 Thomas Huth:
   hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]
14* 0cb9c5880e6b Paolo Bonzini:
   ui/vnc: fix debug output for invalid audio message
15* 477b301000d6 Paolo Bonzini:
   ui/vnc: fix handling of VNC_FEATURE_XVP
16* 35ed01ba5448 Fabiano Rosas:
   optionrom: Remove build-id section
17* b86dc5cb0b41 Mark Cave-Ayland:
   esp: use correct type for esp_dma_enable() in sysbus_esp_gpio_demux()
18* 77668e4b9bca Mark Cave-Ayland:
   esp: restrict non-DMA transfer length to that of available data
19* be2b619a1734 Mark Cave-Ayland:
   scsi-disk: ensure that FORMAT UNIT commands are terminated
20* 3d304620ec6c Paolo Bonzini:
   target/i386: fix operand size of unary SSE operations
21* 9e65829699f9 Paolo Bonzini:
   tests/tcg/i386: correct mask for VPERM2F128/VPERM2I128
22* afa94dabc52b Ricky Zhou:
   target/i386: Fix and add some comments next to SSE/AVX instructions.
23* cab529b0dc15 Ricky Zhou:
   target/i386: Fix exception classes for SSE/AVX instructions.
24* 8bf171c2d126 Ricky Zhou:
   target/i386: Fix exception classes for MOVNTPS/MOVNTPD.
25* a48b26978a09 Paolo Bonzini:
   target/i386: generalize operand size "ph" for use in CVTPS2PD
26* abd41884c530 Paolo Bonzini:
   target/i386: fix memory operand size for CVTPS2PD
27* e0288a778473 Laszlo Ersek:
   hw/display/ramfb: plug slight guest-triggerable leak on mode setting
28* 4f7689f0817a Thomas Huth:
   chardev/char-pty: Avoid losing bytes when the other side just 
   (re-)connected
29* 33bc4fa78b06 Richard Henderson:
   linux-user/hppa: Fix struct target_sigcontext layout
30* 0114c4513095 Akihiko Odaki:
   amd_iommu: Fix APIC address check
31* 86dec715a733 Peter Xu:
   migration/qmp: Fix crash on setting tls-authz with null
32* 00e3b29d065f Volker Rümelin:
   hw/audio/es1370: reset current sample counter
33* cffa99549088 Alvin Chang:
   disas/riscv: Fix the typo of inverted order of pmpaddr13 and pmpaddr14
34* 85fc35afa93c Yuval Shaia:
   hw/pvrdma: Protect against buggy or malicious guest driver
35* caea03279e11 Fabiano Rosas:
   migration: Fix analyze-migration read operation signedness
36* 6fad9b4bb91d Mikulas Patocka:
   linux-user/mips: fix abort on integer overflow
37* 3b894b699c9a Mikulas Patocka:
   linux-user/sh4: Fix crashes on signal delivery
38* a1e6a5c46219 Helge Deller:
   lasips2: LASI PS/2 devices are not user-createable
39* ae5f70baf549 Lu Gao:
   hw/sd/sdhci: Block Size Register bits [14:12] is lost
40* 6f83dc67168d Glenn Miles:
   misc/led: LED state is set opposite of what is expected
41* 7a06a8fec9df Akihiko Odaki:
   tests/migration: Add -fno-stack-protector
42* 580731dcc87e Akihiko Odaki:
   tests/tcg: Add -fno-stack-protector
43* 8b097fd6b06e Andrey Drobyshev:
   qemu-img: rebase: stop when reaching EOF of old backing file
44* 827171c31805 Andrey Drobyshev:
   qemu-iotests: 024: add rebasing test case for overlay_size > backing_size
45* 9f0246539ae8 Daniel P. Berrangé:
   Revert "linux-user: add more compat ioctl definitions"
46* 6003159ce18f Daniel P. Berrangé:
   Revert "linux-user: fix compat with glibc >= 2.36 

[Stable-8.1.3 54/59] tracetool: avoid invalid escape in Python string

2023-11-19 Thread Michael Tokarev
From: Marc-André Lureau 

This is an error in Python 3.12; fix it by using a raw string literal.

Cc:  
Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Hajnoczi 
Message-ID: <20231108105649.60453-1-marcandre.lur...@redhat.com>
(cherry picked from commit 4d96307c5b4fac40c6ca25f38318b4b65d315de0)
Signed-off-by: Michael Tokarev 

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index b29594d75e..b887540a55 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -91,7 +91,7 @@ def out(*lines, **kwargs):
 def validate_type(name):
 bits = name.split(" ")
 for bit in bits:
-bit = re.sub("\*", "", bit)
+bit = re.sub(r"\*", "", bit)
 if bit == "":
 continue
 if bit == "const":
-- 
2.39.2




[Stable-7.2.7 62/62] target/tricore: Rename tricore_feature

2023-11-19 Thread Michael Tokarev
From: Bastian Koppelmann 

this name is used by capstone and will lead to a build failure of QEMU,
when capstone is enabled. So we rename it to tricore_has_feature(), to
match has_feature() in translate.c.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1774
Signed-off-by: Bastian Koppelmann 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Message-Id: <20230721060605.76636-1-kbast...@mail.uni-paderborn.de>
(cherry picked from commit f8cfdd2038c1823301e6df753242e465b1dc8539)
Signed-off-by: Michael Tokarev 
(Mjt: update context in target/tricore/cpu.c, target/tricore/op_helper.c, drop 
chunks in target/tricore/helper.c)

diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 2c54a2825f..0594d3843b 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -100,14 +100,14 @@ static void tricore_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 
 /* Some features automatically imply others */
-if (tricore_feature(env, TRICORE_FEATURE_161)) {
+if (tricore_has_feature(env, TRICORE_FEATURE_161)) {
 set_feature(env, TRICORE_FEATURE_16);
 }
 
-if (tricore_feature(env, TRICORE_FEATURE_16)) {
+if (tricore_has_feature(env, TRICORE_FEATURE_16)) {
 set_feature(env, TRICORE_FEATURE_131);
 }
-if (tricore_feature(env, TRICORE_FEATURE_131)) {
+if (tricore_has_feature(env, TRICORE_FEATURE_131)) {
 set_feature(env, TRICORE_FEATURE_13);
 }
 cpu_reset(cs);
diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index 3b9c533a7c..2e122b44a7 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -269,7 +269,7 @@ enum tricore_features {
 TRICORE_FEATURE_161,
 };
 
-static inline int tricore_feature(CPUTriCoreState *env, int feature)
+static inline int tricore_has_feature(CPUTriCoreState *env, int feature)
 {
 return (env->features & (1ULL << feature)) != 0;
 }
diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c
index 532ae6b74c..676529f754 100644
--- a/target/tricore/op_helper.c
+++ b/target/tricore/op_helper.c
@@ -2528,7 +2528,7 @@ void helper_ret(CPUTriCoreState *env)
 /* PCXI = new_PCXI; */
 env->PCXI = new_PCXI;
 
-if (tricore_feature(env, TRICORE_FEATURE_13)) {
+if (tricore_has_feature(env, TRICORE_FEATURE_13)) {
 /* PSW = new_PSW */
 psw_write(env, new_PSW);
 } else {
@@ -2639,7 +2639,7 @@ void helper_rfm(CPUTriCoreState *env)
 env->gpr_a[10] = cpu_ldl_data(env, env->DCX+8);
 env->gpr_a[11] = cpu_ldl_data(env, env->DCX+12);
 
-if (tricore_feature(env, TRICORE_FEATURE_131)) {
+if (tricore_has_feature(env, TRICORE_FEATURE_131)) {
 env->DBGTCR = 0;
 }
 }
-- 
2.39.2




[Stable-8.1.3 v2 00/59] Patch Round-up for stable 8.1.3, frozen on 2023-11-19

2023-11-19 Thread Michael Tokarev
The following patches are queued for QEMU stable v8.1.3:

  https://gitlab.com/qemu-project/qemu/-/commits/staging-8.1

Patch freeze is 2023-11-19 (frozen), and the release is planned for 2023-11-21:

  https://wiki.qemu.org/Planning/8.1

Please respond here or CC qemu-sta...@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.

v2: added more changes,
removed 6f189a08c1 ("ui/gtk-egl: Check EGLSurface before doing scanout"),
since this one caused a regression and the fix hasn't been found its way
to master still. Hopefully this one can be included (together with the fix)
in the next stable release.

The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.

Thanks!

/mjt

--
01* 85fc35afa93c Yuval Shaia:
   hw/pvrdma: Protect against buggy or malicious guest driver
02* caea03279e11 Fabiano Rosas:
   migration: Fix analyze-migration read operation signedness
03* d4f34485ca8a Juan Quintela:
   migration: Non multifd migration don't care about multifd flushes
04* acf873873ae3 John Snow:
   python/qmp: remove Server.wait_closed() call for Python 3.12
05* a5e3cb3b90a6 Paolo Bonzini:
   tests/docker: avoid invalid escape in Python string
06* e4b6532cc0a5 Paolo Bonzini:
   docs/sphinx: avoid invalid escape in Python string
07* e41c40d101fc Paolo Bonzini:
   target/hexagon: avoid invalid escape in Python string
08* 1b5f3f65cc71 Paolo Bonzini:
   tests/avocado: avoid invalid escape in Python string
09* 86a8989d4557 Paolo Bonzini:
   tests/vm: avoid invalid escape in Python string
10* e6d8e5e6e366 Paolo Bonzini:
   tracetool: avoid invalid escape in Python string
11* e6e66b032873 Richard Henderson:
   linux-user: Fixes for zero_bss
12* 6fad9b4bb91d Mikulas Patocka:
   linux-user/mips: fix abort on integer overflow
13* 3b894b699c9a Mikulas Patocka:
   linux-user/sh4: Fix crashes on signal delivery
14* a1e6a5c46219 Helge Deller:
   lasips2: LASI PS/2 devices are not user-createable
15* d01448c79d89 Michal Orzel:
   target/arm: Fix CNTPCT_EL0 trapping from EL0 when HCR_EL2.E2H is 0
16* ae5f70baf549 Lu Gao:
   hw/sd/sdhci: Block Size Register bits [14:12] is lost
17* 4ab9a7429bf7 Peter Maydell:
   hw/rdma/vmw/pvrdma_cmd: Use correct struct in query_port()
18* 930f1865cc65 Richard Henderson:
   target/sparc: Clear may_lookup for npc == DYNAMIC_PC
19* 307521d6e29e Peter Maydell:
   target/arm: Fix syndrome for FGT traps on ERET
20* 6f83dc67168d Glenn Miles:
   misc/led: LED state is set opposite of what is expected
21* fed824501501 Kevin Wolf:
   block: Fix locking in media change monitor commands
22* 580731dcc87e Akihiko Odaki:
   tests/tcg: Add -fno-stack-protector
23* 8b097fd6b06e Andrey Drobyshev:
   qemu-img: rebase: stop when reaching EOF of old backing file
24* 827171c31805 Andrey Drobyshev:
   qemu-iotests: 024: add rebasing test case for overlay_size > backing_size
25* b11293c212c2 Richard Henderson:
   target/arm: Fix SVE STR increment
26* 4c09abeae870 Peter Maydell:
   target/arm: Correctly propagate stage 1 BTI guarded bit in a two-stage walk
27* 721da0396cfa Cédric Le Goater:
   util/uuid: Add UUID_STR_LEN definition
28* f8d6f3b16c37 Cédric Le Goater:
   vfio/pci: Fix buffer overrun when writing the VF token
29* 4ef9d97b1a37 Cédric Le Goater:
   util/uuid: Remove UUID_FMT_LEN
30* e969f992c656 David Woodhouse:
   i386/xen: Don't advertise XENFEAT_supervisor_mode_kernel
31* e7dbb62ff19c David Woodhouse:
   i386/xen: fix per-vCPU upcall vector for Xen emulation
32* 18e83f28bf39 David Woodhouse:
   hw/xen: select kernel mode for per-vCPU event channel upcall vector
33* 3de75ed35241 David Woodhouse:
   hw/xen: don't clear map_track[] in xen_gnttab_reset()
34* 4a5780f52095 David Woodhouse:
   hw/xen: fix XenStore watch delivery to guest
35* debc995e883b David Woodhouse:
   hw/xen: take iothread mutex in xen_evtchn_reset_op()
36* a1c1082908dd David Woodhouse:
   hw/xen: use correct default protocol for xen-block on x86
37* 9c549ab6895a Marc-André Lureau:
   virtio-gpu: block migration of VMs with blob=true
38* cc8fb0c3ae3c Vladimir Sementsov-Ogievskiy:
   block/nvme: nvme_process_completion() fix bound for cid
39* 5722fc471296 Peter Maydell:
   target/arm: Fix A64 LDRA immediate decode
40* b2b109041ecd Jean-Louis Dupond:
   qcow2: keep reference on zeroize with discard-no-unref enabled
41* 10b9e0802a07 Sam Li:
   block/file-posix: fix update_zones_wp() caller
42* ad4feaca61d7 Naohiro Aota:
   file-posix: fix over-writing of returning zone_append offset
43* 08730ee0cc01 BALATON Zoltan:
   ati-vga: Implement fallback for pixman routines
44* 565f85a9c293 Marc-André Lureau:
   ui/gtk: force realization of drawing area
45* 47fd6ab1e334 Dongwon Kim:
   ui/gtk-egl: apply scale factor when calculating window's dimension
46* 04591b3ddd9a Philippe Mathieu-Daudé:
   target/mips: Fix MSA BZ/BNZ opcodes displacement
47* 18f86aecd6a1 Philippe Mathieu-Daudé:
   target/mips: Fix TX79 

[Stable-7.2.7 61/62] tracetool: avoid invalid escape in Python string

2023-11-19 Thread Michael Tokarev
From: Marc-André Lureau 

This is an error in Python 3.12; fix it by using a raw string literal.

Cc:  
Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Hajnoczi 
Message-ID: <20231108105649.60453-1-marcandre.lur...@redhat.com>
(cherry picked from commit 4d96307c5b4fac40c6ca25f38318b4b65d315de0)
Signed-off-by: Michael Tokarev 

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 5393c7fc5c..cd46e7597c 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -92,7 +92,7 @@ def out(*lines, **kwargs):
 def validate_type(name):
 bits = name.split(" ")
 for bit in bits:
-bit = re.sub("\*", "", bit)
+bit = re.sub(r"\*", "", bit)
 if bit == "":
 continue
 if bit == "const":
-- 
2.39.2




[Stable-8.1.3 59/59] hw/mips: LOONGSON3V depends on UNIMP device

2023-11-19 Thread Michael Tokarev
From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Cc: qemu-sta...@nongnu.org
Fixes: c76b409fef ("hw/mips: Add Loongson-3 machine support")
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20231107140615.3034763-1-marcandre.lur...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
(cherry picked from commit 52c773ce893f6321f20c80101aa4ea9489a6f701)
Signed-off-by: Michael Tokarev 

diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index da3a37e215..e012f947cb 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -45,6 +45,7 @@ config LOONGSON3V
 select PCI_EXPRESS_GENERIC_BRIDGE
 select MSI_NONBROKEN
 select FW_CFG_MIPS
+select UNIMP
 
 config MIPS_CPS
 bool
-- 
2.39.2




[Stable-8.1.3 55/59] target/riscv/kvm: improve 'init_multiext_cfg' error msg

2023-11-19 Thread Michael Tokarev
From: Daniel Henrique Barboza 

Our error message is returning the value of 'ret', which will be always
-1 in case of error, and will not be that useful:

qemu-system-riscv64: Unable to read ISA_EXT KVM register ssaia, error -1

Improve the error message by outputting 'errno' instead of 'ret'. Use
strerrorname_np() to output the error name instead of the error code.
This will give us what we need to know right away:

qemu-system-riscv64: Unable to read ISA_EXT KVM register ssaia, error code: 
ENOENT

Given that we're going to exit(1) in this condition instead of
attempting to recover, remove the 'kvm_riscv_destroy_scratch_vcpu()'
call.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Alistair Francis 
Reviewed-by: Andrew Jones 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20231003132148.797921-2-dbarb...@ventanamicro.com>
Signed-off-by: Alistair Francis 
(cherry picked from commit 082e9e4a58ba80ec056220a2f762a1c6b9a3a96c)
Signed-off-by: Michael Tokarev 

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index dbcf26f27d..c8e1bb9087 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -727,8 +727,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
KVMScratchCPU *kvmcpu)
 val = false;
 } else {
 error_report("Unable to read ISA_EXT KVM register %s, "
- "error %d", multi_ext_cfg->name, ret);
-kvm_riscv_destroy_scratch_vcpu(kvmcpu);
+ "error code: %s", multi_ext_cfg->name,
+ strerrorname_np(errno));
 exit(EXIT_FAILURE);
 }
 } else {
-- 
2.39.2




[Stable-8.1.3 58/59] target/arm: HVC at EL3 should go to EL3, not EL2

2023-11-19 Thread Michael Tokarev
From: Peter Maydell 

AArch64 permits code at EL3 to use the HVC instruction; however the
exception we take should go to EL3, not down to EL2 (see the pseudocode
AArch64.CallHypervisor()). Fix the target EL.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Edgar E. Iglesias 
Message-id: 20231109151917.1925107-1-peter.mayd...@linaro.org
(cherry picked from commit fc58891d0422607d172a3d6b3158798f2556aef1)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 58787ee8a7..7267f172d7 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2355,6 +2355,8 @@ static bool trans_SVC(DisasContext *s, arg_i *a)
 
 static bool trans_HVC(DisasContext *s, arg_i *a)
 {
+int target_el = s->current_el == 3 ? 3 : 2;
+
 if (s->current_el == 0) {
 unallocated_encoding(s);
 return true;
@@ -2367,7 +2369,7 @@ static bool trans_HVC(DisasContext *s, arg_i *a)
 gen_helper_pre_hvc(cpu_env);
 /* Architecture requires ss advance before we do the actual work */
 gen_ss_advance(s);
-gen_exception_insn_el(s, 4, EXCP_HVC, syn_aa64_hvc(a->imm), 2);
+gen_exception_insn_el(s, 4, EXCP_HVC, syn_aa64_hvc(a->imm), target_el);
 return true;
 }
 
-- 
2.39.2




[Stable-8.1.3 56/59] target/riscv/kvm: support KVM_GET_REG_LIST

2023-11-19 Thread Michael Tokarev
From: Daniel Henrique Barboza 

KVM for RISC-V started supporting KVM_GET_REG_LIST in Linux 6.6. It
consists of a KVM ioctl() that retrieves a list of all available regs
for get_one_reg/set_one_reg. Regs that aren't present in the list aren't
supported in the host.

This simplifies our lives when initing the KVM regs since we don't have
to always attempt a KVM_GET_ONE_REG for all regs QEMU knows. We'll only
attempt a get_one_reg() if we're sure the reg is supported, i.e. it was
retrieved by KVM_GET_REG_LIST. Any error in get_one_reg() will then
always considered fatal, instead of having to handle special error codes
that might indicate a non-fatal failure.

Start by moving the current kvm_riscv_init_multiext_cfg() logic into a
new kvm_riscv_read_multiext_legacy() helper. We'll prioritize using
KVM_GET_REG_LIST, so check if we have it available and, in case we
don't, use the legacy() logic.

Otherwise, retrieve the available reg list and use it to check if the
host supports our known KVM regs, doing the usual get_one_reg() for
the supported regs and setting cpu->cfg accordingly.

Signed-off-by: Daniel Henrique Barboza 
Acked-by: Alistair Francis 
Reviewed-by: Andrew Jones 
Message-ID: <20231003132148.797921-3-dbarb...@ventanamicro.com>
Signed-off-by: Alistair Francis 
(cherry picked from commit 608bdebb6075b757e5505f6bbc60c45a54a1390b)
Signed-off-by: Michael Tokarev 
(Mjt: trivial context tweak in target/riscv/kvm.c)

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index c8e1bb9087..3fb29299d9 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -706,7 +706,8 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, 
KVMScratchCPU *kvmcpu,
 }
 }
 
-static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
+static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
+   KVMScratchCPU *kvmcpu)
 {
 CPURISCVState *env = >env;
 uint64_t val;
@@ -747,6 +748,99 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
KVMScratchCPU *kvmcpu)
 }
 }
 
+static int uint64_cmp(const void *a, const void *b)
+{
+uint64_t val1 = *(const uint64_t *)a;
+uint64_t val2 = *(const uint64_t *)b;
+
+if (val1 < val2) {
+return -1;
+}
+
+if (val1 > val2) {
+return 1;
+}
+
+return 0;
+}
+
+static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
+{
+KVMCPUConfig *multi_ext_cfg;
+struct kvm_one_reg reg;
+struct kvm_reg_list rl_struct;
+struct kvm_reg_list *reglist;
+uint64_t val, reg_id, *reg_search;
+int i, ret;
+
+rl_struct.n = 0;
+ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, _struct);
+
+/*
+ * If KVM_GET_REG_LIST isn't supported we'll get errno 22
+ * (EINVAL). Use read_legacy() in this case.
+ */
+if (errno == EINVAL) {
+return kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
+} else if (errno != E2BIG) {
+/*
+ * E2BIG is an expected error message for the API since we
+ * don't know the number of registers. The right amount will
+ * be written in rl_struct.n.
+ *
+ * Error out if we get any other errno.
+ */
+error_report("Error when accessing get-reg-list, code: %s",
+ strerrorname_np(errno));
+exit(EXIT_FAILURE);
+}
+
+reglist = g_malloc(sizeof(struct kvm_reg_list) +
+   rl_struct.n * sizeof(uint64_t));
+reglist->n = rl_struct.n;
+ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, reglist);
+if (ret) {
+error_report("Error when reading KVM_GET_REG_LIST, code %s ",
+ strerrorname_np(errno));
+exit(EXIT_FAILURE);
+}
+
+/* sort reglist to use bsearch() */
+qsort(>reg, reglist->n, sizeof(uint64_t), uint64_cmp);
+
+for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
+multi_ext_cfg = _multi_ext_cfgs[i];
+reg_id = kvm_riscv_reg_id(>env, KVM_REG_RISCV_ISA_EXT,
+  multi_ext_cfg->kvm_reg_id);
+reg_search = bsearch(_id, reglist->reg, reglist->n,
+ sizeof(uint64_t), uint64_cmp);
+if (!reg_search) {
+continue;
+}
+
+reg.id = reg_id;
+reg.addr = (uint64_t)
+ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, );
+if (ret != 0) {
+error_report("Unable to read ISA_EXT KVM register %s, "
+ "error code: %s", multi_ext_cfg->name,
+ strerrorname_np(errno));
+exit(EXIT_FAILURE);
+}
+
+multi_ext_cfg->supported = true;
+kvm_cpu_cfg_set(cpu, multi_ext_cfg, val);
+}
+
+if (cpu->cfg.ext_icbom) {
+kvm_riscv_read_cbomz_blksize(cpu, kvmcpu, _cbom_blocksize);
+}
+
+if (cpu->cfg.ext_icboz) {
+kvm_riscv_read_cbomz_blksize(cpu, kvmcpu, _cboz_blocksize);
+}
+}
+
 void kvm_riscv_init_user_properties(Object 

[Stable-8.1.3 57/59] s390x/pci: only limit DMA aperture if vfio DMA limit reported

2023-11-19 Thread Michael Tokarev
From: Matthew Rosato 

If the host kernel lacks vfio DMA limit reporting, do not attempt
to shrink the guest DMA aperture.

Fixes: df202e3ff3 ("s390x/pci: shrink DMA aperture to be bound by vfio DMA 
limit")
Signed-off-by: Matthew Rosato 
Message-ID: <20231110175108.465851-3-mjros...@linux.ibm.com>
Signed-off-by: Thomas Huth 
(cherry picked from commit 8011b508cf0ddbdbda03820f4fa6cd484a6d9aed)
Signed-off-by: Michael Tokarev 

diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 59a2e03873..4f6f61f02e 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -132,7 +132,7 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
  * to the guest based upon the vfio DMA limit.
  */
 vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS;
-if (vfio_size < (cap->end_dma - cap->start_dma + 1)) {
+if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) {
 pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1;
 }
 }
-- 
2.39.2




Re: [PATCH] hw/i386: fix short-circuit logic with non-optimizing builds

2023-11-19 Thread Dan Hoffman
On Sun, Nov 19, 2023 at 1:23 AM Michael S. Tsirkin  wrote:
>
> On Sat, Nov 18, 2023 at 10:25:31AM -0800, Daniel Hoffman wrote:
> > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > used to remmove references to undefined symbols at the compile stage.
> > Some build configurations with some compilers don't attempt to
> > simplify this logic down in some cases (the pattern appears to be
> > that the literal false must be the first term) and this was causing
> > some builds to emit references to undefined symbols.
> >
> > Signed-off-by: Daniel Hoffman 
>
> Could we add a bit more detail here? Will help make sure
> this does not break again in the future.

The configuration script was ran as such:  ../configure
--without-default-features --target-list=x86_64-softmmu,i386-softmmu
--enable-debug --enable-tcg-interpreter --enable-debug-tcg
--enable-debug-mutex

I'm pretty sure the only relevant flags here are
--without-default-features, --target-list including x86_64-softmmu and
--enable-debug

The only error I see is this: [...]/hw/i386/x86.c:422:(.text+0x1004):
undefined reference to `kvm_hv_vpindex_settable' (the other
kvm_enabled() was moved for the sake of consistency). My compiler is
clang (16.0.6).

I haven't looked into the heuristics or logic for how the compile-time
short-circuit logic works, but I assumed only the first parameter is
"guaranteed" to be checked for a literal false (guaranteed is in
quotes because that's just how clang works, not because it's a feature
of the language IIRC).

This pattern relies on somes subtle behavior with the compiler, so my
suggestion going forward would be to not rely on code optimizations
removing undefined references based on short-circuit logic (instead
have some configuration macro defined that disables all relevant
code). I'm a new contributor, so I submitted the minimum to make it
work on my machine.

If you have any other questions, please let me know.

Thanks!



[PATCH v4 2/2] hw/acpi: Implement the SRAT GI affinity structure

2023-11-19 Thread ankita
From: Ankit Agrawal 

ACPI spec provides a scheme to associate "Generic Initiators" [1]
(e.g. heterogeneous processors and accelerators, GPUs, and I/O devices with
integrated compute or DMA engines GPUs) with Proximity Domains. This is
achieved using Generic Initiator Affinity Structure in SRAT. During bootup,
Linux kernel parse the ACPI SRAT to determine the PXM ids and create a NUMA
node for each unique PXM ID encountered. Qemu currently do not implement
these structures while building SRAT.

Add GI structures while building VM ACPI SRAT. The association between
devices and nodes are stored using acpi-generic-initiator object. Lookup
presence of all such objects and use them to build these structures.

The structure needs a PCI device handle [2] that consists of the device BDF.
The vfio-pci device corresponding to the acpi-generic-initiator object is
located to determine the BDF.

[1] ACPI Spec 6.3, Section 5.2.16.6
[2] ACPI Spec 6.3, Table 5.80

Signed-off-by: Ankit Agrawal 
---
 hw/acpi/acpi-generic-initiator.c | 100 +++
 hw/arm/virt-acpi-build.c |   3 +
 include/hw/acpi/acpi-generic-initiator.h |  26 ++
 3 files changed, 129 insertions(+)

diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c
index 5ea51cb81e..a9222438ec 100644
--- a/hw/acpi/acpi-generic-initiator.c
+++ b/hw/acpi/acpi-generic-initiator.c
@@ -16,6 +16,7 @@
 #include "hw/pci/pci_device.h"
 #include "sysemu/numa.h"
 #include "hw/acpi/acpi-generic-initiator.h"
+#include "qemu/error-report.h"
 
 OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, 
acpi_generic_initiator,
ACPI_GENERIC_INITIATOR, OBJECT,
@@ -82,3 +83,102 @@ static void acpi_generic_initiator_class_init(ObjectClass 
*oc, void *data)
 acpi_generic_initiator_set_host_nodes,
 NULL, NULL);
 }
+
+static int acpi_generic_initiator_list(Object *obj, void *opaque)
+{
+GSList **list = opaque;
+
+if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
+*list = g_slist_append(*list, ACPI_GENERIC_INITIATOR(obj));
+}
+
+object_child_foreach(obj, acpi_generic_initiator_list, opaque);
+return 0;
+}
+
+/*
+ * Identify Generic Initiator objects and link them into the list which is
+ * returned to the caller.
+ *
+ * Note: it is the caller's responsibility to free the list to avoid
+ * memory leak.
+ */
+static GSList *acpi_generic_initiator_get_list(void)
+{
+GSList *list = NULL;
+
+object_child_foreach(object_get_root(), acpi_generic_initiator_list, 
);
+return list;
+}
+
+/*
+ * ACPI 6.3:
+ * Table 5-78 Generic Initiator Affinity Structure
+ */
+static
+void build_srat_generic_pci_initiator_affinity(GArray *table_data, int node,
+   PCIDeviceHandle *handle)
+{
+uint8_t index;
+
+build_append_int_noprefix(table_data, 5, 1);  /* Type */
+build_append_int_noprefix(table_data, 32, 1); /* Length */
+build_append_int_noprefix(table_data, 0, 1);  /* Reserved */
+build_append_int_noprefix(table_data, 1, 1);  /* Device Handle Type: PCI */
+build_append_int_noprefix(table_data, node, 4);  /* Proximity Domain */
+
+/* Device Handle - PCI */
+build_append_int_noprefix(table_data, handle->segment, 2);
+build_append_int_noprefix(table_data, handle->bdf, 2);
+for (index = 0; index < 12; index++) {
+build_append_int_noprefix(table_data, 0, 1);
+}
+
+build_append_int_noprefix(table_data, GEN_AFFINITY_ENABLED, 4); /* Flags */
+build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+}
+
+void build_srat_generic_pci_initiator(GArray *table_data)
+{
+GSList *gi_list, *list = acpi_generic_initiator_get_list();
+AcpiGenericInitiator *gi;
+
+for (gi_list = list; gi_list; gi_list = gi_list->next) {
+Object *o;
+uint16List *l;
+PCIDevice *pci_dev;
+bool node_specified = false;
+
+gi = gi_list->data;
+
+/* User fails to provide a device. */
+g_assert(gi->device);
+
+o = object_resolve_path_type(gi->device, TYPE_PCI_DEVICE, NULL);
+if (!o) {
+error_printf("Specified device must be a PCI device.\n");
+g_assert(o);
+}
+pci_dev = PCI_DEVICE(o);
+
+for (l = gi->nodelist; l; l = l->next) {
+PCIDeviceHandle dev_handle;
+dev_handle.segment = 0;
+dev_handle.bdf = PCI_BUILD_BDF(pci_bus_num(pci_get_bus(pci_dev)),
+   pci_dev->devfn);
+build_srat_generic_pci_initiator_affinity(table_data,
+  l->value, _handle);
+node_specified = true;
+}
+
+if (!node_specified) {
+error_report("Generic Initiator device 0:%x:%x.%x has no 
associated"
+ " NUMA node.", pci_bus_num(pci_get_bus(pci_dev)),
+ 

[PATCH v4 1/2] qom: new object to associate device to numa node

2023-11-19 Thread ankita
From: Ankit Agrawal 

NVIDIA GPU's support MIG (Mult-Instance GPUs) feature [1], which allows
partitioning of the GPU device resources (including device memory) into
several (upto 8) isolated instances. Each of the partitioned memory needs
a dedicated NUMA node to operate. The partitions are not fixed and they
can be created/deleted at runtime.

Unfortunately Linux OS does not provide a means to dynamically create/destroy
NUMA nodes and such feature implementation is not expected to be trivial. The
nodes that OS discovers at the boot time while parsing SRAT remains fixed. So
we utilize the Generic Initiator Affinity structures that allows association
between nodes and devices. Multiple GI structures per BDF is possible,
allowing creation of multiple nodes by exposing unique PXM in each of these
structures.

Introduce a new acpi-generic-initiator object to allow host admin provide the
device and the corresponding NUMA nodes. Qemu maintain this association and
use this object to build the requisite GI Affinity Structure.

An admin can provide the range of nodes through a uint16 array host-nodes
and link it to a device by providing its id. Currently, only PCI device is
supported and an error is returned for acpi device. The following sample
creates 8 nodes and link them to the PCI device dev0:

-numa node,nodeid=2 \
-numa node,nodeid=3 \
-numa node,nodeid=4 \
-numa node,nodeid=5 \
-numa node,nodeid=6 \
-numa node,nodeid=7 \
-numa node,nodeid=8 \
-numa node,nodeid=9 \
-device 
vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \
-object acpi-generic-initiator,id=gi0,pci-dev=dev0,host-nodes=2-9 \

[1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu

Signed-off-by: Ankit Agrawal 
---
 hw/acpi/acpi-generic-initiator.c | 84 
 hw/acpi/meson.build  |  1 +
 include/hw/acpi/acpi-generic-initiator.h | 30 +
 qapi/qom.json| 18 +
 4 files changed, 133 insertions(+)
 create mode 100644 hw/acpi/acpi-generic-initiator.c
 create mode 100644 include/hw/acpi/acpi-generic-initiator.h

diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c
new file mode 100644
index 00..5ea51cb81e
--- /dev/null
+++ b/hw/acpi/acpi-generic-initiator.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qapi/qapi-builtin-visit.h"
+#include "qapi/visitor.h"
+#include "qom/object_interfaces.h"
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+#include "hw/vfio/vfio-common.h"
+#include "hw/vfio/pci.h"
+#include "hw/pci/pci_device.h"
+#include "sysemu/numa.h"
+#include "hw/acpi/acpi-generic-initiator.h"
+
+OBJECT_DEFINE_TYPE_WITH_INTERFACES(AcpiGenericInitiator, 
acpi_generic_initiator,
+   ACPI_GENERIC_INITIATOR, OBJECT,
+   { TYPE_USER_CREATABLE },
+   { NULL })
+
+OBJECT_DECLARE_SIMPLE_TYPE(AcpiGenericInitiator, ACPI_GENERIC_INITIATOR)
+
+static void acpi_generic_initiator_init(Object *obj)
+{
+AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+gi->device = NULL;
+gi->nodelist = NULL;
+}
+
+static void acpi_generic_initiator_finalize(Object *obj)
+{
+AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+g_free(gi->device);
+qapi_free_uint16List(gi->nodelist);
+}
+
+static void acpi_generic_initiator_set_pci_device(Object *obj, const char *val,
+  Error **errp)
+{
+AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+
+gi->device = g_strdup(val);
+}
+
+static void acpi_generic_initiator_set_acpi_device(Object *obj, const char 
*val,
+   Error **errp)
+{
+error_setg(errp, "Generic Initiator ACPI device not supported");
+}
+
+static void
+acpi_generic_initiator_set_host_nodes(Object *obj, Visitor *v, const char 
*name,
+  void *opaque, Error **errp)
+{
+AcpiGenericInitiator *gi = ACPI_GENERIC_INITIATOR(obj);
+uint16List *l;
+
+visit_type_uint16List(v, name, &(gi->nodelist), errp);
+
+for (l = gi->nodelist; l; l = l->next) {
+if (l->value >= MAX_NODES) {
+error_setg(errp, "Invalid host-nodes value: %d", l->value);
+qapi_free_uint16List(gi->nodelist);
+return;
+}
+}
+}
+
+static void acpi_generic_initiator_class_init(ObjectClass *oc, void *data)
+{
+object_class_property_add_str(oc, ACPI_GENERIC_INITIATOR_PCI_DEVICE_PROP,
+  NULL, acpi_generic_initiator_set_pci_device);
+object_class_property_add_str(oc, ACPI_GENERIC_INITIATOR_ACPI_DEVICE_PROP,
+  NULL, 
acpi_generic_initiator_set_acpi_device);
+object_class_property_add(oc, 

[PATCH v4 0/2] acpi: report numa nodes for device memory using GI

2023-11-19 Thread ankita
From: Ankit Agrawal 

There are upcoming devices which allow CPU to cache coherently access
their memory. It is sensible to expose such memory as NUMA nodes separate
from the sysmem node to the OS. The ACPI spec provides a scheme in SRAT
called Generic Initiator Affinity Structure [1] to allow an association
between a Proximity Domain (PXM) and a Generic Initiator (GI) (e.g.
heterogeneous processors and accelerators, GPUs, and I/O devices with
integrated compute or DMA engines).

While a single node per device may cover several use cases, it is however
insufficient for a full utilization of the NVIDIA GPUs MIG
(Mult-Instance GPUs) [2] feature. The feature allows partitioning of the
GPU device resources (including device memory) into several (upto 8)
isolated instances. Each of the partitioned memory requires a dedicated NUMA
node to operate. The partitions are not fixed and they can be created/deleted
at runtime.

Linux OS does not provide a means to dynamically create/destroy NUMA nodes
and such feature implementation is expected to be non-trivial. The nodes
that OS discovers at the boot time while parsing SRAT remains fixed. So we
utilize the GI Affinity structures that allows association between nodes
and devices. Multiple GI structures per device/BDF is possible, allowing
creation of multiple nodes in the VM by exposing unique PXM in each of these
structures.

Implement the mechanism to build the GI affinity structures as Qemu currently
does not. Introduce a new acpi-generic-initiator object that allows an
association of a set of nodes with a device. During SRAT creation, all such
objected are identified and used to add the GI Affinity Structures. Currently,
only PCI device is supported and an error is returned for acpi device.

The admin will create a range of 8 nodes and associate that with the device
using the acpi-generic-initiator object. While a configuration of less than
8 nodes per device is allowed, such configuration will prevent utilization of
the feature to the fullest. This setting is applicable to all the Grace+Hopper
systems. The following is an example of the Qemu command line arguments to
create 8 nodes and link them to the device 'dev0':

-numa node,nodeid=2 \
-numa node,nodeid=3 \
-numa node,nodeid=4 \
-numa node,nodeid=5 \
-numa node,nodeid=6 \
-numa node,nodeid=7 \
-numa node,nodeid=8 \
-numa node,nodeid=9 \
-device 
vfio-pci-nohotplug,host=0009:01:00.0,bus=pcie.0,addr=04.0,rombar=0,id=dev0 \
-object acpi-generic-initiator,id=gi0,pci-dev=dev0,host-nodes=2-9 \

The performance benefits can be realized by providing the NUMA node distances
appropriately (through libvirt tags or Qemu params). The admin can get the
distance among nodes in hardware using `numactl -H`.

This series goes along with the vfio-pci variant driver [3] under review.

Applied over v8.2.0.

[1] ACPI Spec 6.3, Section 5.2.16.6
[2] https://www.nvidia.com/en-in/technologies/multi-instance-gpu
[3] https://lore.kernel.org/all/20231114081611.30550-1-ank...@nvidia.com/

Link for v3:
https://lore.kernel.org/all/20231107190039.19434-1-ank...@nvidia.com/

v3 -> v4
- changed the ':' delimited way to a uint16 array to communicate the
nodes associated with the device.
- added asserts to handle invalid inputs.
- addressed other miscellaneous v3 comments.

v2 -> v3
- changed param to accept a ':' delimited list of numa nodes, instead
of a range.
- Removed nvidia-acpi-generic-initiator object.
- Addressed miscellaneous comments in v2.

v1 -> v2
- Removed dependency on sysfs to communicate the feature with variant module.
- Use GI Affinity SRAT structure instead of Memory Affinity.
- No DSDT entries needed to communicate the PXM for the device. SRAT GI
structure is used instead.
- New objects introduced to establish link between device and nodes.

Ankit Agrawal (2):
  qom: new object to associate device to numa node
  hw/acpi: Implement the SRAT GI affinity structure

 hw/acpi/acpi-generic-initiator.c | 185 +++
 hw/acpi/meson.build  |   1 +
 hw/arm/virt-acpi-build.c |   3 +
 include/hw/acpi/acpi-generic-initiator.h |  56 +++
 qapi/qom.json|  18 +++
 5 files changed, 263 insertions(+)
 create mode 100644 hw/acpi/acpi-generic-initiator.c
 create mode 100644 include/hw/acpi/acpi-generic-initiator.h

-- 
2.34.1




[PATCH] configure: Make only once with pseudo-"in source tree" builds

2023-11-19 Thread Akihiko Odaki
Pseudo-"in source tree" build used to run make in the build directory
as many times as goals. Worse, although .NOTPARALLEL is specified,
it does not work for patterns, and run make in parallel, which can break
things.

Add a new rule "build", and let it call make. The pattern rule only
needs to specify "build" as its prerequisite and have a no-op recipe so
that it does more than canceling built-in implicit rules.

Fixes: dedad02720 ("configure: add support for pseudo-"in source tree" builds")
Signed-off-by: Akihiko Odaki 
---
 configure | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index abcb199aa8..d73a9d811b 100755
--- a/configure
+++ b/configure
@@ -41,12 +41,7 @@ then
 # This file is auto-generated by configure to support in-source tree
 # 'make' command invocation
 
-ifeq ($(MAKECMDGOALS),)
-recurse: all
-endif
-
-.NOTPARALLEL: %
-%: force
+build:
@echo 'changing dir to build for $(MAKE) "$(MAKECMDGOALS)"...'
@$(MAKE) -C build -f Makefile $(MAKECMDGOALS)
@if test "$(MAKECMDGOALS)" = "distclean" && \
@@ -54,8 +49,9 @@ endif
then \
rm -rf build GNUmakefile ; \
fi
-force: ;
-.PHONY: force
+%: build
+   @
+.PHONY: build
 GNUmakefile: ;
 
 EOF
-- 
2.42.1




[PATCH] audio: Free consumed default audio devices

2023-11-19 Thread Akihiko Odaki
Failed default audio devices were removed from the list but not freed,
and that made LeakSanitizer sad. Free default audio devices as they are
consumed.

Signed-off-by: Akihiko Odaki 
---
 audio/audio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/audio/audio.c b/audio/audio.c
index f91e05b72c..becf6cdf46 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1758,12 +1758,14 @@ static AudioState *audio_init(Audiodev *dev, Error 
**errp)
 goto out;
 }
 s->dev = dev = e->dev;
+QSIMPLEQ_REMOVE_HEAD(_audiodevs, next);
+g_free(e);
 drvname = AudiodevDriver_str(dev->driver);
 driver = audio_driver_lookup(drvname);
 if (!audio_driver_init(s, driver, dev, NULL)) {
 break;
 }
-QSIMPLEQ_REMOVE_HEAD(_audiodevs, next);
+qapi_free_Audiodev(dev);
 }
 }
 
-- 
2.42.1