Hi; here's the first target-arm pullreq for the 7.0 cycle. thanks -- PMM The following changes since commit 76b56fdfc9fa43ec6e5986aee33f108c6c6a511e: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2021-12-14 12:46:18 -0800) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211215 for you to fetch changes up to aed176558806674d030a8305d989d4e6a5073359: tests/acpi: add expected blob for VIOT test on virt machine (2021-12-15 10:35:26 +0000) ---------------------------------------------------------------- target-arm queue: * ITS: error reporting cleanup * aspeed: improve documentation * Fix STM32F2XX USART data register readout * allow emulated GICv3 to be disabled in non-TCG builds * fix exception priority for singlestep, misaligned PC, bp, etc * Correct calculation of tlb range invalidate length * npcm7xx_emc: fix missing queue_flush * virt: Add VIOT ACPI table for virtio-iommu * target/i386: Use assert() to sanity-check b1 in SSE decode * Don't include qemu-common unnecessarily ---------------------------------------------------------------- Alex Bennée (1): hw/intc: clean-up error reporting for failed ITS cmd Jean-Philippe Brucker (8): hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu hw/arm/virt: Remove device tree restriction for virtio-iommu hw/arm/virt: Reject instantiation of multiple IOMMUs hw/arm/virt: Use object_property_set instead of qdev_prop_set tests/acpi: allow updates of VIOT expected data files tests/acpi: add test case for VIOT tests/acpi: add expected blobs for VIOT test on q35 machine tests/acpi: add expected blob for VIOT test on virt machine Joel Stanley (4): docs: aspeed: Add new boards docs: aspeed: Update OpenBMC image URL docs: aspeed: Give an example of booting a kernel docs: aspeed: ADC is now modelled Olivier Hériveaux (1): Fix STM32F2XX USART data register readout Patrick Venture (1): hw/net: npcm7xx_emc fix missing queue_flush Peter Maydell (6): target/i386: Use assert() to sanity-check b1 in SSE decode include/hw/i386: Don't include qemu-common.h in .h files target/hexagon/cpu.h: don't include qemu-common.h target/rx/cpu.h: Don't include qemu-common.h hw/arm: Don't include qemu-common.h unnecessarily target/arm: Correct calculation of tlb range invalidate length Philippe Mathieu-Daudé (2): hw/intc/arm_gicv3: Extract gicv3_set_gicv3state from arm_gicv3_cpuif.c hw/intc/arm_gicv3: Introduce CONFIG_ARM_GIC_TCG Kconfig selector Richard Henderson (10): target/arm: Hoist pc_next to a local variable in aarch64_tr_translate_insn target/arm: Hoist pc_next to a local variable in arm_tr_translate_insn target/arm: Hoist pc_next to a local variable in thumb_tr_translate_insn target/arm: Split arm_pre_translate_insn target/arm: Advance pc for arch single-step exception target/arm: Split compute_fsr_fsc out of arm_deliver_fault target/arm: Take an exception if PC is misaligned target/arm: Assert thumb pc is aligned target/arm: Suppress bp for exceptions with more priority tests/tcg: Add arm and aarch64 pc alignment tests docs/system/arm/aspeed.rst | 26 ++++++++++++---- include/hw/i386/microvm.h | 1 - include/hw/i386/x86.h | 1 - target/arm/helper.h | 1 + target/arm/syndrome.h | 5 +++ target/hexagon/cpu.h | 1 - target/rx/cpu.h | 1 - hw/arm/boot.c | 1 - hw/arm/digic_boards.c | 1 - hw/arm/highbank.c | 1 - hw/arm/npcm7xx_boards.c | 1 - hw/arm/sbsa-ref.c | 1 - hw/arm/stm32f405_soc.c | 1 - hw/arm/vexpress.c | 1 - hw/arm/virt-acpi-build.c | 7 +++++ hw/arm/virt.c | 21 ++++++------- hw/char/stm32f2xx_usart.c | 3 +- hw/intc/arm_gicv3.c | 2 +- hw/intc/arm_gicv3_cpuif.c | 10 +----- hw/intc/arm_gicv3_cpuif_common.c | 22 +++++++++++++ hw/intc/arm_gicv3_its.c | 39 +++++++++++++++-------- hw/net/npcm7xx_emc.c | 18 +++++------ hw/virtio/virtio-iommu-pci.c | 12 ++------ linux-user/aarch64/cpu_loop.c | 46 ++++++++++++++++------------ linux-user/hexagon/cpu_loop.c | 1 + target/arm/debug_helper.c | 23 ++++++++++++++ target/arm/gdbstub.c | 9 ++++-- target/arm/helper.c | 6 ++-- target/arm/machine.c | 10 ++++++ target/arm/tlb_helper.c | 63 ++++++++++++++++++++++++++++---------- target/arm/translate-a64.c | 23 ++++++++++++-- target/arm/translate.c | 58 ++++++++++++++++++++++++++--------- target/i386/tcg/translate.c | 12 ++------ tests/qtest/bios-tables-test.c | 38 +++++++++++++++++++++++ tests/tcg/aarch64/pcalign-a64.c | 37 ++++++++++++++++++++++ tests/tcg/arm/pcalign-a32.c | 46 ++++++++++++++++++++++++++++ hw/arm/Kconfig | 1 + hw/intc/Kconfig | 5 +++ hw/intc/meson.build | 11 ++++--- tests/data/acpi/q35/DSDT.viot | Bin 0 -> 9398 bytes tests/data/acpi/q35/VIOT.viot | Bin 0 -> 112 bytes tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes tests/tcg/aarch64/Makefile.target | 4 +-- tests/tcg/arm/Makefile.target | 4 +++ 44 files changed, 429 insertions(+), 145 deletions(-) create mode 100644 hw/intc/arm_gicv3_cpuif_common.c create mode 100644 tests/tcg/aarch64/pcalign-a64.c create mode 100644 tests/tcg/arm/pcalign-a32.c create mode 100644 tests/data/acpi/q35/DSDT.viot create mode 100644 tests/data/acpi/q35/VIOT.viot create mode 100644 tests/data/acpi/virt/VIOT From: Alex Bennée <alex.bennee@linaro.org> While trying to debug a GIC ITS failure I saw some guest errors that had poor formatting as well as leaving me confused as to what failed. As most of the checks aren't possible without a valid dte split that check apart and then check the other conditions in steps. This avoids us relying on undefined data. I still get a failure with the current kvm-unit-tests but at least I know (partially) why now: Exception return from AArch64 EL1 to AArch64 EL1 PC 0x40080588 PASS: gicv3: its-trigger: inv/invall: dev2/eventid=20 now triggers an LPI ITS: MAPD devid=2 size = 0x8 itt=0x40430000 valid=0 INT dev_id=2 event_id=20 process_its_cmd: invalid command attributes: invalid dte: 0 for 2 (MEM_TX: 0) PASS: gicv3: its-trigger: mapd valid=false: no LPI after device unmap SUMMARY: 6 tests, 1 unexpected failures Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20211112170454.3158925-1-alex.bennee@linaro.org Cc: Shashi Mallela <shashi.mallela@linaro.org> Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_its.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -XXX,XX +XXX,XX @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, if (res != MEMTX_OK) { return result; } + } else { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: " + "invalid dte: %"PRIx64" for %d (MEM_TX: %d)\n", + __func__, dte, devid, res); + return result; } - if ((devid > s->dt.maxids.max_devids) || !dte_valid || !ite_valid || - !cte_valid || (eventid > max_eventid)) { + + /* + * In this implementation, in case of guest errors we ignore the + * command and move onto the next command in the queue. + */ + if (devid > s->dt.maxids.max_devids) { qemu_log_mask(LOG_GUEST_ERROR, - "%s: invalid command attributes " - "devid %d or eventid %d or invalid dte %d or" - "invalid cte %d or invalid ite %d\n", - __func__, devid, eventid, dte_valid, cte_valid, - ite_valid); - /* - * in this implementation, in case of error - * we ignore this command and move onto the next - * command in the queue - */ + "%s: invalid command attributes: devid %d>%d", + __func__, devid, s->dt.maxids.max_devids); + + } else if (!dte_valid || !ite_valid || !cte_valid) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: " + "dte: %s, ite: %s, cte: %s\n", + __func__, + dte_valid ? "valid" : "invalid", + ite_valid ? "valid" : "invalid", + cte_valid ? "valid" : "invalid"); + } else if (eventid > max_eventid) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid command attributes: eventid %d > %d\n", + __func__, eventid, max_eventid); } else { /* * Current implementation only supports rdbase == procnum -- 2.25.1 From: Joel Stanley <joel@jms.id.au> Add X11, FP5280G2, G220A, Rainier and Fuji. Mention that Swift will be removed in v7.0. Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-id: 20211117065752.330632-2-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/aspeed.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -XXX,XX +XXX,XX @@ AST2400 SoC based machines : - ``palmetto-bmc`` OpenPOWER Palmetto POWER8 BMC - ``quanta-q71l-bmc`` OpenBMC Quanta BMC +- ``supermicrox11-bmc`` Supermicro X11 BMC AST2500 SoC based machines : @@ -XXX,XX +XXX,XX @@ AST2500 SoC based machines : - ``romulus-bmc`` OpenPOWER Romulus POWER9 BMC - ``witherspoon-bmc`` OpenPOWER Witherspoon POWER9 BMC - ``sonorapass-bmc`` OCP SonoraPass BMC -- ``swift-bmc`` OpenPOWER Swift BMC POWER9 +- ``swift-bmc`` OpenPOWER Swift BMC POWER9 (to be removed in v7.0) +- ``fp5280g2-bmc`` Inspur FP5280G2 BMC +- ``g220a-bmc`` Bytedance G220A BMC AST2600 SoC based machines : - ``ast2600-evb`` Aspeed AST2600 Evaluation board (Cortex-A7) - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC +- ``rainier-bmc`` IBM Rainier POWER10 BMC +- ``fuji-bmc`` Facebook Fuji BMC Supported devices ----------------- -- 2.25.1 From: Joel Stanley <joel@jms.id.au> This is the latest URL for the OpenBMC CI. The old URL still works, but redirects. Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Message-id: 20211117065752.330632-3-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -XXX,XX +XXX,XX @@ The Aspeed machines can be started using the ``-kernel`` option to load a Linux kernel or from a firmware. Images can be downloaded from the OpenBMC jenkins : - https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/distro=ubuntu,label=docker-builder + https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ or directly from the OpenBMC GitHub release repository : -- 2.25.1 From: Joel Stanley <joel@jms.id.au> A common use case for the ASPEED machine is to boot a Linux kernel. Provide a full example command line. Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Message-id: 20211117065752.330632-4-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/aspeed.rst | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -XXX,XX +XXX,XX @@ Missing devices Boot options ------------ -The Aspeed machines can be started using the ``-kernel`` option to -load a Linux kernel or from a firmware. Images can be downloaded from -the OpenBMC jenkins : +The Aspeed machines can be started using the ``-kernel`` and ``-dtb`` options +to load a Linux kernel or from a firmware. Images can be downloaded from the +OpenBMC jenkins : https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ @@ -XXX,XX +XXX,XX @@ or directly from the OpenBMC GitHub release repository : https://github.com/openbmc/openbmc/releases +To boot a kernel directly from a Linux build tree: + +.. code-block:: bash + + $ qemu-system-arm -M ast2600-evb -nographic \ + -kernel arch/arm/boot/zImage \ + -dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ + -initrd rootfs.cpio + The image should be attached as an MTD drive. Run : .. code-block:: bash -- 2.25.1 From: Joel Stanley <joel@jms.id.au> Move it to the supported list. Signed-off-by: Joel Stanley <joel@jms.id.au> Message-id: 20211117065752.330632-5-joel@jms.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -XXX,XX +XXX,XX @@ Supported devices * Front LEDs (PCA9552 on I2C bus) * LPC Peripheral Controller (a subset of subdevices are supported) * Hash/Crypto Engine (HACE) - Hash support only. TODO: HMAC and RSA + * ADC Missing devices --------------- * Coprocessor support - * ADC (out of tree implementation) * PWM and Fan Controller * Slave GPIO Controller * Super I/O Controller -- 2.25.1 From: Olivier Hériveaux <olivier.heriveaux@ledger.fr> Fix issue where the data register may be overwritten by next character reception before being read and returned. Signed-off-by: Olivier Hériveaux <olivier.heriveaux@ledger.fr> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20211128120723.4053-1-olivier.heriveaux@ledger.fr Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/char/stm32f2xx_usart.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c index XXXXXXX..XXXXXXX 100644 --- a/hw/char/stm32f2xx_usart.c +++ b/hw/char/stm32f2xx_usart.c @@ -XXX,XX +XXX,XX @@ static uint64_t stm32f2xx_usart_read(void *opaque, hwaddr addr, return retvalue; case USART_DR: DB_PRINT("Value: 0x%" PRIx32 ", %c\n", s->usart_dr, (char) s->usart_dr); + retvalue = s->usart_dr & 0x3FF; s->usart_sr &= ~USART_SR_RXNE; qemu_chr_fe_accept_input(&s->chr); qemu_set_irq(s->irq, 0); - return s->usart_dr & 0x3FF; + return retvalue; case USART_BRR: return s->usart_brr; case USART_CR1: -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> gicv3_set_gicv3state() is used by arm_gicv3_common.c in arm_gicv3_common_realize(). Since we want to restrict arm_gicv3_cpuif.c to TCG, extract gicv3_set_gicv3state() to a new file. Add this file to the meson 'specific' source set, since it needs access to "cpu.h". Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20211115223619.2599282-2-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_cpuif.c | 10 +--------- hw/intc/arm_gicv3_cpuif_common.c | 22 ++++++++++++++++++++++ hw/intc/meson.build | 1 + 3 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 hw/intc/arm_gicv3_cpuif_common.c diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -XXX,XX +XXX,XX @@ /* - * ARM Generic Interrupt Controller v3 + * ARM Generic Interrupt Controller v3 (emulation) * * Copyright (c) 2016 Linaro Limited * Written by Peter Maydell @@ -XXX,XX +XXX,XX @@ #include "hw/irq.h" #include "cpu.h" -void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s) -{ - ARMCPU *arm_cpu = ARM_CPU(cpu); - CPUARMState *env = &arm_cpu->env; - - env->gicv3state = (void *)s; -}; - static GICv3CPUState *icc_cs_from_env(CPUARMState *env) { return env->gicv3state; diff --git a/hw/intc/arm_gicv3_cpuif_common.c b/hw/intc/arm_gicv3_cpuif_common.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/intc/arm_gicv3_cpuif_common.c @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * ARM Generic Interrupt Controller v3 + * + * Copyright (c) 2016 Linaro Limited + * Written by Peter Maydell + * + * This code is licensed under the GPL, version 2 or (at your option) + * any later version. + */ + +#include "qemu/osdep.h" +#include "gicv3_internal.h" +#include "cpu.h" + +void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s) +{ + ARMCPU *arm_cpu = ARM_CPU(cpu); + CPUARMState *env = &arm_cpu->env; + + env->gicv3state = (void *)s; +}; diff --git a/hw/intc/meson.build b/hw/intc/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: files('xlnx-pmu-iomod-in specific_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: files('allwinner-a10-pic.c')) specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c')) +specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif_common.c')) specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif.c')) specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files('arm_gic_kvm.c')) specific_ss.add(when: ['CONFIG_ARM_GIC_KVM', 'TARGET_AARCH64'], if_true: files('arm_gicv3_kvm.c', 'arm_gicv3_its_kvm.c')) -- 2.25.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> The TYPE_ARM_GICV3 device is an emulated one. When using KVM, it is recommended to use the TYPE_KVM_ARM_GICV3 device (which uses in-kernel support). When using --with-devices-FOO, it is possible to build a binary with a specific set of devices. When this binary is restricted to KVM accelerator, the TYPE_ARM_GICV3 device is irrelevant, and it is desirable to remove it from the binary. Therefore introduce the CONFIG_ARM_GIC_TCG Kconfig selector which select the files required to have the TYPE_ARM_GICV3 device, but also allowing to de-select this device. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20211115223619.2599282-3-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3.c | 2 +- hw/intc/Kconfig | 5 +++++ hw/intc/meson.build | 10 ++++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3.c +++ b/hw/intc/arm_gicv3.c @@ -XXX,XX +XXX,XX @@ /* - * ARM Generic Interrupt Controller v3 + * ARM Generic Interrupt Controller v3 (emulation) * * Copyright (c) 2015 Huawei. * Copyright (c) 2016 Linaro Limited diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/Kconfig +++ b/hw/intc/Kconfig @@ -XXX,XX +XXX,XX @@ config APIC select MSI_NONBROKEN select I8259 +config ARM_GIC_TCG + bool + default y + depends on ARM_GIC && TCG + config ARM_GIC_KVM bool default y diff --git a/hw/intc/meson.build b/hw/intc/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files( 'arm_gic.c', 'arm_gic_common.c', 'arm_gicv2m.c', - 'arm_gicv3.c', 'arm_gicv3_common.c', - 'arm_gicv3_dist.c', 'arm_gicv3_its_common.c', - 'arm_gicv3_redist.c', +)) +softmmu_ss.add(when: 'CONFIG_ARM_GIC_TCG', if_true: files( + 'arm_gicv3.c', + 'arm_gicv3_dist.c', 'arm_gicv3_its.c', + 'arm_gicv3_redist.c', )) softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c')) softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c')) @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: files('xlnx-pmu-iomod-in specific_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: files('allwinner-a10-pic.c')) specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c')) specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif_common.c')) -specific_ss.add(when: 'CONFIG_ARM_GIC', if_true: files('arm_gicv3_cpuif.c')) +specific_ss.add(when: 'CONFIG_ARM_GIC_TCG', if_true: files('arm_gicv3_cpuif.c')) specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files('arm_gic_kvm.c')) specific_ss.add(when: ['CONFIG_ARM_GIC_KVM', 'TARGET_AARCH64'], if_true: files('arm_gicv3_kvm.c', 'arm_gicv3_its_kvm.c')) specific_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('armv7m_nvic.c')) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *s = container_of(dcbase, DisasContext, base); CPUARMState *env = cpu->env_ptr; + uint64_t pc = s->base.pc_next; uint32_t insn; if (s->ss_active && !s->pstate_ss) { @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } - s->pc_curr = s->base.pc_next; - insn = arm_ldl_code(env, &s->base, s->base.pc_next, s->sctlr_b); + s->pc_curr = pc; + insn = arm_ldl_code(env, &s->base, pc, s->sctlr_b); s->insn = insn; - s->base.pc_next += 4; + s->base.pc_next = pc + 4; s->fp_access_checked = false; s->sve_access_checked = false; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); CPUARMState *env = cpu->env_ptr; + uint32_t pc = dc->base.pc_next; unsigned int insn; if (arm_pre_translate_insn(dc)) { - dc->base.pc_next += 4; + dc->base.pc_next = pc + 4; return; } - dc->pc_curr = dc->base.pc_next; - insn = arm_ldl_code(env, &dc->base, dc->base.pc_next, dc->sctlr_b); + dc->pc_curr = pc; + insn = arm_ldl_code(env, &dc->base, pc, dc->sctlr_b); dc->insn = insn; - dc->base.pc_next += 4; + dc->base.pc_next = pc + 4; disas_arm_insn(dc, insn); arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); CPUARMState *env = cpu->env_ptr; + uint32_t pc = dc->base.pc_next; uint32_t insn; bool is_16bit; if (arm_pre_translate_insn(dc)) { - dc->base.pc_next += 2; + dc->base.pc_next = pc + 2; return; } - dc->pc_curr = dc->base.pc_next; - insn = arm_lduw_code(env, &dc->base, dc->base.pc_next, dc->sctlr_b); + dc->pc_curr = pc; + insn = arm_lduw_code(env, &dc->base, pc, dc->sctlr_b); is_16bit = thumb_insn_is_16bit(dc, dc->base.pc_next, insn); - dc->base.pc_next += 2; + pc += 2; if (!is_16bit) { - uint32_t insn2 = arm_lduw_code(env, &dc->base, dc->base.pc_next, - dc->sctlr_b); - + uint32_t insn2 = arm_lduw_code(env, &dc->base, pc, dc->sctlr_b); insn = insn << 16 | insn2; - dc->base.pc_next += 2; + pc += 2; } + dc->base.pc_next = pc; dc->insn = insn; if (dc->pstate_il) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create arm_check_ss_active and arm_check_kernelpage. Reverse the order of the tests. While it doesn't matter in practice, because only user-only has a kernel page and user-only never sets ss_active, ss_active has priority over execution exceptions and it is best to keep them in the proper order. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) dc->insn_start = tcg_last_op(); } -static bool arm_pre_translate_insn(DisasContext *dc) +static bool arm_check_kernelpage(DisasContext *dc) { #ifdef CONFIG_USER_ONLY /* Intercept jump to the magic kernel page. */ @@ -XXX,XX +XXX,XX @@ static bool arm_pre_translate_insn(DisasContext *dc) return true; } #endif + return false; +} +static bool arm_check_ss_active(DisasContext *dc) +{ if (dc->ss_active && !dc->pstate_ss) { /* Singlestep state is Active-pending. * If we're in this state at the start of a TB then either @@ -XXX,XX +XXX,XX @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t pc = dc->base.pc_next; unsigned int insn; - if (arm_pre_translate_insn(dc)) { + if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { dc->base.pc_next = pc + 4; return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t insn; bool is_16bit; - if (arm_pre_translate_insn(dc)) { + if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { dc->base.pc_next = pc + 2; return; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> The size of the code covered by a TranslationBlock cannot be 0; this is checked via assert in tb_gen_code. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) assert(s->base.num_insns == 1); gen_swstep_exception(s, 0, 0); s->base.is_jmp = DISAS_NORETURN; + s->base.pc_next = pc + 4; return; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We will reuse this section of arm_deliver_fault for raising pc alignment faults. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/tlb_helper.c | 45 +++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -XXX,XX +XXX,XX @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn, return syn; } -static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr, - MMUAccessType access_type, - int mmu_idx, ARMMMUFaultInfo *fi) +static uint32_t compute_fsr_fsc(CPUARMState *env, ARMMMUFaultInfo *fi, + int target_el, int mmu_idx, uint32_t *ret_fsc) { - CPUARMState *env = &cpu->env; - int target_el; - bool same_el; - uint32_t syn, exc, fsr, fsc; ARMMMUIdx arm_mmu_idx = core_to_arm_mmu_idx(env, mmu_idx); - - target_el = exception_target_el(env); - if (fi->stage2) { - target_el = 2; - env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4; - if (arm_is_secure_below_el3(env) && fi->s1ns) { - env->cp15.hpfar_el2 |= HPFAR_NS; - } - } - same_el = (arm_current_el(env) == target_el); + uint32_t fsr, fsc; if (target_el == 2 || arm_el_is_aa64(env, target_el) || arm_s1_regime_using_lpae_format(env, arm_mmu_idx)) { @@ -XXX,XX +XXX,XX @@ static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr, fsc = 0x3f; } + *ret_fsc = fsc; + return fsr; +} + +static void QEMU_NORETURN arm_deliver_fault(ARMCPU *cpu, vaddr addr, + MMUAccessType access_type, + int mmu_idx, ARMMMUFaultInfo *fi) +{ + CPUARMState *env = &cpu->env; + int target_el; + bool same_el; + uint32_t syn, exc, fsr, fsc; + + target_el = exception_target_el(env); + if (fi->stage2) { + target_el = 2; + env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4; + if (arm_is_secure_below_el3(env) && fi->s1ns) { + env->cp15.hpfar_el2 |= HPFAR_NS; + } + } + same_el = (arm_current_el(env) == target_el); + + fsr = compute_fsr_fsc(env, fi, target_el, mmu_idx, &fsc); + if (access_type == MMU_INST_FETCH) { syn = syn_insn_abort(same_el, fi->ea, fi->s1ptw, fsc); exc = EXCP_PREFETCH_ABORT; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> For A64, any input to an indirect branch can cause this. For A32, many indirect branch paths force the branch to be aligned, but BXWritePC does not. This includes the BX instruction but also other interworking changes to PC. Prior to v8, this case is UNDEFINED. With v8, this is CONSTRAINED UNPREDICTABLE and may either raise an exception or force align the PC. We choose to raise an exception because we have the infrastructure, it makes the generated code for gen_bx simpler, and it has the possibility of catching more guest bugs. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/syndrome.h | 5 ++++ linux-user/aarch64/cpu_loop.c | 46 ++++++++++++++++++++--------------- target/arm/tlb_helper.c | 18 ++++++++++++++ target/arm/translate-a64.c | 15 ++++++++++++ target/arm/translate.c | 22 ++++++++++++++++- 6 files changed, 87 insertions(+), 20 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, DEF_HELPER_2(exception_internal, void, env, i32) DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, void, env, i32) +DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) DEF_HELPER_2(wfi, void, env, i32) DEF_HELPER_1(wfe, void, env) diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/syndrome.h +++ b/target/arm/syndrome.h @@ -XXX,XX +XXX,XX @@ static inline uint32_t syn_illegalstate(void) return (EC_ILLEGALSTATE << ARM_EL_EC_SHIFT) | ARM_EL_IL; } +static inline uint32_t syn_pcalignment(void) +{ + return (EC_PCALIGNMENT << ARM_EL_EC_SHIFT) | ARM_EL_IL; +} + #endif /* TARGET_ARM_SYNDROME_H */ diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/aarch64/cpu_loop.c +++ b/linux-user/aarch64/cpu_loop.c @@ -XXX,XX +XXX,XX @@ void cpu_loop(CPUARMState *env) break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: - /* We should only arrive here with EC in {DATAABORT, INSNABORT}. */ ec = syn_get_ec(env->exception.syndrome); - assert(ec == EC_DATAABORT || ec == EC_INSNABORT); - - /* Both EC have the same format for FSC, or close enough. */ - fsc = extract32(env->exception.syndrome, 0, 6); - switch (fsc) { - case 0x04 ... 0x07: /* Translation fault, level {0-3} */ - si_signo = TARGET_SIGSEGV; - si_code = TARGET_SEGV_MAPERR; + switch (ec) { + case EC_DATAABORT: + case EC_INSNABORT: + /* Both EC have the same format for FSC, or close enough. */ + fsc = extract32(env->exception.syndrome, 0, 6); + switch (fsc) { + case 0x04 ... 0x07: /* Translation fault, level {0-3} */ + si_signo = TARGET_SIGSEGV; + si_code = TARGET_SEGV_MAPERR; + break; + case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */ + case 0x0d ... 0x0f: /* Permission fault, level {1-3} */ + si_signo = TARGET_SIGSEGV; + si_code = TARGET_SEGV_ACCERR; + break; + case 0x11: /* Synchronous Tag Check Fault */ + si_signo = TARGET_SIGSEGV; + si_code = TARGET_SEGV_MTESERR; + break; + case 0x21: /* Alignment fault */ + si_signo = TARGET_SIGBUS; + si_code = TARGET_BUS_ADRALN; + break; + default: + g_assert_not_reached(); + } break; - case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */ - case 0x0d ... 0x0f: /* Permission fault, level {1-3} */ - si_signo = TARGET_SIGSEGV; - si_code = TARGET_SEGV_ACCERR; - break; - case 0x11: /* Synchronous Tag Check Fault */ - si_signo = TARGET_SIGSEGV; - si_code = TARGET_SEGV_MTESERR; - break; - case 0x21: /* Alignment fault */ + case EC_PCALIGNMENT: si_signo = TARGET_SIGBUS; si_code = TARGET_BUS_ADRALN; break; diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -XXX,XX +XXX,XX @@ #include "cpu.h" #include "internals.h" #include "exec/exec-all.h" +#include "exec/helper-proto.h" static inline uint32_t merge_syn_data_abort(uint32_t template_syn, unsigned int target_el, @@ -XXX,XX +XXX,XX @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, arm_deliver_fault(cpu, vaddr, access_type, mmu_idx, &fi); } +void helper_exception_pc_alignment(CPUARMState *env, target_ulong pc) +{ + ARMMMUFaultInfo fi = { .type = ARMFault_Alignment }; + int target_el = exception_target_el(env); + int mmu_idx = cpu_mmu_index(env, true); + uint32_t fsc; + + env->exception.vaddress = pc; + + /* + * Note that the fsc is not applicable to this exception, + * since any syndrome is pcalignment not insn_abort. + */ + env->exception.fsr = compute_fsr_fsc(env, &fi, target_el, mmu_idx, &fsc); + raise_exception(env, EXCP_PREFETCH_ABORT, syn_pcalignment(), target_el); +} + #if !defined(CONFIG_USER_ONLY) /* diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint64_t pc = s->base.pc_next; uint32_t insn; + /* Singlestep exceptions have the highest priority. */ if (s->ss_active && !s->pstate_ss) { /* Singlestep state is Active-pending. * If we're in this state at the start of a TB then either @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) return; } + if (pc & 3) { + /* + * PC alignment fault. This has priority over the instruction abort + * that we would receive from a translation fault via arm_ldl_code. + * This should only be possible after an indirect branch, at the + * start of the TB. + */ + assert(s->base.num_insns == 1); + gen_helper_exception_pc_alignment(cpu_env, tcg_constant_tl(pc)); + s->base.is_jmp = DISAS_NORETURN; + s->base.pc_next = QEMU_ALIGN_UP(pc, 4); + return; + } + s->pc_curr = pc; insn = arm_ldl_code(env, &s->base, pc, s->sctlr_b); s->insn = insn; diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t pc = dc->base.pc_next; unsigned int insn; - if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { + /* Singlestep exceptions have the highest priority. */ + if (arm_check_ss_active(dc)) { + dc->base.pc_next = pc + 4; + return; + } + + if (pc & 3) { + /* + * PC alignment fault. This has priority over the instruction abort + * that we would receive from a translation fault via arm_ldl_code + * (or the execution of the kernelpage entrypoint). This should only + * be possible after an indirect branch, at the start of the TB. + */ + assert(dc->base.num_insns == 1); + gen_helper_exception_pc_alignment(cpu_env, tcg_constant_tl(pc)); + dc->base.is_jmp = DISAS_NORETURN; + dc->base.pc_next = QEMU_ALIGN_UP(pc, 4); + return; + } + + if (arm_check_kernelpage(dc)) { dc->base.pc_next = pc + 4; return; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Misaligned thumb PC is architecturally impossible. Assert is better than proceeding, in case we've missed something somewhere. Expand a comment about aligning the pc in gdbstub. Fail an incoming migrate if a thumb pc is misaligned. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/gdbstub.c | 9 +++++++-- target/arm/machine.c | 10 ++++++++++ target/arm/translate.c | 3 +++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -XXX,XX +XXX,XX @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) tmp = ldl_p(mem_buf); - /* Mask out low bit of PC to workaround gdb bugs. This will probably - cause problems if we ever implement the Jazelle DBX extensions. */ + /* + * Mask out low bits of PC to workaround gdb bugs. + * This avoids an assert in thumb_tr_translate_insn, because it is + * architecturally impossible to misalign the pc. + * This will probably cause problems if we ever implement the + * Jazelle DBX extensions. + */ if (n == 15) { tmp &= ~1; } diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ static int cpu_post_load(void *opaque, int version_id) return -1; } } + + /* + * Misaligned thumb pc is architecturally impossible. + * We have an assert in thumb_tr_translate_insn to verify this. + * Fail an incoming migrate to avoid this assert. + */ + if (!is_a64(env) && env->thumb && (env->regs[15] & 1)) { + return -1; + } + if (!kvm_enabled()) { pmu_op_finish(&cpu->env); } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t insn; bool is_16bit; + /* Misaligned thumb PC is architecturally impossible. */ + assert((dc->base.pc_next & 1) == 0); + if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { dc->base.pc_next = pc + 2; return; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Both single-step and pc alignment faults have priority over breakpoint exceptions. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ bool arm_debug_check_breakpoint(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; + target_ulong pc; int n; /* @@ -XXX,XX +XXX,XX @@ bool arm_debug_check_breakpoint(CPUState *cs) return false; } + /* + * Single-step exceptions have priority over breakpoint exceptions. + * If single-step state is active-pending, suppress the bp. + */ + if (arm_singlestep_active(env) && !(env->pstate & PSTATE_SS)) { + return false; + } + + /* + * PC alignment faults have priority over breakpoint exceptions. + */ + pc = is_a64(env) ? env->pc : env->regs[15]; + if ((is_a64(env) || !env->thumb) && (pc & 3) != 0) { + return false; + } + + /* + * Instruction aborts have priority over breakpoint exceptions. + * TODO: We would need to look up the page for PC and verify that + * it is present and executable. + */ + for (n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) { if (bp_wp_matches(cpu, n, false)) { return true; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/tcg/aarch64/pcalign-a64.c | 37 +++++++++++++++++++++++++ tests/tcg/arm/pcalign-a32.c | 46 +++++++++++++++++++++++++++++++ tests/tcg/aarch64/Makefile.target | 4 +-- tests/tcg/arm/Makefile.target | 4 +++ 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/aarch64/pcalign-a64.c create mode 100644 tests/tcg/arm/pcalign-a32.c diff --git a/tests/tcg/aarch64/pcalign-a64.c b/tests/tcg/aarch64/pcalign-a64.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/aarch64/pcalign-a64.c @@ -XXX,XX +XXX,XX @@ +/* Test PC misalignment exception */ + +#include <assert.h> +#include <signal.h> +#include <stdlib.h> +#include <stdio.h> + +static void *expected; + +static void sigbus(int sig, siginfo_t *info, void *vuc) +{ + assert(info->si_code == BUS_ADRALN); + assert(info->si_addr == expected); + exit(EXIT_SUCCESS); +} + +int main() +{ + void *tmp; + + struct sigaction sa = { + .sa_sigaction = sigbus, + .sa_flags = SA_SIGINFO + }; + + if (sigaction(SIGBUS, &sa, NULL) < 0) { + perror("sigaction"); + return EXIT_FAILURE; + } + + asm volatile("adr %0, 1f + 1\n\t" + "str %0, %1\n\t" + "br %0\n" + "1:" + : "=&r"(tmp), "=m"(expected)); + abort(); +} diff --git a/tests/tcg/arm/pcalign-a32.c b/tests/tcg/arm/pcalign-a32.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/arm/pcalign-a32.c @@ -XXX,XX +XXX,XX @@ +/* Test PC misalignment exception */ + +#ifdef __thumb__ +#error "This test must be compiled for ARM" +#endif + +#include <assert.h> +#include <signal.h> +#include <stdlib.h> +#include <stdio.h> + +static void *expected; + +static void sigbus(int sig, siginfo_t *info, void *vuc) +{ + assert(info->si_code == BUS_ADRALN); + assert(info->si_addr == expected); + exit(EXIT_SUCCESS); +} + +int main() +{ + void *tmp; + + struct sigaction sa = { + .sa_sigaction = sigbus, + .sa_flags = SA_SIGINFO + }; + + if (sigaction(SIGBUS, &sa, NULL) < 0) { + perror("sigaction"); + return EXIT_FAILURE; + } + + asm volatile("adr %0, 1f + 2\n\t" + "str %0, %1\n\t" + "bx %0\n" + "1:" + : "=&r"(tmp), "=m"(expected)); + + /* + * From v8, it is CONSTRAINED UNPREDICTABLE whether BXWritePC aligns + * the address or not. If so, we can legitimately fall through. + */ + return EXIT_SUCCESS; +} diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -XXX,XX +XXX,XX @@ VPATH += $(ARM_SRC) AARCH64_SRC=$(SRC_PATH)/tests/tcg/aarch64 VPATH += $(AARCH64_SRC) -# Float-convert Tests -AARCH64_TESTS=fcvt +# Base architecture tests +AARCH64_TESTS=fcvt pcalign-a64 fcvt: LDFLAGS+=-lm diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/arm/Makefile.target +++ b/tests/tcg/arm/Makefile.target @@ -XXX,XX +XXX,XX @@ run-fcvt: fcvt $(call run-test,fcvt,$(QEMU) $<,"$< on $(TARGET_NAME)") $(call diff-out,fcvt,$(ARM_SRC)/fcvt.ref) +# PC alignment test +ARM_TESTS += pcalign-a32 +pcalign-a32: CFLAGS+=-marm + ifeq ($(CONFIG_ARM_COMPATIBLE_SEMIHOSTING),y) # Semihosting smoke test for linux-user -- 2.25.1 In the SSE decode function gen_sse(), we combine a byte 'b' and a value 'b1' which can be [0..3], and switch on them: b |= (b1 << 8); switch (b) { ... default: unknown_op: gen_unknown_opcode(env, s); return; } In three cases inside this switch, we were then also checking for "if (b1 >= 2) { goto unknown_op; }". However, this can never happen, because the 'case' values in each place are 0x0nn or 0x1nn and the switch will have directed the b1 == (2, 3) cases to the default already. This check was added in commit c045af25a52e9 in 2010; the added code was unnecessary then as well, and was apparently intended only to ensure that we never accidentally ended up indexing off the end of an sse_op_table with only 2 entries as a result of future bugs in the decode logic. Change the checks to assert() instead, and make sure they're always immediately before the array access they are protecting. Fixes: Coverity CID 1460207 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/i386/tcg/translate.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, case 0x171: /* shift xmm, im */ case 0x172: case 0x173: - if (b1 >= 2) { - goto unknown_op; - } val = x86_ldub_code(env, s); if (is_xmm) { tcg_gen_movi_tl(s->T0, val); @@ -XXX,XX +XXX,XX @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, offsetof(CPUX86State, mmx_t0.MMX_L(1))); op1_offset = offsetof(CPUX86State,mmx_t0); } + assert(b1 < 2); sse_fn_epp = sse_op_table2[((b - 1) & 3) * 8 + (((modrm >> 3)) & 7)][b1]; if (!sse_fn_epp) { @@ -XXX,XX +XXX,XX @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, rm = modrm & 7; reg = ((modrm >> 3) & 7) | REX_R(s); mod = (modrm >> 6) & 3; - if (b1 >= 2) { - goto unknown_op; - } + assert(b1 < 2); sse_fn_epp = sse_op_table6[b].op[b1]; if (!sse_fn_epp) { goto unknown_op; @@ -XXX,XX +XXX,XX @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, rm = modrm & 7; reg = ((modrm >> 3) & 7) | REX_R(s); mod = (modrm >> 6) & 3; - if (b1 >= 2) { - goto unknown_op; - } + assert(b1 < 2); sse_fn_eppi = sse_op_table7[b].op[b1]; if (!sse_fn_eppi) { goto unknown_op; -- 2.25.1 The qemu-common.h header is not supposed to be included from any other header files, only from .c files (as documented in a comment at the start of it). include/hw/i386/x86.h and include/hw/i386/microvm.h break this rule. In fact, the include is not required at all, so we can just drop it from both files. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20211129200510.1233037-2-peter.maydell@linaro.org --- include/hw/i386/microvm.h | 1 - include/hw/i386/x86.h | 1 - 2 files changed, 2 deletions(-) diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i386/microvm.h +++ b/include/hw/i386/microvm.h @@ -XXX,XX +XXX,XX @@ #ifndef HW_I386_MICROVM_H #define HW_I386_MICROVM_H -#include "qemu-common.h" #include "exec/hwaddr.h" #include "qemu/notify.h" diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -XXX,XX +XXX,XX @@ #ifndef HW_I386_X86_H #define HW_I386_X86_H -#include "qemu-common.h" #include "exec/hwaddr.h" #include "qemu/notify.h" -- 2.25.1 The qemu-common.h header is not supposed to be included from any other header files, only from .c files (as documented in a comment at the start of it). Move the include to linux-user/hexagon/cpu_loop.c, which needs it for the declaration of cpu_exec_step_atomic(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> Message-id: 20211129200510.1233037-3-peter.maydell@linaro.org --- target/hexagon/cpu.h | 1 - linux-user/hexagon/cpu_loop.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/hexagon/cpu.h +++ b/target/hexagon/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUHexagonState CPUHexagonState; #include "fpu/softfloat-types.h" -#include "qemu-common.h" #include "exec/cpu-defs.h" #include "hex_regs.h" #include "mmvec/mmvec.h" diff --git a/linux-user/hexagon/cpu_loop.c b/linux-user/hexagon/cpu_loop.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/hexagon/cpu_loop.c +++ b/linux-user/hexagon/cpu_loop.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qemu-common.h" #include "qemu.h" #include "user-internals.h" #include "cpu_loop-common.h" -- 2.25.1 The qemu-common.h header is not supposed to be included from any other header files, only from .c files (as documented in a comment at the start of it). Nothing actually relies on target/rx/cpu.h including it, so we can just drop the include. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp> Message-id: 20211129200510.1233037-4-peter.maydell@linaro.org --- target/rx/cpu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/target/rx/cpu.h b/target/rx/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/rx/cpu.h +++ b/target/rx/cpu.h @@ -XXX,XX +XXX,XX @@ #define RX_CPU_H #include "qemu/bitops.h" -#include "qemu-common.h" #include "hw/registerfields.h" #include "cpu-qom.h" -- 2.25.1 A lot of C files in hw/arm include qemu-common.h when they don't need anything from it. Drop the include lines. omap1.c, pxa2xx.c and strongarm.c retain the include because they use it for the prototype of qemu_get_timedate(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp> Message-id: 20211129200510.1233037-5-peter.maydell@linaro.org --- hw/arm/boot.c | 1 - hw/arm/digic_boards.c | 1 - hw/arm/highbank.c | 1 - hw/arm/npcm7xx_boards.c | 1 - hw/arm/sbsa-ref.c | 1 - hw/arm/stm32f405_soc.c | 1 - hw/arm/vexpress.c | 1 - hw/arm/virt.c | 1 - 8 files changed, 8 deletions(-) diff --git a/hw/arm/boot.c b/hw/arm/boot.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "qemu/error-report.h" #include "qapi/error.h" diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/digic_boards.c +++ b/hw/arm/digic_boards.c @@ -XXX,XX +XXX,XX @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "hw/boards.h" #include "qemu/error-report.h" diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/highbank.c +++ b/hw/arm/highbank.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "qapi/error.h" #include "hw/sysbus.h" diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/npcm7xx_boards.c +++ b/hw/arm/npcm7xx_boards.c @@ -XXX,XX +XXX,XX @@ #include "hw/qdev-core.h" #include "hw/qdev-properties.h" #include "qapi/error.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "qemu/units.h" #include "sysemu/blockdev.h" diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "qapi/error.h" #include "qemu/error-report.h" diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stm32f405_soc.c +++ b/hw/arm/stm32f405_soc.c @@ -XXX,XX +XXX,XX @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "qemu-common.h" #include "exec/address-spaces.h" #include "sysemu/sysemu.h" #include "hw/arm/stm32f405_soc.h" diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -XXX,XX +XXX,XX @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "cpu.h" #include "hw/sysbus.h" diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" -#include "qemu-common.h" #include "qemu/datadir.h" #include "qemu/units.h" #include "qemu/option.h" -- 2.25.1 The calculation of the length of TLB range invalidate operations in tlbi_aa64_range_get_length() is incorrect in two ways: * the NUM field is 5 bits, but we read only 4 bits * we miscalculate the page_shift value, because of an off-by-one error: TG 0b00 is invalid TG 0b01 is 4K granule size == 4096 == 2^12 TG 0b10 is 16K granule size == 16384 == 2^14 TG 0b11 is 64K granule size == 65536 == 2^16 so page_shift should be (TG - 1) * 2 + 12 Thanks to the bug report submitter Cha HyunSoo for identifying both these errors. Fixes: 84940ed82552d3c ("target/arm: Add support for FEAT_TLBIRANGE") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/734 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20211130173257.1274194-1-peter.maydell@linaro.org --- target/arm/helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static uint64_t tlbi_aa64_range_get_length(CPUARMState *env, uint64_t exponent; uint64_t length; - num = extract64(value, 39, 4); + num = extract64(value, 39, 5); scale = extract64(value, 44, 2); page_size_granule = extract64(value, 46, 2); - page_shift = page_size_granule * 2 + 12; - if (page_size_granule == 0) { qemu_log_mask(LOG_GUEST_ERROR, "Invalid page size granule %d\n", page_size_granule); return 0; } + page_shift = (page_size_granule - 1) * 2 + 12; + exponent = (5 * scale) + 1; length = (num + 1) << (exponent + page_shift); -- 2.25.1 From: Patrick Venture <venture@google.com> The rx_active boolean change to true should always trigger a try_read call that flushes the queue. Signed-off-by: Patrick Venture <venture@google.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20211203221002.1719306-1-venture@google.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/npcm7xx_emc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/npcm7xx_emc.c +++ b/hw/net/npcm7xx_emc.c @@ -XXX,XX +XXX,XX @@ static void emc_halt_rx(NPCM7xxEMCState *emc, uint32_t mista_flag) emc_set_mista(emc, mista_flag); } +static void emc_enable_rx_and_flush(NPCM7xxEMCState *emc) +{ + emc->rx_active = true; + qemu_flush_queued_packets(qemu_get_queue(emc->nic)); +} + static void emc_set_next_tx_descriptor(NPCM7xxEMCState *emc, const NPCM7xxEMCTxDesc *tx_desc, uint32_t desc_addr) @@ -XXX,XX +XXX,XX @@ static ssize_t emc_receive(NetClientState *nc, const uint8_t *buf, size_t len1) return len; } -static void emc_try_receive_next_packet(NPCM7xxEMCState *emc) -{ - if (emc_can_receive(qemu_get_queue(emc->nic))) { - qemu_flush_queued_packets(qemu_get_queue(emc->nic)); - } -} - static uint64_t npcm7xx_emc_read(void *opaque, hwaddr offset, unsigned size) { NPCM7xxEMCState *emc = opaque; @@ -XXX,XX +XXX,XX @@ static void npcm7xx_emc_write(void *opaque, hwaddr offset, emc->regs[REG_MGSTA] |= REG_MGSTA_RXHA; } if (value & REG_MCMDR_RXON) { - emc->rx_active = true; + emc_enable_rx_and_flush(emc); } else { emc_halt_rx(emc, 0); } @@ -XXX,XX +XXX,XX @@ static void npcm7xx_emc_write(void *opaque, hwaddr offset, break; case REG_RSDR: if (emc->regs[REG_MCMDR] & REG_MCMDR_RXON) { - emc->rx_active = true; - emc_try_receive_next_packet(emc); + emc_enable_rx_and_flush(emc); } break; case REG_MIIDA: -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> When a virtio-iommu is instantiated, describe it using the ACPI VIOT table. Acked-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-2-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 7 +++++++ hw/arm/Kconfig | 1 + 2 files changed, 8 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -XXX,XX +XXX,XX @@ #include "kvm_arm.h" #include "migration/vmstate.h" #include "hw/acpi/ghes.h" +#include "hw/acpi/viot.h" #define ARM_SPI_BASE 32 @@ -XXX,XX +XXX,XX @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) } #endif + if (vms->iommu == VIRT_IOMMU_VIRTIO) { + acpi_add_table(table_offsets, tables_blob); + build_viot(ms, tables_blob, tables->linker, vms->virtio_iommu_bdf, + vms->oem_id, vms->oem_table_id); + } + /* XSDT is pointed to by RSDP */ xsdt = tables_blob->len; build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id, diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ARM_VIRT select DIMM select ACPI_HW_REDUCED select ACPI_APEI + select ACPI_VIOT config CHEETAH bool -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> virtio-iommu is now supported with ACPI VIOT as well as device tree. Remove the restriction that prevents from instantiating a virtio-iommu device under ACPI. Acked-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-3-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 10 ++-------- hw/virtio/virtio-iommu-pci.c | 12 ++---------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine, MachineClass *mc = MACHINE_GET_CLASS(machine); if (device_is_dynamic_sysbus(mc, dev) || - (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) { + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { return HOTPLUG_HANDLER(machine); } - if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { - VirtMachineState *vms = VIRT_MACHINE(machine); - - if (!vms->bootinfo.firmware_loaded || !virt_is_acpi_enabled(vms)) { - return HOTPLUG_HANDLER(machine); - } - } return NULL; } diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-iommu-pci.c +++ b/hw/virtio/virtio-iommu-pci.c @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) VirtIOIOMMU *s = VIRTIO_IOMMU(vdev); if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) { - MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); - - error_setg(errp, - "%s machine fails to create iommu-map device tree bindings", - mc->name); - error_append_hint(errp, - "Check your machine implements a hotplug handler " - "for the virtio-iommu-pci device\n"); - error_append_hint(errp, "Check the guest is booted without FW or with " - "-no-acpi\n"); + error_setg(errp, "Check your machine implements a hotplug handler " + "for the virtio-iommu-pci device"); return; } for (int i = 0; i < s->nb_reserved_regions; i++) { -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> We do not support instantiating multiple IOMMUs. Before adding a virtio-iommu, check that no other IOMMU is present. This will detect both "iommu=smmuv3" machine parameter and another virtio-iommu instance. Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings") Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-4-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, hwaddr db_start = 0, db_end = 0; char *resv_prop_str; + if (vms->iommu != VIRT_IOMMU_NONE) { + error_setg(errp, "virt machine does not support multiple IOMMUs"); + return; + } + switch (vms->msi_controller) { case VIRT_MSI_CTRL_NONE: return; -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> To propagate errors to the caller of the pre_plug callback, use the object_poperty_set*() functions directly instead of the qdev_prop_set*() helpers. Suggested-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-5-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, db_start, db_end, VIRTIO_IOMMU_RESV_MEM_T_MSI); - qdev_prop_set_uint32(dev, "len-reserved-regions", 1); - qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str); + object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, errp); + object_property_set_str(OBJECT(dev), "reserved-regions[0]", + resv_prop_str, errp); g_free(resv_prop_str); } } -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> Create empty data files and allow updates for the upcoming VIOT tests. Acked-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-6-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ tests/data/acpi/q35/DSDT.viot | 0 tests/data/acpi/q35/VIOT.viot | 0 tests/data/acpi/virt/VIOT | 0 4 files changed, 3 insertions(+) create mode 100644 tests/data/acpi/q35/DSDT.viot create mode 100644 tests/data/acpi/q35/VIOT.viot create mode 100644 tests/data/acpi/virt/VIOT diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/VIOT", +"tests/data/acpi/q35/DSDT.viot", +"tests/data/acpi/q35/VIOT.viot", diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot new file mode 100644 index XXXXXXX..XXXXXXX diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot new file mode 100644 index XXXXXXX..XXXXXXX diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT new file mode 100644 index XXXXXXX..XXXXXXX -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> Add two test cases for VIOT, one on the q35 machine and the other on virt. To test complex topologies the q35 test has two PCIe buses that bypass the IOMMU (and are therefore not described by VIOT), and two buses that are translated by virtio-iommu. Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-7-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -XXX,XX +XXX,XX @@ static void test_acpi_virt_tcg(void) free_test_data(&data); } +static void test_acpi_q35_viot(void) +{ + test_data data = { + .machine = MACHINE_Q35, + .variant = ".viot", + }; + + /* + * To keep things interesting, two buses bypass the IOMMU. + * VIOT should only describes the other two buses. + */ + test_acpi_one("-machine default_bus_bypass_iommu=on " + "-device virtio-iommu-pci " + "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 " + "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on " + "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0", + &data); + free_test_data(&data); +} + +static void test_acpi_virt_viot(void) +{ + test_data data = { + .machine = "virt", + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd", + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", + .ram_start = 0x40000000ULL, + .scan_len = 128ULL * 1024 * 1024, + }; + + test_acpi_one("-cpu cortex-a57 " + "-device virtio-iommu-pci", &data); + free_test_data(&data); +} + static void test_oem_fields(test_data *data) { int i; @@ -XXX,XX +XXX,XX @@ int main(int argc, char *argv[]) qtest_add_func("acpi/q35/kvm/xapic", test_acpi_q35_kvm_xapic); qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar); } + qtest_add_func("acpi/q35/viot", test_acpi_q35_viot); } else if (strcmp(arch, "aarch64") == 0) { if (has_tcg) { qtest_add_func("acpi/virt", test_acpi_virt_tcg); @@ -XXX,XX +XXX,XX @@ int main(int argc, char *argv[]) qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp); qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb); qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt); + qtest_add_func("acpi/virt/viot", test_acpi_virt_viot); } } ret = g_test_run(); -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> Add expected blobs of the VIOT and DSDT table for the VIOT test on the q35 machine. Since the test instantiates a virtio device and two PCIe expander bridges, DSDT.viot has more blocks than the base DSDT. The VIOT table generated for the q35 test is: [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] [004h 0004 4] Table Length : 00000070 [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : 3D [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPC " [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 2] Node count : 0003 [026h 0038 2] Node offset : 0030 [028h 0040 8] Reserved : 0000000000000000 [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] [031h 0049 1] Reserved : 00 [032h 0050 2] Length : 0010 [034h 0052 2] PCI Segment : 0000 [036h 0054 2] PCI BDF number : 0010 [038h 0056 8] Reserved : 0000000000000000 [040h 0064 1] Type : 01 [PCI Range] [041h 0065 1] Reserved : 00 [042h 0066 2] Length : 0018 [044h 0068 4] Endpoint start : 00003000 [048h 0072 2] PCI Segment start : 0000 [04Ah 0074 2] PCI Segment end : 0000 [04Ch 0076 2] PCI BDF start : 3000 [04Eh 0078 2] PCI BDF end : 30FF [050h 0080 2] Output node : 0030 [052h 0082 6] Reserved : 000000000000 [058h 0088 1] Type : 01 [PCI Range] [059h 0089 1] Reserved : 00 [05Ah 0090 2] Length : 0018 [05Ch 0092 4] Endpoint start : 00001000 [060h 0096 2] PCI Segment start : 0000 [062h 0098 2] PCI Segment end : 0000 [064h 0100 2] PCI BDF start : 1000 [066h 0102 2] PCI BDF end : 10FF [068h 0104 2] Output node : 0030 [06Ah 0106 6] Reserved : 000000000000 And the DSDT diff is: @@ -XXX,XX +XXX,XX @@ * * Disassembling to symbolic ASL+ operators * - * Disassembly of tests/data/acpi/q35/DSDT, Fri Dec 10 15:03:08 2021 + * Disassembly of /tmp/aml-H9Y5D1, Fri Dec 10 15:02:27 2021 * * Original Table Header: * Signature "DSDT" - * Length 0x00002061 (8289) + * Length 0x000024B6 (9398) * Revision 0x01 **** 32-bit table (V1), no 64-bit math support - * Checksum 0xFA + * Checksum 0xA7 * OEM ID "BOCHS " * OEM Table ID "BXPC " * OEM Revision 0x00000001 (1) @@ -XXX,XX +XXX,XX @@ } } + Scope (\_SB) + { + Device (PC30) + { + Name (_UID, 0x30) // _UID: Unique ID + Name (_BBN, 0x30) // _BBN: BIOS Bus Number + Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID + Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID + Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities + { + CreateDWordField (Arg3, Zero, CDW1) + If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */)) + { + CreateDWordField (Arg3, 0x04, CDW2) + CreateDWordField (Arg3, 0x08, CDW3) + Local0 = CDW3 /* \_SB_.PC30._OSC.CDW3 */ + Local0 &= 0x1F + If ((Arg1 != One)) + { + CDW1 |= 0x08 + } + + If ((CDW3 != Local0)) + { + CDW1 |= 0x10 + } + + CDW3 = Local0 + } + Else + { + CDW1 |= 0x04 + } + + Return (Arg3) + } + + Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table + { + Local0 = Package (0x80){} + Local1 = Zero + While ((Local1 < 0x80)) + { + Local2 = (Local1 >> 0x02) + Local3 = ((Local1 + Local2) & 0x03) + If ((Local3 == Zero)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKD, + Zero + } + } + + If ((Local3 == One)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKA, + Zero + } + } + + If ((Local3 == 0x02)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKB, + Zero + } + } + + If ((Local3 == 0x03)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKC, + Zero + } + } + + Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF) + Local4 [One] = (Local1 & 0x03) + Local0 [Local1] = Local4 + Local1++ + } + + Return (Local0) + } + + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, + 0x0000, // Granularity + 0x0030, // Range Minimum + 0x0030, // Range Maximum + 0x0000, // Translation Offset + 0x0001, // Length + ,, ) + }) + } + } + + Scope (\_SB) + { + Device (PC20) + { + Name (_UID, 0x20) // _UID: Unique ID + Name (_BBN, 0x20) // _BBN: BIOS Bus Number + Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID + Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID + Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities + { + CreateDWordField (Arg3, Zero, CDW1) + If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */)) + { + CreateDWordField (Arg3, 0x04, CDW2) + CreateDWordField (Arg3, 0x08, CDW3) + Local0 = CDW3 /* \_SB_.PC20._OSC.CDW3 */ + Local0 &= 0x1F + If ((Arg1 != One)) + { + CDW1 |= 0x08 + } + + If ((CDW3 != Local0)) + { + CDW1 |= 0x10 + } + + CDW3 = Local0 + } + Else + { + CDW1 |= 0x04 + } + + Return (Arg3) + } + + Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table + { + Local0 = Package (0x80){} + Local1 = Zero + While ((Local1 < 0x80)) + { + Local2 = (Local1 >> 0x02) + Local3 = ((Local1 + Local2) & 0x03) + If ((Local3 == Zero)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKD, + Zero + } + } + + If ((Local3 == One)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKA, + Zero + } + } + + If ((Local3 == 0x02)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKB, + Zero + } + } + + If ((Local3 == 0x03)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKC, + Zero + } + } + + Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF) + Local4 [One] = (Local1 & 0x03) + Local0 [Local1] = Local4 + Local1++ + } + + Return (Local0) + } + + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, + 0x0000, // Granularity + 0x0020, // Range Minimum + 0x0020, // Range Maximum + 0x0000, // Translation Offset + 0x0001, // Length + ,, ) + }) + } + } + + Scope (\_SB) + { + Device (PC10) + { + Name (_UID, 0x10) // _UID: Unique ID + Name (_BBN, 0x10) // _BBN: BIOS Bus Number + Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID + Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID + Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities + { + CreateDWordField (Arg3, Zero, CDW1) + If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */)) + { + CreateDWordField (Arg3, 0x04, CDW2) + CreateDWordField (Arg3, 0x08, CDW3) + Local0 = CDW3 /* \_SB_.PC10._OSC.CDW3 */ + Local0 &= 0x1F + If ((Arg1 != One)) + { + CDW1 |= 0x08 + } + + If ((CDW3 != Local0)) + { + CDW1 |= 0x10 + } + + CDW3 = Local0 + } + Else + { + CDW1 |= 0x04 + } + + Return (Arg3) + } + + Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table + { + Local0 = Package (0x80){} + Local1 = Zero + While ((Local1 < 0x80)) + { + Local2 = (Local1 >> 0x02) + Local3 = ((Local1 + Local2) & 0x03) + If ((Local3 == Zero)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKD, + Zero + } + } + + If ((Local3 == One)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKA, + Zero + } + } + + If ((Local3 == 0x02)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKB, + Zero + } + } + + If ((Local3 == 0x03)) + { + Local4 = Package (0x04) + { + Zero, + Zero, + LNKC, + Zero + } + } + + Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF) + Local4 [One] = (Local1 & 0x03) + Local0 [Local1] = Local4 + Local1++ + } + + Return (Local0) + } + + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, + 0x0000, // Granularity + 0x0010, // Range Minimum + 0x0010, // Range Maximum + 0x0000, // Translation Offset + 0x0001, // Length + ,, ) + }) + } + } + Scope (\_SB.PCI0) { Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings @@ -XXX,XX +XXX,XX @@ WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode, 0x0000, // Granularity 0x0000, // Range Minimum - 0x00FF, // Range Maximum + 0x000F, // Range Maximum 0x0000, // Translation Offset - 0x0100, // Length + 0x0010, // Length ,, ) IO (Decode16, 0x0CF8, // Range Minimum @@ -XXX,XX +XXX,XX @@ } } + Device (S10) + { + Name (_ADR, 0x00020000) // _ADR: Address + } + + Device (S18) + { + Name (_ADR, 0x00030000) // _ADR: Address + } + + Device (S20) + { + Name (_ADR, 0x00040000) // _ADR: Address + } + + Device (S28) + { + Name (_ADR, 0x00050000) // _ADR: Address + } + Method (PCNT, 0, NotSerialized) { } Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-8-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 2 -- tests/data/acpi/q35/DSDT.viot | Bin 0 -> 9398 bytes tests/data/acpi/q35/VIOT.viot | Bin 0 -> 112 bytes 3 files changed, 2 deletions(-) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -XXX,XX +XXX,XX @@ /* List of comma-separated changed AML files to ignore */ "tests/data/acpi/virt/VIOT", -"tests/data/acpi/q35/DSDT.viot", -"tests/data/acpi/q35/VIOT.viot", diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot index XXXXXXX..XXXXXXX 100644 GIT binary patch literal 9398 zcmeHNO>7&-8J*>iv|O&FB}G~Oi$yp||57BBoWHhc5OS9yDTx$CQgH$r;8Idr*-4Q_ z5(9Az1F`}niVsB-)<KW7p`g9Br(A2Gm-gmc1N78GFS!;)e2V(MnH_0{q<{#yMgn&C zn|*J-d9yqFhO_H6z19~`FlPL*u<DkZ*}|)JH;X@mF-FI<cPg<fti9tEN*yB^i5czN zNq&q?!OZ;BE3B7{KWzJ-`Tn~f`9?Qj8~2^N8{Oc8J%57{==w%rS#;nOCp*nTr@iZ1 zb+?i;JLQUJ=O0?8*>S~D)a>NF1~WVB6^~_B#yhJ`H+JU@=6aXs`?Yv)J2h=N?drcS zeLZ*n<<Bm^n}6`jfBx#u8&(W}1?)}iF9o#mZ~E2+zwdn7yK3AbIzKnxpZ>JRPm3~# z&ICS{+_OayRW-l=Mtk=~uaS3o8z<_udd|(wqg`&JnVPfCe>BUOO`Su3e>pff_^UW% z&JE^NO`)=Amg~iqRB1pPscP?(>#ZuY8GHCmlEvD$9g3%4Db~Dfz2SATnddvrR-Oe^ z;s;dJec!hnzi)ri^I6YN9vtkm{^TdUF8h7gX8-<Qe4p)GQ=)AtYx2VcwdLVAEXEjG z^Mj|UHPqkj-LsWuzQem1>F3atdZn=zv3$#RmZzSHN+6-yyU#8cJb=YDilX&sl}vNm znkgAR^O<3kj4if>{ly5fwRfMWuC5=lrlvKPX~i#654Cp}R_d*JS$9laZ$ra6)<ns8 zFZy28G%xP(nit&F>LDi%G<tIc=TY=gl$jSD&Uv!Yat~XR46h%rI$!}a%!|xG7u8Zn zeY8_|n=K>xz_v_W8VX$W-Fg-qFWcT}7MCyz{%%{ia7hZ>Law-k6NOr}VI&_48U=2l zwqDKFE8eTwwozDdms#e?x?5a|v>&JF;2_v0L~z5n%BYU^52<*cWuD4|GYUm@1+?)) zte^45>Rz)t*<T5V#={r>@t@{%?^i#W{i=HAZ*Dc9y59Va-+#P!jrGs;u38a{fLr`N zvT@rUu>DljxJ?^&Z?-?vyJn3C>3D=qux{Y*bs5|5n)Qmi$TD^Zdn4GU$ocJS2Hh-< z`xPI^^+v0nUVdjMos8k`WGl7hA`{03ju%<lrgAHSpd^DRf-*}_#Ly0mB!LSfVgWcQ z&T$@~G9)JI=hz5m0vkrel+Xy{Oh7pkAu-V!j*W7rY(bO}Q$nMH2`FbGB&N)QaV4<4 zo)~9JXiP9=;}NPl<C@MmXG&;XFlFNrsyfFsonxFSp<}vEgsRSQP3O3#b6nSnP}ON_ zI!#Tdsp~|j>ckUB>FI=~GokB5sOq#dotCE4(sd$KbtW~PNlj-`*NIToiD#j5J#9^= zt?NXn>YUJYPG~wObe#xQos*i*NloXZt`niEb4t@WrRki~bs|)CI+{*L)9L6s5vn>< zn$DD_Go|Z9sOn5>I@6lYw5}7Os&iV?Ij!lO)^#FOb!If38BJ$K*NIToIiu;E(R9w} zIuWWmPiZ<&X*y5oIuWWmF_XaEC!a&Jn$B5WCqh-{X-(&8P3LJ{Cqh-{8P3dyPr@^t zSqL9?X9Uwd3W@23*s~h*tj0X6GZCuHa~kuU#yqDp5vt7d8uPryJg+kms?5hU=3^T3 zF`bD}WnSP+=`t5MQ$FJ_2&Q~+BP6E0f^%BVIW6a$o)e+SX~IDBih-7z6{O~7YTy`& zLjy&Cv?7QikV#>n0>>@MV8oK`Gmun34-FKdlm-J8SZSaNlnhir4-FI{S|bfqV8e)V zss<{chX#reE#g=hsKAC%sF6d-Km}BWs!kZFsFpKfpbC@>6rprQGEjt4Ck#|zITHq| zK*>M_l;<P^MJRQ`Kn0dFVW0|>3{*fllMEE0)CmI>Sk8ojDo`>|0p(0GP=xY&!axO< zGhv_#lnhirIg<<&q0|Wj6<E%MfhtfkPyyvkGEjt4Ck#|zITHq|K*>M_lrzad5lWpf zP=V!47^ngz0~JutBm+e#b;3XemNQ|X3X}{~Ksl2P6rt1!0~J`#gn=qhGEf2KOfpb} zQYQ>lU^x>8szAv=1(Y+%KoLrvFi?TzOc<yFB?A>u&LjgxD0RX>1(q{mpbC@>R6seC z3>2Z%2?G^a&V+#~P%=;f<xDbAgi<FARA4z12C6{GKn0XD$v_cGoiI>=<xCi;0wn_# zP|hR+MJRQ`Kn0dFVW0|>3{*fllMEE0)CmI>Sk8ojDo`>|0p(0GP=rz^3{+q_69%e4 z$v_2^Gs!>^N}VuJf#pmXr~)Me6;RG314Srx!axxz28u{EP=u<1B2)}iVZuNaCK;&0 zBm-5LFi?dF167!0pbC==RAItE6($T+VUmF=Ofpb~2?JG_Fi?d_2C6X0Kouqo6p_5T zFi=FeV!SiSKoR0H$dH(_Z(*Q_WZ%L-5y`$K14StNmJAdjmWs}HV4<vU_xO+1efmLq zZ;W>N_U)fP6Qy6Nw5mbt9Y(#emWSi66=>tq#xoh#Ue=0qyhxi8ZOUe5y0V7VfPUhp zwX=;ymc+i5%sg9Ja~lZ&8oAV@mHc>&CHP9v4R(jhtT?un;O4e9#pno)Xkh7OWgK&a zyj=3Iv0OuoK_;5rOr5f(Kb~ZXDBO+V`OWYo#_C08imwChQxnjdd?wZLDou8aj;$SD zGDYiA3<$Tu<JnHL(KPOChi#zrR32t83}naR$+ym4P_h?z_5#|cW-nw$XD_sOtE62l zrD3@*)NVyiklt0&yF9%+klsBey&I<Y2E<!f(E8TuJte)z(|ZHyy<^gQVfx}=`q&B5 z7nSryp1wGczIaUfVwiq$Fn#<4=@*ssi#+|}K>EdF(l3VTOM~ghPLRH&q%ZOGrGfON zW73zx^yR_y<0nX8R??Sw`tm^f@-gYlNFSp|*<gA{q?Zp5Oe-+l#rmyYmKozi9y=P> zVReJU*h=ZuVXiS$ohTbw-O#v9>(yZbGE|)?8(H1ZIKvV!jWa0>vy!3eMA^vdhQ>`s zuMSg{q3T50$m)j1!HixV<}X9liL#N^4c*tL^y)CF8LCc{jjV3yKAqL8!%SzWI#H%q z=bSrQ&)%JCRttF5g4Zf`6l?y@>PzD7MA^D>wBlcH6r1ucwJ<p0O%rZ?JzIY3-QdmZ zzs|n>`a5r3e|z)wcUaqS>nqFQ-8x}eCF4u`OWUxqst-@1rSmUs%WmKP5e0dcb?e2N z;Z|x*!);VwF|Yuhqs^khqOM!@u*jY!WYldISF(V6`BoNd&6Qfk3>X#SuD^7J>p_D= zBPa51y^_n#=cpOt#Zf$ya$Ae9Mfz56n|<i!a=ELS@)%a{^NIH3SDuN<R~sah1km#P zU@?*f%<rG=4W1wgfi;C?_n|W@%lm$&8YfvNOJodIg&IcIpIJQRHr<+ej11GQ6)&eF z2Lam*jIH}#y0>KnY%4JQfOYS$*uU%f#@$U6`N8I3N-lV?5ErFCdv~xDmu2(wexld4 z4v^;aVAT2k6GJ^m*FD(Wqc(Qg^)6a<?}h$zLoj}4;PP!+(O{@!a1y-hoAhF_7!z+6 zslpAmNtYbjHrw-~#SPVk_FUf>-Obg6yV`8o$8_`PyJe_;bY5_EMBfBfWU!Q=*9HsG z%_Cda{@_Krr!oHVhv9+y+T5qR8zZ2aZ>5r!$*|f$^U%yBUYfR&B!+EYy_PwL!BeUi zJH^}r3r9Q+B)X@Z)fk=P13w&7x#wBtXTZ)g>WITPg5r&pQc!nmyrmk#S(>>b9xnNr zx_b#v9Xv-Y><Wb%?S^0Xe&<)bbKl_=Z|3C$tf|F<bYzE*mfHB;uC)`q-?buaBe?l? zcLTpK*k<49Z32`K?|nSBMFqxTK^_IE-li2fEGdK~(ZdoKBl6ab4a;Hler#`xvEXJG zb?<E%EZExfX>jcOVhS*0rS~RS1dA#xhkv@Nct@#q?LyeKS<$uFec!bw>{@uu$gZ6a zyVen1i{1BKd%~`D7|m$;U0a<I*3I7%^N%N%lGYdU_GS!gaR8T$NA@GzFi~z`l7hdl zarZy6590|88pi(1zq;V(>38zM0sT&<zX;R5$1w3;`_JMG`;&I&0Y23DMx1%@(w(R9 z4M$j;D5J+Gy%fijRQsctzFKf&cv|BAz#YLq3CZJWDdtL4u1u1|mkdcUp7|sxJC+?Y z_@@s`v3j}Q7*z>6X~cwUxUL8G1KT)_XTp!KAbs;vCp{K3&~_X@+ew=-D}v`2MbFV0 zQsVsL=rXi-pI*G|iiz;VTCutgUs)hDzV1+4?8KcoP3xROf<M%qC6lgVdpFt4<-|uM z=#rl_b1#YjSIl6Toj2z_hOZcKupkdE(LozC(fN=FY(x|sk)ym|;Rq2E1xJWD%Z!ol Gu>S+TT-130 literal 0 HcmV?d00001 diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot index XXXXXXX..XXXXXXX 100644 GIT binary patch literal 112 zcmWIZ^baXu00LVle`k+i1*eDrX9XZ&1PX!JAex!M0Hgv8m>C3sGzdcgBZCA3T-xBj Q0Zb)W9Hva*zW_`e0M!8s0RR91 literal 0 HcmV?d00001 -- 2.25.1 From: Jean-Philippe Brucker <jean-philippe@linaro.org> The VIOT blob contains the following: [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table] [004h 0004 4] Table Length : 00000058 [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : 66 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPC " [018h 0024 4] Oem Revision : 00000001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4] Asl Compiler Revision : 00000001 [024h 0036 2] Node count : 0002 [026h 0038 2] Node offset : 0030 [028h 0040 8] Reserved : 0000000000000000 [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] [031h 0049 1] Reserved : 00 [032h 0050 2] Length : 0010 [034h 0052 2] PCI Segment : 0000 [036h 0054 2] PCI BDF number : 0008 [038h 0056 8] Reserved : 0000000000000000 [040h 0064 1] Type : 01 [PCI Range] [041h 0065 1] Reserved : 00 [042h 0066 2] Length : 0018 [044h 0068 4] Endpoint start : 00000000 [048h 0072 2] PCI Segment start : 0000 [04Ah 0074 2] PCI Segment end : 0000 [04Ch 0076 2] PCI BDF start : 0000 [04Eh 0078 2] PCI BDF end : 00FF [050h 0080 2] Output node : 0030 [052h 0082 6] Reserved : 000000000000 Acked-by: Ani Sinha <ani@anisinha.ca> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Message-id: 20211210170415.583179-9-jean-philippe@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 1 - tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes 2 files changed, 1 deletion(-) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index XXXXXXX..XXXXXXX 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/virt/VIOT", diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT index XXXXXXX..XXXXXXX 100644 GIT binary patch literal 88 zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX I{D-Rq0Q5fy0RR91 literal 0 HcmV?d00001 -- 2.25.1 Just flushing my target-arm queue since I won't be working next week :-) -- PMM The following changes since commit b3cd3b5a66f0dddfe3d5ba2bef13cd4f5b89cde9: Merge tag 'pull-riscv-to-apply-20220610' of github.com:alistair23/qemu into staging (2022-06-09 22:08:27 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220610 for you to fetch changes up to 90c072e063737e9e8f431489bbd334452f89056e: semihosting/config: Merge --semihosting-config option groups (2022-06-10 14:32:36 +0100) ---------------------------------------------------------------- * refactor exception routing code * fix SCR_EL3 RAO/RAZ bits * gdbstub: Don't use GDB syscalls if no GDB is attached * semihosting/config: Merge --semihosting-config option groups * tests/qtest: Reduce npcm7xx_sdhci test image size ---------------------------------------------------------------- Hao Wu (1): tests/qtest: Reduce npcm7xx_sdhci test image size Peter Maydell (2): gdbstub: Don't use GDB syscalls if no GDB is attached semihosting/config: Merge --semihosting-config option groups Richard Henderson (25): target/arm: Mark exception helpers as noreturn target/arm: Add coproc parameter to syn_fp_access_trap target/arm: Move exception_target_el out of line target/arm: Move arm_singlestep_active out of line target/arm: Move arm_generate_debug_exceptions out of line target/arm: Use is_a64 in arm_generate_debug_exceptions target/arm: Move exception_bkpt_insn to debug_helper.c target/arm: Move arm_debug_exception_fsr to debug_helper.c target/arm: Rename helper_exception_with_syndrome target/arm: Introduce gen_exception_insn_el_v target/arm: Rename gen_exception_insn to gen_exception_insn_el target/arm: Introduce gen_exception_insn target/arm: Create helper_exception_swstep target/arm: Remove TBFLAG_ANY.DEBUG_TARGET_EL target/arm: Move gen_exception to translate.c target/arm: Rename gen_exception to gen_exception_el target/arm: Introduce gen_exception target/arm: Introduce gen_exception_el_v target/arm: Introduce helper_exception_with_syndrome target/arm: Remove default_exception_el target/arm: Create raise_exception_debug target/arm: Move arm_debug_target_el to debug_helper.c target/arm: Fix Secure PL1 tests in fp_exception_el target/arm: Adjust format test in scr_write target/arm: SCR_EL3.RW is RAO/WI without AArch32 EL[12] target/arm/cpu.h | 133 ++--------------------- target/arm/helper.h | 8 +- target/arm/internals.h | 43 +------- target/arm/syndrome.h | 7 +- target/arm/translate.h | 43 ++------ gdbstub.c | 14 ++- semihosting/config.c | 1 + target/arm/debug_helper.c | 220 +++++++++++++++++++++++++++++++++++++-- target/arm/helper.c | 53 ++++------ target/arm/op_helper.c | 52 +++++---- target/arm/translate-a64.c | 34 +++--- target/arm/translate-m-nocp.c | 15 ++- target/arm/translate-mve.c | 3 +- target/arm/translate-vfp.c | 18 +++- target/arm/translate.c | 106 ++++++++++--------- tests/qtest/npcm7xx_sdhci-test.c | 2 +- 16 files changed, 390 insertions(+), 362 deletions(-) From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) -DEF_HELPER_2(exception_internal, void, env, i32) -DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32) -DEF_HELPER_2(exception_bkpt_insn, void, env, i32) +DEF_HELPER_2(exception_internal, noreturn, env, i32) +DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32) +DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) DEF_HELPER_2(wfi, void, env, i32) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> With ARMv8, this field is always RES0. With ARMv7, targeting EL2 and TA=0, it is always 0xA. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/syndrome.h | 7 ++++--- target/arm/translate-a64.c | 3 ++- target/arm/translate-vfp.c | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/syndrome.h +++ b/target/arm/syndrome.h @@ -XXX,XX +XXX,XX @@ static inline uint32_t syn_cp15_rrt_trap(int cv, int cond, int opc1, int crm, | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; } -static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit) +static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit, + int coproc) { - /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 coproc == 0xa */ + /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 */ return (EC_ADVSIMDFPACCESSTRAP << ARM_EL_EC_SHIFT) | (is_16bit ? 0 : ARM_EL_IL) - | (cv << 24) | (cond << 20) | 0xa; + | (cv << 24) | (cond << 20) | coproc; } static inline uint32_t syn_simd_access_trap(int cv, int cond, bool is_16bit) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static bool fp_access_check(DisasContext *s) s->fp_access_checked = true; gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); + syn_fp_access_trap(1, 0xe, false, 0), + s->fp_excp_el); return false; } s->fp_access_checked = true; diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static void gen_update_fp_context(DisasContext *s) static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) { if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); + /* + * The full syndrome is only used for HSR when HCPTR traps: + * For v8, when TA==0, coproc is RES0. + * For v7, any use of a Floating-point instruction or access + * to a Floating-point Extension register that is trapped to + * Hyp mode because of a trap configured in the HCPTR sets + * this field to 0xA. + */ + int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa; + uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc); + + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); return false; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the function to op_helper.c, near raise_exception. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 16 +--------------- target/arm/op_helper.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx); int aa64_va_parameter_tbid(uint64_t tcr, ARMMMUIdx mmu_idx); -static inline int exception_target_el(CPUARMState *env) -{ - int target_el = MAX(1, arm_current_el(env)); - - /* - * No such thing as secure EL1 if EL3 is aarch32, - * so update the target EL to EL3 in this case. - */ - if (arm_is_secure(env) && !arm_el_is_aa64(env, 3) && target_el == 1) { - target_el = 3; - } - - return target_el; -} - /* Determine if allocation tags are available. */ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el, uint64_t sctlr) @@ -XXX,XX +XXX,XX @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu); bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); +int exception_target_el(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ #define SIGNBIT (uint32_t)0x80000000 #define SIGNBIT64 ((uint64_t)1 << 63) +int exception_target_el(CPUARMState *env) +{ + int target_el = MAX(1, arm_current_el(env)); + + /* + * No such thing as secure EL1 if EL3 is aarch32, + * so update the target EL to EL3 in this case. + */ + if (arm_is_secure(env) && !arm_el_is_aa64(env, 3) && target_el == 1) { + target_el = 3; + } + + return target_el; +} + void raise_exception(CPUARMState *env, uint32_t excp, uint32_t syndrome, uint32_t target_el) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the function to debug_helper.c, and the declaration to internals.h. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 10 ---------- target/arm/internals.h | 1 + target/arm/debug_helper.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool arm_generate_debug_exceptions(CPUARMState *env) } } -/* Is single-stepping active? (Note that the "is EL_D AArch64?" check - * implicitly means this always returns false in pre-v8 CPUs.) - */ -static inline bool arm_singlestep_active(CPUARMState *env) -{ - return extract32(env->cp15.mdscr_el1, 0, 1) - && arm_el_is_aa64(env, arm_debug_target_el(env)) - && arm_generate_debug_exceptions(env); -} - static inline bool arm_sctlr_b(CPUARMState *env) { return diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); int exception_target_el(CPUARMState *env); +bool arm_singlestep_active(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/exec-all.h" #include "exec/helper-proto.h" + +/* + * Is single-stepping active? (Note that the "is EL_D AArch64?" check + * implicitly means this always returns false in pre-v8 CPUs.) + */ +bool arm_singlestep_active(CPUARMState *env) +{ + return extract32(env->cp15.mdscr_el1, 0, 1) + && arm_el_is_aa64(env, arm_debug_target_el(env)) + && arm_generate_debug_exceptions(env); +} + /* Return true if the linked breakpoint entry lbn passes its checks */ static bool linked_bp_matches(ARMCPU *cpu, int lbn) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move arm_generate_debug_exceptions and its two subroutines, {aa32,aa64}_generate_debug_exceptions into debug_helper.c, and the one interface declaration to internals.h. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 91 ------------------------------------- target/arm/internals.h | 1 + target/arm/debug_helper.c | 94 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 91 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu) return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0; } -/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ -static inline bool aa64_generate_debug_exceptions(CPUARMState *env) -{ - int cur_el = arm_current_el(env); - int debug_el; - - if (cur_el == 3) { - return false; - } - - /* MDCR_EL3.SDD disables debug events from Secure state */ - if (arm_is_secure_below_el3(env) - && extract32(env->cp15.mdcr_el3, 16, 1)) { - return false; - } - - /* - * Same EL to same EL debug exceptions need MDSCR_KDE enabled - * while not masking the (D)ebug bit in DAIF. - */ - debug_el = arm_debug_target_el(env); - - if (cur_el == debug_el) { - return extract32(env->cp15.mdscr_el1, 13, 1) - && !(env->daif & PSTATE_D); - } - - /* Otherwise the debug target needs to be a higher EL */ - return debug_el > cur_el; -} - -static inline bool aa32_generate_debug_exceptions(CPUARMState *env) -{ - int el = arm_current_el(env); - - if (el == 0 && arm_el_is_aa64(env, 1)) { - return aa64_generate_debug_exceptions(env); - } - - if (arm_is_secure(env)) { - int spd; - - if (el == 0 && (env->cp15.sder & 1)) { - /* SDER.SUIDEN means debug exceptions from Secure EL0 - * are always enabled. Otherwise they are controlled by - * SDCR.SPD like those from other Secure ELs. - */ - return true; - } - - spd = extract32(env->cp15.mdcr_el3, 14, 2); - switch (spd) { - case 1: - /* SPD == 0b01 is reserved, but behaves as 0b00. */ - case 0: - /* For 0b00 we return true if external secure invasive debug - * is enabled. On real hardware this is controlled by external - * signals to the core. QEMU always permits debug, and behaves - * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high. - */ - return true; - case 2: - return false; - case 3: - return true; - } - } - - return el != 2; -} - -/* Return true if debugging exceptions are currently enabled. - * This corresponds to what in ARM ARM pseudocode would be - * if UsingAArch32() then - * return AArch32.GenerateDebugExceptions() - * else - * return AArch64.GenerateDebugExceptions() - * We choose to push the if() down into this function for clarity, - * since the pseudocode has it at all callsites except for the one in - * CheckSoftwareStep(), where it is elided because both branches would - * always return the same value. - */ -static inline bool arm_generate_debug_exceptions(CPUARMState *env) -{ - if (env->aarch64) { - return aa64_generate_debug_exceptions(env); - } else { - return aa32_generate_debug_exceptions(env); - } -} - static inline bool arm_sctlr_b(CPUARMState *env) { return diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); int exception_target_el(CPUARMState *env); bool arm_singlestep_active(CPUARMState *env); +bool arm_generate_debug_exceptions(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ +static bool aa64_generate_debug_exceptions(CPUARMState *env) +{ + int cur_el = arm_current_el(env); + int debug_el; + + if (cur_el == 3) { + return false; + } + + /* MDCR_EL3.SDD disables debug events from Secure state */ + if (arm_is_secure_below_el3(env) + && extract32(env->cp15.mdcr_el3, 16, 1)) { + return false; + } + + /* + * Same EL to same EL debug exceptions need MDSCR_KDE enabled + * while not masking the (D)ebug bit in DAIF. + */ + debug_el = arm_debug_target_el(env); + + if (cur_el == debug_el) { + return extract32(env->cp15.mdscr_el1, 13, 1) + && !(env->daif & PSTATE_D); + } + + /* Otherwise the debug target needs to be a higher EL */ + return debug_el > cur_el; +} + +static bool aa32_generate_debug_exceptions(CPUARMState *env) +{ + int el = arm_current_el(env); + + if (el == 0 && arm_el_is_aa64(env, 1)) { + return aa64_generate_debug_exceptions(env); + } + + if (arm_is_secure(env)) { + int spd; + + if (el == 0 && (env->cp15.sder & 1)) { + /* + * SDER.SUIDEN means debug exceptions from Secure EL0 + * are always enabled. Otherwise they are controlled by + * SDCR.SPD like those from other Secure ELs. + */ + return true; + } + + spd = extract32(env->cp15.mdcr_el3, 14, 2); + switch (spd) { + case 1: + /* SPD == 0b01 is reserved, but behaves as 0b00. */ + case 0: + /* + * For 0b00 we return true if external secure invasive debug + * is enabled. On real hardware this is controlled by external + * signals to the core. QEMU always permits debug, and behaves + * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high. + */ + return true; + case 2: + return false; + case 3: + return true; + } + } + + return el != 2; +} + +/* + * Return true if debugging exceptions are currently enabled. + * This corresponds to what in ARM ARM pseudocode would be + * if UsingAArch32() then + * return AArch32.GenerateDebugExceptions() + * else + * return AArch64.GenerateDebugExceptions() + * We choose to push the if() down into this function for clarity, + * since the pseudocode has it at all callsites except for the one in + * CheckSoftwareStep(), where it is elided because both branches would + * always return the same value. + */ +bool arm_generate_debug_exceptions(CPUARMState *env) +{ + if (env->aarch64) { + return aa64_generate_debug_exceptions(env); + } else { + return aa32_generate_debug_exceptions(env); + } +} + /* * Is single-stepping active? (Note that the "is EL_D AArch64?" check * implicitly means this always returns false in pre-v8 CPUs.) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Use the accessor rather than the raw structure member. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ static bool aa32_generate_debug_exceptions(CPUARMState *env) */ bool arm_generate_debug_exceptions(CPUARMState *env) { - if (env->aarch64) { + if (is_a64(env)) { return aa64_generate_debug_exceptions(env); } else { return aa32_generate_debug_exceptions(env); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 31 +++++++++++++++++++++++++++++++ target/arm/op_helper.c | 29 ----------------------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) } } +/* + * Raise an EXCP_BKPT with the specified syndrome register value, + * targeting the correct exception level for debug exceptions. + */ +void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* FSR will only be used if the debug target EL is AArch32. */ + env->exception.fsr = arm_debug_exception_fsr(env); + /* + * FAR is UNKNOWN: clear vaddress to avoid potentially exposing + * values to the guest that it shouldn't be able to see at its + * exception/security level. + */ + env->exception.vaddress = 0; + /* + * Other kinds of architectural debug exception are ignored if + * they target an exception level below the current one (in QEMU + * this is checked by arm_generate_debug_exceptions()). Breakpoint + * instructions are special because they always generate an exception + * to somewhere: if they can't go to the configured debug exception + * level they are taken to the current exception level. + */ + if (debug_el < cur_el) { + debug_el = cur_el; + } + raise_exception(env, EXCP_BKPT, syndrome, debug_el); +} + #if !defined(CONFIG_USER_ONLY) vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, raise_exception(env, excp, syndrome, target_el); } -/* Raise an EXCP_BKPT with the specified syndrome register value, - * targeting the correct exception level for debug exceptions. - */ -void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) -{ - int debug_el = arm_debug_target_el(env); - int cur_el = arm_current_el(env); - - /* FSR will only be used if the debug target EL is AArch32. */ - env->exception.fsr = arm_debug_exception_fsr(env); - /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing - * values to the guest that it shouldn't be able to see at its - * exception/security level. - */ - env->exception.vaddress = 0; - /* - * Other kinds of architectural debug exception are ignored if - * they target an exception level below the current one (in QEMU - * this is checked by arm_generate_debug_exceptions()). Breakpoint - * instructions are special because they always generate an exception - * to somewhere: if they can't go to the configured debug exception - * level they are taken to the current exception level. - */ - if (debug_el < cur_el) { - debug_el = cur_el; - } - raise_exception(env, EXCP_BKPT, syndrome, debug_el); -} - uint32_t HELPER(cpsr_read)(CPUARMState *env) { return cpsr_read(env) & ~CPSR_EXEC; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function now now only used in debug_helper.c, so there is no reason to have a declaration in a header. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 25 ------------------------- target/arm/debug_helper.c | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; } -/* Return the FSR value for a debug exception (watchpoint, hardware - * breakpoint or BKPT insn) targeting the specified exception level. - */ -static inline uint32_t arm_debug_exception_fsr(CPUARMState *env) -{ - ARMMMUFaultInfo fi = { .type = ARMFault_Debug }; - int target_el = arm_debug_target_el(env); - bool using_lpae = false; - - if (target_el == 2 || arm_el_is_aa64(env, target_el)) { - using_lpae = true; - } else { - if (arm_feature(env, ARM_FEATURE_LPAE) && - (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) { - using_lpae = true; - } - } - - if (using_lpae) { - return arm_fi_to_lfsc(&fi); - } else { - return arm_fi_to_sfsc(&fi); - } -} - /** * arm_num_brps: Return number of implemented breakpoints. * Note that the ID register BRPS field is "number of bps - 1", diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) return check_watchpoints(cpu); } +/* + * Return the FSR value for a debug exception (watchpoint, hardware + * breakpoint or BKPT insn) targeting the specified exception level. + */ +static uint32_t arm_debug_exception_fsr(CPUARMState *env) +{ + ARMMMUFaultInfo fi = { .type = ARMFault_Debug }; + int target_el = arm_debug_target_el(env); + bool using_lpae = false; + + if (target_el == 2 || arm_el_is_aa64(env, target_el)) { + using_lpae = true; + } else { + if (arm_feature(env, ARM_FEATURE_LPAE) && + (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) { + using_lpae = true; + } + } + + if (using_lpae) { + return arm_fi_to_lfsc(&fi); + } else { + return arm_fi_to_sfsc(&fi); + } +} + void arm_debug_excp_handler(CPUState *cs) { /* -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Rename to helper_exception_with_syndrome_el, to emphasize that the target el is a parameter. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 +- target/arm/translate.h | 6 +++--- target/arm/op_helper.c | 6 +++--- target/arm/translate.c | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) DEF_HELPER_2(exception_internal, noreturn, env, i32) -DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32) +DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_ss_advance(DisasContext *s) static inline void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) { - gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome), + tcg_constant_i32(target_el)); } /* Generate an architectural singlestep exception */ diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(yield)(CPUARMState *env) * those EXCP values which are special cases for QEMU to interrupt * execution and not to be used for exceptions which are passed to * the guest (those must all have syndrome information and thus should - * use exception_with_syndrome). + * use exception_with_syndrome*). */ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp) { @@ -XXX,XX +XXX,XX @@ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp) } /* Raise an exception with the specified syndrome register value */ -void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, - uint32_t syndrome, uint32_t target_el) +void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp, + uint32_t syndrome, uint32_t target_el) { raise_exception(env, excp, syndrome, target_el); } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_el(DisasContext *s, int excp, uint32_t syn, { gen_set_condexec(s); gen_set_pc_im(s, s->pc_curr); - gen_helper_exception_with_syndrome(cpu_env, - tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); + gen_helper_exception_with_syndrome_el(cpu_env, + tcg_constant_i32(excp), + tcg_constant_i32(syn), tcg_el); s->base.is_jmp = DISAS_NORETURN; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a function below gen_exception_insn that takes the target_el as a TCGv_i32, replacing gen_exception_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-11-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el) +static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, TCGv_i32 tcg_el) { if (s->aarch64) { gen_a64_set_pc_im(pc); @@ -XXX,XX +XXX,XX @@ void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, gen_set_condexec(s); gen_set_pc_im(s, pc); } - gen_exception(excp, syn, target_el); + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syn), tcg_el); s->base.is_jmp = DISAS_NORETURN; } +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el) +{ + gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); +} + static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { gen_set_condexec(s); @@ -XXX,XX +XXX,XX @@ void unallocated_encoding(DisasContext *s) default_exception_el(s)); } -static void gen_exception_el(DisasContext *s, int excp, uint32_t syn, - TCGv_i32 tcg_el) -{ - gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); - gen_helper_exception_with_syndrome_el(cpu_env, - tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); - s->base.is_jmp = DISAS_NORETURN; -} - /* Force a TB lookup after an instruction that changes the CPU state. */ void gen_lookup_tb(DisasContext *s) { @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, tcg_el = tcg_constant_i32(3); } - gen_exception_el(s, EXCP_UDEF, syn_uncategorized(), tcg_el); + gen_exception_insn_el_v(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), tcg_el); tcg_temp_free_i32(tcg_el); return false; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 4 ++-- target/arm/translate-a64.c | 36 ++++++++++++++++---------------- target/arm/translate-m-nocp.c | 16 +++++++------- target/arm/translate-mve.c | 4 ++-- target/arm/translate-vfp.c | 6 +++--- target/arm/translate.c | 39 ++++++++++++++++++----------------- 6 files changed, 53 insertions(+), 52 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ void arm_jump_cc(DisasCompare *cmp, TCGLabel *label); void arm_gen_test_cc(int cc, TCGLabel *label); MemOp pow2_align(unsigned i); void unallocated_encoding(DisasContext *s); -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el); +void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el); /* Return state of Alternate Half-precision flag, caller frees result */ static inline TCGv_i32 get_ahp_flag(void) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static bool fp_access_check(DisasContext *s) assert(!s->fp_access_checked); s->fp_access_checked = true; - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false, 0), - s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_fp_access_trap(1, 0xe, false, 0), + s->fp_excp_el); return false; } s->fp_access_checked = true; @@ -XXX,XX +XXX,XX @@ bool sve_access_check(DisasContext *s) assert(!s->sve_access_checked); s->sve_access_checked = true; - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_sve_access_trap(), s->sve_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_sve_access_trap(), s->sve_excp_el); return false; } s->sve_access_checked = true; @@ -XXX,XX +XXX,XX @@ static void gen_sysreg_undef(DisasContext *s, bool isread, } else { syndrome = syn_uncategorized(); } - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome, - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome, + default_exception_el(s)); } /* MRS - move from system register @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_SWI, - syn_aa64_svc(imm16), default_exception_el(s)); + gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI, + syn_aa64_svc(imm16), default_exception_el(s)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_HVC, - syn_aa64_hvc(imm16), 2); + gen_exception_insn_el(s, s->base.pc_next, EXCP_HVC, + syn_aa64_hvc(imm16), 2); break; case 3: /* SMC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa64_smc(imm16))); gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_SMC, - syn_aa64_smc(imm16), 3); + gen_exception_insn_el(s, s->base.pc_next, EXCP_SMC, + syn_aa64_smc(imm16), 3); break; default: unallocated_encoding(s); @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_btitrap(s->btype), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_btitrap(s->btype), + default_exception_el(s)); return; } } else { diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a) tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel); if (s->fp_excp_el != 0) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, if (!vfp_access_check_m(s, true)) { /* * This was only a conditional exception, so override - * gen_exception_insn()'s default to DISAS_NORETURN + * gen_exception_insn_el()'s default to DISAS_NORETURN */ s->base.is_jmp = DISAS_NEXT; break; @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno, if (!vfp_access_check_m(s, true)) { /* * This was only a conditional exception, so override - * gen_exception_insn()'s default to DISAS_NORETURN + * gen_exception_insn_el()'s default to DISAS_NORETURN */ s->base.is_jmp = DISAS_NEXT; break; @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a) } if (a->cp != 10) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), default_exception_el(s)); return true; } if (s->fp_excp_el != 0) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-mve.c +++ b/target/arm/translate-mve.c @@ -XXX,XX +XXX,XX @@ bool mve_eci_check(DisasContext *s) return true; default: /* Reserved value: INVSTATE UsageFault */ - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); return false; } } diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa; uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc); - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); return false; } @@ -XXX,XX +XXX,XX @@ bool vfp_access_check_m(DisasContext *s, bool skip_context_update) * the encoding space handled by the patterns in m-nocp.decode, * and for them we may need to raise NOCP here. */ - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, s->base.is_jmp = DISAS_NORETURN; } -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el) +void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el) { gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); } @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); } /* Force a TB lookup after an instruction that changes the CPU state. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_uncategorized(), exc_target); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), exc_target); return false; } @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, * For the UNPREDICTABLE cases we choose to UNDEF. */ if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), 3); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), 3); return; } @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) * Do the check-and-raise-exception by hand. */ if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tmp = load_cpu_field(v7m.ltpsize); tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); tcg_temp_free_i32(tmp); - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); gen_set_label(skipexc); } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(dc)); + gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(dc)); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) */ tcg_remove_ops_after(dc->insn_eci_rewind); dc->condjmp = 0; - gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(dc)); + gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(dc)); } arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a new wrapper function that passes the default exception target to gen_exception_insn_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 1 + target/arm/translate-a64.c | 15 ++++++--------- target/arm/translate-m-nocp.c | 3 +-- target/arm/translate-mve.c | 3 +-- target/arm/translate.c | 29 +++++++++++++---------------- 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ MemOp pow2_align(unsigned i); void unallocated_encoding(DisasContext *s); void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, uint32_t syn, uint32_t target_el); +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn); /* Return state of Alternate Half-precision flag, caller frees result */ static inline TCGv_i32 get_ahp_flag(void) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void gen_sysreg_undef(DisasContext *s, bool isread, } else { syndrome = syn_uncategorized(); } - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome, - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome); } /* MRS - move from system register @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI, - syn_aa64_svc(imm16), default_exception_el(s)); + gen_exception_insn(s, s->base.pc_next, EXCP_SWI, + syn_aa64_svc(imm16)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_btitrap(s->btype), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_btitrap(s->btype)); return; } } else { diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a) } if (a->cp != 10) { - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized()); return true; } diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-mve.c +++ b/target/arm/translate-mve.c @@ -XXX,XX +XXX,XX @@ bool mve_eci_check(DisasContext *s) return true; default: /* Reserved value: INVSTATE UsageFault */ - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); return false; } } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); } +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) +{ + gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s)); +} + static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { gen_set_condexec(s); @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); } /* Force a TB lookup after an instruction that changes the CPU state. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, * an exception and return false. Otherwise it will return true, * and set *tgtmode and *regno appropriately. */ - int exc_target = default_exception_el(s); - /* These instructions are present only in ARMv8, or in ARMv7 with the * Virtualization Extensions. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_uncategorized(), exc_target); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); return false; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tmp = load_cpu_field(v7m.ltpsize); tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); tcg_temp_free_i32(tmp); - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); gen_set_label(skipexc); } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(dc)); + gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) */ tcg_remove_ops_after(dc->insn_eci_rewind); dc->condjmp = 0; - gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(dc)); + gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, + syn_uncategorized()); } arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the computation from gen_swstep_exception into a helper. This fixes a bug when: - MDSCR_EL1.KDE == 1 to enable debug exceptions within EL_D itself - we singlestep an ERET from EL_D to some lower EL Previously we were computing 'same el' based on the EL which executed the ERET instruction, whereas it ought to be computed based on the EL to which ERET returned. This happens naturally with the new helper, which runs after EL has been changed. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-14-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/translate.h | 12 +++--------- target/arm/debug_helper.c | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, DEF_HELPER_2(exception_internal, noreturn, env, i32) DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) +DEF_HELPER_2(exception_swstep, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) DEF_HELPER_2(wfi, void, env, i32) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_exception(int excp, uint32_t syndrome, /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { - bool same_el = (s->debug_target_el == s->current_el); - - /* - * If singlestep is targeting a lower EL than the current one, - * then s->ss_active must be false and we can never get here. - */ - assert(s->debug_target_el >= s->current_el); - - gen_exception(EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el); + /* Fill in the same_el field of the syndrome in the helper. */ + uint32_t syn = syn_swstep(false, isv, ex); + gen_helper_exception_swstep(cpu_env, tcg_constant_i32(syn)); } /* diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) raise_exception(env, EXCP_BKPT, syndrome, debug_el); } +void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* + * If singlestep is targeting a lower EL than the current one, then + * DisasContext.ss_active must be false and we can never get here. + */ + assert(debug_el >= cur_el); + if (debug_el == cur_el) { + syndrome |= 1 << ARM_EL_EC_SHIFT; + } + raise_exception(env, EXCP_UDEF, syndrome, debug_el); +} + #if !defined(CONFIG_USER_ONLY) vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We no longer need this value during translation, as it is now handled within the helpers. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-15-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 6 ++---- target/arm/translate.h | 2 -- target/arm/helper.c | 12 ++---------- target/arm/translate-a64.c | 1 - target/arm/translate.c | 1 - 5 files changed, 4 insertions(+), 18 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ FIELD(TBFLAG_ANY, BE_DATA, 3, 1) FIELD(TBFLAG_ANY, MMUIDX, 4, 4) /* Target EL if we take a floating-point-disabled exception */ FIELD(TBFLAG_ANY, FPEXC_EL, 8, 2) -/* For A-profile only, target EL for debug exceptions. */ -FIELD(TBFLAG_ANY, DEBUG_TARGET_EL, 10, 2) /* Memory operations require alignment: SCTLR_ELx.A or CCR.UNALIGN_TRP */ -FIELD(TBFLAG_ANY, ALIGN_MEM, 12, 1) -FIELD(TBFLAG_ANY, PSTATE__IL, 13, 1) +FIELD(TBFLAG_ANY, ALIGN_MEM, 10, 1) +FIELD(TBFLAG_ANY, PSTATE__IL, 11, 1) /* * Bit usage when in AArch32 state, both A- and M-profile. diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { */ uint32_t svc_imm; int current_el; - /* Debug target exception level for single-step exceptions */ - int debug_target_el; GHashTable *cp_regs; uint64_t features; /* CPU features bits */ bool aarch64; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static CPUARMTBFlags rebuild_hflags_m32(CPUARMState *env, int fp_el, return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags); } -static CPUARMTBFlags rebuild_hflags_aprofile(CPUARMState *env) -{ - CPUARMTBFlags flags = {}; - - DP_TBFLAG_ANY(flags, DEBUG_TARGET_EL, arm_debug_target_el(env)); - return flags; -} - static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el, ARMMMUIdx mmu_idx) { - CPUARMTBFlags flags = rebuild_hflags_aprofile(env); + CPUARMTBFlags flags = {}; int el = arm_current_el(env); if (arm_sctlr(env, el) & SCTLR_A) { @@ -XXX,XX +XXX,XX @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el, static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, ARMMMUIdx mmu_idx) { - CPUARMTBFlags flags = rebuild_hflags_aprofile(env); + CPUARMTBFlags flags = {}; ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx); uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; uint64_t sctlr; diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->ss_active = EX_TBFLAG_ANY(tb_flags, SS_ACTIVE); dc->pstate_ss = EX_TBFLAG_ANY(tb_flags, PSTATE__SS); dc->is_ldex = false; - dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, DEBUG_TARGET_EL); /* Bound the number of insns to execute to those left on the page. */ bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->v7m_lspact = EX_TBFLAG_M32(tb_flags, LSPACT); dc->mve_no_pred = EX_TBFLAG_M32(tb_flags, MVE_NO_PRED); } else { - dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, DEBUG_TARGET_EL); dc->sctlr_b = EX_TBFLAG_A32(tb_flags, SCTLR__B); dc->hstr_active = EX_TBFLAG_A32(tb_flags, HSTR_ACTIVE); dc->ns = EX_TBFLAG_A32(tb_flags, NS); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is not required by any other translation file. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-16-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 8 -------- target/arm/translate.c | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_ss_advance(DisasContext *s) } } -static inline void gen_exception(int excp, uint32_t syndrome, - uint32_t target_el) -{ - gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); -} - /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } +static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) +{ + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome), + tcg_constant_i32(target_el)); +} + static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, uint32_t syn, TCGv_i32 tcg_el) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-17-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) +static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) { gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), tcg_constant_i32(syndrome), @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), + default_exception_el(dc)); break; case DISAS_HVC: gen_ss_advance(dc); - gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); + gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); break; case DISAS_SMC: gen_ss_advance(dc); - gen_exception(EXCP_SMC, syn_aa32_smc(), 3); + gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3); break; case DISAS_NEXT: case DISAS_TOO_MANY: @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), + default_exception_el(dc)); break; case DISAS_HVC: - gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); + gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); break; case DISAS_SMC: - gen_exception(EXCP_SMC, syn_aa32_smc(), 3); + gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3); break; } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a new wrapper function that passes the default exception target to gen_exception_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-18-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) tcg_constant_i32(target_el)); } +static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) +{ + gen_exception_el(excp, syndrome, default_exception_el(s)); +} + static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, uint32_t syn, TCGv_i32 tcg_el) { @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_ss_advance(dc); @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Split out a common helper function for gen_exception_el and gen_exception_insn_el_v. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-19-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) +static void gen_exception_el_v(int excp, uint32_t syndrome, TCGv_i32 tcg_el) { gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); + tcg_constant_i32(syndrome), tcg_el); +} + +static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) +{ + gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el)); } static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) @@ -XXX,XX +XXX,XX @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, gen_set_condexec(s); gen_set_pc_im(s, pc); } - gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); + gen_exception_el_v(excp, syn, tcg_el); s->base.is_jmp = DISAS_NORETURN; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> With the helper we can use exception_target_el at runtime, instead of default_exception_el at translate time. While we're at it, remove the DisasContext parameter from gen_exception, as it is no longer used. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-20-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/op_helper.c | 10 ++++++++++ target/arm/translate.c | 18 +++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) DEF_HELPER_2(exception_internal, noreturn, env, i32) +DEF_HELPER_3(exception_with_syndrome, noreturn, env, i32, i32) DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_swstep, noreturn, env, i32) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp, raise_exception(env, excp, syndrome, target_el); } +/* + * Raise an exception with the specified syndrome register value + * to the default target el. + */ +void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, + uint32_t syndrome) +{ + raise_exception(env, excp, syndrome, exception_target_el(env)); +} + uint32_t HELPER(cpsr_read)(CPUARMState *env) { return cpsr_read(env) & ~CPSR_EXEC; diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el)); } -static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) +static void gen_exception(int excp, uint32_t syndrome) { - gen_exception_el(excp, syndrome, default_exception_el(s)); + gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome)); } static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, @@ -XXX,XX +XXX,XX @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) { - gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s)); + if (s->aarch64) { + gen_a64_set_pc_im(pc); + } else { + gen_set_condexec(s); + gen_set_pc_im(s, pc); + } + gen_exception(excp, syn); + s->base.is_jmp = DISAS_NORETURN; } static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); + gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_ss_advance(dc); @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); + gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is no longer used. At the same time, remove DisasContext.secure_routed_to_el3, as it in turn becomes unused. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-21-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 16 ---------------- target/arm/translate-a64.c | 5 ----- target/arm/translate.c | 5 ----- 3 files changed, 26 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { int fp_excp_el; /* FP exception EL or 0 if enabled */ int sve_excp_el; /* SVE exception EL or 0 if enabled */ int vl; /* current vector length in bytes */ - /* Flag indicating that exceptions from secure mode are routed to EL3. */ - bool secure_routed_to_el3; bool vfp_enabled; /* FP enabled via FPSCR.EN */ int vec_len; int vec_stride; @@ -XXX,XX +XXX,XX @@ static inline int get_mem_index(DisasContext *s) return arm_to_core_mmu_idx(s->mmu_idx); } -/* Function used to determine the target exception EL when otherwise not known - * or default. - */ -static inline int default_exception_el(DisasContext *s) -{ - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. Otherwise, - * exceptions can only be routed to ELs above 1, so we target the higher of - * 1 or the current EL. - */ - return (s->mmu_idx == ARMMMUIdx_SE10_0 && s->secure_routed_to_el3) - ? 3 : MAX(1, s->current_el); -} - static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn) { /* We don't need to save all of the syndrome so we mask and shift diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->condjmp = 0; dc->aarch64 = true; - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. - */ - dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3); dc->thumb = false; dc->sctlr_b = 0; dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE; diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->condjmp = 0; dc->aarch64 = false; - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. - */ - dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3); dc->thumb = EX_TBFLAG_AM32(tb_flags, THUMB); dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE; condexec = EX_TBFLAG_AM32(tb_flags, CONDEXEC); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Handle the debug vs current el exception test in one place. Leave EXCP_BKPT alone, since that treats debug < current differently. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-22-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 44 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* + * Raise an exception to the debug target el. + * Modify syndrome to indicate when origin and target EL are the same. + */ +G_NORETURN static void +raise_exception_debug(CPUARMState *env, uint32_t excp, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* + * If singlestep is targeting a lower EL than the current one, then + * DisasContext.ss_active must be false and we can never get here. + * Similarly for watchpoint and breakpoint matches. + */ + assert(debug_el >= cur_el); + syndrome |= (debug_el == cur_el) << ARM_EL_EC_SHIFT; + raise_exception(env, excp, syndrome, debug_el); +} + /* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ static bool aa64_generate_debug_exceptions(CPUARMState *env) { @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) if (wp_hit) { if (wp_hit->flags & BP_CPU) { bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0; - bool same_el = arm_debug_target_el(env) == arm_current_el(env); cs->watchpoint_hit = NULL; env->exception.fsr = arm_debug_exception_fsr(env); env->exception.vaddress = wp_hit->hitaddr; - raise_exception(env, EXCP_DATA_ABORT, - syn_watchpoint(same_el, 0, wnr), - arm_debug_target_el(env)); + raise_exception_debug(env, EXCP_DATA_ABORT, + syn_watchpoint(0, 0, wnr)); } } else { uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; - bool same_el = (arm_debug_target_el(env) == arm_current_el(env)); /* * (1) GDB breakpoints should be handled first. @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) * exception/security level. */ env->exception.vaddress = 0; - raise_exception(env, EXCP_PREFETCH_ABORT, - syn_breakpoint(same_el), - arm_debug_target_el(env)); + raise_exception_debug(env, EXCP_PREFETCH_ABORT, syn_breakpoint(0)); } } @@ -XXX,XX +XXX,XX @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome) { - int debug_el = arm_debug_target_el(env); - int cur_el = arm_current_el(env); - - /* - * If singlestep is targeting a lower EL than the current one, then - * DisasContext.ss_active must be false and we can never get here. - */ - assert(debug_el >= cur_el); - if (debug_el == cur_el) { - syndrome |= 1 << ARM_EL_EC_SHIFT; - } - raise_exception(env, EXCP_UDEF, syndrome, debug_el); + raise_exception_debug(env, EXCP_UDEF, syndrome); } #if !defined(CONFIG_USER_ONLY) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is no longer used outside debug_helper.c. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-23-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 21 --------------------- target/arm/debug_helper.c | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef enum ARMASIdx { ARMASIdx_TagS = 3, } ARMASIdx; -/* Return the Exception Level targeted by debug exceptions. */ -static inline int arm_debug_target_el(CPUARMState *env) -{ - bool secure = arm_is_secure(env); - bool route_to_el2 = false; - - if (arm_is_el2_enabled(env)) { - route_to_el2 = env->cp15.hcr_el2 & HCR_TGE || - env->cp15.mdcr_el2 & MDCR_TDE; - } - - if (route_to_el2) { - return 2; - } else if (arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3) && secure) { - return 3; - } else { - return 1; - } -} - static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu) { /* If all the CLIDR.Ctypem bits are 0 there are no caches, and diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* Return the Exception Level targeted by debug exceptions. */ +static int arm_debug_target_el(CPUARMState *env) +{ + bool secure = arm_is_secure(env); + bool route_to_el2 = false; + + if (arm_is_el2_enabled(env)) { + route_to_el2 = env->cp15.hcr_el2 & HCR_TGE || + env->cp15.mdcr_el2 & MDCR_TDE; + } + + if (route_to_el2) { + return 2; + } else if (arm_feature(env, ARM_FEATURE_EL3) && + !arm_el_is_aa64(env, 3) && secure) { + return 3; + } else { + return 1; + } +} + /* * Raise an exception to the debug target el. * Modify syndrome to indicate when origin and target EL are the same. -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We were using arm_is_secure and is_a64, which are tests against the current EL, as opposed to arm_el_is_aa64 and arm_is_secure_below_el3, which can be applied to a different EL than current. Consolidate the two tests. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-24-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ int fp_exception_el(CPUARMState *env, int cur_el) int fpen = FIELD_EX64(env->cp15.cpacr_el1, CPACR_EL1, FPEN); switch (fpen) { + case 1: + if (cur_el != 0) { + break; + } + /* fall through */ case 0: case 2: - if (cur_el == 0 || cur_el == 1) { - /* Trap to PL1, which might be EL1 or EL3 */ - if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) { - return 3; - } - return 1; - } - if (cur_el == 3 && !is_a64(env)) { - /* Secure PL1 running at EL3 */ + /* Trap from Secure PL0 or PL1 to Secure PL1. */ + if (!arm_el_is_aa64(env, 3) + && (cur_el == 3 || arm_is_secure_below_el3(env))) { return 3; } - break; - case 1: - if (cur_el == 0) { + if (cur_el <= 1) { return 1; } break; - case 3: - break; } } -- 2.25.1 From: Hao Wu <wuhaotsh@google.com> Creating 1GB image for a simple qtest is unnecessary and could lead to failures. We reduce the image size to 1MB to reduce the test overhead. Signed-off-by: Hao Wu <wuhaotsh@google.com> Message-id: 20220609214125.4192212-1-wuhaotsh@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/npcm7xx_sdhci-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/npcm7xx_sdhci-test.c +++ b/tests/qtest/npcm7xx_sdhci-test.c @@ -XXX,XX +XXX,XX @@ #define NPCM7XX_REG_SIZE 0x100 #define NPCM7XX_MMC_BA 0xF0842000 #define NPCM7XX_BLK_SIZE 512 -#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30) +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 20) char *sd_path; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Because reset always initializes the AA64 version, SCR_EL3, test the mode of EL3 instead of the type of the cpreg. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609214657.1217913-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) uint32_t valid_mask = 0x3fff; ARMCPU *cpu = env_archcpu(env); - if (ri->state == ARM_CP_STATE_AA64) { - if (arm_feature(env, ARM_FEATURE_AARCH64) && - !cpu_isar_feature(aa64_aa32_el1, cpu)) { - value |= SCR_FW | SCR_AW; /* these two bits are RES1. */ - } - valid_mask &= ~SCR_NET; + /* + * Because SCR_EL3 is the "real" cpreg and SCR is the alias, reset always + * passes the reginfo for SCR_EL3, which has type ARM_CP_STATE_AA64. + * Instead, choose the format based on the mode of EL3. + */ + if (arm_el_is_aa64(env, 3)) { + value |= SCR_FW | SCR_AW; /* RES1 */ + valid_mask &= ~SCR_NET; /* RES0 */ if (cpu_isar_feature(aa64_ras, cpu)) { valid_mask |= SCR_TERR; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Since DDI0487F.a, the RW bit is RAO/WI. When specifically targeting such a cpu, e.g. cortex-a76, it is legitimate to ignore the bit within the secure monitor. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1062 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609214657.1217913-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 5 +++++ target/arm/helper.c | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL1) >= 2; } +static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2; +} + static inline bool isar_feature_aa64_ras(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, RAS) != 0; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) value |= SCR_FW | SCR_AW; /* RES1 */ valid_mask &= ~SCR_NET; /* RES0 */ + if (!cpu_isar_feature(aa64_aa32_el1, cpu) && + !cpu_isar_feature(aa64_aa32_el2, cpu)) { + value |= SCR_RW; /* RAO/WI */ + } if (cpu_isar_feature(aa64_ras, cpu)) { valid_mask |= SCR_TERR; } -- 2.25.1 In two places in gdbstub.c we look at gdbserver_state.init to decide whether we're going to do a semihosting syscall via the gdb remote protocol: * when setting up, if the user didn't explicitly select either native semihosting or gdb semihosting, we autoselect, with the intended behaviour "use gdb if gdb is connected" * when the semihosting layer attempts to do a syscall via gdb, we silently ignore it if the gdbstub wasn't actually set up However, if the user's commandline sets up the gdbstub but tells QEMU to start rather than waiting for a GDB to connect (eg using '-s' but not '-S'), then we will have gdbserver_state.init true but no actual connection; an attempt to use gdb syscalls will then crash because we try to use gdbserver_state.c_cpu when it hasn't been set up: #0 0x00007ffff6803ba8 in qemu_cpu_kick (cpu=0x0) at ../../softmmu/cpus.c:457 #1 0x00007ffff6c03913 in gdb_do_syscallv (cb=0x7ffff6c19944 <common_semi_cb>, fmt=0x7ffff7573b7e "", va=0x7ffff56294c0) at ../../gdbstub.c:2946 #2 0x00007ffff6c19c3a in common_semi_gdb_syscall (cs=0x7ffff83fe060, cb=0x7ffff6c19944 <common_semi_cb>, fmt=0x7ffff7573b75 "isatty,%x") at ../../semihosting/arm-compat-semi.c:494 #3 0x00007ffff6c1a064 in gdb_isattyfn (cs=0x7ffff83fe060, gf=0x7ffff86a3690) at ../../semihosting/arm-compat-semi.c:636 #4 0x00007ffff6c1b20f in do_common_semihosting (cs=0x7ffff83fe060) at ../../semihosting/arm-compat-semi.c:967 #5 0x00007ffff693a037 in handle_semihosting (cs=0x7ffff83fe060) at ../../target/arm/helper.c:10316 You can probably also get into this state via some odd corner cases involving connecting a GDB and then telling it to detach from all the vCPUs. Abstract out the test into a new gdb_attached() function which returns true only if there's actually a GDB connected to the debug stub and attached to at least one vCPU. Reported-by: Liviu Ionescu <ilg@livius.net> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20220526190053.521505-2-peter.maydell@linaro.org --- gdbstub.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -XXX,XX +XXX,XX @@ static int get_char(void) } #endif +/* + * Return true if there is a GDB currently connected to the stub + * and attached to a CPU + */ +static bool gdb_attached(void) +{ + return gdbserver_state.init && gdbserver_state.c_cpu; +} + static enum { GDB_SYS_UNKNOWN, GDB_SYS_ENABLED, @@ -XXX,XX +XXX,XX @@ int use_gdb_syscalls(void) /* -semihosting-config target=auto */ /* On the first call check if gdb is connected and remember. */ if (gdb_syscall_mode == GDB_SYS_UNKNOWN) { - gdb_syscall_mode = gdbserver_state.init ? - GDB_SYS_ENABLED : GDB_SYS_DISABLED; + gdb_syscall_mode = gdb_attached() ? GDB_SYS_ENABLED : GDB_SYS_DISABLED; } return gdb_syscall_mode == GDB_SYS_ENABLED; } @@ -XXX,XX +XXX,XX @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va) target_ulong addr; uint64_t i64; - if (!gdbserver_state.init) { + if (!gdb_attached()) { return; } -- 2.25.1 Currently we mishandle the --semihosting-config option if the user specifies it on the command line more than once. For example with: --semihosting-config target=gdb --semihosting-config arg=foo,arg=bar the function qemu_semihosting_config_options() is called twice, once for each argument. But that function expects to be called only once, and it always unconditionally sets the semihosting.enabled, semihost_chardev and semihosting.target variables. This means that if any of those options were set anywhere except the last --semihosting-config option on the command line, those settings are ignored. In the example above, 'target=gdb' in the first option is overridden by an implied default 'target=auto' in the second. The QemuOptsList machinery has a flag for handling this kind of "option group is setting global state": by setting .merge_lists = true; we make the machinery merge all the --semihosting-config arguments the user passes into a single set of options and call our qemu_semihosting_config_options() just once. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20220526190053.521505-3-peter.maydell@linaro.org --- semihosting/config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/semihosting/config.c b/semihosting/config.c index XXXXXXX..XXXXXXX 100644 --- a/semihosting/config.c +++ b/semihosting/config.c @@ -XXX,XX +XXX,XX @@ QemuOptsList qemu_semihosting_config_opts = { .name = "semihosting-config", + .merge_lists = true, .implied_opt_name = "enable", .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head), .desc = { -- 2.25.1