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 I might squeeze in another pullreq before softfreeze, but the queue was already big enough that I wanted to send this lot out now. -- PMM The following changes since commit 4abf70a661a5df3886ac9d7c19c3617fa92b922a: Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' into staging (2020-07-03 15:34:45 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200703 for you to fetch changes up to 0f10bf84a9d489259a5b11c6aa1b05c1175b76ea: Deprecate TileGX port (2020-07-03 16:59:46 +0100) ---------------------------------------------------------------- target-arm queue: * i.MX6UL EVK board: put PHYs in the correct places * hw/arm/virt: Let the virtio-iommu bypass MSIs * target/arm: kvm: Handle DABT with no valid ISS * hw/arm/virt-acpi-build: Only expose flash on older machine types * target/arm: Fix temp double-free in sve ldr/str * hw/display/bcm2835_fb.c: Initialize all fields of struct * hw/arm/spitz: Code cleanup to fix Coverity-detected memory leak * Deprecate TileGX port ---------------------------------------------------------------- Andrew Jones (4): tests/acpi: remove stale allowed tables tests/acpi: virt: allow DSDT acpi table changes hw/arm/virt-acpi-build: Only expose flash on older machine types tests/acpi: virt: update golden masters for DSDT Beata Michalska (2): target/arm: kvm: Handle DABT with no valid ISS target/arm: kvm: Handle misconfigured dabt injection Eric Auger (5): qdev: Introduce DEFINE_PROP_RESERVED_REGION virtio-iommu: Implement RESV_MEM probe request virtio-iommu: Handle reserved regions in the translation process virtio-iommu-pci: Add array of Interval properties hw/arm/virt: Let the virtio-iommu bypass MSIs Jean-Christophe Dubois (3): Add a phy-num property to the i.MX FEC emulator Add the ability to select a different PHY for each i.MX6UL FEC interface Select MDIO device 2 and 1 as PHY devices for i.MX6UL EVK board. Peter Maydell (19): hw/display/bcm2835_fb.c: Initialize all fields of struct hw/arm/spitz: Detabify hw/arm/spitz: Create SpitzMachineClass abstract base class hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals hw/misc/max111x: provide QOM properties for setting initial values hw/misc/max111x: Don't use vmstate_register() ssi: Add ssi_realize_and_unref() hw/arm/spitz: Use max111x properties to set initial values hw/misc/max111x: Use GPIO lines rather than max111x_set_input() hw/misc/max111x: Create header file for documentation, TYPE_ macros hw/arm/spitz: Encapsulate misc GPIO handling in a device hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Replace uses of FROM_SSI_SLAVE() macro with QOM casts Deprecate TileGX port Richard Henderson (1): target/arm: Fix temp double-free in sve ldr/str docs/system/deprecated.rst | 11 + include/exec/memory.h | 6 + include/hw/arm/fsl-imx6ul.h | 2 + include/hw/arm/pxa.h | 1 - include/hw/arm/sharpsl.h | 3 - include/hw/arm/virt.h | 8 + include/hw/misc/max111x.h | 56 +++ include/hw/net/imx_fec.h | 1 + include/hw/qdev-properties.h | 3 + include/hw/ssi/ssi.h | 31 +- include/hw/virtio/virtio-iommu.h | 2 + include/qemu/typedefs.h | 1 + target/arm/cpu.h | 2 + target/arm/kvm_arm.h | 10 + target/arm/translate-a64.h | 1 + tests/qtest/bios-tables-test-allowed-diff.h | 18 - hw/arm/fsl-imx6ul.c | 10 + hw/arm/mcimx6ul-evk.c | 2 + hw/arm/pxa2xx_pic.c | 9 +- hw/arm/spitz.c | 507 ++++++++++++++++------------ hw/arm/virt-acpi-build.c | 5 +- hw/arm/virt.c | 33 ++ hw/arm/z2.c | 11 +- hw/core/qdev-properties.c | 89 +++++ hw/display/ads7846.c | 9 +- hw/display/bcm2835_fb.c | 4 + hw/display/ssd0323.c | 10 +- hw/gpio/zaurus.c | 12 +- hw/misc/max111x.c | 86 +++-- hw/net/imx_fec.c | 24 +- hw/sd/ssi-sd.c | 4 +- hw/ssi/ssi.c | 7 +- hw/virtio/virtio-iommu-pci.c | 11 + hw/virtio/virtio-iommu.c | 114 ++++++- target/arm/kvm.c | 80 +++++ target/arm/kvm32.c | 34 ++ target/arm/kvm64.c | 49 +++ target/arm/translate-a64.c | 6 + target/arm/translate-sve.c | 8 +- MAINTAINERS | 1 + hw/net/trace-events | 4 +- hw/virtio/trace-events | 1 + tests/data/acpi/virt/DSDT | Bin 5307 -> 5205 bytes tests/data/acpi/virt/DSDT.memhp | Bin 6668 -> 6566 bytes tests/data/acpi/virt/DSDT.numamem | Bin 5307 -> 5205 bytes 45 files changed, 974 insertions(+), 312 deletions(-) create mode 100644 include/hw/misc/max111x.h From: Jean-Christophe Dubois <jcd@tribudubois.net> We need a solution to use an Ethernet PHY that is not the first device on the MDIO bus (device 0 on MDIO bus). As an example with the i.MX6UL the NXP SOC has 2 Ethernet devices but only one MDIO bus on which the 2 related PHY are connected but at unique addresses. Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> Message-id: a1a5c0e139d1c763194b8020573dcb6025daeefa.1593296112.git.jcd@tribudubois.net Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/net/imx_fec.h | 1 + hw/net/imx_fec.c | 24 +++++++++++++++++------- hw/net/trace-events | 4 ++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/net/imx_fec.h +++ b/include/hw/net/imx_fec.h @@ -XXX,XX +XXX,XX @@ typedef struct IMXFECState { uint32_t phy_advertise; uint32_t phy_int; uint32_t phy_int_mask; + uint32_t phy_num; bool is_fec; diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -XXX,XX +XXX,XX @@ static void imx_phy_reset(IMXFECState *s) static uint32_t imx_phy_read(IMXFECState *s, int reg) { uint32_t val; + uint32_t phy = reg / 32; - if (reg > 31) { - /* we only advertise one phy */ + if (phy != s->phy_num) { + qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n", + TYPE_IMX_FEC, __func__, phy); return 0; } + reg %= 32; + switch (reg) { case 0: /* Basic Control */ val = s->phy_control; @@ -XXX,XX +XXX,XX @@ static uint32_t imx_phy_read(IMXFECState *s, int reg) break; } - trace_imx_phy_read(val, reg); + trace_imx_phy_read(val, phy, reg); return val; } static void imx_phy_write(IMXFECState *s, int reg, uint32_t val) { - trace_imx_phy_write(val, reg); + uint32_t phy = reg / 32; - if (reg > 31) { - /* we only advertise one phy */ + if (phy != s->phy_num) { + qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad phy num %u\n", + TYPE_IMX_FEC, __func__, phy); return; } + reg %= 32; + + trace_imx_phy_write(val, phy, reg); + switch (reg) { case 0: /* Basic Control */ if (val & 0x8000) { @@ -XXX,XX +XXX,XX @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value, extract32(value, 18, 10))); } else { - /* This a write operation */ + /* This is a write operation */ imx_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16)); } /* raise the interrupt as the PHY operation is done */ @@ -XXX,XX +XXX,XX @@ static void imx_eth_realize(DeviceState *dev, Error **errp) static Property imx_eth_properties[] = { DEFINE_NIC_PROPERTIES(IMXFECState, conf), DEFINE_PROP_UINT32("tx-ring-num", IMXFECState, tx_ring_num, 1), + DEFINE_PROP_UINT32("phy-num", IMXFECState, phy_num, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/net/trace-events b/hw/net/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/net/trace-events +++ b/hw/net/trace-events @@ -XXX,XX +XXX,XX @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries" i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION" # imx_fec.c -imx_phy_read(uint32_t val, int reg) "0x%04"PRIx32" <= reg[%d]" -imx_phy_write(uint32_t val, int reg) "0x%04"PRIx32" => reg[%d]" +imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]" +imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]" imx_phy_update_link(const char *s) "%s" imx_phy_reset(void) "" imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x" -- 2.20.1 From: Jean-Christophe Dubois <jcd@tribudubois.net> Add properties to the i.MX6UL processor to be able to select a particular PHY on the MDIO bus for each FEC device. Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> Message-id: ea1d604198b6b73ea6521676e45bacfc597aba53.1593296112.git.jcd@tribudubois.net Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/fsl-imx6ul.h | 2 ++ hw/arm/fsl-imx6ul.c | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/fsl-imx6ul.h +++ b/include/hw/arm/fsl-imx6ul.h @@ -XXX,XX +XXX,XX @@ typedef struct FslIMX6ULState { MemoryRegion caam; MemoryRegion ocram; MemoryRegion ocram_alias; + + uint32_t phy_num[FSL_IMX6UL_NUM_ETHS]; } FslIMX6ULState; enum FslIMX6ULMemoryMap { diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx6ul.c +++ b/hw/arm/fsl-imx6ul.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp) FSL_IMX6UL_ENET2_TIMER_IRQ, }; + object_property_set_uint(OBJECT(&s->eth[i]), + s->phy_num[i], + "phy-num", &error_abort); object_property_set_uint(OBJECT(&s->eth[i]), FSL_IMX6UL_ETH_NUM_TX_RINGS, "tx-ring-num", &error_abort); @@ -XXX,XX +XXX,XX @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp) FSL_IMX6UL_OCRAM_ALIAS_ADDR, &s->ocram_alias); } +static Property fsl_imx6ul_properties[] = { + DEFINE_PROP_UINT32("fec1-phy-num", FslIMX6ULState, phy_num[0], 0), + DEFINE_PROP_UINT32("fec2-phy-num", FslIMX6ULState, phy_num[1], 1), + DEFINE_PROP_END_OF_LIST(), +}; + static void fsl_imx6ul_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); + device_class_set_props(dc, fsl_imx6ul_properties); dc->realize = fsl_imx6ul_realize; dc->desc = "i.MX6UL SOC"; /* Reason: Uses serial_hds and nd_table in realize() directly */ -- 2.20.1 From: Jean-Christophe Dubois <jcd@tribudubois.net> The i.MX6UL EVK 14x14 board uses: - PHY 2 for FEC 1 - PHY 1 for FEC 2 Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> Message-id: fb41992126c091a71d76ab3d1898959091f60583.1593296112.git.jcd@tribudubois.net Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/mcimx6ul-evk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mcimx6ul-evk.c +++ b/hw/arm/mcimx6ul-evk.c @@ -XXX,XX +XXX,XX @@ static void mcimx6ul_evk_init(MachineState *machine) s = FSL_IMX6UL(object_new(TYPE_FSL_IMX6UL)); object_property_add_child(OBJECT(machine), "soc", OBJECT(s)); + object_property_set_uint(OBJECT(s), 2, "fec1-phy-num", &error_fatal); + object_property_set_uint(OBJECT(s), 1, "fec2-phy-num", &error_fatal); qdev_realize(DEVICE(s), NULL, &error_fatal); memory_region_add_subregion(get_system_memory(), FSL_IMX6UL_MMDC_ADDR, -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> Introduce a new property defining a reserved region: <low address>:<high address>:<type>. This will be used to encode reserved IOVA regions. For instance, in virtio-iommu use case, reserved IOVA regions will be passed by the machine code to the virtio-iommu-pci device (an array of those). The type of the reserved region will match the virtio_iommu_probe_resv_mem subtype value: - VIRTIO_IOMMU_RESV_MEM_T_RESERVED (0) - VIRTIO_IOMMU_RESV_MEM_T_MSI (1) on PC/Q35 machine, this will be used to inform the virtio-iommu-pci device it should bypass the MSI region. The reserved region will be: 0xfee00000:0xfeefffff:1. On ARM, we can declare the ITS MSI doorbell as an MSI region to prevent MSIs from being mapped on guest side. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-id: 20200629070404.10969-2-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/memory.h | 6 +++ include/hw/qdev-properties.h | 3 ++ include/qemu/typedefs.h | 1 + hw/core/qdev-properties.c | 89 ++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ extern bool global_dirty_log; typedef struct MemoryRegionOps MemoryRegionOps; +struct ReservedRegion { + hwaddr low; + hwaddr high; + unsigned type; +}; + typedef struct IOMMUTLBEntry IOMMUTLBEntry; /* See address_space_translate: bit 0 is read, bit 1 is write. */ diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -XXX,XX +XXX,XX @@ extern const PropertyInfo qdev_prop_string; extern const PropertyInfo qdev_prop_chr; extern const PropertyInfo qdev_prop_tpm; extern const PropertyInfo qdev_prop_macaddr; +extern const PropertyInfo qdev_prop_reserved_region; extern const PropertyInfo qdev_prop_on_off_auto; extern const PropertyInfo qdev_prop_multifd_compression; extern const PropertyInfo qdev_prop_losttickpolicy; @@ -XXX,XX +XXX,XX @@ extern const PropertyInfo qdev_prop_pcie_link_width; DEFINE_PROP(_n, _s, _f, qdev_prop_drive_iothread, BlockBackend *) #define DEFINE_PROP_MACADDR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr) +#define DEFINE_PROP_RESERVED_REGION(_n, _s, _f) \ + DEFINE_PROP(_n, _s, _f, qdev_prop_reserved_region, ReservedRegion) #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto) #define DEFINE_PROP_MULTIFD_COMPRESSION(_n, _s, _f, _d) \ diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -XXX,XX +XXX,XX @@ typedef struct ISABus ISABus; typedef struct ISADevice ISADevice; typedef struct IsaDma IsaDma; typedef struct MACAddr MACAddr; +typedef struct ReservedRegion ReservedRegion; typedef struct MachineClass MachineClass; typedef struct MachineState MachineState; typedef struct MemoryListener MemoryListener; diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -XXX,XX +XXX,XX @@ #include "chardev/char.h" #include "qemu/uuid.h" #include "qemu/units.h" +#include "qemu/cutils.h" void qdev_prop_set_after_realize(DeviceState *dev, const char *name, Error **errp) @@ -XXX,XX +XXX,XX @@ const PropertyInfo qdev_prop_macaddr = { .set = set_mac, }; +/* --- Reserved Region --- */ + +/* + * Accepted syntax: + * <low address>:<high address>:<type> + * where low/high addresses are uint64_t in hexadecimal + * and type is a non-negative decimal integer + */ +static void get_reserved_region(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + ReservedRegion *rr = qdev_get_prop_ptr(dev, prop); + char buffer[64]; + char *p = buffer; + int rc; + + rc = snprintf(buffer, sizeof(buffer), "0x%"PRIx64":0x%"PRIx64":%u", + rr->low, rr->high, rr->type); + assert(rc < sizeof(buffer)); + + visit_type_str(v, name, &p, errp); +} + +static void set_reserved_region(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + ReservedRegion *rr = qdev_get_prop_ptr(dev, prop); + Error *local_err = NULL; + const char *endptr; + char *str; + int ret; + + if (dev->realized) { + qdev_prop_set_after_realize(dev, name, errp); + return; + } + + visit_type_str(v, name, &str, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + ret = qemu_strtou64(str, &endptr, 16, &rr->low); + if (ret) { + error_setg(errp, "start address of '%s'" + " must be a hexadecimal integer", name); + goto out; + } + if (*endptr != ':') { + goto separator_error; + } + + ret = qemu_strtou64(endptr + 1, &endptr, 16, &rr->high); + if (ret) { + error_setg(errp, "end address of '%s'" + " must be a hexadecimal integer", name); + goto out; + } + if (*endptr != ':') { + goto separator_error; + } + + ret = qemu_strtoui(endptr + 1, &endptr, 10, &rr->type); + if (ret) { + error_setg(errp, "type of '%s'" + " must be a non-negative decimal integer", name); + } + goto out; + +separator_error: + error_setg(errp, "reserved region fields must be separated with ':'"); +out: + g_free(str); + return; +} + +const PropertyInfo qdev_prop_reserved_region = { + .name = "reserved_region", + .description = "Reserved Region, example: 0xFEE00000:0xFEEFFFFF:0", + .get = get_reserved_region, + .set = set_reserved_region, +}; + /* --- on/off/auto --- */ const PropertyInfo qdev_prop_on_off_auto = { -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> This patch implements the PROBE request. At the moment, only THE RESV_MEM property is handled. The first goal is to report iommu wide reserved regions such as the MSI regions set by the machine code. On x86 this will be the IOAPIC MSI region, [0xFEE00000 - 0xFEEFFFFF], on ARM this may be the ITS doorbell. In the future we may introduce per device reserved regions. This will be useful when protecting host assigned devices which may expose their own reserved regions Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-id: 20200629070404.10969-3-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/virtio/virtio-iommu.h | 2 + hw/virtio/virtio-iommu.c | 94 ++++++++++++++++++++++++++++++-- hw/virtio/trace-events | 1 + 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio-iommu.h +++ b/include/hw/virtio/virtio-iommu.h @@ -XXX,XX +XXX,XX @@ typedef struct VirtIOIOMMU { GHashTable *as_by_busptr; IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX]; PCIBus *primary_bus; + ReservedRegion *reserved_regions; + uint32_t nb_reserved_regions; GTree *domains; QemuMutex mutex; GTree *endpoints; diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -XXX,XX +XXX,XX @@ /* Max size */ #define VIOMMU_DEFAULT_QUEUE_SIZE 256 +#define VIOMMU_PROBE_SIZE 512 typedef struct VirtIOIOMMUDomain { uint32_t id; @@ -XXX,XX +XXX,XX @@ static int virtio_iommu_unmap(VirtIOIOMMU *s, return ret; } +static ssize_t virtio_iommu_fill_resv_mem_prop(VirtIOIOMMU *s, uint32_t ep, + uint8_t *buf, size_t free) +{ + struct virtio_iommu_probe_resv_mem prop = {}; + size_t size = sizeof(prop), length = size - sizeof(prop.head), total; + int i; + + total = size * s->nb_reserved_regions; + + if (total > free) { + return -ENOSPC; + } + + for (i = 0; i < s->nb_reserved_regions; i++) { + unsigned subtype = s->reserved_regions[i].type; + + assert(subtype == VIRTIO_IOMMU_RESV_MEM_T_RESERVED || + subtype == VIRTIO_IOMMU_RESV_MEM_T_MSI); + prop.head.type = cpu_to_le16(VIRTIO_IOMMU_PROBE_T_RESV_MEM); + prop.head.length = cpu_to_le16(length); + prop.subtype = subtype; + prop.start = cpu_to_le64(s->reserved_regions[i].low); + prop.end = cpu_to_le64(s->reserved_regions[i].high); + + memcpy(buf, &prop, size); + + trace_virtio_iommu_fill_resv_property(ep, prop.subtype, + prop.start, prop.end); + buf += size; + } + return total; +} + +/** + * virtio_iommu_probe - Fill the probe request buffer with + * the properties the device is able to return + */ +static int virtio_iommu_probe(VirtIOIOMMU *s, + struct virtio_iommu_req_probe *req, + uint8_t *buf) +{ + uint32_t ep_id = le32_to_cpu(req->endpoint); + size_t free = VIOMMU_PROBE_SIZE; + ssize_t count; + + if (!virtio_iommu_mr(s, ep_id)) { + return VIRTIO_IOMMU_S_NOENT; + } + + count = virtio_iommu_fill_resv_mem_prop(s, ep_id, buf, free); + if (count < 0) { + return VIRTIO_IOMMU_S_INVAL; + } + buf += count; + free -= count; + + return VIRTIO_IOMMU_S_OK; +} + static int virtio_iommu_iov_to_req(struct iovec *iov, unsigned int iov_cnt, void *req, size_t req_sz) @@ -XXX,XX +XXX,XX @@ virtio_iommu_handle_req(detach) virtio_iommu_handle_req(map) virtio_iommu_handle_req(unmap) +static int virtio_iommu_handle_probe(VirtIOIOMMU *s, + struct iovec *iov, + unsigned int iov_cnt, + uint8_t *buf) +{ + struct virtio_iommu_req_probe req; + int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req, sizeof(req)); + + return ret ? ret : virtio_iommu_probe(s, &req, buf); +} + static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq) { VirtIOIOMMU *s = VIRTIO_IOMMU(vdev); struct virtio_iommu_req_head head; struct virtio_iommu_req_tail tail = {}; + size_t output_size = sizeof(tail), sz; VirtQueueElement *elem; unsigned int iov_cnt; struct iovec *iov; - size_t sz; + void *buf = NULL; for (;;) { elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq) case VIRTIO_IOMMU_T_UNMAP: tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt); break; + case VIRTIO_IOMMU_T_PROBE: + { + struct virtio_iommu_req_tail *ptail; + + output_size = s->config.probe_size + sizeof(tail); + buf = g_malloc0(output_size); + + ptail = (struct virtio_iommu_req_tail *) + (buf + s->config.probe_size); + ptail->status = virtio_iommu_handle_probe(s, iov, iov_cnt, buf); + } default: tail.status = VIRTIO_IOMMU_S_UNSUPP; } @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq) out: sz = iov_from_buf(elem->in_sg, elem->in_num, 0, - &tail, sizeof(tail)); - assert(sz == sizeof(tail)); + buf ? buf : &tail, output_size); + assert(sz == output_size); - virtqueue_push(vq, elem, sizeof(tail)); + virtqueue_push(vq, elem, sz); virtio_notify(vdev, vq); g_free(elem); + g_free(buf); } } @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp) s->config.page_size_mask = TARGET_PAGE_MASK; s->config.input_range.end = -1UL; s->config.domain_range.end = 32; + s->config.probe_size = VIOMMU_PROBE_SIZE; virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX); virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC); @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp) virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MAP_UNMAP); virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS); virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO); + virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE); qemu_mutex_init(&s->mutex); diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/trace-events +++ b/hw/virtio/trace-events @@ -XXX,XX +XXX,XX @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d" virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d" virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d" virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64 +virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start, uint64_t end) "dev= %d, type=%d start=0x%"PRIx64" end=0x%"PRIx64 -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> When translating an address we need to check if it belongs to a reserved virtual address range. If it does, there are 2 cases: - it belongs to a RESERVED region: the guest should neither use this address in a MAP not instruct the end-point to DMA on them. We report an error - It belongs to an MSI region: we bypass the translation. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-id: 20200629070404.10969-4-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/virtio/virtio-iommu.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr, uint32_t sid, flags; bool bypass_allowed; bool found; + int i; interval.low = addr; interval.high = addr + 1; @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr, goto unlock; } + for (i = 0; i < s->nb_reserved_regions; i++) { + ReservedRegion *reg = &s->reserved_regions[i]; + + if (addr >= reg->low && addr <= reg->high) { + switch (reg->type) { + case VIRTIO_IOMMU_RESV_MEM_T_MSI: + entry.perm = flag; + break; + case VIRTIO_IOMMU_RESV_MEM_T_RESERVED: + default: + virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_MAPPING, + VIRTIO_IOMMU_FAULT_F_ADDRESS, + sid, addr); + break; + } + goto unlock; + } + } + if (!ep->domain) { if (!bypass_allowed) { error_report_once("%s %02x:%02x.%01x not attached to any domain", -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> The machine may need to pass reserved regions to the virtio-iommu-pci device (such as the MSI window on x86 or the MSI doorbells on ARM). So let's add an array of Interval properties. Note: if some reserved regions are already set by the machine code - which should be the case in general -, the length of the property array is already set and prevents the end-user from modifying them. For example, attempting to use: -device virtio-iommu-pci,\ len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1 would result in the following error message: qemu-system-aarch64: -device virtio-iommu-pci,addr=0xa, len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1: array size property len-reserved-regions may not be set more than once Otherwise, for example, adding two reserved regions is achieved using the following options: -device virtio-iommu-pci,addr=0xa,len-reserved-regions=2,\ reserved-regions[0]=0xfee00000:0xfeefffff:1,\ reserved-regions[1]=0x1000000:100ffff:1 Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> Message-id: 20200629070404.10969-5-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/virtio/virtio-iommu-pci.c | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 @@ struct VirtIOIOMMUPCI { static Property virtio_iommu_pci_properties[] = { DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), + DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI, + vdev.nb_reserved_regions, vdev.reserved_regions, + qdev_prop_reserved_region, ReservedRegion), DEFINE_PROP_END_OF_LIST(), }; @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) { VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev); DeviceState *vdev = DEVICE(&dev->vdev); + VirtIOIOMMU *s = VIRTIO_IOMMU(vdev); if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); @@ -XXX,XX +XXX,XX @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) "-no-acpi\n"); return; } + for (int i = 0; i < s->nb_reserved_regions; i++) { + if (s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_RESERVED && + s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_MSI) { + error_setg(errp, "reserved region %d has an invalid type", i); + error_append_hint(errp, "Valid values are 0 and 1\n"); + } + } object_property_set_link(OBJECT(dev), OBJECT(pci_get_bus(&vpci_dev->pci_dev)), "primary-bus", &error_abort); -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> At the moment the virtio-iommu translates MSI transactions. This behavior is inherited from ARM SMMU. The virt machine code knows where the guest MSI doorbells are so we can easily declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that setting the guest will not map MSIs through the IOMMU and those transactions will be simply bypassed. Depending on which MSI controller is in use (ITS or GICV2M), we declare either: - the ITS interrupt translation space (ITS_base + 0x10000), containing the GITS_TRANSLATOR or - The GICV2M single frame, containing the MSI_SETSP_NS register. Signed-off-by: Eric Auger <eric.auger@redhat.com> Message-id: 20200629070404.10969-6-eric.auger@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 7 +++++++ hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -XXX,XX +XXX,XX @@ typedef enum VirtIOMMUType { VIRT_IOMMU_VIRTIO, } VirtIOMMUType; +typedef enum VirtMSIControllerType { + VIRT_MSI_CTRL_NONE, + VIRT_MSI_CTRL_GICV2M, + VIRT_MSI_CTRL_ITS, +} VirtMSIControllerType; + typedef enum VirtGICType { VIRT_GIC_VERSION_MAX, VIRT_GIC_VERSION_HOST, @@ -XXX,XX +XXX,XX @@ typedef struct { OnOffAuto acpi; VirtGICType gic_version; VirtIOMMUType iommu; + VirtMSIControllerType msi_controller; uint16_t virtio_iommu_bdf; struct arm_boot_info bootinfo; MemMapEntry *memmap; 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 create_its(VirtMachineState *vms) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base); fdt_add_its_gic_node(vms); + vms->msi_controller = VIRT_MSI_CTRL_ITS; } static void create_v2m(VirtMachineState *vms) @@ -XXX,XX +XXX,XX @@ static void create_v2m(VirtMachineState *vms) } fdt_add_v2m_gic_node(vms); + vms->msi_controller = VIRT_MSI_CTRL_GICV2M; } static void create_gic(VirtMachineState *vms) @@ -XXX,XX +XXX,XX @@ out: static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { virt_memory_pre_plug(hotplug_dev, dev, errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { + hwaddr db_start = 0, db_end = 0; + char *resv_prop_str; + + switch (vms->msi_controller) { + case VIRT_MSI_CTRL_NONE: + return; + case VIRT_MSI_CTRL_ITS: + /* GITS_TRANSLATER page */ + db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000; + db_end = base_memmap[VIRT_GIC_ITS].base + + base_memmap[VIRT_GIC_ITS].size - 1; + break; + case VIRT_MSI_CTRL_GICV2M: + /* MSI_SETSPI_NS page */ + db_start = base_memmap[VIRT_GIC_V2M].base; + db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1; + break; + } + resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u", + 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); + g_free(resv_prop_str); } } -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> On ARMv7 & ARMv8 some load/store instructions might trigger a data abort exception with no valid ISS info to be decoded. The lack of decode info makes it at least tricky to emulate those instruction which is one of the (many) reasons why KVM will not even try to do so. Add support for handling those by requesting KVM to inject external dabt into the quest. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-id: 20200629114110.30723-2-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/kvm.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -XXX,XX +XXX,XX @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = { static bool cap_has_mp_state; static bool cap_has_inject_serror_esr; +static bool cap_has_inject_ext_dabt; static ARMHostCPUFeatures arm_host_cpu_features; @@ -XXX,XX +XXX,XX @@ int kvm_arch_init(MachineState *ms, KVMState *s) ret = -EINVAL; } + if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) { + if (kvm_vm_enable_cap(s, KVM_CAP_ARM_NISV_TO_USER, 0)) { + error_report("Failed to enable KVM_CAP_ARM_NISV_TO_USER cap"); + } else { + /* Set status for supporting the external dabt injection */ + cap_has_inject_ext_dabt = kvm_check_extension(s, + KVM_CAP_ARM_INJECT_EXT_DABT); + } + } + return ret; } @@ -XXX,XX +XXX,XX @@ void kvm_arm_vm_state_change(void *opaque, int running, RunState state) } } +/** + * kvm_arm_handle_dabt_nisv: + * @cs: CPUState + * @esr_iss: ISS encoding (limited) for the exception from Data Abort + * ISV bit set to '0b0' -> no valid instruction syndrome + * @fault_ipa: faulting address for the synchronous data abort + * + * Returns: 0 if the exception has been handled, < 0 otherwise + */ +static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss, + uint64_t fault_ipa) +{ + /* + * Request KVM to inject the external data abort into the guest + */ + if (cap_has_inject_ext_dabt) { + struct kvm_vcpu_events events = { }; + /* + * The external data abort event will be handled immediately by KVM + * using the address fault that triggered the exit on given VCPU. + * Requesting injection of the external data abort does not rely + * on any other VCPU state. Therefore, in this particular case, the VCPU + * synchronization can be exceptionally skipped. + */ + events.exception.ext_dabt_pending = 1; + /* KVM_CAP_ARM_INJECT_EXT_DABT implies KVM_CAP_VCPU_EVENTS */ + return kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events); + } else { + error_report("Data abort exception triggered by guest memory access " + "at physical address: 0x" TARGET_FMT_lx, + (target_ulong)fault_ipa); + error_printf("KVM unable to emulate faulting instruction.\n"); + } + return -1; +} + int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) { int ret = 0; @@ -XXX,XX +XXX,XX @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) ret = EXCP_DEBUG; } /* otherwise return to guest */ break; + case KVM_EXIT_ARM_NISV: + /* External DABT with no valid iss to decode */ + ret = kvm_arm_handle_dabt_nisv(cs, run->arm_nisv.esr_iss, + run->arm_nisv.fault_ipa); + break; default: qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n", __func__, run->exit_reason); -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Injecting external data abort through KVM might trigger an issue on kernels that do not get updated to include the KVM fix. For those and aarch32 guests, the injected abort gets misconfigured to be an implementation defined exception. This leads to the guest repeatedly re-running the faulting instruction. Add support for handling that case. [ Fixed-by: 018f22f95e8a ('KVM: arm: Fix DFSR setting for non-LPAE aarch32 guests') Fixed-by: 21aecdbd7f3a ('KVM: arm: Make inject_abt32() inject an external abort instead') ] Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Acked-by: Andrew Jones <drjones@redhat.com> Message-id: 20200629114110.30723-3-beata.michalska@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 ++ target/arm/kvm_arm.h | 10 +++++++++ target/arm/kvm.c | 30 ++++++++++++++++++++++++++- target/arm/kvm32.c | 34 ++++++++++++++++++++++++++++++ target/arm/kvm64.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 1 deletion(-) 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 struct CPUARMState { uint64_t esr; } serror; + uint8_t ext_dabt_raised; /* Tracking/verifying injection of ext DABT */ + /* State of our input IRQ/FIQ/VIRQ/VFIQ lines */ uint32_t irq_line_state; diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -XXX,XX +XXX,XX @@ bool kvm_arm_hw_debug_active(CPUState *cs); struct kvm_guest_debug_arch; void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr); +/** + * kvm_arm_verify_ext_dabt_pending: + * @cs: CPUState + * + * Verify the fault status code wrt the Ext DABT injection + * + * Returns: true if the fault status code is as expected, false otherwise + */ +bool kvm_arm_verify_ext_dabt_pending(CPUState *cs); + /** * its_class_name: * diff --git a/target/arm/kvm.c b/target/arm/kvm.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -XXX,XX +XXX,XX @@ int kvm_get_vcpu_events(ARMCPU *cpu) void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run) { + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + + if (unlikely(env->ext_dabt_raised)) { + /* + * Verifying that the ext DABT has been properly injected, + * otherwise risking indefinitely re-running the faulting instruction + * Covering a very narrow case for kernels 5.5..5.5.4 + * when injected abort was misconfigured to be + * an IMPLEMENTATION DEFINED exception (for 32-bit EL1) + */ + if (!arm_feature(env, ARM_FEATURE_AARCH64) && + unlikely(!kvm_arm_verify_ext_dabt_pending(cs))) { + + error_report("Data abort exception with no valid ISS generated by " + "guest memory access. KVM unable to emulate faulting " + "instruction. Failed to inject an external data abort " + "into the guest."); + abort(); + } + /* Clear the status */ + env->ext_dabt_raised = 0; + } } MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) @@ -XXX,XX +XXX,XX @@ void kvm_arm_vm_state_change(void *opaque, int running, RunState state) static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss, uint64_t fault_ipa) { + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; /* * Request KVM to inject the external data abort into the guest */ @@ -XXX,XX +XXX,XX @@ static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss, */ events.exception.ext_dabt_pending = 1; /* KVM_CAP_ARM_INJECT_EXT_DABT implies KVM_CAP_VCPU_EVENTS */ - return kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events); + if (!kvm_vcpu_ioctl(cs, KVM_SET_VCPU_EVENTS, &events)) { + env->ext_dabt_raised = 1; + return 0; + } } else { error_report("Data abort exception triggered by guest memory access " "at physical address: 0x" TARGET_FMT_lx, diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -XXX,XX +XXX,XX @@ void kvm_arm_pmu_init(CPUState *cs) { qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); } + +#define ARM_REG_DFSR ARM_CP15_REG32(0, 5, 0, 0) +#define ARM_REG_TTBCR ARM_CP15_REG32(0, 2, 0, 2) +/* + *DFSR: + * TTBCR.EAE == 0 + * FS[4] - DFSR[10] + * FS[3:0] - DFSR[3:0] + * TTBCR.EAE == 1 + * FS, bits [5:0] + */ +#define DFSR_FSC(lpae, v) \ + ((lpae) ? ((v) & 0x3F) : (((v) >> 6) | ((v) & 0x1F))) + +#define DFSC_EXTABT(lpae) ((lpae) ? 0x10 : 0x08) + +bool kvm_arm_verify_ext_dabt_pending(CPUState *cs) +{ + uint32_t dfsr_val; + + if (!kvm_get_one_reg(cs, ARM_REG_DFSR, &dfsr_val)) { + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + uint32_t ttbcr; + int lpae = 0; + + if (!kvm_get_one_reg(cs, ARM_REG_TTBCR, &ttbcr)) { + lpae = arm_feature(env, ARM_FEATURE_LPAE) && (ttbcr & TTBCR_EAE); + } + /* The verification is based on FS filed of the DFSR reg only*/ + return (DFSR_FSC(lpae, dfsr_val) == DFSC_EXTABT(lpae)); + } + return false; +} diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) return false; } + +#define ARM64_REG_ESR_EL1 ARM64_SYS_REG(3, 0, 5, 2, 0) +#define ARM64_REG_TCR_EL1 ARM64_SYS_REG(3, 0, 2, 0, 2) + +/* + * ESR_EL1 + * ISS encoding + * AARCH64: DFSC, bits [5:0] + * AARCH32: + * TTBCR.EAE == 0 + * FS[4] - DFSR[10] + * FS[3:0] - DFSR[3:0] + * TTBCR.EAE == 1 + * FS, bits [5:0] + */ +#define ESR_DFSC(aarch64, lpae, v) \ + ((aarch64 || (lpae)) ? ((v) & 0x3F) \ + : (((v) >> 6) | ((v) & 0x1F))) + +#define ESR_DFSC_EXTABT(aarch64, lpae) \ + ((aarch64) ? 0x10 : (lpae) ? 0x10 : 0x8) + +bool kvm_arm_verify_ext_dabt_pending(CPUState *cs) +{ + uint64_t dfsr_val; + + if (!kvm_get_one_reg(cs, ARM64_REG_ESR_EL1, &dfsr_val)) { + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + int aarch64_mode = arm_feature(env, ARM_FEATURE_AARCH64); + int lpae = 0; + + if (!aarch64_mode) { + uint64_t ttbcr; + + if (!kvm_get_one_reg(cs, ARM64_REG_TCR_EL1, &ttbcr)) { + lpae = arm_feature(env, ARM_FEATURE_LPAE) + && (ttbcr & TTBCR_EAE); + } + } + /* + * The verification here is based on the DFSC bits + * of the ESR_EL1 reg only + */ + return (ESR_DFSC(aarch64_mode, lpae, dfsr_val) == + ESR_DFSC_EXTABT(aarch64_mode, lpae)); + } + return false; +} -- 2.20.1 From: Andrew Jones <drjones@redhat.com> Fixes: 93dd625f8bf7 ("tests/acpi: update expected data files") Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20200629140938.17566-2-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 18 ------------------ 1 file changed, 18 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 @@ -1,19 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/pc/DSDT", -"tests/data/acpi/pc/DSDT.acpihmat", -"tests/data/acpi/pc/DSDT.bridge", -"tests/data/acpi/pc/DSDT.cphp", -"tests/data/acpi/pc/DSDT.dimmpxm", -"tests/data/acpi/pc/DSDT.ipmikcs", -"tests/data/acpi/pc/DSDT.memhp", -"tests/data/acpi/pc/DSDT.numamem", -"tests/data/acpi/q35/DSDT", -"tests/data/acpi/q35/DSDT.acpihmat", -"tests/data/acpi/q35/DSDT.bridge", -"tests/data/acpi/q35/DSDT.cphp", -"tests/data/acpi/q35/DSDT.dimmpxm", -"tests/data/acpi/q35/DSDT.ipmibt", -"tests/data/acpi/q35/DSDT.memhp", -"tests/data/acpi/q35/DSDT.mmio64", -"tests/data/acpi/q35/DSDT.numamem", -"tests/data/acpi/q35/DSDT.tis", -- 2.20.1 From: Andrew Jones <drjones@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20200629140938.17566-3-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) 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/DSDT", +"tests/data/acpi/virt/DSDT.memhp", +"tests/data/acpi/virt/DSDT.numamem", -- 2.20.1 From: Andrew Jones <drjones@redhat.com> The flash device is exclusively for the host-controlled firmware, so we should not expose it to the OS. Exposing it risks the OS messing with it, which could break firmware runtime services and surprise the OS when all its changes disappear after reboot. As firmware needs the device and uses DT, we leave the device exposed there. It's up to firmware to remove the nodes from DT before sending it on to the OS. However, there's no need to force firmware to remove tables from ACPI (which it doesn't know how to do anyway), so we simply don't add the tables in the first place. But, as we've been adding the tables for quite some time and don't want to change the default hardware exposed to versioned machines, then we only stop exposing the flash device tables for 5.1 and later machine types. Suggested-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Suggested-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Message-id: 20200629140938.17566-4-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 1 + hw/arm/virt-acpi-build.c | 5 ++++- hw/arm/virt.c | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -XXX,XX +XXX,XX @@ typedef struct { bool no_highmem_ecam; bool no_ged; /* Machines < 4.2 has no support for ACPI GED device */ bool kvm_no_adjvtime; + bool acpi_expose_flash; } VirtMachineClass; typedef struct { 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 @@ static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker, static void build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { + VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); Aml *scope, *dsdt; MachineState *ms = MACHINE(vms); const MemMapEntry *memmap = vms->memmap; @@ -XXX,XX +XXX,XX @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) acpi_dsdt_add_cpus(scope, vms->smp_cpus); acpi_dsdt_add_uart(scope, &memmap[VIRT_UART], (irqmap[VIRT_UART] + ARM_SPI_BASE)); - acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); + if (vmc->acpi_expose_flash) { + acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); + } acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]); acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO], (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS); 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 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 1) static void virt_machine_5_0_options(MachineClass *mc) { + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc)); + virt_machine_5_1_options(mc); compat_props_add(mc->compat_props, hw_compat_5_0, hw_compat_5_0_len); mc->numa_mem_supported = true; + vmc->acpi_expose_flash = true; } DEFINE_VIRT_MACHINE(5, 0) -- 2.20.1 From: Andrew Jones <drjones@redhat.com> Differences between disassembled ASL files for DSDT: @@ -XXX,XX +XXX,XX @@ * * Disassembling to symbolic ASL+ operators * - * Disassembly of a, Mon Jun 29 09:50:01 2020 + * Disassembly of b, Mon Jun 29 09:50:03 2020 * * Original Table Header: * Signature "DSDT" - * Length 0x000014BB (5307) + * Length 0x00001455 (5205) * Revision 0x02 - * Checksum 0xD1 + * Checksum 0xE1 * OEM ID "BOCHS " * OEM Table ID "BXPCDSDT" * OEM Revision 0x00000001 (1) @@ -XXX,XX +XXX,XX @@ }) } - Device (FLS0) - { - Name (_HID, "LNRO0015") // _HID: Hardware ID - Name (_UID, Zero) // _UID: Unique ID - Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings - { - Memory32Fixed (ReadWrite, - 0x00000000, // Address Base - 0x04000000, // Address Length - ) - }) - } - - Device (FLS1) - { - Name (_HID, "LNRO0015") // _HID: Hardware ID - Name (_UID, One) // _UID: Unique ID - Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings - { - Memory32Fixed (ReadWrite, - 0x04000000, // Address Base - 0x04000000, // Address Length - ) - }) - } - Device (FWCF) { Name (_HID, "QEMU0002") // _HID: Hardware ID The other two binaries have the same changes (the removal of the flash devices). Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Message-id: 20200629140938.17566-5-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 3 --- tests/data/acpi/virt/DSDT | Bin 5307 -> 5205 bytes tests/data/acpi/virt/DSDT.memhp | Bin 6668 -> 6566 bytes tests/data/acpi/virt/DSDT.numamem | Bin 5307 -> 5205 bytes 4 files changed, 3 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 @@ -1,4 +1 @@ /* List of comma-separated changed AML files to ignore */ -"tests/data/acpi/virt/DSDT", -"tests/data/acpi/virt/DSDT.memhp", -"tests/data/acpi/virt/DSDT.numamem", diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmdn3c~yhUCD<h-RD^+n>ET2!X{H9}iRuX(-<}f&0DgxFc>n+a delta 156 zcmcbrv0IbNCD<iow+I6R)5VEg(oAih6V(&y4c&Z#4LIUGJY9Hw{DS-q3=B;fIO0P+ zU4W!>P_UpN7hfAE10w?juv9WcH-WSmV$;Hiu7w4t3#`S$E!^1+q9xGPH`KtuzzAr5 LaERl^1zUvy_;n(J diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmeA%S!T@T66_MPOp<|tiD@F2G*jb@iRuX(-^xn@0CHUjRR910 delta 156 zcmZ2x++)J!66_MfBgMeL^l>7WG*kP$iRuaUhHgH=1|0Doo-VvTenI{Q28N~#9Py!^ zE<n;bC|FRCi?5B7fsp|MSSlH!n?PC&v1wsM*TMqS1=eEW7Vhi@(GuwD8){%+U<5Qj LIK*+|0yaqism~!^ diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmdn3c~yhUCD<h-RD^+n>ET2!X{H9}iRuX(-<}f&0DgxFc>n+a delta 156 zcmcbrv0IbNCD<iow+I6R)5VEg(oAih6V(&y4c&Z#4LIUGJY9Hw{DS-q3=B;fIO0P+ zU4W!>P_UpN7hfAE10w?juv9WcH-WSmV$;Hiu7w4t3#`S$E!^1+q9xGPH`KtuzzAr5 LaERl^1zUvy_;n(J -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The temp that gets assigned to clean_addr has been allocated with new_tmp_a64, which means that it will be freed at the end of the instruction. Freeing it earlier leads to assertion failure. The loop creates a complication, in which we allocate a new local temp, which does need freeing, and the final code path is shared between the loop and non-loop. Fix this complication by adding new_tmp_a64_local so that the new local temp is freed at the end, and can be treated exactly like the non-loop path. Fixes: bba87d0a0f4 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200702175605.1987125-1-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.h | 1 + target/arm/translate-a64.c | 6 ++++++ target/arm/translate-sve.c | 8 ++------ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -XXX,XX +XXX,XX @@ void unallocated_encoding(DisasContext *s); } while (0) TCGv_i64 new_tmp_a64(DisasContext *s); +TCGv_i64 new_tmp_a64_local(DisasContext *s); TCGv_i64 new_tmp_a64_zero(DisasContext *s); TCGv_i64 cpu_reg(DisasContext *s, int reg); TCGv_i64 cpu_reg_sp(DisasContext *s, int reg); 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 @@ TCGv_i64 new_tmp_a64(DisasContext *s) return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_new_i64(); } +TCGv_i64 new_tmp_a64_local(DisasContext *s) +{ + assert(s->tmp_a64_count < TMP_A64_MAX); + return s->tmp_a64[s->tmp_a64_count++] = tcg_temp_local_new_i64(); +} + TCGv_i64 new_tmp_a64_zero(DisasContext *s) { TCGv_i64 t = new_tmp_a64(s); diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-sve.c +++ b/target/arm/translate-sve.c @@ -XXX,XX +XXX,XX @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm) /* Copy the clean address into a local temp, live across the loop. */ t0 = clean_addr; - clean_addr = tcg_temp_local_new_i64(); + clean_addr = new_tmp_a64_local(s); tcg_gen_mov_i64(clean_addr, t0); - tcg_temp_free_i64(t0); gen_set_label(loop); @@ -XXX,XX +XXX,XX @@ static void do_ldr(DisasContext *s, uint32_t vofs, int len, int rn, int imm) tcg_gen_st_i64(t0, cpu_env, vofs + len_align); tcg_temp_free_i64(t0); } - tcg_temp_free_i64(clean_addr); } /* Similarly for stores. */ @@ -XXX,XX +XXX,XX @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm) /* Copy the clean address into a local temp, live across the loop. */ t0 = clean_addr; - clean_addr = tcg_temp_local_new_i64(); + clean_addr = new_tmp_a64_local(s); tcg_gen_mov_i64(clean_addr, t0); - tcg_temp_free_i64(t0); gen_set_label(loop); @@ -XXX,XX +XXX,XX @@ static void do_str(DisasContext *s, uint32_t vofs, int len, int rn, int imm) } tcg_temp_free_i64(t0); } - tcg_temp_free_i64(clean_addr); } static bool trans_LDR_zri(DisasContext *s, arg_rri *a) -- 2.20.1 In bcm2835_fb_mbox_push(), Coverity complains (CID 1429989) that we pass a pointer to a local struct to another function without initializing all its fields. This is a real bug: bcm2835_fb_reconfigure() copies the whole of our new BCM2385FBConfig struct into s->config, so any fields we don't initialize will corrupt the state of the device. Copy the two fields which we don't want to update (pixo and alpha) from the existing config so we don't accidentally change them. Fixes: cfb7ba983857e40e88 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200628195436.27582-1-peter.maydell@linaro.org --- hw/display/bcm2835_fb.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c index XXXXXXX..XXXXXXX 100644 --- a/hw/display/bcm2835_fb.c +++ b/hw/display/bcm2835_fb.c @@ -XXX,XX +XXX,XX @@ static void bcm2835_fb_mbox_push(BCM2835FBState *s, uint32_t value) newconf.base = s->vcram_base | (value & 0xc0000000); newconf.base += BCM2835_FB_OFFSET; + /* Copy fields which we don't want to change from the existing config */ + newconf.pixo = s->config.pixo; + newconf.alpha = s->config.alpha; + bcm2835_fb_validate_config(&newconf); pitch = bcm2835_fb_get_pitch(&newconf); -- 2.20.1 The spitz board has been around a long time, and still has a fair number of hard-coded tab characters in it. We're about to do some work on this source file, so start out by expanding out the tabs. This commit is a pure whitespace only change. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-2-peter.maydell@linaro.org --- hw/arm/spitz.c | 156 ++++++++++++++++++++++++------------------------- 1 file changed, 78 insertions(+), 78 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ #include "cpu.h" #undef REG_FMT -#define REG_FMT "0x%02lx" +#define REG_FMT "0x%02lx" /* Spitz Flash */ -#define FLASH_BASE 0x0c000000 -#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */ -#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */ -#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */ -#define FLASH_ECCCNTR 0x0c /* ECC byte counter */ -#define FLASH_ECCCLRR 0x10 /* Clear ECC */ -#define FLASH_FLASHIO 0x14 /* Flash I/O */ -#define FLASH_FLASHCTL 0x18 /* Flash Control */ +#define FLASH_BASE 0x0c000000 +#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */ +#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */ +#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */ +#define FLASH_ECCCNTR 0x0c /* ECC byte counter */ +#define FLASH_ECCCLRR 0x10 /* Clear ECC */ +#define FLASH_FLASHIO 0x14 /* Flash I/O */ +#define FLASH_FLASHCTL 0x18 /* Flash Control */ -#define FLASHCTL_CE0 (1 << 0) -#define FLASHCTL_CLE (1 << 1) -#define FLASHCTL_ALE (1 << 2) -#define FLASHCTL_WP (1 << 3) -#define FLASHCTL_CE1 (1 << 4) -#define FLASHCTL_RYBY (1 << 5) -#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1) +#define FLASHCTL_CE0 (1 << 0) +#define FLASHCTL_CLE (1 << 1) +#define FLASHCTL_ALE (1 << 2) +#define FLASHCTL_WP (1 << 3) +#define FLASHCTL_CE1 (1 << 4) +#define FLASHCTL_RYBY (1 << 5) +#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1) #define TYPE_SL_NAND "sl-nand" #define SL_NAND(obj) OBJECT_CHECK(SLNANDState, (obj), TYPE_SL_NAND) @@ -XXX,XX +XXX,XX @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size) int ryby; switch (addr) { -#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to)) +#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to)) case FLASH_ECCLPLB: return BSHR(0, 4, 0) | BSHR(0, 5, 2) | BSHR(0, 6, 4) | BSHR(0, 7, 6) | BSHR(1, 4, 1) | BSHR(1, 5, 3) | BSHR(1, 6, 5) | BSHR(1, 7, 7); -#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to)) +#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to)) case FLASH_ECCLPUB: return BSHL(0, 0, 0) | BSHL(0, 1, 2) | BSHL(0, 2, 4) | BSHL(0, 3, 6) | BSHL(1, 0, 1) | BSHL(1, 1, 3) | BSHL(1, 2, 5) | BSHL(1, 3, 7); @@ -XXX,XX +XXX,XX @@ static void sl_nand_realize(DeviceState *dev, Error **errp) /* Spitz Keyboard */ -#define SPITZ_KEY_STROBE_NUM 11 -#define SPITZ_KEY_SENSE_NUM 7 +#define SPITZ_KEY_STROBE_NUM 11 +#define SPITZ_KEY_SENSE_NUM 7 static const int spitz_gpio_key_sense[SPITZ_KEY_SENSE_NUM] = { 12, 17, 91, 34, 36, 38, 39 @@ -XXX,XX +XXX,XX @@ static int spitz_keymap[SPITZ_KEY_SENSE_NUM + 1][SPITZ_KEY_STROBE_NUM] = { { 0x52, 0x43, 0x01, 0x47, 0x49, -1 , -1 , -1 , -1 , -1 , -1 }, }; -#define SPITZ_GPIO_AK_INT 13 /* Remote control */ -#define SPITZ_GPIO_SYNC 16 /* Sync button */ -#define SPITZ_GPIO_ON_KEY 95 /* Power button */ -#define SPITZ_GPIO_SWA 97 /* Lid */ -#define SPITZ_GPIO_SWB 96 /* Tablet mode */ +#define SPITZ_GPIO_AK_INT 13 /* Remote control */ +#define SPITZ_GPIO_SYNC 16 /* Sync button */ +#define SPITZ_GPIO_ON_KEY 95 /* Power button */ +#define SPITZ_GPIO_SWA 97 /* Lid */ +#define SPITZ_GPIO_SWB 96 /* Tablet mode */ /* The special buttons are mapped to unused keys */ static const int spitz_gpiomap[5] = { @@ -XXX,XX +XXX,XX @@ static void spitz_keyboard_keydown(SpitzKeyboardState *s, int keycode) #define SPITZ_MOD_CTRL (1 << 8) #define SPITZ_MOD_FN (1 << 9) -#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c +#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c static void spitz_keyboard_handler(void *opaque, int keycode) { @@ -XXX,XX +XXX,XX @@ static void spitz_keyboard_handler(void *opaque, int keycode) uint16_t code; int mapcode; switch (keycode) { - case 0x2a: /* Left Shift */ + case 0x2a: /* Left Shift */ s->modifiers |= 1; break; case 0xaa: s->modifiers &= ~1; break; - case 0x36: /* Right Shift */ + case 0x36: /* Right Shift */ s->modifiers |= 2; break; case 0xb6: s->modifiers &= ~2; break; - case 0x1d: /* Control */ + case 0x1d: /* Control */ s->modifiers |= 4; break; case 0x9d: s->modifiers &= ~4; break; - case 0x38: /* Alt */ + case 0x38: /* Alt */ s->modifiers |= 8; break; case 0xb8: @@ -XXX,XX +XXX,XX @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp) /* LCD backlight controller */ -#define LCDTG_RESCTL 0x00 -#define LCDTG_PHACTRL 0x01 -#define LCDTG_DUTYCTRL 0x02 -#define LCDTG_POWERREG0 0x03 -#define LCDTG_POWERREG1 0x04 -#define LCDTG_GPOR3 0x05 -#define LCDTG_PICTRL 0x06 -#define LCDTG_POLCTRL 0x07 +#define LCDTG_RESCTL 0x00 +#define LCDTG_PHACTRL 0x01 +#define LCDTG_DUTYCTRL 0x02 +#define LCDTG_POWERREG0 0x03 +#define LCDTG_POWERREG1 0x04 +#define LCDTG_GPOR3 0x05 +#define LCDTG_PICTRL 0x06 +#define LCDTG_POLCTRL 0x07 typedef struct { SSISlave ssidev; @@ -XXX,XX +XXX,XX @@ static void spitz_lcdtg_realize(SSISlave *dev, Error **errp) /* SSP devices */ -#define CORGI_SSP_PORT 2 +#define CORGI_SSP_PORT 2 -#define SPITZ_GPIO_LCDCON_CS 53 -#define SPITZ_GPIO_ADS7846_CS 14 -#define SPITZ_GPIO_MAX1111_CS 20 -#define SPITZ_GPIO_TP_INT 11 +#define SPITZ_GPIO_LCDCON_CS 53 +#define SPITZ_GPIO_ADS7846_CS 14 +#define SPITZ_GPIO_MAX1111_CS 20 +#define SPITZ_GPIO_TP_INT 11 static DeviceState *max1111; @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level) s->enable[line] = !level; } -#define MAX1111_BATT_VOLT 1 -#define MAX1111_BATT_TEMP 2 -#define MAX1111_ACIN_VOLT 3 +#define MAX1111_BATT_VOLT 1 +#define MAX1111_BATT_TEMP 2 +#define MAX1111_ACIN_VOLT 3 -#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */ -#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */ -#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */ +#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */ +#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */ +#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */ static void spitz_adc_temp_on(void *opaque, int line, int level) { @@ -XXX,XX +XXX,XX @@ static void spitz_microdrive_attach(PXA2xxState *cpu, int slot) /* Wm8750 and Max7310 on I2C */ -#define AKITA_MAX_ADDR 0x18 -#define SPITZ_WM_ADDRL 0x1b -#define SPITZ_WM_ADDRH 0x1a +#define AKITA_MAX_ADDR 0x18 +#define SPITZ_WM_ADDRL 0x1b +#define SPITZ_WM_ADDRH 0x1a -#define SPITZ_GPIO_WM 5 +#define SPITZ_GPIO_WM 5 static void spitz_wm8750_addr(void *opaque, int line, int level) { @@ -XXX,XX +XXX,XX @@ static void spitz_out_switch(void *opaque, int line, int level) } } -#define SPITZ_SCP_LED_GREEN 1 -#define SPITZ_SCP_JK_B 2 -#define SPITZ_SCP_CHRG_ON 3 -#define SPITZ_SCP_MUTE_L 4 -#define SPITZ_SCP_MUTE_R 5 -#define SPITZ_SCP_CF_POWER 6 -#define SPITZ_SCP_LED_ORANGE 7 -#define SPITZ_SCP_JK_A 8 -#define SPITZ_SCP_ADC_TEMP_ON 9 -#define SPITZ_SCP2_IR_ON 1 -#define SPITZ_SCP2_AKIN_PULLUP 2 -#define SPITZ_SCP2_BACKLIGHT_CONT 7 -#define SPITZ_SCP2_BACKLIGHT_ON 8 -#define SPITZ_SCP2_MIC_BIAS 9 +#define SPITZ_SCP_LED_GREEN 1 +#define SPITZ_SCP_JK_B 2 +#define SPITZ_SCP_CHRG_ON 3 +#define SPITZ_SCP_MUTE_L 4 +#define SPITZ_SCP_MUTE_R 5 +#define SPITZ_SCP_CF_POWER 6 +#define SPITZ_SCP_LED_ORANGE 7 +#define SPITZ_SCP_JK_A 8 +#define SPITZ_SCP_ADC_TEMP_ON 9 +#define SPITZ_SCP2_IR_ON 1 +#define SPITZ_SCP2_AKIN_PULLUP 2 +#define SPITZ_SCP2_BACKLIGHT_CONT 7 +#define SPITZ_SCP2_BACKLIGHT_ON 8 +#define SPITZ_SCP2_MIC_BIAS 9 static void spitz_scoop_gpio_setup(PXA2xxState *cpu, DeviceState *scp0, DeviceState *scp1) @@ -XXX,XX +XXX,XX @@ static void spitz_scoop_gpio_setup(PXA2xxState *cpu, qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]); } -#define SPITZ_GPIO_HSYNC 22 -#define SPITZ_GPIO_SD_DETECT 9 -#define SPITZ_GPIO_SD_WP 81 -#define SPITZ_GPIO_ON_RESET 89 -#define SPITZ_GPIO_BAT_COVER 90 -#define SPITZ_GPIO_CF1_IRQ 105 -#define SPITZ_GPIO_CF1_CD 94 -#define SPITZ_GPIO_CF2_IRQ 106 -#define SPITZ_GPIO_CF2_CD 93 +#define SPITZ_GPIO_HSYNC 22 +#define SPITZ_GPIO_SD_DETECT 9 +#define SPITZ_GPIO_SD_WP 81 +#define SPITZ_GPIO_ON_RESET 89 +#define SPITZ_GPIO_BAT_COVER 90 +#define SPITZ_GPIO_CF1_IRQ 105 +#define SPITZ_GPIO_CF1_CD 94 +#define SPITZ_GPIO_CF2_IRQ 106 +#define SPITZ_GPIO_CF2_CD 93 static int spitz_hsync; @@ -XXX,XX +XXX,XX @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots) /* Board init. */ enum spitz_model_e { spitz, akita, borzoi, terrier }; -#define SPITZ_RAM 0x04000000 -#define SPITZ_ROM 0x00800000 +#define SPITZ_RAM 0x04000000 +#define SPITZ_ROM 0x00800000 static struct arm_boot_info spitz_binfo = { .loader_start = PXA2XX_SDRAM_BASE, -- 2.20.1 For the four Spitz-family machines (akita, borzoi, spitz, terrier) create a proper abstract class SpitzMachineClass which encapsulates the common behaviour, rather than having them all derive directly from TYPE_MACHINE: * instead of each machine class setting mc->init to a wrapper function which calls spitz_common_init() with parameters, put that data in the SpitzMachineClass and make spitz_common_init the SpitzMachineClass machine-init function * move the settings of mc->block_default_type and mc->ignore_memory_transaction_failures into the SpitzMachineClass class init rather than repeating them in each machine's class init (The motivation is that we're going to want to keep some state in the SpitzMachineState so we can connect GPIOs between devices created in one sub-function of the machine init to devices created in a different sub-function.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200628142429.17111-3-peter.maydell@linaro.org --- hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ #include "exec/address-spaces.h" #include "cpu.h" +enum spitz_model_e { spitz, akita, borzoi, terrier }; + +typedef struct { + MachineClass parent; + enum spitz_model_e model; + int arm_id; +} SpitzMachineClass; + +typedef struct { + MachineState parent; +} SpitzMachineState; + +#define TYPE_SPITZ_MACHINE "spitz-common" +#define SPITZ_MACHINE(obj) \ + OBJECT_CHECK(SpitzMachineState, obj, TYPE_SPITZ_MACHINE) +#define SPITZ_MACHINE_GET_CLASS(obj) \ + OBJECT_GET_CLASS(SpitzMachineClass, obj, TYPE_SPITZ_MACHINE) +#define SPITZ_MACHINE_CLASS(klass) \ + OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE) + #undef REG_FMT #define REG_FMT "0x%02lx" @@ -XXX,XX +XXX,XX @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots) } /* Board init. */ -enum spitz_model_e { spitz, akita, borzoi, terrier }; - #define SPITZ_RAM 0x04000000 #define SPITZ_ROM 0x00800000 @@ -XXX,XX +XXX,XX @@ static struct arm_boot_info spitz_binfo = { .ram_size = 0x04000000, }; -static void spitz_common_init(MachineState *machine, - enum spitz_model_e model, int arm_id) +static void spitz_common_init(MachineState *machine) { + SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine); + enum spitz_model_e model = smc->model; PXA2xxState *mpu; DeviceState *scp0, *scp1 = NULL; MemoryRegion *address_space_mem = get_system_memory(); @@ -XXX,XX +XXX,XX @@ static void spitz_common_init(MachineState *machine, /* A 4.0 GB microdrive is permanently sitting in CF slot 0. */ spitz_microdrive_attach(mpu, 0); - spitz_binfo.board_id = arm_id; + spitz_binfo.board_id = smc->arm_id; arm_load_kernel(mpu->cpu, machine, &spitz_binfo); sl_bootparam_write(SL_PXA_PARAM_BASE); } -static void spitz_init(MachineState *machine) +static void spitz_common_class_init(ObjectClass *oc, void *data) { - spitz_common_init(machine, spitz, 0x2c9); + MachineClass *mc = MACHINE_CLASS(oc); + + mc->block_default_type = IF_IDE; + mc->ignore_memory_transaction_failures = true; + mc->init = spitz_common_init; } -static void borzoi_init(MachineState *machine) -{ - spitz_common_init(machine, borzoi, 0x33f); -} - -static void akita_init(MachineState *machine) -{ - spitz_common_init(machine, akita, 0x2e8); -} - -static void terrier_init(MachineState *machine) -{ - spitz_common_init(machine, terrier, 0x33f); -} +static const TypeInfo spitz_common_info = { + .name = TYPE_SPITZ_MACHINE, + .parent = TYPE_MACHINE, + .abstract = true, + .instance_size = sizeof(SpitzMachineState), + .class_size = sizeof(SpitzMachineClass), + .class_init = spitz_common_class_init, +}; static void akitapda_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); + SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc); mc->desc = "Sharp SL-C1000 (Akita) PDA (PXA270)"; - mc->init = akita_init; - mc->ignore_memory_transaction_failures = true; mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0"); + smc->model = akita; + smc->arm_id = 0x2e8; } static const TypeInfo akitapda_type = { .name = MACHINE_TYPE_NAME("akita"), - .parent = TYPE_MACHINE, + .parent = TYPE_SPITZ_MACHINE, .class_init = akitapda_class_init, }; static void spitzpda_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); + SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc); mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)"; - mc->init = spitz_init; - mc->block_default_type = IF_IDE; - mc->ignore_memory_transaction_failures = true; mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0"); + smc->model = spitz; + smc->arm_id = 0x2c9; } static const TypeInfo spitzpda_type = { .name = MACHINE_TYPE_NAME("spitz"), - .parent = TYPE_MACHINE, + .parent = TYPE_SPITZ_MACHINE, .class_init = spitzpda_class_init, }; static void borzoipda_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); + SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc); mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)"; - mc->init = borzoi_init; - mc->block_default_type = IF_IDE; - mc->ignore_memory_transaction_failures = true; mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0"); + smc->model = borzoi; + smc->arm_id = 0x33f; } static const TypeInfo borzoipda_type = { .name = MACHINE_TYPE_NAME("borzoi"), - .parent = TYPE_MACHINE, + .parent = TYPE_SPITZ_MACHINE, .class_init = borzoipda_class_init, }; static void terrierpda_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); + SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc); mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)"; - mc->init = terrier_init; - mc->block_default_type = IF_IDE; - mc->ignore_memory_transaction_failures = true; mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c5"); + smc->model = terrier; + smc->arm_id = 0x33f; } static const TypeInfo terrierpda_type = { .name = MACHINE_TYPE_NAME("terrier"), - .parent = TYPE_MACHINE, + .parent = TYPE_SPITZ_MACHINE, .class_init = terrierpda_class_init, }; static void spitz_machine_init(void) { + type_register_static(&spitz_common_info); type_register_static(&akitapda_type); type_register_static(&spitzpda_type); type_register_static(&borzoipda_type); -- 2.20.1 Keep pointers to the MPU and the SSI devices in SpitzMachineState. We're going to want to make GPIO connections between some of the SSI devices and the SCPs, so we want to keep hold of a pointer to those; putting the MPU into the struct allows us to pass just one thing to spitz_ssp_attach() rather than two. We have to retain the setting of the global "max1111" variable for the moment as it is used in spitz_adc_temp_on(); later in this series of commits we will be able to remove it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-4-peter.maydell@linaro.org --- hw/arm/spitz.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ typedef struct { typedef struct { MachineState parent; + PXA2xxState *mpu; + DeviceState *mux; + DeviceState *lcdtg; + DeviceState *ads7846; + DeviceState *max1111; } SpitzMachineState; #define TYPE_SPITZ_MACHINE "spitz-common" @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_realize(SSISlave *d, Error **errp) s->bus[2] = ssi_create_bus(dev, "ssi2"); } -static void spitz_ssp_attach(PXA2xxState *cpu) +static void spitz_ssp_attach(SpitzMachineState *sms) { - DeviceState *mux; - DeviceState *dev; void *bus; - mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp"); + sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp"); - bus = qdev_get_child_bus(mux, "ssi0"); - ssi_create_slave(bus, "spitz-lcdtg"); + bus = qdev_get_child_bus(sms->mux, "ssi0"); + sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg"); - bus = qdev_get_child_bus(mux, "ssi1"); - dev = ssi_create_slave(bus, "ads7846"); - qdev_connect_gpio_out(dev, 0, - qdev_get_gpio_in(cpu->gpio, SPITZ_GPIO_TP_INT)); + bus = qdev_get_child_bus(sms->mux, "ssi1"); + sms->ads7846 = ssi_create_slave(bus, "ads7846"); + qdev_connect_gpio_out(sms->ads7846, 0, + qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT)); - bus = qdev_get_child_bus(mux, "ssi2"); - max1111 = ssi_create_slave(bus, "max1111"); - max111x_set_input(max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT); - max111x_set_input(max1111, MAX1111_BATT_TEMP, 0); - max111x_set_input(max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN); + bus = qdev_get_child_bus(sms->mux, "ssi2"); + sms->max1111 = ssi_create_slave(bus, "max1111"); + max1111 = sms->max1111; + max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT); + max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0); + max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN); - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_LCDCON_CS, - qdev_get_gpio_in(mux, 0)); - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_ADS7846_CS, - qdev_get_gpio_in(mux, 1)); - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_MAX1111_CS, - qdev_get_gpio_in(mux, 2)); + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS, + qdev_get_gpio_in(sms->mux, 0)); + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_ADS7846_CS, + qdev_get_gpio_in(sms->mux, 1)); + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_MAX1111_CS, + qdev_get_gpio_in(sms->mux, 2)); } /* CF Microdrive */ @@ -XXX,XX +XXX,XX @@ static struct arm_boot_info spitz_binfo = { static void spitz_common_init(MachineState *machine) { SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine); + SpitzMachineState *sms = SPITZ_MACHINE(machine); enum spitz_model_e model = smc->model; PXA2xxState *mpu; DeviceState *scp0, *scp1 = NULL; @@ -XXX,XX +XXX,XX @@ static void spitz_common_init(MachineState *machine) /* Setup CPU & memory */ mpu = pxa270_init(address_space_mem, spitz_binfo.ram_size, machine->cpu_type); + sms->mpu = mpu; sl_flash_register(mpu, (model == spitz) ? FLASH_128M : FLASH_1024M); @@ -XXX,XX +XXX,XX @@ static void spitz_common_init(MachineState *machine) /* Setup peripherals */ spitz_keyboard_register(mpu); - spitz_ssp_attach(mpu); + spitz_ssp_attach(sms); scp0 = sysbus_create_simple("scoop", 0x10800000, NULL); if (model != akita) { -- 2.20.1 Keep pointers to scp0, scp1 in SpitzMachineState, and just pass that to spitz_scoop_gpio_setup(). (We'll want to use some of the other fields in SpitzMachineState in that function in the next commit.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-5-peter.maydell@linaro.org --- hw/arm/spitz.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ typedef struct { DeviceState *lcdtg; DeviceState *ads7846; DeviceState *max1111; + DeviceState *scp0; + DeviceState *scp1; } SpitzMachineState; #define TYPE_SPITZ_MACHINE "spitz-common" @@ -XXX,XX +XXX,XX @@ static void spitz_out_switch(void *opaque, int line, int level) #define SPITZ_SCP2_BACKLIGHT_ON 8 #define SPITZ_SCP2_MIC_BIAS 9 -static void spitz_scoop_gpio_setup(PXA2xxState *cpu, - DeviceState *scp0, DeviceState *scp1) +static void spitz_scoop_gpio_setup(SpitzMachineState *sms) { - qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, cpu, 8); + qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8); - qdev_connect_gpio_out(scp0, SPITZ_SCP_CHRG_ON, outsignals[0]); - qdev_connect_gpio_out(scp0, SPITZ_SCP_JK_B, outsignals[1]); - qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_GREEN, outsignals[2]); - qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]); - if (scp1) { - qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_CONT, outsignals[4]); - qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_ON, outsignals[5]); + if (sms->scp1) { + qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT, + outsignals[4]); + qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON, + outsignals[5]); } - qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]); } #define SPITZ_GPIO_HSYNC 22 @@ -XXX,XX +XXX,XX @@ static void spitz_common_init(MachineState *machine) SpitzMachineState *sms = SPITZ_MACHINE(machine); enum spitz_model_e model = smc->model; PXA2xxState *mpu; - DeviceState *scp0, *scp1 = NULL; MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *rom = g_new(MemoryRegion, 1); @@ -XXX,XX +XXX,XX @@ static void spitz_common_init(MachineState *machine) spitz_ssp_attach(sms); - scp0 = sysbus_create_simple("scoop", 0x10800000, NULL); + sms->scp0 = sysbus_create_simple("scoop", 0x10800000, NULL); if (model != akita) { - scp1 = sysbus_create_simple("scoop", 0x08800040, NULL); + sms->scp1 = sysbus_create_simple("scoop", 0x08800040, NULL); + } else { + sms->scp1 = NULL; } - spitz_scoop_gpio_setup(mpu, scp0, scp1); + spitz_scoop_gpio_setup(sms); spitz_gpio_setup(mpu, (model == akita) ? 1 : 2); -- 2.20.1 Currently the Spitz board uses a nasty hack for the GPIO lines that pass "bit5" and "power" information to the LCD controller: the lcdtg realize function sets a global variable to point to the instance it just realized, and then the functions spitz_bl_power() and spitz_bl_bit5() use that to find the device they are changing the internal state of. There is a comment reading: FIXME: Implement GPIO properly and remove this hack. which was added in 2009. Implement GPIO properly and remove this hack. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-6-peter.maydell@linaro.org --- hw/arm/spitz.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ static void spitz_bl_update(SpitzLCDTG *s) zaurus_printf("LCD Backlight now off\n"); } -/* FIXME: Implement GPIO properly and remove this hack. */ -static SpitzLCDTG *spitz_lcdtg; - static inline void spitz_bl_bit5(void *opaque, int line, int level) { - SpitzLCDTG *s = spitz_lcdtg; + SpitzLCDTG *s = opaque; int prev = s->bl_intensity; if (level) @@ -XXX,XX +XXX,XX @@ static inline void spitz_bl_bit5(void *opaque, int line, int level) static inline void spitz_bl_power(void *opaque, int line, int level) { - SpitzLCDTG *s = spitz_lcdtg; + SpitzLCDTG *s = opaque; s->bl_power = !!level; spitz_bl_update(s); } @@ -XXX,XX +XXX,XX @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value) return 0; } -static void spitz_lcdtg_realize(SSISlave *dev, Error **errp) +static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp) { - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev); + SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi); + DeviceState *dev = DEVICE(s); - spitz_lcdtg = s; s->bl_power = 0; s->bl_intensity = 0x20; + + qdev_init_gpio_in_named(dev, spitz_bl_bit5, "bl_bit5", 1); + qdev_init_gpio_in_named(dev, spitz_bl_power, "bl_power", 1); } /* SSP devices */ @@ -XXX,XX +XXX,XX @@ static void spitz_out_switch(void *opaque, int line, int level) case 3: zaurus_printf("Orange LED %s.\n", level ? "on" : "off"); break; - case 4: - spitz_bl_bit5(opaque, line, level); - break; - case 5: - spitz_bl_power(opaque, line, level); - break; case 6: spitz_adc_temp_on(opaque, line, level); break; + default: + g_assert_not_reached(); } } @@ -XXX,XX +XXX,XX @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms) if (sms->scp1) { qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT, - outsignals[4]); + qdev_get_gpio_in_named(sms->lcdtg, "bl_bit5", 0)); qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON, - outsignals[5]); + qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0)); } qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]); -- 2.20.1 Add some QOM properties to the max111x ADC device to allow the initial values to be configured. Currently this is done by board code calling max111x_set_input() after it creates the device, which doesn't work on system reset. This requires us to implement a reset method for this device, so while we're doing that make sure we reset the other parts of the device state. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-7-peter.maydell@linaro.org --- hw/misc/max111x.c | 57 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/max111x.c +++ b/hw/misc/max111x.c @@ -XXX,XX +XXX,XX @@ #include "hw/ssi/ssi.h" #include "migration/vmstate.h" #include "qemu/module.h" +#include "hw/qdev-properties.h" typedef struct { SSISlave parent_obj; qemu_irq interrupt; + /* Values of inputs at system reset (settable by QOM property) */ + uint8_t reset_input[8]; + uint8_t tb1, rb2, rb3; int cycle; @@ -XXX,XX +XXX,XX @@ static int max111x_init(SSISlave *d, int inputs) qdev_init_gpio_out(dev, &s->interrupt, 1); s->inputs = inputs; - /* TODO: add a user interface for setting these */ - s->input[0] = 0xf0; - s->input[1] = 0xe0; - s->input[2] = 0xd0; - s->input[3] = 0xc0; - s->input[4] = 0xb0; - s->input[5] = 0xa0; - s->input[6] = 0x90; - s->input[7] = 0x80; - s->com = 0; vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY, &vmstate_max111x, s); @@ -XXX,XX +XXX,XX @@ void max111x_set_input(DeviceState *dev, int line, uint8_t value) s->input[line] = value; } +static void max111x_reset(DeviceState *dev) +{ + MAX111xState *s = MAX_111X(dev); + int i; + + for (i = 0; i < s->inputs; i++) { + s->input[i] = s->reset_input[i]; + } + s->com = 0; + s->tb1 = 0; + s->rb2 = 0; + s->rb3 = 0; + s->cycle = 0; +} + +static Property max1110_properties[] = { + /* Reset values for ADC inputs */ + DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0), + DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0), + DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0), + DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0), + DEFINE_PROP_END_OF_LIST(), +}; + +static Property max1111_properties[] = { + /* Reset values for ADC inputs */ + DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0), + DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0), + DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0), + DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0), + DEFINE_PROP_UINT8("input4", MAX111xState, reset_input[4], 0xb0), + DEFINE_PROP_UINT8("input5", MAX111xState, reset_input[5], 0xa0), + DEFINE_PROP_UINT8("input6", MAX111xState, reset_input[6], 0x90), + DEFINE_PROP_UINT8("input7", MAX111xState, reset_input[7], 0x80), + DEFINE_PROP_END_OF_LIST(), +}; + static void max111x_class_init(ObjectClass *klass, void *data) { SSISlaveClass *k = SSI_SLAVE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); k->transfer = max111x_transfer; + dc->reset = max111x_reset; } static const TypeInfo max111x_info = { @@ -XXX,XX +XXX,XX @@ static const TypeInfo max111x_info = { static void max1110_class_init(ObjectClass *klass, void *data) { SSISlaveClass *k = SSI_SLAVE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); k->realize = max1110_realize; + device_class_set_props(dc, max1110_properties); } static const TypeInfo max1110_info = { @@ -XXX,XX +XXX,XX @@ static const TypeInfo max1110_info = { static void max1111_class_init(ObjectClass *klass, void *data) { SSISlaveClass *k = SSI_SLAVE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); k->realize = max1111_realize; + device_class_set_props(dc, max1111_properties); } static const TypeInfo max1111_info = { -- 2.20.1 The max111x is a proper qdev device; we can use dc->vmsd rather than directly calling vmstate_register(). It's possible that this is a migration compat break, but the only boards that use this device are the spitz-family ('akita', 'borzoi', 'spitz', 'terrier'). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-8-peter.maydell@linaro.org --- hw/misc/max111x.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/max111x.c +++ b/hw/misc/max111x.c @@ -XXX,XX +XXX,XX @@ static int max111x_init(SSISlave *d, int inputs) s->inputs = inputs; - vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY, - &vmstate_max111x, s); return 0; } @@ -XXX,XX +XXX,XX @@ static void max111x_class_init(ObjectClass *klass, void *data) k->transfer = max111x_transfer; dc->reset = max111x_reset; + dc->vmsd = &vmstate_max111x; } static const TypeInfo max111x_info = { -- 2.20.1 Add an ssi_realize_and_unref(), for the benefit of callers who want to be able to create an SSI device, set QOM properties on it, and then do the realize-and-unref afterwards. The API works on the same principle as the recently added qdev_realize_and_undef(), sysbus_realize_and_undef(), etc. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-9-peter.maydell@linaro.org --- include/hw/ssi/ssi.h | 26 ++++++++++++++++++++++++++ hw/ssi/ssi.c | 7 ++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/ssi.h +++ b/include/hw/ssi/ssi.h @@ -XXX,XX +XXX,XX @@ extern const VMStateDescription vmstate_ssi_slave; } DeviceState *ssi_create_slave(SSIBus *bus, const char *name); +/** + * ssi_realize_and_unref: realize and unref an SSI slave device + * @dev: SSI slave device to realize + * @bus: SSI bus to put it on + * @errp: error pointer + * + * Call 'realize' on @dev, put it on the specified @bus, and drop the + * reference to it. Errors are reported via @errp and by returning + * false. + * + * This function is useful if you have created @dev via qdev_new() + * (which takes a reference to the device it returns to you), so that + * you can set properties on it before realizing it. If you don't need + * to set properties then ssi_create_slave() is probably better (as it + * does the create, init and realize in one step). + * + * If you are embedding the SSI slave into another QOM device and + * initialized it via some variant on object_initialize_child() then + * do not use this function, because that family of functions arrange + * for the only reference to the child device to be held by the parent + * via the child<> property, and so the reference-count-drop done here + * would be incorrect. (Instead you would want ssi_realize(), which + * doesn't currently exist but would be trivial to create if we had + * any code that wanted it.) + */ +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp); /* Master interface. */ SSIBus *ssi_create_bus(DeviceState *parent, const char *name); diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/ssi.c +++ b/hw/ssi/ssi.c @@ -XXX,XX +XXX,XX @@ static const TypeInfo ssi_slave_info = { .abstract = true, }; +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp) +{ + return qdev_realize_and_unref(dev, &bus->parent_obj, errp); +} + DeviceState *ssi_create_slave(SSIBus *bus, const char *name) { DeviceState *dev = qdev_new(name); - qdev_realize_and_unref(dev, &bus->parent_obj, &error_fatal); + ssi_realize_and_unref(dev, bus, &error_fatal); return dev; } -- 2.20.1 Use the new max111x qdev properties to set the initial input values rather than calling max111x_set_input(); this means that on system reset the inputs will correctly return to their initial values. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200628142429.17111-10-peter.maydell@linaro.org --- hw/arm/spitz.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ static void spitz_ssp_attach(SpitzMachineState *sms) qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT)); bus = qdev_get_child_bus(sms->mux, "ssi2"); - sms->max1111 = ssi_create_slave(bus, "max1111"); + sms->max1111 = qdev_new("max1111"); max1111 = sms->max1111; - max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT); - max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0); - max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN); + qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */, + SPITZ_BATTERY_VOLT); + qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0); + qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */, + SPITZ_CHARGEON_ACIN); + ssi_realize_and_unref(sms->max1111, bus, &error_fatal); qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS, qdev_get_gpio_in(sms->mux, 0)); -- 2.20.1 The max111x ADC device model allows other code to set the level on the 8 ADC inputs using the max111x_set_input() function. Replace this with generic qdev GPIO inputs, which also allow inputs to be set to arbitrary values. Using GPIO lines will make it easier for board code to wire things up, so that if device A wants to set the ADC input it doesn't need to have a direct pointer to the max111x but can just set that value on its output GPIO, which is then wired up by the board to the appropriate max111x input. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-11-peter.maydell@linaro.org --- include/hw/ssi/ssi.h | 3 --- hw/arm/spitz.c | 9 +++++---- hw/misc/max111x.c | 16 +++++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/ssi.h +++ b/include/hw/ssi/ssi.h @@ -XXX,XX +XXX,XX @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name); uint32_t ssi_transfer(SSIBus *bus, uint32_t val); -/* max111x.c */ -void max111x_set_input(DeviceState *dev, int line, uint8_t value); - #endif diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level) static void spitz_adc_temp_on(void *opaque, int line, int level) { + int batt_temp; + if (!max1111) return; - if (level) - max111x_set_input(max1111, MAX1111_BATT_TEMP, SPITZ_BATTERY_TEMP); - else - max111x_set_input(max1111, MAX1111_BATT_TEMP, 0); + batt_temp = level ? SPITZ_BATTERY_TEMP : 0; + + qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp); } static void corgi_ssp_realize(SSISlave *d, Error **errp) diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/max111x.c +++ b/hw/misc/max111x.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_max111x = { } }; +static void max111x_input_set(void *opaque, int line, int value) +{ + MAX111xState *s = MAX_111X(opaque); + + assert(line >= 0 && line < s->inputs); + s->input[line] = value; +} + static int max111x_init(SSISlave *d, int inputs) { DeviceState *dev = DEVICE(d); MAX111xState *s = MAX_111X(dev); qdev_init_gpio_out(dev, &s->interrupt, 1); + qdev_init_gpio_in(dev, max111x_input_set, inputs); s->inputs = inputs; @@ -XXX,XX +XXX,XX @@ static void max1111_realize(SSISlave *dev, Error **errp) max111x_init(dev, 4); } -void max111x_set_input(DeviceState *dev, int line, uint8_t value) -{ - MAX111xState *s = MAX_111X(dev); - assert(line >= 0 && line < s->inputs); - s->input[line] = value; -} - static void max111x_reset(DeviceState *dev) { MAX111xState *s = MAX_111X(dev); -- 2.20.1 Create a header file for the hw/misc/max111x device, in the usual modern style for QOM devices: * definition of the TYPE_ constants and macros * definition of the device's state struct so that it can be embedded in other structs if desired * documentation of the interface This allows us to use TYPE_MAX_1111 in the spitz.c code rather than the string "max1111". Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200628142429.17111-12-peter.maydell@linaro.org --- include/hw/misc/max111x.h | 56 +++++++++++++++++++++++++++++++++++++++ hw/arm/spitz.c | 3 ++- hw/misc/max111x.c | 24 +---------------- MAINTAINERS | 1 + 4 files changed, 60 insertions(+), 24 deletions(-) create mode 100644 include/hw/misc/max111x.h diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/include/hw/misc/max111x.h @@ -XXX,XX +XXX,XX @@ +/* + * Maxim MAX1110/1111 ADC chip emulation. + * + * Copyright (c) 2006 Openedhand Ltd. + * Written by Andrzej Zaborowski <balrog@zabor.org> + * + * This code is licensed under the GNU GPLv2. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + */ + +#ifndef HW_MISC_MAX111X_H +#define HW_MISC_MAX111X_H + +#include "hw/ssi/ssi.h" + +/* + * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU + * is an SSI slave device. It has either 4 (max1110) or 8 (max1111) + * 8-bit ADC channels. + * + * QEMU interface: + * + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value + * of each ADC input, as an unsigned 8-bit value + * + GPIO output 0: interrupt line + * + Properties "input0" to "input3" (max1110) or "input0" to "input7" + * (max1111): initial reset values for ADC inputs. + * + * Known bugs: + * + the interrupt line is not correctly implemented, and will never + * be lowered once it has been asserted. + */ +typedef struct { + SSISlave parent_obj; + + qemu_irq interrupt; + /* Values of inputs at system reset (settable by QOM property) */ + uint8_t reset_input[8]; + + uint8_t tb1, rb2, rb3; + int cycle; + + uint8_t input[8]; + int inputs, com; +} MAX111xState; + +#define TYPE_MAX_111X "max111x" + +#define MAX_111X(obj) \ + OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X) + +#define TYPE_MAX_1110 "max1110" +#define TYPE_MAX_1111 "max1111" + +#endif diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ #include "audio/audio.h" #include "hw/boards.h" #include "hw/sysbus.h" +#include "hw/misc/max111x.h" #include "migration/vmstate.h" #include "exec/address-spaces.h" #include "cpu.h" @@ -XXX,XX +XXX,XX @@ static void spitz_ssp_attach(SpitzMachineState *sms) qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT)); bus = qdev_get_child_bus(sms->mux, "ssi2"); - sms->max1111 = qdev_new("max1111"); + sms->max1111 = qdev_new(TYPE_MAX_1111); max1111 = sms->max1111; qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */, SPITZ_BATTERY_VOLT); diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/max111x.c +++ b/hw/misc/max111x.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "hw/misc/max111x.h" #include "hw/irq.h" -#include "hw/ssi/ssi.h" #include "migration/vmstate.h" #include "qemu/module.h" #include "hw/qdev-properties.h" -typedef struct { - SSISlave parent_obj; - - qemu_irq interrupt; - /* Values of inputs at system reset (settable by QOM property) */ - uint8_t reset_input[8]; - - uint8_t tb1, rb2, rb3; - int cycle; - - uint8_t input[8]; - int inputs, com; -} MAX111xState; - -#define TYPE_MAX_111X "max111x" - -#define MAX_111X(obj) \ - OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X) - -#define TYPE_MAX_1110 "max1110" -#define TYPE_MAX_1111 "max1111" - /* Control-byte bitfields */ #define CB_PD0 (1 << 0) #define CB_PD1 (1 << 1) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: hw/gpio/max7310.c F: hw/gpio/zaurus.c F: hw/misc/mst_fpga.c F: hw/misc/max111x.c +F: include/hw/misc/max111x.h F: include/hw/arm/pxa.h F: include/hw/arm/sharpsl.h F: include/hw/display/tc6393xb.h -- 2.20.1 Currently we have a free-floating set of IRQs and a function spitz_out_switch() which handle some miscellaneous GPIO lines for the spitz board. Encapsulate this behaviour in a simple QOM device. At this point we can finally remove the 'max1111' global, because the ADC battery-temperature value is now handled by the misc-gpio device writing the value to its outbound "adc-temp" GPIO, which the board code wires up to the appropriate inbound GPIO line on the max1111. This commit also fixes Coverity issue CID 1421913 (which pointed out that the 'outsignals' in spitz_scoop_gpio_setup() were leaked), because it removes the use of the qemu_allocate_irqs() API from this code entirely. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-13-peter.maydell@linaro.org --- hw/arm/spitz.c | 129 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 87 insertions(+), 42 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ typedef struct { DeviceState *max1111; DeviceState *scp0; DeviceState *scp1; + DeviceState *misc_gpio; } SpitzMachineState; #define TYPE_SPITZ_MACHINE "spitz-common" @@ -XXX,XX +XXX,XX @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp) #define SPITZ_GPIO_MAX1111_CS 20 #define SPITZ_GPIO_TP_INT 11 -static DeviceState *max1111; - /* "Demux" the signal based on current chipselect */ typedef struct { SSISlave ssidev; @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level) #define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */ #define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */ -static void spitz_adc_temp_on(void *opaque, int line, int level) -{ - int batt_temp; - - if (!max1111) - return; - - batt_temp = level ? SPITZ_BATTERY_TEMP : 0; - - qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp); -} - static void corgi_ssp_realize(SSISlave *d, Error **errp) { DeviceState *dev = DEVICE(d); @@ -XXX,XX +XXX,XX @@ static void spitz_ssp_attach(SpitzMachineState *sms) bus = qdev_get_child_bus(sms->mux, "ssi2"); sms->max1111 = qdev_new(TYPE_MAX_1111); - max1111 = sms->max1111; qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */, SPITZ_BATTERY_VOLT); qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0); @@ -XXX,XX +XXX,XX @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu) /* Other peripherals */ -static void spitz_out_switch(void *opaque, int line, int level) +/* + * Encapsulation of some miscellaneous GPIO line behaviour for the Spitz boards. + * + * QEMU interface: + * + named GPIO inputs "green-led", "orange-led", "charging", "discharging": + * these currently just print messages that the line has been signalled + * + named GPIO input "adc-temp-on": set to cause the battery-temperature + * value to be passed to the max111x ADC + * + named GPIO output "adc-temp": the ADC value, to be wired up to the max111x + */ +#define TYPE_SPITZ_MISC_GPIO "spitz-misc-gpio" +#define SPITZ_MISC_GPIO(obj) \ + OBJECT_CHECK(SpitzMiscGPIOState, (obj), TYPE_SPITZ_MISC_GPIO) + +typedef struct SpitzMiscGPIOState { + SysBusDevice parent_obj; + + qemu_irq adc_value; +} SpitzMiscGPIOState; + +static void spitz_misc_charging(void *opaque, int n, int level) { - switch (line) { - case 0: - zaurus_printf("Charging %s.\n", level ? "off" : "on"); - break; - case 1: - zaurus_printf("Discharging %s.\n", level ? "on" : "off"); - break; - case 2: - zaurus_printf("Green LED %s.\n", level ? "on" : "off"); - break; - case 3: - zaurus_printf("Orange LED %s.\n", level ? "on" : "off"); - break; - case 6: - spitz_adc_temp_on(opaque, line, level); - break; - default: - g_assert_not_reached(); - } + zaurus_printf("Charging %s.\n", level ? "off" : "on"); +} + +static void spitz_misc_discharging(void *opaque, int n, int level) +{ + zaurus_printf("Discharging %s.\n", level ? "off" : "on"); +} + +static void spitz_misc_green_led(void *opaque, int n, int level) +{ + zaurus_printf("Green LED %s.\n", level ? "off" : "on"); +} + +static void spitz_misc_orange_led(void *opaque, int n, int level) +{ + zaurus_printf("Orange LED %s.\n", level ? "off" : "on"); +} + +static void spitz_misc_adc_temp(void *opaque, int n, int level) +{ + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(opaque); + int batt_temp = level ? SPITZ_BATTERY_TEMP : 0; + + qemu_set_irq(s->adc_value, batt_temp); +} + +static void spitz_misc_gpio_init(Object *obj) +{ + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(obj); + DeviceState *dev = DEVICE(obj); + + qdev_init_gpio_in_named(dev, spitz_misc_charging, "charging", 1); + qdev_init_gpio_in_named(dev, spitz_misc_discharging, "discharging", 1); + qdev_init_gpio_in_named(dev, spitz_misc_green_led, "green-led", 1); + qdev_init_gpio_in_named(dev, spitz_misc_orange_led, "orange-led", 1); + qdev_init_gpio_in_named(dev, spitz_misc_adc_temp, "adc-temp-on", 1); + + qdev_init_gpio_out_named(dev, &s->adc_value, "adc-temp", 1); } #define SPITZ_SCP_LED_GREEN 1 @@ -XXX,XX +XXX,XX @@ static void spitz_out_switch(void *opaque, int line, int level) static void spitz_scoop_gpio_setup(SpitzMachineState *sms) { - qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8); + DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL); - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]); - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]); - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]); - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]); + sms->misc_gpio = miscdev; + + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, + qdev_get_gpio_in_named(miscdev, "charging", 0)); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, + qdev_get_gpio_in_named(miscdev, "discharging", 0)); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, + qdev_get_gpio_in_named(miscdev, "green-led", 0)); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, + qdev_get_gpio_in_named(miscdev, "orange-led", 0)); + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, + qdev_get_gpio_in_named(miscdev, "adc-temp-on", 0)); + qdev_connect_gpio_out_named(miscdev, "adc-temp", 0, + qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP)); if (sms->scp1) { qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT, @@ -XXX,XX +XXX,XX @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms) qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON, qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0)); } - - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]); } #define SPITZ_GPIO_HSYNC 22 @@ -XXX,XX +XXX,XX @@ static const TypeInfo spitz_lcdtg_info = { .class_init = spitz_lcdtg_class_init, }; +static const TypeInfo spitz_misc_gpio_info = { + .name = TYPE_SPITZ_MISC_GPIO, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(SpitzMiscGPIOState), + .instance_init = spitz_misc_gpio_init, + /* + * No class_init required: device has no internal state so does not + * need to set up reset or vmstate, and does not have a realize method. + */ +}; + static void spitz_register_types(void) { type_register_static(&corgi_ssp_info); type_register_static(&spitz_lcdtg_info); type_register_static(&spitz_keyboard_info); type_register_static(&sl_nand_info); + type_register_static(&spitz_misc_gpio_info); } type_init(spitz_register_types) -- 2.20.1 Instead of logging guest accesses to invalid register offsets in this device using zaurus_printf() (which just prints to stderr), use the usual qemu_log_mask(LOG_GUEST_ERROR,...). Since this was the only use of the zaurus_printf() macro outside spitz.c, we can move the definition of that macro from sharpsl.h to spitz.c. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-14-peter.maydell@linaro.org --- include/hw/arm/sharpsl.h | 3 --- hw/arm/spitz.c | 3 +++ hw/gpio/zaurus.c | 12 +++++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/sharpsl.h +++ b/include/hw/arm/sharpsl.h @@ -XXX,XX +XXX,XX @@ #include "exec/hwaddr.h" -#define zaurus_printf(format, ...) \ - fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__) - /* zaurus.c */ #define SL_PXA_PARAM_BASE 0xa0000a00 diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ typedef struct { #define SPITZ_MACHINE_CLASS(klass) \ OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE) +#define zaurus_printf(format, ...) \ + fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__) + #undef REG_FMT #define REG_FMT "0x%02lx" diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/zaurus.c +++ b/hw/gpio/zaurus.c @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "migration/vmstate.h" #include "qemu/module.h" - -#undef REG_FMT -#define REG_FMT "0x%02lx" +#include "qemu/log.h" /* SCOOP devices */ @@ -XXX,XX +XXX,XX @@ static uint64_t scoop_read(void *opaque, hwaddr addr, case SCOOP_GPRR: return s->gpio_level; default: - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr); + qemu_log_mask(LOG_GUEST_ERROR, + "scoop_read: bad register offset 0x%02" HWADDR_PRIx "\n", + addr); } return 0; @@ -XXX,XX +XXX,XX @@ static void scoop_write(void *opaque, hwaddr addr, scoop_gpio_handler_update(s); break; default: - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr); + qemu_log_mask(LOG_GUEST_ERROR, + "scoop_write: bad register offset 0x%02" HWADDR_PRIx "\n", + addr); } } -- 2.20.1 Instead of logging guest accesses to invalid register offsets in the Spitz flash device with zaurus_printf() (which just prints to stderr), use the usual qemu_log_mask(LOG_GUEST_ERROR,...). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-15-peter.maydell@linaro.org --- hw/arm/spitz.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ #include "hw/ssi/ssi.h" #include "hw/block/flash.h" #include "qemu/timer.h" +#include "qemu/log.h" #include "hw/arm/sharpsl.h" #include "ui/console.h" #include "hw/audio/wm8750.h" @@ -XXX,XX +XXX,XX @@ typedef struct { #define zaurus_printf(format, ...) \ fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__) -#undef REG_FMT -#define REG_FMT "0x%02lx" - /* Spitz Flash */ #define FLASH_BASE 0x0c000000 #define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */ @@ -XXX,XX +XXX,XX @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size) return ecc_digest(&s->ecc, nand_getio(s->nand)); default: - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr); + qemu_log_mask(LOG_GUEST_ERROR, + "sl_read: bad register offset 0x%02" HWADDR_PRIx "\n", + addr); } return 0; } @@ -XXX,XX +XXX,XX @@ static void sl_write(void *opaque, hwaddr addr, break; default: - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr); + qemu_log_mask(LOG_GUEST_ERROR, + "sl_write: bad register offset 0x%02" HWADDR_PRIx "\n", + addr); } } -- 2.20.1 Instead of using printf() for logging guest accesses to invalid register offsets in the pxa2xx PIC device, use the usual qemu_log_mask(LOG_GUEST_ERROR,...). This was the only user of the REG_FMT macro in pxa.h, so we can remove that. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-16-peter.maydell@linaro.org --- include/hw/arm/pxa.h | 1 - hw/arm/pxa2xx_pic.c | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/pxa.h +++ b/include/hw/arm/pxa.h @@ -XXX,XX +XXX,XX @@ struct PXA2xxI2SState { }; # define PA_FMT "0x%08lx" -# define REG_FMT "0x" TARGET_FMT_plx PXA2xxState *pxa270_init(MemoryRegion *address_space, unsigned int sdram_size, const char *revision); diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/pxa2xx_pic.c +++ b/hw/arm/pxa2xx_pic.c @@ -XXX,XX +XXX,XX @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/module.h" +#include "qemu/log.h" #include "cpu.h" #include "hw/arm/pxa.h" #include "hw/sysbus.h" @@ -XXX,XX +XXX,XX @@ static uint64_t pxa2xx_pic_mem_read(void *opaque, hwaddr offset, case ICHP: /* Highest Priority register */ return pxa2xx_pic_highest(s); default: - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset); + qemu_log_mask(LOG_GUEST_ERROR, + "pxa2xx_pic_mem_read: bad register offset 0x%" HWADDR_PRIx + "\n", offset); return 0; } } @@ -XXX,XX +XXX,XX @@ static void pxa2xx_pic_mem_write(void *opaque, hwaddr offset, s->priority[32 + ((offset - IPR32) >> 2)] = value & 0x8000003f; break; default: - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset); + qemu_log_mask(LOG_GUEST_ERROR, + "pxa2xx_pic_mem_write: bad register offset 0x%" + HWADDR_PRIx "\n", offset); return; } pxa2xx_pic_update(opaque); -- 2.20.1 The QOM types "spitz-lcdtg" and "corgi-ssp" are missing the usual QOM TYPE and casting macros; provide and use them. In particular, we can safely use the QOM cast macros instead of FROM_SSI_SLAVE() because in both cases the 'ssidev' field of the instance state struct is the first field in it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-17-peter.maydell@linaro.org --- hw/arm/spitz.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp) #define LCDTG_PICTRL 0x06 #define LCDTG_POLCTRL 0x07 +#define TYPE_SPITZ_LCDTG "spitz-lcdtg" +#define SPITZ_LCDTG(obj) OBJECT_CHECK(SpitzLCDTG, (obj), TYPE_SPITZ_LCDTG) + typedef struct { SSISlave ssidev; uint32_t bl_intensity; @@ -XXX,XX +XXX,XX @@ static inline void spitz_bl_power(void *opaque, int line, int level) static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value) { - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev); + SpitzLCDTG *s = SPITZ_LCDTG(dev); int addr; addr = value >> 5; value &= 0x1f; @@ -XXX,XX +XXX,XX @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value) static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp) { - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi); + SpitzLCDTG *s = SPITZ_LCDTG(ssi); DeviceState *dev = DEVICE(s); s->bl_power = 0; @@ -XXX,XX +XXX,XX @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp) #define SPITZ_GPIO_MAX1111_CS 20 #define SPITZ_GPIO_TP_INT 11 +#define TYPE_CORGI_SSP "corgi-ssp" +#define CORGI_SSP(obj) OBJECT_CHECK(CorgiSSPState, (obj), TYPE_CORGI_SSP) + /* "Demux" the signal based on current chipselect */ typedef struct { SSISlave ssidev; @@ -XXX,XX +XXX,XX @@ typedef struct { static uint32_t corgi_ssp_transfer(SSISlave *dev, uint32_t value) { - CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, dev); + CorgiSSPState *s = CORGI_SSP(dev); int i; for (i = 0; i < 3; i++) { @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level) static void corgi_ssp_realize(SSISlave *d, Error **errp) { DeviceState *dev = DEVICE(d); - CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, d); + CorgiSSPState *s = CORGI_SSP(d); qdev_init_gpio_in(dev, corgi_ssp_gpio_cs, 3); s->bus[0] = ssi_create_bus(dev, "ssi0"); @@ -XXX,XX +XXX,XX @@ static void spitz_ssp_attach(SpitzMachineState *sms) { void *bus; - sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp"); + sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], + TYPE_CORGI_SSP); bus = qdev_get_child_bus(sms->mux, "ssi0"); - sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg"); + sms->lcdtg = ssi_create_slave(bus, TYPE_SPITZ_LCDTG); bus = qdev_get_child_bus(sms->mux, "ssi1"); sms->ads7846 = ssi_create_slave(bus, "ads7846"); @@ -XXX,XX +XXX,XX @@ static void corgi_ssp_class_init(ObjectClass *klass, void *data) } static const TypeInfo corgi_ssp_info = { - .name = "corgi-ssp", + .name = TYPE_CORGI_SSP, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(CorgiSSPState), .class_init = corgi_ssp_class_init, @@ -XXX,XX +XXX,XX @@ static void spitz_lcdtg_class_init(ObjectClass *klass, void *data) } static const TypeInfo spitz_lcdtg_info = { - .name = "spitz-lcdtg", + .name = TYPE_SPITZ_LCDTG, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(SpitzLCDTG), .class_init = spitz_lcdtg_class_init, -- 2.20.1 The FROM_SSI_SLAVE() macro predates QOM and is used as a typesafe way to cast from an SSISlave* to the instance struct of a subtype of TYPE_SSI_SLAVE. Switch to using the QOM cast macros instead, which have the same effect (by writing the QOM macros if the types were previously missing them.) (The FROM_SSI_SLAVE() macro allows the SSISlave member of the subtype's struct to be anywhere as long as it is named "ssidev", whereas a QOM cast macro insists that it is the first thing in the subtype's struct. This is true for all the types we convert here.) This removes all the uses of FROM_SSI_SLAVE() so we can delete the definition. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-id: 20200628142429.17111-18-peter.maydell@linaro.org --- include/hw/ssi/ssi.h | 2 -- hw/arm/z2.c | 11 +++++++---- hw/display/ads7846.c | 9 ++++++--- hw/display/ssd0323.c | 10 +++++++--- hw/sd/ssi-sd.c | 4 ++-- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/ssi.h +++ b/include/hw/ssi/ssi.h @@ -XXX,XX +XXX,XX @@ struct SSISlave { bool cs; }; -#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev) - extern const VMStateDescription vmstate_ssi_slave; #define VMSTATE_SSI_SLAVE(_field, _state) { \ diff --git a/hw/arm/z2.c b/hw/arm/z2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/z2.c +++ b/hw/arm/z2.c @@ -XXX,XX +XXX,XX @@ typedef struct { int pos; } ZipitLCD; +#define TYPE_ZIPIT_LCD "zipit-lcd" +#define ZIPIT_LCD(obj) OBJECT_CHECK(ZipitLCD, (obj), TYPE_ZIPIT_LCD) + static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value) { - ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev); + ZipitLCD *z = ZIPIT_LCD(dev); uint16_t val; if (z->selected) { z->buf[z->pos] = value & 0xff; @@ -XXX,XX +XXX,XX @@ static void z2_lcd_cs(void *opaque, int line, int level) static void zipit_lcd_realize(SSISlave *dev, Error **errp) { - ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev); + ZipitLCD *z = ZIPIT_LCD(dev); z->selected = 0; z->enabled = 0; z->pos = 0; @@ -XXX,XX +XXX,XX @@ static void zipit_lcd_class_init(ObjectClass *klass, void *data) } static const TypeInfo zipit_lcd_info = { - .name = "zipit-lcd", + .name = TYPE_ZIPIT_LCD, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(ZipitLCD), .class_init = zipit_lcd_class_init, @@ -XXX,XX +XXX,XX @@ static void z2_init(MachineState *machine) type_register_static(&zipit_lcd_info); type_register_static(&aer915_info); - z2_lcd = ssi_create_slave(mpu->ssp[1], "zipit-lcd"); + z2_lcd = ssi_create_slave(mpu->ssp[1], TYPE_ZIPIT_LCD); bus = pxa2xx_i2c_bus(mpu->i2c[0]); i2c_create_slave(bus, TYPE_AER915, 0x55); wm = i2c_create_slave(bus, TYPE_WM8750, 0x1b); diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c index XXXXXXX..XXXXXXX 100644 --- a/hw/display/ads7846.c +++ b/hw/display/ads7846.c @@ -XXX,XX +XXX,XX @@ typedef struct { int output; } ADS7846State; +#define TYPE_ADS7846 "ads7846" +#define ADS7846(obj) OBJECT_CHECK(ADS7846State, (obj), TYPE_ADS7846) + /* Control-byte bitfields */ #define CB_PD0 (1 << 0) #define CB_PD1 (1 << 1) @@ -XXX,XX +XXX,XX @@ static void ads7846_int_update(ADS7846State *s) static uint32_t ads7846_transfer(SSISlave *dev, uint32_t value) { - ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, dev); + ADS7846State *s = ADS7846(dev); switch (s->cycle ++) { case 0: @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_ads7846 = { static void ads7846_realize(SSISlave *d, Error **errp) { DeviceState *dev = DEVICE(d); - ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, d); + ADS7846State *s = ADS7846(d); qdev_init_gpio_out(dev, &s->interrupt, 1); @@ -XXX,XX +XXX,XX @@ static void ads7846_class_init(ObjectClass *klass, void *data) } static const TypeInfo ads7846_info = { - .name = "ads7846", + .name = TYPE_ADS7846, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(ADS7846State), .class_init = ads7846_class_init, diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c index XXXXXXX..XXXXXXX 100644 --- a/hw/display/ssd0323.c +++ b/hw/display/ssd0323.c @@ -XXX,XX +XXX,XX @@ typedef struct { uint8_t framebuffer[128 * 80 / 2]; } ssd0323_state; +#define TYPE_SSD0323 "ssd0323" +#define SSD0323(obj) OBJECT_CHECK(ssd0323_state, (obj), TYPE_SSD0323) + + static uint32_t ssd0323_transfer(SSISlave *dev, uint32_t data) { - ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, dev); + ssd0323_state *s = SSD0323(dev); switch (s->mode) { case SSD0323_DATA: @@ -XXX,XX +XXX,XX @@ static const GraphicHwOps ssd0323_ops = { static void ssd0323_realize(SSISlave *d, Error **errp) { DeviceState *dev = DEVICE(d); - ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, d); + ssd0323_state *s = SSD0323(d); s->col_end = 63; s->row_end = 79; @@ -XXX,XX +XXX,XX @@ static void ssd0323_class_init(ObjectClass *klass, void *data) } static const TypeInfo ssd0323_info = { - .name = "ssd0323", + .name = TYPE_SSD0323, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(ssd0323_state), .class_init = ssd0323_class_init, diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index XXXXXXX..XXXXXXX 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -XXX,XX +XXX,XX @@ typedef struct { static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val) { - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev); + ssi_sd_state *s = SSI_SD(dev); /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data. */ if (s->mode == SSI_SD_DATA_READ && val == 0x4d) { @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_ssi_sd = { static void ssi_sd_realize(SSISlave *d, Error **errp) { - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d); + ssi_sd_state *s = SSI_SD(d); DeviceState *carddev; DriveInfo *dinfo; Error *err = NULL; -- 2.20.1 Deprecate our TileGX target support: * we have no active maintainer for it * it has had essentially no contributions (other than tree-wide cleanups and similar) since it was first added * the Linux kernel dropped support in 2018, as has glibc Note the deprecation in the manual, but don't try to print a warning when QEMU runs -- printing unsuppressable messages is more obtrusive for linux-user mode than it would be for system-emulation mode, and it doesn't seem worth trying to invent a new suppressible-error system for linux-user just for this. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20200619154831.26319-1-peter.maydell@linaro.org --- docs/system/deprecated.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -XXX,XX +XXX,XX @@ The above, converted to the current supported format:: json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"} +linux-user mode CPUs +-------------------- + +``tilegx`` CPUs (since 5.1.0) +''''''''''''''''''''''''''''' + +The ``tilegx`` guest CPU support (which was only implemented in +linux-user mode) is deprecated and will be removed in a future version +of QEMU. Support for this CPU was removed from the upstream Linux +kernel in 2018, and has also been dropped from glibc. + Related binaries ---------------- -- 2.20.1