From 35e71ec53581fc8a90dfff1565e4ba344945cf80 Mon Sep 17 00:00:00 2001 From: Shiva sagar Myana Date: Tue, 18 Jun 2024 16:22:20 +0100 Subject: [PATCH 01/18] hw/net/can/xlnx-versal-canfd: Fix sorting of the tx queue Returning an uint32_t casted to a gint from g_cmp_ids causes the tx queue to become wrongly sorted when executing g_slist_sort. Fix this by always returning -1 or 1 from g_cmp_ids based on the ID comparison instead. Also, if two message IDs are the same, sort them by using their index and transmit the message at the lowest index first. Signed-off-by: Shiva sagar Myana Reviewed-by: Francisco Iglesias Message-id: 20240603051732.3334571-1-Shivasagar.Myana@amd.com Signed-off-by: Peter Maydell --- hw/net/can/xlnx-versal-canfd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/net/can/xlnx-versal-canfd.c b/hw/net/can/xlnx-versal-canfd.c index 47a14cfe63..5f083c21e9 100644 --- a/hw/net/can/xlnx-versal-canfd.c +++ b/hw/net/can/xlnx-versal-canfd.c @@ -1312,7 +1312,10 @@ static gint g_cmp_ids(gconstpointer data1, gconstpointer data2) tx_ready_reg_info *tx_reg_1 = (tx_ready_reg_info *) data1; tx_ready_reg_info *tx_reg_2 = (tx_ready_reg_info *) data2; - return tx_reg_1->can_id - tx_reg_2->can_id; + if (tx_reg_1->can_id == tx_reg_2->can_id) { + return (tx_reg_1->reg_num < tx_reg_2->reg_num) ? -1 : 1; + } + return (tx_reg_1->can_id < tx_reg_2->can_id) ? -1 : 1; } static void free_list(GSList *list) From 7edca16e745ca2105066832c9da34077af41347e Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz Date: Tue, 18 Jun 2024 16:22:20 +0100 Subject: [PATCH 02/18] hw/arm/sbsa-ref: switch to 1GHz timer frequency Updated firmware for QEMU CI is already in merge queue so we can move platform to be future proof. All supported cpus work fine with 1GHz timer frequency when firmware is fresh enough. Signed-off-by: Marcin Juszkiewicz Reviewed-by: Leif Lindholm Message-id: 20240531093729.220758-2-marcin.juszkiewicz@linaro.org Signed-off-by: Peter Maydell --- hw/arm/sbsa-ref.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index e884692f07..87884400e3 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -62,16 +62,12 @@ /* * Generic timer frequency in Hz (which drives both the CPU generic timers - * and the SBSA watchdog-timer). Older versions of the TF-A firmware - * typically used with sbsa-ref (including the binaries in our Avocado test - * Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef - * assume it is this value. + * and the SBSA watchdog-timer). Older (<2.11) versions of the TF-A firmware + * assumed 62.5MHz here. * - * TODO: this value is not architecturally correct for an Armv8.6 or - * better CPU, so we should move to 1GHz once the TF-A fix above has - * made it into a release and into our Avocado test. + * Starting with Armv8.6 CPU 1GHz timer frequency is mandated. */ -#define SBSA_GTIMER_HZ 62500000 +#define SBSA_GTIMER_HZ 1000000000 enum { SBSA_FLASH, From 7175a562f157d39725ab396e39c1e8e410d206b3 Mon Sep 17 00:00:00 2001 From: "Edgar E. Iglesias" Date: Tue, 18 Jun 2024 16:22:20 +0100 Subject: [PATCH 03/18] hw/intc/arm_gic: Fix deactivation of SPI lines Julien reported that he has seen strange behaviour when running Xen on QEMU using GICv2. When Xen migrates a guest's vCPU from one pCPU to another while the vCPU is handling an interrupt, the guest is unable to properly deactivate interrupts. Looking at it a little closer, our GICv2 model treats deactivation of SPI lines as if they were PPI's, i.e banked per CPU core. The state for active interrupts should only be banked for PPI lines, not for SPI lines. Make deactivation of SPI lines unbanked, similar to how we handle writes to GICD_ICACTIVER. Reported-by: Julien Grall Signed-off-by: Edgar E. Iglesias Message-id: 20240605143044.2029444-2-edgar.iglesias@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/intc/gic_internal.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index 8d29b40ca1..8ddbf554c6 100644 --- a/hw/intc/gic_internal.h +++ b/hw/intc/gic_internal.h @@ -280,6 +280,8 @@ static inline void gic_set_active(GICState *s, int irq, int cpu) static inline void gic_clear_active(GICState *s, int irq, int cpu) { + unsigned int cm; + if (gic_is_vcpu(cpu)) { uint32_t *entry = gic_get_lr_entry(s, irq, cpu); GICH_LR_CLEAR_ACTIVE(*entry); @@ -301,11 +303,13 @@ static inline void gic_clear_active(GICState *s, int irq, int cpu) * the GIC is secure. */ if (!s->security_extn || GIC_DIST_TEST_GROUP(phys_irq, 1 << rcpu)) { - GIC_DIST_CLEAR_ACTIVE(phys_irq, 1 << rcpu); + cm = phys_irq < GIC_INTERNAL ? 1 << rcpu : ALL_CPU_MASK; + GIC_DIST_CLEAR_ACTIVE(phys_irq, cm); } } } else { - GIC_DIST_CLEAR_ACTIVE(irq, 1 << cpu); + cm = irq < GIC_INTERNAL ? 1 << cpu : ALL_CPU_MASK; + GIC_DIST_CLEAR_ACTIVE(irq, cm); } } From 9b113a09ff34857d463c130faa873c7b6fc4a004 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Tue, 18 Jun 2024 16:22:21 +0100 Subject: [PATCH 04/18] hw/arm/xilinx_zynq: Fix IRQ/FIQ routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the system bus interrupt line to CPU core assignment. Fixes: ddcf58e044ce0 ("hw/arm/xilinx_zynq: Support up to two CPU cores") Signed-off-by: Sebastian Huber Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240610052906.4432-1-sebastian.huber@embedded-brains.de Signed-off-by: Peter Maydell --- hw/arm/xilinx_zynq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index 7f7a3d23fb..c79661bbc1 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -252,10 +252,11 @@ static void zynq_init(MachineState *machine) zynq_binfo.gic_cpu_if_addr = MPCORE_PERIPHBASE + 0x100; sysbus_create_varargs("l2x0", MPCORE_PERIPHBASE + 0x2000, NULL); for (n = 0; n < smp_cpus; n++) { + /* See "hw/intc/arm_gic.h" for the IRQ line association */ DeviceState *cpudev = DEVICE(zynq_machine->cpu[n]); - sysbus_connect_irq(busdev, (2 * n) + 0, + sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); - sysbus_connect_irq(busdev, (2 * n) + 1, + sysbus_connect_irq(busdev, smp_cpus + n, qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); } From 057f7680f4ed1cc27a0520c0628bfb94f850c56a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 18 Jun 2024 16:22:21 +0100 Subject: [PATCH 05/18] scripts/coverity-scan/COMPONENTS.md: Update paths to match gitlab CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 83aa1baa069c we have been running the build for Coverity Scan as a Gitlab CI job, rather than the old setup where it was run on a local developer's machine. This is working well, but the absolute paths of files are different for the Gitlab CI job, which means that the regexes we use to identify Coverity components no longer work. With Gitlab CI builds the file paths are of the form /builds/qemu-project/qemu/accel/kvm/kvm-all.c rather than the old /qemu/accel/kvm/kvm-all.c and our regexes all don't match. Update all the regexes to start with .*/qemu/ . This will hopefully avoid the need to change them again in future if the build path changes again. This change was made with a search-and-replace of (/qemu)? to .*/qemu . Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-2-peter.maydell@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 104 ++++++++++++++-------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 1537e49cd5..98d4bcd6a5 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -1,157 +1,157 @@ This is the list of currently configured Coverity components: alpha - ~ (/qemu)?((/include)?/hw/alpha/.*|/target/alpha/.*) + ~ .*/qemu((/include)?/hw/alpha/.*|/target/alpha/.*) arm - ~ (/qemu)?((/include)?/hw/arm/.*|(/include)?/hw/.*/(arm|allwinner-a10|bcm28|digic|exynos|imx|omap|stellaris|pxa2xx|versatile|zynq|cadence).*|/hw/net/xgmac.c|/hw/ssi/xilinx_spips.c|/target/arm/.*) + ~ .*/qemu((/include)?/hw/arm/.*|(/include)?/hw/.*/(arm|allwinner-a10|bcm28|digic|exynos|imx|omap|stellaris|pxa2xx|versatile|zynq|cadence).*|/hw/net/xgmac.c|/hw/ssi/xilinx_spips.c|/target/arm/.*) avr - ~ (/qemu)?((/include)?/hw/avr/.*|/target/avr/.*) + ~ .*/qemu((/include)?/hw/avr/.*|/target/avr/.*) cris - ~ (/qemu)?((/include)?/hw/cris/.*|/target/cris/.*) + ~ .*/qemu((/include)?/hw/cris/.*|/target/cris/.*) hexagon-gen (component should be ignored in analysis) - ~ (/qemu)?(/target/hexagon/.*generated.*) + ~ .*/qemu(/target/hexagon/.*generated.*) hexagon - ~ (/qemu)?(/target/hexagon/.*) + ~ .*/qemu(/target/hexagon/.*) hppa - ~ (/qemu)?((/include)?/hw/hppa/.*|/target/hppa/.*) + ~ .*/qemu((/include)?/hw/hppa/.*|/target/hppa/.*) i386 - ~ (/qemu)?((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c) + ~ .*/qemu((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c) loongarch - ~ (/qemu)?((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) + ~ .*/qemu((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) m68k - ~ (/qemu)?((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*|(/include)?/hw/nubus/.*) + ~ .*/qemu((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*|(/include)?/hw/nubus/.*) microblaze - ~ (/qemu)?((/include)?/hw/microblaze/.*|/target/microblaze/.*) + ~ .*/qemu((/include)?/hw/microblaze/.*|/target/microblaze/.*) mips - ~ (/qemu)?((/include)?/hw/mips/.*|/target/mips/.*) + ~ .*/qemu((/include)?/hw/mips/.*|/target/mips/.*) openrisc - ~ (/qemu)?((/include)?/hw/openrisc/.*|/target/openrisc/.*) + ~ .*/qemu((/include)?/hw/openrisc/.*|/target/openrisc/.*) ppc - ~ (/qemu)?((/include)?/hw/ppc/.*|/target/ppc/.*|/hw/pci-host/(uninorth.*|dec.*|prep.*|ppc.*)|/hw/misc/macio/.*|(/include)?/hw/.*/(xics|openpic|spapr).*) + ~ .*/qemu((/include)?/hw/ppc/.*|/target/ppc/.*|/hw/pci-host/(uninorth.*|dec.*|prep.*|ppc.*)|/hw/misc/macio/.*|(/include)?/hw/.*/(xics|openpic|spapr).*) riscv - ~ (/qemu)?((/include)?/hw/riscv/.*|/target/riscv/.*|/hw/.*/(riscv_|ibex_|sifive_).*) + ~ .*/qemu((/include)?/hw/riscv/.*|/target/riscv/.*|/hw/.*/(riscv_|ibex_|sifive_).*) rx - ~ (/qemu)?((/include)?/hw/rx/.*|/target/rx/.*) + ~ .*/qemu((/include)?/hw/rx/.*|/target/rx/.*) s390 - ~ (/qemu)?((/include)?/hw/s390x/.*|/target/s390x/.*|/hw/.*/s390_.*) + ~ .*/qemu((/include)?/hw/s390x/.*|/target/s390x/.*|/hw/.*/s390_.*) sh4 - ~ (/qemu)?((/include)?/hw/sh4/.*|/target/sh4/.*) + ~ .*/qemu((/include)?/hw/sh4/.*|/target/sh4/.*) sparc - ~ (/qemu)?((/include)?/hw/sparc(64)?.*|/target/sparc/.*|/hw/.*/grlib.*|/hw/display/cg3.c) + ~ .*/qemu((/include)?/hw/sparc(64)?.*|/target/sparc/.*|/hw/.*/grlib.*|/hw/display/cg3.c) tricore - ~ (/qemu)?((/include)?/hw/tricore/.*|/target/tricore/.*) + ~ .*/qemu((/include)?/hw/tricore/.*|/target/tricore/.*) xtensa - ~ (/qemu)?((/include)?/hw/xtensa/.*|/target/xtensa/.*) + ~ .*/qemu((/include)?/hw/xtensa/.*|/target/xtensa/.*) 9pfs - ~ (/qemu)?(/hw/9pfs/.*|/fsdev/.*) + ~ .*/qemu(/hw/9pfs/.*|/fsdev/.*) audio - ~ (/qemu)?((/include)?/(audio|hw/audio)/.*) + ~ .*/qemu((/include)?/(audio|hw/audio)/.*) block - ~ (/qemu)?(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*) + ~ .*/qemu(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*) char - ~ (/qemu)?(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*) + ~ .*/qemu(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*) crypto - ~ (/qemu)?((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) + ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) disas - ~ (/qemu)?((/include)?/disas.*) + ~ .*/qemu((/include)?/disas.*) fpu - ~ (/qemu)?((/include)?(/fpu|/libdecnumber)/.*) + ~ .*/qemu((/include)?(/fpu|/libdecnumber)/.*) io - ~ (/qemu)?((/include)?/io/.*) + ~ .*/qemu((/include)?/io/.*) ipmi - ~ (/qemu)?((/include)?/hw/ipmi/.*) + ~ .*/qemu((/include)?/hw/ipmi/.*) migration - ~ (/qemu)?((/include)?/migration/.*) + ~ .*/qemu((/include)?/migration/.*) monitor - ~ (/qemu)?(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*) + ~ .*/qemu(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*) nbd - ~ (/qemu)?(/nbd/.*|/include/block/nbd.*|/qemu-nbd\.c) + ~ .*/qemu(/nbd/.*|/include/block/nbd.*|/qemu-nbd\.c) net - ~ (/qemu)?((/include)?(/hw)?/(net|rdma)/.*) + ~ .*/qemu((/include)?(/hw)?/(net|rdma)/.*) pci - ~ (/qemu)?(/include)?/hw/(cxl/|pci).* + ~ .*/qemu(/include)?/hw/(cxl/|pci).* qemu-ga - ~ (/qemu)?(/qga/.*) + ~ .*/qemu(/qga/.*) scsi - ~ (/qemu)?(/scsi/.*|/hw/scsi/.*|/include/hw/scsi/.*) + ~ .*/qemu(/scsi/.*|/hw/scsi/.*|/include/hw/scsi/.*) trace - ~ (/qemu)?(/.*trace.*\.[ch]) + ~ .*/qemu(/.*trace.*\.[ch]) ui - ~ (/qemu)?((/include)?(/ui|/hw/display|/hw/input)/.*) + ~ .*/qemu((/include)?(/ui|/hw/display|/hw/input)/.*) usb - ~ (/qemu)?(/hw/usb/.*|/include/hw/usb/.*) + ~ .*/qemu(/hw/usb/.*|/include/hw/usb/.*) user - ~ (/qemu)?(/linux-user/.*|/bsd-user/.*|/user-exec\.c|/thunk\.c|/include/user/.*) + ~ .*/qemu(/linux-user/.*|/bsd-user/.*|/user-exec\.c|/thunk\.c|/include/user/.*) util - ~ (/qemu)?(/util/.*|/include/qemu/.*) + ~ .*/qemu(/util/.*|/include/qemu/.*) vfio - ~ (/qemu)?(/include)?/hw/vfio/.* + ~ .*/qemu(/include)?/hw/vfio/.* virtio - ~ (/qemu)?(/include)?/hw/virtio/.* + ~ .*/qemu(/include)?/hw/virtio/.* xen - ~ (/qemu)?(.*/xen.*) + ~ .*/qemu(.*/xen.*) hvf - ~ (/qemu)?(.*/hvf.*) + ~ .*/qemu(.*/hvf.*) kvm - ~ (/qemu)?(.*/kvm.*) + ~ .*/qemu(.*/kvm.*) tcg - ~ (/qemu)?(/accel/tcg|/replay|/tcg)/.* + ~ .*/qemu(/accel/tcg|/replay|/tcg)/.* sysemu - ~ (/qemu)?(/system/.*|/accel/.*) + ~ .*/qemu(/system/.*|/accel/.*) (headers) - ~ (/qemu)?(/include/.*) + ~ .*/qemu(/include/.*) testlibs - ~ (/qemu)?(/tests/qtest(/libqos/.*|/libqtest.*)) + ~ .*/qemu(/tests/qtest(/libqos/.*|/libqtest.*)) tests - ~ (/qemu)?(/tests/.*) + ~ .*/qemu(/tests/.*) From 7966cf71d36560024f07863fef26e265b2941f25 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 18 Jun 2024 16:22:21 +0100 Subject: [PATCH 06/18] scripts/coverity-scan/COMPONENTS.md: Fix 'char' component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'char' component: * includes the no-longer-present qemu-char.c, which has been long since split into the chardev/ backend code * also includes the hw/char devices Split it into two components: * char is the hw/char devices * chardev is the chardev backends with regexes matching our current sources. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-3-peter.maydell@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 98d4bcd6a5..fb081a5926 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -73,7 +73,10 @@ block ~ .*/qemu(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*) char - ~ .*/qemu(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*) + ~ .*/qemu((/include)?/hw/char/.*) + +chardev + ~ .*/qemu((/include)?/chardev/.*) crypto ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) From 9c43c934d1a998d46a4a15675950f0c5eccb6b4c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 18 Jun 2024 16:22:22 +0100 Subject: [PATCH 07/18] scripts/coverity-scan/COMPONENTS.md: Add crypto headers in host/include to the crypto component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit host/include/*/host/crypto/ are relatively new headers; add them to the crypto component. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-4-peter.maydell@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index fb081a5926..205ab23b28 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -79,7 +79,7 @@ chardev ~ .*/qemu((/include)?/chardev/.*) crypto - ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) + ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*|/host/include/.*/host/crypto/.*) disas ~ .*/qemu((/include)?/disas.*) From 8e055eab4fea581644cd6df28984f1a2a5fde08e Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 18 Jun 2024 16:22:22 +0100 Subject: [PATCH 08/18] scripts/coverity-scan/COMPONENTS.md: Fix monitor component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the 'monitor' component: * qapi/ and monitor/ are now subdirectories * add job-qmp.c Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-5-peter.maydell@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 205ab23b28..3864f8eda0 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -97,7 +97,7 @@ migration ~ .*/qemu((/include)?/migration/.*) monitor - ~ .*/qemu(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*) + ~ .*/qemu((/include)?/(qapi|qobject|monitor)/.*|/job-qmp.c) nbd ~ .*/qemu(/nbd/.*|/include/block/nbd.*|/qemu-nbd\.c) From 5d173f30f6828a1a4e6133eb324cc4ab0277a06d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 18 Jun 2024 16:22:22 +0100 Subject: [PATCH 09/18] scripts/coverity-scan/COMPONENTS.md: Include libqmp in testlibs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add libqmp to the testlibs component. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-6-peter.maydell@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 3864f8eda0..858190be09 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -154,7 +154,7 @@ sysemu ~ .*/qemu(/include/.*) testlibs - ~ .*/qemu(/tests/qtest(/libqos/.*|/libqtest.*)) + ~ .*/qemu(/tests/qtest(/libqos/.*|/libqtest.*|/libqmp.*)) tests ~ .*/qemu(/tests/.*) From ff8aff01fa20c4fd5bbe46e1d25fbefdf996ef73 Mon Sep 17 00:00:00 2001 From: Zheyu Ma Date: Tue, 18 Jun 2024 16:40:09 +0200 Subject: [PATCH 10/18] hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit updates the a9_gtimer_get_current_cpu() function to handle cases where QTest is enabled. When QTest is used, it returns 0 instead of dereferencing the current_cpu, which can be NULL. This prevents the program from crashing during QTest runs. Reproducer: cat << EOF | qemu-system-aarch64 -display \ none -machine accel=qtest, -m 512M -machine npcm750-evb -qtest stdio writel 0xf03fe20c 0x26d7468c EOF Signed-off-by: Zheyu Ma Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240618144009.3137806-1-zheyuma97@gmail.com Signed-off-by: Peter Maydell --- hw/timer/a9gtimer.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c index a2ac5bdfb9..64d80cdf6a 100644 --- a/hw/timer/a9gtimer.c +++ b/hw/timer/a9gtimer.c @@ -32,6 +32,7 @@ #include "qemu/log.h" #include "qemu/module.h" #include "hw/core/cpu.h" +#include "sysemu/qtest.h" #ifndef A9_GTIMER_ERR_DEBUG #define A9_GTIMER_ERR_DEBUG 0 @@ -48,6 +49,10 @@ static inline int a9_gtimer_get_current_cpu(A9GTimerState *s) { + if (qtest_enabled()) { + return 0; + } + if (current_cpu->cpu_index >= s->num_cpu) { hw_error("a9gtimer: num-cpu %d but this cpu is %d!\n", s->num_cpu, current_cpu->cpu_index); From 813b59e8b8ab5d284672e0bbd79173df2e25e01d Mon Sep 17 00:00:00 2001 From: Zheyu Ma Date: Tue, 18 Jun 2024 15:56:10 +0200 Subject: [PATCH 11/18] hw/usb/hcd-dwc2: Handle invalid address access in read and write functions This commit modifies the dwc2_hsotg_read() and dwc2_hsotg_write() functions to handle invalid address access gracefully. Instead of using g_assert_not_reached(), which causes the program to abort, the functions now log an error message and return a default value for reads or do nothing for writes. This change prevents the program from aborting and provides clear log messages indicating when an invalid memory address is accessed. Reproducer: cat << EOF | qemu-system-aarch64 -display none \ -machine accel=qtest, -m 512M -machine raspi2b -m 1G -nodefaults \ -usb -drive file=null-co://,if=none,format=raw,id=disk0 -device \ usb-storage,port=1,drive=disk0 -qtest stdio readl 0x3f980dfb EOF Signed-off-by: Zheyu Ma Reviewed-by: Paul Zimmerman Message-id: 20240618135610.3109175-1-zheyuma97@gmail.com Signed-off-by: Peter Maydell --- hw/usb/hcd-dwc2.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c index 8cac9c0a06..b4f0652c7d 100644 --- a/hw/usb/hcd-dwc2.c +++ b/hw/usb/hcd-dwc2.c @@ -1128,7 +1128,10 @@ static uint64_t dwc2_hsotg_read(void *ptr, hwaddr addr, unsigned size) val = dwc2_pcgreg_read(ptr, addr, (addr - HSOTG_REG(0xe00)) >> 2, size); break; default: - g_assert_not_reached(); + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n", + __func__, addr); + val = 0; + break; } return val; @@ -1160,7 +1163,9 @@ static void dwc2_hsotg_write(void *ptr, hwaddr addr, uint64_t val, dwc2_pcgreg_write(ptr, addr, (addr - HSOTG_REG(0xe00)) >> 2, val, size); break; default: - g_assert_not_reached(); + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n", + __func__, addr); + break; } } From 9ed2fb65cc827904241dee189c801c10079c2fe3 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Jun 2024 17:23:41 +0100 Subject: [PATCH 12/18] hw/arm/virt: Add serial aliases in DTB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If there is more than one UART in the DTB, then there is no guarantee on which order a guest is supposed to initialise them. The standard solution to this is "serialN" entries in the "/aliases" node of the dtb which give the nodename of the UARTs. At the moment we only have two UARTs in the DTB when one is for the Secure world and one for the Non-Secure world, so this isn't really a problem. However if we want to add a second NS UART we'll need the aliases to ensure guests pick the right one. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240610162343.2131524-2-peter.maydell@linaro.org --- hw/arm/virt.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c7a1f754e7..61a9d47c02 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -284,6 +284,8 @@ static void create_fdt(VirtMachineState *vms) } } + qemu_fdt_add_subnode(fdt, "/aliases"); + /* Clock node, for the benefit of the UART. The kernel device tree * binding documentation claims the PL011 node clock properties are * optional but in practice if you omit them the kernel refuses to @@ -939,7 +941,9 @@ static void create_uart(const VirtMachineState *vms, int uart, if (uart == VIRT_UART) { qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename); + qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename); } else { + qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial1", nodename); /* Mark as not usable by the normal world */ qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled"); qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay"); From fe22cba940d82e93818135c044afed4099056628 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Jun 2024 17:23:42 +0100 Subject: [PATCH 13/18] hw/arm/virt: Rename VIRT_UART and VIRT_SECURE_UART to VIRT_UART[01] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're going to make the second UART not always a secure-only device. Rename the constants VIRT_UART and VIRT_SECURE_UART to VIRT_UART0 and VIRT_UART1 accordingly. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240610162343.2131524-3-peter.maydell@linaro.org --- hw/arm/virt-acpi-build.c | 12 ++++++------ hw/arm/virt.c | 14 +++++++------- include/hw/arm/virt.h | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index c3ccfef026..eb5796e309 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -440,10 +440,10 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) .base_addr.width = 32, .base_addr.offset = 0, .base_addr.size = 3, - .base_addr.addr = vms->memmap[VIRT_UART].base, + .base_addr.addr = vms->memmap[VIRT_UART0].base, .interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/ .pc_interrupt = 0, /* IRQ */ - .interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE), + .interrupt = (vms->irqmap[VIRT_UART0] + ARM_SPI_BASE), .baud_rate = 3, /* 9600 */ .parity = 0, /* No Parity */ .stop_bits = 1, /* 1 Stop bit */ @@ -631,11 +631,11 @@ build_dbg2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) /* BaseAddressRegister[] */ build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3, - vms->memmap[VIRT_UART].base); + vms->memmap[VIRT_UART0].base); /* AddressSize[] */ build_append_int_noprefix(table_data, - vms->memmap[VIRT_UART].size, 4); + vms->memmap[VIRT_UART0].size, 4); /* NamespaceString[] */ g_array_append_vals(table_data, name, namespace_length); @@ -816,8 +816,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) */ scope = aml_scope("\\_SB"); acpi_dsdt_add_cpus(scope, vms); - acpi_dsdt_add_uart(scope, &memmap[VIRT_UART], - (irqmap[VIRT_UART] + ARM_SPI_BASE)); + acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], + (irqmap[VIRT_UART0] + ARM_SPI_BASE)); if (vmc->acpi_expose_flash) { acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 61a9d47c02..ffb4983885 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -165,11 +165,11 @@ static const MemMapEntry base_memmap[] = { [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 }, /* This redistributor space allows up to 2*64kB*123 CPUs */ [VIRT_GIC_REDIST] = { 0x080A0000, 0x00F60000 }, - [VIRT_UART] = { 0x09000000, 0x00001000 }, + [VIRT_UART0] = { 0x09000000, 0x00001000 }, [VIRT_RTC] = { 0x09010000, 0x00001000 }, [VIRT_FW_CFG] = { 0x09020000, 0x00000018 }, [VIRT_GPIO] = { 0x09030000, 0x00001000 }, - [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, + [VIRT_UART1] = { 0x09040000, 0x00001000 }, [VIRT_SMMU] = { 0x09050000, 0x00020000 }, [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN }, [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, @@ -212,11 +212,11 @@ static MemMapEntry extended_memmap[] = { }; static const int a15irqmap[] = { - [VIRT_UART] = 1, + [VIRT_UART0] = 1, [VIRT_RTC] = 2, [VIRT_PCIE] = 3, /* ... to 6 */ [VIRT_GPIO] = 7, - [VIRT_SECURE_UART] = 8, + [VIRT_UART1] = 8, [VIRT_ACPI_GED] = 9, [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */ @@ -939,7 +939,7 @@ static void create_uart(const VirtMachineState *vms, int uart, qemu_fdt_setprop(ms->fdt, nodename, "clock-names", clocknames, sizeof(clocknames)); - if (uart == VIRT_UART) { + if (uart == VIRT_UART0) { qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename); qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename); } else { @@ -2317,11 +2317,11 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); - create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); + create_uart(vms, VIRT_UART0, sysmem, serial_hd(0)); if (vms->secure) { create_secure_ram(vms, secure_sysmem, secure_tag_sysmem); - create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); + create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1)); } if (tag_sysmem) { diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index bb486d36b1..1227e7f7f0 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -59,7 +59,7 @@ enum { VIRT_GIC_ITS, VIRT_GIC_REDIST, VIRT_SMMU, - VIRT_UART, + VIRT_UART0, VIRT_MMIO, VIRT_RTC, VIRT_FW_CFG, @@ -69,7 +69,7 @@ enum { VIRT_PCIE_ECAM, VIRT_PLATFORM_BUS, VIRT_GPIO, - VIRT_SECURE_UART, + VIRT_UART1, VIRT_SECURE_MEM, VIRT_SECURE_GPIO, VIRT_PCDIMM_ACPI, From e7100972f2df313d1e47a0714aed968991437e86 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Jun 2024 17:23:43 +0100 Subject: [PATCH 14/18] hw/arm/virt: allow creation of a second NonSecure UART MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For some use-cases, it is helpful to have more than one UART available to the guest. If the second UART slot is not already used for a TrustZone Secure-World-only UART, create it as a NonSecure UART only when the user provides a serial backend (e.g. via a second -serial command line option). This avoids problems where existing guest software only expects a single UART, and gets confused by the second UART in the DTB. The major example of this is older EDK2 firmware, which will send the GRUB bootloader output to UART1 and the guest serial output to UART0. Users who want to use both UARTs with a guest setup including EDK2 are advised to update to EDK2 release edk2-stable202311 or newer. (The prebuilt EDK2 blobs QEMU upstream provides are new enough.) The relevant EDK2 changes are the ones described here: https://bugzilla.tianocore.org/show_bug.cgi?id=4577 Inspired-by: Axel Heider Signed-off-by: Peter Maydell Tested-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240610162343.2131524-4-peter.maydell@linaro.org --- docs/system/arm/virt.rst | 6 +++++- hw/arm/virt-acpi-build.c | 12 ++++++++---- hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++--- include/hw/arm/virt.h | 1 + 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst index 26fcba00b7..e67e7f0f7c 100644 --- a/docs/system/arm/virt.rst +++ b/docs/system/arm/virt.rst @@ -26,7 +26,7 @@ The virt board supports: - PCI/PCIe devices - Flash memory -- One PL011 UART +- Either one or two PL011 UARTs for the NonSecure World - An RTC - The fw_cfg device that allows a guest to obtain data from QEMU - A PL061 GPIO controller @@ -48,6 +48,10 @@ The virt board supports: - A secure flash memory - 16MB of secure RAM +The second NonSecure UART only exists if a backend is configured +explicitly (e.g. with a second -serial command line option) and +TrustZone emulation is not enabled. + Supported guest CPU types: - ``cortex-a7`` (32-bit) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index eb5796e309..b2366f24f9 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -79,11 +79,11 @@ static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) } static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, - uint32_t uart_irq) + uint32_t uart_irq, int uartidx) { - Aml *dev = aml_device("COM0"); + Aml *dev = aml_device("COM%d", uartidx); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011"))); - aml_append(dev, aml_name_decl("_UID", aml_int(0))); + aml_append(dev, aml_name_decl("_UID", aml_int(uartidx))); Aml *crs = aml_resource_template(); aml_append(crs, aml_memory32_fixed(uart_memmap->base, @@ -817,7 +817,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) scope = aml_scope("\\_SB"); acpi_dsdt_add_cpus(scope, vms); acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], - (irqmap[VIRT_UART0] + ARM_SPI_BASE)); + (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0); + if (vms->second_ns_uart_present) { + acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1], + (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1); + } if (vmc->acpi_expose_flash) { acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index ffb4983885..8555615256 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -906,7 +906,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) } static void create_uart(const VirtMachineState *vms, int uart, - MemoryRegion *mem, Chardev *chr) + MemoryRegion *mem, Chardev *chr, bool secure) { char *nodename; hwaddr base = vms->memmap[uart].base; @@ -944,6 +944,8 @@ static void create_uart(const VirtMachineState *vms, int uart, qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename); } else { qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial1", nodename); + } + if (secure) { /* Mark as not usable by the normal world */ qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled"); qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay"); @@ -2317,11 +2319,41 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); - create_uart(vms, VIRT_UART0, sysmem, serial_hd(0)); + /* + * The first UART always exists. If the security extensions are + * enabled, the second UART also always exists. Otherwise, it only exists + * if a backend is configured explicitly via '-serial '. + * This avoids potentially breaking existing user setups that expect + * only one NonSecure UART to be present (for instance, older EDK2 + * binaries). + * + * The nodes end up in the DTB in reverse order of creation, so we must + * create UART0 last to ensure it appears as the first node in the DTB, + * for compatibility with guest software that just iterates through the + * DTB to find the first UART, as older versions of EDK2 do. + * DTB readers that follow the spec, as Linux does, should honour the + * aliases node information and /chosen/stdout-path regardless of + * the order that nodes appear in the DTB. + * + * For similar back-compatibility reasons, if UART1 is the secure UART + * we create it second (and so it appears first in the DTB), because + * that's what QEMU has always done. + */ + if (!vms->secure) { + Chardev *serial1 = serial_hd(1); + + if (serial1) { + vms->second_ns_uart_present = true; + create_uart(vms, VIRT_UART1, sysmem, serial1, false); + } + } + create_uart(vms, VIRT_UART0, sysmem, serial_hd(0), false); + if (vms->secure) { + create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1), true); + } if (vms->secure) { create_secure_ram(vms, secure_sysmem, secure_tag_sysmem); - create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1)); } if (tag_sysmem) { diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 1227e7f7f0..ab961bb6a9 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -151,6 +151,7 @@ struct VirtMachineState { bool ras; bool mte; bool dtb_randomness; + bool second_ns_uart_present; OnOffAuto acpi; VirtGICType gic_version; VirtIOMMUType iommu; From dda533087ad5559674ff486e7031c88dc01e0abd Mon Sep 17 00:00:00 2001 From: Zhenyu Zhang Date: Tue, 11 Jun 2024 22:05:06 -0400 Subject: [PATCH 15/18] hw/arm/virt: Avoid unexpected warning from Linux guest on host with Fujitsu CPUs Multiple warning messages and corresponding backtraces are observed when Linux guest is booted on the host with Fujitsu CPUs. One of them is shown as below. [ 0.032443] ------------[ cut here ]------------ [ 0.032446] uart-pl011 9000000.pl011: ARCH_DMA_MINALIGN smaller than CTR_EL0.CWG (128 < 256) [ 0.032454] WARNING: CPU: 0 PID: 1 at arch/arm64/mm/dma-mapping.c:54 arch_setup_dma_ops+0xbc/0xcc [ 0.032470] Modules linked in: [ 0.032475] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-452.el9.aarch64 [ 0.032481] Hardware name: linux,dummy-virt (DT) [ 0.032484] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.032490] pc : arch_setup_dma_ops+0xbc/0xcc [ 0.032496] lr : arch_setup_dma_ops+0xbc/0xcc [ 0.032501] sp : ffff80008003b860 [ 0.032503] x29: ffff80008003b860 x28: 0000000000000000 x27: ffffaae4b949049c [ 0.032510] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000 [ 0.032517] x23: 0000000000000100 x22: 0000000000000000 x21: 0000000000000000 [ 0.032523] x20: 0000000100000000 x19: ffff2f06c02ea400 x18: ffffffffffffffff [ 0.032529] x17: 00000000208a5f76 x16: 000000006589dbcb x15: ffffaae4ba071c89 [ 0.032535] x14: 0000000000000000 x13: ffffaae4ba071c84 x12: 455f525443206e61 [ 0.032541] x11: 68742072656c6c61 x10: 0000000000000029 x9 : ffffaae4b7d21da4 [ 0.032547] x8 : 0000000000000029 x7 : 4c414e494d5f414d x6 : 0000000000000029 [ 0.032553] x5 : 000000000000000f x4 : ffffaae4b9617a00 x3 : 0000000000000001 [ 0.032558] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff2f06c029be40 [ 0.032564] Call trace: [ 0.032566] arch_setup_dma_ops+0xbc/0xcc [ 0.032572] of_dma_configure_id+0x138/0x300 [ 0.032591] amba_dma_configure+0x34/0xc0 [ 0.032600] really_probe+0x78/0x3dc [ 0.032614] __driver_probe_device+0x108/0x160 [ 0.032619] driver_probe_device+0x44/0x114 [ 0.032624] __device_attach_driver+0xb8/0x14c [ 0.032629] bus_for_each_drv+0x88/0xe4 [ 0.032634] __device_attach+0xb0/0x1e0 [ 0.032638] device_initial_probe+0x18/0x20 [ 0.032643] bus_probe_device+0xa8/0xb0 [ 0.032648] device_add+0x4b4/0x6c0 [ 0.032652] amba_device_try_add.part.0+0x48/0x360 [ 0.032657] amba_device_add+0x104/0x144 [ 0.032662] of_amba_device_create.isra.0+0x100/0x1c4 [ 0.032666] of_platform_bus_create+0x294/0x35c [ 0.032669] of_platform_populate+0x5c/0x150 [ 0.032672] of_platform_default_populate_init+0xd0/0xec [ 0.032697] do_one_initcall+0x4c/0x2e0 [ 0.032701] do_initcalls+0x100/0x13c [ 0.032707] kernel_init_freeable+0x1c8/0x21c [ 0.032712] kernel_init+0x28/0x140 [ 0.032731] ret_from_fork+0x10/0x20 [ 0.032735] ---[ end trace 0000000000000000 ]--- In Linux, a check is applied to every device which is exposed through device-tree node. The warning message is raised when the device isn't DMA coherent and the cache line size is larger than ARCH_DMA_MINALIGN (128 bytes). The cache line is sorted from CTR_EL0[CWG], which corresponds to 256 bytes on the guest CPUs. The DMA coherent capability is claimed through 'dma-coherent' in their device-tree nodes or parent nodes. This happens even when the device doesn't implement or use DMA at all, for legacy reasons. Fix the issue by adding 'dma-coherent' property to the device-tree root node, meaning all devices are capable of DMA coherent by default. This both suppresses the spurious kernel warnings and also guards against possible future QEMU bugs where we add a DMA-capable device and forget to mark it as dma-coherent. Signed-off-by: Zhenyu Zhang Reviewed-by: Gavin Shan Reviewed-by: Donald Dutile Message-id: 20240612020506.307793-1-zhenyzha@redhat.com [PMM: tweaked commit message] Signed-off-by: Peter Maydell --- hw/arm/virt.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 8555615256..0784ee7f46 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -271,6 +271,17 @@ static void create_fdt(VirtMachineState *vms) qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2); qemu_fdt_setprop_string(fdt, "/", "model", "linux,dummy-virt"); + /* + * For QEMU, all DMA is coherent. Advertising this in the root node + * has two benefits: + * + * - It avoids potential bugs where we forget to mark a DMA + * capable device as being dma-coherent + * - It avoids spurious warnings from the Linux kernel about + * devices which can't do DMA at all + */ + qemu_fdt_setprop(fdt, "/", "dma-coherent", NULL, 0); + /* /chosen must exist for load_dtb to fill in necessary properties later */ qemu_fdt_add_subnode(fdt, "/chosen"); if (vms->dtb_randomness) { From d338b5a80922433a2c4a63986b41f82ae4137dfc Mon Sep 17 00:00:00 2001 From: Zheyu Ma Date: Tue, 18 Jun 2024 18:37:01 +0200 Subject: [PATCH 16/18] hw/misc: Set valid access size for Exynos4210 RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Exynos4210 RNG module requires 32-bit (4-byte) accesses to its registers. According to the User Manual Section 25.3[1], the registers for RNG operations are 32-bit. This change ensures that the memory region operations for the RNG module enforce the correct access sizes, preventing invalid memory accesses. [1] http://www.mediafire.com/view/8ly2fqls3c9c31c/Exynos_4412_SCP_Users_Manual_Ver.0.10.00_Preliminary0.pdf Reproducer: cat << EOF | qemu-system-aarch64 -display none \ -machine accel=qtest, -m 512M -machine smdkc210 -qtest stdio readb 0x10830454 EOF Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Zheyu Ma Message-id: 20240618163701.3204975-1-zheyuma97@gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/misc/exynos4210_rng.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c index 0756bd3205..674d8eece5 100644 --- a/hw/misc/exynos4210_rng.c +++ b/hw/misc/exynos4210_rng.c @@ -217,6 +217,8 @@ static const MemoryRegionOps exynos4210_rng_ops = { .read = exynos4210_rng_read, .write = exynos4210_rng_write, .endianness = DEVICE_NATIVE_ENDIAN, + .valid.min_access_size = 4, + .valid.max_access_size = 4, }; static void exynos4210_rng_reset(DeviceState *dev) From 53aaa88105e2f9cbef3a4d17df007dbdf985d6e2 Mon Sep 17 00:00:00 2001 From: David Hubbard Date: Mon, 20 May 2024 18:26:34 -0500 Subject: [PATCH 17/18] hw/usb/hcd-ohci: Fix ohci_service_td: accept zero-length TDs where CBP=BE+1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the way the ohci emulation handles a Transfer Descriptor with "Buffer End" set to "Current Buffer Pointer" - 1, specifically in the case of a zero-length packet. The OHCI spec 4.3.1.2 Table 4-2 specifies td.cbp to be zero for a zero-length packet. Peter Maydell tracked down commit 1328fe0c32 (hw: usb: hcd-ohci: check len and frame_number variables) where qemu started checking this according to the spec. What this patch does is loosen the qemu ohci implementation to allow a zero-length packet if td.be (Buffer End) is set to td.cbp - 1, and with a non-zero td.cbp value. The spec is unclear whether this is valid or not -- it is not the clearly documented way to send a zero length TD (which is CBP=BE=0), but it isn't specifically forbidden. Actual hw seems to be ok with it. Does any OS rely on this behavior? There have been no reports to qemu-devel of this problem. This is attempting to have qemu behave like actual hardware, but this is just a minor change. With a tiny OS[1] that boots and executes a test, the issue can be seen: * OS that sends USB requests to a USB mass storage device but sends td.cbp = td.be + 1 * qemu 4.2 * qemu HEAD (4e66a0854) * Actual OHCI controller (hardware) Command line: qemu-system-x86_64 -m 20 \ -device pci-ohci,id=ohci \ -drive if=none,format=raw,id=d,file=testmbr.raw \ -device usb-storage,bus=ohci.0,drive=d \ --trace "usb_*" --trace "ohci_*" -D qemu.log Results are: qemu 4.2 | qemu HEAD | actual HW -----------+------------+----------- works fine | ohci_die() | works fine Tip: if the flags "-serial pty -serial stdio" are added to the command line the test will output USB requests like this: Testing qemu HEAD: > Free mem 2M ohci port2 conn FS > setup { 80 6 0 1 0 0 8 0 } > ED info=80000 { mps=8 en=0 d=0 } tail=c20920 > td0 c20880 nxt=c20960 f2000000 setup cbp=c20900 be=c20907 > td1 c20960 nxt=c20980 f3140000 in cbp=c20908 be=c2090f > td2 c20980 nxt=c20920 f3080000 out cbp=c20910 be=c2090f ohci20 host err > usb stopped And in qemu.log: usb_ohci_iso_td_bad_cc_overrun ISO_TD start_offset=0x00c20910 > next_offset=0x00c2090f Testing qemu 4.2: > Free mem 2M ohci port2 conn FS > setup { 80 6 0 1 0 0 8 0 } > ED info=80000 { mps=8 en=0 d=0 } tail=620920 > td0 620880 nxt=620960 f2000000 setup cbp=620900 be=620907 cbp=0 be=620907 > td1 620960 nxt=620980 f3140000 in cbp=620908 be=62090f cbp=0 be=62090f > td2 620980 nxt=620920 f3080000 out cbp=620910 be=62090f cbp=0 be=62090f > rx { 12 1 0 2 0 0 0 8 } > setup { 0 5 1 0 0 0 0 0 } tx {} > ED info=80000 { mps=8 en=0 d=0 } tail=620880 > td0 620920 nxt=620960 f2000000 setup cbp=620900 be=620907 cbp=0 be=620907 > td1 620960 nxt=620880 f3100000 in cbp=620908 be=620907 cbp=0 be=620907 > setup { 80 6 0 1 0 0 12 0 } > ED info=80001 { mps=8 en=0 d=1 } tail=620960 > td0 620880 nxt=6209c0 f2000000 setup cbp=620920 be=620927 cbp=0 be=620927 > td1 6209c0 nxt=6209e0 f3140000 in cbp=620928 be=620939 cbp=0 be=620939 > td2 6209e0 nxt=620960 f3080000 out cbp=62093a be=620939 cbp=0 be=620939 > rx { 12 1 0 2 0 0 0 8 f4 46 1 0 0 0 1 2 3 1 } > setup { 80 6 0 2 0 0 0 1 } > ED info=80001 { mps=8 en=0 d=1 } tail=620880 > td0 620960 nxt=6209a0 f2000000 setup cbp=620a20 be=620a27 cbp=0 be=620a27 > td1 6209a0 nxt=6209c0 f3140004 in cbp=620a28 be=620b27 cbp=620a48 be=620b27 > td2 6209c0 nxt=620880 f3080000 out cbp=620b28 be=620b27 cbp=0 be=620b27 > rx { 9 2 20 0 1 1 4 c0 0 9 4 0 0 2 8 6 50 0 7 5 81 2 40 0 0 7 5 2 2 40 0 0 } > setup { 0 9 1 0 0 0 0 0 } tx {} > ED info=80001 { mps=8 en=0 d=1 } tail=620900 > td0 620880 nxt=620940 f2000000 setup cbp=620a00 be=620a07 cbp=0 be=620a07 > td1 620940 nxt=620900 f3100000 in cbp=620a08 be=620a07 cbp=0 be=620a07 [1] The OS disk image has been emailed to philmd@linaro.org, mjt@tls.msk.ru, and kraxel@redhat.com: * testCbpOffBy1.img.xz * sha256: f87baddcb86de845de12f002c698670a426affb40946025cc32694f9daa3abed Signed-off-by: David Hubbard Reviewed-by: Alex Bennée Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/usb/hcd-ohci.c | 4 ++-- hw/usb/trace-events | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index acd6016980..71b54914d3 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -941,8 +941,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) if ((td.cbp & 0xfffff000) != (td.be & 0xfffff000)) { len = (td.be & 0xfff) + 0x1001 - (td.cbp & 0xfff); } else { - if (td.cbp > td.be) { - trace_usb_ohci_iso_td_bad_cc_overrun(td.cbp, td.be); + if (td.cbp - 1 > td.be) { /* rely on td.cbp != 0 */ + trace_usb_ohci_td_bad_buf(td.cbp, td.be); ohci_die(ohci); return 1; } diff --git a/hw/usb/trace-events b/hw/usb/trace-events index 46732717a9..dd04f14add 100644 --- a/hw/usb/trace-events +++ b/hw/usb/trace-events @@ -29,6 +29,7 @@ usb_ohci_iso_td_data_underrun(int ret) "DataUnderrun %d" usb_ohci_iso_td_nak(int ret) "got NAK/STALL %d" usb_ohci_iso_td_bad_response(int ret) "Bad device response %d" usb_ohci_td_bad_pid(const char *s, uint32_t edf, uint32_t tdf) "Bad pid %s: ed.flags 0x%x td.flags 0x%x" +usb_ohci_td_bad_buf(uint32_t cbp, uint32_t be) "Bad cbp = 0x%x > be = 0x%x" usb_ohci_port_attach(int index) "port #%d" usb_ohci_port_detach(int index) "port #%d" usb_ohci_port_wakeup(int index) "port #%d" From 3b36cead6ecc0e40edb8b2f3e253baa01ebc1e9a Mon Sep 17 00:00:00 2001 From: Xiong Yining Date: Fri, 7 Jun 2024 10:38:25 +0000 Subject: [PATCH 18/18] hw/arm/sbsa-ref: Enable CPU cluster on ARM sbsa machine Enable CPU cluster support on SbsaQemu platform, so that users can specify a 4-level CPU hierarchy sockets/clusters/cores/threads. And this topology can be passed to the firmware through /cpus/topology Device Tree. Signed-off-by: Xiong Yining Reviewed-by: Marcin Juszkiewicz Reviewed-by: Leif Lindholm Message-id: 20240607103825.1295328-2-xiongyining1480@phytium.com.cn Tested-by: Marcin Juszkiewicz Signed-off-by: Peter Maydell --- docs/system/arm/sbsa.rst | 4 ++++ hw/arm/sbsa-ref.c | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst index 2bf22a1d0b..2bf3fc8d59 100644 --- a/docs/system/arm/sbsa.rst +++ b/docs/system/arm/sbsa.rst @@ -62,6 +62,7 @@ The devicetree reports: - platform version - GIC addresses - NUMA node id for CPUs and memory + - CPU topology information Platform version '''''''''''''''' @@ -88,3 +89,6 @@ Platform version changes: 0.3 The USB controller is an XHCI device, not EHCI. + +0.4 + CPU topology information is present in devicetree. diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 87884400e3..ae37a92301 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -219,7 +219,7 @@ static void create_fdt(SBSAMachineState *sms) * fw compatibility. */ qemu_fdt_setprop_cell(fdt, "/", "machine-version-major", 0); - qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 3); + qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 4); if (ms->numa_state->have_numa_distance) { int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t); @@ -276,6 +276,14 @@ static void create_fdt(SBSAMachineState *sms) g_free(nodename); } + /* Add CPU topology description through fdt node topology. */ + qemu_fdt_add_subnode(sms->fdt, "/cpus/topology"); + + qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "sockets", ms->smp.sockets); + qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "clusters", ms->smp.clusters); + qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "cores", ms->smp.cores); + qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "threads", ms->smp.threads); + sbsa_fdt_add_gic_node(sms); } @@ -898,6 +906,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data) mc->default_ram_size = 1 * GiB; mc->default_ram_id = "sbsa-ref.ram"; mc->default_cpus = 4; + mc->smp_props.clusters_supported = true; mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids; mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props; mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;