* [PULL 01/38] hw/arm/virt: KVM: Enable PAuth when supported by the host
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 02/38] hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/ Peter Maydell
` (36 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
Add basic support for Pointer Authentication when running a KVM
guest and that the host supports it, loosely based on the SVE
support.
Although the feature is enabled by default when the host advertises
it, it is possible to disable it by setting the 'pauth=off' CPU
property. The 'pauth' comment is removed from cpu-features.rst,
as it is now common to both TCG and KVM.
Tested on an Apple M1 running 5.16-rc6.
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220107150154.2490308-1-maz@kernel.org
[PMM: fixed indentation]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
docs/system/arm/cpu-features.rst | 4 ----
target/arm/cpu.h | 1 +
target/arm/cpu.c | 16 +++++-----------
target/arm/cpu64.c | 31 +++++++++++++++++++++++++++----
target/arm/kvm64.c | 21 +++++++++++++++++++++
5 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index 584eb170974..3e626c4b68a 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -217,10 +217,6 @@ TCG VCPU Features
TCG VCPU features are CPU features that are specific to TCG.
Below is the list of TCG VCPU features and their descriptions.
- pauth Enable or disable ``FEAT_Pauth``, pointer
- authentication. By default, the feature is
- enabled with ``-cpu max``.
-
pauth-impdef When ``FEAT_Pauth`` is enabled, either the
*impdef* (Implementation Defined) algorithm
is enabled or the *architected* QARMA algorithm
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e33f37b70ad..c6a4d50e821 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1076,6 +1076,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
void aarch64_sve_change_el(CPUARMState *env, int old_el,
int new_el, bool el0_a64);
void aarch64_add_sve_properties(Object *obj);
+void aarch64_add_pauth_properties(Object *obj);
/*
* SVE registers are encoded in KVM's memory in an endianness-invariant format.
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a211804fd3d..cdbc4cdd012 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
return;
}
- /*
- * KVM does not support modifications to this feature.
- * We have not registered the cpu properties when KVM
- * is in use, so the user will not be able to set them.
- */
- if (!kvm_enabled()) {
- arm_cpu_pauth_finalize(cpu, &local_err);
- if (local_err != NULL) {
- error_propagate(errp, local_err);
- return;
- }
+ arm_cpu_pauth_finalize(cpu, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
}
}
@@ -2091,6 +2084,7 @@ static void arm_host_initfn(Object *obj)
kvm_arm_set_cpu_features_from_host(cpu);
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
aarch64_add_sve_properties(obj);
+ aarch64_add_pauth_properties(obj);
}
#else
hvf_arm_set_cpu_features_from_host(cpu);
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 15245a60a8c..8786be7783e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -630,6 +630,15 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
int arch_val = 0, impdef_val = 0;
uint64_t t;
+ /* Exit early if PAuth is enabled, and fall through to disable it */
+ if (kvm_enabled() && cpu->prop_pauth) {
+ if (!cpu_isar_feature(aa64_pauth, cpu)) {
+ error_setg(errp, "'pauth' feature not supported by KVM on this host");
+ }
+
+ return;
+ }
+
/* TODO: Handle HaveEnhancedPAC, HaveEnhancedPAC2, HaveFPAC. */
if (cpu->prop_pauth) {
if (cpu->prop_pauth_impdef) {
@@ -655,6 +664,23 @@ static Property arm_cpu_pauth_property =
static Property arm_cpu_pauth_impdef_property =
DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false);
+void aarch64_add_pauth_properties(Object *obj)
+{
+ ARMCPU *cpu = ARM_CPU(obj);
+
+ /* Default to PAUTH on, with the architected algorithm on TCG. */
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
+ if (kvm_enabled()) {
+ /*
+ * Mirror PAuth support from the probed sysregs back into the
+ * property for KVM. Is it just a bit backward? Yes it is!
+ */
+ cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
+ } else {
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+ }
+}
+
/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
* otherwise, a CPU with as many features enabled as our emulation supports.
* The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
@@ -829,13 +855,10 @@ static void aarch64_max_initfn(Object *obj)
cpu->dcz_blocksize = 7; /* 512 bytes */
#endif
- /* Default to PAUTH on, with the architected algorithm. */
- qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
- qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
-
bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
}
+ aarch64_add_pauth_properties(obj);
aarch64_add_sve_properties(obj);
object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
cpu_max_set_sve_max_vq, NULL, NULL);
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index e790d6c9a57..71c3ca69717 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -491,6 +491,12 @@ static int read_sys_reg64(int fd, uint64_t *pret, uint64_t id)
return ioctl(fd, KVM_GET_ONE_REG, &idreg);
}
+static bool kvm_arm_pauth_supported(void)
+{
+ return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
+ kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
+}
+
bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
{
/* Identify the feature bits corresponding to the host CPU, and
@@ -521,6 +527,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
*/
struct kvm_vcpu_init init = { .target = -1, };
+ /*
+ * Ask for Pointer Authentication if supported. We can't play the
+ * SVE trick of synthesising the ID reg as KVM won't tell us
+ * whether we have the architected or IMPDEF version of PAuth, so
+ * we have to use the actual ID regs.
+ */
+ if (kvm_arm_pauth_supported()) {
+ init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
+ 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
+ }
+
if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
return false;
}
@@ -865,6 +882,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
assert(kvm_arm_sve_supported());
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
}
+ if (cpu_isar_feature(aa64_pauth, cpu)) {
+ cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
+ 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
+ }
/* Do KVM_ARM_VCPU_INIT ioctl */
ret = kvm_arm_vcpu_init(cs);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 02/38] hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
2022-01-20 12:35 ` [PULL 01/38] hw/arm/virt: KVM: Enable PAuth when supported by the host Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 03/38] hw/arm/musicpal: Fix coding style of code related to MV88W8618 device Peter Maydell
` (35 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
The Marvell 88W8618 is a system-on-chip with an ARM core.
We implement its audio codecs and network interface.
Homogeneous SoC Kconfig are usually defined in the hw/$ARCH
directory. Move it there.
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220107184429.423572-2-f4bug@amsat.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/Kconfig | 3 +++
hw/audio/Kconfig | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e6525909438..c459c897cc7 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -94,6 +94,9 @@ config MUSCA
select SPLIT_IRQ
select UNIMP
+config MARVELL_88W8618
+ bool
+
config MUSICPAL
bool
select OR_IRQ
diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
index e9c6fed8261..e76c69ca7e7 100644
--- a/hw/audio/Kconfig
+++ b/hw/audio/Kconfig
@@ -47,6 +47,3 @@ config PL041
config CS4231
bool
-
-config MARVELL_88W8618
- bool
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 03/38] hw/arm/musicpal: Fix coding style of code related to MV88W8618 device
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
2022-01-20 12:35 ` [PULL 01/38] hw/arm/virt: KVM: Enable PAuth when supported by the host Peter Maydell
2022-01-20 12:35 ` [PULL 02/38] hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/ Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 04/38] hw/net: Move MV88W8618 network device out of hw/arm/ directory Peter Maydell
` (34 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
We are going to move this code, so fix its style first to avoid:
ERROR: spaces required around that '/' (ctx:VxV)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220107184429.423572-3-f4bug@amsat.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/musicpal.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 2680ec55b5a..1291eb98aba 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -310,13 +310,13 @@ static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset,
return s->imr;
case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
- return s->frx_queue[(offset - MP_ETH_FRDP0)/4];
+ return s->frx_queue[(offset - MP_ETH_FRDP0) / 4];
case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
- return s->rx_queue[(offset - MP_ETH_CRDP0)/4];
+ return s->rx_queue[(offset - MP_ETH_CRDP0) / 4];
case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
- return s->tx_queue[(offset - MP_ETH_CTDP0)/4];
+ return s->tx_queue[(offset - MP_ETH_CTDP0) / 4];
default:
return 0;
@@ -361,16 +361,16 @@ static void mv88w8618_eth_write(void *opaque, hwaddr offset,
break;
case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
- s->frx_queue[(offset - MP_ETH_FRDP0)/4] = value;
+ s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value;
break;
case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
- s->rx_queue[(offset - MP_ETH_CRDP0)/4] =
- s->cur_rx[(offset - MP_ETH_CRDP0)/4] = value;
+ s->rx_queue[(offset - MP_ETH_CRDP0) / 4] =
+ s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value;
break;
case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
- s->tx_queue[(offset - MP_ETH_CTDP0)/4] = value;
+ s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value;
break;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 04/38] hw/net: Move MV88W8618 network device out of hw/arm/ directory
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2022-01-20 12:35 ` [PULL 03/38] hw/arm/musicpal: Fix coding style of code related to MV88W8618 device Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 05/38] hw/arm/virt: Support CPU cluster on ARM virt machine Peter Maydell
` (33 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
The Marvell 88W8618 network device is hidden in the Musicpal
machine. Move it into a new unit file under the hw/net/ directory.
Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220107184429.423572-4-f4bug@amsat.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/net/mv88w8618_eth.h | 12 +
hw/arm/musicpal.c | 381 +------------------------------
hw/net/mv88w8618_eth.c | 403 +++++++++++++++++++++++++++++++++
MAINTAINERS | 2 +
hw/net/meson.build | 1 +
5 files changed, 419 insertions(+), 380 deletions(-)
create mode 100644 include/hw/net/mv88w8618_eth.h
create mode 100644 hw/net/mv88w8618_eth.c
diff --git a/include/hw/net/mv88w8618_eth.h b/include/hw/net/mv88w8618_eth.h
new file mode 100644
index 00000000000..8f4c746092f
--- /dev/null
+++ b/include/hw/net/mv88w8618_eth.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Marvell MV88W8618 / Freecom MusicPal emulation.
+ *
+ * Copyright (c) 2008-2021 QEMU contributors
+ */
+#ifndef HW_NET_MV88W8618_H
+#define HW_NET_MV88W8618_H
+
+#define TYPE_MV88W8618_ETH "mv88w8618_eth"
+
+#endif
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 1291eb98aba..7c840fb4283 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -34,12 +34,12 @@
#include "ui/pixel_ops.h"
#include "qemu/cutils.h"
#include "qom/object.h"
+#include "hw/net/mv88w8618_eth.h"
#define MP_MISC_BASE 0x80002000
#define MP_MISC_SIZE 0x00001000
#define MP_ETH_BASE 0x80008000
-#define MP_ETH_SIZE 0x00001000
#define MP_WLAN_BASE 0x8000C000
#define MP_WLAN_SIZE 0x00000800
@@ -84,384 +84,6 @@
/* Wolfson 8750 I2C address */
#define MP_WM_ADDR 0x1A
-/* Ethernet register offsets */
-#define MP_ETH_SMIR 0x010
-#define MP_ETH_PCXR 0x408
-#define MP_ETH_SDCMR 0x448
-#define MP_ETH_ICR 0x450
-#define MP_ETH_IMR 0x458
-#define MP_ETH_FRDP0 0x480
-#define MP_ETH_FRDP1 0x484
-#define MP_ETH_FRDP2 0x488
-#define MP_ETH_FRDP3 0x48C
-#define MP_ETH_CRDP0 0x4A0
-#define MP_ETH_CRDP1 0x4A4
-#define MP_ETH_CRDP2 0x4A8
-#define MP_ETH_CRDP3 0x4AC
-#define MP_ETH_CTDP0 0x4E0
-#define MP_ETH_CTDP1 0x4E4
-
-/* MII PHY access */
-#define MP_ETH_SMIR_DATA 0x0000FFFF
-#define MP_ETH_SMIR_ADDR 0x03FF0000
-#define MP_ETH_SMIR_OPCODE (1 << 26) /* Read value */
-#define MP_ETH_SMIR_RDVALID (1 << 27)
-
-/* PHY registers */
-#define MP_ETH_PHY1_BMSR 0x00210000
-#define MP_ETH_PHY1_PHYSID1 0x00410000
-#define MP_ETH_PHY1_PHYSID2 0x00610000
-
-#define MP_PHY_BMSR_LINK 0x0004
-#define MP_PHY_BMSR_AUTONEG 0x0008
-
-#define MP_PHY_88E3015 0x01410E20
-
-/* TX descriptor status */
-#define MP_ETH_TX_OWN (1U << 31)
-
-/* RX descriptor status */
-#define MP_ETH_RX_OWN (1U << 31)
-
-/* Interrupt cause/mask bits */
-#define MP_ETH_IRQ_RX_BIT 0
-#define MP_ETH_IRQ_RX (1 << MP_ETH_IRQ_RX_BIT)
-#define MP_ETH_IRQ_TXHI_BIT 2
-#define MP_ETH_IRQ_TXLO_BIT 3
-
-/* Port config bits */
-#define MP_ETH_PCXR_2BSM_BIT 28 /* 2-byte incoming suffix */
-
-/* SDMA command bits */
-#define MP_ETH_CMD_TXHI (1 << 23)
-#define MP_ETH_CMD_TXLO (1 << 22)
-
-typedef struct mv88w8618_tx_desc {
- uint32_t cmdstat;
- uint16_t res;
- uint16_t bytes;
- uint32_t buffer;
- uint32_t next;
-} mv88w8618_tx_desc;
-
-typedef struct mv88w8618_rx_desc {
- uint32_t cmdstat;
- uint16_t bytes;
- uint16_t buffer_size;
- uint32_t buffer;
- uint32_t next;
-} mv88w8618_rx_desc;
-
-#define TYPE_MV88W8618_ETH "mv88w8618_eth"
-OBJECT_DECLARE_SIMPLE_TYPE(mv88w8618_eth_state, MV88W8618_ETH)
-
-struct mv88w8618_eth_state {
- /*< private >*/
- SysBusDevice parent_obj;
- /*< public >*/
-
- MemoryRegion iomem;
- qemu_irq irq;
- MemoryRegion *dma_mr;
- AddressSpace dma_as;
- uint32_t smir;
- uint32_t icr;
- uint32_t imr;
- int mmio_index;
- uint32_t vlan_header;
- uint32_t tx_queue[2];
- uint32_t rx_queue[4];
- uint32_t frx_queue[4];
- uint32_t cur_rx[4];
- NICState *nic;
- NICConf conf;
-};
-
-static void eth_rx_desc_put(AddressSpace *dma_as, uint32_t addr,
- mv88w8618_rx_desc *desc)
-{
- cpu_to_le32s(&desc->cmdstat);
- cpu_to_le16s(&desc->bytes);
- cpu_to_le16s(&desc->buffer_size);
- cpu_to_le32s(&desc->buffer);
- cpu_to_le32s(&desc->next);
- dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
-}
-
-static void eth_rx_desc_get(AddressSpace *dma_as, uint32_t addr,
- mv88w8618_rx_desc *desc)
-{
- dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
- le32_to_cpus(&desc->cmdstat);
- le16_to_cpus(&desc->bytes);
- le16_to_cpus(&desc->buffer_size);
- le32_to_cpus(&desc->buffer);
- le32_to_cpus(&desc->next);
-}
-
-static ssize_t eth_receive(NetClientState *nc, const uint8_t *buf, size_t size)
-{
- mv88w8618_eth_state *s = qemu_get_nic_opaque(nc);
- uint32_t desc_addr;
- mv88w8618_rx_desc desc;
- int i;
-
- for (i = 0; i < 4; i++) {
- desc_addr = s->cur_rx[i];
- if (!desc_addr) {
- continue;
- }
- do {
- eth_rx_desc_get(&s->dma_as, desc_addr, &desc);
- if ((desc.cmdstat & MP_ETH_RX_OWN) && desc.buffer_size >= size) {
- dma_memory_write(&s->dma_as, desc.buffer + s->vlan_header,
- buf, size, MEMTXATTRS_UNSPECIFIED);
- desc.bytes = size + s->vlan_header;
- desc.cmdstat &= ~MP_ETH_RX_OWN;
- s->cur_rx[i] = desc.next;
-
- s->icr |= MP_ETH_IRQ_RX;
- if (s->icr & s->imr) {
- qemu_irq_raise(s->irq);
- }
- eth_rx_desc_put(&s->dma_as, desc_addr, &desc);
- return size;
- }
- desc_addr = desc.next;
- } while (desc_addr != s->rx_queue[i]);
- }
- return size;
-}
-
-static void eth_tx_desc_put(AddressSpace *dma_as, uint32_t addr,
- mv88w8618_tx_desc *desc)
-{
- cpu_to_le32s(&desc->cmdstat);
- cpu_to_le16s(&desc->res);
- cpu_to_le16s(&desc->bytes);
- cpu_to_le32s(&desc->buffer);
- cpu_to_le32s(&desc->next);
- dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
-}
-
-static void eth_tx_desc_get(AddressSpace *dma_as, uint32_t addr,
- mv88w8618_tx_desc *desc)
-{
- dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
- le32_to_cpus(&desc->cmdstat);
- le16_to_cpus(&desc->res);
- le16_to_cpus(&desc->bytes);
- le32_to_cpus(&desc->buffer);
- le32_to_cpus(&desc->next);
-}
-
-static void eth_send(mv88w8618_eth_state *s, int queue_index)
-{
- uint32_t desc_addr = s->tx_queue[queue_index];
- mv88w8618_tx_desc desc;
- uint32_t next_desc;
- uint8_t buf[2048];
- int len;
-
- do {
- eth_tx_desc_get(&s->dma_as, desc_addr, &desc);
- next_desc = desc.next;
- if (desc.cmdstat & MP_ETH_TX_OWN) {
- len = desc.bytes;
- if (len < 2048) {
- dma_memory_read(&s->dma_as, desc.buffer, buf, len,
- MEMTXATTRS_UNSPECIFIED);
- qemu_send_packet(qemu_get_queue(s->nic), buf, len);
- }
- desc.cmdstat &= ~MP_ETH_TX_OWN;
- s->icr |= 1 << (MP_ETH_IRQ_TXLO_BIT - queue_index);
- eth_tx_desc_put(&s->dma_as, desc_addr, &desc);
- }
- desc_addr = next_desc;
- } while (desc_addr != s->tx_queue[queue_index]);
-}
-
-static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset,
- unsigned size)
-{
- mv88w8618_eth_state *s = opaque;
-
- switch (offset) {
- case MP_ETH_SMIR:
- if (s->smir & MP_ETH_SMIR_OPCODE) {
- switch (s->smir & MP_ETH_SMIR_ADDR) {
- case MP_ETH_PHY1_BMSR:
- return MP_PHY_BMSR_LINK | MP_PHY_BMSR_AUTONEG |
- MP_ETH_SMIR_RDVALID;
- case MP_ETH_PHY1_PHYSID1:
- return (MP_PHY_88E3015 >> 16) | MP_ETH_SMIR_RDVALID;
- case MP_ETH_PHY1_PHYSID2:
- return (MP_PHY_88E3015 & 0xFFFF) | MP_ETH_SMIR_RDVALID;
- default:
- return MP_ETH_SMIR_RDVALID;
- }
- }
- return 0;
-
- case MP_ETH_ICR:
- return s->icr;
-
- case MP_ETH_IMR:
- return s->imr;
-
- case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
- return s->frx_queue[(offset - MP_ETH_FRDP0) / 4];
-
- case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
- return s->rx_queue[(offset - MP_ETH_CRDP0) / 4];
-
- case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
- return s->tx_queue[(offset - MP_ETH_CTDP0) / 4];
-
- default:
- return 0;
- }
-}
-
-static void mv88w8618_eth_write(void *opaque, hwaddr offset,
- uint64_t value, unsigned size)
-{
- mv88w8618_eth_state *s = opaque;
-
- switch (offset) {
- case MP_ETH_SMIR:
- s->smir = value;
- break;
-
- case MP_ETH_PCXR:
- s->vlan_header = ((value >> MP_ETH_PCXR_2BSM_BIT) & 1) * 2;
- break;
-
- case MP_ETH_SDCMR:
- if (value & MP_ETH_CMD_TXHI) {
- eth_send(s, 1);
- }
- if (value & MP_ETH_CMD_TXLO) {
- eth_send(s, 0);
- }
- if (value & (MP_ETH_CMD_TXHI | MP_ETH_CMD_TXLO) && s->icr & s->imr) {
- qemu_irq_raise(s->irq);
- }
- break;
-
- case MP_ETH_ICR:
- s->icr &= value;
- break;
-
- case MP_ETH_IMR:
- s->imr = value;
- if (s->icr & s->imr) {
- qemu_irq_raise(s->irq);
- }
- break;
-
- case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
- s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value;
- break;
-
- case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
- s->rx_queue[(offset - MP_ETH_CRDP0) / 4] =
- s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value;
- break;
-
- case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
- s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value;
- break;
- }
-}
-
-static const MemoryRegionOps mv88w8618_eth_ops = {
- .read = mv88w8618_eth_read,
- .write = mv88w8618_eth_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static void eth_cleanup(NetClientState *nc)
-{
- mv88w8618_eth_state *s = qemu_get_nic_opaque(nc);
-
- s->nic = NULL;
-}
-
-static NetClientInfo net_mv88w8618_info = {
- .type = NET_CLIENT_DRIVER_NIC,
- .size = sizeof(NICState),
- .receive = eth_receive,
- .cleanup = eth_cleanup,
-};
-
-static void mv88w8618_eth_init(Object *obj)
-{
- SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
- DeviceState *dev = DEVICE(sbd);
- mv88w8618_eth_state *s = MV88W8618_ETH(dev);
-
- sysbus_init_irq(sbd, &s->irq);
- memory_region_init_io(&s->iomem, obj, &mv88w8618_eth_ops, s,
- "mv88w8618-eth", MP_ETH_SIZE);
- sysbus_init_mmio(sbd, &s->iomem);
-}
-
-static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
-{
- mv88w8618_eth_state *s = MV88W8618_ETH(dev);
-
- if (!s->dma_mr) {
- error_setg(errp, TYPE_MV88W8618_ETH " 'dma-memory' link not set");
- return;
- }
-
- address_space_init(&s->dma_as, s->dma_mr, "emac-dma");
- s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf,
- object_get_typename(OBJECT(dev)), dev->id, s);
-}
-
-static const VMStateDescription mv88w8618_eth_vmsd = {
- .name = "mv88w8618_eth",
- .version_id = 1,
- .minimum_version_id = 1,
- .fields = (VMStateField[]) {
- VMSTATE_UINT32(smir, mv88w8618_eth_state),
- VMSTATE_UINT32(icr, mv88w8618_eth_state),
- VMSTATE_UINT32(imr, mv88w8618_eth_state),
- VMSTATE_UINT32(vlan_header, mv88w8618_eth_state),
- VMSTATE_UINT32_ARRAY(tx_queue, mv88w8618_eth_state, 2),
- VMSTATE_UINT32_ARRAY(rx_queue, mv88w8618_eth_state, 4),
- VMSTATE_UINT32_ARRAY(frx_queue, mv88w8618_eth_state, 4),
- VMSTATE_UINT32_ARRAY(cur_rx, mv88w8618_eth_state, 4),
- VMSTATE_END_OF_LIST()
- }
-};
-
-static Property mv88w8618_eth_properties[] = {
- DEFINE_NIC_PROPERTIES(mv88w8618_eth_state, conf),
- DEFINE_PROP_LINK("dma-memory", mv88w8618_eth_state, dma_mr,
- TYPE_MEMORY_REGION, MemoryRegion *),
- DEFINE_PROP_END_OF_LIST(),
-};
-
-static void mv88w8618_eth_class_init(ObjectClass *klass, void *data)
-{
- DeviceClass *dc = DEVICE_CLASS(klass);
-
- dc->vmsd = &mv88w8618_eth_vmsd;
- device_class_set_props(dc, mv88w8618_eth_properties);
- dc->realize = mv88w8618_eth_realize;
-}
-
-static const TypeInfo mv88w8618_eth_info = {
- .name = TYPE_MV88W8618_ETH,
- .parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(mv88w8618_eth_state),
- .instance_init = mv88w8618_eth_init,
- .class_init = mv88w8618_eth_class_init,
-};
-
/* LCD register offsets */
#define MP_LCD_IRQCTRL 0x180
#define MP_LCD_IRQSTAT 0x184
@@ -1746,7 +1368,6 @@ static void musicpal_register_types(void)
type_register_static(&mv88w8618_pic_info);
type_register_static(&mv88w8618_pit_info);
type_register_static(&mv88w8618_flashcfg_info);
- type_register_static(&mv88w8618_eth_info);
type_register_static(&mv88w8618_wlan_info);
type_register_static(&musicpal_lcd_info);
type_register_static(&musicpal_gpio_info);
diff --git a/hw/net/mv88w8618_eth.c b/hw/net/mv88w8618_eth.c
new file mode 100644
index 00000000000..ef30b0d4a6a
--- /dev/null
+++ b/hw/net/mv88w8618_eth.c
@@ -0,0 +1,403 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Marvell MV88W8618 / Freecom MusicPal emulation.
+ *
+ * Copyright (c) 2008 Jan Kiszka
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/net/mv88w8618_eth.h"
+#include "migration/vmstate.h"
+#include "sysemu/dma.h"
+#include "net/net.h"
+
+#define MP_ETH_SIZE 0x00001000
+
+/* Ethernet register offsets */
+#define MP_ETH_SMIR 0x010
+#define MP_ETH_PCXR 0x408
+#define MP_ETH_SDCMR 0x448
+#define MP_ETH_ICR 0x450
+#define MP_ETH_IMR 0x458
+#define MP_ETH_FRDP0 0x480
+#define MP_ETH_FRDP1 0x484
+#define MP_ETH_FRDP2 0x488
+#define MP_ETH_FRDP3 0x48C
+#define MP_ETH_CRDP0 0x4A0
+#define MP_ETH_CRDP1 0x4A4
+#define MP_ETH_CRDP2 0x4A8
+#define MP_ETH_CRDP3 0x4AC
+#define MP_ETH_CTDP0 0x4E0
+#define MP_ETH_CTDP1 0x4E4
+
+/* MII PHY access */
+#define MP_ETH_SMIR_DATA 0x0000FFFF
+#define MP_ETH_SMIR_ADDR 0x03FF0000
+#define MP_ETH_SMIR_OPCODE (1 << 26) /* Read value */
+#define MP_ETH_SMIR_RDVALID (1 << 27)
+
+/* PHY registers */
+#define MP_ETH_PHY1_BMSR 0x00210000
+#define MP_ETH_PHY1_PHYSID1 0x00410000
+#define MP_ETH_PHY1_PHYSID2 0x00610000
+
+#define MP_PHY_BMSR_LINK 0x0004
+#define MP_PHY_BMSR_AUTONEG 0x0008
+
+#define MP_PHY_88E3015 0x01410E20
+
+/* TX descriptor status */
+#define MP_ETH_TX_OWN (1U << 31)
+
+/* RX descriptor status */
+#define MP_ETH_RX_OWN (1U << 31)
+
+/* Interrupt cause/mask bits */
+#define MP_ETH_IRQ_RX_BIT 0
+#define MP_ETH_IRQ_RX (1 << MP_ETH_IRQ_RX_BIT)
+#define MP_ETH_IRQ_TXHI_BIT 2
+#define MP_ETH_IRQ_TXLO_BIT 3
+
+/* Port config bits */
+#define MP_ETH_PCXR_2BSM_BIT 28 /* 2-byte incoming suffix */
+
+/* SDMA command bits */
+#define MP_ETH_CMD_TXHI (1 << 23)
+#define MP_ETH_CMD_TXLO (1 << 22)
+
+typedef struct mv88w8618_tx_desc {
+ uint32_t cmdstat;
+ uint16_t res;
+ uint16_t bytes;
+ uint32_t buffer;
+ uint32_t next;
+} mv88w8618_tx_desc;
+
+typedef struct mv88w8618_rx_desc {
+ uint32_t cmdstat;
+ uint16_t bytes;
+ uint16_t buffer_size;
+ uint32_t buffer;
+ uint32_t next;
+} mv88w8618_rx_desc;
+
+OBJECT_DECLARE_SIMPLE_TYPE(mv88w8618_eth_state, MV88W8618_ETH)
+
+struct mv88w8618_eth_state {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+
+ MemoryRegion iomem;
+ qemu_irq irq;
+ MemoryRegion *dma_mr;
+ AddressSpace dma_as;
+ uint32_t smir;
+ uint32_t icr;
+ uint32_t imr;
+ int mmio_index;
+ uint32_t vlan_header;
+ uint32_t tx_queue[2];
+ uint32_t rx_queue[4];
+ uint32_t frx_queue[4];
+ uint32_t cur_rx[4];
+ NICState *nic;
+ NICConf conf;
+};
+
+static void eth_rx_desc_put(AddressSpace *dma_as, uint32_t addr,
+ mv88w8618_rx_desc *desc)
+{
+ cpu_to_le32s(&desc->cmdstat);
+ cpu_to_le16s(&desc->bytes);
+ cpu_to_le16s(&desc->buffer_size);
+ cpu_to_le32s(&desc->buffer);
+ cpu_to_le32s(&desc->next);
+ dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
+}
+
+static void eth_rx_desc_get(AddressSpace *dma_as, uint32_t addr,
+ mv88w8618_rx_desc *desc)
+{
+ dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
+ le32_to_cpus(&desc->cmdstat);
+ le16_to_cpus(&desc->bytes);
+ le16_to_cpus(&desc->buffer_size);
+ le32_to_cpus(&desc->buffer);
+ le32_to_cpus(&desc->next);
+}
+
+static ssize_t eth_receive(NetClientState *nc, const uint8_t *buf, size_t size)
+{
+ mv88w8618_eth_state *s = qemu_get_nic_opaque(nc);
+ uint32_t desc_addr;
+ mv88w8618_rx_desc desc;
+ int i;
+
+ for (i = 0; i < 4; i++) {
+ desc_addr = s->cur_rx[i];
+ if (!desc_addr) {
+ continue;
+ }
+ do {
+ eth_rx_desc_get(&s->dma_as, desc_addr, &desc);
+ if ((desc.cmdstat & MP_ETH_RX_OWN) && desc.buffer_size >= size) {
+ dma_memory_write(&s->dma_as, desc.buffer + s->vlan_header,
+ buf, size, MEMTXATTRS_UNSPECIFIED);
+ desc.bytes = size + s->vlan_header;
+ desc.cmdstat &= ~MP_ETH_RX_OWN;
+ s->cur_rx[i] = desc.next;
+
+ s->icr |= MP_ETH_IRQ_RX;
+ if (s->icr & s->imr) {
+ qemu_irq_raise(s->irq);
+ }
+ eth_rx_desc_put(&s->dma_as, desc_addr, &desc);
+ return size;
+ }
+ desc_addr = desc.next;
+ } while (desc_addr != s->rx_queue[i]);
+ }
+ return size;
+}
+
+static void eth_tx_desc_put(AddressSpace *dma_as, uint32_t addr,
+ mv88w8618_tx_desc *desc)
+{
+ cpu_to_le32s(&desc->cmdstat);
+ cpu_to_le16s(&desc->res);
+ cpu_to_le16s(&desc->bytes);
+ cpu_to_le32s(&desc->buffer);
+ cpu_to_le32s(&desc->next);
+ dma_memory_write(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
+}
+
+static void eth_tx_desc_get(AddressSpace *dma_as, uint32_t addr,
+ mv88w8618_tx_desc *desc)
+{
+ dma_memory_read(dma_as, addr, desc, sizeof(*desc), MEMTXATTRS_UNSPECIFIED);
+ le32_to_cpus(&desc->cmdstat);
+ le16_to_cpus(&desc->res);
+ le16_to_cpus(&desc->bytes);
+ le32_to_cpus(&desc->buffer);
+ le32_to_cpus(&desc->next);
+}
+
+static void eth_send(mv88w8618_eth_state *s, int queue_index)
+{
+ uint32_t desc_addr = s->tx_queue[queue_index];
+ mv88w8618_tx_desc desc;
+ uint32_t next_desc;
+ uint8_t buf[2048];
+ int len;
+
+ do {
+ eth_tx_desc_get(&s->dma_as, desc_addr, &desc);
+ next_desc = desc.next;
+ if (desc.cmdstat & MP_ETH_TX_OWN) {
+ len = desc.bytes;
+ if (len < 2048) {
+ dma_memory_read(&s->dma_as, desc.buffer, buf, len,
+ MEMTXATTRS_UNSPECIFIED);
+ qemu_send_packet(qemu_get_queue(s->nic), buf, len);
+ }
+ desc.cmdstat &= ~MP_ETH_TX_OWN;
+ s->icr |= 1 << (MP_ETH_IRQ_TXLO_BIT - queue_index);
+ eth_tx_desc_put(&s->dma_as, desc_addr, &desc);
+ }
+ desc_addr = next_desc;
+ } while (desc_addr != s->tx_queue[queue_index]);
+}
+
+static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ mv88w8618_eth_state *s = opaque;
+
+ switch (offset) {
+ case MP_ETH_SMIR:
+ if (s->smir & MP_ETH_SMIR_OPCODE) {
+ switch (s->smir & MP_ETH_SMIR_ADDR) {
+ case MP_ETH_PHY1_BMSR:
+ return MP_PHY_BMSR_LINK | MP_PHY_BMSR_AUTONEG |
+ MP_ETH_SMIR_RDVALID;
+ case MP_ETH_PHY1_PHYSID1:
+ return (MP_PHY_88E3015 >> 16) | MP_ETH_SMIR_RDVALID;
+ case MP_ETH_PHY1_PHYSID2:
+ return (MP_PHY_88E3015 & 0xFFFF) | MP_ETH_SMIR_RDVALID;
+ default:
+ return MP_ETH_SMIR_RDVALID;
+ }
+ }
+ return 0;
+
+ case MP_ETH_ICR:
+ return s->icr;
+
+ case MP_ETH_IMR:
+ return s->imr;
+
+ case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
+ return s->frx_queue[(offset - MP_ETH_FRDP0) / 4];
+
+ case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
+ return s->rx_queue[(offset - MP_ETH_CRDP0) / 4];
+
+ case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
+ return s->tx_queue[(offset - MP_ETH_CTDP0) / 4];
+
+ default:
+ return 0;
+ }
+}
+
+static void mv88w8618_eth_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+ mv88w8618_eth_state *s = opaque;
+
+ switch (offset) {
+ case MP_ETH_SMIR:
+ s->smir = value;
+ break;
+
+ case MP_ETH_PCXR:
+ s->vlan_header = ((value >> MP_ETH_PCXR_2BSM_BIT) & 1) * 2;
+ break;
+
+ case MP_ETH_SDCMR:
+ if (value & MP_ETH_CMD_TXHI) {
+ eth_send(s, 1);
+ }
+ if (value & MP_ETH_CMD_TXLO) {
+ eth_send(s, 0);
+ }
+ if (value & (MP_ETH_CMD_TXHI | MP_ETH_CMD_TXLO) && s->icr & s->imr) {
+ qemu_irq_raise(s->irq);
+ }
+ break;
+
+ case MP_ETH_ICR:
+ s->icr &= value;
+ break;
+
+ case MP_ETH_IMR:
+ s->imr = value;
+ if (s->icr & s->imr) {
+ qemu_irq_raise(s->irq);
+ }
+ break;
+
+ case MP_ETH_FRDP0 ... MP_ETH_FRDP3:
+ s->frx_queue[(offset - MP_ETH_FRDP0) / 4] = value;
+ break;
+
+ case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
+ s->rx_queue[(offset - MP_ETH_CRDP0) / 4] =
+ s->cur_rx[(offset - MP_ETH_CRDP0) / 4] = value;
+ break;
+
+ case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
+ s->tx_queue[(offset - MP_ETH_CTDP0) / 4] = value;
+ break;
+ }
+}
+
+static const MemoryRegionOps mv88w8618_eth_ops = {
+ .read = mv88w8618_eth_read,
+ .write = mv88w8618_eth_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void eth_cleanup(NetClientState *nc)
+{
+ mv88w8618_eth_state *s = qemu_get_nic_opaque(nc);
+
+ s->nic = NULL;
+}
+
+static NetClientInfo net_mv88w8618_info = {
+ .type = NET_CLIENT_DRIVER_NIC,
+ .size = sizeof(NICState),
+ .receive = eth_receive,
+ .cleanup = eth_cleanup,
+};
+
+static void mv88w8618_eth_init(Object *obj)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ DeviceState *dev = DEVICE(sbd);
+ mv88w8618_eth_state *s = MV88W8618_ETH(dev);
+
+ sysbus_init_irq(sbd, &s->irq);
+ memory_region_init_io(&s->iomem, obj, &mv88w8618_eth_ops, s,
+ "mv88w8618-eth", MP_ETH_SIZE);
+ sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
+{
+ mv88w8618_eth_state *s = MV88W8618_ETH(dev);
+
+ if (!s->dma_mr) {
+ error_setg(errp, TYPE_MV88W8618_ETH " 'dma-memory' link not set");
+ return;
+ }
+
+ address_space_init(&s->dma_as, s->dma_mr, "emac-dma");
+ s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf,
+ object_get_typename(OBJECT(dev)), dev->id, s);
+}
+
+static const VMStateDescription mv88w8618_eth_vmsd = {
+ .name = "mv88w8618_eth",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(smir, mv88w8618_eth_state),
+ VMSTATE_UINT32(icr, mv88w8618_eth_state),
+ VMSTATE_UINT32(imr, mv88w8618_eth_state),
+ VMSTATE_UINT32(vlan_header, mv88w8618_eth_state),
+ VMSTATE_UINT32_ARRAY(tx_queue, mv88w8618_eth_state, 2),
+ VMSTATE_UINT32_ARRAY(rx_queue, mv88w8618_eth_state, 4),
+ VMSTATE_UINT32_ARRAY(frx_queue, mv88w8618_eth_state, 4),
+ VMSTATE_UINT32_ARRAY(cur_rx, mv88w8618_eth_state, 4),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static Property mv88w8618_eth_properties[] = {
+ DEFINE_NIC_PROPERTIES(mv88w8618_eth_state, conf),
+ DEFINE_PROP_LINK("dma-memory", mv88w8618_eth_state, dma_mr,
+ TYPE_MEMORY_REGION, MemoryRegion *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mv88w8618_eth_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->vmsd = &mv88w8618_eth_vmsd;
+ device_class_set_props(dc, mv88w8618_eth_properties);
+ dc->realize = mv88w8618_eth_realize;
+}
+
+static const TypeInfo mv88w8618_eth_info = {
+ .name = TYPE_MV88W8618_ETH,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(mv88w8618_eth_state),
+ .instance_init = mv88w8618_eth_init,
+ .class_init = mv88w8618_eth_class_init,
+};
+
+static void musicpal_register_types(void)
+{
+ type_register_static(&mv88w8618_eth_info);
+}
+
+type_init(musicpal_register_types)
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 893a5567100..e4b3a4bcdf4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -774,6 +774,8 @@ M: Peter Maydell <peter.maydell@linaro.org>
L: qemu-arm@nongnu.org
S: Odd Fixes
F: hw/arm/musicpal.c
+F: hw/net/mv88w8618_eth.c
+F: include/hw/net/mv88w8618_eth.h
F: docs/system/arm/musicpal.rst
Nuvoton NPCM7xx
diff --git a/hw/net/meson.build b/hw/net/meson.build
index bdf71f1f405..685b75badb4 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -26,6 +26,7 @@ softmmu_ss.add(when: 'CONFIG_ALLWINNER_EMAC', if_true: files('allwinner_emac.c')
softmmu_ss.add(when: 'CONFIG_ALLWINNER_SUN8I_EMAC', if_true: files('allwinner-sun8i-emac.c'))
softmmu_ss.add(when: 'CONFIG_IMX_FEC', if_true: files('imx_fec.c'))
softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-emac.c'))
+softmmu_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: files('mv88w8618_eth.c'))
softmmu_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c'))
softmmu_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: files('stellaris_enet.c'))
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 05/38] hw/arm/virt: Support CPU cluster on ARM virt machine
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2022-01-20 12:35 ` [PULL 04/38] hw/net: Move MV88W8618 network device out of hw/arm/ directory Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 06/38] hw/arm/virt: Support cluster level in DT cpu-map Peter Maydell
` (32 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
ARM64 machines like Kunpeng Family Server Chips have a level
of hardware topology in which a group of CPU cores share L3
cache tag or L2 cache. For example, Kunpeng 920 typically
has 6 or 8 clusters in each NUMA node (also represent range
of CPU die), and each cluster has 4 CPU cores. All clusters
share L3 cache data, but CPU cores in each cluster share a
local L3 tag.
Running a guest kernel with Cluster-Aware Scheduling on the
Hosts which have physical clusters, if we can design a vCPU
topology with cluster level for guest kernel and then have
a dedicated vCPU pinning, the guest will gain scheduling
performance improvement from cache affinity of CPU cluster.
So let's enable the support for this new parameter on ARM
virt machines. After this patch, we can define a 4-level
CPU hierarchy like: cpus=*,maxcpus=*,sockets=*,clusters=*,
cores=*,threads=*.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20220107083232.16256-2-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 1 +
qemu-options.hx | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 84c2444fff6..688d8736909 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2718,6 +2718,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
hc->unplug_request = virt_machine_device_unplug_request_cb;
hc->unplug = virt_machine_device_unplug_cb;
mc->nvdimm_supported = true;
+ mc->smp_props.clusters_supported = true;
mc->auto_enable_numa_with_memhp = true;
mc->auto_enable_numa_with_memdev = true;
mc->default_ram_id = "mach-virt.ram";
diff --git a/qemu-options.hx b/qemu-options.hx
index ec90505d84e..ba3ae6a42aa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -277,6 +277,16 @@ SRST
-smp 16,sockets=2,dies=2,cores=2,threads=2,maxcpus=16
+ The following sub-option defines a CPU topology hierarchy (2 sockets
+ totally on the machine, 2 clusters per socket, 2 cores per cluster,
+ 2 threads per core) for ARM virt machines which support sockets/clusters
+ /cores/threads. Some members of the option can be omitted but their values
+ will be automatically computed:
+
+ ::
+
+ -smp 16,sockets=2,clusters=2,cores=2,threads=2,maxcpus=16
+
Historically preference was given to the coarsest topology parameters
when computing missing values (ie sockets preferred over cores, which
were preferred over threads), however, this behaviour is considered
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 06/38] hw/arm/virt: Support cluster level in DT cpu-map
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (4 preceding siblings ...)
2022-01-20 12:35 ` [PULL 05/38] hw/arm/virt: Support CPU cluster on ARM virt machine Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:35 ` [PULL 07/38] hw/acpi/aml-build: Improve scalability of PPTT generation Peter Maydell
` (31 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
Support one cluster level between core and physical package in the
cpu-map of Arm/virt devicetree. This is also consistent with Linux
Doc "Documentation/devicetree/bindings/cpu/cpu-topology.txt".
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20220107083232.16256-3-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 688d8736909..177db1da12e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -434,9 +434,8 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
* can contain several layers of clustering within a single physical
* package and cluster nodes can be contained in parent cluster nodes.
*
- * Given that cluster is not yet supported in the vCPU topology,
- * we currently generate one cluster node within each socket node
- * by default.
+ * Note: currently we only support one layer of clustering within
+ * each physical package.
*/
qemu_fdt_add_subnode(ms->fdt, "/cpus/cpu-map");
@@ -446,14 +445,16 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
if (ms->smp.threads > 1) {
map_path = g_strdup_printf(
- "/cpus/cpu-map/socket%d/cluster0/core%d/thread%d",
- cpu / (ms->smp.cores * ms->smp.threads),
+ "/cpus/cpu-map/socket%d/cluster%d/core%d/thread%d",
+ cpu / (ms->smp.clusters * ms->smp.cores * ms->smp.threads),
+ (cpu / (ms->smp.cores * ms->smp.threads)) % ms->smp.clusters,
(cpu / ms->smp.threads) % ms->smp.cores,
cpu % ms->smp.threads);
} else {
map_path = g_strdup_printf(
- "/cpus/cpu-map/socket%d/cluster0/core%d",
- cpu / ms->smp.cores,
+ "/cpus/cpu-map/socket%d/cluster%d/core%d",
+ cpu / (ms->smp.clusters * ms->smp.cores),
+ (cpu / ms->smp.cores) % ms->smp.clusters,
cpu % ms->smp.cores);
}
qemu_fdt_add_path(ms->fdt, map_path);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 07/38] hw/acpi/aml-build: Improve scalability of PPTT generation
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (5 preceding siblings ...)
2022-01-20 12:35 ` [PULL 06/38] hw/arm/virt: Support cluster level in DT cpu-map Peter Maydell
@ 2022-01-20 12:35 ` Peter Maydell
2022-01-20 12:36 ` [PULL 08/38] tests/acpi/bios-tables-test: Allow changes to virt/PPTT file Peter Maydell
` (30 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:35 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
Use g_queue APIs to reduce the nested loops and code indentation
with the processor hierarchy levels increasing. Consenquently,
it's more scalable to add new topology level to build_pptt.
No functional change intended.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20220107083232.16256-4-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/acpi/aml-build.c | 50 +++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index b3b3310df32..6aaedca2e56 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2001,7 +2001,10 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
const char *oem_id, const char *oem_table_id)
{
- int pptt_start = table_data->len;
+ GQueue *list = g_queue_new();
+ guint pptt_start = table_data->len;
+ guint parent_offset;
+ guint length, i;
int uid = 0;
int socket;
AcpiTable table = { .sig = "PPTT", .rev = 2,
@@ -2010,9 +2013,8 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
acpi_table_begin(&table, table_data);
for (socket = 0; socket < ms->smp.sockets; socket++) {
- uint32_t socket_offset = table_data->len - pptt_start;
- int core;
-
+ g_queue_push_tail(list,
+ GUINT_TO_POINTER(table_data->len - pptt_start));
build_processor_hierarchy_node(
table_data,
/*
@@ -2021,35 +2023,47 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
*/
(1 << 0),
0, socket, NULL, 0);
+ }
+ length = g_queue_get_length(list);
+ for (i = 0; i < length; i++) {
+ int core;
+
+ parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
for (core = 0; core < ms->smp.cores; core++) {
- uint32_t core_offset = table_data->len - pptt_start;
- int thread;
-
if (ms->smp.threads > 1) {
+ g_queue_push_tail(list,
+ GUINT_TO_POINTER(table_data->len - pptt_start));
build_processor_hierarchy_node(
table_data,
(0 << 0), /* not a physical package */
- socket_offset, core, NULL, 0);
-
- for (thread = 0; thread < ms->smp.threads; thread++) {
- build_processor_hierarchy_node(
- table_data,
- (1 << 1) | /* ACPI Processor ID valid */
- (1 << 2) | /* Processor is a Thread */
- (1 << 3), /* Node is a Leaf */
- core_offset, uid++, NULL, 0);
- }
+ parent_offset, core, NULL, 0);
} else {
build_processor_hierarchy_node(
table_data,
(1 << 1) | /* ACPI Processor ID valid */
(1 << 3), /* Node is a Leaf */
- socket_offset, uid++, NULL, 0);
+ parent_offset, uid++, NULL, 0);
}
}
}
+ length = g_queue_get_length(list);
+ for (i = 0; i < length; i++) {
+ int thread;
+
+ parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
+ for (thread = 0; thread < ms->smp.threads; thread++) {
+ build_processor_hierarchy_node(
+ table_data,
+ (1 << 1) | /* ACPI Processor ID valid */
+ (1 << 2) | /* Processor is a Thread */
+ (1 << 3), /* Node is a Leaf */
+ parent_offset, uid++, NULL, 0);
+ }
+ }
+
+ g_queue_free(list);
acpi_table_end(linker, &table);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 08/38] tests/acpi/bios-tables-test: Allow changes to virt/PPTT file
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (6 preceding siblings ...)
2022-01-20 12:35 ` [PULL 07/38] hw/acpi/aml-build: Improve scalability of PPTT generation Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 09/38] hw/acpi/aml-build: Support cluster level in PPTT generation Peter Maydell
` (29 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
List test/data/acpi/virt/PPTT as the expected files allowed to
be changed in tests/qtest/bios-tables-test-allowed-diff.h
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
Message-id: 20220107083232.16256-5-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf..cb143a55a64 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/PPTT",
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 09/38] hw/acpi/aml-build: Support cluster level in PPTT generation
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (7 preceding siblings ...)
2022-01-20 12:36 ` [PULL 08/38] tests/acpi/bios-tables-test: Allow changes to virt/PPTT file Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 10/38] tests/acpi/bios-table-test: Update expected virt/PPTT file Peter Maydell
` (28 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
Support CPU cluster topology level in generation of ACPI
Processor Properties Topology Table (PPTT).
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20220107083232.16256-6-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/acpi/aml-build.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 6aaedca2e56..bb2cad63b53 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2001,6 +2001,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
const char *oem_id, const char *oem_table_id)
{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
GQueue *list = g_queue_new();
guint pptt_start = table_data->len;
guint parent_offset;
@@ -2025,6 +2026,23 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
0, socket, NULL, 0);
}
+ if (mc->smp_props.clusters_supported) {
+ length = g_queue_get_length(list);
+ for (i = 0; i < length; i++) {
+ int cluster;
+
+ parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
+ for (cluster = 0; cluster < ms->smp.clusters; cluster++) {
+ g_queue_push_tail(list,
+ GUINT_TO_POINTER(table_data->len - pptt_start));
+ build_processor_hierarchy_node(
+ table_data,
+ (0 << 0), /* not a physical package */
+ parent_offset, cluster, NULL, 0);
+ }
+ }
+ }
+
length = g_queue_get_length(list);
for (i = 0; i < length; i++) {
int core;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 10/38] tests/acpi/bios-table-test: Update expected virt/PPTT file
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (8 preceding siblings ...)
2022-01-20 12:36 ` [PULL 09/38] hw/acpi/aml-build: Support cluster level in PPTT generation Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 11/38] docs/can: convert to restructuredText Peter Maydell
` (27 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Yanan Wang <wangyanan55@huawei.com>
Run ./tests/data/acpi/rebuild-expected-aml.sh from build directory
to update PPTT binary. Also empty bios-tables-test-allowed-diff.h.
The disassembled differences between actual and expected PPTT:
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20200528 (64-bit version)
* Copyright (c) 2000 - 2020 Intel Corporation
*
- * Disassembly of tests/data/acpi/virt/PPTT, Tue Jan 4 12:51:11 2022
+ * Disassembly of /tmp/aml-2ZGOF1, Tue Jan 4 12:51:11 2022
*
* ACPI Data Table [PPTT]
*
* Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
*/
[000h 0000 4] Signature : "PPTT" [Processor Properties Topology Table]
-[004h 0004 4] Table Length : 0000004C
+[004h 0004 4] Table Length : 00000060
[008h 0008 1] Revision : 02
-[009h 0009 1] Checksum : A8
+[009h 0009 1] Checksum : 48
[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 1] Subtable Type : 00 [Processor Hierarchy Node]
[025h 0037 1] Length : 14
[026h 0038 2] Reserved : 0000
[028h 0040 4] Flags (decoded below) : 00000001
Physical package : 1
ACPI Processor ID valid : 0
Processor is a thread : 0
Node is a leaf : 0
Identical Implementation : 0
[02Ch 0044 4] Parent : 00000000
[030h 0048 4] ACPI Processor ID : 00000000
[034h 0052 4] Private Resource Number : 00000000
[038h 0056 1] Subtable Type : 00 [Processor Hierarchy Node]
[039h 0057 1] Length : 14
[03Ah 0058 2] Reserved : 0000
-[03Ch 0060 4] Flags (decoded below) : 0000000A
+[03Ch 0060 4] Flags (decoded below) : 00000000
Physical package : 0
- ACPI Processor ID valid : 1
+ ACPI Processor ID valid : 0
Processor is a thread : 0
- Node is a leaf : 1
+ Node is a leaf : 0
Identical Implementation : 0
[040h 0064 4] Parent : 00000024
[044h 0068 4] ACPI Processor ID : 00000000
[048h 0072 4] Private Resource Number : 00000000
-Raw Table Data: Length 76 (0x4C)
+[04Ch 0076 1] Subtable Type : 00 [Processor Hierarchy Node]
+[04Dh 0077 1] Length : 14
+[04Eh 0078 2] Reserved : 0000
+[050h 0080 4] Flags (decoded below) : 0000000A
+ Physical package : 0
+ ACPI Processor ID valid : 1
+ Processor is a thread : 0
+ Node is a leaf : 1
+ Identical Implementation : 0
+[054h 0084 4] Parent : 00000038
+[058h 0088 4] ACPI Processor ID : 00000000
+[05Ch 0092 4] Private Resource Number : 00000000
+
+Raw Table Data: Length 96 (0x60)
- 0000: 50 50 54 54 4C 00 00 00 02 A8 42 4F 43 48 53 20 // PPTTL.....BOCHS
+ 0000: 50 50 54 54 60 00 00 00 02 48 42 4F 43 48 53 20 // PPTT`....HBOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPC ....BXPC
0020: 01 00 00 00 00 14 00 00 01 00 00 00 00 00 00 00 // ................
- 0030: 00 00 00 00 00 00 00 00 00 14 00 00 0A 00 00 00 // ................
- 0040: 24 00 00 00 00 00 00 00 00 00 00 00 // $...........
+ 0030: 00 00 00 00 00 00 00 00 00 14 00 00 00 00 00 00 // ................
+ 0040: 24 00 00 00 00 00 00 00 00 00 00 00 00 14 00 00 // $...............
+ 0050: 0A 00 00 00 38 00 00 00 00 00 00 00 00 00 00 00 // ....8...........
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Message-id: 20220107083232.16256-7-wangyanan55@huawei.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/virt/PPTT | Bin 76 -> 96 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 cb143a55a64..dfb8523c8bf 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/PPTT",
diff --git a/tests/data/acpi/virt/PPTT b/tests/data/acpi/virt/PPTT
index 7a1258ecf123555b24462c98ccbb76b4ac1d0c2b..f56ea63b369a604877374ad696c396e796ab1c83 100644
GIT binary patch
delta 53
pcmeZC;0g!`2}xjJU|{l?$YrDgWH5jU5Ca567#O&Klm(arApowi1QY-O
delta 32
fcmYfB;R*-{3GrcIU|?D?k;`ae01J-_kOKn%ZFdCM
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 11/38] docs/can: convert to restructuredText
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (9 preceding siblings ...)
2022-01-20 12:36 ` [PULL 10/38] tests/acpi/bios-table-test: Update expected virt/PPTT file Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 12/38] virtio-mem: Correct default THP size for ARM64 Peter Maydell
` (26 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Lucas Ramage <lucas.ramage@infinite-omicron.com>
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/527
Signed-off-by: Lucas Ramage <lucas.ramage@infinite-omicron.com>
Message-id: 20220105205628.5491-1-oxr463@gmx.us
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: Move to docs/system/devices/ rather than top-level;
fix a pre-existing typo in passing]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
docs/system/device-emulation.rst | 1 +
docs/{can.txt => system/devices/can.rst} | 90 +++++++++++-------------
2 files changed, 41 insertions(+), 50 deletions(-)
rename docs/{can.txt => system/devices/can.rst} (68%)
diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst
index 19944f526ce..0b3a3d73ad1 100644
--- a/docs/system/device-emulation.rst
+++ b/docs/system/device-emulation.rst
@@ -82,6 +82,7 @@ Emulated Devices
.. toctree::
:maxdepth: 1
+ devices/can.rst
devices/ivshmem.rst
devices/net.rst
devices/nvme.rst
diff --git a/docs/can.txt b/docs/system/devices/can.rst
similarity index 68%
rename from docs/can.txt
rename to docs/system/devices/can.rst
index 0d310237dfa..16d72c3ac37 100644
--- a/docs/can.txt
+++ b/docs/system/devices/can.rst
@@ -1,6 +1,5 @@
-QEMU CAN bus emulation support
-==============================
-
+CAN Bus Emulation Support
+=========================
The CAN bus emulation provides mechanism to connect multiple
emulated CAN controller chips together by one or multiple CAN busses
(the controller device "canbus" parameter). The individual busses
@@ -32,34 +31,39 @@ emulated environment for testing and RTEMS GSoC slot has been donated
to work on CAN hardware emulation on QEMU.
Examples how to use CAN emulation for SJA1000 based boards
-==========================================================
-
+----------------------------------------------------------
When QEMU with CAN PCI support is compiled then one of the next
CAN boards can be selected
- (1) CAN bus Kvaser PCI CAN-S (single SJA1000 channel) boad. QEMU startup options
+(1) CAN bus Kvaser PCI CAN-S (single SJA1000 channel) board. QEMU startup options::
+
-object can-bus,id=canbus0
-device kvaser_pci,canbus=canbus0
- Add "can-host-socketcan" object to connect device to host system CAN bus
+
+Add "can-host-socketcan" object to connect device to host system CAN bus::
+
-object can-host-socketcan,id=canhost0,if=can0,canbus=canbus0
- (2) CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation
+(2) CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation::
+
-object can-bus,id=canbus0
-device pcm3680_pci,canbus0=canbus0,canbus1=canbus0
- another example:
+Another example::
+
-object can-bus,id=canbus0
-object can-bus,id=canbus1
-device pcm3680_pci,canbus0=canbus0,canbus1=canbus1
- (3) CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation
+(3) CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation::
+
-device mioe3680_pci,canbus0=canbus0
-
The ''kvaser_pci'' board/device model is compatible with and has been tested with
-''kvaser_pci'' driver included in mainline Linux kernel.
+the ''kvaser_pci'' driver included in mainline Linux kernel.
The tested setup was Linux 4.9 kernel on the host and guest side.
-Example for qemu-system-x86_64:
+
+Example for qemu-system-x86_64::
qemu-system-x86_64 -accel kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \
-initrd ramdisk.cpio \
@@ -69,7 +73,7 @@ Example for qemu-system-x86_64:
-device kvaser_pci,canbus=canbus0 \
-nographic -append "console=ttyS0"
-Example for qemu-system-arm:
+Example for qemu-system-arm::
qemu-system-arm -cpu arm1176 -m 256 -M versatilepb \
-kernel kernel-qemu-arm1176-versatilepb \
@@ -84,24 +88,23 @@ Example for qemu-system-arm:
The CAN interface of the host system has to be configured for proper
bitrate and set up. Configuration is not propagated from emulated
devices through bus to the physical host device. Example configuration
-for 1 Mbit/s
+for 1 Mbit/s::
ip link set can0 type can bitrate 1000000
ip link set can0 up
Virtual (host local only) can interface can be used on the host
-side instead of physical interface
+side instead of physical interface::
ip link add dev can0 type vcan
The CAN interface on the host side can be used to analyze CAN
-traffic with "candump" command which is included in "can-utils".
+traffic with "candump" command which is included in "can-utils"::
candump can0
CTU CAN FD support examples
-===========================
-
+---------------------------
This open-source core provides CAN FD support. CAN FD drames are
delivered even to the host systems when SocketCAN interface is found
CAN FD capable.
@@ -113,7 +116,7 @@ on the board.
Example how to connect the canbus0-bus (virtual wire) to the host
Linux system (SocketCAN used) and to both CTU CAN FD cores emulated
on the corresponding PCI card expects that host system CAN bus
-is setup according to the previous SJA1000 section.
+is setup according to the previous SJA1000 section::
qemu-system-x86_64 -enable-kvm -kernel /boot/vmlinuz-4.19.52+ \
-initrd ramdisk.cpio \
@@ -125,7 +128,7 @@ is setup according to the previous SJA1000 section.
-device ctucan_pci,canbus0=canbus0-bus,canbus1=canbus0-bus \
-nographic
-Setup of CTU CAN FD controller in a guest Linux system
+Setup of CTU CAN FD controller in a guest Linux system::
insmod ctucanfd.ko || modprobe ctucanfd
insmod ctucanfd_pci.ko || modprobe ctucanfd_pci
@@ -150,19 +153,19 @@ Setup of CTU CAN FD controller in a guest Linux system
/bin/ip link set $ifc up
done
-The test can run for example
+The test can run for example::
candump can1
-in the guest system and next commands in the host system for basic CAN
+in the guest system and next commands in the host system for basic CAN::
cangen can0
-for CAN FD without bitrate switch
+for CAN FD without bitrate switch::
cangen can0 -f
-and with bitrate switch
+and with bitrate switch::
cangen can0 -b
@@ -170,29 +173,16 @@ The test can be run viceversa, generate messages in the guest system and capture
in the host one and much more combinations.
Links to other resources
-========================
+------------------------
- (1) CAN related projects at Czech Technical University, Faculty of Electrical Engineering
- http://canbus.pages.fel.cvut.cz/
- (2) Repository with development can-pci branch at Czech Technical University
- https://gitlab.fel.cvut.cz/canbus/qemu-canbus
- (3) RTEMS page describing project
- https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation
- (4) RTLWS 2015 article about the project and its use with CANopen emulation
- http://cmp.felk.cvut.cz/~pisa/can/doc/rtlws-17-pisa-qemu-can.pdf
- (5) GNU/Linux, CAN and CANopen in Real-time Control Applications
- Slides from LinuxDays 2017 (include updated RTLWS 2015 content)
- https://www.linuxdays.cz/2017/video/Pavel_Pisa-CAN_canopen.pdf
- (6) Linux SocketCAN utilities
- https://github.com/linux-can/can-utils/
- (7) CTU CAN FD project including core VHDL design, Linux driver,
- test utilities etc.
- https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core
- (8) CTU CAN FD Core Datasheet Documentation
- http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf
- (9) CTU CAN FD Core System Architecture Documentation
- http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/ctu_can_fd_architecture.pdf
- (10) CTU CAN FD Driver Documentation
- http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/driver_doc/ctucanfd-driver.html
- (11) Integration with PCIe interfacing for Intel/Altera Cyclone IV based board
- https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd
+ (1) `CAN related projects at Czech Technical University, Faculty of Electrical Engineering <http://canbus.pages.fel.cvut.cz>`_
+ (2) `Repository with development can-pci branch at Czech Technical University <https://gitlab.fel.cvut.cz/canbus/qemu-canbus>`_
+ (3) `RTEMS page describing project <https://devel.rtems.org/wiki/Developer/Simulators/QEMU/CANEmulation>`_
+ (4) `RTLWS 2015 article about the project and its use with CANopen emulation <http://cmp.felk.cvut.cz/~pisa/can/doc/rtlws-17-pisa-qemu-can.pdf>`_
+ (5) `GNU/Linux, CAN and CANopen in Real-time Control Applications Slides from LinuxDays 2017 (include updated RTLWS 2015 content) <https://www.linuxdays.cz/2017/video/Pavel_Pisa-CAN_canopen.pdf>`_
+ (6) `Linux SocketCAN utilities <https://github.com/linux-can/can-utils>`_
+ (7) `CTU CAN FD project including core VHDL design, Linux driver, test utilities etc. <https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core>`_
+ (8) `CTU CAN FD Core Datasheet Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf>`_
+ (9) `CTU CAN FD Core System Architecture Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/ctu_can_fd_architecture.pdf>`_
+ (10) `CTU CAN FD Driver Documentation <http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/driver_doc/ctucanfd-driver.html>`_
+ (11) `Integration with PCIe interfacing for Intel/Altera Cyclone IV based board <https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd>`_
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 12/38] virtio-mem: Correct default THP size for ARM64
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (10 preceding siblings ...)
2022-01-20 12:36 ` [PULL 11/38] docs/can: convert to restructuredText Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 13/38] hw/arm/virt: Support for virtio-mem-pci Peter Maydell
` (25 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Gavin Shan <gshan@redhat.com>
The default block size is same as to the THP size, which is either
retrieved from "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
or hardcoded to 2MB. There are flaws in both mechanisms and this
intends to fix them up.
* When "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" is
used to getting the THP size, 32MB and 512MB are valid values
when we have 16KB and 64KB page size on ARM64.
* When the hardcoded THP size is used, 2MB, 32MB and 512MB are
valid values when we have 4KB, 16KB and 64KB page sizes on
ARM64.
Co-developed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Message-id: 20220111063329.74447-2-gshan@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/virtio/virtio-mem.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 04c223b0c97..1ca45336dc2 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -46,14 +46,25 @@
*/
#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)(1 * MiB))
-#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__) || \
- defined(__powerpc64__)
-#define VIRTIO_MEM_DEFAULT_THP_SIZE ((uint32_t)(2 * MiB))
-#else
- /* fallback to 1 MiB (e.g., the THP size on s390x) */
-#define VIRTIO_MEM_DEFAULT_THP_SIZE VIRTIO_MEM_MIN_BLOCK_SIZE
+static uint32_t virtio_mem_default_thp_size(void)
+{
+ uint32_t default_thp_size = VIRTIO_MEM_MIN_BLOCK_SIZE;
+
+#if defined(__x86_64__) || defined(__arm__) || defined(__powerpc64__)
+ default_thp_size = 2 * MiB;
+#elif defined(__aarch64__)
+ if (qemu_real_host_page_size == 4 * KiB) {
+ default_thp_size = 2 * MiB;
+ } else if (qemu_real_host_page_size == 16 * KiB) {
+ default_thp_size = 32 * MiB;
+ } else if (qemu_real_host_page_size == 64 * KiB) {
+ default_thp_size = 512 * MiB;
+ }
#endif
+ return default_thp_size;
+}
+
/*
* We want to have a reasonable default block size such that
* 1. We avoid splitting THPs when unplugging memory, which degrades
@@ -86,11 +97,8 @@ static uint32_t virtio_mem_thp_size(void)
if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, &content, NULL, NULL) &&
!qemu_strtou64(content, &endptr, 0, &tmp) &&
(!endptr || *endptr == '\n')) {
- /*
- * Sanity-check the value, if it's too big (e.g., aarch64 with 64k base
- * pages) or weird, fallback to something smaller.
- */
- if (!tmp || !is_power_of_2(tmp) || tmp > 16 * MiB) {
+ /* Sanity-check the value and fallback to something reasonable. */
+ if (!tmp || !is_power_of_2(tmp)) {
warn_report("Read unsupported THP size: %" PRIx64, tmp);
} else {
thp_size = tmp;
@@ -98,7 +106,7 @@ static uint32_t virtio_mem_thp_size(void)
}
if (!thp_size) {
- thp_size = VIRTIO_MEM_DEFAULT_THP_SIZE;
+ thp_size = virtio_mem_default_thp_size();
warn_report("Could not detect THP size, falling back to %" PRIx64
" MiB.", thp_size / MiB);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 13/38] hw/arm/virt: Support for virtio-mem-pci
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (11 preceding siblings ...)
2022-01-20 12:36 ` [PULL 12/38] virtio-mem: Correct default THP size for ARM64 Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 14/38] hw/intc/arm_gic: Implement read of GICC_IIDR Peter Maydell
` (24 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Gavin Shan <gshan@redhat.com>
This supports virtio-mem-pci device on "virt" platform, by simply
following the implementation on x86.
* This implements the hotplug handlers to support virtio-mem-pci
device hot-add, while the hot-remove isn't supported as we have
on x86.
* The block size is 512MB on ARM64 instead of 128MB on x86.
* It has been passing the tests with various combinations like 64KB
and 4KB page sizes on host and guest, different memory device
backends like normal, transparent huge page and HugeTLB, plus
migration.
Co-developed-by: David Hildenbrand <david@redhat.com>
Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Message-id: 20220111063329.74447-3-gshan@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 70 ++++++++++++++++++++++++++++++++++++++++++
hw/virtio/virtio-mem.c | 4 ++-
hw/arm/Kconfig | 1 +
3 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 177db1da12e..a76d86b5926 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -71,9 +71,11 @@
#include "hw/arm/smmuv3.h"
#include "hw/acpi/acpi.h"
#include "target/arm/internals.h"
+#include "hw/mem/memory-device.h"
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
#include "hw/acpi/generic_event_device.h"
+#include "hw/virtio/virtio-mem-pci.h"
#include "hw/virtio/virtio-iommu.h"
#include "hw/char/pl011.h"
#include "qemu/guest-random.h"
@@ -2501,6 +2503,64 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
dev, &error_abort);
}
+static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
+ Error *local_err = NULL;
+
+ if (!hotplug_dev2 && dev->hotplugged) {
+ /*
+ * Without a bus hotplug handler, we cannot control the plug/unplug
+ * order. We should never reach this point when hotplugging on ARM.
+ * However, it's nice to add a safety net, similar to what we have
+ * on x86.
+ */
+ error_setg(errp, "hotplug of virtio based memory devices not supported"
+ " on this bus.");
+ return;
+ }
+ /*
+ * First, see if we can plug this memory device at all. If that
+ * succeeds, branch of to the actual hotplug handler.
+ */
+ memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
+ &local_err);
+ if (!local_err && hotplug_dev2) {
+ hotplug_handler_pre_plug(hotplug_dev2, dev, &local_err);
+ }
+ error_propagate(errp, local_err);
+}
+
+static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
+ Error *local_err = NULL;
+
+ /*
+ * Plug the memory device first and then branch off to the actual
+ * hotplug handler. If that one fails, we can easily undo the memory
+ * device bits.
+ */
+ memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
+ if (hotplug_dev2) {
+ hotplug_handler_plug(hotplug_dev2, dev, &local_err);
+ if (local_err) {
+ memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
+ }
+ }
+ error_propagate(errp, local_err);
+}
+
+static void virt_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ /* We don't support hot unplug of virtio based memory devices */
+ error_setg(errp, "virtio based memory devices cannot be unplugged.");
+}
+
+
static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -2508,6 +2568,8 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *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_MEM_PCI)) {
+ virt_virtio_md_pci_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;
@@ -2559,6 +2621,11 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
virt_memory_plug(hotplug_dev, dev, errp);
}
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+ virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
+ }
+
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
PCIDevice *pdev = PCI_DEVICE(dev);
@@ -2615,6 +2682,8 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
{
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
virt_dimm_unplug_request(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+ virt_virtio_md_pci_unplug_request(hotplug_dev, dev, errp);
} else {
error_setg(errp, "device unplug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -2639,6 +2708,7 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
if (device_is_dynamic_sysbus(mc, dev) ||
object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+ object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
return HOTPLUG_HANDLER(machine);
}
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 1ca45336dc2..f55dcf61f20 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -146,7 +146,7 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
* The memory block size corresponds mostly to the section size.
*
* This allows e.g., to add 20MB with a section size of 128MB on x86_64, and
- * a section size of 1GB on arm64 (as long as the start address is properly
+ * a section size of 512MB on arm64 (as long as the start address is properly
* aligned, similar to ordinary DIMMs).
*
* We can change this at any time and maybe even make it configurable if
@@ -155,6 +155,8 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
*/
#if defined(TARGET_X86_64) || defined(TARGET_I386)
#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
+#elif defined(TARGET_ARM)
+#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
#else
#error VIRTIO_MEM_USABLE_EXTENT not defined
#endif
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index c459c897cc7..2e0049196d6 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM_VIRT
select ACPI_HW_REDUCED
select ACPI_APEI
select ACPI_VIOT
+ select VIRTIO_MEM_SUPPORTED
config CHEETAH
bool
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 14/38] hw/intc/arm_gic: Implement read of GICC_IIDR
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (12 preceding siblings ...)
2022-01-20 12:36 ` [PULL 13/38] hw/arm/virt: Support for virtio-mem-pci Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 15/38] hw/intc/arm_gic: Allow reset of the running priority Peter Maydell
` (23 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Petr Pavlu <petr.pavlu@suse.com>
Implement support for reading GICC_IIDR. This register is used by the
Linux kernel to recognize that GICv2 with GICC_APRn is present.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Message-id: 20220113151916.17978-2-ppavlu@suse.cz
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/arm_gic.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a994b1f0245..0cd9ceca8d1 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -1662,6 +1662,15 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
}
break;
}
+ case 0xfc:
+ if (s->revision == REV_11MPCORE) {
+ /* Reserved on 11MPCore */
+ *data = 0;
+ } else {
+ /* GICv1 or v2; Arm implementation */
+ *data = (s->revision << 16) | 0x43b;
+ }
+ break;
default:
qemu_log_mask(LOG_GUEST_ERROR,
"gic_cpu_read: Bad offset %x\n", (int)offset);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 15/38] hw/intc/arm_gic: Allow reset of the running priority
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (13 preceding siblings ...)
2022-01-20 12:36 ` [PULL 14/38] hw/intc/arm_gic: Implement read of GICC_IIDR Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 16/38] hw/arm/virt: Add a control for the the highmem PCIe MMIO Peter Maydell
` (22 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Petr Pavlu <petr.pavlu@suse.com>
When running Linux on a machine with GICv2, the kernel can crash while
processing an interrupt and can subsequently start a kdump kernel from
the active interrupt handler. In such a case, the crashed kernel might
not gracefully signal the end of interrupt to the GICv2 hardware. The
kdump kernel will however try to reset the GIC state on startup to get
the controller into a sane state, in particular the kernel writes ones
to GICD_ICACTIVERn and wipes out GICC_APRn to make sure that no
interrupt is active.
The patch adds a logic to recalculate the running priority when
GICC_APRn/GICC_NSAPRn is written which makes sure that the mentioned
reset works with the GICv2 emulation in QEMU too and the kdump kernel
starts receiving interrupts.
The described scenario can be reproduced on an AArch64 QEMU virt machine
with a kdump-enabled Linux system by using the softdog module. The kdump
kernel will hang at some point because QEMU still thinks the running
priority is that of the timer interrupt and asserts no new interrupts to
the system:
$ modprobe softdog soft_margin=10 soft_panic=1
$ cat > /dev/watchdog
[Press Enter to start the watchdog, wait for its timeout and observe
that the kdump kernel hangs on startup.]
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Message-id: 20220113151916.17978-3-ppavlu@suse.cz
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/arm_gic.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 0cd9ceca8d1..492b2421ab4 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -1736,6 +1736,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
} else {
s->apr[regno][cpu] = value;
}
+ s->running_priority[cpu] = gic_get_prio_from_apr_bits(s, cpu);
break;
}
case 0xe0: case 0xe4: case 0xe8: case 0xec:
@@ -1752,6 +1753,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
return MEMTX_OK;
}
s->nsapr[regno][cpu] = value;
+ s->running_priority[cpu] = gic_get_prio_from_apr_bits(s, cpu);
break;
}
case 0x1000:
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 16/38] hw/arm/virt: Add a control for the the highmem PCIe MMIO
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (14 preceding siblings ...)
2022-01-20 12:36 ` [PULL 15/38] hw/intc/arm_gic: Allow reset of the running priority Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 17/38] hw/arm/virt: Add a control for the the highmem redistributors Peter Maydell
` (21 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
Just like we can control the enablement of the highmem PCIe ECAM
region using highmem_ecam, let's add a control for the highmem
PCIe MMIO region.
Similarily to highmem_ecam, this region is disabled when highmem
is off.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20220114140741.1358263-2-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/virt.h | 1 +
hw/arm/virt-acpi-build.c | 10 ++++------
hw/arm/virt.c | 7 +++++--
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index be0534608f8..cf5d8b83ded 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -143,6 +143,7 @@ struct VirtMachineState {
bool secure;
bool highmem;
bool highmem_ecam;
+ bool highmem_mmio;
bool its;
bool tcg_its;
bool virt;
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f2514ce77c0..449fab00805 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -158,10 +158,9 @@ static void acpi_dsdt_add_virtio(Aml *scope,
}
static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
- uint32_t irq, bool use_highmem, bool highmem_ecam,
- VirtMachineState *vms)
+ uint32_t irq, VirtMachineState *vms)
{
- int ecam_id = VIRT_ECAM_ID(highmem_ecam);
+ int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
struct GPEXConfig cfg = {
.mmio32 = memmap[VIRT_PCIE_MMIO],
.pio = memmap[VIRT_PCIE_PIO],
@@ -170,7 +169,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
.bus = vms->bus,
};
- if (use_highmem) {
+ if (vms->highmem_mmio) {
cfg.mmio64 = memmap[VIRT_HIGH_PCIE_MMIO];
}
@@ -869,8 +868,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
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);
- acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
- vms->highmem, vms->highmem_ecam, vms);
+ acpi_dsdt_add_pci(scope, memmap, irqmap[VIRT_PCIE] + ARM_SPI_BASE, vms);
if (vms->acpi_dev) {
build_ged_aml(scope, "\\_SB."GED_DEVICE,
HOTPLUG_HANDLER(vms->acpi_dev),
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a76d86b5926..16369ce10e4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1419,7 +1419,7 @@ static void create_pcie(VirtMachineState *vms)
mmio_reg, base_mmio, size_mmio);
memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
- if (vms->highmem) {
+ if (vms->highmem_mmio) {
/* Map high MMIO space */
MemoryRegion *high_mmio_alias = g_new0(MemoryRegion, 1);
@@ -1473,7 +1473,7 @@ static void create_pcie(VirtMachineState *vms)
qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
2, base_ecam, 2, size_ecam);
- if (vms->highmem) {
+ if (vms->highmem_mmio) {
qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "ranges",
1, FDT_PCI_RANGE_IOPORT, 2, 0,
2, base_pio, 2, size_pio,
@@ -2112,6 +2112,8 @@ static void machvirt_init(MachineState *machine)
virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
+ vms->highmem_mmio &= vms->highmem;
+
create_gic(vms, sysmem);
virt_cpu_post_init(vms, sysmem);
@@ -2899,6 +2901,7 @@ static void virt_instance_init(Object *obj)
vms->gic_version = VIRT_GIC_VERSION_NOSEL;
vms->highmem_ecam = !vmc->no_highmem_ecam;
+ vms->highmem_mmio = true;
if (vmc->no_its) {
vms->its = false;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 17/38] hw/arm/virt: Add a control for the the highmem redistributors
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (15 preceding siblings ...)
2022-01-20 12:36 ` [PULL 16/38] hw/arm/virt: Add a control for the the highmem PCIe MMIO Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map Peter Maydell
` (20 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
Just like we can control the enablement of the highmem PCIe region
using highmem_ecam, let's add a control for the highmem GICv3
redistributor region.
Similarily to highmem_ecam, these redistributors are disabled when
highmem is off.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20220114140741.1358263-3-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/virt.h | 4 +++-
hw/arm/virt-acpi-build.c | 2 ++
hw/arm/virt.c | 2 ++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index cf5d8b83ded..c1ea17d0def 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -144,6 +144,7 @@ struct VirtMachineState {
bool highmem;
bool highmem_ecam;
bool highmem_mmio;
+ bool highmem_redists;
bool its;
bool tcg_its;
bool virt;
@@ -191,7 +192,8 @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms)
assert(vms->gic_version == VIRT_GIC_VERSION_3);
- return MACHINE(vms)->smp.cpus > redist0_capacity ? 2 : 1;
+ return (MACHINE(vms)->smp.cpus > redist0_capacity &&
+ vms->highmem_redists) ? 2 : 1;
}
#endif /* QEMU_ARM_VIRT_H */
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 449fab00805..0757c28f69c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -947,6 +947,8 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
+ vms->highmem_redists &= vms->highmem;
+
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 16369ce10e4..62bdce1eb4b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2113,6 +2113,7 @@ static void machvirt_init(MachineState *machine)
virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
vms->highmem_mmio &= vms->highmem;
+ vms->highmem_redists &= vms->highmem;
create_gic(vms, sysmem);
@@ -2902,6 +2903,7 @@ static void virt_instance_init(Object *obj)
vms->highmem_ecam = !vmc->no_highmem_ecam;
vms->highmem_mmio = true;
+ vms->highmem_redists = true;
if (vmc->no_its) {
vms->its = false;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (16 preceding siblings ...)
2022-01-20 12:36 ` [PULL 17/38] hw/arm/virt: Add a control for the the highmem redistributors Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-02-13 5:05 ` Akihiko Odaki
2022-01-20 12:36 ` [PULL 19/38] hw/arm/virt: Use the PA range to compute " Peter Maydell
` (19 subsequent siblings)
37 siblings, 1 reply; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
Even when the VM is configured with highmem=off, the highest_gpa
field includes devices that are above the 4GiB limit.
Similarily, nothing seem to check that the memory is within
the limit set by the highmem=off option.
This leads to failures in virt_kvm_type() on systems that have
a crippled IPA range, as the reported IPA space is larger than
what it should be.
Instead, honor the user-specified limit to only use the devices
at the lowest end of the spectrum, and fail if we have memory
crossing the 4GiB limit.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Message-id: 20220114140741.1358263-4-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 62bdce1eb4b..3b839ba78ba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
static void virt_set_memmap(VirtMachineState *vms)
{
MachineState *ms = MACHINE(vms);
- hwaddr base, device_memory_base, device_memory_size;
+ hwaddr base, device_memory_base, device_memory_size, memtop;
int i;
vms->memmap = extended_memmap;
@@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
/* Base address of the high IO region */
- base = device_memory_base + ROUND_UP(device_memory_size, GiB);
+ memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
+ if (!vms->highmem && memtop > 4 * GiB) {
+ error_report("highmem=off, but memory crosses the 4GiB limit\n");
+ exit(EXIT_FAILURE);
+ }
if (base < device_memory_base) {
error_report("maxmem/slots too huge");
exit(EXIT_FAILURE);
@@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
vms->memmap[i].size = size;
base += size;
}
- vms->highest_gpa = base - 1;
+ vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
if (device_memory_size > 0) {
ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
ms->device_memory->base = device_memory_base;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-01-20 12:36 ` [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map Peter Maydell
2022-02-13 5:05 ` Akihiko Odaki
@ 2022-02-13 5:05 ` Akihiko Odaki
0 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 5:05 UTC (permalink / raw)
To: Marc Zyngier
Cc: qemu-devel, Peter Maydell, Andrew Jones, Eric Auger,
Peter Maydell, kvmarm, kvm, kernel-team
On 2022/01/20 21:36, Peter Maydell wrote:
> From: Marc Zyngier <maz@kernel.org>
>
> Even when the VM is configured with highmem=off, the highest_gpa
> field includes devices that are above the 4GiB limit.
> Similarily, nothing seem to check that the memory is within
> the limit set by the highmem=off option.
>
> This leads to failures in virt_kvm_type() on systems that have
> a crippled IPA range, as the reported IPA space is larger than
> what it should be.
>
> Instead, honor the user-specified limit to only use the devices
> at the lowest end of the spectrum, and fail if we have memory
> crossing the 4GiB limit.
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Message-id: 20220114140741.1358263-4-maz@kernel.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/virt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 62bdce1eb4b..3b839ba78ba 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> static void virt_set_memmap(VirtMachineState *vms)
> {
> MachineState *ms = MACHINE(vms);
> - hwaddr base, device_memory_base, device_memory_size;
> + hwaddr base, device_memory_base, device_memory_size, memtop;
> int i;
>
> vms->memmap = extended_memmap;
> @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
>
> /* Base address of the high IO region */
> - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + if (!vms->highmem && memtop > 4 * GiB) {
> + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> + exit(EXIT_FAILURE);
> + }
> if (base < device_memory_base) {
> error_report("maxmem/slots too huge");
> exit(EXIT_FAILURE);
> @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> vms->memmap[i].size = size;
> base += size;
> }
> - vms->highest_gpa = base - 1;
> + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> if (device_memory_size > 0) {
> ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> ms->device_memory->base = device_memory_base;
Hi,
This breaks in a case where highmem is disabled but can have more than 4
GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
which is not enough for highmem MMIO but is enough to contain 32 GiB of RAM.
Where the magic number of 4 GiB / 32-bit came from? I also don't quite
understand what failures virt_kvm_type() had.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 5:05 ` Akihiko Odaki
0 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 5:05 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kvm, qemu-devel, kernel-team, kvmarm
On 2022/01/20 21:36, Peter Maydell wrote:
> From: Marc Zyngier <maz@kernel.org>
>
> Even when the VM is configured with highmem=off, the highest_gpa
> field includes devices that are above the 4GiB limit.
> Similarily, nothing seem to check that the memory is within
> the limit set by the highmem=off option.
>
> This leads to failures in virt_kvm_type() on systems that have
> a crippled IPA range, as the reported IPA space is larger than
> what it should be.
>
> Instead, honor the user-specified limit to only use the devices
> at the lowest end of the spectrum, and fail if we have memory
> crossing the 4GiB limit.
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Message-id: 20220114140741.1358263-4-maz@kernel.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/virt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 62bdce1eb4b..3b839ba78ba 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> static void virt_set_memmap(VirtMachineState *vms)
> {
> MachineState *ms = MACHINE(vms);
> - hwaddr base, device_memory_base, device_memory_size;
> + hwaddr base, device_memory_base, device_memory_size, memtop;
> int i;
>
> vms->memmap = extended_memmap;
> @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
>
> /* Base address of the high IO region */
> - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + if (!vms->highmem && memtop > 4 * GiB) {
> + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> + exit(EXIT_FAILURE);
> + }
> if (base < device_memory_base) {
> error_report("maxmem/slots too huge");
> exit(EXIT_FAILURE);
> @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> vms->memmap[i].size = size;
> base += size;
> }
> - vms->highest_gpa = base - 1;
> + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> if (device_memory_size > 0) {
> ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> ms->device_memory->base = device_memory_base;
Hi,
This breaks in a case where highmem is disabled but can have more than 4
GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
which is not enough for highmem MMIO but is enough to contain 32 GiB of RAM.
Where the magic number of 4 GiB / 32-bit came from? I also don't quite
understand what failures virt_kvm_type() had.
Regards,
Akihiko Odaki
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 5:05 ` Akihiko Odaki
0 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 5:05 UTC (permalink / raw)
To: Marc Zyngier
Cc: Peter Maydell, Andrew Jones, kvm, qemu-devel, Eric Auger,
kernel-team, kvmarm
On 2022/01/20 21:36, Peter Maydell wrote:
> From: Marc Zyngier <maz@kernel.org>
>
> Even when the VM is configured with highmem=off, the highest_gpa
> field includes devices that are above the 4GiB limit.
> Similarily, nothing seem to check that the memory is within
> the limit set by the highmem=off option.
>
> This leads to failures in virt_kvm_type() on systems that have
> a crippled IPA range, as the reported IPA space is larger than
> what it should be.
>
> Instead, honor the user-specified limit to only use the devices
> at the lowest end of the spectrum, and fail if we have memory
> crossing the 4GiB limit.
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Message-id: 20220114140741.1358263-4-maz@kernel.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/virt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 62bdce1eb4b..3b839ba78ba 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> static void virt_set_memmap(VirtMachineState *vms)
> {
> MachineState *ms = MACHINE(vms);
> - hwaddr base, device_memory_base, device_memory_size;
> + hwaddr base, device_memory_base, device_memory_size, memtop;
> int i;
>
> vms->memmap = extended_memmap;
> @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
>
> /* Base address of the high IO region */
> - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> + if (!vms->highmem && memtop > 4 * GiB) {
> + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> + exit(EXIT_FAILURE);
> + }
> if (base < device_memory_base) {
> error_report("maxmem/slots too huge");
> exit(EXIT_FAILURE);
> @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> vms->memmap[i].size = size;
> base += size;
> }
> - vms->highest_gpa = base - 1;
> + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> if (device_memory_size > 0) {
> ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> ms->device_memory->base = device_memory_base;
Hi,
This breaks in a case where highmem is disabled but can have more than 4
GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
which is not enough for highmem MMIO but is enough to contain 32 GiB of RAM.
Where the magic number of 4 GiB / 32-bit came from? I also don't quite
understand what failures virt_kvm_type() had.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-02-13 5:05 ` Akihiko Odaki
(?)
@ 2022-02-13 10:22 ` Marc Zyngier
-1 siblings, 0 replies; 54+ messages in thread
From: Marc Zyngier @ 2022-02-13 10:22 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Andrew Jones, Eric Auger, kvmarm, kvm,
kernel-team, Alexander Graf
[+ Alex for HVF]
On Sun, 13 Feb 2022 05:05:33 +0000,
Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> On 2022/01/20 21:36, Peter Maydell wrote:
> > From: Marc Zyngier <maz@kernel.org>
> >
> > Even when the VM is configured with highmem=off, the highest_gpa
> > field includes devices that are above the 4GiB limit.
> > Similarily, nothing seem to check that the memory is within
> > the limit set by the highmem=off option.
> >
> > This leads to failures in virt_kvm_type() on systems that have
> > a crippled IPA range, as the reported IPA space is larger than
> > what it should be.
> >
> > Instead, honor the user-specified limit to only use the devices
> > at the lowest end of the spectrum, and fail if we have memory
> > crossing the 4GiB limit.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Message-id: 20220114140741.1358263-4-maz@kernel.org
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/arm/virt.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 62bdce1eb4b..3b839ba78ba 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> > static void virt_set_memmap(VirtMachineState *vms)
> > {
> > MachineState *ms = MACHINE(vms);
> > - hwaddr base, device_memory_base, device_memory_size;
> > + hwaddr base, device_memory_base, device_memory_size, memtop;
> > int i;
> > vms->memmap = extended_memmap;
> > @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> > device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
> > /* Base address of the high IO region */
> > - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + if (!vms->highmem && memtop > 4 * GiB) {
> > + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> > + exit(EXIT_FAILURE);
> > + }
> > if (base < device_memory_base) {
> > error_report("maxmem/slots too huge");
> > exit(EXIT_FAILURE);
> > @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> > vms->memmap[i].size = size;
> > base += size;
> > }
> > - vms->highest_gpa = base - 1;
> > + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> > if (device_memory_size > 0) {
> > ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> > ms->device_memory->base = device_memory_base;
>
> Hi,
> This breaks in a case where highmem is disabled but can have more than
> 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> which is not enough for highmem MMIO but is enough to contain 32 GiB
> of RAM.
Funny. The whole point of this series is to make it all work correctly
on M1.
> Where the magic number of 4 GiB / 32-bit came from?
Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
<quote>
highmem
Set ``on``/``off`` to enable/disable placing devices and RAM in physical
address space above 32 bits. The default is ``on`` for machine types
later than ``virt-2.12``.
</quote>
TL;DR: Removing the bogus 'highmem=off' option from your command-line
should get you going with large memory spaces, up to the IPA limit.
The fact that you could run with 32GB of RAM while mandating that the
guest IPA space was limited to 32bit was nothing but a bug, further
"exploited" by HVF to allow disabling the highhmem devices which are
out of reach given the HW limitations (see [1] for details on the
discussion, specially around patch 3).
This is now fixed, and has been extended to work with any IPA size
(including 36bit machines such as M1).
> I also don't quite understand what failures virt_kvm_type() had.
QEMU works by first computing the memory map and passing the required
IPA limit to KVM as part of the VM type. By failing to take into
account the initial limit requirements to the IPA space (either via a
command-line option such as 'highmem', or by using the value provided
by KVM itself), QEMU would try to create a VM that cannot run on the
HW, and KVM would simply return an error.
All of this is documented as part of the KVM/arm64 API [2]. And with
this fixed, QEMU is able to correctly drive KVM on M1.
M.
[1] https://lore.kernel.org/all/20210822144441.1290891-1-maz@kernel.org
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/virt/kvm/api.rst#n138
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 10:22 ` Marc Zyngier
0 siblings, 0 replies; 54+ messages in thread
From: Marc Zyngier @ 2022-02-13 10:22 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Andrew Jones, kvm, qemu-devel, Eric Auger,
Alexander Graf, kernel-team, kvmarm
[+ Alex for HVF]
On Sun, 13 Feb 2022 05:05:33 +0000,
Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> On 2022/01/20 21:36, Peter Maydell wrote:
> > From: Marc Zyngier <maz@kernel.org>
> >
> > Even when the VM is configured with highmem=off, the highest_gpa
> > field includes devices that are above the 4GiB limit.
> > Similarily, nothing seem to check that the memory is within
> > the limit set by the highmem=off option.
> >
> > This leads to failures in virt_kvm_type() on systems that have
> > a crippled IPA range, as the reported IPA space is larger than
> > what it should be.
> >
> > Instead, honor the user-specified limit to only use the devices
> > at the lowest end of the spectrum, and fail if we have memory
> > crossing the 4GiB limit.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Message-id: 20220114140741.1358263-4-maz@kernel.org
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/arm/virt.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 62bdce1eb4b..3b839ba78ba 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> > static void virt_set_memmap(VirtMachineState *vms)
> > {
> > MachineState *ms = MACHINE(vms);
> > - hwaddr base, device_memory_base, device_memory_size;
> > + hwaddr base, device_memory_base, device_memory_size, memtop;
> > int i;
> > vms->memmap = extended_memmap;
> > @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> > device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
> > /* Base address of the high IO region */
> > - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + if (!vms->highmem && memtop > 4 * GiB) {
> > + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> > + exit(EXIT_FAILURE);
> > + }
> > if (base < device_memory_base) {
> > error_report("maxmem/slots too huge");
> > exit(EXIT_FAILURE);
> > @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> > vms->memmap[i].size = size;
> > base += size;
> > }
> > - vms->highest_gpa = base - 1;
> > + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> > if (device_memory_size > 0) {
> > ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> > ms->device_memory->base = device_memory_base;
>
> Hi,
> This breaks in a case where highmem is disabled but can have more than
> 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> which is not enough for highmem MMIO but is enough to contain 32 GiB
> of RAM.
Funny. The whole point of this series is to make it all work correctly
on M1.
> Where the magic number of 4 GiB / 32-bit came from?
Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
<quote>
highmem
Set ``on``/``off`` to enable/disable placing devices and RAM in physical
address space above 32 bits. The default is ``on`` for machine types
later than ``virt-2.12``.
</quote>
TL;DR: Removing the bogus 'highmem=off' option from your command-line
should get you going with large memory spaces, up to the IPA limit.
The fact that you could run with 32GB of RAM while mandating that the
guest IPA space was limited to 32bit was nothing but a bug, further
"exploited" by HVF to allow disabling the highhmem devices which are
out of reach given the HW limitations (see [1] for details on the
discussion, specially around patch 3).
This is now fixed, and has been extended to work with any IPA size
(including 36bit machines such as M1).
> I also don't quite understand what failures virt_kvm_type() had.
QEMU works by first computing the memory map and passing the required
IPA limit to KVM as part of the VM type. By failing to take into
account the initial limit requirements to the IPA space (either via a
command-line option such as 'highmem', or by using the value provided
by KVM itself), QEMU would try to create a VM that cannot run on the
HW, and KVM would simply return an error.
All of this is documented as part of the KVM/arm64 API [2]. And with
this fixed, QEMU is able to correctly drive KVM on M1.
M.
[1] https://lore.kernel.org/all/20210822144441.1290891-1-maz@kernel.org
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/virt/kvm/api.rst#n138
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 10:22 ` Marc Zyngier
0 siblings, 0 replies; 54+ messages in thread
From: Marc Zyngier @ 2022-02-13 10:22 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: kvm, qemu-devel, Alexander Graf, kernel-team, kvmarm
[+ Alex for HVF]
On Sun, 13 Feb 2022 05:05:33 +0000,
Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> On 2022/01/20 21:36, Peter Maydell wrote:
> > From: Marc Zyngier <maz@kernel.org>
> >
> > Even when the VM is configured with highmem=off, the highest_gpa
> > field includes devices that are above the 4GiB limit.
> > Similarily, nothing seem to check that the memory is within
> > the limit set by the highmem=off option.
> >
> > This leads to failures in virt_kvm_type() on systems that have
> > a crippled IPA range, as the reported IPA space is larger than
> > what it should be.
> >
> > Instead, honor the user-specified limit to only use the devices
> > at the lowest end of the spectrum, and fail if we have memory
> > crossing the 4GiB limit.
> >
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Message-id: 20220114140741.1358263-4-maz@kernel.org
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/arm/virt.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 62bdce1eb4b..3b839ba78ba 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
> > static void virt_set_memmap(VirtMachineState *vms)
> > {
> > MachineState *ms = MACHINE(vms);
> > - hwaddr base, device_memory_base, device_memory_size;
> > + hwaddr base, device_memory_base, device_memory_size, memtop;
> > int i;
> > vms->memmap = extended_memmap;
> > @@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
> > device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
> > /* Base address of the high IO region */
> > - base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
> > + if (!vms->highmem && memtop > 4 * GiB) {
> > + error_report("highmem=off, but memory crosses the 4GiB limit\n");
> > + exit(EXIT_FAILURE);
> > + }
> > if (base < device_memory_base) {
> > error_report("maxmem/slots too huge");
> > exit(EXIT_FAILURE);
> > @@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
> > vms->memmap[i].size = size;
> > base += size;
> > }
> > - vms->highest_gpa = base - 1;
> > + vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
> > if (device_memory_size > 0) {
> > ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> > ms->device_memory->base = device_memory_base;
>
> Hi,
> This breaks in a case where highmem is disabled but can have more than
> 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> which is not enough for highmem MMIO but is enough to contain 32 GiB
> of RAM.
Funny. The whole point of this series is to make it all work correctly
on M1.
> Where the magic number of 4 GiB / 32-bit came from?
Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
<quote>
highmem
Set ``on``/``off`` to enable/disable placing devices and RAM in physical
address space above 32 bits. The default is ``on`` for machine types
later than ``virt-2.12``.
</quote>
TL;DR: Removing the bogus 'highmem=off' option from your command-line
should get you going with large memory spaces, up to the IPA limit.
The fact that you could run with 32GB of RAM while mandating that the
guest IPA space was limited to 32bit was nothing but a bug, further
"exploited" by HVF to allow disabling the highhmem devices which are
out of reach given the HW limitations (see [1] for details on the
discussion, specially around patch 3).
This is now fixed, and has been extended to work with any IPA size
(including 36bit machines such as M1).
> I also don't quite understand what failures virt_kvm_type() had.
QEMU works by first computing the memory map and passing the required
IPA limit to KVM as part of the VM type. By failing to take into
account the initial limit requirements to the IPA space (either via a
command-line option such as 'highmem', or by using the value provided
by KVM itself), QEMU would try to create a VM that cannot run on the
HW, and KVM would simply return an error.
All of this is documented as part of the KVM/arm64 API [2]. And with
this fixed, QEMU is able to correctly drive KVM on M1.
M.
[1] https://lore.kernel.org/all/20210822144441.1290891-1-maz@kernel.org
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/virt/kvm/api.rst#n138
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-02-13 10:22 ` Marc Zyngier
(?)
@ 2022-02-13 10:45 ` Peter Maydell
-1 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 10:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: Akihiko Odaki, qemu-devel, Andrew Jones, Eric Auger, kvmarm, kvm,
kernel-team, Alexander Graf
On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
>
> [+ Alex for HVF]
>
> On Sun, 13 Feb 2022 05:05:33 +0000,
> Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > Hi,
> > This breaks in a case where highmem is disabled but can have more than
> > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > of RAM.
>
> Funny. The whole point of this series is to make it all work correctly
> on M1.
>
> > Where the magic number of 4 GiB / 32-bit came from?
>
> Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
>
> <quote>
> highmem
> Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> address space above 32 bits. The default is ``on`` for machine types
> later than ``virt-2.12``.
> </quote>
>
> TL;DR: Removing the bogus 'highmem=off' option from your command-line
> should get you going with large memory spaces, up to the IPA limit.
Yep. I've tested this with hvf, and we now correctly:
* refuse to put RAM above 32-bits if you asked for a 32-bit
IPA space with highmem=off
* use the full 36-bit address space if you don't say highmem=off
on an M1
Note that there is a macos bug where if you don't say highmem=off
on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
thanks
-- PMM
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 10:45 ` Peter Maydell
0 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 10:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: Andrew Jones, kvm, qemu-devel, Eric Auger, Alexander Graf,
Akihiko Odaki, kernel-team, kvmarm
On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
>
> [+ Alex for HVF]
>
> On Sun, 13 Feb 2022 05:05:33 +0000,
> Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > Hi,
> > This breaks in a case where highmem is disabled but can have more than
> > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > of RAM.
>
> Funny. The whole point of this series is to make it all work correctly
> on M1.
>
> > Where the magic number of 4 GiB / 32-bit came from?
>
> Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
>
> <quote>
> highmem
> Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> address space above 32 bits. The default is ``on`` for machine types
> later than ``virt-2.12``.
> </quote>
>
> TL;DR: Removing the bogus 'highmem=off' option from your command-line
> should get you going with large memory spaces, up to the IPA limit.
Yep. I've tested this with hvf, and we now correctly:
* refuse to put RAM above 32-bits if you asked for a 32-bit
IPA space with highmem=off
* use the full 36-bit address space if you don't say highmem=off
on an M1
Note that there is a macos bug where if you don't say highmem=off
on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
thanks
-- PMM
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 10:45 ` Peter Maydell
0 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 10:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, qemu-devel, Alexander Graf, Akihiko Odaki, kernel-team, kvmarm
On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
>
> [+ Alex for HVF]
>
> On Sun, 13 Feb 2022 05:05:33 +0000,
> Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > Hi,
> > This breaks in a case where highmem is disabled but can have more than
> > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > of RAM.
>
> Funny. The whole point of this series is to make it all work correctly
> on M1.
>
> > Where the magic number of 4 GiB / 32-bit came from?
>
> Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
>
> <quote>
> highmem
> Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> address space above 32 bits. The default is ``on`` for machine types
> later than ``virt-2.12``.
> </quote>
>
> TL;DR: Removing the bogus 'highmem=off' option from your command-line
> should get you going with large memory spaces, up to the IPA limit.
Yep. I've tested this with hvf, and we now correctly:
* refuse to put RAM above 32-bits if you asked for a 32-bit
IPA space with highmem=off
* use the full 36-bit address space if you don't say highmem=off
on an M1
Note that there is a macos bug where if you don't say highmem=off
on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-02-13 10:45 ` Peter Maydell
(?)
@ 2022-02-13 11:38 ` Akihiko Odaki
-1 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 11:38 UTC (permalink / raw)
To: Peter Maydell
Cc: Marc Zyngier, qemu Developers, Andrew Jones, Eric Auger, kvmarm,
kvm, kernel-team, Alexander Graf
On Sun, Feb 13, 2022 at 7:46 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
> >
> > [+ Alex for HVF]
> >
> > On Sun, 13 Feb 2022 05:05:33 +0000,
> > Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > > Hi,
> > > This breaks in a case where highmem is disabled but can have more than
> > > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > > of RAM.
> >
> > Funny. The whole point of this series is to make it all work correctly
> > on M1.
> >
> > > Where the magic number of 4 GiB / 32-bit came from?
> >
> > Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
> >
> > <quote>
> > highmem
> > Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> > address space above 32 bits. The default is ``on`` for machine types
> > later than ``virt-2.12``.
> > </quote>
> >
> > TL;DR: Removing the bogus 'highmem=off' option from your command-line
> > should get you going with large memory spaces, up to the IPA limit.
>
> Yep. I've tested this with hvf, and we now correctly:
> * refuse to put RAM above 32-bits if you asked for a 32-bit
> IPA space with highmem=off
> * use the full 36-bit address space if you don't say highmem=off
> on an M1
>
> Note that there is a macos bug where if you don't say highmem=off
> on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
>
> thanks
> -- PMM
I found that it actually gets the available PA bit of the emulated CPU
when highmem=on. I used "cortex-a72", which can have more than 36
bits. I just simply switched to "host"; hvf didn't support "host" when
I set up my VM but now it does.
Thanks for your prompt replies.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 11:38 ` Akihiko Odaki
0 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 11:38 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm, Marc Zyngier, qemu Developers, Alexander Graf, kernel-team, kvmarm
On Sun, Feb 13, 2022 at 7:46 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
> >
> > [+ Alex for HVF]
> >
> > On Sun, 13 Feb 2022 05:05:33 +0000,
> > Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > > Hi,
> > > This breaks in a case where highmem is disabled but can have more than
> > > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > > of RAM.
> >
> > Funny. The whole point of this series is to make it all work correctly
> > on M1.
> >
> > > Where the magic number of 4 GiB / 32-bit came from?
> >
> > Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
> >
> > <quote>
> > highmem
> > Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> > address space above 32 bits. The default is ``on`` for machine types
> > later than ``virt-2.12``.
> > </quote>
> >
> > TL;DR: Removing the bogus 'highmem=off' option from your command-line
> > should get you going with large memory spaces, up to the IPA limit.
>
> Yep. I've tested this with hvf, and we now correctly:
> * refuse to put RAM above 32-bits if you asked for a 32-bit
> IPA space with highmem=off
> * use the full 36-bit address space if you don't say highmem=off
> on an M1
>
> Note that there is a macos bug where if you don't say highmem=off
> on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
>
> thanks
> -- PMM
I found that it actually gets the available PA bit of the emulated CPU
when highmem=on. I used "cortex-a72", which can have more than 36
bits. I just simply switched to "host"; hvf didn't support "host" when
I set up my VM but now it does.
Thanks for your prompt replies.
Regards,
Akihiko Odaki
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 11:38 ` Akihiko Odaki
0 siblings, 0 replies; 54+ messages in thread
From: Akihiko Odaki @ 2022-02-13 11:38 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jones, kvm, Marc Zyngier, qemu Developers, Eric Auger,
Alexander Graf, kernel-team, kvmarm
On Sun, Feb 13, 2022 at 7:46 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sun, 13 Feb 2022 at 10:22, Marc Zyngier <maz@kernel.org> wrote:
> >
> > [+ Alex for HVF]
> >
> > On Sun, 13 Feb 2022 05:05:33 +0000,
> > Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > > Hi,
> > > This breaks in a case where highmem is disabled but can have more than
> > > 4 GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
> > > which is not enough for highmem MMIO but is enough to contain 32 GiB
> > > of RAM.
> >
> > Funny. The whole point of this series is to make it all work correctly
> > on M1.
> >
> > > Where the magic number of 4 GiB / 32-bit came from?
> >
> > Not exactly a magic number. From QEMU's docs/system/arm/virt.rst:
> >
> > <quote>
> > highmem
> > Set ``on``/``off`` to enable/disable placing devices and RAM in physical
> > address space above 32 bits. The default is ``on`` for machine types
> > later than ``virt-2.12``.
> > </quote>
> >
> > TL;DR: Removing the bogus 'highmem=off' option from your command-line
> > should get you going with large memory spaces, up to the IPA limit.
>
> Yep. I've tested this with hvf, and we now correctly:
> * refuse to put RAM above 32-bits if you asked for a 32-bit
> IPA space with highmem=off
> * use the full 36-bit address space if you don't say highmem=off
> on an M1
>
> Note that there is a macos bug where if you don't say highmem=off
> on an M1 Pro then you'll get a macos kernel panic. M1 non-Pro is fine.
>
> thanks
> -- PMM
I found that it actually gets the available PA bit of the emulated CPU
when highmem=on. I used "cortex-a72", which can have more than 36
bits. I just simply switched to "host"; hvf didn't support "host" when
I set up my VM but now it does.
Thanks for your prompt replies.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
2022-02-13 11:38 ` Akihiko Odaki
(?)
@ 2022-02-13 12:57 ` Peter Maydell
-1 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 12:57 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Marc Zyngier, qemu Developers, Andrew Jones, Eric Auger, kvmarm,
kvm, kernel-team, Alexander Graf
On Sun, 13 Feb 2022 at 11:38, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> I found that it actually gets the available PA bit of the emulated CPU
> when highmem=on. I used "cortex-a72", which can have more than 36
> bits. I just simply switched to "host"; hvf didn't support "host" when
> I set up my VM but now it does.
It's a bug that we accept 'cortex-a72' there. What should happen
is something like:
* we want to use the ID register values of a cortex-a72
* QEMU's hvf layer should say "no, that doesn't match the actual
CPU we're running on", and give an error
This works correctly with KVM because there the kernel refuses
attempts to set ID registers to values that don't match the host;
for hvf the hvf APIs do permit lying to the guest about ID register
values so we need to do the check ourselves.
(The other approach would be to check the ID register values
and allow them to the extent that the host CPU actually has
the support for the features they imply, so you could "downgrade"
to a less capable CPU but not tell the guest it has feature X
if it isn't really there. But this is (a) a lot more complicated
and (b) gets into the swamp of trying to figure out how to tell
the guest about CPU errata -- the guest needs to apply errata
workarounds for the real host CPU, not for whatever the emulated
CPU is. So "just reject anything that's not an exact match" is
the easy approach.)
-- PMM
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 12:57 ` Peter Maydell
0 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 12:57 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Andrew Jones, kvm, Marc Zyngier, qemu Developers, Eric Auger,
Alexander Graf, kernel-team, kvmarm
On Sun, 13 Feb 2022 at 11:38, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> I found that it actually gets the available PA bit of the emulated CPU
> when highmem=on. I used "cortex-a72", which can have more than 36
> bits. I just simply switched to "host"; hvf didn't support "host" when
> I set up my VM but now it does.
It's a bug that we accept 'cortex-a72' there. What should happen
is something like:
* we want to use the ID register values of a cortex-a72
* QEMU's hvf layer should say "no, that doesn't match the actual
CPU we're running on", and give an error
This works correctly with KVM because there the kernel refuses
attempts to set ID registers to values that don't match the host;
for hvf the hvf APIs do permit lying to the guest about ID register
values so we need to do the check ourselves.
(The other approach would be to check the ID register values
and allow them to the extent that the host CPU actually has
the support for the features they imply, so you could "downgrade"
to a less capable CPU but not tell the guest it has feature X
if it isn't really there. But this is (a) a lot more complicated
and (b) gets into the swamp of trying to figure out how to tell
the guest about CPU errata -- the guest needs to apply errata
workarounds for the real host CPU, not for whatever the emulated
CPU is. So "just reject anything that's not an exact match" is
the easy approach.)
-- PMM
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
@ 2022-02-13 12:57 ` Peter Maydell
0 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-02-13 12:57 UTC (permalink / raw)
To: Akihiko Odaki
Cc: kvm, Marc Zyngier, qemu Developers, Alexander Graf, kernel-team, kvmarm
On Sun, 13 Feb 2022 at 11:38, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> I found that it actually gets the available PA bit of the emulated CPU
> when highmem=on. I used "cortex-a72", which can have more than 36
> bits. I just simply switched to "host"; hvf didn't support "host" when
> I set up my VM but now it does.
It's a bug that we accept 'cortex-a72' there. What should happen
is something like:
* we want to use the ID register values of a cortex-a72
* QEMU's hvf layer should say "no, that doesn't match the actual
CPU we're running on", and give an error
This works correctly with KVM because there the kernel refuses
attempts to set ID registers to values that don't match the host;
for hvf the hvf APIs do permit lying to the guest about ID register
values so we need to do the check ourselves.
(The other approach would be to check the ID register values
and allow them to the extent that the host CPU actually has
the support for the features they imply, so you could "downgrade"
to a less capable CPU but not tell the guest it has feature X
if it isn't really there. But this is (a) a lot more complicated
and (b) gets into the swamp of trying to figure out how to tell
the guest about CPU errata -- the guest needs to apply errata
workarounds for the real host CPU, not for whatever the emulated
CPU is. So "just reject anything that's not an exact match" is
the easy approach.)
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PULL 19/38] hw/arm/virt: Use the PA range to compute the memory map
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (17 preceding siblings ...)
2022-01-20 12:36 ` [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 20/38] hw/arm/virt: Disable highmem devices that don't fit in the PA range Peter Maydell
` (18 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
The highmem attribute is nothing but another way to express the
PA range of a VM. To support HW that has a smaller PA range then
what QEMU assumes, pass this PA range to the virt_set_memmap()
function, allowing it to correctly exclude highmem devices
if they are outside of the PA range.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20220114140741.1358263-5-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 64 +++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 12 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3b839ba78ba..8627f5ab953 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1667,7 +1667,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
return arm_cpu_mp_affinity(idx, clustersz);
}
-static void virt_set_memmap(VirtMachineState *vms)
+static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
{
MachineState *ms = MACHINE(vms);
hwaddr base, device_memory_base, device_memory_size, memtop;
@@ -1685,6 +1685,14 @@ static void virt_set_memmap(VirtMachineState *vms)
exit(EXIT_FAILURE);
}
+ /*
+ * !highmem is exactly the same as limiting the PA space to 32bit,
+ * irrespective of the underlying capabilities of the HW.
+ */
+ if (!vms->highmem) {
+ pa_bits = 32;
+ }
+
/*
* We compute the base of the high IO region depending on the
* amount of initial and device memory. The device memory start/size
@@ -1698,8 +1706,9 @@ static void virt_set_memmap(VirtMachineState *vms)
/* Base address of the high IO region */
memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
- if (!vms->highmem && memtop > 4 * GiB) {
- error_report("highmem=off, but memory crosses the 4GiB limit\n");
+ if (memtop > BIT_ULL(pa_bits)) {
+ error_report("Addressing limited to %d bits, but memory exceeds it by %llu bytes\n",
+ pa_bits, memtop - BIT_ULL(pa_bits));
exit(EXIT_FAILURE);
}
if (base < device_memory_base) {
@@ -1718,7 +1727,13 @@ static void virt_set_memmap(VirtMachineState *vms)
vms->memmap[i].size = size;
base += size;
}
- vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
+
+ /*
+ * If base fits within pa_bits, all good. If it doesn't, limit it
+ * to the end of RAM, which is guaranteed to fit within pa_bits.
+ */
+ vms->highest_gpa = (base <= BIT_ULL(pa_bits) ? base : memtop) - 1;
+
if (device_memory_size > 0) {
ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
ms->device_memory->base = device_memory_base;
@@ -1909,12 +1924,43 @@ static void machvirt_init(MachineState *machine)
unsigned int smp_cpus = machine->smp.cpus;
unsigned int max_cpus = machine->smp.max_cpus;
+ if (!cpu_type_valid(machine->cpu_type)) {
+ error_report("mach-virt: CPU type %s not supported", machine->cpu_type);
+ exit(1);
+ }
+
+ possible_cpus = mc->possible_cpu_arch_ids(machine);
+
/*
* In accelerated mode, the memory map is computed earlier in kvm_type()
* to create a VM with the right number of IPA bits.
*/
if (!vms->memmap) {
- virt_set_memmap(vms);
+ Object *cpuobj;
+ ARMCPU *armcpu;
+ int pa_bits;
+
+ /*
+ * Instanciate a temporary CPU object to find out about what
+ * we are about to deal with. Once this is done, get rid of
+ * the object.
+ */
+ cpuobj = object_new(possible_cpus->cpus[0].type);
+ armcpu = ARM_CPU(cpuobj);
+
+ if (object_property_get_bool(cpuobj, "aarch64", NULL)) {
+ pa_bits = arm_pamax(armcpu);
+ } else if (arm_feature(&armcpu->env, ARM_FEATURE_LPAE)) {
+ /* v7 with LPAE */
+ pa_bits = 40;
+ } else {
+ /* Anything else */
+ pa_bits = 32;
+ }
+
+ object_unref(cpuobj);
+
+ virt_set_memmap(vms, pa_bits);
}
/* We can probe only here because during property set
@@ -1922,11 +1968,6 @@ static void machvirt_init(MachineState *machine)
*/
finalize_gic_version(vms);
- if (!cpu_type_valid(machine->cpu_type)) {
- error_report("mach-virt: CPU type %s not supported", machine->cpu_type);
- exit(1);
- }
-
if (vms->secure) {
/*
* The Secure view of the world is the same as the NonSecure,
@@ -1996,7 +2037,6 @@ static void machvirt_init(MachineState *machine)
create_fdt(vms);
- possible_cpus = mc->possible_cpu_arch_ids(machine);
assert(possible_cpus->len == max_cpus);
for (n = 0; n < possible_cpus->len; n++) {
Object *cpuobj;
@@ -2735,7 +2775,7 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);
/* we freeze the memory map to compute the highest gpa */
- virt_set_memmap(vms);
+ virt_set_memmap(vms, max_vm_pa_size);
requested_pa_size = 64 - clz64(vms->highest_gpa);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 20/38] hw/arm/virt: Disable highmem devices that don't fit in the PA range
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (18 preceding siblings ...)
2022-01-20 12:36 ` [PULL 19/38] hw/arm/virt: Use the PA range to compute " Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 21/38] hw/arm/virt: Drop superfluous checks against highmem Peter Maydell
` (17 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
In order to only keep the highmem devices that actually fit in
the PA range, check their location against the range and update
highest_gpa if they fit. If they don't, mark them as disabled.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20220114140741.1358263-6-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8627f5ab953..8d02c2267d0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1719,21 +1719,43 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
base = vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES;
}
+ /* We know for sure that at least the memory fits in the PA space */
+ vms->highest_gpa = memtop - 1;
+
for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
hwaddr size = extended_memmap[i].size;
+ bool fits;
base = ROUND_UP(base, size);
vms->memmap[i].base = base;
vms->memmap[i].size = size;
+
+ /*
+ * Check each device to see if they fit in the PA space,
+ * moving highest_gpa as we go.
+ *
+ * For each device that doesn't fit, disable it.
+ */
+ fits = (base + size) <= BIT_ULL(pa_bits);
+ if (fits) {
+ vms->highest_gpa = base + size - 1;
+ }
+
+ switch (i) {
+ case VIRT_HIGH_GIC_REDIST2:
+ vms->highmem_redists &= fits;
+ break;
+ case VIRT_HIGH_PCIE_ECAM:
+ vms->highmem_ecam &= fits;
+ break;
+ case VIRT_HIGH_PCIE_MMIO:
+ vms->highmem_mmio &= fits;
+ break;
+ }
+
base += size;
}
- /*
- * If base fits within pa_bits, all good. If it doesn't, limit it
- * to the end of RAM, which is guaranteed to fit within pa_bits.
- */
- vms->highest_gpa = (base <= BIT_ULL(pa_bits) ? base : memtop) - 1;
-
if (device_memory_size > 0) {
ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
ms->device_memory->base = device_memory_base;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 21/38] hw/arm/virt: Drop superfluous checks against highmem
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (19 preceding siblings ...)
2022-01-20 12:36 ` [PULL 20/38] hw/arm/virt: Disable highmem devices that don't fit in the PA range Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 22/38] hw/arm: kudo add lm75s behind bus 1 switch at 75 Peter Maydell
` (16 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Marc Zyngier <maz@kernel.org>
Now that the devices present in the extended memory map are checked
against the available PA space and disabled when they don't fit,
there is no need to keep the same checks against highmem, as
highmem really is a shortcut for the PA space being 32bit.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Message-id: 20220114140741.1358263-7-maz@kernel.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt-acpi-build.c | 2 --
hw/arm/virt.c | 5 +----
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0757c28f69c..449fab00805 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -947,8 +947,6 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
- vms->highmem_redists &= vms->highmem;
-
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8d02c2267d0..141350bf215 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2178,9 +2178,6 @@ static void machvirt_init(MachineState *machine)
virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
- vms->highmem_mmio &= vms->highmem;
- vms->highmem_redists &= vms->highmem;
-
create_gic(vms, sysmem);
virt_cpu_post_init(vms, sysmem);
@@ -2199,7 +2196,7 @@ static void machvirt_init(MachineState *machine)
machine->ram_size, "mach-virt.tag");
}
- vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
+ vms->highmem_ecam &= (!firmware_loaded || aarch64);
create_rtc(vms);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 22/38] hw/arm: kudo add lm75s behind bus 1 switch at 75
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (20 preceding siblings ...)
2022-01-20 12:36 ` [PULL 21/38] hw/arm/virt: Drop superfluous checks against highmem Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 23/38] hw/misc/aspeed_i3c.c: Introduce a dummy AST2600 I3C model Peter Maydell
` (15 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
Message-id: 20220111172338.1525587-1-venture@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/npcm7xx_boards.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 7d0f3148be0..d701e5cc55a 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -332,7 +332,15 @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
{
I2CSlave *i2c_mux;
- i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x75);
+ i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1),
+ TYPE_PCA9548, 0x75);
+
+ /* tmp105 is compatible with the lm75 */
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 4), "tmp105", 0x5c);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 5), "tmp105", 0x5c);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 6), "tmp105", 0x5c);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 7), "tmp105", 0x5c);
+
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 1), TYPE_PCA9548, 0x77);
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), TYPE_PCA9548, 0x77);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 23/38] hw/misc/aspeed_i3c.c: Introduce a dummy AST2600 I3C model.
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (21 preceding siblings ...)
2022-01-20 12:36 ` [PULL 22/38] hw/arm: kudo add lm75s behind bus 1 switch at 75 Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 24/38] hw/arm/aspeed: Add the i3c device to the AST2600 SoC Peter Maydell
` (14 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Troy Lee <troy_lee@aspeedtech.com>
Aspeed 2600 SDK enables I3C support by default. The I3C driver will try
to reset the device controller and set it up through device address table
register. This dummy model responds to these registers with default values
as listed in the ast2600v10 datasheet chapter 54.2.
This avoids a guest machine kernel panic due to referencing an
invalid kernel address if the device address table register isn't
set correctly.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Reviewed-by: Graeme Gregory <quic_ggregory@quicinc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Graeme Gregory <quic_ggregory@quicinc.com>
Message-id: 20220111084546.4145785-2-troy_lee@aspeedtech.com
[PMM: tidied commit message]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/misc/aspeed_i3c.h | 48 +++++
hw/misc/aspeed_i3c.c | 381 +++++++++++++++++++++++++++++++++++
hw/misc/meson.build | 1 +
hw/misc/trace-events | 6 +
4 files changed, 436 insertions(+)
create mode 100644 include/hw/misc/aspeed_i3c.h
create mode 100644 hw/misc/aspeed_i3c.c
diff --git a/include/hw/misc/aspeed_i3c.h b/include/hw/misc/aspeed_i3c.h
new file mode 100644
index 00000000000..39679dfa1ae
--- /dev/null
+++ b/include/hw/misc/aspeed_i3c.h
@@ -0,0 +1,48 @@
+/*
+ * ASPEED I3C Controller
+ *
+ * Copyright (C) 2021 ASPEED Technology Inc.
+ *
+ * This code is licensed under the GPL version 2 or later. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef ASPEED_I3C_H
+#define ASPEED_I3C_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_I3C "aspeed.i3c"
+#define TYPE_ASPEED_I3C_DEVICE "aspeed.i3c.device"
+OBJECT_DECLARE_TYPE(AspeedI3CState, AspeedI3CClass, ASPEED_I3C)
+
+#define ASPEED_I3C_NR_REGS (0x70 >> 2)
+#define ASPEED_I3C_DEVICE_NR_REGS (0x300 >> 2)
+#define ASPEED_I3C_NR_DEVICES 6
+
+OBJECT_DECLARE_SIMPLE_TYPE(AspeedI3CDevice, ASPEED_I3C_DEVICE)
+typedef struct AspeedI3CDevice {
+ /* <private> */
+ SysBusDevice parent;
+
+ /* <public> */
+ MemoryRegion mr;
+ qemu_irq irq;
+
+ uint8_t id;
+ uint32_t regs[ASPEED_I3C_DEVICE_NR_REGS];
+} AspeedI3CDevice;
+
+typedef struct AspeedI3CState {
+ /* <private> */
+ SysBusDevice parent;
+
+ /* <public> */
+ MemoryRegion iomem;
+ MemoryRegion iomem_container;
+ qemu_irq irq;
+
+ uint32_t regs[ASPEED_I3C_NR_REGS];
+ AspeedI3CDevice devices[ASPEED_I3C_NR_DEVICES];
+} AspeedI3CState;
+#endif /* ASPEED_I3C_H */
diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
new file mode 100644
index 00000000000..43771d768ad
--- /dev/null
+++ b/hw/misc/aspeed_i3c.c
@@ -0,0 +1,381 @@
+/*
+ * ASPEED I3C Controller
+ *
+ * Copyright (C) 2021 ASPEED Technology Inc.
+ *
+ * This code is licensed under the GPL version 2 or later. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_i3c.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+/* I3C Controller Registers */
+REG32(I3C1_REG0, 0x10)
+REG32(I3C1_REG1, 0x14)
+ FIELD(I3C1_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C1_REG1, SA_EN, 15, 1)
+REG32(I3C2_REG0, 0x20)
+REG32(I3C2_REG1, 0x24)
+ FIELD(I3C2_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C2_REG1, SA_EN, 15, 1)
+REG32(I3C3_REG0, 0x30)
+REG32(I3C3_REG1, 0x34)
+ FIELD(I3C3_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C3_REG1, SA_EN, 15, 1)
+REG32(I3C4_REG0, 0x40)
+REG32(I3C4_REG1, 0x44)
+ FIELD(I3C4_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C4_REG1, SA_EN, 15, 1)
+REG32(I3C5_REG0, 0x50)
+REG32(I3C5_REG1, 0x54)
+ FIELD(I3C5_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C5_REG1, SA_EN, 15, 1)
+REG32(I3C6_REG0, 0x60)
+REG32(I3C6_REG1, 0x64)
+ FIELD(I3C6_REG1, I2C_MODE, 0, 1)
+ FIELD(I3C6_REG1, SA_EN, 15, 1)
+
+/* I3C Device Registers */
+REG32(DEVICE_CTRL, 0x00)
+REG32(DEVICE_ADDR, 0x04)
+REG32(HW_CAPABILITY, 0x08)
+REG32(COMMAND_QUEUE_PORT, 0x0c)
+REG32(RESPONSE_QUEUE_PORT, 0x10)
+REG32(RX_TX_DATA_PORT, 0x14)
+REG32(IBI_QUEUE_STATUS, 0x18)
+REG32(IBI_QUEUE_DATA, 0x18)
+REG32(QUEUE_THLD_CTRL, 0x1c)
+REG32(DATA_BUFFER_THLD_CTRL, 0x20)
+REG32(IBI_QUEUE_CTRL, 0x24)
+REG32(IBI_MR_REQ_REJECT, 0x2c)
+REG32(IBI_SIR_REQ_REJECT, 0x30)
+REG32(RESET_CTRL, 0x34)
+REG32(SLV_EVENT_CTRL, 0x38)
+REG32(INTR_STATUS, 0x3c)
+REG32(INTR_STATUS_EN, 0x40)
+REG32(INTR_SIGNAL_EN, 0x44)
+REG32(INTR_FORCE, 0x48)
+REG32(QUEUE_STATUS_LEVEL, 0x4c)
+REG32(DATA_BUFFER_STATUS_LEVEL, 0x50)
+REG32(PRESENT_STATE, 0x54)
+REG32(CCC_DEVICE_STATUS, 0x58)
+REG32(DEVICE_ADDR_TABLE_POINTER, 0x5c)
+ FIELD(DEVICE_ADDR_TABLE_POINTER, DEPTH, 16, 16)
+ FIELD(DEVICE_ADDR_TABLE_POINTER, ADDR, 0, 16)
+REG32(DEV_CHAR_TABLE_POINTER, 0x60)
+REG32(VENDOR_SPECIFIC_REG_POINTER, 0x6c)
+REG32(SLV_MIPI_PID_VALUE, 0x70)
+REG32(SLV_PID_VALUE, 0x74)
+REG32(SLV_CHAR_CTRL, 0x78)
+REG32(SLV_MAX_LEN, 0x7c)
+REG32(MAX_READ_TURNAROUND, 0x80)
+REG32(MAX_DATA_SPEED, 0x84)
+REG32(SLV_DEBUG_STATUS, 0x88)
+REG32(SLV_INTR_REQ, 0x8c)
+REG32(DEVICE_CTRL_EXTENDED, 0xb0)
+REG32(SCL_I3C_OD_TIMING, 0xb4)
+REG32(SCL_I3C_PP_TIMING, 0xb8)
+REG32(SCL_I2C_FM_TIMING, 0xbc)
+REG32(SCL_I2C_FMP_TIMING, 0xc0)
+REG32(SCL_EXT_LCNT_TIMING, 0xc8)
+REG32(SCL_EXT_TERMN_LCNT_TIMING, 0xcc)
+REG32(BUS_FREE_TIMING, 0xd4)
+REG32(BUS_IDLE_TIMING, 0xd8)
+REG32(I3C_VER_ID, 0xe0)
+REG32(I3C_VER_TYPE, 0xe4)
+REG32(EXTENDED_CAPABILITY, 0xe8)
+REG32(SLAVE_CONFIG, 0xec)
+
+static const uint32_t ast2600_i3c_device_resets[ASPEED_I3C_DEVICE_NR_REGS] = {
+ [R_HW_CAPABILITY] = 0x000e00bf,
+ [R_QUEUE_THLD_CTRL] = 0x01000101,
+ [R_I3C_VER_ID] = 0x3130302a,
+ [R_I3C_VER_TYPE] = 0x6c633033,
+ [R_DEVICE_ADDR_TABLE_POINTER] = 0x00080280,
+ [R_DEV_CHAR_TABLE_POINTER] = 0x00020200,
+ [A_VENDOR_SPECIFIC_REG_POINTER] = 0x000000b0,
+ [R_SLV_MAX_LEN] = 0x00ff00ff,
+};
+
+static uint64_t aspeed_i3c_device_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+ AspeedI3CDevice *s = ASPEED_I3C_DEVICE(opaque);
+ uint32_t addr = offset >> 2;
+ uint64_t value;
+
+ switch (addr) {
+ case R_COMMAND_QUEUE_PORT:
+ value = 0;
+ break;
+ default:
+ value = s->regs[addr];
+ break;
+ }
+
+ trace_aspeed_i3c_device_read(s->id, offset, value);
+
+ return value;
+}
+
+static void aspeed_i3c_device_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+ AspeedI3CDevice *s = ASPEED_I3C_DEVICE(opaque);
+ uint32_t addr = offset >> 2;
+
+ trace_aspeed_i3c_device_write(s->id, offset, value);
+
+ switch (addr) {
+ case R_HW_CAPABILITY:
+ case R_RESPONSE_QUEUE_PORT:
+ case R_IBI_QUEUE_DATA:
+ case R_QUEUE_STATUS_LEVEL:
+ case R_PRESENT_STATE:
+ case R_CCC_DEVICE_STATUS:
+ case R_DEVICE_ADDR_TABLE_POINTER:
+ case R_VENDOR_SPECIFIC_REG_POINTER:
+ case R_SLV_CHAR_CTRL:
+ case R_SLV_MAX_LEN:
+ case R_MAX_READ_TURNAROUND:
+ case R_I3C_VER_ID:
+ case R_I3C_VER_TYPE:
+ case R_EXTENDED_CAPABILITY:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: write to readonly register[%02lx] = %08lx\n",
+ __func__, offset, value);
+ break;
+ case R_RX_TX_DATA_PORT:
+ break;
+ case R_RESET_CTRL:
+ break;
+ default:
+ s->regs[addr] = value;
+ break;
+ }
+}
+
+static const VMStateDescription aspeed_i3c_device_vmstate = {
+ .name = TYPE_ASPEED_I3C,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]){
+ VMSTATE_UINT32_ARRAY(regs, AspeedI3CDevice, ASPEED_I3C_DEVICE_NR_REGS),
+ VMSTATE_END_OF_LIST(),
+ }
+};
+
+static const MemoryRegionOps aspeed_i3c_device_ops = {
+ .read = aspeed_i3c_device_read,
+ .write = aspeed_i3c_device_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void aspeed_i3c_device_reset(DeviceState *dev)
+{
+ AspeedI3CDevice *s = ASPEED_I3C_DEVICE(dev);
+
+ memcpy(s->regs, ast2600_i3c_device_resets, sizeof(s->regs));
+}
+
+static void aspeed_i3c_device_realize(DeviceState *dev, Error **errp)
+{
+ AspeedI3CDevice *s = ASPEED_I3C_DEVICE(dev);
+ g_autofree char *name = g_strdup_printf(TYPE_ASPEED_I3C_DEVICE ".%d",
+ s->id);
+
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
+
+ memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i3c_device_ops,
+ s, name, ASPEED_I3C_DEVICE_NR_REGS << 2);
+}
+
+static uint64_t aspeed_i3c_read(void *opaque, hwaddr addr, unsigned int size)
+{
+ AspeedI3CState *s = ASPEED_I3C(opaque);
+ uint64_t val = 0;
+
+ val = s->regs[addr >> 2];
+
+ trace_aspeed_i3c_read(addr, val);
+
+ return val;
+}
+
+static void aspeed_i3c_write(void *opaque,
+ hwaddr addr,
+ uint64_t data,
+ unsigned int size)
+{
+ AspeedI3CState *s = ASPEED_I3C(opaque);
+
+ trace_aspeed_i3c_write(addr, data);
+
+ addr >>= 2;
+
+ /* I3C controller register */
+ switch (addr) {
+ case R_I3C1_REG1:
+ case R_I3C2_REG1:
+ case R_I3C3_REG1:
+ case R_I3C4_REG1:
+ case R_I3C5_REG1:
+ case R_I3C6_REG1:
+ if (data & R_I3C1_REG1_I2C_MODE_MASK) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: Not support I2C mode [%08lx]=%08lx",
+ __func__, addr << 2, data);
+ break;
+ }
+ if (data & R_I3C1_REG1_SA_EN_MASK) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: Not support slave mode [%08lx]=%08lx",
+ __func__, addr << 2, data);
+ break;
+ }
+ s->regs[addr] = data;
+ break;
+ default:
+ s->regs[addr] = data;
+ break;
+ }
+}
+
+static const MemoryRegionOps aspeed_i3c_ops = {
+ .read = aspeed_i3c_read,
+ .write = aspeed_i3c_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ }
+};
+
+static void aspeed_i3c_reset(DeviceState *dev)
+{
+ AspeedI3CState *s = ASPEED_I3C(dev);
+ memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void aspeed_i3c_instance_init(Object *obj)
+{
+ AspeedI3CState *s = ASPEED_I3C(obj);
+ int i;
+
+ for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {
+ object_initialize_child(obj, "device[*]", &s->devices[i],
+ TYPE_ASPEED_I3C_DEVICE);
+ }
+}
+
+static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
+{
+ int i;
+ AspeedI3CState *s = ASPEED_I3C(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+ memory_region_init(&s->iomem_container, OBJECT(s),
+ TYPE_ASPEED_I3C ".container", 0x8000);
+
+ sysbus_init_mmio(sbd, &s->iomem_container);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i3c_ops, s,
+ TYPE_ASPEED_I3C ".regs", ASPEED_I3C_NR_REGS << 2);
+
+ memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
+
+ for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {
+ Object *dev = OBJECT(&s->devices[i]);
+
+ if (!object_property_set_uint(dev, "device-id", i, errp)) {
+ return;
+ }
+
+ if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) {
+ return;
+ }
+
+ /*
+ * Register Address of I3CX Device =
+ * (Base Address of Global Register) + (Offset of I3CX) + Offset
+ * X = 0, 1, 2, 3, 4, 5
+ * Offset of I3C0 = 0x2000
+ * Offset of I3C1 = 0x3000
+ * Offset of I3C2 = 0x4000
+ * Offset of I3C3 = 0x5000
+ * Offset of I3C4 = 0x6000
+ * Offset of I3C5 = 0x7000
+ */
+ memory_region_add_subregion(&s->iomem_container,
+ 0x2000 + i * 0x1000, &s->devices[i].mr);
+ }
+
+}
+
+static Property aspeed_i3c_device_properties[] = {
+ DEFINE_PROP_UINT8("device-id", AspeedI3CDevice, id, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void aspeed_i3c_device_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->desc = "Aspeed I3C Device";
+ dc->realize = aspeed_i3c_device_realize;
+ dc->reset = aspeed_i3c_device_reset;
+ device_class_set_props(dc, aspeed_i3c_device_properties);
+}
+
+static const TypeInfo aspeed_i3c_device_info = {
+ .name = TYPE_ASPEED_I3C_DEVICE,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedI3CDevice),
+ .class_init = aspeed_i3c_device_class_init,
+};
+
+static const VMStateDescription vmstate_aspeed_i3c = {
+ .name = TYPE_ASPEED_I3C,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(regs, AspeedI3CState, ASPEED_I3C_NR_REGS),
+ VMSTATE_STRUCT_ARRAY(devices, AspeedI3CState, ASPEED_I3C_NR_DEVICES, 1,
+ aspeed_i3c_device_vmstate, AspeedI3CDevice),
+ VMSTATE_END_OF_LIST(),
+ }
+};
+
+static void aspeed_i3c_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = aspeed_i3c_realize;
+ dc->reset = aspeed_i3c_reset;
+ dc->desc = "Aspeed I3C Controller";
+ dc->vmsd = &vmstate_aspeed_i3c;
+}
+
+static const TypeInfo aspeed_i3c_info = {
+ .name = TYPE_ASPEED_I3C,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_init = aspeed_i3c_instance_init,
+ .instance_size = sizeof(AspeedI3CState),
+ .class_init = aspeed_i3c_class_init,
+};
+
+static void aspeed_i3c_register_types(void)
+{
+ type_register_static(&aspeed_i3c_device_info);
+ type_register_static(&aspeed_i3c_info);
+}
+
+type_init(aspeed_i3c_register_types);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 3f41a3a5b27..d1a11691087 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -105,6 +105,7 @@ softmmu_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c'))
softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c'))
softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
'aspeed_hace.c',
+ 'aspeed_i3c.c',
'aspeed_lpc.c',
'aspeed_scu.c',
'aspeed_sdmc.c',
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 2da96d167a7..1c373dd0a4c 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -199,6 +199,12 @@ armsse_mhu_write(uint64_t offset, uint64_t data, unsigned size) "SSE-200 MHU wri
# aspeed_xdma.c
aspeed_xdma_write(uint64_t offset, uint64_t data) "XDMA write: offset 0x%" PRIx64 " data 0x%" PRIx64
+# aspeed_i3c.c
+aspeed_i3c_read(uint64_t offset, uint64_t data) "I3C read: offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_i3c_write(uint64_t offset, uint64_t data) "I3C write: offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_i3c_device_read(uint32_t deviceid, uint64_t offset, uint64_t data) "I3C Dev[%u] read: offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_i3c_device_write(uint32_t deviceid, uint64_t offset, uint64_t data) "I3C Dev[%u] write: offset 0x%" PRIx64 " data 0x%" PRIx64
+
# bcm2835_property.c
bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 24/38] hw/arm/aspeed: Add the i3c device to the AST2600 SoC
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (22 preceding siblings ...)
2022-01-20 12:36 ` [PULL 23/38] hw/misc/aspeed_i3c.c: Introduce a dummy AST2600 I3C model Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 25/38] hw/intc/arm_gicv3_its: Fix event ID bounds checks Peter Maydell
` (13 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Troy Lee <troy_lee@aspeedtech.com>
Add the new i3c device to the AST2600 SoC.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Reviewed-by: Graeme Gregory <quic_ggregory@quicinc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Graeme Gregory <quic_ggregory@quicinc.com>
Message-id: 20220111084546.4145785-3-troy_lee@aspeedtech.com
[PMM: tidied commit message]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/aspeed_soc.h | 3 +++
hw/arm/aspeed_ast2600.c | 16 ++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 18fb7eed461..cae9906684c 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -21,6 +21,7 @@
#include "hw/timer/aspeed_timer.h"
#include "hw/rtc/aspeed_rtc.h"
#include "hw/i2c/aspeed_i2c.h"
+#include "hw/misc/aspeed_i3c.h"
#include "hw/ssi/aspeed_smc.h"
#include "hw/misc/aspeed_hace.h"
#include "hw/watchdog/wdt_aspeed.h"
@@ -51,6 +52,7 @@ struct AspeedSoCState {
AspeedRtcState rtc;
AspeedTimerCtrlState timerctrl;
AspeedI2CState i2c;
+ AspeedI3CState i3c;
AspeedSCUState scu;
AspeedHACEState hace;
AspeedXDMAState xdma;
@@ -141,6 +143,7 @@ enum {
ASPEED_DEV_HACE,
ASPEED_DEV_DPMCU,
ASPEED_DEV_DP,
+ ASPEED_DEV_I3C,
};
#endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index e33483fb5dd..8f37bdb1d87 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -61,6 +61,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
[ASPEED_DEV_UART1] = 0x1E783000,
[ASPEED_DEV_UART5] = 0x1E784000,
[ASPEED_DEV_VUART] = 0x1E787000,
+ [ASPEED_DEV_I3C] = 0x1E7A0000,
[ASPEED_DEV_SDRAM] = 0x80000000,
};
@@ -108,6 +109,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
[ASPEED_DEV_ETH4] = 33,
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
[ASPEED_DEV_DP] = 62,
+ [ASPEED_DEV_I3C] = 102, /* 102 -> 107 */
};
static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -223,6 +225,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
object_initialize_child(obj, "hace", &s->hace, typename);
+
+ object_initialize_child(obj, "i3c", &s->i3c, TYPE_ASPEED_I3C);
}
/*
@@ -523,6 +527,18 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
+ /* I3C */
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp)) {
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+ for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
+ qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+ sc->irqmap[ASPEED_DEV_I3C] + i);
+ /* The AST2600 I3C controller has one IRQ per bus. */
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq);
+ }
}
static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 25/38] hw/intc/arm_gicv3_its: Fix event ID bounds checks
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (23 preceding siblings ...)
2022-01-20 12:36 ` [PULL 24/38] hw/arm/aspeed: Add the i3c device to the AST2600 SoC Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 26/38] hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention Peter Maydell
` (12 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
In process_its_cmd() and process_mapti() we must check the
event ID against a limit defined by the size field in the DTE,
which specifies the number of ID bits minus one. Convert
this code to our num_foo convention:
* change the variable names
* use uint64_t and 1ULL when calculating the number
of valid event IDs, because DTE.SIZE is 5 bits and
so num_eventids may be up to 2^32
* fix the off-by-one error in the comparison
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-2-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index fa3cdb57554..6d11fa02040 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -225,7 +225,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
MemTxResult res = MEMTX_OK;
bool dte_valid;
uint64_t dte = 0;
- uint32_t max_eventid;
+ uint64_t num_eventids;
uint16_t icid = 0;
uint32_t pIntid = 0;
bool ite_valid = false;
@@ -258,7 +258,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
dte_valid = FIELD_EX64(dte, DTE, VALID);
if (dte_valid) {
- max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
+ num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
@@ -299,10 +299,11 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
dte_valid ? "valid" : "invalid",
ite_valid ? "valid" : "invalid",
cte_valid ? "valid" : "invalid");
- } else if (eventid > max_eventid) {
+ } else if (eventid >= num_eventids) {
qemu_log_mask(LOG_GUEST_ERROR,
- "%s: invalid command attributes: eventid %d > %d\n",
- __func__, eventid, max_eventid);
+ "%s: invalid command attributes: eventid %d >= %"
+ PRId64 "\n",
+ __func__, eventid, num_eventids);
} else {
/*
* Current implementation only supports rdbase == procnum
@@ -336,7 +337,8 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid, eventid;
uint32_t pIntid = 0;
- uint32_t max_eventid, max_Intid;
+ uint64_t num_eventids;
+ uint32_t max_Intid;
bool dte_valid;
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
@@ -376,11 +378,11 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
return result;
}
dte_valid = FIELD_EX64(dte, DTE, VALID);
- max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1);
+ num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
- || !dte_valid || (eventid > max_eventid) ||
+ || !dte_valid || (eventid >= num_eventids) ||
(((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
(pIntid != INTID_SPURIOUS))) {
qemu_log_mask(LOG_GUEST_ERROR,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 26/38] hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (24 preceding siblings ...)
2022-01-20 12:36 ` [PULL 25/38] hw/intc/arm_gicv3_its: Fix event ID bounds checks Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 27/38] hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value Peter Maydell
` (11 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
The bounds check on the number of interrupt IDs is correct, but
doesn't match our convention; change the variable name, initialize it
to the 2^n value rather than (2^n)-1, and use >= instead of > in the
comparison.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-3-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 6d11fa02040..5919b1a3b7f 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -338,7 +338,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
uint32_t devid, eventid;
uint32_t pIntid = 0;
uint64_t num_eventids;
- uint32_t max_Intid;
+ uint32_t num_intids;
bool dte_valid;
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
@@ -379,11 +379,11 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
}
dte_valid = FIELD_EX64(dte, DTE, VALID);
num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
- max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
+ num_intids = 1ULL << (GICD_TYPER_IDBITS + 1);
if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
|| !dte_valid || (eventid >= num_eventids) ||
- (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
+ (((pIntid < GICV3_LPI_INTID_START) || (pIntid >= num_intids)) &&
(pIntid != INTID_SPURIOUS))) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes "
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 27/38] hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (25 preceding siblings ...)
2022-01-20 12:36 ` [PULL 26/38] hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 28/38] hw/intc/arm_gicv3_its: Don't use data if reading command failed Peter Maydell
` (10 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
process_its_cmd() returns a bool, like all the other process_ functions.
However we were putting its return value into 'res', not 'result',
which meant we would ignore it when deciding whether to continue
or stall the command queue. Fix the typo.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220111171048.3545974-4-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 5919b1a3b7f..a6c2299a091 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -678,10 +678,10 @@ static void process_cmdq(GICv3ITSState *s)
switch (cmd) {
case GITS_CMD_INT:
- res = process_its_cmd(s, data, cq_offset, INTERRUPT);
+ result = process_its_cmd(s, data, cq_offset, INTERRUPT);
break;
case GITS_CMD_CLEAR:
- res = process_its_cmd(s, data, cq_offset, CLEAR);
+ result = process_its_cmd(s, data, cq_offset, CLEAR);
break;
case GITS_CMD_SYNC:
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 28/38] hw/intc/arm_gicv3_its: Don't use data if reading command failed
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (26 preceding siblings ...)
2022-01-20 12:36 ` [PULL 27/38] hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions Peter Maydell
` (9 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
In process_cmdq(), we read 64 bits of the command packet, which
contain the command identifier, which we then switch() on to dispatch
to an appropriate sub-function. However, if address_space_ldq_le()
reports a memory transaction failure, we still read the command
identifier out of the data and switch() on it. Restructure the code
so that we stop immediately (stalling the command queue) in this
case.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-5-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index a6c2299a091..c1f76682d04 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -672,8 +672,13 @@ static void process_cmdq(GICv3ITSState *s)
data = address_space_ldq_le(as, s->cq.base_addr + cq_offset,
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- result = false;
+ s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: could not read command at 0x%" PRIx64 "\n",
+ __func__, s->cq.base_addr + cq_offset);
+ break;
}
+
cmd = (data & CMD_MASK);
switch (cmd) {
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (27 preceding siblings ...)
2022-01-20 12:36 ` [PULL 28/38] hw/intc/arm_gicv3_its: Don't use data if reading command failed Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 30/38] hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd() Peter Maydell
` (8 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
When an ITS detects an error in a command, it has an
implementation-defined (CONSTRAINED UNPREDICTABLE) choice of whether
to ignore the command, proceeding to the next one in the queue, or to
stall the ITS command queue, processing nothing further. The
behaviour required when the read of the command packet from memory
fails is less clearly documented, but the same set of choices as for
command errors seem reasonable.
The intention of the QEMU implementation, as documented in the
comments, is that if we encounter a memory error reading the command
packet or one of the various data tables then we should stall, but
for command parameter errors we should ignore the queue and continue.
However, we don't actually do this. To get the desired behaviour,
the various process_* functions need to return true to cause
process_cmdq() to advance to the next command and keep processing,
and false to stall command processing. What they mostly do is return
false for any kind of error.
To make the code clearer, replace the 'bool' return from the process_
functions with an enum which may be either CMD_STALL or CMD_CONTINUE.
In this commit no behaviour changes; in subsequent commits we will
adjust the error-return paths for the process_ functions one by one.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220111171048.3545974-6-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 59 ++++++++++++++++++++++++++---------------
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index c1f76682d04..10901a5e709 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -45,6 +45,23 @@ typedef struct {
uint64_t itel;
} IteEntry;
+/*
+ * The ITS spec permits a range of CONSTRAINED UNPREDICTABLE options
+ * if a command parameter is not correct. These include both "stall
+ * processing of the command queue" and "ignore this command, and
+ * keep processing the queue". In our implementation we choose that
+ * memory transaction errors reading the command packet provoke a
+ * stall, but errors in parameters cause us to ignore the command
+ * and continue processing.
+ * The process_* functions which handle individual ITS commands all
+ * return an ItsCmdResult which tells process_cmdq() whether it should
+ * stall or keep going.
+ */
+typedef enum ItsCmdResult {
+ CMD_STALL = 0,
+ CMD_CONTINUE = 1,
+} ItsCmdResult;
+
static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
{
uint64_t result = 0;
@@ -217,8 +234,8 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
* 3. handling of ITS CLEAR command
* 4. handling of ITS DISCARD command
*/
-static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
- ItsCmdType cmd)
+static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
+ uint32_t offset, ItsCmdType cmd)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid, eventid;
@@ -231,7 +248,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
bool ite_valid = false;
uint64_t cte = 0;
bool cte_valid = false;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
uint64_t rdbase;
if (cmd == NONE) {
@@ -324,15 +341,15 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
if (cmd == DISCARD) {
IteEntry ite = {};
/* remove mapping from interrupt translation table */
- result = update_ite(s, eventid, dte, ite);
+ result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
}
return result;
}
-static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
- bool ignore_pInt)
+static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
+ uint32_t offset, bool ignore_pInt)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid, eventid;
@@ -343,7 +360,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
uint64_t dte = 0;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
offset += NUM_BYTES_IN_DW;
@@ -404,7 +421,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
- result = update_ite(s, eventid, dte, ite);
+ result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
return result;
@@ -472,14 +489,14 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
}
}
-static bool process_mapc(GICv3ITSState *s, uint32_t offset)
+static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
{
AddressSpace *as = &s->gicv3->dma_as;
uint16_t icid;
uint64_t rdbase;
bool valid;
MemTxResult res = MEMTX_OK;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
uint64_t value;
offset += NUM_BYTES_IN_DW;
@@ -509,7 +526,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
* command in the queue
*/
} else {
- result = update_cte(s, icid, valid, rdbase);
+ result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL;
}
return result;
@@ -578,7 +595,8 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
}
}
-static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
+static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
+ uint32_t offset)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid;
@@ -586,7 +604,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
uint64_t itt_addr;
bool valid;
MemTxResult res = MEMTX_OK;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
@@ -623,7 +641,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
* command in the queue
*/
} else {
- result = update_dte(s, devid, valid, size, itt_addr);
+ result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL;
}
return result;
@@ -641,7 +659,6 @@ static void process_cmdq(GICv3ITSState *s)
uint64_t data;
AddressSpace *as = &s->gicv3->dma_as;
MemTxResult res = MEMTX_OK;
- bool result = true;
uint8_t cmd;
int i;
@@ -668,6 +685,8 @@ static void process_cmdq(GICv3ITSState *s)
}
while (wr_offset != rd_offset) {
+ ItsCmdResult result = CMD_CONTINUE;
+
cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE);
data = address_space_ldq_le(as, s->cq.base_addr + cq_offset,
MEMTXATTRS_UNSPECIFIED, &res);
@@ -726,18 +745,16 @@ static void process_cmdq(GICv3ITSState *s)
default:
break;
}
- if (result) {
+ if (result == CMD_CONTINUE) {
rd_offset++;
rd_offset %= s->cq.num_entries;
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset);
} else {
- /*
- * in this implementation, in case of dma read/write error
- * we stall the command processing
- */
+ /* CMD_STALL */
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1);
qemu_log_mask(LOG_GUEST_ERROR,
- "%s: %x cmd processing failed\n", __func__, cmd);
+ "%s: 0x%x cmd processing failed, stalling\n",
+ __func__, cmd);
break;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 30/38] hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd()
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (28 preceding siblings ...)
2022-01-20 12:36 ` [PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 31/38] hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting Peter Maydell
` (7 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
Fix process_its_cmd() to consistently return CMD_STALL for
memory errors and CMD_CONTINUE for parameter errors, as
we claim in the comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-7-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 10901a5e709..0929116c0fe 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -248,7 +248,6 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
bool ite_valid = false;
uint64_t cte = 0;
bool cte_valid = false;
- ItsCmdResult result = CMD_STALL;
uint64_t rdbase;
if (cmd == NONE) {
@@ -262,7 +261,7 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
}
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
eventid = (value & EVENTID_MASK);
@@ -270,7 +269,7 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
dte = get_dte(s, devid, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
dte_valid = FIELD_EX64(dte, DTE, VALID);
@@ -280,7 +279,7 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
if (ite_valid) {
@@ -288,14 +287,14 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
}
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
} 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;
+ return CMD_CONTINUE;
}
@@ -307,7 +306,7 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: devid %d>=%d",
__func__, devid, s->dt.num_ids);
-
+ return CMD_CONTINUE;
} else if (!dte_valid || !ite_valid || !cte_valid) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: "
@@ -316,11 +315,13 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
dte_valid ? "valid" : "invalid",
ite_valid ? "valid" : "invalid",
cte_valid ? "valid" : "invalid");
+ return CMD_CONTINUE;
} else if (eventid >= num_eventids) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: eventid %d >= %"
PRId64 "\n",
__func__, eventid, num_eventids);
+ return CMD_CONTINUE;
} else {
/*
* Current implementation only supports rdbase == procnum
@@ -329,7 +330,7 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
rdbase = FIELD_EX64(cte, CTE, RDBASE);
if (rdbase >= s->gicv3->num_cpu) {
- return result;
+ return CMD_CONTINUE;
}
if ((cmd == CLEAR) || (cmd == DISCARD)) {
@@ -341,11 +342,10 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
if (cmd == DISCARD) {
IteEntry ite = {};
/* remove mapping from interrupt translation table */
- result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
+ return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
+ return CMD_CONTINUE;
}
-
- return result;
}
static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 31/38] hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (29 preceding siblings ...)
2022-01-20 12:36 ` [PULL 30/38] hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd() Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 32/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapti() Peter Maydell
` (6 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
Refactor process_its_cmd() so that it consistently uses
the structure
do thing;
if (error condition) {
return early;
}
do next thing;
rather than doing some of the work nested inside if (not error)
code blocks.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-8-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 103 +++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 53 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 0929116c0fe..5dc6846fe3f 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -273,79 +273,76 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
}
dte_valid = FIELD_EX64(dte, DTE, VALID);
- if (dte_valid) {
- num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
-
- ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
-
- if (res != MEMTX_OK) {
- return CMD_STALL;
- }
-
- if (ite_valid) {
- cte_valid = get_cte(s, icid, &cte, &res);
- }
-
- if (res != MEMTX_OK) {
- return CMD_STALL;
- }
- } else {
+ if (!dte_valid) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: "
- "invalid dte: %"PRIx64" for %d (MEM_TX: %d)\n",
- __func__, dte, devid, res);
+ "invalid dte: %"PRIx64" for %d\n",
+ __func__, dte, devid);
return CMD_CONTINUE;
}
+ num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
+
+ ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
+ if (res != MEMTX_OK) {
+ return CMD_STALL;
+ }
+
+ if (!ite_valid) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid command attributes: invalid ITE\n",
+ __func__);
+ return CMD_CONTINUE;
+ }
+
+ cte_valid = get_cte(s, icid, &cte, &res);
+ if (res != MEMTX_OK) {
+ return CMD_STALL;
+ }
+ if (!cte_valid) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid command attributes: "
+ "invalid cte: %"PRIx64"\n",
+ __func__, cte);
+ return CMD_CONTINUE;
+ }
- /*
- * 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.num_ids) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: devid %d>=%d",
__func__, devid, s->dt.num_ids);
return CMD_CONTINUE;
- } 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");
- return CMD_CONTINUE;
- } else if (eventid >= num_eventids) {
+ }
+ if (eventid >= num_eventids) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes: eventid %d >= %"
PRId64 "\n",
__func__, eventid, num_eventids);
return CMD_CONTINUE;
- } else {
- /*
- * Current implementation only supports rdbase == procnum
- * Hence rdbase physical address is ignored
- */
- rdbase = FIELD_EX64(cte, CTE, RDBASE);
+ }
- if (rdbase >= s->gicv3->num_cpu) {
- return CMD_CONTINUE;
- }
+ /*
+ * Current implementation only supports rdbase == procnum
+ * Hence rdbase physical address is ignored
+ */
+ rdbase = FIELD_EX64(cte, CTE, RDBASE);
- if ((cmd == CLEAR) || (cmd == DISCARD)) {
- gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 0);
- } else {
- gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 1);
- }
-
- if (cmd == DISCARD) {
- IteEntry ite = {};
- /* remove mapping from interrupt translation table */
- return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
- }
+ if (rdbase >= s->gicv3->num_cpu) {
return CMD_CONTINUE;
}
+
+ if ((cmd == CLEAR) || (cmd == DISCARD)) {
+ gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 0);
+ } else {
+ gicv3_redist_process_lpi(&s->gicv3->cpu[rdbase], pIntid, 1);
+ }
+
+ if (cmd == DISCARD) {
+ IteEntry ite = {};
+ /* remove mapping from interrupt translation table */
+ return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
+ }
+ return CMD_CONTINUE;
}
static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 32/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapti()
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (30 preceding siblings ...)
2022-01-20 12:36 ` [PULL 31/38] hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 33/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapc() Peter Maydell
` (5 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
Fix process_mapti() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-9-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 5dc6846fe3f..010779a9fdc 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -357,7 +357,7 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
uint64_t dte = 0;
- ItsCmdResult result = CMD_STALL;
+ IteEntry ite = {};
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
offset += NUM_BYTES_IN_DW;
@@ -365,7 +365,7 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
eventid = (value & EVENTID_MASK);
@@ -381,7 +381,7 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
icid = value & ICID_MASK;
@@ -389,7 +389,7 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
dte = get_dte(s, devid, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
dte_valid = FIELD_EX64(dte, DTE, VALID);
num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
@@ -409,19 +409,17 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
* we ignore this command and move onto the next
* command in the queue
*/
- } else {
- /* add ite entry to interrupt translation table */
- IteEntry ite = {};
- ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
- ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
- ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
- ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
-
- result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
+ return CMD_CONTINUE;
}
- return result;
+ /* add ite entry to interrupt translation table */
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, dte_valid);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
+ ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
+
+ return update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 33/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapc()
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (31 preceding siblings ...)
2022-01-20 12:36 ` [PULL 32/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapti() Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 34/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapd() Peter Maydell
` (4 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
Fix process_mapc() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-10-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 010779a9fdc..80ef4dbcadf 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -491,7 +491,6 @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
uint64_t rdbase;
bool valid;
MemTxResult res = MEMTX_OK;
- ItsCmdResult result = CMD_STALL;
uint64_t value;
offset += NUM_BYTES_IN_DW;
@@ -501,7 +500,7 @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
icid = value & ICID_MASK;
@@ -520,11 +519,10 @@ static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
* we ignore this command and move onto the next
* command in the queue
*/
- } else {
- result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL;
+ return CMD_CONTINUE;
}
- return result;
+ return update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL;
}
static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 34/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapd()
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (32 preceding siblings ...)
2022-01-20 12:36 ` [PULL 33/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapc() Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 35/38] hw/intc/arm_gicv3_its: Factor out "find address of table entry" code Peter Maydell
` (3 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
Fix process_mapd() to consistently return CMD_STALL for memory
errors and CMD_CONTINUE for parameter errors, as we claim in the
comments that we do.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220111171048.3545974-11-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 80ef4dbcadf..917201c148f 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -597,7 +597,6 @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
uint64_t itt_addr;
bool valid;
MemTxResult res = MEMTX_OK;
- ItsCmdResult result = CMD_STALL;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
@@ -606,7 +605,7 @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
size = (value & SIZE_MASK);
@@ -616,7 +615,7 @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
MEMTXATTRS_UNSPECIFIED, &res);
if (res != MEMTX_OK) {
- return result;
+ return CMD_STALL;
}
itt_addr = (value & ITTADDR_MASK) >> ITTADDR_SHIFT;
@@ -633,11 +632,10 @@ static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
* we ignore this command and move onto the next
* command in the queue
*/
- } else {
- result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL;
+ return CMD_CONTINUE;
}
- return result;
+ return update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL;
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 35/38] hw/intc/arm_gicv3_its: Factor out "find address of table entry" code
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (33 preceding siblings ...)
2022-01-20 12:36 ` [PULL 34/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapd() Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 36/38] hw/intc/arm_gicv3_its: Check indexes before use, not after Peter Maydell
` (2 subsequent siblings)
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
The ITS has several tables which all share a similar format,
described by the TableDesc struct: the guest may configure them
to be a single-level table or a two-level table. Currently we
open-code the process of finding the table entry in all the
functions which read or write the device table or the collection
table. Factor out the "get the address of the table entry"
logic into a new function, so that the code which needs to
read or write a table entry only needs to call table_entry_addr()
and then perform a suitable load or store to that address.
Note that the error handling is slightly complicated because
we want to handle two cases differently:
* failure to read the L1 table entry should end up causing
a command stall, like other kinds of DMA error
* an L1 table entry that says there is no L2 table for this
index (ie whose valid bit is 0) must result in us treating
the table entry as not-valid on read, and discarding
writes (this is mandated by the spec)
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>
Message-id: 20220111171048.3545974-12-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 212 +++++++++++++---------------------------
1 file changed, 70 insertions(+), 142 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 917201c148f..985e316eda9 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -83,44 +83,62 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
return result;
}
+static uint64_t table_entry_addr(GICv3ITSState *s, TableDesc *td,
+ uint32_t idx, MemTxResult *res)
+{
+ /*
+ * Given a TableDesc describing one of the ITS in-guest-memory
+ * tables and an index into it, return the guest address
+ * corresponding to that table entry.
+ * If there was a memory error reading the L1 table of an
+ * indirect table, *res is set accordingly, and we return -1.
+ * If the L1 table entry is marked not valid, we return -1 with
+ * *res set to MEMTX_OK.
+ *
+ * The specification defines the format of level 1 entries of a
+ * 2-level table, but the format of level 2 entries and the format
+ * of flat-mapped tables is IMPDEF.
+ */
+ AddressSpace *as = &s->gicv3->dma_as;
+ uint32_t l2idx;
+ uint64_t l2;
+ uint32_t num_l2_entries;
+
+ *res = MEMTX_OK;
+
+ if (!td->indirect) {
+ /* Single level table */
+ return td->base_addr + idx * td->entry_sz;
+ }
+
+ /* Two level table */
+ l2idx = idx / (td->page_sz / L1TABLE_ENTRY_SIZE);
+
+ l2 = address_space_ldq_le(as,
+ td->base_addr + (l2idx * L1TABLE_ENTRY_SIZE),
+ MEMTXATTRS_UNSPECIFIED, res);
+ if (*res != MEMTX_OK) {
+ return -1;
+ }
+ if (!(l2 & L2_TABLE_VALID_MASK)) {
+ return -1;
+ }
+
+ num_l2_entries = td->page_sz / td->entry_sz;
+ return (l2 & ((1ULL << 51) - 1)) + (idx % num_l2_entries) * td->entry_sz;
+}
+
static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
MemTxResult *res)
{
AddressSpace *as = &s->gicv3->dma_as;
- uint64_t l2t_addr;
- uint64_t value;
- bool valid_l2t;
- uint32_t l2t_id;
- uint32_t num_l2_entries;
+ uint64_t entry_addr = table_entry_addr(s, &s->ct, icid, res);
- if (s->ct.indirect) {
- l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
-
- value = address_space_ldq_le(as,
- s->ct.base_addr +
- (l2t_id * L1TABLE_ENTRY_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
-
- if (*res == MEMTX_OK) {
- valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
-
- if (valid_l2t) {
- num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
-
- l2t_addr = value & ((1ULL << 51) - 1);
-
- *cte = address_space_ldq_le(as, l2t_addr +
- ((icid % num_l2_entries) * GITS_CTE_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
- }
- }
- } else {
- /* Flat level table */
- *cte = address_space_ldq_le(as, s->ct.base_addr +
- (icid * GITS_CTE_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
+ if (entry_addr == -1) {
+ return false; /* not valid */
}
+ *cte = address_space_ldq_le(as, entry_addr, MEMTXATTRS_UNSPECIFIED, res);
return FIELD_EX64(*cte, CTE, VALID);
}
@@ -189,41 +207,12 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res)
{
AddressSpace *as = &s->gicv3->dma_as;
- uint64_t l2t_addr;
- uint64_t value;
- bool valid_l2t;
- uint32_t l2t_id;
- uint32_t num_l2_entries;
+ uint64_t entry_addr = table_entry_addr(s, &s->dt, devid, res);
- if (s->dt.indirect) {
- l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE);
-
- value = address_space_ldq_le(as,
- s->dt.base_addr +
- (l2t_id * L1TABLE_ENTRY_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
-
- if (*res == MEMTX_OK) {
- valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
-
- if (valid_l2t) {
- num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
-
- l2t_addr = value & ((1ULL << 51) - 1);
-
- value = address_space_ldq_le(as, l2t_addr +
- ((devid % num_l2_entries) * GITS_DTE_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
- }
- }
- } else {
- /* Flat level table */
- value = address_space_ldq_le(as, s->dt.base_addr +
- (devid * GITS_DTE_SIZE),
- MEMTXATTRS_UNSPECIFIED, res);
+ if (entry_addr == -1) {
+ return 0; /* a DTE entry with the Valid bit clear */
}
-
- return value;
+ return address_space_ldq_le(as, entry_addr, MEMTXATTRS_UNSPECIFIED, res);
}
/*
@@ -426,11 +415,7 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
uint64_t rdbase)
{
AddressSpace *as = &s->gicv3->dma_as;
- uint64_t value;
- uint64_t l2t_addr;
- bool valid_l2t;
- uint32_t l2t_id;
- uint32_t num_l2_entries;
+ uint64_t entry_addr;
uint64_t cte = 0;
MemTxResult res = MEMTX_OK;
@@ -444,44 +429,18 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
cte = FIELD_DP64(cte, CTE, RDBASE, rdbase);
}
- /*
- * The specification defines the format of level 1 entries of a
- * 2-level table, but the format of level 2 entries and the format
- * of flat-mapped tables is IMPDEF.
- */
- if (s->ct.indirect) {
- l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
-
- value = address_space_ldq_le(as,
- s->ct.base_addr +
- (l2t_id * L1TABLE_ENTRY_SIZE),
- MEMTXATTRS_UNSPECIFIED, &res);
-
- if (res != MEMTX_OK) {
- return false;
- }
-
- valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
-
- if (valid_l2t) {
- num_l2_entries = s->ct.page_sz / s->ct.entry_sz;
-
- l2t_addr = value & ((1ULL << 51) - 1);
-
- address_space_stq_le(as, l2t_addr +
- ((icid % num_l2_entries) * GITS_CTE_SIZE),
- cte, MEMTXATTRS_UNSPECIFIED, &res);
- }
- } else {
- /* Flat level table */
- address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE),
- cte, MEMTXATTRS_UNSPECIFIED, &res);
- }
+ entry_addr = table_entry_addr(s, &s->ct, icid, &res);
if (res != MEMTX_OK) {
+ /* memory access error: stall */
return false;
- } else {
+ }
+ if (entry_addr == -1) {
+ /* No L2 table for this index: discard write and continue */
return true;
}
+
+ address_space_stq_le(as, entry_addr, cte, MEMTXATTRS_UNSPECIFIED, &res);
+ return res == MEMTX_OK;
}
static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
@@ -529,11 +488,7 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
uint8_t size, uint64_t itt_addr)
{
AddressSpace *as = &s->gicv3->dma_as;
- uint64_t value;
- uint64_t l2t_addr;
- bool valid_l2t;
- uint32_t l2t_id;
- uint32_t num_l2_entries;
+ uint64_t entry_addr;
uint64_t dte = 0;
MemTxResult res = MEMTX_OK;
@@ -548,44 +503,17 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
return true;
}
- /*
- * The specification defines the format of level 1 entries of a
- * 2-level table, but the format of level 2 entries and the format
- * of flat-mapped tables is IMPDEF.
- */
- if (s->dt.indirect) {
- l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE);
-
- value = address_space_ldq_le(as,
- s->dt.base_addr +
- (l2t_id * L1TABLE_ENTRY_SIZE),
- MEMTXATTRS_UNSPECIFIED, &res);
-
- if (res != MEMTX_OK) {
- return false;
- }
-
- valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
-
- if (valid_l2t) {
- num_l2_entries = s->dt.page_sz / s->dt.entry_sz;
-
- l2t_addr = value & ((1ULL << 51) - 1);
-
- address_space_stq_le(as, l2t_addr +
- ((devid % num_l2_entries) * GITS_DTE_SIZE),
- dte, MEMTXATTRS_UNSPECIFIED, &res);
- }
- } else {
- /* Flat level table */
- address_space_stq_le(as, s->dt.base_addr + (devid * GITS_DTE_SIZE),
- dte, MEMTXATTRS_UNSPECIFIED, &res);
- }
+ entry_addr = table_entry_addr(s, &s->dt, devid, &res);
if (res != MEMTX_OK) {
+ /* memory access error: stall */
return false;
- } else {
+ }
+ if (entry_addr == -1) {
+ /* No L2 table for this index: discard write and continue */
return true;
}
+ address_space_stq_le(as, entry_addr, dte, MEMTXATTRS_UNSPECIFIED, &res);
+ return res == MEMTX_OK;
}
static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 36/38] hw/intc/arm_gicv3_its: Check indexes before use, not after
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (34 preceding siblings ...)
2022-01-20 12:36 ` [PULL 35/38] hw/intc/arm_gicv3_its: Factor out "find address of table entry" code Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 37/38] hw/intc/arm_gicv3_its: Range-check ICID before indexing into collection table Peter Maydell
2022-01-20 12:36 ` [PULL 38/38] hw/intc/arm_gicv3: Check for !MEMTX_OK instead of MEMTX_ERROR Peter Maydell
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
In a few places in the ITS command handling functions, we were
doing the range-check of an event ID or device ID only after using
it as a table index; move the checks to before the uses.
This misordering wouldn't have very bad effects because the
tables are in guest memory anyway.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-13-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 42 ++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 985e316eda9..ef6c0f55ff9 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -255,6 +255,13 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
eventid = (value & EVENTID_MASK);
+ if (devid >= s->dt.num_ids) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid command attributes: devid %d>=%d",
+ __func__, devid, s->dt.num_ids);
+ return CMD_CONTINUE;
+ }
+
dte = get_dte(s, devid, &res);
if (res != MEMTX_OK) {
@@ -272,6 +279,14 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
+ if (eventid >= num_eventids) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid command attributes: eventid %d >= %"
+ PRId64 "\n",
+ __func__, eventid, num_eventids);
+ return CMD_CONTINUE;
+ }
+
ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res);
if (res != MEMTX_OK) {
return CMD_STALL;
@@ -296,20 +311,6 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
return CMD_CONTINUE;
}
- if (devid >= s->dt.num_ids) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "%s: invalid command attributes: devid %d>=%d",
- __func__, devid, s->dt.num_ids);
- return CMD_CONTINUE;
- }
- if (eventid >= num_eventids) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "%s: invalid command attributes: eventid %d >= %"
- PRId64 "\n",
- __func__, eventid, num_eventids);
- return CMD_CONTINUE;
- }
-
/*
* Current implementation only supports rdbase == procnum
* Hence rdbase physical address is ignored
@@ -375,6 +376,13 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
icid = value & ICID_MASK;
+ if (devid >= s->dt.num_ids) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid command attributes: devid %d>=%d",
+ __func__, devid, s->dt.num_ids);
+ return CMD_CONTINUE;
+ }
+
dte = get_dte(s, devid, &res);
if (res != MEMTX_OK) {
@@ -384,14 +392,14 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
num_intids = 1ULL << (GICD_TYPER_IDBITS + 1);
- if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
+ if ((icid >= s->ct.num_ids)
|| !dte_valid || (eventid >= num_eventids) ||
(((pIntid < GICV3_LPI_INTID_START) || (pIntid >= num_intids)) &&
(pIntid != INTID_SPURIOUS))) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid command attributes "
- "devid %d or icid %d or eventid %d or pIntid %d or"
- "unmapped dte %d\n", __func__, devid, icid, eventid,
+ "icid %d or eventid %d or pIntid %d or"
+ "unmapped dte %d\n", __func__, icid, eventid,
pIntid, dte_valid);
/*
* in this implementation, in case of error
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 37/38] hw/intc/arm_gicv3_its: Range-check ICID before indexing into collection table
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (35 preceding siblings ...)
2022-01-20 12:36 ` [PULL 36/38] hw/intc/arm_gicv3_its: Check indexes before use, not after Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
2022-01-20 12:36 ` [PULL 38/38] hw/intc/arm_gicv3: Check for !MEMTX_OK instead of MEMTX_ERROR Peter Maydell
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
In process_its_cmd(), we read an ICID out of the interrupt table
entry, and then use it as an index into the collection table. Add a
check that it is within range for the collection table first.
This check is not strictly necessary, because:
* we range check the ICID from the guest before writing it into
the interrupt table entry, so the the only way to get an
out of range ICID in process_its_cmd() is if a badly-behaved
guest is writing directly to the interrupt table memory
* the collection table is in guest memory, so QEMU won't fall
over if we read off the end of it
However, it seems clearer to include the check.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20220111171048.3545974-14-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index ef6c0f55ff9..b2f6a8c7f00 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -299,6 +299,13 @@ static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
return CMD_CONTINUE;
}
+ if (icid >= s->ct.num_ids) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid ICID 0x%x in ITE (table corrupted?)\n",
+ __func__, icid);
+ return CMD_CONTINUE;
+ }
+
cte_valid = get_cte(s, icid, &cte, &res);
if (res != MEMTX_OK) {
return CMD_STALL;
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PULL 38/38] hw/intc/arm_gicv3: Check for !MEMTX_OK instead of MEMTX_ERROR
2022-01-20 12:35 [PULL 00/38] target-arm queue Peter Maydell
` (36 preceding siblings ...)
2022-01-20 12:36 ` [PULL 37/38] hw/intc/arm_gicv3_its: Range-check ICID before indexing into collection table Peter Maydell
@ 2022-01-20 12:36 ` Peter Maydell
37 siblings, 0 replies; 54+ messages in thread
From: Peter Maydell @ 2022-01-20 12:36 UTC (permalink / raw)
To: qemu-devel
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Quoting Peter Maydell:
"These MEMTX_* aren't from the memory transaction
API functions; they're just being used by gicd_readl() and
friends as a way to indicate a success/failure so that the
actual MemoryRegionOps read/write fns like gicv3_dist_read()
can log a guest error."
We are going to introduce more MemTxResult bits, so it is
safer to check for !MEMTX_OK rather than MEMTX_ERROR.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/arm_gicv3_redist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index c8ff3eca085..99b11ca5eee 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -462,7 +462,7 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data,
break;
}
- if (r == MEMTX_ERROR) {
+ if (r != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid guest read at offset " TARGET_FMT_plx
" size %u\n", __func__, offset, size);
@@ -521,7 +521,7 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
break;
}
- if (r == MEMTX_ERROR) {
+ if (r != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid guest write at offset " TARGET_FMT_plx
" size %u\n", __func__, offset, size);
--
2.25.1
^ permalink raw reply related [flat|nested] 54+ messages in thread