Re: [PATCH v6 12/15] hw/pci: Use -1 as a default value for rombar
Akihiko Odaki writes:
> Currently there is no way to distinguish the case that rombar is
> explicitly specified as 1 and the case that rombar is not specified.
>
> Set rombar -1 by default to distinguish these cases just as it is done
> for addr and romsize. It was confirmed that changing the default value
> to -1 will not change the behavior by looking at occurences of rom_bar.
>
> $ git grep -w rom_bar
> hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(&d->rom_bar);
> hw/display/qxl.c:431:qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size);
> hw/display/qxl.c:1048:QXLRom *rom =
> memory_region_get_ram_ptr(&qxl->rom_bar);
> hw/display/qxl.c:2131:memory_region_init_rom(&qxl->rom_bar, OBJECT(qxl),
> "qxl.vrom",
> hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar);
> hw/display/qxl.h:101:MemoryRegion rom_bar;
> hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
> hw/pci/pci.c:2329:if (!pdev->rom_bar) {
> hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
> hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) {
> include/hw/pci/pci_device.h:150:uint32_t rom_bar;
>
> rom_bar refers to a different variable in qxl. It is only tested if
> the value is 0 or not in the other places.
Makes me wonder why it's uint32_t. Not this patch's problem.
> Signed-off-by: Akihiko Odaki
> ---
> hw/pci/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 54b375da2d26..909c5b3ee4ee 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -71,7 +71,7 @@ static Property pci_props[] = {
> DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
> -DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
> +DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, -1),
> DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
> QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
> DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,
Re: [PATCH] hw/virtio: Add support for VDPA network simulation devices
On Wed, Feb 21, 2024 at 03:38:02PM +0800, Hao Chen wrote: > This patch adds support for VDPA network simulation devices. > The device is developed based on virtio-net and tap backend, > and supports hardware live migration function. > > For more details, please refer to "docs/system/devices/vdpa-net.rst" > > Signed-off-by: Hao Chen I am not really inclined to merge this, virtio TC is now working on LM support that physical device can support, feel free to join that effort. Thanks! > --- > MAINTAINERS | 5 + > docs/system/device-emulation.rst| 1 + > docs/system/devices/vdpa-net.rst| 121 + > hw/net/virtio-net.c | 16 ++ > hw/virtio/virtio-pci.c | 189 +++- > hw/virtio/virtio.c | 39 > include/hw/virtio/virtio-pci.h | 5 + > include/hw/virtio/virtio.h | 19 ++ > include/standard-headers/linux/virtio_pci.h | 7 + > 9 files changed, 399 insertions(+), 3 deletions(-) > create mode 100644 docs/system/devices/vdpa-net.rst > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d61fb9319..a1bde36bb0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2359,6 +2359,11 @@ F: hw/virtio/vhost-user-scmi* > F: include/hw/virtio/vhost-user-scmi.h > F: tests/qtest/libqos/virtio-scmi.* > > +vdpa-net > +M: Hao Chen > +S: Maintained > +F: docs/system/devices/vdpa-net.rst > + > virtio-crypto > M: Gonglei > S: Supported > diff --git a/docs/system/device-emulation.rst > b/docs/system/device-emulation.rst > index f19777411c..e4a27f53c8 100644 > --- a/docs/system/device-emulation.rst > +++ b/docs/system/device-emulation.rst > @@ -99,3 +99,4 @@ Emulated Devices > devices/canokey.rst > devices/usb-u2f.rst > devices/igb.rst > + devices/vdpa-net.rst > diff --git a/docs/system/devices/vdpa-net.rst > b/docs/system/devices/vdpa-net.rst > new file mode 100644 > index 00..323d8c926a > --- /dev/null > +++ b/docs/system/devices/vdpa-net.rst > @@ -0,0 +1,121 @@ > +vdpa net > + > + > +This document explains the setup and usage of the vdpa network device. > +The vdpa network device is a paravirtualized vdpa emulate device. > + > +Description > +--- > + > +VDPA net devices support dirty page bitmap mark and vring state saving and > recovery. > + > +Users can use this VDPA device for live migration simulation testing in a > nested virtualization environment. > + > +Registers layout > + > + > +The vdpa device add live migrate registers layout as follow:: > + > + Offset Register Name Bitwidth Associated vq > + 0x0 LM_LOGGING_CTRL 4bits > + 0x10 LM_BASE_ADDR_LOW 32bits > + 0x14 LM_BASE_ADDR_HIGH32bits > + 0x18 LM_END_ADDR_LOW 32bits > + 0x1c LM_END_ADDR_HIGH 32bits > + 0x20 LM_RING_STATE_OFFSET 32bits vq0 > + 0x24 LM_RING_STATE_OFFSET 32bits vq1 > + 0x28 LM_RING_STATE_OFFSET 32bits vq2 > + .. > + 0x20+1023*4 LM_RING_STATE_OFFSET 32bits vq1023 > + > +These registers are extended at the end of the notify bar space. > + > +Architecture diagram > + > +:: > + > + || > + | guest-L1-user-space| > + || > + | || > + | | [virtio-net driver] | > + | | ^ guest-L2-src(iommu=on) | > + | |--|-| > + | | | qemu-L2-src(viommu)| > + | [dpdk-vdpa]<->[vhost socket]<-+->[vhost-user backend(iommu=on)]| > + -- > + -- > + | ^ guest-L1-kernel-space | > + | || > + |[VFIO] | > + | ^| > + | | guest-L1-src(iommu=on) | > + |- > + |- > + | [vdpa net device(iommu=on)][manager nic device]| > + | ||| > + | ||
Re: [PATCH] tests: skip dbus-display tests that need a console
On 21/02/2024 08.37, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau When compiling with "configure --without-default-devices", the dbus-display-test fails since it implicitly assumes that the machine comes with a default console. There doesn't seem to be an easy way to figure this during build time, so skip the tests requiring the Console interface at runtime. Reported-by: Thomas Huth Signed-off-by: Marc-André Lureau --- tests/qtest/dbus-display-test.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) Thanks, that works fine! Tested-by: Thomas Huth
Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled
Akihiko Odaki writes:
> vfio determines if rombar is explicitly enabled by inspecting QDict.
> Inspecting QDict is not nice because QDict is untyped and depends on the
> details on the external interface. Add an infrastructure to determine if
> rombar is explicitly enabled to hw/pci.
>
> Signed-off-by: Akihiko Odaki
> ---
> include/hw/pci/pci_device.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ca151325085d..c4fdc96ef50d 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
> return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
> }
>
> +static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
> +{
> +return dev->rom_bar && dev->rom_bar != -1;
Compares signed with unsigned: rom_bar is uint32_t, -1 is int.
If int is at most 32 bits, the comparison converts -1 to
(uint32_t)0x.
If int is wider, it converts dev->rom_bar to int without changing its
value. In particular, it converts the default value 0x (written
as -1 in the previous patch) to (int)0x. Oops.
Best not to mix signed and unsigned in comparisons. Easy enough to
avoid here.
Still, we don't actually test "rom_bar has not been set". We test
"rom_bar has the default value". Nothing stops a user from passing
rombar=0x to -device. See my review of the next patch.
> +}
> +
> static inline void pci_set_power(PCIDevice *pci_dev, bool state)
> {
> /*
Re: [PATCH v6 14/15] vfio: Avoid inspecting option QDict for rombar
Akihiko Odaki writes:
> Use pci_rom_bar_explicitly_enabled() to determine if rombar is explicitly
> enabled.
>
> Signed-off-by: Akihiko Odaki
> ---
> hw/vfio/pci.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 4fa387f0430d..647f15b2a060 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1012,7 +1012,6 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
> {
> uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK);
> off_t offset = vdev->config_offset + PCI_ROM_ADDRESS;
> -DeviceState *dev = DEVICE(vdev);
> char *name;
> int fd = vdev->vbasedev.fd;
>
> @@ -1046,7 +1045,7 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
> }
>
> if (vfio_opt_rom_in_denylist(vdev)) {
> -if (dev->opts && qdict_haskey(dev->opts, "rombar")) {
> +if (pci_rom_bar_explicitly_enabled(&vdev->pdev)) {
> warn_report("Device at %s is known to cause system instability"
> " issues during option rom execution",
> vdev->vbasedev.name);
Consider -device ...,rombar=0x.
Before the patch, the condition is true.
Afterwards, it's false.
Do we care?
Re: [PATCH v6 15/15] hw/qdev: Remove opts member
Akihiko Odaki writes:
> It is no longer used.
>
> Signed-off-by: Akihiko Odaki
> Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/hw/qdev-core.h | 4
> hw/core/qdev.c | 1 -
> system/qdev-monitor.c | 12 +++-
> 3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 9228e96c87e9..5954404dcbfe 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -237,10 +237,6 @@ struct DeviceState {
> * @pending_deleted_expires_ms: optional timeout for deletion events
> */
> int64_t pending_deleted_expires_ms;
> -/**
> - * @opts: QDict of options for the device
> - */
> -QDict *opts;
> /**
> * @hotplugged: was device added after PHASE_MACHINE_READY?
> */
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index c68d0f7c512f..7349c9a86be8 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -706,7 +706,6 @@ static void device_finalize(Object *obj)
> dev->canonical_path = NULL;
> }
>
> -qobject_unref(dev->opts);
> g_free(dev->id);
> }
>
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index a13db763e5dd..71c00f62ee38 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -625,6 +625,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
> char *id;
> DeviceState *dev = NULL;
> BusState *bus = NULL;
> +QDict *properties;
>
> driver = qdict_get_try_str(opts, "driver");
> if (!driver) {
> @@ -705,13 +706,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> }
>
> /* set properties */
> -dev->opts = qdict_clone_shallow(opts);
> -qdict_del(dev->opts, "driver");
> -qdict_del(dev->opts, "bus");
> -qdict_del(dev->opts, "id");
> +properties = qdict_clone_shallow(opts);
> +qdict_del(properties, "driver");
> +qdict_del(properties, "bus");
> +qdict_del(properties, "id");
>
> -object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json,
> +object_set_properties_from_keyval(&dev->parent_obj, properties,
> from_json,
>errp);
> +qobject_unref(properties);
> if (*errp) {
> goto err_del_dev;
> }
Reviewed-by: Markus Armbruster
Depends on the previous few patches, of course.
[Stable-8.2.2 v0 00/60] Patch Round-up for stable 8.2.2, freeze on 2024-03-02
The following patches are queued for QEMU stable v8.2.2:
https://gitlab.com/qemu-project/qemu/-/commits/staging-8.2
Patch freeze is 2024-03-02, and the release is planned for 2024-03-04:
https://wiki.qemu.org/Planning/8.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.
The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.
Thanks!
/mjt
--
01 918f620d30a9 Markus Armbruster:
migration: Plug memory leak on HMP migrate error path
02 27eb8499edb2 Fabiano Rosas:
migration: Fix use-after-free of migration state object
03 d2b668fca565 Cédric Le Goater:
vfio/pci: Clear MSI-X IRQ index always
04 57fd4b4e1075 Het Gala:
Make 'uri' optional for migrate QAPI
05 db101376af52 Yihuan Pan:
qemu-docs: Update options for graphical frontends
06 615eaeab3d31 Richard W.M. Jones:
block/blkio: Make s->mem_region_alignment be 64 bits
07 f670be1aad33 Jan Klötzke:
target/arm: fix exception syndrome for AArch32 bkpt insn
08 d2019a9d0c34 Peter Maydell:
system/vl.c: Fix handling of '-serial none -serial something'
09 747bfaf3a9d2 Peter Maydell:
qemu-options.hx: Improve -serial option documentation
10 185e3fdf8d10 Peter Maydell:
target/arm: Reinstate "vfp" property on AArch32 CPUs
11 8a7315202033 Guenter Roeck:
pci-host: designware: Limit value range of iATU viewport register
12 45bf0e7aa648 Richard Henderson:
tcg/loongarch64: Set vector registers call clobbered
13 6400be014f80 Richard Henderson:
linux-user/aarch64: Add padding before __kernel_rt_sigreturn
14 8b09b7fe4708 Sven Schnelle:
hw/scsi/lsi53c895a: add missing decrement of reentrancy counter
15 c645bac4e06b Daniel P. Berrangé:
iotests: fix leak of tmpdir in dry-run mode
16 7d2faf0ce2cc Daniel P. Berrangé:
iotests: give tempdir an identifying name
17 c42c3833e0cf Hanna Czenczek:
virtio-scsi: Attach event vq notifier with no_poll
18 5bdbaebcce18 Hanna Czenczek:
virtio: Re-enable notifications after drain
19 bfa36802d170 Stefan Hajnoczi:
virtio-blk: avoid using ioeventfd state in irqfd conditional
20 3205bebd4fc6 Avihai Horon:
migration: Fix logic of channels and transport compatibility check
21 1a49762c07d0 Daniel Henrique Barboza:
hw/riscv/virt-acpi-build.c: fix leak in build_rhct()
22 7485508341f4 Fabiano Rosas:
tests/docker: Add sqlite3 module to openSUSE Leap container
23 15cc10336249 Paolo Bonzini:
configure: run plugin TCG tests again
24 cd8a35b913c2 Akihiko Odaki:
hw/smbios: Fix OEM strings table option validation
25 196578c9d051 Akihiko Odaki:
hw/smbios: Fix port connector option validation
26 9b60a3ed5569 Sven Schnelle:
hw/net/tulip: add chip status register values
27 c0e688153f29 Richard Henderson:
tcg: Increase width of temp_subindex
28 e41f1825b437 Richard Henderson:
tcg/arm: Fix goto_tb for large translation blocks
29 aa05bd9ef407 Andrey Ignatov:
vhost-user.rst: Fix vring address description
30 c62926f730d0 Ira Weiny:
cxl/cdat: Handle cdat table build errors
31 64fdad5e6758 Ira Weiny:
cxl/cdat: Fix header sum value in CDAT checksum
32 f7509f462c78 Hyeonggon Yoo:
hw/cxl/device: read from register values in mdev_reg_read()
33 729d45a6af06 Li Zhijian:
hw/cxl: Pass CXLComponentState to cache_mem_ops
34 574b64aa6754 Dmitry Osipenko:
virtio-gpu: Correct virgl_renderer_resource_get_info() error check
35 9a457383ce9d Zhenzhong Duan:
virtio_iommu: Clear IOMMUPciBus pointer cache when system reset
36 8a6b3f4dc95a Zhenzhong Duan:
smmu: Clear SMMUPciBus pointer cache when system reset
37 14ec4ff3e429 Jonathan Cameron:
tests/acpi: Allow update of DSDT.cxl
38 d9ae5802f656 Jonathan Cameron:
hw/i386: Fix _STA return value for ACPI0017
39 b24a981b9f1c Jonathan Cameron:
tests/acpi: Update DSDT.cxl to reflect change _STA return value.
40 681dfc0d5529 Richard Henderson:
linux-user/aarch64: Choose SYNC as the preferred MTE mode
41 64c6e7444dff Richard Henderson:
target/arm: Fix nregs computation in do_{ld,st}_zpa
42 b12a7671b609 Richard Henderson:
target/arm: Adjust and validate mtedesc sizem1
43 96fcc9982b4a Richard Henderson:
target/arm: Split out make_svemte_desc
44 623507ccfcfe Richard Henderson:
target/arm: Handle mte in do_ldrq, do_ldro
45 855f94eca80c Richard Henderson:
target/arm: Fix SVE/SME gross MTE suppression checks
46 ac1d88e9e7ca Peter Maydell:
target/arm: Don't get MDCR_EL2 in pmu_counter_enabled() before checking
ARM_FEATURE_PMU
47 cc29c12ec629 Kevin Wolf:
iotests: Make 144 deterministic again
48 8e31b744fdf2 Peter Maydell:
.gitlab-ci/windows.yml: Don't install libusb or spice packages on 32-bit
49 81f5cad3858f Xiaoyao Li:
i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not
available
50 a11a365159b9 Xiaoyao Li:
i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and
FEAT_XSAVE_XSS_HI le
[Stable-8.2.2 01/60] migration: Plug memory leak on HMP migrate error path
From: Markus Armbruster
hmp_migrate() leaks @caps when qmp_migrate() fails. Plug the leak
with g_autoptr().
Fixes: 967f2de5c9ec (migration: Implement MigrateChannelList to hmp migration
flow.) v8.2.0-rc0
Fixes: CID 1533125
Signed-off-by: Markus Armbruster
Link: https://lore.kernel.org/r/20240117140722.3979657-1-arm...@redhat.com
[peterx: fix CID number as reported by Peter Maydell]
Signed-off-by: Peter Xu
(cherry picked from commit 918f620d30a9b0095b7824b8d77a2d6059a439d9)
Signed-off-by: Michael Tokarev
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 86ae832176..2faa5cad46 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -762,7 +762,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
bool resume = qdict_get_try_bool(qdict, "resume", false);
const char *uri = qdict_get_str(qdict, "uri");
Error *err = NULL;
-MigrationChannelList *caps = NULL;
+g_autoptr(MigrationChannelList) caps = NULL;
g_autoptr(MigrationChannel) channel = NULL;
if (inc) {
@@ -787,8 +787,6 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
return;
}
-qapi_free_MigrationChannelList(caps);
-
if (!detach) {
HMPMigrationStatus *status;
--
2.39.2
Re: [RESEND PATCH v4 00/17] Add boot LoongArch elf kernel with FDT
Ping ! 在 2024/1/18 下午7:31, Song Gao 写道: Hi, All We already support boot efi kernel with bios, but not support boot elf kernel. This series adds boot elf kernel with FDT. 'LoongArch supports ACPI and FDT. The information that needs to be passed to the kernel includes the memmap, the initrd, the command line, optionally the ACPI/FDT tables, and so on' see [1]. Patch 2-8 : Create efi system table, and three efi configuration table boot_memmap, initd, FDT. Patch 9-17 : Fixes FDT problems. Test: - Start kernel See [2] start_kernel.sh - Start qcow2 See [2] start_qcow2.sh V4: - patch 3 change slave_boot_code[] to const, and 'static void *p ' to 'void *p'; - patch 4 fixes build error; - patch 10-13, add project and commit link. V3: - Load initrd at kernel_high + 4 * kernel_size; - Load 'boot_rom' at [0 - 1M], the 'boot_rom' includes slave_boot_code, cmdline_buf and systab_tables; - R-b and rebase. V2: - FDT pcie node adds cells 'msi-map'; [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arch/loongarch/booting.rst?h=v6.7-rc4 [2]: https://github.com/gaosong-loongson/loongarch-binary/releases Please review! Thanks. Song Gao Song Gao (17): hw/loongarch: Move boot fucntions to boot.c hw/loongarch: Add load initrd hw/loongarch: Add slave cpu boot_code hw/loongarch: Add init_cmdline hw/loongarch: Init efi_system_table hw/loongarch: Init efi_boot_memmap table hw/loongarch: Init efi_initrd table hw/loongarch: Init efi_fdt table hw/loongarch: Fix fdt memory node wrong 'reg' hw/loongarch: fdt adds cpu interrupt controller node hw/loongarch: fdt adds Extend I/O Interrupt Controller hw/loongarch: fdt adds pch_pic Controller hw/loongarch: fdt adds pch_msi Controller hw/loongarch: fdt adds pcie irq_map node hw/loongarch: fdt remove unused irqchip node hw/loongarch: Add cells missing from uart node hw/loongarch: Add cells missing from rtc node include/hw/intc/loongarch_extioi.h | 1 + include/hw/loongarch/boot.h| 109 + include/hw/loongarch/virt.h| 14 ++ include/hw/pci-host/ls7a.h | 2 + target/loongarch/cpu.h | 2 + hw/loongarch/boot.c| 330 ++ hw/loongarch/virt.c| 364 - hw/loongarch/meson.build | 1 + 8 files changed, 661 insertions(+), 162 deletions(-) create mode 100644 include/hw/loongarch/boot.h create mode 100644 hw/loongarch/boot.c
[Stable-8.2.2 16/60] iotests: give tempdir an identifying name
From: Daniel P. Berrangé
If something goes wrong causing the iotests not to cleanup their
temporary directory, it is useful if the dir had an identifying
name to show what is to blame.
Signed-off-by: Daniel P. Berrangé
Message-ID: <20240205155158.1843304-1-berra...@redhat.com>
Revieved-by: Michael Tokarev
Reviewed-by: Kevin Wolf
Signed-off-by: Kevin Wolf
(cherry picked from commit 7d2faf0ce2ccc896ac56bc5ed2cdf4a55056a8bb)
Signed-off-by: Michael Tokarev
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 3ff38f2661..588f30a4f1 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -126,7 +126,7 @@ def init_directories(self) -> None:
self.tmp_sock_dir = False
Path(self.sock_dir).mkdir(parents=True, exist_ok=True)
except KeyError:
-self.sock_dir = tempfile.mkdtemp()
+self.sock_dir = tempfile.mkdtemp(prefix="qemu-iotests-")
self.tmp_sock_dir = True
self.sample_img_dir = os.getenv('SAMPLE_IMG_DIR',
--
2.39.2
[Stable-8.2.2 11/60] pci-host: designware: Limit value range of iATU viewport register
From: Guenter Roeck
The latest version of qemu (v8.2.0-869-g7a1dc45af5) crashes when booting
the mcimx7d-sabre emulation with Linux v5.11 and later.
qemu-system-arm: ../system/memory.c:2750: memory_region_set_alias_offset:
Assertion `mr->alias' failed.
Problem is that the Designware PCIe emulation accepts the full value range
for the iATU Viewport Register. However, both hardware and emulation only
support four inbound and four outbound viewports.
The Linux kernel determines the number of supported viewports by writing
0xff into the viewport register and reading the value back. The expected
value when reading the register is the highest supported viewport index.
Match that code by masking the supported viewport value range when the
register is written. With this change, the Linux kernel reports
imx6q-pcie 3380.pcie: iATU: unroll F, 4 ob, 4 ib, align 0K, limit 4G
as expected and supported.
Fixes: d64e5eabc4c7 ("pci: Add support for Designware IP block")
Cc: Andrey Smirnov
Cc: Nikita Ostrenkov
Signed-off-by: Guenter Roeck
Message-id: 20240129060055.2616989-1-li...@roeck-us.net
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
(cherry picked from commit 8a73152020337a7fbf34daf0a006d4d89ec1494e)
Signed-off-by: Michael Tokarev
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index f477f97847..f016f02109 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -340,6 +340,8 @@ static void designware_pcie_root_config_write(PCIDevice *d,
uint32_t address,
break;
case DESIGNWARE_PCIE_ATU_VIEWPORT:
+val &= DESIGNWARE_PCIE_ATU_REGION_INBOUND |
+(DESIGNWARE_PCIE_NUM_VIEWPORTS - 1);
root->atu_viewport = val;
break;
--
2.39.2
[Stable-8.2.2 13/60] linux-user/aarch64: Add padding before __kernel_rt_sigreturn
From: Richard Henderson
Without this padding, an unwind through the signal handler
will pick up the unwind info for the preceding syscall.
This fixes gcc's 30_threads/thread/native_handle/cancel.cc.
Cc: qemu-sta...@nongnu.org
Fixes: ee95fae075c6 ("linux-user/aarch64: Add vdso")
Resolves: https://linaro.atlassian.net/browse/GNU-974
Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
Message-Id: <20240202034427.504686-1-richard.hender...@linaro.org>
(cherry picked from commit 6400be014f80e4c2c246eb8be709ea3a96428233)
Signed-off-by: Michael Tokarev
diff --git a/linux-user/aarch64/vdso-be.so b/linux-user/aarch64/vdso-be.so
index 6084f3d1a7..808206ade8 100755
Binary files a/linux-user/aarch64/vdso-be.so and
b/linux-user/aarch64/vdso-be.so differ
diff --git a/linux-user/aarch64/vdso-le.so b/linux-user/aarch64/vdso-le.so
index 947d534ec1..941aaf2993 100755
Binary files a/linux-user/aarch64/vdso-le.so and
b/linux-user/aarch64/vdso-le.so differ
diff --git a/linux-user/aarch64/vdso.S b/linux-user/aarch64/vdso.S
index 34d3a9ebd2..a0ac1487b0 100644
--- a/linux-user/aarch64/vdso.S
+++ b/linux-user/aarch64/vdso.S
@@ -63,7 +63,11 @@ vdso_syscall __kernel_clock_getres, __NR_clock_getres
* For now, elide the unwind info for __kernel_rt_sigreturn and rely on
* the libgcc fallback routine as we have always done. This requires
* that the code sequence used be exact.
+ *
+ * Add a nop as a spacer to ensure that unwind does not pick up the
+ * unwind info from the preceding syscall.
*/
+ nop
__kernel_rt_sigreturn:
/* No BTI C insn here -- we arrive via RET. */
mov x8, #__NR_rt_sigreturn
--
2.39.2
[Stable-8.2.2 10/60] target/arm: Reinstate "vfp" property on AArch32 CPUs
From: Peter Maydell
In commit 4315f7c614743 we restructured the logic for creating the
VFP related properties to avoid testing the aa32_simd_r32 feature on
AArch64 CPUs. However in the process we accidentally stopped
exposing the "vfp" QOM property on AArch32 TCG CPUs.
This mostly hasn't had any ill effects because not many people want
to disable VFP, but it wasn't intentional. Reinstate the property.
Cc: qemu-sta...@nongnu.org
Fixes: 4315f7c614743 ("target/arm: Restructure has_vfp_d32 test")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2098
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20240126193432.2210558-1-peter.mayd...@linaro.org
(cherry picked from commit 185e3fdf8d106cb2f7d234d5e6453939c66db2a9)
Signed-off-by: Michael Tokarev
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index efb22a87f9..5d9bca5b8d 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1615,6 +1615,10 @@ void arm_cpu_post_init(Object *obj)
}
} else if (cpu_isar_feature(aa32_vfp, cpu)) {
cpu->has_vfp = true;
+if (tcg_enabled() || qtest_enabled()) {
+qdev_property_add_static(DEVICE(obj),
+ &arm_cpu_has_vfp_property);
+}
if (cpu_isar_feature(aa32_simd_r32, cpu)) {
cpu->has_vfp_d32 = true;
/*
--
2.39.2
[Stable-8.2.2 12/60] tcg/loongarch64: Set vector registers call clobbered
From: Richard Henderson
Because there are more call clobbered registers than
call saved registers, we begin with all registers as
call clobbered and then reset those that are saved.
This was missed when we introduced the LSX support.
Cc: qemu-sta...@nongnu.org
Fixes: 16288ded944 ("tcg/loongarch64: Lower basic tcg vec ops to LSX")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2136
Signed-off-by: Richard Henderson
Reviewed-by: Song Gao
Message-Id: <20240201233414.500588-1-richard.hender...@linaro.org>
(cherry picked from commit 45bf0e7aa648369cf8ab2333bd20144806fc1be3)
Signed-off-by: Michael Tokarev
diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index bab0a173a3..dcf0205458 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -2327,7 +2327,7 @@ static void tcg_target_init(TCGContext *s)
tcg_target_available_regs[TCG_TYPE_I32] = ALL_GENERAL_REGS;
tcg_target_available_regs[TCG_TYPE_I64] = ALL_GENERAL_REGS;
-tcg_target_call_clobber_regs = ALL_GENERAL_REGS;
+tcg_target_call_clobber_regs = ALL_GENERAL_REGS | ALL_VECTOR_REGS;
tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S0);
tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S1);
tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S2);
--
2.39.2
[Stable-8.2.2 09/60] qemu-options.hx: Improve -serial option documentation
From: Peter Maydell The -serial option documentation is a bit brief about '-serial none' and '-serial null'. In particular it's not very clear about the difference between them, and it doesn't mention that it's up to the machine model whether '-serial none' means "don't create the serial port" or "don't wire the serial port up to anything". Expand on these points. Signed-off-by: Peter Maydell Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240122163607.459769-3-peter.mayd...@linaro.org (cherry picked from commit 747bfaf3a9d2f3cd51674763dc1f7575100cd200) Signed-off-by: Michael Tokarev diff --git a/qemu-options.hx b/qemu-options.hx index 42fd09e4de..b6b4ad9e67 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4118,7 +4118,8 @@ SRST This option can be used several times to simulate up to 4 serial ports. -Use ``-serial none`` to disable all serial ports. +You can use ``-serial none`` to suppress the creation of default +serial devices. Available character devices are: @@ -4140,10 +4141,17 @@ SRST [Linux only] Pseudo TTY (a new PTY is automatically allocated) ``none`` -No device is allocated. +No device is allocated. Note that for machine types which +emulate systems where a serial device is always present in +real hardware, this may be equivalent to the ``null`` option, +in that the serial device is still present but all output +is discarded. For boards where the number of serial ports is +truly variable, this suppresses the creation of the device. ``null`` -void device +A guest will see the UART or serial device as present in the +machine, but all output is discarded, and there is no input. +Conceptually equivalent to redirecting the output to ``/dev/null``. ``chardev:id`` Use a named character device defined with the ``-chardev`` -- 2.39.2
[Stable-8.2.2 07/60] target/arm: fix exception syndrome for AArch32 bkpt insn
From: Jan Klötzke
Debug exceptions that target AArch32 Hyp mode are reported differently
than on AAarch64. Internally, Qemu uses the AArch64 syndromes. Therefore
such exceptions need to be either converted to a prefetch abort
(breakpoints, vector catch) or a data abort (watchpoints).
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jan Klötzke
Reviewed-by: Richard Henderson
Message-id: 20240127202758.3326381-1-jan.kloet...@kernkonzept.com
Signed-off-by: Peter Maydell
(cherry picked from commit f670be1aad33e801779af580398895b9455747ee)
Signed-off-by: Michael Tokarev
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2746d3fdac..6515c5e89c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10823,6 +10823,24 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
}
if (env->exception.target_el == 2) {
+/* Debug exceptions are reported differently on AArch32 */
+switch (syn_get_ec(env->exception.syndrome)) {
+case EC_BREAKPOINT:
+case EC_BREAKPOINT_SAME_EL:
+case EC_AA32_BKPT:
+case EC_VECTORCATCH:
+env->exception.syndrome = syn_insn_abort(arm_current_el(env) == 2,
+ 0, 0, 0x22);
+break;
+case EC_WATCHPOINT:
+env->exception.syndrome = syn_set_ec(env->exception.syndrome,
+ EC_DATAABORT);
+break;
+case EC_WATCHPOINT_SAME_EL:
+env->exception.syndrome = syn_set_ec(env->exception.syndrome,
+ EC_DATAABORT_SAME_EL);
+break;
+}
arm_cpu_do_interrupt_aarch32_hyp(cs);
return;
}
diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
index 95454b5b3b..eccb759da6 100644
--- a/target/arm/syndrome.h
+++ b/target/arm/syndrome.h
@@ -25,6 +25,8 @@
#ifndef TARGET_ARM_SYNDROME_H
#define TARGET_ARM_SYNDROME_H
+#include "qemu/bitops.h"
+
/* Valid Syndrome Register EC field values */
enum arm_exception_class {
EC_UNCATEGORIZED = 0x00,
@@ -80,6 +82,7 @@ typedef enum {
SME_ET_InactiveZA,
} SMEExceptionType;
+#define ARM_EL_EC_LENGTH 6
#define ARM_EL_EC_SHIFT 26
#define ARM_EL_IL_SHIFT 25
#define ARM_EL_ISV_SHIFT 24
@@ -91,6 +94,11 @@ static inline uint32_t syn_get_ec(uint32_t syn)
return syn >> ARM_EL_EC_SHIFT;
}
+static inline uint32_t syn_set_ec(uint32_t syn, uint32_t ec)
+{
+return deposit32(syn, ARM_EL_EC_SHIFT, ARM_EL_EC_LENGTH, ec);
+}
+
/*
* Utility functions for constructing various kinds of syndrome value.
* Note that in general we follow the AArch64 syndrome values; in a
--
2.39.2
[Stable-8.2.2 34/60] virtio-gpu: Correct virgl_renderer_resource_get_info() error check
From: Dmitry Osipenko
virgl_renderer_resource_get_info() returns errno and not -1 on error.
Correct the return-value check.
Reviewed-by: Marc-André Lureau
Signed-off-by: Dmitry Osipenko
Message-Id: <20240129073921.446869-1-dmitry.osipe...@collabora.com>
Cc: qemu-sta...@nongnu.org
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
(cherry picked from commit 574b64aa6754ba491f51024c5a823a674d48a658)
Signed-off-by: Michael Tokarev
diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
index d1ccdf7d06..51da0e3667 100644
--- a/contrib/vhost-user-gpu/virgl.c
+++ b/contrib/vhost-user-gpu/virgl.c
@@ -327,7 +327,7 @@ virgl_get_resource_info_modifiers(uint32_t resource_id,
#ifdef VIRGL_RENDERER_RESOURCE_INFO_EXT_VERSION
struct virgl_renderer_resource_info_ext info_ext;
ret = virgl_renderer_resource_get_info_ext(resource_id, &info_ext);
-if (ret < 0) {
+if (ret) {
return ret;
}
@@ -335,7 +335,7 @@ virgl_get_resource_info_modifiers(uint32_t resource_id,
*modifiers = info_ext.modifiers;
#else
ret = virgl_renderer_resource_get_info(resource_id, info);
-if (ret < 0) {
+if (ret) {
return ret;
}
@@ -372,7 +372,7 @@ virgl_cmd_set_scanout(VuGpu *g,
uint64_t modifiers = 0;
ret = virgl_get_resource_info_modifiers(ss.resource_id, &info,
&modifiers);
-if (ret == -1) {
+if (ret) {
g_critical("%s: illegal resource specified %d\n",
__func__, ss.resource_id);
cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 8bb7a2c21f..9f34d0e661 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -181,7 +181,7 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
memset(&info, 0, sizeof(info));
ret = virgl_renderer_resource_get_info(ss.resource_id, &info);
#endif
-if (ret == -1) {
+if (ret) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: illegal resource specified %d\n",
__func__, ss.resource_id);
--
2.39.2
[Stable-8.2.2 08/60] system/vl.c: Fix handling of '-serial none -serial something'
From: Peter Maydell
Currently if the user passes multiple -serial options on the command
line, we mostly treat those as applying to the different serial
devices in order, so that for example
-serial stdio -serial file:filename
will connect the first serial port to stdio and the second to the
named file.
The exception to this is the '-serial none' serial device type. This
means "don't allocate this serial device", but a bug means that
following -serial options are not correctly handled, so that
-serial none -serial stdio
has the unexpected effect that stdio is connected to the first serial
port, not the second.
This is a very long-standing bug that dates back at least as far as
commit 998bbd74b9d81 from 2009.
Make the 'none' serial type move forward in the indexing of serial
devices like all the other serial types, so that any subsequent
-serial options are correctly handled.
Note that if your commandline mistakenly had a '-serial none' that
was being overridden by a following '-serial something' option, you
should delete the unnecessary '-serial none'. This will give you the
same behaviour as before, on QEMU versions both with and without this
bug fix.
Cc: qemu-sta...@nongnu.org
Reported-by: Bohdan Kostiv
Signed-off-by: Peter Maydell
Reviewed-by: Daniel P. Berrangé
Reviewed-by: Richard Henderson
Message-id: 20240122163607.459769-2-peter.mayd...@linaro.org
Fixes: 998bbd74b9d81 ("default devices: core code & serial lines")
Signed-off-by: Peter Maydell
(cherry picked from commit d2019a9d0c34a4fdcb5b5df550d73040dc0637d9)
Signed-off-by: Michael Tokarev
diff --git a/system/vl.c b/system/vl.c
index 6b87bfa32c..938b7b5acc 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1440,18 +1440,22 @@ static void qemu_create_default_devices(void)
static int serial_parse(const char *devname)
{
int index = num_serial_hds;
-char label[32];
-if (strcmp(devname, "none") == 0)
-return 0;
-snprintf(label, sizeof(label), "serial%d", index);
serial_hds = g_renew(Chardev *, serial_hds, index + 1);
-serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
-if (!serial_hds[index]) {
-error_report("could not connect serial device"
- " to character backend '%s'", devname);
-return -1;
+if (strcmp(devname, "none") == 0) {
+/* Don't allocate a serial device for this index */
+serial_hds[index] = NULL;
+} else {
+char label[32];
+snprintf(label, sizeof(label), "serial%d", index);
+
+serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
+if (!serial_hds[index]) {
+error_report("could not connect serial device"
+ " to character backend '%s'", devname);
+return -1;
+}
}
num_serial_hds++;
return 0;
--
2.39.2
[Stable-8.2.2 06/60] block/blkio: Make s->mem_region_alignment be 64 bits
From: "Richard W.M. Jones"
With GCC 14 the code failed to compile on i686 (and was wrong for any
version of GCC):
../block/blkio.c: In function ‘blkio_file_open’:
../block/blkio.c:857:28: error: passing argument 3 of ‘blkio_get_uint64’ from
incompatible pointer type [-Wincompatible-pointer-types]
857 |&s->mem_region_alignment);
|^~~~
||
|size_t * {aka unsigned int *}
In file included from ../block/blkio.c:12:
/usr/include/blkio.h:49:67: note: expected ‘uint64_t *’ {aka ‘long long
unsigned int *’} but argument is of type ‘size_t *’ {aka ‘unsigned int *’}
49 | int blkio_get_uint64(struct blkio *b, const char *name, uint64_t
*value);
| ~~^
Signed-off-by: Richard W.M. Jones
Message-id: 20240130122006.2977938-1-rjo...@redhat.com
Signed-off-by: Stefan Hajnoczi
(cherry picked from commit 615eaeab3d318ba239d54141a4251746782f65c1)
Signed-off-by: Michael Tokarev
diff --git a/block/blkio.c b/block/blkio.c
index 0a0a6c0f5f..bc2f21784c 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -68,7 +68,7 @@ typedef struct {
CoQueue bounce_available;
/* The value of the "mem-region-alignment" property */
-size_t mem_region_alignment;
+uint64_t mem_region_alignment;
/* Can we skip adding/deleting blkio_mem_regions? */
bool needs_mem_regions;
--
2.39.2
[Stable-8.2.2 02/60] migration: Fix use-after-free of migration state object
From: Fabiano Rosas We're currently allowing the process_incoming_migration_bh bottom-half to run without holding a reference to the 'current_migration' object, which leads to a segmentation fault if the BH is still live after migration_shutdown() has dropped the last reference to current_migration. In my system the bug manifests as migrate_multifd() returning true when it shouldn't and multifd_load_shutdown() calling multifd_recv_terminate_threads() which crashes due to an uninitialized multifd_recv_state. Fix the issue by holding a reference to the object when scheduling the BH and dropping it before returning from the BH. The same is already done for the cleanup_bh at migrate_fd_cleanup_schedule(). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1969 Signed-off-by: Fabiano Rosas Link: https://lore.kernel.org/r/20240119233922.32588-2-faro...@suse.de Signed-off-by: Peter Xu (cherry picked from commit 27eb8499edb2bc952c29ddae0bdac9fc959bf7b1) Signed-off-by: Michael Tokarev diff --git a/migration/migration.c b/migration/migration.c index 3ce04b2aaf..ee5e0ba97a 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -650,6 +650,7 @@ static void process_incoming_migration_bh(void *opaque) MIGRATION_STATUS_COMPLETED); qemu_bh_delete(mis->bh); migration_incoming_state_destroy(); +object_unref(OBJECT(migrate_get_current())); } static void coroutine_fn @@ -708,6 +709,7 @@ process_incoming_migration_co(void *opaque) } mis->bh = qemu_bh_new(process_incoming_migration_bh, mis); +object_ref(OBJECT(migrate_get_current())); qemu_bh_schedule(mis->bh); return; fail: -- 2.39.2
[Stable-8.2.2 05/60] qemu-docs: Update options for graphical frontends
From: Yihuan Pan The command line options `-ctrl-grab` and `-alt-grab` have been removed in QEMU 7.1. Instead, use the `-display sdl,grab-mod=` option to specify the grab modifiers. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2103 Signed-off-by: Yihuan Pan Signed-off-by: Michael Tokarev (cherry picked from commit db101376af52e81f740a27f5fa38260ad171323c) diff --git a/docs/system/keys.rst.inc b/docs/system/keys.rst.inc index bd9b8e5f6f..2e2c97aa23 100644 --- a/docs/system/keys.rst.inc +++ b/docs/system/keys.rst.inc @@ -1,8 +1,9 @@ -During the graphical emulation, you can use special key combinations to -change modes. The default key mappings are shown below, but if you use -``-alt-grab`` then the modifier is Ctrl-Alt-Shift (instead of Ctrl-Alt) -and if you use ``-ctrl-grab`` then the modifier is the right Ctrl key -(instead of Ctrl-Alt): +During the graphical emulation, you can use special key combinations from +the following table to change modes. By default the modifier is Ctrl-Alt +(used in the table below) which can be changed with ``-display`` suboption +``mod=`` where appropriate. For example, ``-display sdl, +grab-mod=lshift-lctrl-lalt`` changes the modifier key to Ctrl-Alt-Shift, +while ``-display sdl,grab-mod=rctrl`` changes it to the right Ctrl key. Ctrl-Alt-f Toggle full screen -- 2.39.2
Re: [PATCH 3/6] hw/mem/cxl_type3: Fix missing ERRP_GUARD() in ct3_realize()
Zhao Liu writes:
> From: Zhao Liu
>
> As the comment in qapi/error, dereferencing @errp requires
> ERRP_GUARD():
>
> * = Why, when and how to use ERRP_GUARD() =
> *
> * Without ERRP_GUARD(), use of the @errp parameter is restricted:
> * - It must not be dereferenced, because it may be null.
> * - It should not be passed to error_prepend() or
> * error_append_hint(), because that doesn't work with &error_fatal.
> * ERRP_GUARD() lifts these restrictions.
> *
> * To use ERRP_GUARD(), add it right at the beginning of the function.
> * @errp can then be used without worrying about the argument being
> * NULL or &error_fatal.
> *
> * Using it when it's not needed is safe, but please avoid cluttering
> * the source with useless code.
>
> Currently, since ct3_realize() - as a PCIDeviceClass.realize() method -
> doesn't get the NULL errp parameter, it doesn't trigger the dereference
> issue.
>
> To follow the requirement of errp, add missing ERRP_GUARD() in
> ct3_realize().
>
> Suggested-by: Markus Armbruster
> Signed-off-by: Zhao Liu
> ---
> Suggested by credit:
> Markus: Referred his explanation about ERRP_GUARD().
> ---
> hw/mem/cxl_type3.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index e8801805b90f..a3b0761f843b 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -645,6 +645,7 @@ static DOEProtocol doe_cdat_prot[] = {
>
> static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> {
> +ERRP_GUARD();
> CXLType3Dev *ct3d = CXL_TYPE3(pci_dev);
> CXLComponentState *cxl_cstate = &ct3d->cxl_cstate;
> ComponentRegisters *regs = &cxl_cstate->crb;
The dereference is
cxl_doe_cdat_init(cxl_cstate, errp);
if (*errp) {
goto err_free_special_ops;
}
We check *errp, because cxl_doe_cdat_init() returns void. Could be
improved to return bool, along with its callees ct3_load_cdat() and
ct3_build_cdat(), but that's a slightly more ambitious cleanup, so
Reviewed-by: Markus Armbruster
[Stable-8.2.2 30/60] cxl/cdat: Handle cdat table build errors
From: Ira Weiny
The callback for building CDAT tables may return negative error codes.
This was previously unhandled and will result in potentially huge
allocations later on in ct3_build_cdat()
Detect the negative error code and defer cdat building.
Fixes: f5ee7413d592 ("hw/mem/cxl-type3: Add CXL CDAT Data Object Exchange")
Cc: Huai-Cheng Kuo
Reviewed-by: Dave Jiang
Reviewed-by: Fan Ni
Signed-off-by: Ira Weiny
Signed-off-by: Jonathan Cameron
Message-Id: <20240126120132.24248-2-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
(cherry picked from commit c62926f730d08450502d36548e28dd727c998ace)
Signed-off-by: Michael Tokarev
diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
index 639a2db3e1..24829cf242 100644
--- a/hw/cxl/cxl-cdat.c
+++ b/hw/cxl/cxl-cdat.c
@@ -63,7 +63,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
cdat->built_buf_len = cdat->build_cdat_table(&cdat->built_buf,
cdat->private);
-if (!cdat->built_buf_len) {
+if (cdat->built_buf_len <= 0) {
/* Build later as not all data available yet */
cdat->to_update = true;
return;
--
2.39.2
[RFC PATCH v2 00/22] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI
This patch set implements FEAT_NMI and FEAT_GICv3_NMI for armv8. These introduce support for a new category of interrupts in the architecture which we can use to provide NMI like functionality. There are two modes for using this FEAT_NMI. When PSTATE.ALLINT or PSTATE.SP & SCTLR_ELx.SCTLR_SPINTMASK is set, any entry to ELx causes all interrupts including those with superpriority to be masked on entry to ELn until the mask is explicitly removed by software or hardware. PSTATE.ALLINT can be managed by software using the new register control ALLINT.ALLINT. Independent controls are provided for this feature at each EL, usage at EL1 should not disrupt EL2 or EL3. I have tested it with the following linux patches which try to support FEAT_NMI in linux kernel: https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/T/#mb4ba4a2c045bf72c10c2202c1dd1b82d3240dc88 In the test, SGI, PPI and SPI interrupts can all be set to have super priority to be converted to a hardware NMI interrupt. The SGI is tested with kernel IPI as NMI framework, and the PPI interrupt is tested with "perf top" command with hardware NMI enabled, and the PPI interrupt is tested with a custom test module, in which NMI interrupts can be received and transmitted normally. Changes in v2: - Break up the patches so that each one does only one thing. - Remove the command line option and just implement it in "max" cpu. Jinjie Ruan (22): target/arm: Add FEAT_NMI to max target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NMI target/arm: Add PSTATE.ALLINT target/arm: Implement ALLINT MSR (immediate) target/arm: Support MSR access to ALLINT target/arm: Add support for Non-maskable Interrupt target/arm: Add support for NMI event state target/arm: Handle IS/FS in ISR_EL1 for NMI target/arm: Add support for FEAT_NMI, Non-maskable Interrupt target/arm: Handle PSTATE.ALLINT on taking an exception target/arm: Set pstate.ALLINT in arm_cpu_reset_hold hw/arm/virt: Wire NMI irq line from GIC to CPU hw/intc/arm_gicv3: Add external IRQ lines for NMI target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64() hw/intc/arm_gicv3_redist: Implement GICR_INMIR0 hw/intc/arm_gicv3: Implement GICD_INMIR hw/intc: Enable FEAT_GICv3_NMI Feature hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC hw/intc/arm_gicv3: Add irq superpriority information hw/intc/arm_gicv3: Add NMI handling CPU interface registers hw/intc/arm_gicv3: Implement NMI interrupt prioirty hw/intc/arm_gicv3: Report the NMI interrupt in gicv3_cpuif_update() docs/system/arm/emulation.rst | 1 + hw/arm/virt.c | 7 ++- hw/intc/arm_gicv3.c| 61 ++--- hw/intc/arm_gicv3_common.c | 4 ++ hw/intc/arm_gicv3_cpuif.c | 53 -- hw/intc/arm_gicv3_dist.c | 40 + hw/intc/arm_gicv3_redist.c | 23 ++ hw/intc/gicv3_internal.h | 5 +++ include/hw/intc/arm_gic_common.h | 1 + include/hw/intc/arm_gicv3_common.h | 6 +++ target/arm/cpu-features.h | 5 +++ target/arm/cpu-qom.h | 3 +- target/arm/cpu.c | 46 --- target/arm/cpu.h | 15 ++- target/arm/helper.c| 72 ++ target/arm/internals.h | 3 ++ target/arm/tcg/a64.decode | 1 + target/arm/tcg/cpu64.c | 1 + target/arm/tcg/helper-a64.c| 24 ++ target/arm/tcg/helper-a64.h| 1 + target/arm/tcg/translate-a64.c | 10 + 21 files changed, 362 insertions(+), 20 deletions(-) -- 2.34.1
[Stable-8.2.2 28/60] tcg/arm: Fix goto_tb for large translation blocks
From: Richard Henderson
Correct arithmetic for separating high and low
on a large negative number.
Cc: qemu-sta...@nongnu.org
Fixes: 79ffece4447 ("tcg/arm: Implement direct branch for goto_tb")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1714
Signed-off-by: Richard Henderson
Reviewed-by: Michael Tokarev
(cherry picked from commit e41f1825b43796c3508ef309ed0b150ef89acc44)
Signed-off-by: Michael Tokarev
diff --git a/tcg/arm/tcg-target.c.inc b/tcg/arm/tcg-target.c.inc
index a9aa8aa91c..c9a47b7ea1 100644
--- a/tcg/arm/tcg-target.c.inc
+++ b/tcg/arm/tcg-target.c.inc
@@ -1736,9 +1736,9 @@ static void tcg_out_goto_tb(TCGContext *s, int which)
* shifted immediate from pc.
*/
int h = -i_disp;
-int l = h & 0xfff;
+int l = -(h & 0xfff);
-h = encode_imm_nofail(h - l);
+h = encode_imm_nofail(h + l);
tcg_out_dat_imm(s, COND_AL, ARITH_SUB, TCG_REG_R0, TCG_REG_PC, h);
tcg_out_ld32_12(s, COND_AL, TCG_REG_PC, TCG_REG_R0, l);
}
--
2.39.2
[qemu-web PATCH] Replace deprecated "bundle install --path vendor"
When running "bundle install --path vendor", the command complains that the --path parameter is deprecated nowadays and that "bundle config set --local path 'vendor'" should be used instead. So let's update our README accordingly. Signed-off-by: Thomas Huth --- README | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README b/README index 31be741..ee3119e 100644 --- a/README +++ b/README @@ -32,7 +32,8 @@ required Jekyll software should be installed locally: * Install Jekyll and its dependencies - # bundle install --path vendor + # bundle config set --local path 'vendor' + # bundle install NB this last command must be run from the qemu-web.git checkout root directory. -- 2.43.2
[Stable-8.2.2 27/60] tcg: Increase width of temp_subindex
From: Richard Henderson
We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.
Cc: qemu-sta...@nongnu.org
Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael Tokarev
Tested-by: Michael Tokarev
(cherry picked from commit c0e688153f299d5d493989c80bcc84c9cf36d6a6)
Signed-off-by: Michael Tokarev
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index daf2a5bf9e..451f3fec41 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -412,7 +412,7 @@ typedef struct TCGTemp {
unsigned int mem_coherent:1;
unsigned int mem_allocated:1;
unsigned int temp_allocated:1;
-unsigned int temp_subindex:1;
+unsigned int temp_subindex:2;
int64_t val;
struct TCGTemp *mem_base;
--
2.39.2
[Stable-8.2.2 04/60] Make 'uri' optional for migrate QAPI
From: Het Gala
'uri' argument should be optional, as 'uri' and 'channels'
arguments are mutally exclusive in nature.
Fixes: 074dbce5fcce (migration: New migrate and migrate-incoming argument
'channels')
Signed-off-by: Het Gala
Link: https://lore.kernel.org/r/20240123064219.40514-1-het.g...@nutanix.com
Signed-off-by: Peter Xu
(cherry picked from commit 57fd4b4e10756448acd6c90ce041ba8dc9313efc)
Signed-off-by: Michael Tokarev
diff --git a/qapi/migration.json b/qapi/migration.json
index eb2f883513..197d3faa43 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1757,7 +1757,7 @@
#
##
{ 'command': 'migrate',
- 'data': {'uri': 'str',
+ 'data': {'*uri': 'str',
'*channels': [ 'MigrationChannel' ],
'*blk': { 'type': 'bool', 'features': [ 'deprecated' ] },
'*inc': { 'type': 'bool', 'features': [ 'deprecated' ] },
--
2.39.2
[RFC PATCH v2 11/22] target/arm: Set pstate.ALLINT in arm_cpu_reset_hold
Set pstate.ALLINT in arm_cpu_reset_hold as daif do it.
Signed-off-by: Jinjie Ruan
---
target/arm/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 055670343e..e850763158 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -357,6 +357,10 @@ static void arm_cpu_reset_hold(Object *obj)
}
env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+env->allint = PSTATE_ALLINT;
+}
+
/* AArch32 has a hard highvec setting of 0x. If we are currently
* executing as AArch32 then check if highvecs are enabled and
* adjust the PC accordingly.
--
2.34.1
[Stable-8.2.2 47/60] iotests: Make 144 deterministic again
From: Kevin Wolf
Since commit effd60c8 changed how QMP commands are processed, the order
of the block-commit return value and job events in iotests 144 wasn't
fixed and more and caused the test to fail intermittently.
Change the test to cache events first and then print them in a
predefined order.
Waiting three times for JOB_STATUS_CHANGE is a bit uglier than just
waiting for the JOB_STATUS_CHANGE that has "status": "ready", but the
tooling we have doesn't seem to allow the latter easily.
Fixes: effd60c878176bcaf97fa7ce2b12d04bb8ead6f7
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2126
Signed-off-by: Kevin Wolf
Reviewed-by: Stefan Hajnoczi
Message-id: 20240209173103.239994-1-kw...@redhat.com
Signed-off-by: Peter Maydell
(cherry picked from commit cc29c12ec629ba68a4a6cb7d165c94cc8502815a)
Signed-off-by: Michael Tokarev
diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144
index bdcc498fa2..d284a0e442 100755
--- a/tests/qemu-iotests/144
+++ b/tests/qemu-iotests/144
@@ -83,12 +83,22 @@ echo
echo === Performing block-commit on active layer ===
echo
+capture_events="BLOCK_JOB_READY JOB_STATUS_CHANGE"
+
# Block commit on active layer, push the new overlay into base
_send_qemu_cmd $h "{ 'execute': 'block-commit',
'arguments': {
'device': 'virtio0'
}
-}" "READY"
+}" "return"
+
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+
+_wait_event $h "BLOCK_JOB_READY"
+
+capture_events=
_send_qemu_cmd $h "{ 'execute': 'block-job-complete',
'arguments': {
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index b3b4812015..2245ddfa10 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -25,9 +25,9 @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 cluster_size=65536
extended_l2=off co
'device': 'virtio0'
}
}
+{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "virtio0"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "virtio0"}}
-{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "virtio0"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0,
"speed": 0, "type": "commit"}}
{ 'execute': 'block-job-complete',
--
2.39.2
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Thanks, Markus.
On Wed, Feb 21, 2024 at 12:36:57PM +0530, Het Gala wrote:
> > > diff --git a/qapi/migration.json b/qapi/migration.json
> > > index 5a565d9b8d..5756e650b0 100644
> > > --- a/qapi/migration.json
> > > +++ b/qapi/migration.json
> > > @@ -1728,6 +1728,7 @@
> > > #
> > > # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
> > > # <- { "return": {} }
> > > +#
> > > # -> { "execute": "migrate",
> > > # "arguments": {
> > > # "channels": [ { "channel-type": "main",
> > > @@ -1796,19 +1797,19 @@
> > > #
> > > # 3. The uri format is the same as for -incoming
> > > #
> > > -# 5. For now, number of migration streams is restricted to one,
> > > +# 4. For now, number of migration streams is restricted to one,
> > > #i.e number of items in 'channels' list is just 1.
> > > #
> > > -# 4. The 'uri' and 'channels' arguments are mutually exclusive;
> > > +# 5. The 'uri' and 'channels' arguments are mutually exclusive;
> > > #exactly one of the two should be present.
> > > #
> > > # Example:
> > > #
> > > # -> { "execute": "migrate-incoming",
> > > -# "arguments": { "uri": "tcp::4446" } }
> > > +# "arguments": { "uri": "tcp:0:4446" } }
> > > # <- { "return": {} }
> > > #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > > # "arguments": {
> > > # "channels": [ { "channel-type": "main",
> > > # "addr": { "transport": "socket",
> > > @@ -1817,7 +1818,7 @@
> > > #"port": "1050" } } ] } }
> > > # <- { "return": {} }
> > > #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > > # "arguments": {
> > > # "channels": [ { "channel-type": "main",
> > > # "addr": { "transport": "exec",
> > > @@ -1825,7 +1826,7 @@
> > > # "/some/sock" ] } } ] } }
> > > # <- { "return": {} }
> > > #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > > # "arguments": {
> > > # "channels": [ { "channel-type": "main",
> > > # "addr": { "transport": "rdma",
Reviewed-by: Peter Xu
Markus, do you want us to pick it up, or let it go via qapi?
Thanks,
--
Peter Xu
[Stable-8.2.2 03/60] vfio/pci: Clear MSI-X IRQ index always
From: Cédric Le Goater
When doing device assignment of a physical device, MSI-X can be
enabled with no vectors enabled and this sets the IRQ index to
VFIO_PCI_MSIX_IRQ_INDEX. However, when MSI-X is disabled, the IRQ
index is left untouched if no vectors are in use. Then, when INTx
is enabled, the IRQ index value is considered incompatible (set to
MSI-X) and VFIO_DEVICE_SET_IRQS fails. QEMU complains with :
qemu-system-x86_64: vfio :08:00.0: Failed to set up TRIGGER eventfd
signaling for interrupt INTX-0: VFIO_DEVICE_SET_IRQS failure: Invalid argument
To avoid that, unconditionaly clear the IRQ index when MSI-X is
disabled.
Buglink: https://issues.redhat.com/browse/RHEL-21293
Fixes: 5ebffa4e87e7 ("vfio/pci: use an invalid fd to enable MSI-X")
Cc: Jing Liu
Cc: Alex Williamson
Reviewed-by: Alex Williamson
Signed-off-by: Cédric Le Goater
(cherry picked from commit d2b668fca5652760b435ce812a743bba03d2f316)
Signed-off-by: Michael Tokarev
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c62c02f7b6..e167bef2ad 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -824,9 +824,11 @@ static void vfio_msix_disable(VFIOPCIDevice *vdev)
}
}
-if (vdev->nr_vectors) {
-vfio_disable_irqindex(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX);
-}
+/*
+ * Always clear MSI-X IRQ index. A PF device could have enabled
+ * MSI-X with no vectors. See vfio_msix_enable().
+ */
+vfio_disable_irqindex(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX);
vfio_msi_disable_common(vdev);
vfio_intx_enable(vdev, &err);
--
2.39.2
Re: [PATCH v2 1/2] qapi/migration.json: add reason to MIGRATION event
If I understood you right, you suggest to improve migrate_generate_event to accept MigrationState* instead of int* state (which is pointing to field MigrationState.state in all usages), and get error reason from MigrationState.error, if the new state is MIGRATION_STATE_FAILED, is it? That sounds reasonable, thanks! But I'm not sure if I got the idea of changing migrate_set_error correctly, can you explain in more details, please? On 2/20/24 10:39, Peter Xu wrote: On Thu, Feb 15, 2024 at 05:27:58PM +0500, Roman Khapov wrote: migrate_set_state(&mis->state, MIGRATION_STATUS_COLO, - MIGRATION_STATUS_COMPLETED); + MIGRATION_STATUS_COMPLETED, NULL); Instead of enforcing migrate_set_error() to always pass an error, maybe we can allow migrate_generate_event() to fetch s->error in FAILED state, if that hint ever existed? -- Best regards, Roman Khapov
Re: [PATCH v2] virtio-iommu: Use qemu_real_host_page_mask as default page_size_mask
On Wed, Feb 21, 2024 at 11:41:57AM +0100, Eric Auger wrote: > Hi, > > On 2/13/24 13:00, Michael S. Tsirkin wrote: > > On Tue, Feb 13, 2024 at 12:24:22PM +0100, Eric Auger wrote: > >> Hi Michael, > >> On 2/13/24 12:09, Michael S. Tsirkin wrote: > >>> On Tue, Feb 13, 2024 at 11:32:13AM +0100, Eric Auger wrote: > Do you have an other concern? > >>> I also worry a bit about migrating between hosts with different > >>> page sizes. Not with kvm I am guessing but with tcg it does work I think? > >> I have never tried but is it a valid use case? Adding Peter in CC. > >>> Is this just for vfio and vdpa? Can we limit this to these setups > >>> maybe? > >> I am afraid we know the actual use case too later. If the VFIO device is > >> hotplugged we have started working with 4kB granule. > >> > >> The other way is to introduce a min_granule option as done for aw-bits. > >> But it is heavier. > >> > >> Thanks > >> > >> Eric > > Let's say, if you are changing the default then we definitely want > > a way to get the cmpatible behaviour for tcg. > > So the compat machinery should be user-accessible too and documented. > > I guess I need to add a new option to guarantee the machine compat. > > I was thinking about an enum GranuleMode property taking the following > values, 4KB, 64KB, host > Jean, do you think there is a rationale offering something richer? 16KB seems to be gaining popularity, we should include that (I think it's the only granule supported by Apple IOMMU?). Hopefully that will be enough. Thanks, Jean > > Obviously being able to set the exact page_size_mask + host mode would > be better but this does not really fit into any std property type. > > Thanks > > Eric > > >
[Stable-8.2.2 23/60] configure: run plugin TCG tests again
From: Paolo Bonzini
Commit 39fb3cfc28b ("configure: clean up plugin option handling", 2023-10-18)
dropped the CONFIG_PLUGIN line from tests/tcg/config-host.mak, due to confusion
caused by the shadowing of $config_host_mak. However, TCG tests were still
expecting it. Oops.
Put it back, in the meanwhile the shadowing is gone so it's clear that it goes
in the tests/tcg configuration.
Cc:
Fixes: 39fb3cfc28b ("configure: clean up plugin option handling", 2023-10-18)
Signed-off-by: Paolo Bonzini
Message-Id: <20240124115332.612162-1-pbonz...@redhat.com>
Signed-off-by: Alex Bennée
Message-Id: <20240207163812.3231697-4-alex.ben...@linaro.org>
(cherry picked from commit 15cc103362499bd94c5aec5fa66543d0de3bf4b5)
Signed-off-by: Michael Tokarev
(Mjt: context fixup)
diff --git a/configure b/configure
index d7e0926ff1..d3ab436045 100755
--- a/configure
+++ b/configure
@@ -1675,6 +1675,9 @@ fi
mkdir -p tests/tcg
echo "# Automatically generated by configure - do not modify" >
$config_host_mak
echo "SRC_PATH=$source_path" >> $config_host_mak
+if test "$plugins" = "yes" ; then
+echo "CONFIG_PLUGIN=y" >> tests/tcg/$config_host_mak
+fi
tcg_tests_targets=
for target in $target_list; do
--
2.39.2
Re: [PATCH v2 1/7] migration/multifd: Add new migration option zero-page-detection.
On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang wrote:
> This new parameter controls where the zero page checking is running.
> 1. If this parameter is set to 'legacy', zero page checking is
> done in the migration main thread.
> 2. If this parameter is set to 'none', zero page checking is disabled.
>
>
Hello Hao
Few questions and comments.
First the commit message states that the parameter control where the
checking is done, but it also controls
if sending of zero pages is done by multifd threads or not.
> Signed-off-by: Hao Xiang
> ---
> hw/core/qdev-properties-system.c| 10 ++
> include/hw/qdev-properties-system.h | 4
> migration/migration-hmp-cmds.c | 9 +
> migration/options.c | 21
> migration/options.h | 1 +
> migration/ram.c | 4
> qapi/migration.json | 30 ++---
> 7 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 1a396521d5..63843f18b5 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = {
> .set_default_value = qdev_propinfo_set_default_value_enum,
> };
>
> +const PropertyInfo qdev_prop_zero_page_detection = {
> +.name = "ZeroPageDetection",
> +.description = "zero_page_detection values, "
> + "multifd,legacy,none",
> +.enum_table = &ZeroPageDetection_lookup,
> +.get = qdev_propinfo_get_enum,
> +.set = qdev_propinfo_set_enum,
> +.set_default_value = qdev_propinfo_set_default_value_enum,
> +};
> +
> /* --- Reserved Region --- */
>
> /*
> diff --git a/include/hw/qdev-properties-system.h
> b/include/hw/qdev-properties-system.h
> index 06c359c190..839b170235 100644
> --- a/include/hw/qdev-properties-system.h
> +++ b/include/hw/qdev-properties-system.h
> @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr;
> extern const PropertyInfo qdev_prop_reserved_region;
> extern const PropertyInfo qdev_prop_multifd_compression;
> extern const PropertyInfo qdev_prop_mig_mode;
> +extern const PropertyInfo qdev_prop_zero_page_detection;
> extern const PropertyInfo qdev_prop_losttickpolicy;
> extern const PropertyInfo qdev_prop_blockdev_on_error;
> extern const PropertyInfo qdev_prop_bios_chs_trans;
> @@ -47,6 +48,9 @@ extern const PropertyInfo
> qdev_prop_iothread_vq_mapping_list;
> #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \
> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \
> MigMode)
> +#define DEFINE_PROP_ZERO_PAGE_DETECTION(_n, _s, _f, _d) \
> +DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_zero_page_detection, \
> + ZeroPageDetection)
> #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
> LostTickPolicy)
> diff --git a/migration/migration-hmp-cmds.c
> b/migration/migration-hmp-cmds.c
> index 99b49df5dd..7e96ae6ffd 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -344,6 +344,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const
> QDict *qdict)
> monitor_printf(mon, "%s: %s\n",
>
> MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
> MultiFDCompression_str(params->multifd_compression));
> +assert(params->has_zero_page_detection);
>
What is the reason to have assert here?
> +monitor_printf(mon, "%s: %s\n",
> +
> MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION),
> +qapi_enum_lookup(&ZeroPageDetection_lookup,
> +params->zero_page_detection));
> monitor_printf(mon, "%s: %" PRIu64 " bytes\n",
> MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
> params->xbzrle_cache_size);
> @@ -634,6 +639,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> p->has_multifd_zstd_level = true;
> visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
> break;
> +case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION:
> +p->has_zero_page_detection = true;
> +visit_type_ZeroPageDetection(v, param, &p->zero_page_detection,
> &err);
> +break;
> case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
> p->has_xbzrle_cache_size = true;
> if (!visit_type_size(v, param, &cache_size, &err)) {
> diff --git a/migration/options.c b/migration/options.c
> index 3e3e0b93b4..3c603391b0 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -179,6 +179,9 @@ Property migration_properties[] = {
> DEFINE_PROP_MIG_MODE("mode", MigrationState,
>parameters.mode,
>MIG_MODE_NORMAL),
> +DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
[RFC PATCH v2 07/22] target/arm: Add support for NMI event state
The NMI exception state include whether the interrupt with super priority
is IRQ or FIQ, so add a nmi_is_irq flag in CPUARMState to distinguish it.
Signed-off-by: Jinjie Ruan
---
target/arm/cpu.h| 2 ++
target/arm/helper.c | 9 +
2 files changed, 11 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5257343bcb..051e589e19 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -603,6 +603,8 @@ typedef struct CPUArchState {
/* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
uint32_t irq_line_state;
+bool nmi_is_irq;
+
/* Thumb-2 EE state. */
uint32_t teecr;
uint32_t teehbr;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bd7858e02e..0bd7a87e51 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10575,6 +10575,15 @@ uint32_t arm_phys_excp_target_el(CPUState *cs,
uint32_t excp_idx,
scr = ((env->cp15.scr_el3 & SCR_FIQ) == SCR_FIQ);
hcr = hcr_el2 & HCR_FMO;
break;
+case EXCP_NMI:
+if (env->nmi_is_irq) {
+scr = ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
+hcr = hcr_el2 & HCR_IMO;
+} else {
+scr = ((env->cp15.scr_el3 & SCR_FIQ) == SCR_FIQ);
+hcr = hcr_el2 & HCR_FMO;
+}
+break;
default:
scr = ((env->cp15.scr_el3 & SCR_EA) == SCR_EA);
hcr = hcr_el2 & HCR_AMO;
--
2.34.1
Re: [PATCH 01/10] hw/i386: Store pointers to IDE buses in PCMachineState
Hi Peter,
On 20/2/24 17:06, Peter Maydell wrote:
Add the two IDE bus BusState pointers to the set we keep in PCMachineState.
This allows us to avoid passing them to pc_cmos_init(), and also will
allow a refactoring of how we call pc_cmos_init_late().
Signed-off-by: Peter Maydell
---
include/hw/i386/pc.h | 4 +++-
hw/i386/pc.c | 5 ++---
hw/i386/pc_piix.c| 16 +++-
hw/i386/pc_q35.c | 9 -
4 files changed, 16 insertions(+), 18 deletions(-)
@@ -300,13 +299,13 @@ static void pc_q35_init(MachineState *machine)
ICH9_SATA1_FUNC),
"ich9-ahci");
ich9 = ICH9_AHCI(pdev);
-idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
-idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
+pcms->idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
+pcms->idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
ide_drive_get(hd, ich9->ahci.ports);
ahci_ide_create_devs(&ich9->ahci, hd);
} else {
-idebus[0] = idebus[1] = NULL;
+pcms->idebus[0] = pcms->idebus[1] = NULL;
Since PCMachineState is zero-initialized, this part is now
pointless.
Since my ICH9 series clashes with your patch, I'm inclined to
queue it in my hw-misc tree, with the following squashed:
-- >8 --
-} else {
-pcms->idebus[0] = pcms->idebus[1] = NULL;
---
}
if (machine_usb(machine)) {
@@ -327,7 +326,7 @@ static void pc_q35_init(MachineState *machine)
smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
}
-pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
+pc_cmos_init(pcms, rtc_state);
/* the rest devices to which pci devfn is automatically assigned */
pc_vga_init(isa_bus, host_bus);
[Stable-8.2.2 38/60] hw/i386: Fix _STA return value for ACPI0017
From: Jonathan Cameron
Found whilst testing a series for the linux kernel that actually
bothers to check if enabled is set. 0xB is the option used
for vast majority of DSDT entries in QEMU.
It is a little odd for a device that doesn't really exist and
is simply a hook to tell the OS there is a CEDT table but 0xB
seems a reasonable choice and avoids need to special case
this device in the OS.
Means:
* Device present.
* Device enabled and decoding it's resources.
* Not shown in UI
* Functioning properly
* No battery (on this device!)
Signed-off-by: Jonathan Cameron
Message-Id: <20240126120132.24248-12-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
(cherry picked from commit d9ae5802f656f6fb53b788747ba557a826b6e740)
Signed-off-by: Michael Tokarev
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 80db183b78..1e178341de 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1415,7 +1415,7 @@ static void build_acpi0017(Aml *table)
aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0017")));
method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-aml_append(method, aml_return(aml_int(0x01)));
+aml_append(method, aml_return(aml_int(0x0B)));
aml_append(dev, method);
build_cxl_dsm_method(dev);
--
2.39.2
[RFC PATCH v2 09/22] target/arm: Add support for FEAT_NMI, Non-maskable Interrupt
Add support for FEAT_NMI. NMI (FEAT_NMI) is an mandatory feature in
ARMv8.8-A and ARM v9.3-A.
Signed-off-by: Jinjie Ruan
---
target/arm/internals.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 50bff44549..fee65caba5 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1078,6 +1078,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const
ARMISARegisters *id)
if (isar_feature_aa64_mte(id)) {
valid |= PSTATE_TCO;
}
+if (isar_feature_aa64_nmi(id)) {
+valid |= PSTATE_ALLINT;
+}
return valid;
}
--
2.34.1
[Stable-8.2.2 51/60] i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F
From: Xiaoyao Li
Existing code misses a decrement of cpuid_i when skip leaf 0x1F.
There's a blank CPUID entry(with leaf, subleaf as 0, and all fields
stuffed 0s) left in the CPUID array.
It conflicts with correct CPUID leaf 0.
Signed-off-by: Xiaoyao Li
Reviewed-by:Yang Weijiang
Message-ID: <20240125024016.2521244-2-xiaoyao...@intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
(cherry picked from commit 10f92799af8ba3c3cef2352adcd4780f13fbab31)
Signed-off-by: Michael Tokarev
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 4ce80555b4..e68eb8f5e6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1914,6 +1914,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
case 0x1f:
if (env->nr_dies < 2) {
+cpuid_i--;
break;
}
/* fallthrough */
--
2.39.2
[RFC PATCH v2 16/22] hw/intc/arm_gicv3: Implement GICD_INMIR
Add GICD_INMIR0, GICD_INMIRnE register and support access GICD_INMIR0.
Signed-off-by: Jinjie Ruan
---
hw/intc/arm_gicv3_dist.c | 38 ++
hw/intc/gicv3_internal.h | 2 ++
2 files changed, 40 insertions(+)
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 35e850685c..2f7280c524 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -89,6 +89,29 @@ static int gicd_ns_access(GICv3State *s, int irq)
return extract32(s->gicd_nsacr[irq / 16], (irq % 16) * 2, 2);
}
+static void gicd_write_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
+ uint32_t *bmp, maskfn *maskfn,
+ int offset, uint32_t val)
+{
+/*
+ * Helper routine to implement writing to a "set" register
+ * (GICD_INMIR, etc).
+ * Semantics implemented here:
+ * RAZ/WI for SGIs, PPIs, unimplemented IRQs
+ * Bits corresponding to Group 0 or Secure Group 1 interrupts RAZ/WI.
+ * offset should be the offset in bytes of the register from the start
+ * of its group.
+ */
+int irq = offset * 8;
+
+if (irq < GIC_INTERNAL || irq >= s->num_irq) {
+return;
+}
+val &= mask_group_and_nsacr(s, attrs, maskfn, irq);
+*gic_bmp_ptr32(bmp, irq) = val;
+gicv3_update(s, irq, 32);
+}
+
static void gicd_write_set_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
uint32_t *bmp,
maskfn *maskfn,
@@ -543,6 +566,14 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
/* RAZ/WI since affinity routing is always enabled */
*data = 0;
return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+if (!s->nmi_support) {
+*data = 0;
+return true;
+}
+*data = gicd_read_bitmap_reg(s, attrs, s->superprio, NULL,
+ offset - GICD_INMIR);
+return true;
case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
{
uint64_t r;
@@ -752,6 +783,13 @@ static bool gicd_writel(GICv3State *s, hwaddr offset,
case GICD_SPENDSGIR ... GICD_SPENDSGIR + 0xf:
/* RAZ/WI since affinity routing is always enabled */
return true;
+case GICD_INMIR ... GICD_INMIR + 0x7f:
+if (!s->nmi_support) {
+return true;
+}
+gicd_write_bitmap_reg(s, attrs, s->superprio, NULL,
+ offset - GICD_INMIR, value);
+return true;
case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
{
uint64_t r;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index f35b7d2f03..a1fc34597e 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -52,6 +52,8 @@
#define GICD_SGIR0x0F00
#define GICD_CPENDSGIR 0x0F10
#define GICD_SPENDSGIR 0x0F20
+#define GICD_INMIR 0x0F80
+#define GICD_INMIRnE 0x3B00
#define GICD_IROUTER 0x6000
#define GICD_IDREGS 0xFFD0
--
2.34.1
[PATCH] docs/system: Fix key for input grab
Key for input grab should be Ctrl-Alt-g, not just Ctrl-Alt. Signed-off-by: Tianlan Zhou --- v1: - Initial patch --- docs/system/keys.rst.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/keys.rst.inc b/docs/system/keys.rst.inc index 2e2c97aa23..59966a3fe7 100644 --- a/docs/system/keys.rst.inc +++ b/docs/system/keys.rst.inc @@ -29,7 +29,7 @@ Ctrl-Alt-n *3* Serial port -Ctrl-Alt +Ctrl-Alt-g Toggle mouse and keyboard grab. In the virtual consoles, you can use Ctrl-Up, Ctrl-Down, Ctrl-PageUp and -- 2.38.1.windows.1
[Stable-8.2.2 39/60] tests/acpi: Update DSDT.cxl to reflect change _STA return value.
From: Jonathan Cameron _STA will now return 0xB (in common with most other devices) rather than not setting the bits to indicate this fake device has not been enabled, and self tests haven't passed. Signed-off-by: Jonathan Cameron Message-Id: <20240126120132.24248-13-jonathan.came...@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin (cherry picked from commit b24a981b9f1c4767aaea815e504a2c7aeb405d72) Signed-off-by: Michael Tokarev diff --git a/tests/data/acpi/q35/DSDT.cxl b/tests/data/acpi/q35/DSDT.cxl index 145301c52a..afcdc0d0ba 100644 Binary files a/tests/data/acpi/q35/DSDT.cxl and b/tests/data/acpi/q35/DSDT.cxl differ diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index 9ce0f596cc..dfb8523c8b 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1,2 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/q35/DSDT.cxl", -- 2.39.2
[PATCH 5/6] hw/pci-bridge/cxl_upstream: Fix missing ERRP_GUARD() in cxl_usp_realize()
From: Zhao Liu
As the comment in qapi/error, dereferencing @errp requires
ERRP_GUARD():
* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
* - It must not be dereferenced, because it may be null.
* - It should not be passed to error_prepend() or
* error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.
*
* Using it when it's not needed is safe, but please avoid cluttering
* the source with useless code.
Currently, since cxl_usp_realize() - as a PCIDeviceClass.realize()
method - doesn't get the NULL errp parameter, it doesn't trigger the
dereference issue.
To follow the requirement of errp, add missing ERRP_GUARD() in
cxl_usp_realize()().
Suggested-by: Markus Armbruster
Signed-off-by: Zhao Liu
---
Suggested by credit:
Markus: Referred his explanation about ERRP_GUARD().
---
hw/pci-bridge/cxl_upstream.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index e87eb4017713..03d123cca0ef 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -289,6 +289,7 @@ static void free_default_cdat_table(CDATSubHeader
**cdat_table, int num,
static void cxl_usp_realize(PCIDevice *d, Error **errp)
{
+ERRP_GUARD();
PCIEPort *p = PCIE_PORT(d);
CXLUpstreamPort *usp = CXL_USP(d);
CXLComponentState *cxl_cstate = &usp->cxl_cstate;
--
2.34.1
Re: [PATCH] migration: Fix qmp_query_migrate mbps value
Peter Xu writes:
> On Mon, Feb 19, 2024 at 04:44:57PM -0300, Fabiano Rosas wrote:
>> The QMP command query_migrate might see incorrect throughput numbers
>> if it runs after we've set the migration completion status but before
>> migration_calculate_complete() has updated s->total_time and s->mbps.
>>
>> The migration status would show COMPLETED, but the throughput value
>> would be the one from the last iteration and not the one from the
>> whole migration. This will usually be a larger value due to the time
>> period being smaller (one iteration).
>>
>> Move migration_calculate_complete() earlier so that the status
>> MIGRATION_STATUS_COMPLETED is only emitted after the final counters
>> update.
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> CI run: https://gitlab.com/farosas/qemu/-/pipelines/1182405776
>> ---
>> migration/migration.c | 10 ++
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index ab21de2cad..7486d59da0 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -102,6 +102,7 @@ static int migration_maybe_pause(MigrationState *s,
>> int new_state);
>> static void migrate_fd_cancel(MigrationState *s);
>> static bool close_return_path_on_source(MigrationState *s);
>> +static void migration_calculate_complete(MigrationState *s);
>>
>> static void migration_downtime_start(MigrationState *s)
>> {
>> @@ -2746,6 +2747,7 @@ static void migration_completion(MigrationState *s)
>> migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
>>MIGRATION_STATUS_COLO);
>> } else {
>> +migration_calculate_complete(s);
>> migrate_set_state(&s->state, current_active_state,
>>MIGRATION_STATUS_COMPLETED);
>> }
>> @@ -2784,6 +2786,7 @@ static void bg_migration_completion(MigrationState *s)
>> goto fail;
>> }
>>
>> +migration_calculate_complete(s);
>> migrate_set_state(&s->state, current_active_state,
>>MIGRATION_STATUS_COMPLETED);
>> return;
>> @@ -2993,12 +2996,15 @@ static void
>> migration_calculate_complete(MigrationState *s)
>> int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> int64_t transfer_time;
>>
>> +/* QMP could read from these concurrently */
>> +bql_lock();
>> migration_downtime_end(s);
>> s->total_time = end_time - s->start_time;
>> transfer_time = s->total_time - s->setup_time;
>> if (transfer_time) {
>> s->mbps = ((double) bytes * 8.0) / transfer_time / 1000;
>> }
>> +bql_unlock();
>
> The lock is not needed?
>
> AFAIU that was needed because of things like runstate_set() rather than
> setting of these fields.
>
Don't we need to keep the total_time and mbps update atomic? Otherwise
query-migrate might see (say) total_time=0 and mbps= or
total_time= and mbps=.
Also, what orders s->mbps update before the s->state update? I'd say we
should probably hold the lock around the whole total_time,mbps,state
update.
I'm not entirely sure, what do you think?
> See migration_update_counters() where it also updates mbps without holding
> a lock.
Here it might be less important since it's the middle of the migration,
there will proabably be more than one query-migrate which would see the
correct values.
>
> Other than that it looks all good.
>
>> }
>>
>> static void update_iteration_initial_status(MigrationState *s)
>> @@ -3145,7 +3151,6 @@ static void migration_iteration_finish(MigrationState
>> *s)
>> bql_lock();
>> switch (s->state) {
>> case MIGRATION_STATUS_COMPLETED:
>> -migration_calculate_complete(s);
>> runstate_set(RUN_STATE_POSTMIGRATE);
>> break;
>> case MIGRATION_STATUS_COLO:
>> @@ -3189,9 +3194,6 @@ static void
>> bg_migration_iteration_finish(MigrationState *s)
>> bql_lock();
>> switch (s->state) {
>> case MIGRATION_STATUS_COMPLETED:
>> -migration_calculate_complete(s);
>> -break;
>> -
>> case MIGRATION_STATUS_ACTIVE:
>> case MIGRATION_STATUS_FAILED:
>> case MIGRATION_STATUS_CANCELLED:
>> --
>> 2.35.3
>>
Re: [PATCH v3] pc: q35: Bump max_cpus to 4096 vcpus
Hi, > Looking at the edk2 GH, are these the PR that are waiting for upstream > review/merge that relate to vcpu scaling? > > https://github.com/tianocore/edk2/pull/5375 > https://github.com/tianocore/edk2/pull/5327 These are draft MRs for running CI. The current devel branches are: https://github.com/kraxel/edk2/tree/devel/many-vcpus https://github.com/kraxel/edk2/tree/devel/many-vcpus-mpinitlib All of them will expire at some point though, so I don't think it is a good idea to include them in the commit message. They will point into nowhere in a year or so. take care, Gerd
Re: [PATCH V4 2/5] qapi: QAPI_LIST_LENGTH
Steve Sistare writes:
> Signed-off-by: Steve Sistare
> Reviewed-by: Marc-André Lureau
> ---
> include/qapi/util.h | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 81a2b13..e1b8b1d 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -56,4 +56,17 @@ int parse_qapi_name(const char *name, bool complete);
> (tail) = &(*(tail))->next; \
> } while (0)
>
> +/*
> + * For any GenericList @list, return its length.
> + */
> +#define QAPI_LIST_LENGTH(list) \
> +({ \
> +int len = 0; \
size_t
> +typeof(list) elem; \
Name this @tail, please.
> +for (elem = list; elem != NULL; elem = elem->next) { \
> +len++; \
> +} \
> +len; \
> +})
> +
> #endif
This is a macro instead of a function so users don't have to cast their
FooList * to GenericList *.
The only user outside tests is strv_from_strList(). I'd be tempted to
open-code it there and call it a day. Or do you have more users in
mind?
If we keep the macro, please align the backslashes like this:
#define QAPI_LIST_LENGTH(list) \
({ \
int len = 0;\
typeof(list) elem; \
for (elem = list; elem != NULL; elem = elem->next) {\
len++; \
} \
len;\
})
[Stable-8.2.2 56/60] ui/clipboard: add asserts for update and request
From: Fiona Ebner
Should an issue like CVE-2023-6683 ever appear again in the future,
it will be more obvious which assumption was violated.
Suggested-by: Marc-André Lureau
Signed-off-by: Fiona Ebner
Reviewed-by: Marc-André Lureau
Message-ID: <20240124105749.204610-2-f.eb...@proxmox.com>
(cherry picked from commit 9c416582611b7495bdddb4c5456c7acb64b78938)
Signed-off-by: Michael Tokarev
diff --git a/ui/clipboard.c b/ui/clipboard.c
index b3f6fa3c9e..4264884a6c 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -65,12 +65,24 @@ bool qemu_clipboard_check_serial(QemuClipboardInfo *info,
bool client)
void qemu_clipboard_update(QemuClipboardInfo *info)
{
+uint32_t type;
QemuClipboardNotify notify = {
.type = QEMU_CLIPBOARD_UPDATE_INFO,
.info = info,
};
assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
+for (type = 0; type < QEMU_CLIPBOARD_TYPE__COUNT; type++) {
+/*
+ * If data is missing, the clipboard owner's 'request' callback needs
to
+ * be set. Otherwise, there is no way to get the clipboard data and
+ * qemu_clipboard_request() cannot be called.
+ */
+if (info->types[type].available && !info->types[type].data) {
+assert(info->owner && info->owner->request);
+}
+}
+
notifier_list_notify(&clipboard_notifiers, ¬ify);
if (cbinfo[info->selection] != info) {
@@ -132,6 +144,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info,
!info->owner)
return;
+assert(info->owner->request);
+
info->types[type].requested = true;
info->owner->request(info, type);
}
--
2.39.2
Re: [PATCH] qga-win: Add support of Windows Server 2025 in get-osinfo command
On Wed, Feb 21, 2024 at 11:51 AM Dehan Meng wrote:
> Add support of Windows Server 2025 in get-osinfo command
>
> Signed-off-by: Dehan Meng
> ---
> qga/commands-win32.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 697c65507c..f3c7e604c9 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2154,6 +2154,7 @@ static ga_win_10_0_t const
> WIN_10_0_SERVER_VERSION_MATRIX[4] = {
>
You don't update the array size, there are out of range elements
> {14393, "Microsoft Windows Server 2016","2016"},
> {17763, "Microsoft Windows Server 2019","2019"},
> {20344, "Microsoft Windows Server 2022","2022"},
> +{26040, "MIcrosoft Windows Server 2025","2025"},
> {0, 0}
> };
>
> --
> 2.35.1
>
>
[RFC PATCH v2 17/22] hw/intc: Enable FEAT_GICv3_NMI Feature
Added properties to enable FEAT_GICv3_NMI feature, setup distributor
and redistributor registers to indicate NMI support.
Signed-off-by: Jinjie Ruan
---
hw/intc/arm_gicv3_common.c | 1 +
hw/intc/arm_gicv3_dist.c | 2 ++
hw/intc/gicv3_internal.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
4 files changed, 5 insertions(+)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 6249c3edc9..9abbe9b71d 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -566,6 +566,7 @@ static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
+DEFINE_PROP_BOOL("has-nmi", GICv3State, nmi_support, 0),
DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
/*
* Compatibility property: force 8 bits of physical priority, even
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 2f7280c524..65e7ca29cf 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -412,6 +412,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
* by GICD_TYPER.IDbits)
* MBIS == 0 (message-based SPIs not supported)
* SecurityExtn == 1 if security extns supported
+ * NMI = 1 if Non-maskable interrupt property is supported
* CPUNumber == 0 since for us ARE is always 1
* ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
*/
@@ -425,6 +426,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
bool dvis = s->revision >= 4;
*data = (1 << 25) | (1 << 24) | (dvis << 18) | (sec_extn << 10) |
+(s->nmi_support << GICD_TYPER_NMI_SHIFT) |
(s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
(0xf << 19) | itlinesnumber;
return true;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index a1fc34597e..8d793243f4 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -70,6 +70,7 @@
#define GICD_CTLR_E1NWF (1U << 7)
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_TYPER_NMI_SHIFT 9
#define GICD_TYPER_LPIS_SHIFT 17
/* 16 bits EventId */
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 1eb8c39239..c9d31c7c7d 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -248,6 +248,7 @@ struct GICv3State {
uint32_t num_irq;
uint32_t revision;
bool lpi_enable;
+bool nmi_support;
bool security_extn;
bool force_8bit_prio;
bool irq_reset_nonsecure;
--
2.34.1
[Stable-8.2.2 58/60] audio: Depend on dbus_display1_dep
From: Akihiko Odaki
dbusaudio needs dbus_display1_dep.
Fixes: 739362d4205c ("audio: add "dbus" audio backend")
Signed-off-by: Akihiko Odaki
Reviewed-by: Marc-André Lureau
Message-Id: <20240214-dbus-v7-1-7eff29f04...@daynix.com>
(cherry picked from commit d67611907590a1e6c998b7c5a5cb4394acf84329)
Signed-off-by: Michael Tokarev
diff --git a/audio/meson.build b/audio/meson.build
index c8f658611f..608f35e6af 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -30,7 +30,8 @@ endforeach
if dbus_display
module_ss = ss.source_set()
-module_ss.add(when: [gio, pixman], if_true: files('dbusaudio.c'))
+module_ss.add(when: [gio, dbus_display1_dep, pixman],
+ if_true: files('dbusaudio.c'))
audio_modules += {'dbus': module_ss}
endif
--
2.39.2
Re: [PATCH 2/2] migration/multifd: Drop registered_yank
Peter Xu writes:
> On Thu, Feb 08, 2024 at 09:48:33AM -0300, Fabiano Rosas wrote:
>> pet...@redhat.com writes:
>>
>> > From: Peter Xu
>> >
>> > With a clear definition of p->c protocol, where we only set it up if the
>> > channel is fully established (TLS or non-TLS), registered_yank boolean will
>> > have equal meaning of "p->c != NULL".
>> >
>> > Drop registered_yank by checking p->c instead.
>> >
>> > Reviewed-by: Fabiano Rosas
>> > Signed-off-by: Peter Xu
>> > ---
>> > migration/multifd.h | 2 --
>> > migration/multifd.c | 7 +++
>> > 2 files changed, 3 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/migration/multifd.h b/migration/multifd.h
>> > index 8a1cad0996..b3fe27ae93 100644
>> > --- a/migration/multifd.h
>> > +++ b/migration/multifd.h
>> > @@ -78,8 +78,6 @@ typedef struct {
>> > bool tls_thread_created;
>> > /* communication channel */
>> > QIOChannel *c;
>> > -/* is the yank function registered */
>> > -bool registered_yank;
>> > /* packet allocated len */
>> > uint32_t packet_len;
>> > /* guest page size */
>> > diff --git a/migration/multifd.c b/migration/multifd.c
>> > index 4a85a6b7b3..278453cf84 100644
>> > --- a/migration/multifd.c
>> > +++ b/migration/multifd.c
>> > @@ -648,11 +648,11 @@ static int multifd_send_channel_destroy(QIOChannel
>> > *send)
>> >
>> > static bool multifd_send_cleanup_channel(MultiFDSendParams *p, Error
>> > **errp)
>> > {
>> > -if (p->registered_yank) {
>> > +if (p->c) {
>> > migration_ioc_unregister_yank(p->c);
>> > +multifd_send_channel_destroy(p->c);
>>
>> At socket_send_channel_destroy the clean up of outgoing_args.saddr will
>> now be skipped. The failure at multifd_new_send_channel_async might have
>> been due to TLS, in which case all of plain socket setup will have
>> happened properly.
>
> Right, IMHO it's a hack to free globals in a per-channel helper. We should
> have moved:
>
> if (outgoing_args.saddr) {
> qapi_free_SocketAddress(outgoing_args.saddr);
> outgoing_args.saddr = NULL;
> }
>
> Outside irrelevant of that..
>
> That could be done later I guess, because we have one more guard:
>
> socket_start_outgoing_migration():
> /* in case previous migration leaked it */
> qapi_free_SocketAddress(outgoing_args.saddr);
> outgoing_args.saddr = addr;
>
> If you think proper, I can add one more patch to do that cleanup, IOW, move
> above free() into multifd_send_cleanup_state().
Sounds good.
>
> Thanks,
[PATCH 3/6] hw/mem/cxl_type3: Fix missing ERRP_GUARD() in ct3_realize()
From: Zhao Liu
As the comment in qapi/error, dereferencing @errp requires
ERRP_GUARD():
* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
* - It must not be dereferenced, because it may be null.
* - It should not be passed to error_prepend() or
* error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.
*
* Using it when it's not needed is safe, but please avoid cluttering
* the source with useless code.
Currently, since ct3_realize() - as a PCIDeviceClass.realize() method -
doesn't get the NULL errp parameter, it doesn't trigger the dereference
issue.
To follow the requirement of errp, add missing ERRP_GUARD() in
ct3_realize().
Suggested-by: Markus Armbruster
Signed-off-by: Zhao Liu
---
Suggested by credit:
Markus: Referred his explanation about ERRP_GUARD().
---
hw/mem/cxl_type3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index e8801805b90f..a3b0761f843b 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -645,6 +645,7 @@ static DOEProtocol doe_cdat_prot[] = {
static void ct3_realize(PCIDevice *pci_dev, Error **errp)
{
+ERRP_GUARD();
CXLType3Dev *ct3d = CXL_TYPE3(pci_dev);
CXLComponentState *cxl_cstate = &ct3d->cxl_cstate;
ComponentRegisters *regs = &cxl_cstate->crb;
--
2.34.1
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Peter Xu writes: > Thanks, Markus. [...] > Reviewed-by: Peter Xu > > Markus, do you want us to pick it up, or let it go via qapi? I can stick it into my next qapi PR in a few days, if you guys don't beat me to the punch.
Re: [PATCH v9 4/5] qmp: Added new command to retrieve eBPF blob.
Commit message style nitpick:
qmp: Add command to retrieve eBPF blob
or
qmp: New command to retrieve eBPF blob
It's a title, not a sentence.
Andrew Melnychenko writes:
> Now, the binary objects may be retrieved by id.
> It would require for future qmp commands that may require specific
> eBPF blob.
>
> Added command "request-ebpf". This command returns
> eBPF program encoded base64. The program taken from the
> skeleton and essentially is an ELF object that can be
> loaded in the future with libbpf.
>
> The reason to use the command to provide the eBPF object
> instead of a separate artifact was to avoid issues related
> to finding the eBPF itself. eBPF object is an ELF binary
> that contains the eBPF program and eBPF map description(BTF).
> Overall, eBPF object should contain the program and enough
> metadata to create/load eBPF with libbpf. As the eBPF
> maps/program should correspond to QEMU, the eBPF can't
> be used from different QEMU build.
>
> The first solution was a helper that comes with QEMU
> and loads appropriate eBPF objects. And the issue is
> to find a proper helper if the system has several
> different QEMUs installed and/or built from the source,
> which helpers may not be compatible.
>
> Another issue is QEMU updating while there is a running
> QEMU instance. With an updated helper, it may not be
> possible to hotplug virtio-net device to the already
> running QEMU. Overall, requesting the eBPF object from
> QEMU itself solves possible failures with acceptable effort.
>
> Links:
> [PATCH 3/5] qmp: Added the helper stamp check.
> https://lore.kernel.org/all/20230219162100.174318-4-and...@daynix.com/
>
> Signed-off-by: Andrew Melnychenko
> ---
> ebpf/ebpf.c | 69 +++
> ebpf/ebpf.h | 29 ++
> ebpf/ebpf_rss.c | 6
> ebpf/meson.build | 2 +-
> qapi/ebpf.json| 66 +
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> 7 files changed, 173 insertions(+), 1 deletion(-)
> create mode 100644 ebpf/ebpf.c
> create mode 100644 ebpf/ebpf.h
> create mode 100644 qapi/ebpf.json
>
> diff --git a/ebpf/ebpf.c b/ebpf/ebpf.c
> new file mode 100644
> index 00..2d73beb479
> --- /dev/null
> +++ b/ebpf/ebpf.c
> @@ -0,0 +1,69 @@
> +/*
> + * QEMU eBPF binary declaration routine.
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Andrew Melnychenko
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/queue.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-ebpf.h"
> +#include "ebpf/ebpf.h"
> +
> +typedef struct ElfBinaryDataEntry {
> +int id;
> +const void *data;
> +size_t datalen;
> +
> +QSLIST_ENTRY(ElfBinaryDataEntry) node;
> +} ElfBinaryDataEntry;
> +
> +static QSLIST_HEAD(, ElfBinaryDataEntry) ebpf_elf_obj_list =
> +QSLIST_HEAD_INITIALIZER();
> +
> +void ebpf_register_binary_data(int id, const void *data, size_t datalen)
> +{
> +struct ElfBinaryDataEntry *dataentry = NULL;
> +
> +dataentry = g_new0(struct ElfBinaryDataEntry, 1);
> +dataentry->data = data;
> +dataentry->datalen = datalen;
> +dataentry->id = id;
> +
> +QSLIST_INSERT_HEAD(&ebpf_elf_obj_list, dataentry, node);
> +}
> +
> +const void *ebpf_find_binary_by_id(int id, size_t *sz, Error **errp)
> +{
> +struct ElfBinaryDataEntry *it = NULL;
> +QSLIST_FOREACH(it, &ebpf_elf_obj_list, node) {
> +if (id == it->id) {
> +*sz = it->datalen;
> +return it->data;
> +}
> +}
> +
> +error_setg(errp, "can't find eBPF object with id: %d", id);
> +
> +return NULL;
> +}
> +
> +EbpfObject *qmp_request_ebpf(EbpfProgramID id, Error **errp)
> +{
> +EbpfObject *ret = NULL;
> +size_t size = 0;
> +const void *data = ebpf_find_binary_by_id(id, &size, errp);
> +if (!data) {
> +return NULL;
> +}
> +
> +ret = g_new0(EbpfObject, 1);
> +ret->object = g_base64_encode(data, size);
> +
> +return ret;
> +}
> diff --git a/ebpf/ebpf.h b/ebpf/ebpf.h
> new file mode 100644
> index 00..378d4e9c70
> --- /dev/null
> +++ b/ebpf/ebpf.h
> @@ -0,0 +1,29 @@
> +/*
> + * QEMU eBPF binary declaration routine.
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Andrew Melnychenko
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef EBPF_H
> +#define EBPF_H
> +
> +
> +void ebpf_register_binary_data(int id, const void *data,
> + size_t datalen);
> +const void *ebpf_find_binary_by_id(int id, size_t *sz,
> + struct Error **errp);
> +
> +#define ebpf_binary_init(id, fn) \
> +static void __attribute__((constructor)) ebpf_binary_init_ ## fn(void) \
> +
Re: [PATCH 2/9] hw/i386/pc_piix: Share pc_cmos_init() invocation between pc and isapc machines
On 8/2/24 23:03, Bernhard Beschow wrote: Both invocations are the same and either one is always executed. Avoid this redundancy. Signed-off-by: Bernhard Beschow --- hw/i386/pc_piix.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[Stable-8.2.2 40/60] linux-user/aarch64: Choose SYNC as the preferred MTE mode
From: Richard Henderson
The API does not generate an error for setting ASYNC | SYNC; that merely
constrains the selection vs the per-cpu default. For qemu linux-user,
choose SYNC as the default.
Cc: qemu-sta...@nongnu.org
Reported-by: Gustavo Romero
Signed-off-by: Richard Henderson
Tested-by: Gustavo Romero
Message-id: 20240207025210.8837-2-richard.hender...@linaro.org
Signed-off-by: Peter Maydell
(cherry picked from commit 681dfc0d552963d4d598350d26097a692900b408)
Signed-off-by: Michael Tokarev
diff --git a/linux-user/aarch64/target_prctl.h
b/linux-user/aarch64/target_prctl.h
index 5067e7d731..aa8e203c15 100644
--- a/linux-user/aarch64/target_prctl.h
+++ b/linux-user/aarch64/target_prctl.h
@@ -173,21 +173,26 @@ static abi_long
do_prctl_set_tagged_addr_ctrl(CPUArchState *env, abi_long arg2)
env->tagged_addr_enable = arg2 & PR_TAGGED_ADDR_ENABLE;
if (cpu_isar_feature(aa64_mte, cpu)) {
-switch (arg2 & PR_MTE_TCF_MASK) {
-case PR_MTE_TCF_NONE:
-case PR_MTE_TCF_SYNC:
-case PR_MTE_TCF_ASYNC:
-break;
-default:
-return -EINVAL;
-}
-
/*
* Write PR_MTE_TCF to SCTLR_EL1[TCF0].
- * Note that the syscall values are consistent with hw.
+ *
+ * The kernel has a per-cpu configuration for the sysadmin,
+ * /sys/devices/system/cpu/cpu/mte_tcf_preferred,
+ * which qemu does not implement.
+ *
+ * Because there is no performance difference between the modes, and
+ * because SYNC is most useful for debugging MTE errors, choose SYNC
+ * as the preferred mode. With this preference, and the way the API
+ * uses only two bits, there is no way for the program to select
+ * ASYMM mode.
*/
-env->cp15.sctlr_el[1] =
-deposit64(env->cp15.sctlr_el[1], 38, 2, arg2 >> PR_MTE_TCF_SHIFT);
+unsigned tcf = 0;
+if (arg2 & PR_MTE_TCF_SYNC) {
+tcf = 1;
+} else if (arg2 & PR_MTE_TCF_ASYNC) {
+tcf = 2;
+}
+env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
/*
* Write PR_MTE_TAG to GCR_EL1[Exclude].
--
2.39.2
[PATCH 1/1] tests/9p: add 'use-after-unlink' test
After removing a file from the file system, we should still be able to
do I/O on the file if we already had it open before removal.
Signed-off-by: Christian Schoenebeck
---
OK, this was a bit surprising to me. I was expecting this test to fail,
but it works!
tests/qtest/virtio-9p-test.c | 41
1 file changed, 41 insertions(+)
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 65e69491e5..7638c0a183 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -693,6 +693,45 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
g_assert(stat(real_file, &st_real) == 0);
}
+static void fs_use_after_unlink(void *obj, void *data,
+QGuestAllocator *t_alloc)
+{
+QVirtio9P *v9p = obj;
+v9fs_set_allocator(t_alloc);
+static const uint32_t write_count = P9_MAX_SIZE / 2;
+g_autofree char *real_file = virtio_9p_test_path("09/doa_file");
+g_autofree char *buf = g_malloc0(write_count);
+struct stat st_file;
+uint32_t fid_file;
+uint32_t count;
+
+tattach({ .client = v9p });
+
+/* create a file "09/doa_file" and make sure it exists and is regular */
+tmkdir({ .client = v9p, .atPath = "/", .name = "09" });
+tlcreate({ .client = v9p, .atPath = "09", .name = "doa_file" });
+g_assert(stat(real_file, &st_file) == 0);
+g_assert((st_file.st_mode & S_IFMT) == S_IFREG);
+
+/* request a FID for that regular file that we can work with next */
+fid_file = twalk({
+.client = v9p, .fid = 0, .path = "09/doa_file"
+}).newfid;
+g_assert(fid_file != 0);
+
+/* now first open the file in write mode before ... */
+tlopen({ .client = v9p, .fid = fid_file, .flags = O_WRONLY });
+/* ... removing the file from file system */
+tunlinkat({ .client = v9p, .atPath = "09", .name = "doa_file" });
+
+/* file is removed, but we still have it open, so this should succeed */
+count = twrite({
+.client = v9p, .fid = fid_file, .offset = 0, .count = write_count,
+.data = buf
+}).count;
+g_assert_cmpint(count, ==, write_count);
+}
+
static void *assign_9p_local_driver(GString *cmd_line, void *arg)
{
virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -756,6 +795,8 @@ static void register_virtio_9p_test(void)
qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts);
qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink,
&opts);
+qos_add_test("local/use_after_unlink", "virtio-9p", fs_use_after_unlink,
+ &opts);
}
libqos_init(register_virtio_9p_test);
--
2.30.2
[RFC PATCH v2 03/22] target/arm: Add PSTATE.ALLINT
The ALLINT bit in PSTATE is used to mask all IRQ or FIQ interrupts.
Place this in its own field within ENV, as that will
make it easier to reset from within TCG generated code.
With the change to pstate_read/write, exception entry
and return are automatically handled.
Signed-off-by: Jinjie Ruan
---
target/arm/cpu.c | 3 +++
target/arm/cpu.h | 9 +++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5fa86bc8d5..5e5978c302 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1104,6 +1104,9 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
if (cpu_isar_feature(aa64_bti, cpu)) {
qemu_fprintf(f, " BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
}
+if (cpu_isar_feature(aa64_nmi, cpu)) {
+qemu_fprintf(f, " ALLINT=%d", (psr & PSTATE_ALLINT) >> 13);
+}
qemu_fprintf(f, "%s%s%s",
(hcr & HCR_NV) ? " NV" : "",
(hcr & HCR_NV1) ? " NV1" : "",
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 63f31e0d98..f9646dbbfb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -224,6 +224,7 @@ typedef struct CPUArchState {
*semantics as for AArch32, as described in the comments on each field)
* nRW (also known as M[4]) is kept, inverted, in env->aarch64
* DAIF (exception masks) are kept in env->daif
+ * ALLINT (all IRQ or FIQ interrupts masks) are kept in env->allint
* BTYPE is kept in env->btype
* SM and ZA are kept in env->svcr
* all other bits are stored in their correct places in env->pstate
@@ -261,6 +262,7 @@ typedef struct CPUArchState {
uint32_t btype; /* BTI branch type. spsr[11:10]. */
uint64_t daif; /* exception masks, in the bits they are in PSTATE */
uint64_t svcr; /* PSTATE.{SM,ZA} in the bits they are in SVCR */
+uint64_t allint; /* All IRQ or FIQ interrupt mask, in the bit in PSTATE */
uint64_t elr_el[4]; /* AArch64 exception link regs */
uint64_t sp_el[4]; /* AArch64 banked stack pointers */
@@ -1543,6 +1545,7 @@ FIELD(VTCR, SL2, 33, 1)
#define PSTATE_D (1U << 9)
#define PSTATE_BTYPE (3U << 10)
#define PSTATE_SSBS (1U << 12)
+#define PSTATE_ALLINT (1U << 13)
#define PSTATE_IL (1U << 20)
#define PSTATE_SS (1U << 21)
#define PSTATE_PAN (1U << 22)
@@ -1555,7 +1558,8 @@ FIELD(VTCR, SL2, 33, 1)
#define PSTATE_N (1U << 31)
#define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
-#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF | PSTATE_BTYPE)
+#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF | PSTATE_BTYPE | \
+PSTATE_ALLINT)
/* Mode values for AArch64 */
#define PSTATE_MODE_EL3h 13
#define PSTATE_MODE_EL3t 12
@@ -1595,7 +1599,7 @@ static inline uint32_t pstate_read(CPUARMState *env)
ZF = (env->ZF == 0);
return (env->NF & 0x8000) | (ZF << 30)
| (env->CF << 29) | ((env->VF & 0x8000) >> 3)
-| env->pstate | env->daif | (env->btype << 10);
+| env->pstate | env->allint | env->daif | (env->btype << 10);
}
static inline void pstate_write(CPUARMState *env, uint32_t val)
@@ -1604,6 +1608,7 @@ static inline void pstate_write(CPUARMState *env,
uint32_t val)
env->NF = val;
env->CF = (val >> 29) & 1;
env->VF = (val << 3) & 0x8000;
+env->allint = val & PSTATE_ALLINT;
env->daif = val & PSTATE_DAIF;
env->btype = (val >> 10) & 3;
env->pstate = val & ~CACHED_PSTATE_BITS;
--
2.34.1
Re: [PATCH v5 2/7] trans_rvv.c.inc: remove 'is_store' bool from load/store fns
On 21/2/24 03:22, Daniel Henrique Barboza wrote: After the 'mark_vs_dirty' changes from the previous patch the 'is_store' bool is unused in all load/store functions that were changed. Remove it. Signed-off-by: Daniel Henrique Barboza --- target/riscv/insn_trans/trans_rvv.c.inc | 69 - 1 file changed, 34 insertions(+), 35 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[Stable-8.2.2 59/60] meson: Explicitly specify dbus-display1.h dependency
From: Akihiko Odaki
Explicitly specify dbus-display1.h as a dependency so that files
depending on it will not get compiled too early.
Fixes: 1222070e7728 ("meson: ensure dbus-display generated code is built before
other units")
Signed-off-by: Akihiko Odaki
Reviewed-by: Marc-André Lureau
Message-Id: <20240214-dbus-v7-2-7eff29f04...@daynix.com>
(cherry picked from commit 7aee57df930da2cf6361c5183aff96468ae4027d)
Signed-off-by: Michael Tokarev
diff --git a/ui/meson.build b/ui/meson.build
index 0ccb3387ee..0f09d31c60 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -92,7 +92,7 @@ if dbus_display
'--c-namespace', 'QemuDBus',
'--generate-c-code', '@BASENAME@'])
dbus_display1_lib = static_library('dbus-display1', dbus_display1,
dependencies: gio)
- dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib,
include_directories: include_directories('.'))
+ dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib,
sources: dbus_display1[0])
dbus_ss.add(when: [gio, dbus_display1_dep],
if_true: [files(
'dbus-chardev.c',
--
2.39.2
[PATCH v4 0/3] Adjust the output of x-query-virtio-status
v4: - Rebase on master - Fix the syntax mistake within the commit message of [PATCH v3 1/3] - Adjust the linking file in hw/virtio/meson.build suggested by Markus Please review, Yong v3: - Rebase on master - Use the refined commit message furnished by Markus for [PATCH v2 1/2] - Drop the [PATCH v2 2/2] - Add [PATCH v3 2/3] to declare the decoding functions to static - Add [PATCH v3 3/3] to Define VhostDeviceProtocols and VirtioDeviceFeatures as plain C types v2: - Changing the hmp_virtio_dump_xxx function signatures to implement the bitmap decoding, suggested by Philippe. This patchset is derived from the series: https://lore.kernel.org/qemu-devel/cover.1699793550.git.yong.hu...@smartx.com/ Please go to the link to see more background information. The following points are what we have done in the patchset: 1. Take the policy of adding human-readable output just in HMP. 2. For the HMP output, display the human-readable information and drop the unknown bits in practice. 3. For the QMP output, remove the descriptive strings and only display bits encoded as numbers. Hyman Huang (3): qmp: Switch x-query-virtio-status back to numeric encoding virtio: Declare the decoding functions to static qapi: Define VhostDeviceProtocols and VirtioDeviceFeatures as plain C types hw/virtio/meson.build | 4 +- hw/virtio/virtio-hmp-cmds.c | 702 +++- hw/virtio/virtio-qmp.c | 684 +-- hw/virtio/virtio-qmp.h | 3 - qapi/virtio.json| 231 +--- 5 files changed, 724 insertions(+), 900 deletions(-) -- 2.39.3
Re: [PATCH] vhost-user: fix the issue of vhost deadlock in nested virtualization
在 2024/2/21 17:02, Maxime Coquelin 写道:
On 2/20/24 12:43, Michael S. Tsirkin wrote:
On Tue, Feb 20, 2024 at 12:26:49PM +0100, Maxime Coquelin wrote:
On 2/13/24 11:05, Michael S. Tsirkin wrote:
On Fri, Jan 26, 2024 at 06:07:37PM +0800, Hao Chen wrote:
I run "dpdk-vdpa" and "qemur-L2" in "qemu-L1".
In a nested virtualization environment, "qemu-L2" vhost-user socket
sends
a "VHOST_USER_IOTLB_MSG" message to "dpdk-vdpa" and blocks waiting for
"dpdk-vdpa" to process the message.
If "dpdk-vdpa" doesn't complete the processing of the
"VHOST_USER_IOTLB_MSG"
message and sends a message that needs to be replied in another
thread,
such as "VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG", "dpdk-vdpa"
will also
block and wait for "qemu-L2" to process this message. However,
"qemu-L2"
vhost-user's socket is blocking while waiting for a reply from
"dpdk-vdpa"
after processing the message "VHOSTr_USER_IOTLB_MSG", and
"VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG" will not be processed.
In this case, both "dpdk-vdpa" and "qemu-L2" are blocked on the
vhost read, resulting in a deadlock.
You can modify "VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG" or
"VHOST_USER_IOTLB_MSG" to "no need reply" to fix this issue.
There are too many messages in dpdk that are similar to
"VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG", and I would prefer the
latter.
Fixes: 24e34754eb78 ("vhost-user: factor out msg head and payload")
Signed-off-by: Hao Chen
I would be very worried that IOTLB becomes stale and
guest memory is corrupted if we just proceed without waiting.
Maxime what do you think? How would you address the issue?
I agree with you, this is not possible.
For example, in case of IOTLB invalidate, the frontend relies on the
backend reply to ensure it is no more accessing the memory before
proceeding.
The reply-ack for VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG request is
less important, if it fails the host notifications won't work but would
not risk corruption. Maybe on Qemu side we could fail init if processing
the request fails, as I think that if negotiated, we can expect it to
succeed.
What do you think about this proposal?
Regards,
Maxime
Fundamentally, I think that if qemu blocks guest waiting for a rely
that is ok but it really has to process incoming messages meanwhile.
Same should apply to backend I think ...
I understand your point.
For DPDK Vhost library, it will likely imply ABI breakage as it would
require to asynchronous handling of Vhost-user requests. We would only
be able to do it at next LTS release.
Hao, as your driver is not available upstream it will be difficult to
assist you more. But if you look at other DPDK vDPA driver like SFC for
instance, the way they implemented host notification control should be
safe against this kind of deadlocks.
Okay, I can also avoid this issue by sending the
"VHOST_USER_SLAVE_VRING_HOSTNOTIFIER_MSG" message as late as possible to
avoid conflicts with the "VHOST-USER-IOTLB-MSG" message. In summary,
thank you.
---
hw/virtio/vhost-user.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index f214df804b..02caa94b6c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2371,20 +2371,14 @@ static int vhost_user_net_set_mtu(struct
vhost_dev *dev, uint16_t mtu)
static int vhost_user_send_device_iotlb_msg(struct vhost_dev *dev,
struct
vhost_iotlb_msg *imsg)
{
- int ret;
VhostUserMsg msg = {
.hdr.request = VHOST_USER_IOTLB_MSG,
.hdr.size = sizeof(msg.payload.iotlb),
- .hdr.flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+ .hdr.flags = VHOST_USER_VERSION,
.payload.iotlb = *imsg,
};
- ret = vhost_user_write(dev, &msg, NULL, 0);
- if (ret < 0) {
- return ret;
- }
-
- return process_message_reply(dev, &msg);
+ return vhost_user_write(dev, &msg, NULL, 0);
}
--
2.27.0
[Stable-8.2.2 48/60] .gitlab-ci/windows.yml: Don't install libusb or spice packages on 32-bit
From: Peter Maydell
When msys2 updated their libusb packages to libusb 1.0.27, they
dropped support for building them for mingw32, leaving only mingw64
packages. This broke our CI job, as the 'pacman' package install now
fails with:
error: target not found: mingw-w64-i686-libusb
error: target not found: mingw-w64-i686-usbredir
(both these binary packages are from the libusb source package).
Similarly, spice is now 64-bit only:
error: target not found: mingw-w64-i686-spice
Fix this by dropping these packages from the list we install for our
msys2-32bit build. We do this with a simple mechanism for the
msys2-64bit and msys2-32bit jobs to specify a list of extra packages
to install on top of the common ones we install for both jobs.
Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2160
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael Tokarev
Message-id: 20240215155009.2422335-1-peter.mayd...@linaro.org
(cherry picked from commit 8e31b744fdf2c5d933681e4128acee72a83af4b8)
Signed-off-by: Michael Tokarev
diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index f7645f72b7..5c1e385dc8 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -88,7 +88,6 @@
$MINGW_TARGET-libpng
$MINGW_TARGET-libssh
$MINGW_TARGET-libtasn1
- $MINGW_TARGET-libusb
$MINGW_TARGET-lzo2
$MINGW_TARGET-nettle
$MINGW_TARGET-ninja
@@ -98,9 +97,8 @@
$MINGW_TARGET-SDL2
$MINGW_TARGET-SDL2_image
$MINGW_TARGET-snappy
- $MINGW_TARGET-spice
- $MINGW_TARGET-usbredir
- $MINGW_TARGET-zstd "
+ $MINGW_TARGET-zstd
+ $EXTRA_PACKAGES "
- Write-Output "Running build at $(Get-Date -Format u)"
- $env:CHERE_INVOKING = 'yes' # Preserve the current working directory
- $env:MSYS = 'winsymlinks:native' # Enable native Windows symlink
@@ -123,6 +121,8 @@ msys2-64bit:
variables:
MINGW_TARGET: mingw-w64-x86_64
MSYSTEM: MINGW64
+# msys2 only ship these packages for 64-bit, not 32-bit
+EXTRA_PACKAGES: $MINGW_TARGET-libusb $MINGW_TARGET-usbredir
$MINGW_TARGET-spice
# do not remove "--without-default-devices"!
# commit 9f8e6cad65a6 ("gitlab-ci: Speed up the msys2-64bit job by using
--without-default-devices"
# changed to compile QEMU with the --without-default-devices switch
@@ -137,5 +137,6 @@ msys2-32bit:
variables:
MINGW_TARGET: mingw-w64-i686
MSYSTEM: MINGW32
+EXTRA_PACKAGES:
CONFIGURE_ARGS: --target-list=ppc64-softmmu -Ddebug=false -Doptimization=0
TEST_ARGS: --no-suite qtest
--
2.39.2
Re: [PATCH v4 30/34] monitor: Honor QMP request for fd removal immediately
Fabiano Rosas writes:
> We're currently only removing an fd from the fdset if the VM is
> running. This causes a QMP call to "remove-fd" to not actually remove
> the fd if the VM happens to be stopped.
>
> While the fd would eventually be removed when monitor_fdset_cleanup()
> is called again, the user request should be honored and the fd
> actually removed. Calling remove-fd + query-fdset shows a recently
> removed fd still present.
>
> The runstate_is_running() check was introduced by commit ebe52b592d
> ("monitor: Prevent removing fd from set during init"), which by the
> shortlog indicates that they were trying to avoid removing an
> yet-unduplicated fd too early.
>
> I don't see why an fd explicitly removed with qmp_remove_fd() should
> be under runstate_is_running(). I'm assuming this was a mistake when
> adding the parenthesis around the expression.
>
> Move the runstate_is_running() check to apply only to the
> QLIST_EMPTY(dup_fds) side of the expression and ignore it when
> mon_fdset_fd->removed has been explicitly set.
>
> Signed-off-by: Fabiano Rosas
Eric, Kevin, your fingerprints are on commit ebe52b592d. Could you have
a look at this fix?
> ---
> monitor/fds.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/monitor/fds.c b/monitor/fds.c
> index d86c2c674c..4ec3b7eea9 100644
> --- a/monitor/fds.c
> +++ b/monitor/fds.c
> @@ -173,9 +173,9 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset)
> MonFdsetFd *mon_fdset_fd_next;
>
> QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next,
> mon_fdset_fd_next) {
> -if ((mon_fdset_fd->removed ||
> -(QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) &&
> -runstate_is_running()) {
> +if (mon_fdset_fd->removed ||
> +(QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0 &&
> + runstate_is_running())) {
> close(mon_fdset_fd->fd);
> g_free(mon_fdset_fd->opaque);
> QLIST_REMOVE(mon_fdset_fd, next);
Re: [PATCH 2/6] hw/display/macfb: Fix missing ERRP_GUARD() in macfb_nubus_realize()
Zhao Liu writes:
> From: Zhao Liu
>
> As the comment in qapi/error, dereferencing @errp requires
> ERRP_GUARD():
>
> * = Why, when and how to use ERRP_GUARD() =
> *
> * Without ERRP_GUARD(), use of the @errp parameter is restricted:
> * - It must not be dereferenced, because it may be null.
> * - It should not be passed to error_prepend() or
> * error_append_hint(), because that doesn't work with &error_fatal.
> * ERRP_GUARD() lifts these restrictions.
> *
> * To use ERRP_GUARD(), add it right at the beginning of the function.
> * @errp can then be used without worrying about the argument being
> * NULL or &error_fatal.
> *
> * Using it when it's not needed is safe, but please avoid cluttering
> * the source with useless code.
>
> Currently, since macfb_nubus_realize() - as a DeviceClass.realize()
> method - doesn't get the NULL errp parameter, it doesn't trigger the
> dereference issue.
>
> To follow the requirement of errp, add missing ERRP_GUARD() in
> macfb_nubus_realize().
>
> Suggested-by: Markus Armbruster
> Signed-off-by: Zhao Liu
> ---
> Suggested by credit:
> Markus: Referred his explanation about ERRP_GUARD().
> ---
> hw/display/macfb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 418e99c8e18e..1ace341a0ff4 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -714,6 +714,7 @@ static void macfb_nubus_set_irq(void *opaque, int n, int
> level)
>
> static void macfb_nubus_realize(DeviceState *dev, Error **errp)
> {
> +ERRP_GUARD();
> NubusDevice *nd = NUBUS_DEVICE(dev);
> MacfbNubusState *s = NUBUS_MACFB(dev);
> MacfbNubusDeviceClass *ndc = NUBUS_MACFB_GET_CLASS(dev);
The dereference is
ndc->parent_realize(dev, errp);
if (*errp) {
return;
}
We check *errp, because neither the callback returns void.
Reviewed-by: Markus Armbruster
Re: [PATCH 3/6] hw/mem/cxl_type3: Fix missing ERRP_GUARD() in ct3_realize()
On Wed, Feb 21, 2024 at 12:35:47PM +0100, Markus Armbruster wrote:
> Date: Wed, 21 Feb 2024 12:35:47 +0100
> From: Markus Armbruster
> Subject: Re: [PATCH 3/6] hw/mem/cxl_type3: Fix missing ERRP_GUARD() in
> ct3_realize()
>
> Zhao Liu writes:
>
> > From: Zhao Liu
> >
> > As the comment in qapi/error, dereferencing @errp requires
> > ERRP_GUARD():
> >
> > * = Why, when and how to use ERRP_GUARD() =
> > *
> > * Without ERRP_GUARD(), use of the @errp parameter is restricted:
> > * - It must not be dereferenced, because it may be null.
> > * - It should not be passed to error_prepend() or
> > * error_append_hint(), because that doesn't work with &error_fatal.
> > * ERRP_GUARD() lifts these restrictions.
> > *
> > * To use ERRP_GUARD(), add it right at the beginning of the function.
> > * @errp can then be used without worrying about the argument being
> > * NULL or &error_fatal.
> > *
> > * Using it when it's not needed is safe, but please avoid cluttering
> > * the source with useless code.
> >
> > Currently, since ct3_realize() - as a PCIDeviceClass.realize() method -
> > doesn't get the NULL errp parameter, it doesn't trigger the dereference
> > issue.
> >
> > To follow the requirement of errp, add missing ERRP_GUARD() in
> > ct3_realize().
> >
> > Suggested-by: Markus Armbruster
> > Signed-off-by: Zhao Liu
> > ---
> > Suggested by credit:
> > Markus: Referred his explanation about ERRP_GUARD().
> > ---
> > hw/mem/cxl_type3.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index e8801805b90f..a3b0761f843b 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -645,6 +645,7 @@ static DOEProtocol doe_cdat_prot[] = {
> >
> > static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> > {
> > +ERRP_GUARD();
> > CXLType3Dev *ct3d = CXL_TYPE3(pci_dev);
> > CXLComponentState *cxl_cstate = &ct3d->cxl_cstate;
> > ComponentRegisters *regs = &cxl_cstate->crb;
>
> The dereference is
>
>cxl_doe_cdat_init(cxl_cstate, errp);
>if (*errp) {
>goto err_free_special_ops;
>}
>
> We check *errp, because cxl_doe_cdat_init() returns void. Could be
> improved to return bool, along with its callees ct3_load_cdat() and
> ct3_build_cdat(), but that's a slightly more ambitious cleanup, so
>
> Reviewed-by: Markus Armbruster
>
Thanks! I can continue to consider such the cleanup in the follow up.
Will also add the dereference description in commit message to make
review easier.
Regards,
Zhao
[Stable-8.2.2 45/60] target/arm: Fix SVE/SME gross MTE suppression checks
From: Richard Henderson
The TBI and TCMA bits are located within mtedesc, not desc.
Cc: qemu-sta...@nongnu.org
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
Tested-by: Gustavo Romero
Message-id: 20240207025210.8837-7-richard.hender...@linaro.org
Signed-off-by: Peter Maydell
(cherry picked from commit 855f94eca80c85a99f459e36684ea2f98f6a3243)
Signed-off-by: Michael Tokarev
diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
index 1ee2690ceb..904bfdac43 100644
--- a/target/arm/tcg/sme_helper.c
+++ b/target/arm/tcg/sme_helper.c
@@ -573,8 +573,8 @@ void sme_ld1_mte(CPUARMState *env, void *za, uint64_t *vg,
desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
/* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
mtedesc = 0;
}
@@ -750,8 +750,8 @@ void sme_st1_mte(CPUARMState *env, void *za, uint64_t *vg,
target_ulong addr,
desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
/* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
mtedesc = 0;
}
diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index f006d152cc..5699dfe667 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -5800,8 +5800,8 @@ void sve_ldN_r_mte(CPUARMState *env, uint64_t *vg,
target_ulong addr,
desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
/* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
mtedesc = 0;
}
@@ -6156,8 +6156,8 @@ void sve_ldnfff1_r_mte(CPUARMState *env, void *vg,
target_ulong addr,
desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
/* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
mtedesc = 0;
}
@@ -6410,8 +6410,8 @@ void sve_stN_r_mte(CPUARMState *env, uint64_t *vg,
target_ulong addr,
desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
/* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
mtedesc = 0;
}
--
2.39.2
Re: [PATCH 18/23] plugins: add an API to read registers
Akihiko Odaki writes:
> On 2024/02/21 19:02, Alex Bennée wrote:
>> Akihiko Odaki writes:
>>
>>> On 2024/02/20 23:14, Alex Bennée wrote:
Akihiko Odaki writes:
> On 2024/02/17 1:30, Alex Bennée wrote:
>> We can only request a list of registers once the vCPU has been
>> initialised so the user needs to use either call the get function on
>> vCPU initialisation or during the translation phase.
>> We don't expose the reg number to the plugin instead hiding it
>> behind
>> an opaque handle. This allows for a bit of future proofing should the
>> internals need to be changed while also being hashed against the
>> CPUClass so we can handle different register sets per-vCPU in
>> hetrogenous situations.
>> Having an internal state within the plugins also allows us to expand
>> the interface in future (for example providing callbacks on register
>> change if the translator can track changes).
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
>> Cc: Akihiko Odaki
>> Message-Id: <20240103173349.398526-39-alex.ben...@linaro.org>
>> Based-on: <20231025093128.33116-18-akihiko.od...@daynix.com>
>> Signed-off-by: Alex Bennée
>> Reviewed-by: Pierrick Bouvier
>> +/*
>> + * Register handles
>> + *
>> + * The plugin infrastructure keeps hold of these internal data
>> + * structures which are presented to plugins as opaque handles. They
>> + * are global to the system and therefor additions to the hash table
>> + * must be protected by the @reg_handle_lock.
>> + *
>> + * In order to future proof for up-coming heterogeneous work we want
>> + * different entries for each CPU type while sharing them in the
>> + * common case of multiple cores of the same type.
>> + */
>> +
>> +static QemuMutex reg_handle_lock;
>> +
>> +struct qemu_plugin_register {
>> +const char *name;
>> +int gdb_reg_num;
>> +};
>> +
>> +static GHashTable *reg_handles; /* hash table of PluginReg */
>> +
>> +/* Generate a stable key - would xxhash be overkill? */
>> +static gpointer cpu_plus_reg_to_key(CPUState *cs, int gdb_regnum)
>> +{
>> +uintptr_t key = (uintptr_t) cs->cc;
>> +key ^= gdb_regnum;
>> +return GUINT_TO_POINTER(key);
>> +}
>
> I have pointed out this is theoretically prone to collisions and
> unsafe.
How is it unsafe? The aim is to share handles for the same CPUClass
rather than having a unique handle per register/cpu combo.
>>>
>>> THe intention is legitimate, but the implementation is not safe. It
>>> assumes (uintptr)cs->cc ^ gdb_regnum is unique, but there is no such
>>> guarantee. The key of GHashTable must be unique; generating hashes of
>>> keys should be done with hash_func given to g_hash_table_new().
>> This isn't a hash its a non-unique key. It is however unique for
>> the same register on the same class of CPU so for each vCPU in a system
>> can share the same opaque handles.
>> The hashing is done internally by glib. We would assert if there was
>> a
>> duplicate key referring to a different register.
>> I'm unsure what you want here? Do you have a suggestion for the key
>> generation algorithm? As the comment notes I did consider a more complex
>> mixing algorithm using xxhash but that wouldn't guarantee no clash
>> either.
>
> I suggest using a struct that holds both of cs->cc and gdb_regnum, and
> pass g_direct_equal() and g_direct_hash() to g_hash_table_new().
We already do:
if (!reg_handles) {
reg_handles = g_hash_table_new(g_direct_hash, g_direct_equal);
}
But we can't use g_direct_equal with something that exceeds the width of
gpointer as it is a straight equality test of the key. What you are
suggesting requires allocating memory for each key and de-referencing
with a custom GEqualFunc.
This seems overkill for something that as I have said doesn't happen.
The reason it doesn't happen is you will never see two CPUClass
instances so close to each other they share all bits apart from where
gdb_regnum is being xor'd. We could assert that is the case with
something like:
#define MAX_GDBREGS 300
/* Generate a stable key - would xxhash be overkill? */
static gpointer cpu_plus_reg_to_key(CPUState *cs, int gdb_regnum)
{
uintptr_t key = (uintptr_t) cs->cc;
qemu_build_assert(sizeof(*cs->cc) >= MAX_GDBREGS);
g_assert(gdb_regnum < MAX_GDBREGS);
key ^= gdb_regnum;
return GUINT_TO_POINTER(key);
}
although MAX_GDBREGS is currently a guess based on aarch64. In practice
though there are so many allocations thing are much farther apart. As we
can see in the one heterogeneous model we support at the moment (the
last 2 CPUs are cortex-r5f's):
./qemu-system-aarch64 -M xlnx-zcu102 -audio none -smp 6 -serial mon:stdio -s
-S -smp 6
cpu_common_class_init: k = 0x5565bebf10f0
arm_cpu_initfn: 0x7f32ee0a836
Re: [PATCH] qapi: Craft the BlockdevCreateOptionsLUKS comment
Yong Huang writes: > On Wed, Feb 21, 2024 at 2:43 PM Markus Armbruster wrote: > >> Hyman Huang writes: >> >> > Add comment in detail for commit 433957bb7f (qapi: >> > Make parameter 'file' optional for >> > BlockdevCreateOptionsLUKS). >> > >> > Signed-off-by: Hyman Huang >> > --- >> > qapi/block-core.json | 20 +++- >> > 1 file changed, 19 insertions(+), 1 deletion(-) >> > >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> > index ab5a93a966..42b0840d43 100644 >> > --- a/qapi/block-core.json >> > +++ b/qapi/block-core.json >> > @@ -4973,7 +4973,25 @@ >> > ## >> > # @BlockdevCreateOptionsLUKS: >> > # >> > -# Driver specific image creation options for LUKS. >> > +# Driver specific image creation options for LUKS. Note that >> > +# @file is required if @preallocation is specified and equals >> > +# PREALLOC_MODE_ON. The following three scenarios determine how >> > +# creation logic behaves when @preallocation is either equal to >> > +# PREALLOC_MODE_OFF or is not given: >> > +# >> > +# 1) When @file is given only, format the block device referenced >> > +# by @file as the LUKS specification and trunk it to the @size. >> >> Do you mean "truncate it to @size"? >> > Yes, :( sorry for the spelling mistake. Writing good documentation in a second language is *hard*. All we can reasonably expect from contributors to try their best. And then we improve the text together in review. Just like we do for code :) >> > +# In this case, the @size should reflect amount of space made >> > +# available to the guest, so the trunk size must take account >> > +# of that which will be used by the crypto header. >> > +# >> > +# 2) When @header is given only, just format the block device >> > +# referenced by @header as the LUKS specification. >> > +# >> > +# 3) When both @file and @header are given, block device >> > +# referenced by @file should be trunked to @size, and block >> > +# device referenced by @header should be formatted as the LUKS >> > +# specification. >> > # >> > # @file: Node to create the image format on, mandatory except when >> > #'preallocation' is not requested >> >> Let's see whether I understand. >> >> blockdev-create with "driver": "luks" can work in three different ways: >> >> 1. Create an image with a LUKS header >> >> 2. Create just a detached LUKS header >> >> 3. Create an image and a detached LUKS header >> >> Correct? >> > > Yes > > >> @file and @header are BlockdevRef, which means they refer to existing >> images with arbitrary driver. Could be "file", "qcow2", or anything. >> >> Correct? >> > Yes > > >> >> To get 1., specify @file, but not @header. >> >> To get 2., specify @header, but not @file. >> >> To get 3., specify both. >> >> Specifying neither is an error. >> >> Correct? >> > > Yes > > >> In any case, @size is the logical size of the image (how much data it >> can hold). >> > > Yes > > >> >> With 1., the actual image size is a bit larger due to the LUKS header. >> The @file image is resized to that size: if it's shorter, it's grown, if >> it's longer, it's truncated. >> > > Yes > > >> With 2., @size is merely recorded in the detached LUKS header. >> > > In LUKS1 specification, payload data size is not contained in the header, > so in this case, @size is not recorded in the detached LUKS header. > The creation logic just does the LUKS header formatting only. Is @size unused then? >> With 3., @size is recorded in the detached LUKS header, and the @file >> image is resized as with 1. >> > > Same reason as above, @size is not recorded and @file image is > resized to @size exactly. > > >> >> Correct? >> >> > Thanks for the comment, > > Yong
[Stable-8.2.2 33/60] hw/cxl: Pass CXLComponentState to cache_mem_ops
From: Li Zhijian
cache_mem_ops.{read,write}() interprets opaque as
CXLComponentState(cxl_cstate) instead of ComponentRegisters(cregs).
Fortunately, cregs is the first member of cxl_cstate, so their values are
the same.
Fixes: 9e58f52d3f8 ("hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5)")
Reviewed-by: Fan Ni
Signed-off-by: Li Zhijian
Signed-off-by: Jonathan Cameron
Message-Id: <20240126120132.24248-8-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
(cherry picked from commit 729d45a6af06753d3e330f589c248fe9687c5cd5)
Signed-off-by: Michael Tokarev
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 29d477492b..9dfde6c0b3 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -199,7 +199,7 @@ void cxl_component_register_block_init(Object *obj,
/* io registers controls link which we don't care about in QEMU */
memory_region_init_io(&cregs->io, obj, NULL, cregs, ".io",
CXL2_COMPONENT_IO_REGION_SIZE);
-memory_region_init_io(&cregs->cache_mem, obj, &cache_mem_ops, cregs,
+memory_region_init_io(&cregs->cache_mem, obj, &cache_mem_ops, cxl_cstate,
".cache_mem", CXL2_COMPONENT_CM_REGION_SIZE);
memory_region_add_subregion(&cregs->component_registers, 0, &cregs->io);
--
2.39.2
Re: [PATCH] vl, pc: turn -no-fd-bootchk into a machine property
On Tue, Feb 20, 2024 at 11:43 PM Bernhard Beschow wrote:
>
>
>
> Am 20. Februar 2024 15:53:52 UTC schrieb Paolo Bonzini :
> >Add a fd-bootchk property to PC machine types, so that -no-fd-bootchk
> >returns an error if the machine does not support booting from floppies
> >and checking for boot signatures therein.
> >
> >Suggested-by: Philippe Mathieu-Daudé
> >Signed-off-by: Paolo Bonzini
> >---
> > include/hw/i386/pc.h | 2 +-
> > hw/i386/pc.c | 30 +-
> > system/globals.c | 1 -
> > system/vl.c | 2 +-
> > qemu-options.hx | 2 +-
> > 5 files changed, 28 insertions(+), 9 deletions(-)
> >
> >diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >index 02a0deedd3c..e5382a02e7a 100644
> >--- a/include/hw/i386/pc.h
> >+++ b/include/hw/i386/pc.h
> >@@ -50,6 +50,7 @@ typedef struct PCMachineState {
> > bool hpet_enabled;
> > bool i8042_enabled;
> > bool default_bus_bypass_iommu;
> >+bool fd_bootchk;
> > uint64_t max_fw_size;
> >
> > /* ACPI Memory hotplug IO base address */
> >@@ -147,7 +148,6 @@ OBJECT_DECLARE_TYPE(PCMachineState, PCMachineClass,
> >PC_MACHINE)
> > GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
> >
> > /* pc.c */
> >-extern int fd_bootchk;
> >
> > void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
> >
> >diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >index 28194014f82..31f4bb25a3e 100644
> >--- a/hw/i386/pc.c
> >+++ b/hw/i386/pc.c
> >@@ -399,8 +399,8 @@ static int boot_device2nibble(char boot_device)
> > return 0;
> > }
> >
> >-static void set_boot_dev(MC146818RtcState *s, const char *boot_device,
> >- Error **errp)
> >+static void set_boot_dev(PCMachineState *pcms, MC146818RtcState *s,
> >+ const char *boot_device, Error **errp)
> > {
> > #define PC_MAX_BOOT_DEVICES 3
> > int nbds, bds[3] = { 0, };
> >@@ -420,12 +420,14 @@ static void set_boot_dev(MC146818RtcState *s, const
> >char *boot_device,
> > }
> > }
> > mc146818rtc_set_cmos_data(s, 0x3d, (bds[1] << 4) | bds[0]);
> >-mc146818rtc_set_cmos_data(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 :
> >0x1));
> >+mc146818rtc_set_cmos_data(s, 0x38, (bds[2] << 4) | !pcms->fd_bootchk);
> > }
> >
> > static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
> > {
> >-set_boot_dev(opaque, boot_device, errp);
> >+PCMachineState *pcms = PC_MACHINE(current_machine);
> >+
> >+set_boot_dev(pcms, opaque, boot_device, errp);
> > }
> >
> > static void pc_cmos_init_floppy(MC146818RtcState *rtc_state, ISADevice
> > *floppy)
> >@@ -617,6 +619,9 @@ void pc_cmos_init(PCMachineState *pcms,
> > mc146818rtc_set_cmos_data(s, 0x5c, val >> 8);
> > mc146818rtc_set_cmos_data(s, 0x5d, val >> 16);
> >
> >+object_property_add_bool(obj, "fd-bootchk", pc_machine_get_fd_bootchk,
> >+ pc_machine_set_fd_bootchk);
>
> Isn't it possible to turn this into a class property or add the property in
> pc_machine_initfn()? Aggregating properties in one place seems more
> comprehensible to me.
Sure, I placed it in pc_cmos_init because rtc_state is already created here.
Paolo
Re: [PATCH 3/5] hw/isa: Embed TYPE_PORT92 in south bridges used in PC machines
On Wed, 21 Feb 2024, Mark Cave-Ayland wrote: On 18/02/2024 13:16, Bernhard Beschow wrote: Port 92 is an integral part of the PIIX and ICH south bridges, so instantiate it there. The isapc machine now needs to instantiate it explicitly, analoguous to the RTC. Note that due to migration compatibility, port92 is optional in the south bridges. It is always instantiated the isapc machine for simplicity. Signed-off-by: Bernhard Beschow --- include/hw/i386/pc.h | 2 +- include/hw/southbridge/ich9.h | 4 include/hw/southbridge/piix.h | 3 +++ hw/i386/pc.c | 18 -- hw/i386/pc_piix.c | 9 +++-- hw/i386/pc_q35.c | 8 +--- hw/isa/lpc_ich9.c | 9 + hw/isa/piix.c | 9 + hw/isa/Kconfig| 2 ++ 9 files changed, 52 insertions(+), 12 deletions(-) [...] diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index efdf43e92c..f42a087c07 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -47,6 +47,7 @@ config PIIX select IDE_PIIX select ISA_BUS select MC146818RTC +select PORT92 select USB_UHCI config PORT92 @@ -78,3 +79,4 @@ config LPC_ICH9 select ISA_BUS select ACPI_ICH9 select MC146818RTC +select PORT92 I had a look at this (and did a bit of revision around 8042 and A20), and I am starting to wonder if the PORT92 device isn't something that belongs to the southbridge, but more specifically to the superio chip? A couple of thoughts as to why I came to this conclusion: firstly the superio chip is generally considered to be a single integrated implementation of legacy IO devices, so this feels like a natural home for the PORT92 device. Secondly the value of the "has-port92" property is controlled by pcms->i8042_enabled, and this value is already passed into functions such as pc_superio_init() for example. One more argument for that may be that this A20 gate was originally controlled by the keyboard controller and that's part of the superio. The VIA southbridge docs even mention something about the KBC and this port92 register controlling the A20 gate together. So to implement that correctly it may need to be in the same device. However this may be specific to the VIA chip so not sure if this implementation can be shared by all the southbridge devices. I think this would also help reduce the changes required for the individual machines, however the devil is always in the details particularly when migration is involved. One thing that bothers me very much about this port92 device is that we have about 100 lines of code of which the actual functionality is just a qemu_irq and an uint8_t controlling it and storing the register value. That shouldn't be more than 10 lines or maybe 20 with migration support, it's only 100 lines because it's wrapped in a QDev. Bernhard wanted to move it out from the PC machines to clean those up but as a result this mess is just pushed down in the southbridge (or into superio as you propose). It could easily be implemented in the southbridge or superio by just adding the qemu_irq and the register value and maybe export it as a property so the machine can set it for migration compatibility and then we don't need a separate QDev wrapper around it as the superio or southbridge is already a QDev and has the needed stuff to store this. But one could also argue that while these southbridges implement this functionality, it's only used on the PC machines so as we already have it modelled in a separate object we could just let the PC machines instantiate it and leave it as it is and don't add this to other machines where it's not needed. (I like Mark's proposal a bit better only because that leaves the southbridge untouched and changes the superio instead that I care less about but the issue is the same there.) Regards, BALATON Zoltan
Re: [PATCH 2/5] hw/i386/port92: Allow for TYPE_PORT92 to be embedded in devices
On 18/02/2024 13:16, Bernhard Beschow wrote:
Port 92 is an integral part of south bridges. Allow for embedding it there.
South bridges aren't architecture-specific, so move port92.c to hw/isa which is
accessible to other architectures than x86.
Signed-off-by: Bernhard Beschow
---
include/hw/i386/pc.h | 5 -
include/hw/isa/port92.h | 30 ++
hw/i386/pc.c | 1 +
hw/{i386 => isa}/port92.c | 14 +-
hw/i386/Kconfig | 1 +
hw/i386/meson.build | 3 +--
hw/i386/trace-events | 4
hw/isa/Kconfig| 3 +++
hw/isa/meson.build| 1 +
hw/isa/trace-events | 4
10 files changed, 42 insertions(+), 24 deletions(-)
create mode 100644 include/hw/isa/port92.h
rename hw/{i386 => isa}/port92.c (91%)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ec0e5efcb2..b2987209b1 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -188,11 +188,6 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus,
PCIBus *pci_bus);
void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
-/* port92.c */
-#define PORT92_A20_LINE "a20"
-
-#define TYPE_PORT92 "port92"
-
/* pc_sysfw.c */
void pc_system_flash_create(PCMachineState *pcms);
void pc_system_flash_cleanup_unused(PCMachineState *pcms);
diff --git a/include/hw/isa/port92.h b/include/hw/isa/port92.h
new file mode 100644
index 00..214783d071
--- /dev/null
+++ b/include/hw/isa/port92.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU I/O port 0x92 (System Control Port A, to handle Fast Gate A20)
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef HW_PORT92_H
+#define HW_PORT92_H
+
+#include "exec/memory.h"
+#include "hw/irq.h"
+#include "hw/isa/isa.h"
+#include "qom/object.h"
+
+#define TYPE_PORT92 "port92"
+OBJECT_DECLARE_SIMPLE_TYPE(Port92State, PORT92)
+
+struct Port92State {
+ISADevice parent_obj;
+
+MemoryRegion io;
+uint8_t outport;
+qemu_irq a20_out;
+};
+
+#define PORT92_A20_LINE "a20"
+
+#endif /* HW_PORT92_H */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 196827531a..0b11d4576e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -32,6 +32,7 @@
#include "hw/i386/vmport.h"
#include "sysemu/cpus.h"
#include "hw/ide/internal.h"
+#include "hw/isa/port92.h"
#include "hw/timer/hpet.h"
#include "hw/loader.h"
#include "hw/rtc/mc146818rtc.h"
diff --git a/hw/i386/port92.c b/hw/isa/port92.c
similarity index 91%
rename from hw/i386/port92.c
rename to hw/isa/port92.c
index 1070bfbf36..06df06b088 100644
--- a/hw/i386/port92.c
+++ b/hw/isa/port92.c
@@ -9,20 +9,8 @@
#include "qemu/osdep.h"
#include "sysemu/runstate.h"
#include "migration/vmstate.h"
-#include "hw/irq.h"
-#include "hw/i386/pc.h"
+#include "hw/isa/port92.h"
#include "trace.h"
-#include "qom/object.h"
-
-OBJECT_DECLARE_SIMPLE_TYPE(Port92State, PORT92)
-
-struct Port92State {
-ISADevice parent_obj;
-
-MemoryRegion io;
-uint8_t outport;
-qemu_irq a20_out;
-};
static void port92_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index a1846be6f7..ccf6de4a00 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -37,6 +37,7 @@ config PC
select I8254
select PCKBD
select PCSPK
+select PORT92
select I8257
select MC146818RTC
# For ACPI builder:
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index b9c1ca39cb..94d558edd6 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -24,8 +24,7 @@ i386_ss.add(when: 'CONFIG_ACPI', if_true:
files('acpi-common.c'))
i386_ss.add(when: 'CONFIG_PC', if_true: files(
'pc.c',
'pc_sysfw.c',
- 'acpi-build.c',
- 'port92.c'))
+ 'acpi-build.c'))
i386_ss.add(when: 'CONFIG_X86_FW_OVMF', if_true: files('pc_sysfw_ovmf.c'),
if_false:
files('pc_sysfw_ovmf-stubs.c'))
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 53c02d7ac8..b730589394 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -118,10 +118,6 @@ vmport_command(unsigned char command) "command: 0x%02x"
x86_gsi_interrupt(int irqn, int level) "GSI interrupt #%d level:%d"
x86_pic_interrupt(int irqn, int level) "PIC interrupt #%d level:%d"
-# port92.c
-port92_read(uint8_t val) "port92: read 0x%02x"
-port92_write(uint8_t val) "port92: write 0x%02x"
-
# vmmouse.c
vmmouse_get_status(void) ""
vmmouse_mouse_event(int x, int y, int dz, int buttons_state) "event: x=%d y=%d
dz=%d state=%d"
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 73c6470805..efdf43e92c 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -49,6 +49,9 @@ config PIIX
select MC146818RTC
select USB_UHCI
+config PORT92
+bool
+
config VT82C686
bool
select ISA_BUS
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index 3219282217..fb7cd44984 100644
--- a/hw/i
Re: [PATCH V4 4/5] util: strList unit tests
Steve Sistare writes:
> Signed-off-by: Steve Sistare
> Reviewed-by: Marc-André Lureau
> ---
> tests/unit/meson.build| 1 +
> tests/unit/test-strList.c | 80
> +++
> 2 files changed, 81 insertions(+)
> create mode 100644 tests/unit/test-strList.c
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 69f9c05..113d12e 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -35,6 +35,7 @@ tests = {
>'test-rcu-simpleq': [],
>'test-rcu-tailq': [],
>'test-rcu-slist': [],
> + 'test-strList': [],
>'test-qdist': [],
>'test-qht': [],
>'test-qtree': [],
> diff --git a/tests/unit/test-strList.c b/tests/unit/test-strList.c
> new file mode 100644
> index 000..49a1cfd
> --- /dev/null
> +++ b/tests/unit/test-strList.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (c) 2022 - 2024 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/strList.h"
> +
> +static strList *make_list(int length)
> +{
> +strList *head = 0, *list, **prev = &head;
> +
> +while (length--) {
> +list = *prev = g_new0(strList, 1);
> +list->value = g_strdup("aaa");
> +prev = &list->next;
> +}
> +return head;
> +}
> +
> +static void test_length(void)
> +{
> +strList *list;
> +int i;
> +
> +for (i = 0; i < 5; i++) {
> +list = make_list(i);
> +g_assert_cmpint(i, ==, QAPI_LIST_LENGTH(list));
> +qapi_free_strList(list);
> +}
> +}
> +
> +struct {
> +const char *string;
> +const char *delim;
> +const char *args[5];
> +} list_data[] = {
> +{ 0, ",", { 0 } },
> +{ "", ",", { 0 } },
> +{ "a", ",", { "a", 0 } },
> +{ "a,b", ",", { "a", "b", 0 } },
> +{ "a,b,c", ",", { "a", "b", "c", 0 } },
> +{ "first last", " ", { "first", "last", 0 } },
> +{ "a:", ":", { "a", "", 0 } },
> +{ "a::b", ":", { "a", "", "b", 0 } },
> +{ ":", ":", { "", "", 0 } },
> +{ ":a", ":", { "", "a", 0 } },
> +{ "::a", ":", { "", "", "a", 0 } },
> +};
NULL instead of 0, please.
> +
> +static void test_strv(void)
> +{
> +int i, j;
> +const char **expect;
> +strList *list;
> +GStrv args;
I'd prefer char **argv;
> +
> +for (i = 0; i < ARRAY_SIZE(list_data); i++) {
> +expect = list_data[i].args;
> +list = strList_from_string(list_data[i].string, list_data[i].delim);
> +args = strv_from_strList(list);
> +qapi_free_strList(list);
> +for (j = 0; expect[j] && args[j]; j++) {
Loop stops when either array element is null. That's wrong, we need to
exhaust both arrays: || instead of &&.
> +g_assert_cmpstr(expect[j], ==, args[j]);
> +}
> +g_assert_null(expect[j]);
> +g_assert_null(args[j]);
> +g_strfreev(args);
> +}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +g_test_init(&argc, &argv, NULL);
> +g_test_add_func("/test-string/length", test_length);
> +g_test_add_func("/test-string/strv", test_strv);
> +return g_test_run();
> +}
[RFC PATCH v2 05/22] target/arm: Support MSR access to ALLINT
Support ALLINT msr access as follow:
mrs , ALLINT// read allint
msr ALLINT, // write allint with imm
Signed-off-by: Jinjie Ruan
---
target/arm/helper.c | 32
1 file changed, 32 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a3062cb2ad..211156d640 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4618,6 +4618,31 @@ static void aa64_daif_write(CPUARMState *env, const
ARMCPRegInfo *ri,
env->daif = value & PSTATE_DAIF;
}
+static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->allint = value & PSTATE_ALLINT;
+}
+
+static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->allint & PSTATE_ALLINT;
+}
+
+static CPAccessResult aa64_allint_access(CPUARMState *env,
+ const ARMCPRegInfo *ri, bool isread)
+{
+if (arm_current_el(env) == 0) {
+return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+
+if (arm_current_el(env) == 1 && arm_is_el2_enabled(env) &&
+cpu_isar_feature(aa64_hcx, env_archcpu(env)) &&
+(env->cp15.hcrx_el2 & HCRX_TALLINT))
+return CP_ACCESS_TRAP_EL2;
+return CP_ACCESS_OK;
+}
+
static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
return env->pstate & PSTATE_PAN;
@@ -5437,6 +5462,13 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.access = PL0_RW, .accessfn = aa64_daif_access,
.fieldoffset = offsetof(CPUARMState, daif),
.writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore },
+{ .name = "ALLINT", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
+ .type = ARM_CP_NO_RAW,
+ .access = PL1_RW, .accessfn = aa64_allint_access,
+ .fieldoffset = offsetof(CPUARMState, allint),
+ .writefn = aa64_allint_write, .readfn = aa64_allint_read,
+ .resetfn = arm_cp_reset_ignore },
{ .name = "FPCR", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
.access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END,
--
2.34.1
[RFC PATCH v2 13/22] hw/intc/arm_gicv3: Add external IRQ lines for NMI
Augment the GICv3's QOM device interface by adding one
new set of sysbus IRQ line, to signal NMI to each CPU.
Signed-off-by: Jinjie Ruan
---
hw/intc/arm_gicv3_common.c | 3 +++
include/hw/intc/arm_gic_common.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index cb55c72681..6249c3edc9 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -299,6 +299,9 @@ void gicv3_init_irqs_and_mmio(GICv3State *s,
qemu_irq_handler handler,
for (i = 0; i < s->num_cpu; i++) {
sysbus_init_irq(sbd, &s->cpu[i].parent_vfiq);
}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, &s->cpu[i].parent_nmi);
+}
memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s,
"gicv3_dist", 0x1);
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 7080375008..fc89be96d4 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -71,6 +71,7 @@ struct GICState {
qemu_irq parent_fiq[GIC_NCPU];
qemu_irq parent_virq[GIC_NCPU];
qemu_irq parent_vfiq[GIC_NCPU];
+qemu_irq parent_nmi[GIC_NCPU];
qemu_irq maintenance_irq[GIC_NCPU];
/* GICD_CTLR; for a GIC with the security extensions the NS banked version
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 4e2fb518e7..1eb8c39239 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -155,6 +155,7 @@ struct GICv3CPUState {
qemu_irq parent_fiq;
qemu_irq parent_virq;
qemu_irq parent_vfiq;
+qemu_irq parent_nmi;
/* Redistributor */
uint32_t level; /* Current IRQ level */
--
2.34.1
Re: [PATCH v4 11/34] migration/ram: Introduce 'fixed-ram' migration capability
Markus Armbruster writes:
> Fabiano Rosas writes:
>
>> Add a new migration capability 'fixed-ram'.
>>
>> The core of the feature is to ensure that each RAM page has a specific
>> offset in the resulting migration stream. The reasons why we'd want
>> such behavior are:
>>
>> - The resulting file will have a bounded size, since pages which are
>>dirtied multiple times will always go to a fixed location in the
>>file, rather than constantly being added to a sequential
>>stream. This eliminates cases where a VM with, say, 1G of RAM can
>>result in a migration file that's 10s of GBs, provided that the
>>workload constantly redirties memory.
>>
>> - It paves the way to implement O_DIRECT-enabled save/restore of the
>>migration stream as the pages are ensured to be written at aligned
>>offsets.
>>
>> - It allows the usage of multifd so we can write RAM pages to the
>>migration file in parallel.
>>
>> For now, enabling the capability has no effect. The next couple of
>> patches implement the core functionality.
>>
>> Signed-off-by: Fabiano Rosas
>
> [...]
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 5a565d9b8d..3fce5fe53e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -531,6 +531,10 @@
>> # and can result in more stable read performance. Requires KVM
>> # with accelerator property "dirty-ring-size" set. (Since 8.1)
>> #
>> +# @fixed-ram: Migrate using fixed offsets in the migration file for
>> +# each RAM page. Requires a migration URI that supports seeking,
>> +# such as a file. (since 9.0)
>> +#
>> # Features:
>> #
>> # @deprecated: Member @block is deprecated. Use blockdev-mirror with
>> @@ -555,7 +559,7 @@
>> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>> 'validate-uuid', 'background-snapshot',
>> 'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
>> - 'dirty-limit'] }
>> + 'dirty-limit', 'fixed-ram'] }
>>
>> ##
>> # @MigrationCapabilityStatus:
>
> Can we find a better name than @fixed-ram? @fixed-ram-offsets?
> @use-seek?
I have no idea how we came to fixed-ram. The archives don't provide any
clarification. I find it confusing at first glance as well.
A little brainstorming on how fixed-ram is different from exiting
migration:
Fixed-ram:
uses a file, like the 'file:' migration;
needs a seeking medium, such as a file;
migrates ram by placing a page always in the same offset in the
file, contrary to normal migration which streams the page changes
continuously;
ensures a migration file of size bounded to VM RAM size, contrary to
normal 'file:' migration which creates a file with unbounded size;
enables multi-threaded RAM migration, even though we only use it when
multifd is enabled;
uses scatter-gatter APIs (pwritev, preadv);
So a few options:
(disconsidering use-seek, it might be even more generic/vague)
- fixed-ram-offsets
- non-streaming (or streaming: false)
- ram-scatter-gather (ram-sg)
- parallel-ram (even with the slight inaccuracy that we sometimes do it
single-threaded)
Remember we also use this name internally, so I think a broader
"feature" name is better that a super specific one.
Does anyone have a strong preference? Other suggestions?
> Apart from that, QAPI schema
> Acked-by: Markus Armbruster
Thanks!
[RFC PATCH v2 20/22] hw/intc/arm_gicv3: Add NMI handling CPU interface registers
Add the NMIAR CPU interface registers which deal with acknowledging NMI.
When introduce NMI interrupt, there are some updates to the semantics for the
register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
should return 1022 if the intid has super priority. And for ICC_NMIAR1_EL1
register, it should return 1023 if the intid do not have super priority.
Howerever, these are not necessary for ICC_HPPIR1_EL1 register.
Signed-off-by: Jinjie Ruan
---
hw/intc/arm_gicv3_cpuif.c | 46 ---
hw/intc/gicv3_internal.h | 1 +
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e1a60d8c15..f5bf8df32b 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -1097,7 +1097,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState *cs,
CPUARMState *env)
return cs->hppi.irq;
}
-static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
+static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env,
+ bool is_nmi, bool is_hppi)
{
/* Return the highest priority pending interrupt register value
* for group 1.
@@ -1108,6 +1109,16 @@ static uint64_t icc_hppir1_value(GICv3CPUState *cs,
CPUARMState *env)
return INTID_SPURIOUS;
}
+if (!is_hppi) {
+if (is_nmi && (!cs->hppi.superprio)) {
+return INTID_SPURIOUS;
+}
+
+if ((!is_nmi) && cs->hppi.superprio) {
+return INTID_NMI;
+}
+}
+
/* Check whether we can return the interrupt or if we should return
* a special identifier, as per the CheckGroup1ForSpecialIdentifiers
* pseudocode. (We can simplify a little because for us ICC_SRE_EL1.RM
@@ -1168,7 +1179,30 @@ static uint64_t icc_iar1_read(CPUARMState *env, const
ARMCPRegInfo *ri)
if (!icc_hppi_can_preempt(cs)) {
intid = INTID_SPURIOUS;
} else {
-intid = icc_hppir1_value(cs, env);
+intid = icc_hppir1_value(cs, env, false, false);
+}
+
+if (!gicv3_intid_is_special(intid)) {
+icc_activate_irq(cs, intid);
+}
+
+trace_gicv3_icc_iar1_read(gicv3_redist_affid(cs), intid);
+return intid;
+}
+
+static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+GICv3CPUState *cs = icc_cs_from_env(env);
+uint64_t intid;
+
+if (icv_access(env, HCR_IMO)) {
+return icv_iar_read(env, ri);
+}
+
+if (!icc_hppi_can_preempt(cs)) {
+intid = INTID_SPURIOUS;
+} else {
+intid = icc_hppir1_value(cs, env, true, false);
}
if (!gicv3_intid_is_special(intid)) {
@@ -1555,7 +1589,7 @@ static uint64_t icc_hppir1_read(CPUARMState *env, const
ARMCPRegInfo *ri)
return icv_hppir_read(env, ri);
}
-value = icc_hppir1_value(cs, env);
+value = icc_hppir1_value(cs, env, false, true);
trace_gicv3_icc_hppir1_read(gicv3_redist_affid(cs), value);
return value;
}
@@ -2344,6 +2378,12 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
.access = PL1_R, .accessfn = gicv3_irq_access,
.readfn = icc_iar1_read,
},
+{ .name = "ICC_NMIAR1_EL1", .state = ARM_CP_STATE_BOTH,
+ .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 5,
+ .type = ARM_CP_IO | ARM_CP_NO_RAW,
+ .access = PL1_R, .accessfn = gicv3_irq_access,
+ .readfn = icc_nmiar1_read,
+},
{ .name = "ICC_EOIR1_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
.type = ARM_CP_IO | ARM_CP_NO_RAW,
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 8d793243f4..93e56b3726 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -511,6 +511,7 @@ FIELD(VTE, RDBASE, 42, RDBASE_PROCNUM_LENGTH)
/* Special interrupt IDs */
#define INTID_SECURE 1020
#define INTID_NONSECURE 1021
+#define INTID_NMI 1022
#define INTID_SPURIOUS 1023
/* Functions internal to the emulated GICv3 */
--
2.34.1
Re: [PATCH 06/14] hw/pci-bridge: Extract QOM ICH definitions to 'ich_dmi_pci.h'
On 20/2/24 20:25, Bernhard Beschow wrote: Am 19. Februar 2024 16:38:46 UTC schrieb "Philippe Mathieu-Daudé" : Expose TYPE_ICH_DMI_PCI_BRIDGE to the new "hw/pci-bridge/ich_dmi_pci.h" header. Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + include/hw/pci-bridge/ich_dmi_pci.h | 20 include/hw/southbridge/ich9.h | 2 -- hw/pci-bridge/i82801b11.c | 11 --- 4 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 include/hw/pci-bridge/ich_dmi_pci.h diff --git a/MAINTAINERS b/MAINTAINERS index 1b210c5cc1..50507c3dd6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2609,6 +2609,7 @@ F: hw/acpi/ich9*.c F: hw/i2c/smbus_ich9.c F: hw/isa/lpc_ich9.c F: include/hw/acpi/ich9*.h +F: include/hw/pci-bridge/ich_dmi_pci.h F: include/hw/southbridge/ich9.h PIIX4 South Bridge (i82371AB) diff --git a/include/hw/pci-bridge/ich_dmi_pci.h b/include/hw/pci-bridge/ich_dmi_pci.h new file mode 100644 index 00..7623b32b8e --- /dev/null +++ b/include/hw/pci-bridge/ich_dmi_pci.h Shouldn't we name the new header like its source file, i.e. i82801b11.h? I'll rename sources to use the "ichX_FOO.c" pattern. @@ -0,0 +1,20 @@ +/* + * QEMU ICH4 i82801b11 dmi-to-pci Bridge Emulation + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef HW_PCI_BRIDGE_ICH_D2P_H +#define HW_PCI_BRIDGE_ICH_D2P_H ...
Re: [PATCH v8 00/21] Introduce smp.modules for x86 in QEMU
On Wed, Feb 21, 2024 at 01:41:35PM +0100, Markus Armbruster wrote:
> Date: Wed, 21 Feb 2024 13:41:35 +0100
> From: Markus Armbruster
> Subject: Re: [PATCH v8 00/21] Introduce smp.modules for x86 in QEMU
>
> Zhao Liu writes:
>
> > From: Zhao Liu
> >
> > Hi list,
> >
> > This is the our v8 patch series, rebased on the master branch at the
> > commit 11be70677c70 ("Merge tag 'pull-vfio-20240129' of
> > https://github.com/legoater/qemu into staging").
> >
> > Compared with v7 [1], v8 mainly has the following changes:
> > * Introduced smp.modules for x86 instead of reusing current
> > smp.clusters.
> > * Reworte the CPUID[0x1F] encoding.
> >
> > Given the code change, I dropped the most previously gotten tags
> > (Acked-by/Reviewed-by/Tested-by from Michael & Babu, thanks for your
> > previous reviews and tests!) in v8.
> >
> > With the description of the new modules added to x86 arch code in v7 [1]
> > cover letter, the following sections are mainly the description of
> > the newly added smp.modules (since v8) as supplement.
> >
> > Welcome your comments!
> >
> >
> > Why We Need a New CPU Topology Level
> >
> >
> > For the discussion in v7 about whether we should reuse current
> > smp.clusters for x86 module, the core point is what's the essential
> > differences between x86 module and general cluster.
> >
> > Since, cluster (for ARM/riscv) lacks a comprehensive and rigorous
> > hardware definition, and judging from the description of smp.clusters
> > [2] when it was introduced by QEMU, x86 module is very similar to
> > general smp.clusters: they are all a layer above existing core level
> > to organize the physical cores and share L2 cache.
> >
> > However, after digging deeper into the description and use cases of
> > cluster in the device tree [3], I realized that the essential
> > difference between clusters and modules is that cluster is an extremely
> > abstract concept:
> > * Cluster supports nesting though currently QEMU doesn't support
> > nested cluster topology. However, modules will not support nesting.
> > * Also due to nesting, there is great flexibility in sharing resources
> > on clusters, rather than narrowing cluster down to sharing L2 (and
> > L3 tags) as the lowest topology level that contains cores.
> > * Flexible nesting of cluster allows it to correspond to any level
> > between the x86 package and core.
> >
> > Based on the above considerations, and in order to eliminate the naming
> > confusion caused by the mapping between general cluster and x86 module
> > in v7, we now formally introduce smp.modules as the new topology level.
> >
> >
> > Where to Place Module in Existing Topology Levels
> > =
> >
> > The module is, in existing hardware practice, the lowest layer that
> > contains the core, while the cluster is able to have a higher topological
> > scope than the module due to its nesting.
> >
> > Thereby, we place the module between the cluster and the core, viz:
> >
> > drawer/book/socket/die/cluster/module/core/thread
> >
> >
> > Additional Consideration on CPU Topology
> >
> >
> > Beyond this patchset, nowadays, different arches have different topology
> > requirements, and maintaining arch-agnostic general topology in SMP
> > becomes to be an increasingly difficult thing due to differences in
> > sharing resources and special flexibility (e.g., nesting):
> > * It becomes difficult to put together all CPU topology hierarchies of
> > different arches to define complete topology order.
> > * It also becomes complex to ensure the correctness of the topology
> > calculations.
> > - Now the max_cpus is calculated by multiplying all topology
> > levels, and too many topology levels can easily cause omissions.
> >
> > Maybe we should consider implementing arch-specfic topology hierarchies.
> >
> >
> > [1]:
> > https://lore.kernel.org/qemu-devel/20240108082727.420817-1-zhao1@linux.intel.com/
> > [2]: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg04051.html
> > [3]:
> > https://www.kernel.org/doc/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>
> Have you considered putting an abridged version of your lovely rationale
> into a commit message, so it can be found later more easily?
>
Sure, I'll. Thanks for helping me improve my commit message. ;-)
Regards,
Zhao
Re: [PATCH V4 5/5] migration: simplify exec migration functions
Steve Sistare writes:
> Simplify the exec migration code by using list utility functions.
>
> As a side effect, this also fixes a minor memory leak. On function return,
> "g_auto(GStrv) argv" frees argv and each element, which is wrong, because
> the function does not own the individual elements. To compensate, the code
> uses g_steal_pointer which NULLs argv and prevents the destructor from
> running, but argv is leaked.
>
> Fixes: cbab4face57b ("migration: convert exec backend ...")
> Signed-off-by: Steve Sistare
Reviewed-by: Fabiano Rosas
Re: [PATCH v2 0/2] Field 'reason' for MIGRATION event
Roman Khapov writes:
Hi Roman,
> This is resending of series 20240215082659.1378342-1-rkha...@yandex-team.ru,
> where patch subjects numbers were broken in patch 2/2.
>
> Sometimes, when migration fails, it is hard to find out
> the cause of the problems: you have to grep qemu logs.
> At the same time, there is MIGRATION event, which looks like
> suitable place to hold such error descriptions.
query-migrate after the event is received should be enough for giving
you the failure reason. We have that in error-desc. See commit
c94143e587 ("migration: Display error in query-migrate irrelevant of
status").
>
> To handle situation like this (maybe one day it will be useful
> for other MIGRATION statuses to have additional 'reason' strings),
I find it unlikely. There's no "reason" for making progress except
that's how things work. Only the exceptional (i.e. failure) statuses
would have a reason. Today that's FAILED only, maybe also
POSTCOPY_PAUSED.
> the general optional field 'reason' can be added.
>
> The series proposes next changes:
>
> 1. Adding optional 'reason' field of type str into
>qapi/migration.json MIGRATION event
>
> 2. Passing some error description as reason for every place, which
>sets migration state to MIGRATION_STATUS_FAILED
>
> After the series, MIGRATION event will looks like this:
> {"execute": "qmp_capabilities"}
> {"return": {}}
> {"event": "MIGRATION", "data": {"status": "setup"}}
> {"event": "MIGRATION", "data": {"status": "failed", "reason": "Failed to
> connect to '/tmp/sock.sock': No such file or directory"}}
>
> Roman Khapov (2):
> qapi/migration.json: add reason to MIGRATION event
> migration: add error reason for failed MIGRATION events
>
> migration/colo.c | 6 +-
> migration/migration.c | 128 --
> migration/migration.h | 5 +-
> migration/multifd.c | 10 ++--
> migration/savevm.c| 24
> qapi/migration.json | 3 +-
> 6 files changed, 112 insertions(+), 64 deletions(-)
Please remember to run make check:
380/383 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test ERROR
104.77s killed by signal 6 SIGABRT
―――
stderr: Broken pipe ../tests/qtest/libqtest.c:204: kill_qemu() detected
QEMU death from signal 11 (Segmentation fault) (core dumped)
Most likely one of the new error_setg has broken postcopy recovery. Some
of those paths are not intended to trigger cleanup.
[PATCH] qga-win: Add support of Windows Server 2025 in get-osinfo command
Add support of Windows Server 2025 in get-osinfo command
Signed-off-by: Dehan Meng
---
qga/commands-win32.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..f3c7e604c9 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2154,6 +2154,7 @@ static ga_win_10_0_t const
WIN_10_0_SERVER_VERSION_MATRIX[4] = {
{14393, "Microsoft Windows Server 2016","2016"},
{17763, "Microsoft Windows Server 2019","2019"},
{20344, "Microsoft Windows Server 2022","2022"},
+{26040, "MIcrosoft Windows Server 2025","2025"},
{0, 0}
};
--
2.35.1
[PATCH 1/1] qga/linux: Add new api 'guest-network-get-route'
v1 -> v2 - Replace snprintf() to g_strdup_printf() to avoid memory problems. - Remove the parameter 'char ipAddress[16]' in function 'char *hexToIPAddress()'. - Add a piece of logic to skip traversing the first line of the file Dehan Meng (1): qga/linux: Add new api 'guest-network-get-route' qga/commands-posix.c | 78 ++ qga/commands-win32.c | 6 qga/qapi-schema.json | 80 3 files changed, 164 insertions(+) -- 2.35.1
Re: [PATCH] linux-user: Add FIFREEZE and FITHAW ioctls
Adding the linux-user maintainer to the CC list
On Tue, Feb 20, 2024 at 11:57:21AM +0100, Michael Vogt wrote:
> Add missing FIFREEZE and FITHAW ioctls.
>
> Signed-off-by: Michael Vogt
> ---
> linux-user/ioctls.h | 6 ++
> linux-user/syscall_defs.h | 3 +++
> 2 files changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé
>
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 071f7ca253..1aec9d5836 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -134,6 +134,12 @@
> IOCTL(FICLONE, IOC_W, TYPE_INT)
> IOCTL(FICLONERANGE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_file_clone_range)))
> #endif
> +#ifdef FIFREEZE
> + IOCTL(FIFREEZE, IOC_W | IOC_R, TYPE_INT)
> +#endif
> +#ifdef FITHAW
> + IOCTL(FITHAW, IOC_W | IOC_R, TYPE_INT)
> +#endif
>
> IOCTL(FIGETBSZ, IOC_R, MK_PTR(TYPE_LONG))
> #ifdef CONFIG_FIEMAP
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 77ba343c85..744fda599e 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -943,6 +943,9 @@ struct target_rtc_pll_info {
> #define TARGET_FICLONETARGET_IOW(0x94, 9, abi_int)
> #define TARGET_FICLONERANGE TARGET_IOW(0x94, 13, struct file_clone_range)
>
> +#define TARGET_FIFREEZETARGET_IOWR('X', 119, abi_int)
> +#define TARGET_FITHAWTARGET_IOWR('X', 120, abi_int)
> +
> /*
> * Note that the ioctl numbers for FS_IOC_
> * claim type "long" but the actual type used by the kernel is "int".
> --
> 2.43.0
>
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 1/1] loongarch: Change the UEFI loading mode to loongarch
From: Xianglai Li
The UEFI loading mode in loongarch is very different
from that in other architectures:loongarch's UEFI code
is in rom, while other architectures' UEFI code is in flash.
loongarch UEFI can be loaded as follows:
-machine virt,pflash=pflash0-format
-bios ./QEMU_EFI.fd
Other architectures load UEFI using the following methods:
-machine virt,pflash0=pflash0-format,pflash1=pflash1-format
loongarch's UEFI loading method makes qemu and libvirt incompatible
when using NVRAM, and the cost of loongarch's current loading method
far outweighs the benefits, so we decided to use the same UEFI loading
scheme as other architectures.
Cc: Andrea Bolognani
Cc: maob...@loongson.cn
Cc: Philippe Mathieu-Daudé
Cc: Song Gao
Cc: zhaotian...@loongson.cn
Signed-off-by: Xianglai Li
Tested-by: Andrea Bolognani
Reviewed-by: Song Gao
Message-Id:
<0bd892aa9b88e0f4cc904cb70efd0251fc1cde29.1708336919.git.lixiang...@loongson.cn>
Signed-off-by: Song Gao
---
hw/loongarch/acpi-build.c | 29 +--
hw/loongarch/virt.c | 101 ++--
include/hw/loongarch/virt.h | 10 ++--
3 files changed, 107 insertions(+), 33 deletions(-)
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index a1c4198741..6c75f216ea 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -314,16 +314,39 @@ static void build_pci_device_aml(Aml *scope,
LoongArchMachineState *lams)
static void build_flash_aml(Aml *scope, LoongArchMachineState *lams)
{
Aml *dev, *crs;
+MemoryRegion *flash_mem;
-hwaddr flash_base = VIRT_FLASH_BASE;
-hwaddr flash_size = VIRT_FLASH_SIZE;
+hwaddr flash0_base;
+hwaddr flash0_size;
+
+hwaddr flash1_base;
+hwaddr flash1_size;
+
+flash_mem = pflash_cfi01_get_memory(lams->flash[0]);
+flash0_base = flash_mem->addr;
+flash0_size = flash_mem->size;
+
+flash_mem = pflash_cfi01_get_memory(lams->flash[1]);
+flash1_base = flash_mem->addr;
+flash1_size = flash_mem->size;
dev = aml_device("FLS0");
aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
aml_append(dev, aml_name_decl("_UID", aml_int(0)));
crs = aml_resource_template();
-aml_append(crs, aml_memory32_fixed(flash_base, flash_size,
AML_READ_WRITE));
+aml_append(crs, aml_memory32_fixed(flash0_base, flash0_size,
+ AML_READ_WRITE));
+aml_append(dev, aml_name_decl("_CRS", crs));
+aml_append(scope, dev);
+
+dev = aml_device("FLS1");
+aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
+aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+
+crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(flash1_base, flash1_size,
+ AML_READ_WRITE));
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
}
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 0ad7d8c887..a7b9199e70 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -54,7 +54,9 @@ struct loaderparams {
const char *initrd_filename;
};
-static void virt_flash_create(LoongArchMachineState *lams)
+static PFlashCFI01 *virt_flash_create1(LoongArchMachineState *lams,
+ const char *name,
+ const char *alias_prop_name)
{
DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01);
@@ -66,45 +68,78 @@ static void virt_flash_create(LoongArchMachineState *lams)
qdev_prop_set_uint16(dev, "id1", 0x18);
qdev_prop_set_uint16(dev, "id2", 0x00);
qdev_prop_set_uint16(dev, "id3", 0x00);
-qdev_prop_set_string(dev, "name", "virt.flash");
-object_property_add_child(OBJECT(lams), "virt.flash", OBJECT(dev));
-object_property_add_alias(OBJECT(lams), "pflash",
+qdev_prop_set_string(dev, "name", name);
+object_property_add_child(OBJECT(lams), name, OBJECT(dev));
+object_property_add_alias(OBJECT(lams), alias_prop_name,
OBJECT(dev), "drive");
+return PFLASH_CFI01(dev);
+}
-lams->flash = PFLASH_CFI01(dev);
+static void virt_flash_create(LoongArchMachineState *lams)
+{
+lams->flash[0] = virt_flash_create1(lams, "virt.flash0", "pflash0");
+lams->flash[1] = virt_flash_create1(lams, "virt.flash1", "pflash1");
}
-static void virt_flash_map(LoongArchMachineState *lams,
- MemoryRegion *sysmem)
+static void virt_flash_map1(PFlashCFI01 *flash,
+hwaddr base, hwaddr size,
+MemoryRegion *sysmem)
{
-PFlashCFI01 *flash = lams->flash;
DeviceState *dev = DEVICE(flash);
-hwaddr base = VIRT_FLASH_BASE;
-hwaddr size = VIRT_FLASH_SIZE;
+BlockBackend *blk;
+hwaddr real_size = size;
+
+blk = pflash_cfi01_get_blk(flash);
+if (blk) {
+real_size = blk_getlength(blk);
+assert(real_size && real_size <= size);
+}
-assert(QEMU_
Re: [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers
On 1/19/24 03:31, BALATON Zoltan wrote:
After previous changes the hypercall handling in 7xx and 74xx
exception handlers can be folded into one if statement to simpilfy
this code. Also add "unlikely" to mark the less freqiently used branch
for the compiler.
Signed-off-by: BALATON Zoltan
Nice cleanup.
Reviewed-by: Harsh Prateek Bora
---
target/ppc/excp_helper.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 411d67376c..035a9fd968 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -762,26 +762,22 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception
*/
{
int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 7xx CPUs, so although the 7xx don't have
* HV mode, we need to keep hypercall support.
*/
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
powerpc_reset_excp_state(cpu);
return;
+} else {
+dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception
*/
@@ -907,26 +903,22 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception
*/
{
int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 74xx CPUs, so although the 74xx don't have
* HV mode, we need to keep hypercall support.
*/
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
powerpc_reset_excp_state(cpu);
return;
+} else {
+dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception
*/
Re: [RFC PATCH 0/2] hw: Replace cpu_interrupt() calls by qemu_irq(QDev GPIO)
On 20/02/2024 19:26, Philippe Mathieu-Daudé wrote: Hi, cpu_interrupt() doesn't scale well with heterogenous machines because its mask is target specific (defined in target/ARCH/cpu.h). While it is (often...) used by target-specific hw to notify cpu, there is no restriction to use such target-specific hw in a heterogeneous setup, where it'd still target the same kind of target cpus. The Alpha Typhoon HW is unlikely to be used heterogeneously, but it is the simplest case I found to illustrate how I plan to remove cpu_interrupt() calls from hw/: use the QDev GPIO API. Does that sound good enough? I think the basic mechanism of setting/resetting the interrupt using qdev GPIOs should work, but I wonder if there isn't a better way of defining them to avoid more #ifdeffery. Is it possible to come up with some declarative syntax in either CPUClass or CPUClass::sysemu_ops that would avoid the developer manually having to call qdev_init_gpio_in_named() manually during CPU init() and/or create the various _irq() callback functions by hand? Thanks, Phil. Philippe Mathieu-Daudé (2): target/alpha: Expose TMR and SMP IRQ lines via QDev hw/alpha/typhoon: Set CPU IRQs using QDev API hw/alpha/typhoon.c | 15 +-- target/alpha/cpu.c | 30 ++ 2 files changed, 39 insertions(+), 6 deletions(-) ATB, Mark.
[PATCH 1/6] hw/cxl/cxl-host: Fix missing ERRP_GUARD() in cxl_fixed_memory_window_config()
From: Zhao Liu
As the comment in qapi/error, dereferencing @errp requires
ERRP_GUARD():
* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
* - It must not be dereferenced, because it may be null.
* - It should not be passed to error_prepend() or
* error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.
*
* Using it when it's not needed is safe, but please avoid cluttering
* the source with useless code.
Currently, since machine_set_cfmw() - the caller of
cxl_fixed_memory_window_config() - doesn't get the NULL errp parameter
as the "set" method of object property, cxl_fixed_memory_window_config()
doesn't trigger the dereference issue.
To follow the requirement of errp, add missing ERRP_GUARD() in
cxl_fixed_memory_window_config().
Suggested-by: Markus Armbruster
Signed-off-by: Zhao Liu
---
Suggested by credit:
Markus: Referred his explanation about ERRP_GUARD().
---
hw/cxl/cxl-host.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 2aa776c79c74..c5f5fcfd64d0 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -26,6 +26,7 @@ static void cxl_fixed_memory_window_config(CXLState
*cxl_state,
CXLFixedMemoryWindowOptions *object,
Error **errp)
{
+ERRP_GUARD();
g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
strList *target;
int i;
--
2.34.1
Re: [PATCH 5/6] hw/pci-bridge/cxl_upstream: Fix missing ERRP_GUARD() in cxl_usp_realize()
On Wed, Feb 21, 2024 at 12:49:46PM +0100, Markus Armbruster wrote:
> Date: Wed, 21 Feb 2024 12:49:46 +0100
> From: Markus Armbruster
> Subject: Re: [PATCH 5/6] hw/pci-bridge/cxl_upstream: Fix missing
> ERRP_GUARD() in cxl_usp_realize()
>
> Zhao Liu writes:
>
> > From: Zhao Liu
> >
> > As the comment in qapi/error, dereferencing @errp requires
> > ERRP_GUARD():
> >
> > * = Why, when and how to use ERRP_GUARD() =
> > *
> > * Without ERRP_GUARD(), use of the @errp parameter is restricted:
> > * - It must not be dereferenced, because it may be null.
> > * - It should not be passed to error_prepend() or
> > * error_append_hint(), because that doesn't work with &error_fatal.
> > * ERRP_GUARD() lifts these restrictions.
> > *
> > * To use ERRP_GUARD(), add it right at the beginning of the function.
> > * @errp can then be used without worrying about the argument being
> > * NULL or &error_fatal.
> > *
> > * Using it when it's not needed is safe, but please avoid cluttering
> > * the source with useless code.
> >
> > Currently, since cxl_usp_realize() - as a PCIDeviceClass.realize()
> > method - doesn't get the NULL errp parameter, it doesn't trigger the
> > dereference issue.
> >
> > To follow the requirement of errp, add missing ERRP_GUARD() in
> > cxl_usp_realize()().
> >
> > Suggested-by: Markus Armbruster
> > Signed-off-by: Zhao Liu
> > ---
> > Suggested by credit:
> > Markus: Referred his explanation about ERRP_GUARD().
> > ---
> > hw/pci-bridge/cxl_upstream.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
> > index e87eb4017713..03d123cca0ef 100644
> > --- a/hw/pci-bridge/cxl_upstream.c
> > +++ b/hw/pci-bridge/cxl_upstream.c
> > @@ -289,6 +289,7 @@ static void free_default_cdat_table(CDATSubHeader
> > **cdat_table, int num,
> >
> > static void cxl_usp_realize(PCIDevice *d, Error **errp)
> > {
> > +ERRP_GUARD();
> > PCIEPort *p = PCIE_PORT(d);
> > CXLUpstreamPort *usp = CXL_USP(d);
> > CXLComponentState *cxl_cstate = &usp->cxl_cstate;
>
> The dereference is
>
>cxl_doe_cdat_init(cxl_cstate, errp);
>if (*errp) {
>goto err_cap;
>}
>
> As noted in review of PATCH 3, we check *errp, because
> cxl_doe_cdat_init() returns void. Could be improved to return bool,
> along with its callees ct3_load_cdat() and ct3_build_cdat(), but that's
> a slightly more ambitious cleanup, so
>
> Reviewed-by: Markus Armbruster
>
Thanks! It's a good idea and I can continue to consider such the cleanup
in the follow up.
Will also add the dereference description in commit message to make
review easier.
Regards,
Zhao
[Stable-8.2.2 37/60] tests/acpi: Allow update of DSDT.cxl
From: Jonathan Cameron The _STA value returned currently indicates the ACPI0017 device is not enabled. Whilst this isn't a real device, setting _STA like this may prevent an OS from enumerating it correctly and hence from parsing the CEDT table. Signed-off-by: Jonathan Cameron Message-Id: <20240126120132.24248-11-jonathan.came...@huawei.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin (cherry picked from commit 14ec4ff3e4293635240ba5a7afe7a0f3ba447d31) Signed-off-by: Michael Tokarev diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..9ce0f596cc 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/q35/DSDT.cxl", -- 2.39.2
Re: [PATCH V4 5/5] migration: simplify exec migration functions
Het Gala, Peter, or Fabiano, please review.
Steve Sistare writes:
> Simplify the exec migration code by using list utility functions.
>
> As a side effect, this also fixes a minor memory leak. On function return,
> "g_auto(GStrv) argv" frees argv and each element, which is wrong, because
> the function does not own the individual elements. To compensate, the code
> uses g_steal_pointer which NULLs argv and prevents the destructor from
> running, but argv is leaked.
>
> Fixes: cbab4face57b ("migration: convert exec backend ...")
> Signed-off-by: Steve Sistare
> ---
> migration/exec.c | 58
>
> 1 file changed, 8 insertions(+), 50 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 47d2f3b..1312ca7 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -19,12 +19,12 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qemu/strList.h"
> #include "channel.h"
> #include "exec.h"
> #include "migration.h"
> #include "io/channel-command.h"
> #include "trace.h"
> -#include "qemu/cutils.h"
>
> #ifdef WIN32
> const char *exec_get_cmd_path(void)
> @@ -39,51 +39,16 @@ const char *exec_get_cmd_path(void)
> }
> #endif
>
> -/* provides the length of strList */
> -static int
> -str_list_length(strList *list)
> -{
> -int len = 0;
> -strList *elem;
> -
> -for (elem = list; elem != NULL; elem = elem->next) {
> -len++;
> -}
> -
> -return len;
> -}
> -
> -static void
> -init_exec_array(strList *command, char **argv, Error **errp)
> -{
> -int i = 0;
> -strList *lst;
> -
> -for (lst = command; lst; lst = lst->next) {
> -argv[i++] = lst->value;
> -}
> -
> -argv[i] = NULL;
> -return;
> -}
> -
> void exec_start_outgoing_migration(MigrationState *s, strList *command,
> Error **errp)
> {
> -QIOChannel *ioc;
> -
> -int length = str_list_length(command);
> -g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
> -
> -init_exec_array(command, argv, errp);
> +QIOChannel *ioc = NULL;
> +g_auto(GStrv) argv = strv_from_strList(command);
> +const char * const *args = (const char * const *) argv;
> g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>
> trace_migration_exec_outgoing(new_command);
> -ioc = QIO_CHANNEL(
> -qio_channel_command_new_spawn(
> -(const char * const *) g_steal_pointer(&argv),
> -O_RDWR,
> -errp));
> +ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
> if (!ioc) {
> return;
> }
> @@ -105,19 +70,12 @@ static gboolean
> exec_accept_incoming_migration(QIOChannel *ioc,
> void exec_start_incoming_migration(strList *command, Error **errp)
> {
> QIOChannel *ioc;
> -
> -int length = str_list_length(command);
> -g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
> -
> -init_exec_array(command, argv, errp);
> +g_auto(GStrv) argv = strv_from_strList(command);
> +const char * const *args = (const char * const *) argv;
> g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>
> trace_migration_exec_incoming(new_command);
> -ioc = QIO_CHANNEL(
> -qio_channel_command_new_spawn(
> -(const char * const *) g_steal_pointer(&argv),
> -O_RDWR,
> -errp));
> +ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
> if (!ioc) {
> return;
> }
Re: [PATCH 1/6] hw/arm: Inline sysbus_create_simple(PL110 / PL111)
On 19/02/2024 13:35, Peter Maydell wrote: On Mon, 19 Feb 2024 at 13:33, Mark Cave-Ayland wrote: On 19/02/2024 13:05, Peter Maydell wrote: On Mon, 19 Feb 2024 at 12:49, Mark Cave-Ayland wrote: On 19/02/2024 12:00, BALATON Zoltan wrote: For new people trying to contribute to QEMU QDev is overwhelming so having some way to need less of it to do simple things would help them to get started. It depends what how you define "simple": for QEMU developers most people search for similar examples in the codebase and copy/paste them. I'd much rather have a slightly longer, but consistent API for setting properties rather than coming up with many special case wrappers that need to be maintained just to keep the line count down for "simplicity". I think that Phil's approach here is the best one for now, particularly given that it allows us to take another step towards heterogeneous machines. As the work in this area matures it might be that we can consider other approaches, but that's not a decision that can be made right now and so shouldn't be a reason to block this change. Mmm. It's unfortunate that we're working with C, so we're a bit limited in what tools we have to try to make a better and lower-boilerplate interface for the "create, configure, realize and wire up devices" task. (I think you could do much better in a higher level language...) sysbus_create_simple() was handy at the time, but it doesn't work so well for more complicated SoC-based boards. It's noticeable that if you look at the code that uses it, it's almost entirely the older and less maintained board models, especially those which don't actually model an SoC and just have the board code create all the devices. Yeah I was thinking that you'd use the DSL (e.g. YAML templates or similar) to provide some of the boilerplating around common actions, rather than the C API itself. Even better, once everything has been moved to use a DSL then the C API shouldn't really matter so much as it is no longer directly exposed to the user. That does feel like it's rather a long way away, though, so there might be scope for improving our C APIs in the meantime. (Also, doing the boilerplating with fragments of YAML or whatever means that checking of eg typos and other syntax errors shifts from compile time to runtime, which is a shame.) I think most people who frequently use QOM/qdev have ideas as to how to improve the C API, but what seems clear to me is that the analysis of scenarios by Phil, Markus and others as part of the heterogeneous machine work is useful and should be used to help guide future work in this area. If there are proposed changes in the C API, I'd be keen to see a short RFC detailing the changes and their rationale to aid developers/reviewers before they are introduced into the codebase. ATB, Mark.
[PATCH v4] pc: q35: Bump max_cpus to 4096 vcpus
Since commit f10a570b093e6 ("KVM: x86: Add CONFIG_KVM_MAX_NR_VCPUS to allow up
to 4096 vCPUs")
Linux kernel can support upto a maximum number of 4096 vCPUS when MAXSMP is
enabled in the kernel. At present, QEMU has been tested to correctly boot a
linux guest with 4096 vcpus with edk2 pending various upstream EDK2 fixes
which will probably be in the 2024-05 release to be released in the coming
months. With current seabios firmware, it boots fine with 4096 vcpus already.
So bump up the value max_cpus to 4096 for q35 machines versions 9 and newer.
Q35 machines versions 8.2 and older continue to support 1024 maximum vcpus
as before for compatibility reasons.
If KVM is not able to support the specified number of vcpus, QEMU would
return the following error messages:
$ ./qemu-system-x86_64 -cpu host -accel kvm -machine q35 -smp 1728
qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (1728)
exceeds the recommended cpus supported by KVM (12)
qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus requested
(1728) exceeds the recommended cpus supported by KVM (12)
Number of SMP cpus requested (1728) exceeds the maximum cpus supported by KVM
(1024)
Cc: Daniel P. Berrangé
Cc: Igor Mammedov
Cc: Michael S. Tsirkin
Cc: Julia Suvorova
Cc: kra...@redhat.com
Reviewed-by: Daniel P. Berrangé
Signed-off-by: Ani Sinha
---
hw/i386/pc_q35.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Changelog:
v4: tweaked commit message as per suggestion from danpb explicitly
stating that 4096 vcpus work with edk2 fixes that are going to be
available in the coming edk2 release.
v3: bump up to 4096 vcpus. It has now been tested to work with edk2.
See RH Jira: https://issues.redhat.com/browse/RHEL-22202
v2: bump up the vcpu number to 1856. Add failure messages from ekd2 in
the commit description.
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d346fa3b1d..ae60e6b919 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -366,7 +366,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->default_nic = "e1000e";
m->default_kernel_irqchip_split = false;
m->no_floppy = 1;
-m->max_cpus = 1024;
+m->max_cpus = 4096;
m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -387,6 +387,7 @@ static void pc_q35_8_2_machine_options(MachineClass *m)
{
pc_q35_9_0_machine_options(m);
m->alias = NULL;
+m->max_cpus = 1024;
compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len);
compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len);
}
--
2.42.0
Re: [PATCH 18/23] plugins: add an API to read registers
On 2024/02/21 19:02, Alex Bennée wrote:
Akihiko Odaki writes:
On 2024/02/20 23:14, Alex Bennée wrote:
Akihiko Odaki writes:
On 2024/02/17 1:30, Alex Bennée wrote:
We can only request a list of registers once the vCPU has been
initialised so the user needs to use either call the get function on
vCPU initialisation or during the translation phase.
We don't expose the reg number to the plugin instead hiding it
behind
an opaque handle. This allows for a bit of future proofing should the
internals need to be changed while also being hashed against the
CPUClass so we can handle different register sets per-vCPU in
hetrogenous situations.
Having an internal state within the plugins also allows us to expand
the interface in future (for example providing callbacks on register
change if the translator can track changes).
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
Cc: Akihiko Odaki
Message-Id: <20240103173349.398526-39-alex.ben...@linaro.org>
Based-on: <20231025093128.33116-18-akihiko.od...@daynix.com>
Signed-off-by: Alex Bennée
Reviewed-by: Pierrick Bouvier
+/*
+ * Register handles
+ *
+ * The plugin infrastructure keeps hold of these internal data
+ * structures which are presented to plugins as opaque handles. They
+ * are global to the system and therefor additions to the hash table
+ * must be protected by the @reg_handle_lock.
+ *
+ * In order to future proof for up-coming heterogeneous work we want
+ * different entries for each CPU type while sharing them in the
+ * common case of multiple cores of the same type.
+ */
+
+static QemuMutex reg_handle_lock;
+
+struct qemu_plugin_register {
+const char *name;
+int gdb_reg_num;
+};
+
+static GHashTable *reg_handles; /* hash table of PluginReg */
+
+/* Generate a stable key - would xxhash be overkill? */
+static gpointer cpu_plus_reg_to_key(CPUState *cs, int gdb_regnum)
+{
+uintptr_t key = (uintptr_t) cs->cc;
+key ^= gdb_regnum;
+return GUINT_TO_POINTER(key);
+}
I have pointed out this is theoretically prone to collisions and
unsafe.
How is it unsafe? The aim is to share handles for the same CPUClass
rather than having a unique handle per register/cpu combo.
THe intention is legitimate, but the implementation is not safe. It
assumes (uintptr)cs->cc ^ gdb_regnum is unique, but there is no such
guarantee. The key of GHashTable must be unique; generating hashes of
keys should be done with hash_func given to g_hash_table_new().
This isn't a hash its a non-unique key. It is however unique for
the same register on the same class of CPU so for each vCPU in a system
can share the same opaque handles.
The hashing is done internally by glib. We would assert if there was a
duplicate key referring to a different register.
I'm unsure what you want here? Do you have a suggestion for the key
generation algorithm? As the comment notes I did consider a more complex
mixing algorithm using xxhash but that wouldn't guarantee no clash
either.
I suggest using a struct that holds both of cs->cc and gdb_regnum, and
pass g_direct_equal() and g_direct_hash() to g_hash_table_new().
[Stable-8.2.2 32/60] hw/cxl/device: read from register values in mdev_reg_read()
From: Hyeonggon Yoo <42.hye...@gmail.com>
In the current mdev_reg_read() implementation, it consistently returns
that the Media Status is Ready (01b). This was fine until commit
25a52959f99d ("hw/cxl: Add support for device sanitation") because the
media was presumed to be ready.
However, as per the CXL 3.0 spec "8.2.9.8.5.1 Sanitize (Opcode 4400h)",
during sanitation, the Media State should be set to Disabled (11b). The
mentioned commit correctly sets it to Disabled, but mdev_reg_read()
still returns Media Status as Ready.
To address this, update mdev_reg_read() to read register values instead
of returning dummy values.
Note that __toggle_media() managed to not only write something
that no one read, it did it to the wrong register storage and
so changed the reported mailbox size which was definitely not
the intent. That gets fixed as a side effect of allocating
separate state storage for this register.
Fixes: commit 25a52959f99d ("hw/cxl: Add support for device sanitation")
Signed-off-by: Hyeonggon Yoo <42.hye...@gmail.com>
Reviewed-by: Fan Ni
Signed-off-by: Jonathan Cameron
Message-Id: <20240126120132.24248-7-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
(cherry picked from commit f7509f462c788a347521f90f19d623908c4fbcc5)
Signed-off-by: Michael Tokarev
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 61a3c4dc2e..40b619ffd9 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -229,12 +229,9 @@ static void mailbox_reg_write(void *opaque, hwaddr offset,
uint64_t value,
static uint64_t mdev_reg_read(void *opaque, hwaddr offset, unsigned size)
{
-uint64_t retval = 0;
-
-retval = FIELD_DP64(retval, CXL_MEM_DEV_STS, MEDIA_STATUS, 1);
-retval = FIELD_DP64(retval, CXL_MEM_DEV_STS, MBOX_READY, 1);
+CXLDeviceState *cxl_dstate = opaque;
-return retval;
+return cxl_dstate->memdev_status;
}
static void ro_reg_write(void *opaque, hwaddr offset, uint64_t value,
@@ -371,7 +368,15 @@ static void mailbox_reg_init_common(CXLDeviceState
*cxl_dstate)
cxl_dstate->mbox_msi_n = msi_n;
}
-static void memdev_reg_init_common(CXLDeviceState *cxl_dstate) { }
+static void memdev_reg_init_common(CXLDeviceState *cxl_dstate)
+{
+uint64_t memdev_status_reg;
+
+memdev_status_reg = FIELD_DP64(0, CXL_MEM_DEV_STS, MEDIA_STATUS, 1);
+memdev_status_reg = FIELD_DP64(memdev_status_reg, CXL_MEM_DEV_STS,
+ MBOX_READY, 1);
+cxl_dstate->memdev_status = memdev_status_reg;
+}
void cxl_device_register_init_t3(CXLType3Dev *ct3d)
{
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index befb5f884b..31d2afcd3d 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -202,6 +202,9 @@ typedef struct cxl_device_state {
};
};
+/* Stash the memory device status value */
+uint64_t memdev_status;
+
struct {
bool set;
uint64_t last_set;
@@ -353,8 +356,10 @@ static inline void __toggle_media(CXLDeviceState
*cxl_dstate, int val)
{
uint64_t dev_status_reg;
-dev_status_reg = FIELD_DP64(0, CXL_MEM_DEV_STS, MEDIA_STATUS, val);
-cxl_dstate->mbox_reg_state64[R_CXL_MEM_DEV_STS] = dev_status_reg;
+dev_status_reg = cxl_dstate->memdev_status;
+dev_status_reg = FIELD_DP64(dev_status_reg, CXL_MEM_DEV_STS, MEDIA_STATUS,
+val);
+cxl_dstate->memdev_status = dev_status_reg;
}
#define cxl_dev_disable_media(cxlds)\
do { __toggle_media((cxlds), 0x3); } while (0)
--
2.39.2
Re: [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers
On 1/19/24 03:31, BALATON Zoltan wrote:
After previous changes the hypercall handling in 7xx and 74xx
exception handlers can be folded into one if statement to simpilfy
this code. Also add "unlikely" to mark the less freqiently used branch
for the compiler.
Signed-off-by: BALATON Zoltan
Reviewed-by: Harsh Prateek Bora
---
target/ppc/excp_helper.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 411d67376c..035a9fd968 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -762,26 +762,22 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception
*/
{
int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 7xx CPUs, so although the 7xx don't have
* HV mode, we need to keep hypercall support.
*/
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
powerpc_reset_excp_state(cpu);
return;
+} else {
+dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception
*/
@@ -907,26 +903,22 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception
*/
{
int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 74xx CPUs, so although the 74xx don't have
* HV mode, we need to keep hypercall support.
*/
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
powerpc_reset_excp_state(cpu);
return;
+} else {
+dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception
*/
[Stable-8.2.2 53/60] target/i386: Generate an illegal opcode exception on cmp instructions with lock prefix
From: Ziqiao Kong
target/i386: As specified by Intel Manual Vol2 3-180, cmp instructions
are not allowed to have lock prefix and a `UD` should be raised. Without
this patch, s1->T0 will be uninitialized and used in the case OP_CMPL.
Signed-off-by: Ziqiao Kong
Message-ID: <20240215095015.570748-2-ziqiaok...@gmail.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
(cherry picked from commit 99d0dcd7f102c07a510200d768cae65e5db25d23)
Signed-off-by: Michael Tokarev
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 8fd49ff474..83c2b40480 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1480,12 +1480,13 @@ static bool check_iopl(DisasContext *s)
/* if d == OR_TMP0, it means memory operand (address in A0) */
static void gen_op(DisasContext *s1, int op, MemOp ot, int d)
{
+/* Invalid lock prefix when destination is not memory or OP_CMPL. */
+if ((d != OR_TMP0 || op == OP_CMPL) && s1->prefix & PREFIX_LOCK) {
+gen_illegal_opcode(s1);
+return;
+}
+
if (d != OR_TMP0) {
-if (s1->prefix & PREFIX_LOCK) {
-/* Lock prefix when destination is not memory. */
-gen_illegal_opcode(s1);
-return;
-}
gen_op_mov_v_reg(s1, ot, s1->T0, d);
} else if (!(s1->prefix & PREFIX_LOCK)) {
gen_op_ld_v(s1, ot, s1->T0, s1->A0);
--
2.39.2
