Re: PCIe with Designware RC.
On 24/12/2023 09:51, Shlomo Pongratz wrote: Hi, I'm working on a AARCH64 project that uses the designeware (hw/pci-host/designware.c). I've copied the designware initialization from hw/arm/fsl-imx7.c and I hope I've updated the dtsi correctly. After fixing an issue with the iATU windows (see patch https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg02643.html) Hmm, thought I had fixed this a while ago when doing some work with another systems and 64bit PCIe support as well. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
[PATCH v2] hw/intc/arm_gicv3: ICC_PMR_EL1 high bits should be RAZ
The ICC_PMR_ELx and ICV_PMR_ELx bit masks returned from
ic{c,v}_fullprio_mask should technically also remove any
bit above 7 as these are marked reserved (read 0) and should
therefore should not be written as anything other than 0.
This was noted during a run of a proprietary test system and
discused on the mailing list [1] and initially thought not to
be an issue due to RES0 being technically allowed to be
written to and read back as long as the implementation does
not use the RES0 bits. It is very possible that the values
are used in comparison without masking, as pointed out by
Peter in [2], if (cs->hppi.prio >= cs->icc_pmr_el1) may well
do the wrong thing.
Masking these values in ic{c,v}_fullprio_mask() should fix
this and prevent any future problems with playing with the
values.
[1]: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00607.html
[2]: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00737.html
Signed-off-by: Ben Dooks
Suggested-by: Peter Maydell
---
v2:
- fixes as suggested by Peter Maydell to include icv_fullprio_mask()
---
hw/intc/arm_gicv3_cpuif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index d07b13eb27..ab1a00508e 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -146,7 +146,7 @@ static uint32_t icv_fullprio_mask(GICv3CPUState *cs)
* with the group priority, whose mask depends on the value of VBPR
* for the interrupt group.)
*/
-return ~0U << (8 - cs->vpribits);
+return (~0U << (8 - cs->vpribits)) & 0xff;
}
static int ich_highest_active_virt_prio(GICv3CPUState *cs)
@@ -803,7 +803,7 @@ static uint32_t icc_fullprio_mask(GICv3CPUState *cs)
* with the group priority, whose mask depends on the value of BPR
* for the interrupt group.)
*/
-return ~0U << (8 - cs->pribits);
+return (~0U << (8 - cs->pribits)) & 0xff;
}
static inline int icc_min_bpr(GICv3CPUState *cs)
--
2.37.2.352.g3c44437643
Re: [PATCH] hw/intc/arm_gicv3: ICC_PMR_EL1 high bits should be RAZ
On 14/11/2023 17:14, Peter Maydell wrote: On Tue, 14 Nov 2023 at 16:54, Ben Dooks wrote: The ICC_PMR_ELx bit msak returned from icc_fullprio_mask should technically also remove any bit above 7 as these are marked reserved (read 0) and should therefore should not be written as anything other than 0. Signed-off-by: Ben Dooks --- hw/intc/arm_gicv3_cpuif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index d07b13eb27..986044df79 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -803,7 +803,7 @@ static uint32_t icc_fullprio_mask(GICv3CPUState *cs) * with the group priority, whose mask depends on the value of BPR * for the interrupt group.) */ -return ~0U << (8 - cs->pribits); +return (~0U << (8 - cs->pribits)) & 0xff; } The upper bits of ICC_PMR_ELx are defined as RES0, which has a complicated technical definition which you can find in the GIC architecture specification glossary. It's valid for RES0 bits to be implemented as reads-as-written, which is the way our current implementation works. Valid guest code should never be writing any non-zero value into those bits. Yeah, got some proprietary test code that is trying write-1 and then assuming read-0. What problem are you running into that you're trying to fix with this patch? If our implementation misbehaves as a result of letting these high bits through into cs->icc_pmr_el1 that would be a good reason for making the change. See above, local test code issue. > If we do want to change this, for consistency we'd want to change icv_fullprio_mask() too. If this isn't useful then I'll keep it as a local patch for now. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
[PATCH] hw/intc/arm_gicv3: ICC_PMR_EL1 high bits should be RAZ
The ICC_PMR_ELx bit msak returned from icc_fullprio_mask should technically also remove any bit above 7 as these are marked reserved (read 0) and should therefore should not be written as anything other than 0. Signed-off-by: Ben Dooks --- hw/intc/arm_gicv3_cpuif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index d07b13eb27..986044df79 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -803,7 +803,7 @@ static uint32_t icc_fullprio_mask(GICv3CPUState *cs) * with the group priority, whose mask depends on the value of BPR * for the interrupt group.) */ -return ~0U << (8 - cs->pribits); +return (~0U << (8 - cs->pribits)) & 0xff; } static inline int icc_min_bpr(GICv3CPUState *cs) -- 2.37.2.352.g3c44437643
[PATCH 00/44] Raspberry Pi 4B machine
Hi, is there an git tree with this series or a newer one available please? -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
[PATCH] riscv: add config for asid size
Add a config to the cpu state to control the size of the ASID area
in the SATP CSR to enable testing with smaller than the default (which
is currently maximum for both rv32 and rv64). It also adds the ability
to stop the ASID feature by using 0 to disable it.
For example, an rv64 with only 8 asid bits:
-cpu rv64,asid-bits=8
or no asids:
-cpu rv64,asid-bits=0
Signed-off-by: Ben Dooks
---
target/riscv/cpu.c | 42 +
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 2 ++
target/riscv/cpu_cfg.h | 1 +
target/riscv/csr.c | 1 +
5 files changed, 47 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4035fe0e62..c703005aba 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1281,6 +1281,38 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu,
Error **errp)
}
}
}
+
+static void riscv_cpu_asid_finalized_features(RISCVCPU *cpu, Error **errp)
+{
+bool rv32 = riscv_cpu_mxl(>env) == MXL_RV32;
+target_ulong asid_mask, asid_shift;
+target_ulong calc_mask;
+
+if (rv32) {
+asid_mask = SATP32_ASID;
+asid_shift = SATP32_ASID_SHIFT;
+} else {
+asid_mask = SATP64_ASID;
+asid_shift = SATP64_ASID_SHIFT;
+}
+
+if (cpu->cfg.asid_bits < 0) {
+cpu->env.asid_clear = 0;
+return;
+}
+
+calc_mask = ((target_ulong)1 << cpu->cfg.asid_bits) - 1;
+calc_mask <<= asid_shift;
+
+if (calc_mask > asid_mask) {
+error_setg(errp, "invalid ASID bits [0 %d]",
+ __builtin_clz(asid_mask >> asid_shift));
+return;
+}
+
+cpu->env.asid_clear = calc_mask ^ asid_mask;
+}
+
#endif
static void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
@@ -1293,6 +1325,12 @@ static void riscv_cpu_finalize_features(RISCVCPU *cpu,
Error **errp)
error_propagate(errp, local_err);
return;
}
+
+riscv_cpu_asid_finalized_features(cpu, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
#endif
}
@@ -1648,6 +1686,10 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("zicboz", RISCVCPU, cfg.ext_icboz, true),
DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
+#ifndef CONFIG_USER_ONLY
+DEFINE_PROP_INT32("asid-bits", RISCVCPU, cfg.asid_bits, -1),
+#endif
+
DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
DEFINE_PROP_BOOL("zca", RISCVCPU, cfg.ext_zca, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7adb8706ac..5b35770795 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -194,6 +194,7 @@ struct CPUArchState {
uint64_t mideleg;
target_ulong satp; /* since: priv-1.10.0 */
+target_ulong asid_clear;/* always clear these bits in satp */
target_ulong stval;
target_ulong medeleg;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 59f0ffd9e1..fd753ce3f4 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -617,11 +617,13 @@ typedef enum {
/* RV32 satp CSR field masks */
#define SATP32_MODE 0x8000
#define SATP32_ASID 0x7fc0
+#define SATP32_ASID_SHIFT 22
#define SATP32_PPN 0x003f
/* RV64 satp CSR field masks */
#define SATP64_MODE 0xF000ULL
#define SATP64_ASID 0x0000ULL
+#define SATP64_ASID_SHIFT 44
#define SATP64_PPN 0x0FFFULL
/* VM modes (satp.mode) privileged ISA 1.10 */
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index c4a627d335..4d578797cc 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -128,6 +128,7 @@ struct RISCVCPUConfig {
bool short_isa_string;
#ifndef CONFIG_USER_ONLY
+int32_t asid_bits;
RISCVSATPMap satp_mode;
#endif
};
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58499b5afc..215b71bd31 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2731,6 +2731,7 @@ static RISCVException write_satp(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
+val &= ~env->asid_clear;
if (riscv_cpu_mxl(env) == MXL_RV32) {
vm = validate_vm(env, get_field(val, SATP32_MODE));
mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
--
2.40.1
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
On Fri, Mar 31, 2023 at 01:01:15AM +, Joe Komlodi wrote: > Hi all, > > This series adds I3C bus support to QEMU and adds more functionality to the > Aspeed I3C controller. > > This implementation is a basic implementation that introduces IBIs > (including hot-join), CCCs, and SDR data transfer. As-is, it doesnt support > multi-controller buses or HDR transfers. > > First we add the I3C bus and controller model. With that added we > gradually extend the functionality of the Aspeed I3C controller so it > can do transfers. > > With that added, we add 2 targets. The first target is a mock I3C > target. It's intended to be a very simple target just to verify that I3C > is working on the guest. Internally, we've used it on Linux to verify > that i3C devices can be probed and can send/receive data and send IBIs. > > The second target is a remote target. The intention of this is to be > able to communicate to a target that exists outside of QEMU. > > Lastly we add hotplugging support. The hotplugging doesn't do anything too > complicated, it just adds the device attempting to hotplug to the bus. It is > the device's responsibility to hot-join and go through the DAA process to > participate on the bus. > > Thanks! > Joe > > Joe Komlodi (16): > hw/misc/aspeed_i3c: Move to i3c directory > hw/i3c: Add bus support > hw/i3c/aspeed_i3c: Add more register fields > hw/i3c/aspeed_i3c: Add more reset values > hw/i3c/aspeed_i3c: Add register RO field masks > hw/i3c/aspeed_i3c: Treat more registers as read-as-zero > hw/i3c/aspeed_i3c: Use 32 bits on MMIO writes > hw/i3c/aspeed_i3c: Add IRQ MMIO behavior > hw/i3c/aspeed_i3c: Add data TX and RX > hw/i3c/aspeed_i3c: Add IBI handling > hw/i3c/aspeed_i3c: Add ctrl MMIO handling > hw/i3c/aspeed_i3c: Add controller resets > hw/i3c: Add Mock target > hw/i3c: remote_i3c: Add model > qtest: remote_i3c: Add remote I3C qtest > hw/i3c: Add hotplug support Isn't this the designware i3c ip block, and as such could we name it so? I was going to send an i2c only version of this but it seems you've beaten me to it and got the i3c core going. > hw/Kconfig|1 + > hw/arm/Kconfig|2 + > hw/i3c/Kconfig| 17 + > hw/i3c/aspeed_i3c.c | 2044 + > hw/i3c/core.c | 646 +++ > hw/i3c/meson.build|6 + > hw/i3c/mock-target.c | 314 + > hw/i3c/remote-i3c.c | 469 > hw/i3c/trace-events | 52 + > hw/i3c/trace.h|1 + > hw/meson.build|1 + > hw/misc/aspeed_i3c.c | 384 --- > hw/misc/meson.build |1 - > hw/misc/trace-events |6 - > include/hw/arm/aspeed_soc.h |2 +- > include/hw/i3c/aspeed_i3c.h | 207 > include/hw/i3c/i3c.h | 275 + > include/hw/i3c/mock-target.h | 60 + > include/hw/i3c/remote-i3c.h | 72 ++ > include/hw/misc/aspeed_i3c.h | 48 - > meson.build |1 + > tests/qtest/meson.build |1 + > tests/qtest/remote-i3c-test.c | 610 ++ > 23 files changed, 4780 insertions(+), 440 deletions(-) > create mode 100644 hw/i3c/Kconfig > create mode 100644 hw/i3c/aspeed_i3c.c > create mode 100644 hw/i3c/core.c > create mode 100644 hw/i3c/meson.build > create mode 100644 hw/i3c/mock-target.c > create mode 100644 hw/i3c/remote-i3c.c > create mode 100644 hw/i3c/trace-events > create mode 100644 hw/i3c/trace.h > delete mode 100644 hw/misc/aspeed_i3c.c > create mode 100644 include/hw/i3c/aspeed_i3c.h > create mode 100644 include/hw/i3c/i3c.h > create mode 100644 include/hw/i3c/mock-target.h > create mode 100644 include/hw/i3c/remote-i3c.h > delete mode 100644 include/hw/misc/aspeed_i3c.h > create mode 100644 tests/qtest/remote-i3c-test.c > > -- > 2.40.0.348.gf938b09366-goog > > -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH v2] hw/net/can: Add mcp25625 model
On 17/03/2023 14:11, Pavel Pisa wrote: Hello Ben, thanks for update. On Thursday 16 of March 2023 13:41:13 Ben Dooks wrote: From: Ben Dooks Add support for Microchip MCP25625 SPI based CAN controller which is very similar to the MCP2515 (and covered by the same Linux driver). This can be added to any machine with SPI support in the machine model file. Example for using this when configured into a machine: -object can-bus,id=canbus0 \ -object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \ -global driver=mcp25625,property=canbus,value=canbus0 There is tracing support with --trace "*mcp25*" Code looks good, I have patched actual QEMU sources and build it successfully with your change. I have not seen any warning. I would like to test the mcp25625 CAN functionality. I would prefer against some target which is already available in QEMU and Linux kernel mainlines, so if somebody can suggest some ARM which can connect SPI/SSI device it would be great. I have setup /srv/nfs/debian-riscv64 chroot and used it to prepare minimal 3 MB ramdisk.cpio with busybox and full GLIBC and ip package. I can run it with Debian provided RISC-V kernel under QEMU compiled with your mcp25625 chip emulation qemu-system-riscv64 -m 1G -M sifive_u -smp 2 \ -initrd ramdisk.cpio \ -kernel vmlinux-6.1.0-6-riscv64 \ -nographic \ -object can-bus,id=canbus0 \ -object can-host-socketcan,id=canhost0,if=can0,canbus=canbus0 \ -global driver=mcp25625,property=canbus,value=canbus0 I can see /sys/bus/platform/devices/1004.spi /sys/bus/platform/devices/1005.spi I can run modprobe spi-sifive.ko [ 41.524160] sifive_spi 1004.spi: mapped; irq=21, cs=1 [ 41.529305] sifive_spi 1005.spi: mapped; irq=22, cs=1 modprobe mcp251x.ko I can imagine to build device tree overlay and setup it from within kernel if the device is already mapped cd /sys/kernel/config/device-tree/overlays [ -d sifive_u-mcp25625 ] && rmdir sifive_u-mcp25625 mkdir sifive_u-mcp25625 cd sifive_u-mcp25625 cat sifive_u-mcp25625.dtbo >dtbo echo 1 >status which is what we do with CTU CAN FD ip on Zynq system to run PL/FPGA update. But from QEMU info qtree, I see that device is not mapped in QEMU... Which is logic... So please, can you send instruction how to proceed forward. Do you have DTB prepared for testing or something similar? In a longer term perspective, it would be ideal to provide some update for documentation, how to use mcp25625 emulation https://www.qemu.org/docs/master/system/devices/can.html By the way, if the Raspberry Pi emulation does not provide right SPI emulation as you have noticed, what about BeagleBoneBlack? At the moment it seems that the as a whole qemu doesn't have a good way of adding a generic spi device to a bus. Does it support SPI? It could be good target to test that mcp25625 chip emulation is portable.. I've pushed our test branch out to: https://gitlab.com/CodethinkLabs/qemu/-/commits/mcp25625_test That adds an spi channel to the sifive_u machine and puts the right dtb entry in there. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
[PATCH v2] hw/net/can: Add mcp25625 model
From: Ben Dooks
Add support for Microchip MCP25625 SPI based CAN controller which is
very similar to the MCP2515 (and covered by the same Linux driver).
This can be added to any machine with SPI support in the machine
model file.
Example for using this when configured into a machine:
-object can-bus,id=canbus0 \
-object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \
-global driver=mcp25625,property=canbus,value=canbus0
There is tracing support with --trace "*mcp25*"
Signed-off-by: Ben Dooks
Co-developed-by: Nazar Kazakov
Signed-off-by: Nazar Kazakov
Co-developed-by: Lawrence Hunter
Signed-off-by: Lawrence Hunter
Reviewed-by: Frank Chang
---
hw/net/Kconfig|5 +
hw/net/can/can_mcp25625.c | 1312 +
hw/net/can/meson.build|1 +
hw/net/can/trace-events | 13 +
include/hw/net/can_mcp25625.h | 10 +
5 files changed, 1341 insertions(+)
create mode 100644 hw/net/can/can_mcp25625.c
create mode 100644 include/hw/net/can_mcp25625.h
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 18c7851efe..36f1cd1f3e 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -137,6 +137,11 @@ config ROCKER
config CAN_BUS
bool
+config CAN_MCP25625
+bool
+default y if SSI
+select CAN_BUS
+
config CAN_SJA1000
bool
default y if PCI_DEVICES
diff --git a/hw/net/can/can_mcp25625.c b/hw/net/can/can_mcp25625.c
new file mode 100644
index 00..bc035721d9
--- /dev/null
+++ b/hw/net/can/can_mcp25625.c
@@ -0,0 +1,1312 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CAN device - MCP25625 chip model
+ *
+ * Copyright (c) 2022 SiFive, Inc.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "chardev/char.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "net/can_emu.h"
+#include "hw/ssi/ssi.h"
+#include "hw/qdev-properties.h"
+#include "hw/net/can_mcp25625.h"
+
+#include "trace.h"
+
+/*
+ * Register definitons, addresses and offsets. Since the RX and TX registers
+ * exist in multiple banks we don't tend to refer to these by absolute
addresses
+ * as each bank has the same offsets for each register.
+ *
+ * note: filter registers read back 0 unless in config mode
+ */
+
+#define BANK_TXB(__b) ((__b) + 3)
+#define BANK_RXB(__b) ((__b) + 6)
+
+#define ADDR_RXB0SIDH (0x61)
+
+#define OFF_RXFSIDH (0x0)
+#define OFF_RXFSIDL (0x1)
+#define OFF_RXFEID8 (0x2)
+#define OFF_RXFEID0 (0x3)
+
+#define RXFSIDL_EXIDE (1 << 3)
+#define RXFSIDL_WRITEMASK (0xE0 | RXFSIDL_EXIDE | 0x3)
+
+struct rxfilter {
+uint8_t data[4];
+};
+
+#define BFPCTRL_WRITEMASK (0x3F)
+#define BFPCTRL_B0BFM (1 << 0)
+#define BFPCTRL_B0BFE (1 << 2)
+#define BFPCTRL_B0BFS (1 << 4)
+
+#define TXRTSCTRL_WRITEMASK (0x7)
+
+/* rx mask register offsets (read back 0 unless in config mode) */
+#define OFF_RXMSIDH (0x0)
+#define OFF_RXMSIDL (0x1)
+#define OFF_RXMEID8 (0x2)
+#define OFF_RXMEID0 (0x3)
+
+#define RXMSIDL_WRITEMASK (0xE3)
+
+struct rxmask {
+uint8_t data[4];
+};
+
+#define CNF3_WRITEMASK (0xC7)
+
+#define ADDR_TXB0SIDH (0x31)
+
+#define OFF_TXBCTRL (0x0)
+#define TXBCTRL_ABTF(1 << 6)
+#define TXBCTRL_MLOA(1 << 5)
+#define TXBCTRL_TXREQ (1 << 3)
+#define TXBCTRL_TXP1(1 << 1)
+#define TXBCTRL_TXP0(1 << 0)
+#define TXBCTRL_TXP (TXBCTRL_TXP1 | TXBCTRL_TXP0)
+#define TXBCTRL_WRITEMASK (TXBCTRL_TXREQ | TXBCTRL_TXP)
+
+#define OFF_TXBSIDH (0x1)
+
+#define OFF_TXBSIDL (0x2)
+#define TXBSIDL_EXIDE (1 << 3)
+/* bits 7..5 are SID[2:0], bits 1..0 are EID[17:16] */
+#define TXBSIDL_WRITEMASK (0xE0 | TXBSIDL_EXIDE | 0x3)
+
+#define OFF_TXBEID8 (0x3) /* this is EID[15:8] */
+#define OFF_TXBEID0 (0x4) /* this is EID[7:0] */
+
+#define OFF_TXBDLC (0x5) /* bits 3..0 are DLC */
+#define TXBDLC_RTR (1 << 6)
+#define TXBDLC_WRITEMASK(TXBDLC_RTR | 0xF)
+
+#define OFF_TXBDATA (0x6)
+
+struct txbuff {
+uint8_t data[14];
+};
+
+#define OFF_RXBCTRL (0x0)
+#define RXBCTRL_RXM_ANY (3 << 5)
+#define RXBCTRL_RXM_VALID (0 << 5)
+#define RXBCTRL_RXM_MASK(3 << 5)
+
+#define RXBCTRL_RXRTR (1 << 3)
+#define RXBCTRL_BUKT(1 << 2)
+#define RXBCTRL_BUKT1 (1 << 1)
+/* RXBCTRL0, bit0 = filter hit for message */
+/* RXBCTRL1, bits 2..0 show filter hit */
+#define RXBCTRL0_WRITEMASK (RXBCTRL_RXM_MASK | RXBCTRL_BUKT)
+#d
Re: [PATCH] hw/net/can: Add mcp25625 model
On Tue, Jan 17, 2023 at 07:16:35PM +0100, Pavel Pisa wrote:
> Dear Ben,
>
> sorry for longer response times...
I think we've both dropped the ball on this one, just got reminded about
this set and found it got deleted from work email.
We've done review upates and will try and get some branches out today and
maybe a new patch out for inclusion.
> On Tuesday 17 of January 2023 14:32:29 Ben Dooks wrote:
> > On 04/01/2023 12:22, Ben Dooks wrote:
> > > From: Ben Dooks
> > >
> > > Add support for Microchip MCP25625 SPI based CAN controller which is
> > > very similar to the MCP2515 (and covered by the same Linux driver).
> ...
> > Has anyone had chance to review this, it would be great to get
> > this moving along.
>
> Generally, I am happy that you consider use and extend our work.
>
> I have looked at the code. But even that implementation of CAN
> subsystem in QEMU was my idea and I led studnets working on
> it and sometimes heavily rewritten code to be acceptable,
> I am not QEMU expert and I have not studied its SSI subsystem
> so for comment from QEMU code requiremts I would be happy
> for some other to step in.
>
> I would like to test the peripheral. Please, can you try to elaborate
> and prepare description how to config QEMU for RPi emulation with
> mcp2515 overlay?
>
>
> https://github.com/raspberrypi/linux/blob/rpi-6.1.y/arch/arm/boot/dts/overlays/mcp2515-can0-overlay.dts
>
> I have RPi images and I have experience with this hardware.
> Even that I consider mcp2515 as really unfortunate solution
> and we have spent lot of time to help colleagues to enhance a little
> latencies of this solution when they chose that HW for serious
> wok instead of some NXP, TI, Xilinx or other sane SoC with CAN.
I had a quick look at setting an pi3b emulation but it does not currently
have an SSI port available by default. I will try and post the branch I
used with the SiFive Unmatched board later.
>
> https://dspace.cvut.cz/bitstream/handle/10467/68605/F3-DP-2017-Prudek-Martin-Dp_2017_prudek_martin.pdf
>
> But yes, people are using this chip a lot so it would worth to have
> emulation in QEMU. If the SPI connection is required then mcp251xfd
> seems to have chance for lower SPI transfer count overhead nd performance.
> But real SoC bus connected controllers are much better for serious
> project, if you design chip there are more cores available M-CAN,
> GRCAN and even our own CTU CAN FD which has already emulation in QEMU.
>
> Back to MCP25x1x. I have gone through code and try to understand
> the function. There is lot of connected to the locations in the
> registers maps in the chip
>
> > s->ssi_addr = 0x31;
A lot of these where basically used in one place and we map the regs
into internal data structs. Nazar and I have been through and tried
to tidy some of this up and make it more explicit what is being done.
> I have took manual but I think that it would help to add there comments
> with registers names or even use defines for these. But may it be,
> that for the easy ampping in the table and increment logic numbers
> are reasonable option... But comments with register symbolic names
> would help.
>
> The code does define only single property ("canbus") to select CAN
> bus to connect controller to. Mapping to the SPI peripheral is
> provided by device tree on QEMU side or by some other machine specific
> glue code? Please, can you provide more information for intended
> target use and RPi option to use for testing?
Yes, at the moment it seems that command-line mapping of SSI is not
easy, or well documented? Currently, other than the CS line that is
generally done with the controller, the only ouput needed is the IRQ
line. We do have the RXB0 and RXB1 outputs but they're not really needed
and only there for people to try if they want.
> As for the code, I have read it the first time and for full check
> I would need to spent more time with it. But I expect that
> functionality check with the respect to mcp25x1x datasheet has been
> done mainly by you so the full check bit by bit is not necessary.
> If there is some omitted case it would be (hopefully) found during
> code use. As for generic code style and redability, I see no problem.
> I expect that you have checked for QEMU style and if there has been
> some problem you have propably received QEMU CI and static analysis
> feedback.
>
> > +static void mcp25625_rx_into_buf(MCP25625State *s,
> > + const qemu_can_frame *frame,
> > + unsigned buffnr, int filthit)
> > +{
> > +struct rxbuff *rxbuff = >rxbuffs[buffnr];
> > +qemu_canid_t e_id, sidl, id, q_id = frame->
Re: [PATCH 1/1] hw/riscv/virt.c: add cbom-block-size fdt property
On 01/03/2023 21:59, Daniel Henrique Barboza wrote:
From: Anup Patel
The cbom-block-size fdt property property is used to inform the OS about
the blocksize in bytes for the Zicbom cache operations.
Linux documents it in Documentation/devicetree/bindings/riscv/cpus.yaml
as:
riscv,cbom-block-size:
$ref: /schemas/types.yaml#/definitions/uint32
description:
The blocksize in bytes for the Zicbom cache operations.
Signed-off-by: Anup Patel
---
hw/riscv/virt.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 49acb57da4..31b55cc62f 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -244,6 +244,12 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
socket,
name = riscv_isa_string(cpu_ptr);
qemu_fdt_setprop_string(ms->fdt, cpu_name, "riscv,isa", name);
g_free(name);
+
+if (cpu_ptr->cfg.ext_icbom) {
+qemu_fdt_setprop_cell(ms->fdt, cpu_name, "riscv,cbom-block-size",
+ cpu_ptr->cfg.cbom_blocksize);
+}
+
qemu_fdt_setprop_string(ms->fdt, cpu_name, "compatible", "riscv");
qemu_fdt_setprop_string(ms->fdt, cpu_name, "status", "okay");
qemu_fdt_setprop_cell(ms->fdt, cpu_name, "reg",
You'll need the same for riscv,cboz-block-size as well.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
Re: out of CI pipeline minutes again
On Thu, Feb 23, 2023 at 12:56:56PM +, Peter Maydell wrote: > Hi; the project is out of gitlab CI pipeline minutes again. > In the absence of any other proposals, no more pull request > merges will happen til 1st March... Is there a way of sponsoring more minutes, could people provide runner resources to help? -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH] hw/net/can: Add mcp25625 model
On 04/01/2023 12:22, Ben Dooks wrote: From: Ben Dooks Add support for Microchip MCP25625 SPI based CAN controller which is very similar to the MCP2515 (and covered by the same Linux driver). This can be added to any machine with SPI support in the machine model file. Example for using this when configured into a machine: -object can-bus,id=canbus0 \ -object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \ -global driver=mcp25625,property=canbus,value=canbus0 There is tracing support with --trace "*mcp25*" Signed-off-by: Ben Dooks Co-developed-by: Nazar Kazakov Signed-off-by: Nazar Kazakov Co-developed-by: Lawrence Hunter Signed-off-by: Lawrence Hunter Reviewed-by: Frank Chang Has anyone had chance to review this, it would be great to get this moving along. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
Re: [PATCH] hw/net/can: Add mcp25625 model
On 04/01/2023 12:22, Ben Dooks wrote: From: Ben Dooks Add support for Microchip MCP25625 SPI based CAN controller which is very similar to the MCP2515 (and covered by the same Linux driver). This can be added to any machine with SPI support in the machine model file. Example for using this when configured into a machine: -object can-bus,id=canbus0 \ -object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \ -global driver=mcp25625,property=canbus,value=canbus0 There is tracing support with --trace "*mcp25*" Signed-off-by: Ben Dooks Co-developed-by: Nazar Kazakov Signed-off-by: Nazar Kazakov Co-developed-by: Lawrence Hunter Signed-off-by: Lawrence Hunter Reviewed-by: Frank Chang [snip] OOPS, just noticed I forgot to send user.name and user.email in this copy of the git repo and sent from @codethink instead of @sifive. Will fix this if needed. -- Ben
[PATCH] hw/net/can: Add mcp25625 model
From: Ben Dooks
Add support for Microchip MCP25625 SPI based CAN controller which is
very similar to the MCP2515 (and covered by the same Linux driver).
This can be added to any machine with SPI support in the machine
model file.
Example for using this when configured into a machine:
-object can-bus,id=canbus0 \
-object can-host-socketcan,id=canhost0,if=vcan0,canbus=canbus0 \
-global driver=mcp25625,property=canbus,value=canbus0
There is tracing support with --trace "*mcp25*"
Signed-off-by: Ben Dooks
Co-developed-by: Nazar Kazakov
Signed-off-by: Nazar Kazakov
Co-developed-by: Lawrence Hunter
Signed-off-by: Lawrence Hunter
Reviewed-by: Frank Chang
---
hw/net/Kconfig|5 +
hw/net/can/can_mcp25625.c | 1317 +
hw/net/can/meson.build|1 +
hw/net/can/trace-events | 13 +
include/hw/net/can_mcp25625.h | 10 +
5 files changed, 1346 insertions(+)
create mode 100644 hw/net/can/can_mcp25625.c
create mode 100644 include/hw/net/can_mcp25625.h
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 6d795ec752..e058b97c23 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -132,6 +132,11 @@ config ROCKER
config CAN_BUS
bool
+config CAN_MCP25625
+bool
+default y if SSI
+select CAN_BUS
+
config CAN_SJA1000
bool
default y if PCI_DEVICES
diff --git a/hw/net/can/can_mcp25625.c b/hw/net/can/can_mcp25625.c
new file mode 100644
index 00..8a95208ee2
--- /dev/null
+++ b/hw/net/can/can_mcp25625.c
@@ -0,0 +1,1317 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CAN device - MCP25625 chip model
+ *
+ * Copyright (c) 2022 SiFive, Inc.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "chardev/char.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "net/can_emu.h"
+#include "hw/ssi/ssi.h"
+#include "hw/qdev-properties.h"
+#include "hw/net/can_mcp25625.h"
+
+#include "trace.h"
+
+/*
+ * note: filter registers read back 0 unless in config mode
+ */
+#define OFF_RXFSIDH (0x0)
+#define OFF_RXFSIDL (0x1)
+#define OFF_RXFEID8 (0x2)
+#define OFF_RXFEID0 (0x3)
+
+#define RXFSIDL_EXIDE (1 << 3)
+#define RXFSIDL_WRITEMASK (0xE0 | RXFSIDL_EXIDE | 0x3)
+
+struct rxfilter {
+uint8_t data[4];
+};
+
+#define BFPCTRL_WRITEMASK (0x3F)
+#define BFPCTRL_B0BFM (1 << 0)
+#define BFPCTRL_B0BFE (1 << 2)
+#define BFPCTRL_B0BFS (1 << 4)
+
+#define TXRTSCTRL_WRITEMASK (0x7)
+
+/* rx mask register offsets (read back 0 unless in config mode) */
+#define OFF_RXMSIDH (0x0)
+#define OFF_RXMSIDL (0x1)
+#define OFF_RXMEID8 (0x2)
+#define OFF_RXMEID0 (0x3)
+
+#define RXMSIDL_WRITEMASK (0xE3)
+
+struct rxmask {
+uint8_t data[4];
+};
+
+#define CNF3_WRITEMASK (0xC7)
+
+#define OFF_TXBCTRL (0x0)
+#define TXBCTRL_ABTF(1 << 6)
+#define TXBCTRL_MLOA(1 << 5)
+#define TXBCTRL_TXREQ (1 << 3)
+#define TXBCTRL_TXP1(1 << 1)
+#define TXBCTRL_TXP0(1 << 0)
+#define TXBCTRL_TXP (TXBCTRL_TXP1 | TXBCTRL_TXP0)
+#define TXBCTRL_WRITEMASK (TXBCTRL_TXREQ | TXBCTRL_TXP)
+
+#define OFF_TXBSIDH (0x1)
+
+#define OFF_TXBSIDL (0x2)
+#define TXBSIDL_EXIDE (1 << 3)
+/* bits 7..5 are SID[2:0], bits 1..0 are EID[17:16] */
+#define TXBSIDL_WRITEMASK (0xE0 | TXBSIDL_EXIDE | 0x3)
+
+#define OFF_TXBEID8 (0x3) /* this is EID[15:8] */
+#define OFF_TXBEID0 (0x4) /* this is EID[7:0] */
+
+#define OFF_TXBDLC (0x5) /* bits 3..0 are DLC */
+#define TXBDLC_RTR (1 << 6)
+#define TXBDLC_WRITEMASK(TXBDLC_RTR | 0xF)
+
+#define OFF_TXBDATA (0x6)
+
+struct txbuff {
+uint8_t data[14];
+};
+
+#define OFF_RXBCTRL (0x0)
+#define RXBCTRL_RXM_ANY (3 << 5)
+#define RXBCTRL_RXM_VALID (0 << 5)
+#define RXBCTRL_RXM_MASK(3 << 5)
+
+#define RXBCTRL_RXRTR (1 << 3)
+#define RXBCTRL_BUKT(1 << 2)
+#define RXBCTRL_BUKT1 (1 << 1)
+/* RXBCTRL0, bit0 = filter hit for message */
+/* RXBCTRL1, bits 2..0 show filter hit */
+#define RXBCTRL0_WRITEMASK (RXBCTRL_RXM_MASK | RXBCTRL_BUKT)
+#define RXBCTRL1_WRITEMASK (RXBCTRL_RXM_MASK)
+
+#define OFF_RXBSIDH (0x1)
+/* bits 7..0 = SID[10:3] */
+
+#define OFF_RXBSIDL (0x2)
+#define RXBSIDL_SRR (1 << 4)
+#define RXBSIDL_IDE (1 << 3)
+/* bits 7..5 = SID[2:0], bits 1..0 = EID[17:16] */
+
+#define OFF_RXBEID8 (0x3) /* bits 7..0 = EID[
Re: [PATCH] m25p80: Add the is25wp256 SFPD table
On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote: > On 12/21/22 13:22, Guenter Roeck wrote: > > Generated from hardware using the following command and then padding > > with 0xff to fill out a power-of-2: > > xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > > > > Cc: Michael Walle > > Cc: Tudor Ambarus > > Signed-off-by: Guenter Roeck > > Reviewed-by: Cédric Le Goater If SFDP is a standard, couldn't we have an function to generate it from the flash parameters? -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: add qemu_fdt_setprop_strings
On 21/10/2022 08:00, Andrew Jones wrote: On Fri, Oct 21, 2022 at 06:58:02AM +0100, Ben Dooks wrote: Add a qemu_fdt_setprop_strings to set a string array into a device-tree. Only minor updates from v4 to fix a couple of minor patch issues. Please see the comments I made on patch 1 of the v4 series, they should be addressed. Also, I'm pretty sure I gave r-b's on most, or the rest, of the series, but I don't see those here in v5. And, please CC previous reviewers when sending out new versions. Finally, why not generate this cover letter with git-format-patch? I'll go back and check the reports, I think I missed the "static const" one. I did completely forget to update the reviewed comments, so will do that and post a V6 this week. Thanks, drew -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
[PATCH v5 4/6] hw/core: use qemu_fdt_setprop_strings()
Change to using the qemu_fdt_setprop_strings() helper in
hw/core code.
Signed-off-by: Ben Dooks
---
hw/core/guest-loader.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index c61ebc4144..7b8e32e06f 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -56,18 +56,15 @@ static void loader_insert_platform_data(GuestLoaderState
*s, int size,
qemu_fdt_setprop(fdt, node, "reg", _attr, sizeof(reg_attr));
if (s->kernel) {
-const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat));
+qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,kernel");
+
if (s->args) {
qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
}
} else if (s->initrd) {
-const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat));
+qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,ramdisk");
}
}
--
2.35.1
[PATCH v5 2/6] hw/core: don't check return on qemu_fdt_setprop_string_array()
The qemu_fdt_setprop_string_array() does not return error codes and
will call exit() if any of the fdt calls fails (and should print an
error with the node being altered). This is done to prepare for the
change for qemu_fdt_setprop_strings() helper which does not return
any error codes (hw/core/guest-loader.c is the only place where an
return is checked).
Signed-off-by: Ben Dooks
---
hw/core/guest-loader.c | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index 391c875a29..c61ebc4144 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -57,25 +57,17 @@ static void loader_insert_platform_data(GuestLoaderState
*s, int size,
if (s->kernel) {
const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
-error_setg(errp, "couldn't set %s/compatible", node);
-return;
-}
+qemu_fdt_setprop_string_array(fdt, node, "compatible",
+ (char **) ,
+ ARRAY_SIZE(compat));
if (s->args) {
-if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) {
-error_setg(errp, "couldn't set %s/bootargs", node);
-}
+qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
}
} else if (s->initrd) {
const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
-error_setg(errp, "couldn't set %s/compatible", node);
-return;
-}
+qemu_fdt_setprop_string_array(fdt, node, "compatible",
+ (char **) ,
+ ARRAY_SIZE(compat));
}
}
--
2.35.1
add qemu_fdt_setprop_strings
Add a qemu_fdt_setprop_strings to set a string array into a device-tree. Only minor updates from v4 to fix a couple of minor patch issues.
[PATCH v5 6/6] hw/arm: change to use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() instead of using
\0 separated string arrays. Note, also there were a few places
where qemu_fdt_setprop_string() can be used in the same areas.
Signed-off-by: Ben Dooks
---
v4:
- fixed checkpatch errors with string
- fixed patch subject
---
hw/arm/boot.c | 8 +++---
hw/arm/virt.c | 28 +
hw/arm/xlnx-versal-virt.c | 51 ---
3 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index b0b92af188..8cce4e0f4d 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -490,11 +490,11 @@ static void fdt_add_psci_node(void *fdt)
qemu_fdt_add_subnode(fdt, "/psci");
if (armcpu->psci_version >= QEMU_PSCI_VERSION_0_2) {
if (armcpu->psci_version < QEMU_PSCI_VERSION_1_0) {
-const char comp[] = "arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-0.2", "arm,psci");
} else {
-const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-1.0", "arm,psci-0.2",
"arm,psci");
}
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index cda9defe8f..1917929e6a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -345,9 +345,8 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
armcpu = ARM_CPU(qemu_get_cpu(0));
if (arm_feature(>env, ARM_FEATURE_V8)) {
-const char compat[] = "arm,armv8-timer\0arm,armv7-timer";
-qemu_fdt_setprop(ms->fdt, "/timer", "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, "/timer", "compatible",
+ "arm,armv8-timer", "arm,armv7-timer");
} else {
qemu_fdt_setprop_string(ms->fdt, "/timer", "compatible",
"arm,armv7-timer");
@@ -846,8 +845,6 @@ static void create_uart(const VirtMachineState *vms, int
uart,
hwaddr base = vms->memmap[uart].base;
hwaddr size = vms->memmap[uart].size;
int irq = vms->irqmap[uart];
-const char compat[] = "arm,pl011\0arm,primecell";
-const char clocknames[] = "uartclk\0apb_pclk";
DeviceState *dev = qdev_new(TYPE_PL011);
SysBusDevice *s = SYS_BUS_DEVICE(dev);
MachineState *ms = MACHINE(vms);
@@ -861,8 +858,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
nodename = g_strdup_printf("/pl011@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
/* Note that we can't use setprop_string because of the embedded NUL */
-qemu_fdt_setprop(ms->fdt, nodename, "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+ "arm,pl011", "arm,primecell");
qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
2, base, 2, size);
qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -870,8 +867,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
GIC_FDT_IRQ_FLAGS_LEVEL_HI);
qemu_fdt_setprop_cells(ms->fdt, nodename, "clocks",
vms->clock_phandle, vms->clock_phandle);
-qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
- clocknames, sizeof(clocknames));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "clock-names",
+ "uartclk", "apb_pclk");
if (uart == VIRT_UART) {
qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
@@ -893,14 +890,14 @@ static void create_rtc(const VirtMachineState *vms)
hwaddr base = vms->memmap[VIRT_RTC].base;
hwaddr size = vms->memmap[VIRT_RTC].size;
int irq = vms->irqmap[VIRT_RTC];
-const char compat[] = "arm,pl031\0arm,primecell";
MachineState *ms = MACHINE(vms);
sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq));
nodename = g_strdup_printf("/pl031@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
-qemu_fdt_set
[PATCH v5 5/6] hw/mips: use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() helper in hw/mips.
Signed-off-by: Ben Dooks
Reviewed-by: Peter Maydell
---
hw/mips/boston.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index d2ab9da1a0..759f6daafe 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
MachineState *mc = s->mach;
uint32_t platreg_ph, gic_ph, clk_ph;
char *name, *gic_name, *platreg_name, *stdout_name;
-static const char * const syscon_compat[2] = {
-"img,boston-platform-regs", "syscon"
-};
fdt = create_device_tree(dt_size);
if (!fdt) {
@@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
memmap[BOSTON_PLATREG].base);
qemu_fdt_add_subnode(fdt, platreg_name);
-qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
- (char **)_compat,
- ARRAY_SIZE(syscon_compat));
+qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
+ "img,boston-platform-regs", "syscon");
qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
memmap[BOSTON_PLATREG].base,
memmap[BOSTON_PLATREG].size);
--
2.35.1
[PATCH v5 3/6] hw/riscv: use qemu_fdt_setprop_strings() for string arrays
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.
Signed-off-by: Ben Dooks
---
v5: fix re-ordering in sifive_u
---
hw/riscv/sifive_u.c | 18 +-
hw/riscv/spike.c| 7 ++-
hw/riscv/virt.c | 25 ++---
3 files changed, 13 insertions(+), 37 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index b139824aab..2e0e78cbee 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,8 +214,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_cells(fdt, nodename, "reg",
0x0, memmap[SIFIVE_U_DEV_CLINT].base,
0x0, memmap[SIFIVE_U_DEV_CLINT].size);
@@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 1e1d752c00..80480d4ab4 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -61,9 +61,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
uint32_t cpu_phandle, intc_phandle, phandle = 1;
char *name, *mem_name, *clint_name, *clust_name;
char *core_name, *cpu_name, *intc_name;
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
fdt = s->fdt = create_device_tree(>fdt_size);
if (!fdt) {
@@ -161,8 +158,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
(memmap[SPIKE_CLINT].size * socket);
clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
qemu_fdt_add_subnode(fdt, clint_name);
-qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_cells(fdt, clint_name, "reg",
0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a5bc7353b4..5c644212a2 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -303,9 +303,6 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
uint32_t *clint_cells;
unsigned long clint_addr;
MachineState *mc = MACHINE(s);
-static const char * const clint_compat[2] = {
-
[PATCH v5 1/6] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
---
v4:
- go back to the non-return call, no-one is using the result
v3;
- fix return value for the call
- add better help text
v2:
- fix node/path in comment
---
include/sysemu/device_tree.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index e7c5441f56..cb49df471b 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ *
+ * This is a helper for the qemu_fdt_setprop_string_array() function
+ * which takes a va-arg set of strings instead of having to setup a
+ * single use string array.
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+do {\
+static const char * const __strs[] = { __VA_ARGS__ }; \
+qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+ } while(0)
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
Re: [PATCH] mem/cxl-type3: Add sn option to provide serial number for PCI ecap
On 22-09-23 17:18:35, Jonathan Cameron wrote:
> The Device Serial Number Extended Capability PCI r6.0 sec 7.9.3
> provides a standard way to provide a device serial number as
> an IEEE defined 64-bit extended unique identifier EUI-64.
>
> CXL 2.0 section 8.1.12.2 Memory Device PCIe Capabilities and
> Extended Capabilities requires this to be used to uniquely
> identify CXL memory devices.
>
> Signed-off-by: Jonathan Cameron
Reviewed-by: Ben Widawsky
> ---
>
> This is the missing element to be able to use the Linux kernel
> support for PMEM region creation. Without this you can create
> a region, but not remount it after reboot (as the label
> is not valid).
>
> hw/mem/cxl_type3.c | 14 +-
> include/hw/cxl/cxl_device.h | 1 +
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 3bf2869573..e0c1535b73 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -14,6 +14,12 @@
> #include "sysemu/hostmem.h"
> #include "hw/cxl/cxl.h"
>
> +/*
> + * Null value of all Fs suggested by IEEE RA guidelines for use of
> + * EU, OUI and CID
> + */
> +#define UI64_NULL ~(0ULL)
> +
> static void build_dvsecs(CXLType3Dev *ct3d)
> {
> CXLComponentState *cxl_cstate = >cxl_cstate;
> @@ -149,7 +155,12 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> pci_config_set_class(pci_conf, PCI_CLASS_MEMORY_CXL);
>
> pcie_endpoint_cap_init(pci_dev, 0x80);
> -cxl_cstate->dvsec_offset = 0x100;
> +if (ct3d->sn != UI64_NULL) {
> +pcie_dev_ser_num_init(pci_dev, 0x100, ct3d->sn);
> +cxl_cstate->dvsec_offset = 0x100 + 0x0c;
> +} else {
> +cxl_cstate->dvsec_offset = 0x100;
> +}
Perhaps just always make it 0x10c to keep it simple and debuggable?
>
> ct3d->cxl_cstate.pdev = pci_dev;
> build_dvsecs(ct3d);
> @@ -275,6 +286,7 @@ static Property ct3_props[] = {
> HostMemoryBackend *),
> DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND,
> HostMemoryBackend *),
> +DEFINE_PROP_UINT64("sn", CXLType3Dev, sn, UI64_NULL),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 1e141b6621..e4d221cdb3 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -237,6 +237,7 @@ struct CXLType3Dev {
> /* Properties */
> HostMemoryBackend *hostmem;
> HostMemoryBackend *lsa;
> +uint64_t sn;
>
> /* State */
> AddressSpace hostmem_as;
> --
> 2.32.0
>
[PATCH v4 6/6] hw/arm: change to use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() instead of using
\0 separated string arrays. Note, also there were a few places
where qemu_fdt_setprop_string() can be used in the same areas.
Signed-off-by: Ben Dooks
---
v4:
- fixed checkpatch errors with string
- fixed patch subject
---
hw/arm/boot.c | 8 +++---
hw/arm/virt.c | 28 +
hw/arm/xlnx-versal-virt.c | 51 ---
3 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..766257cbfb 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -490,11 +490,11 @@ static void fdt_add_psci_node(void *fdt)
qemu_fdt_add_subnode(fdt, "/psci");
if (armcpu->psci_version >= QEMU_PSCI_VERSION_0_2) {
if (armcpu->psci_version < QEMU_PSCI_VERSION_1_0) {
-const char comp[] = "arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-0.2", "arm,psci");
} else {
-const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-1.0", "arm,psci-0.2",
"arm,psci");
}
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9633f822f3..2670c13ef1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -343,9 +343,8 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
armcpu = ARM_CPU(qemu_get_cpu(0));
if (arm_feature(>env, ARM_FEATURE_V8)) {
-const char compat[] = "arm,armv8-timer\0arm,armv7-timer";
-qemu_fdt_setprop(ms->fdt, "/timer", "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, "/timer", "compatible",
+ "arm,armv8-timer", "arm,armv7-timer");
} else {
qemu_fdt_setprop_string(ms->fdt, "/timer", "compatible",
"arm,armv7-timer");
@@ -843,8 +842,6 @@ static void create_uart(const VirtMachineState *vms, int
uart,
hwaddr base = vms->memmap[uart].base;
hwaddr size = vms->memmap[uart].size;
int irq = vms->irqmap[uart];
-const char compat[] = "arm,pl011\0arm,primecell";
-const char clocknames[] = "uartclk\0apb_pclk";
DeviceState *dev = qdev_new(TYPE_PL011);
SysBusDevice *s = SYS_BUS_DEVICE(dev);
MachineState *ms = MACHINE(vms);
@@ -858,8 +855,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
nodename = g_strdup_printf("/pl011@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
/* Note that we can't use setprop_string because of the embedded NUL */
-qemu_fdt_setprop(ms->fdt, nodename, "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+ "arm,pl011", "arm,primecell");
qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
2, base, 2, size);
qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -867,8 +864,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
GIC_FDT_IRQ_FLAGS_LEVEL_HI);
qemu_fdt_setprop_cells(ms->fdt, nodename, "clocks",
vms->clock_phandle, vms->clock_phandle);
-qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
- clocknames, sizeof(clocknames));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "clock-names",
+ "uartclk", "apb_pclk");
if (uart == VIRT_UART) {
qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
@@ -890,14 +887,14 @@ static void create_rtc(const VirtMachineState *vms)
hwaddr base = vms->memmap[VIRT_RTC].base;
hwaddr size = vms->memmap[VIRT_RTC].size;
int irq = vms->irqmap[VIRT_RTC];
-const char compat[] = "arm,pl031\0arm,primecell";
MachineState *ms = MACHINE(vms);
sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq));
nodename = g_strdup_printf("/pl031@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
-qemu_fdt_set
[PATCH v4 4/6] hw/core: use qemu_fdt_setprop_strings()
Change to using the qemu_fdt_setprop_strings() helper in
hw/core code.
Signed-off-by: Ben Dooks
---
hw/core/guest-loader.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index c61ebc4144..7b8e32e06f 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -56,18 +56,15 @@ static void loader_insert_platform_data(GuestLoaderState
*s, int size,
qemu_fdt_setprop(fdt, node, "reg", _attr, sizeof(reg_attr));
if (s->kernel) {
-const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat));
+qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,kernel");
+
if (s->args) {
qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
}
} else if (s->initrd) {
-const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat));
+qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,ramdisk");
}
}
--
2.35.1
Re: add qemu_fdt_setprop_strings() and use it in most places
On Tue, Aug 09, 2022 at 07:56:34PM +0100, Ben Dooks wrote:
> Add a helper for qemu_fdt_setprop_strings() to take a set of strings
> to put into a device-tree, which removes several open-coded methods
> such as setting an char arr[] = {..} or setting char val[] = "str\0str2";
>
>
> This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
> is not fully tested over all of those, I may not have caught all places
> this is to be replaced.
I've put a github repo up with this and possibly some other bits I am
working on:
https://github.com/bendooks/qemu/tree/dev/fdt-string-array-v4
> Changes:
>
> v4:
> - fix checkpatch issues
> - remove error checking in hw/core/guest-loader.c
> v3:
> - fix return value for the call
>
> - add better help text
>
> v2:
>
> - fix node/path in comment
>
>
>
>
--
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
[PATCH v4 1/6] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
---
v4:
- go back to the non-return call, no-one is using the result
v3;
- fix return value for the call
- add better help text
v2:
- fix node/path in comment
---
include/sysemu/device_tree.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..d5c05b5ebb 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ *
+ * This is a helper for the qemu_fdt_setprop_string_array() function
+ * which takes a va-arg set of strings instead of having to setup a
+ * single use string array.
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+do {\
+static const char * const __strs[] = { __VA_ARGS__ }; \
+qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+ } while(0)
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
[PATCH v4 2/6] hw/core: don't check return on qemu_fdt_setprop_string_array()
The qemu_fdt_setprop_string_array() does not return error codes and
will call exit() if any of the fdt calls fails (and should print an
error with the node being altered). This is done to prepare for the
change for qemu_fdt_setprop_strings() helper which does not return
any error codes (hw/core/guest-loader.c is the only place where an
return is checked).
Signed-off-by: Ben Dooks
---
hw/core/guest-loader.c | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index 391c875a29..c61ebc4144 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -57,25 +57,17 @@ static void loader_insert_platform_data(GuestLoaderState
*s, int size,
if (s->kernel) {
const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
-error_setg(errp, "couldn't set %s/compatible", node);
-return;
-}
+qemu_fdt_setprop_string_array(fdt, node, "compatible",
+ (char **) ,
+ ARRAY_SIZE(compat));
if (s->args) {
-if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) {
-error_setg(errp, "couldn't set %s/bootargs", node);
-}
+qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
}
} else if (s->initrd) {
const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
-error_setg(errp, "couldn't set %s/compatible", node);
-return;
-}
+qemu_fdt_setprop_string_array(fdt, node, "compatible",
+ (char **) ,
+ ARRAY_SIZE(compat));
}
}
--
2.35.1
add qemu_fdt_setprop_strings() and use it in most places
Add a helper for qemu_fdt_setprop_strings() to take a set of strings
to put into a device-tree, which removes several open-coded methods
such as setting an char arr[] = {..} or setting char val[] = "str\0str2";
This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
is not fully tested over all of those, I may not have caught all places
this is to be replaced.
Changes:
v4:
- fix checkpatch issues
- remove error checking in hw/core/guest-loader.c
v3:
- fix return value for the call
- add better help text
v2:
- fix node/path in comment
[PATCH v4 5/6] hw/mips: use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() helper in hw/mips.
Signed-off-by: Ben Dooks
Reviewed-by: Peter Maydell
---
hw/mips/boston.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index d2ab9da1a0..759f6daafe 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
MachineState *mc = s->mach;
uint32_t platreg_ph, gic_ph, clk_ph;
char *name, *gic_name, *platreg_name, *stdout_name;
-static const char * const syscon_compat[2] = {
-"img,boston-platform-regs", "syscon"
-};
fdt = create_device_tree(dt_size);
if (!fdt) {
@@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
memmap[BOSTON_PLATREG].base);
qemu_fdt_add_subnode(fdt, platreg_name);
-qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
- (char **)_compat,
- ARRAY_SIZE(syscon_compat));
+qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
+ "img,boston-platform-regs", "syscon");
qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
memmap[BOSTON_PLATREG].base,
memmap[BOSTON_PLATREG].size);
--
2.35.1
[PATCH v4 3/6] hw/riscv: use qemu_fdt_setprop_strings() for string arrays
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.
Signed-off-by: Ben Dooks
---
hw/riscv/sifive_u.c | 18 +-
hw/riscv/spike.c| 7 ++-
hw/riscv/virt.c | 32
3 files changed, 15 insertions(+), 42 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..dc112a253a 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,11 +214,11 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
qemu_fdt_setprop_cells(fdt, nodename, "reg",
0x0, memmap[SIFIVE_U_DEV_CLINT].base,
0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
g_free(cells);
@@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e41b6aa9f0..aa895779cd 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -59,9 +59,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
uint32_t cpu_phandle, intc_phandle, phandle = 1;
char *name, *mem_name, *clint_name, *clust_name;
char *core_name, *cpu_name, *intc_name;
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
fdt = s->fdt = create_device_tree(>fdt_size);
if (!fdt) {
@@ -159,8 +156,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
(memmap[SPIKE_CLINT].size * socket);
clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
qemu_fdt_add_subnode(fdt, clint_name);
-qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_cells(fdt, clint_name, "reg",
0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc424dd2f5..c6aaa611a6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -261,11 +261,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
socket,
Re: [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
On Mon, Aug 01, 2022 at 12:30:22PM +0100, Peter Maydell wrote: > On Wed, 27 Jul 2022 at 23:39, Ben Dooks wrote: > > > > Change to using the qemu_fdt_setprop_strings() helper in > > hw/core code. > > > > Signed-off-by: Ben Dooks > > --- > > Reviewed-by: Peter Maydell I've had to make a second version, so assuming it'll need reviewing again -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
On Mon, Aug 01, 2022 at 12:37:33PM +0100, Peter Maydell wrote:
> On Wed, 27 Jul 2022 at 23:44, Ben Dooks wrote:
> >
> > Change to using qemu_fdt_setprop_strings() instead of using
> > \0 separated string arrays.
> >
> > Signed-off-by: Ben Dooks
> > ---
> > hw/arm/boot.c | 8 +++---
> > hw/arm/virt.c | 28 +
> > hw/arm/xlnx-versal-virt.c | 51 ---
> > 3 files changed, 37 insertions(+), 50 deletions(-)
> >
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index ada2717f76..bf29b7ae60 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -490,11 +490,11 @@ static void fdt_add_psci_node(void *fdt)
> > qemu_fdt_add_subnode(fdt, "/psci");
> > if (armcpu->psci_version >= QEMU_PSCI_VERSION_0_2) {
> > if (armcpu->psci_version < QEMU_PSCI_VERSION_1_0) {
> > -const char comp[] = "arm,psci-0.2\0arm,psci";
> > -qemu_fdt_setprop(fdt, "/psci", "compatible", comp,
> > sizeof(comp));
> > +qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
> > + "arm,psci-0.2", "arm,psci");
>
> I think you may have some stray trailing whitespace here.
> checkpatch should be able to tell you.
ok, will check
> > @@ -858,8 +855,8 @@ static void create_uart(const VirtMachineState *vms,
> > int uart,
> > nodename = g_strdup_printf("/pl011@%" PRIx64, base);
> > qemu_fdt_add_subnode(ms->fdt, nodename);
> > /* Note that we can't use setprop_string because of the embedded NUL */
>
> With this change, this comment becomes obsolete, and we should delete it too.
>
> > -qemu_fdt_setprop(ms->fdt, nodename, "compatible",
> > - compat, sizeof(compat));
> > +qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
> > + "arm,pl011", "arm,primecell");
> > qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
> > 2, base, 2, size);
> > qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
>
>
>
> > @@ -285,8 +280,6 @@ static void fdt_add_gem_nodes(VersalVirt *s)
> >
> > static void fdt_add_zdma_nodes(VersalVirt *s)
> > {
> > -const char clocknames[] = "clk_main\0clk_apb";
> > -const char compat[] = "xlnx,zynqmp-dma-1.0";
>
> This looks suspiciously like a pre-existing bug to me.
> Alaistair, Edgar -- shouldn't this be a NUL-separated
> 'compatible' string, rather than a comma-separated one?
I think the compat[] is fine, I should have probably added I also fixed
up to just call qemu_fdt_setprop_string()
> > int i;
> >
> > for (i = XLNX_VERSAL_NR_ADMAS - 1; i >= 0; i--) {
> > @@ -298,22 +291,21 @@ static void fdt_add_zdma_nodes(VersalVirt *s)
> > qemu_fdt_setprop_cell(s->fdt, name, "xlnx,bus-width", 64);
> > qemu_fdt_setprop_cells(s->fdt, name, "clocks",
> > s->phandle.clk_25Mhz, s->phandle.clk_25Mhz);
> > -qemu_fdt_setprop(s->fdt, name, "clock-names",
> > - clocknames, sizeof(clocknames));
> > +qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
> > + "clk_main", "clk_apb");
> > qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
> > GIC_FDT_IRQ_TYPE_SPI, VERSAL_ADMA_IRQ_0 + i,
> > GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
> > 2, addr, 2, 0x1000);
> > -qemu_fdt_setprop(s->fdt, name, "compatible", compat,
> > sizeof(compat));
> > +qemu_fdt_setprop_string(s->fdt, name, "compatible",
> > +"xlnx,zynqmp-dma-1.0");
> > g_free(name);
> > }
> > }
> >
> > static void fdt_add_sd_nodes(VersalVirt *s)
> > {
> > -const char clocknames[] = "clk_xin\0clk_ahb";
> > -const char compat[] = "arasan,sdhci-8.9a";
>
> Ditto here...
>
> > int i;
> >
> > for (i = ARRAY_SIZE(s->soc.pmc.iou.sd) - 1; i >= 0; i--) {
> > @@ -324,22 +316,2
Re: [PATCH v7 2/3] target/riscv: Add stimecmp support
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (env->priv == PRV_M) {
+ return RISCV_EXCP_NONE;
+}
+
+if (env->priv != PRV_S) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
These seem to be checking env->priv twice, wouldnt
one check for nv->priv != PRV_S be sufficient?
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
Re: [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
On Thu, Jul 28, 2022 at 11:22:27AM +0200, Andrew Jones wrote:
> On Wed, Jul 27, 2022 at 11:39:01PM +0100, Ben Dooks wrote:
> > Add a helper to set a property from a set of strings
> > to reduce the following code:
> >
> > static const char * const clint_compat[2] = {
> > "sifive,clint0", "riscv,clint0"
> > };
> >
> > qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> > (char **)_compat, ARRAY_SIZE(clint_compat));
> >
> > Signed-off-by: Ben Dooks
> > ---
> > v3;
> > - fix return value for the call
> > - add better help text
> > v2:
> > - fix node/path in comment
> > ---
> > include/sysemu/device_tree.h | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> > index ef060a9759..83bdfe390e 100644
> > --- a/include/sysemu/device_tree.h
> > +++ b/include/sysemu/device_tree.h
> > @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char
> > *node_path,
> > int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
> >const char *prop, char **array, int len);
> >
> > +/**
> > + * qemu_fdt_setprop_strings: set a property from a set of strings
> > + *
> > + * @fdt: pointer to the dt blob
> > + * @path: node name
> > + * @prop: property array
> > + *
> > + * This is a helper for the qemu_fdt_setprop_string_array() function
> > + * which takes a va-arg set of strings instead of having to setup a
> > + * single use string array.
> > + */
> > +#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
> > +({ int __ret; do { \
> > +static const char * const __strs[] = { __VA_ARGS__ }; \
> > +__ret = qemu_fdt_setprop_string_array(fdt, path, prop, \
> > +(char **)&__strs, ARRAY_SIZE(__strs)); \
> > + } while(0); __ret; })
>
> The do { } while (0) shouldn't be necessary inside the ({}), but I
> think we should change the places that are checking the return value
> of qemu_fdt_setprop_string_array() to not check the return value,
> because it will always be zero. qemu_fdt_setprop_string_array() calls
> qemu_fdt_setprop() which exits QEMU on error. Then, after there are
> no callers checking the return value we can change this macro to
I think I did this due to the hw/ppc changes that /do/ check the
return code but are different enough to not be trivially changable.
I'll go back and make this the original do {} while() tongith and
post a v4 for people to look. The hw/ppc stuff can stay as is for
now.
Thank you for the review.
--
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr
On 28/07/2022 07:15, Mayuresh Chitale wrote:
On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:
在 2022/7/24 下午11:49, Mayuresh Chitale 写道:
On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:
在 2022/7/21 下午11:31, Mayuresh Chitale 写道:
If smstateen is implemented and sstateen0.fcsr is clear then
the
floating point operations must return illegal instruction
exception.
Signed-off-by: Mayuresh Chitale
---
target/riscv/csr.c| 23 ++
target/riscv/insn_trans/trans_rvf.c.inc | 38
+--
target/riscv/insn_trans/trans_rvzfh.c.inc | 4 +++
3 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,10 @@ static RISCVException fs(CPURISCVState *env,
int
csrno)
!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
return RISCV_EXCP_ILLEGAL_INST;
}
+
+if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
+}
#endif
return RISCV_EXCP_NONE;
}
@@ -1876,6 +1880,9 @@ static RISCVException
write_mstateen0(CPURISCVState *env, int csrno,
target_ulong new_val)
{
uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
return write_mstateen(env, csrno, wr_mask, new_val);
}
@@ -1924,6 +1931,10 @@ static RISCVException
write_mstateen0h(CPURISCVState *env, int csrno,
{
uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
return write_mstateenh(env, csrno, wr_mask, new_val);
}
@@ -1973,6 +1984,10 @@ static RISCVException
write_hstateen0(CPURISCVState *env, int csrno,
{
uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
return write_hstateen(env, csrno, wr_mask, new_val);
}
@@ -2024,6 +2039,10 @@ static RISCVException
write_hstateen0h(CPURISCVState *env, int csrno,
{
uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
return write_hstateenh(env, csrno, wr_mask, new_val);
}
@@ -2083,6 +2102,10 @@ static RISCVException
write_sstateen0(CPURISCVState *env, int csrno,
{
uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
return write_sstateen(env, csrno, wr_mask, new_val);
}
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc
b/target/riscv/insn_trans/trans_rvf.c.inc
index a1d3eb52ad..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
return false; \
} while (0)
+#ifndef CONFIG_USER_ONLY
+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS))
{\eventually
meaning
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \
given the size of that I would have thought an "static inline"
function would be easier to write and maintain for SMSTATEEN_CHECK
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
Re: add qemu_fdt_setprop_strings() and use it in most places
On Wed, Jul 27, 2022 at 11:39:00PM +0100, Ben Dooks wrote:
> Add a helper for qemu_fdt_setprop_strings() to take a set of strings
> to put into a device-tree, which removes several open-coded methods
> such as setting an char arr[] = {..} or setting char val[] = "str\0str2";
>
> This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
> is not fully tested over all of those, I may not have caught all places
> this is to be replaced.
I've added a branch at
https://github.com/bendooks/qemu.git dev/fdt-string-array
--
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
[PATCH v3 4/5] hw/mips: use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() helper in hw/mips.
Signed-off-by: Ben Dooks
---
hw/mips/boston.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index d2ab9da1a0..759f6daafe 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
MachineState *mc = s->mach;
uint32_t platreg_ph, gic_ph, clk_ph;
char *name, *gic_name, *platreg_name, *stdout_name;
-static const char * const syscon_compat[2] = {
-"img,boston-platform-regs", "syscon"
-};
fdt = create_device_tree(dt_size);
if (!fdt) {
@@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
memmap[BOSTON_PLATREG].base);
qemu_fdt_add_subnode(fdt, platreg_name);
-qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
- (char **)_compat,
- ARRAY_SIZE(syscon_compat));
+qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
+ "img,boston-platform-regs", "syscon");
qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
memmap[BOSTON_PLATREG].base,
memmap[BOSTON_PLATREG].size);
--
2.35.1
[PATCH v3 2/5] hw/riscv: use qemu_fdt_setprop_strings() for string arrays
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.
Signed-off-by: Ben Dooks
---
hw/riscv/sifive_u.c | 18 +-
hw/riscv/spike.c| 7 ++-
hw/riscv/virt.c | 32
3 files changed, 15 insertions(+), 42 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..dc112a253a 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,11 +214,11 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
qemu_fdt_setprop_cells(fdt, nodename, "reg",
0x0, memmap[SIFIVE_U_DEV_CLINT].base,
0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
g_free(cells);
@@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e41b6aa9f0..aa895779cd 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -59,9 +59,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
uint32_t cpu_phandle, intc_phandle, phandle = 1;
char *name, *mem_name, *clint_name, *clust_name;
char *core_name, *cpu_name, *intc_name;
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
fdt = s->fdt = create_device_tree(>fdt_size);
if (!fdt) {
@@ -159,8 +156,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry
*memmap,
(memmap[SPIKE_CLINT].size * socket);
clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
qemu_fdt_add_subnode(fdt, clint_name);
-qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_cells(fdt, clint_name, "reg",
0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc424dd2f5..c6aaa611a6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -261,11 +261,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
socket,
add qemu_fdt_setprop_strings() and use it in most places
Add a helper for qemu_fdt_setprop_strings() to take a set of strings
to put into a device-tree, which removes several open-coded methods
such as setting an char arr[] = {..} or setting char val[] = "str\0str2";
This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
is not fully tested over all of those, I may not have caught all places
this is to be replaced.
[PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
---
v3;
- fix return value for the call
- add better help text
v2:
- fix node/path in comment
---
include/sysemu/device_tree.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..83bdfe390e 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ *
+ * This is a helper for the qemu_fdt_setprop_string_array() function
+ * which takes a va-arg set of strings instead of having to setup a
+ * single use string array.
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+({ int __ret; do { \
+static const char * const __strs[] = { __VA_ARGS__ }; \
+__ret = qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+ } while(0); __ret; })
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
[PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
Change to using qemu_fdt_setprop_strings() instead of using
\0 separated string arrays.
Signed-off-by: Ben Dooks
---
hw/arm/boot.c | 8 +++---
hw/arm/virt.c | 28 +
hw/arm/xlnx-versal-virt.c | 51 ---
3 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..bf29b7ae60 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -490,11 +490,11 @@ static void fdt_add_psci_node(void *fdt)
qemu_fdt_add_subnode(fdt, "/psci");
if (armcpu->psci_version >= QEMU_PSCI_VERSION_0_2) {
if (armcpu->psci_version < QEMU_PSCI_VERSION_1_0) {
-const char comp[] = "arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-0.2", "arm,psci");
} else {
-const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+ "arm,psci-1.0", "arm,psci-0.2",
"arm,psci");
}
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9633f822f3..2670c13ef1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -343,9 +343,8 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
armcpu = ARM_CPU(qemu_get_cpu(0));
if (arm_feature(>env, ARM_FEATURE_V8)) {
-const char compat[] = "arm,armv8-timer\0arm,armv7-timer";
-qemu_fdt_setprop(ms->fdt, "/timer", "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, "/timer", "compatible",
+ "arm,armv8-timer", "arm,armv7-timer");
} else {
qemu_fdt_setprop_string(ms->fdt, "/timer", "compatible",
"arm,armv7-timer");
@@ -843,8 +842,6 @@ static void create_uart(const VirtMachineState *vms, int
uart,
hwaddr base = vms->memmap[uart].base;
hwaddr size = vms->memmap[uart].size;
int irq = vms->irqmap[uart];
-const char compat[] = "arm,pl011\0arm,primecell";
-const char clocknames[] = "uartclk\0apb_pclk";
DeviceState *dev = qdev_new(TYPE_PL011);
SysBusDevice *s = SYS_BUS_DEVICE(dev);
MachineState *ms = MACHINE(vms);
@@ -858,8 +855,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
nodename = g_strdup_printf("/pl011@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
/* Note that we can't use setprop_string because of the embedded NUL */
-qemu_fdt_setprop(ms->fdt, nodename, "compatible",
- compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+ "arm,pl011", "arm,primecell");
qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
2, base, 2, size);
qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -867,8 +864,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
GIC_FDT_IRQ_FLAGS_LEVEL_HI);
qemu_fdt_setprop_cells(ms->fdt, nodename, "clocks",
vms->clock_phandle, vms->clock_phandle);
-qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
- clocknames, sizeof(clocknames));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "clock-names",
+ "uartclk", "apb_pclk");
if (uart == VIRT_UART) {
qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
@@ -890,14 +887,14 @@ static void create_rtc(const VirtMachineState *vms)
hwaddr base = vms->memmap[VIRT_RTC].base;
hwaddr size = vms->memmap[VIRT_RTC].size;
int irq = vms->irqmap[VIRT_RTC];
-const char compat[] = "arm,pl031\0arm,primecell";
MachineState *ms = MACHINE(vms);
sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq));
nodename = g_strdup_printf("/pl031@%" PRIx64, base);
qemu_fdt_add_subnode(ms->fdt, nodename);
-qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+
[PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
Change to using the qemu_fdt_setprop_strings() helper in
hw/core code.
Signed-off-by: Ben Dooks
---
hw/core/guest-loader.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index 391c875a29..203090503e 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -56,10 +56,8 @@ static void loader_insert_platform_data(GuestLoaderState *s,
int size,
qemu_fdt_setprop(fdt, node, "reg", _attr, sizeof(reg_attr));
if (s->kernel) {
-const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
+if (qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,kernel") <
0) {
error_setg(errp, "couldn't set %s/compatible", node);
return;
}
@@ -69,10 +67,8 @@ static void loader_insert_platform_data(GuestLoaderState *s,
int size,
}
}
} else if (s->initrd) {
-const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
- (char **) ,
- ARRAY_SIZE(compat)) < 0) {
+if (qemu_fdt_setprop_strings(fdt, node, "compatible",
+ "multiboot,module", "multiboot,ramdisk")
< 0) {
error_setg(errp, "couldn't set %s/compatible", node);
return;
}
--
2.35.1
Re: fu740 target
On 27/07/2022 15:38, Bin Meng wrote: On Wed, Jul 27, 2022 at 10:24 PM Ben Dooks wrote: Is anyone working on adding a sifive-u74 core to the list of supported CPU types? I was looking at full emulation of the Unmatched but at the moment the best we have is sifive-u54 and I think that misses at least two CSRs the sifive-u74 has. Does anyone have plans to add the sifive-u74, and if not, would a plan to add gradual support for it like adding CSRs 0x7c1 and 0x7c2 so we can run an Unmatched U-boot SPL against it. Adding 0x7c1/7c2 would be a vendor-specific CSR approach? Part of the FU740 feature disable controls If not, is there a definitive U54->U74 set of public differnces around we could use to start from? I'd like to be able to run a full Unmatched image using qemu at some point to add to the current real-board testing we're doing. (I have a basic addition of the type and the two CSRs as a couple of patches if that would help as a start) I am not aware of anyone doing U74 modeling in QEMU, but SiFive folks (+Frank) may have one downstream as I see they posted several bug fixes in the existing U54 model. Regards, Bin Thanks -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html
fu740 target
Is anyone working on adding a sifive-u74 core to the list of supported CPU types? I was looking at full emulation of the Unmatched but at the moment the best we have is sifive-u54 and I think that misses at least two CSRs the sifive-u74 has. Does anyone have plans to add the sifive-u74, and if not, would a plan to add gradual support for it like adding CSRs 0x7c1 and 0x7c2 so we can run an Unmatched U-boot SPL against it. If not, is there a definitive U54->U74 set of public differnces around we could use to start from? I'd like to be able to run a full Unmatched image using qemu at some point to add to the current real-board testing we're doing. (I have a basic addition of the type and the two CSRs as a couple of patches if that would help as a start) -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: updates for designware pci-host
On 27/07/2022 13:56, Peter Maydell wrote: On Wed, 27 Jul 2022 at 12:15, Ben Dooks wrote: On Wed, Jul 13, 2022 at 05:54:42PM +0100, Ben Dooks wrote: As part of a project we have been looking at using the DesignWare PCIe host. We found a few issues of missing features or small bugs when using this with a recent Linux kernel (v5.17.x) Whilst doing this we also made a start on some tracing events. Hi, has anyone had a chance to review these. If so can this series get applied? If not should anyone else be added to the review list? If it would be easier I can try and find a git tree to publish this branch on if a pull request would be easier. Is there a public spec for the hardware? There isn't anything listed in the source file in the tree. Without the h/w specs it's pretty difficult to review changes. Would it be helpful if i put out a v2 series with pointers into the Linux tree to where the info is available there?
Re: updates for designware pci-host
On 27/07/2022 13:56, Peter Maydell wrote: On Wed, 27 Jul 2022 at 12:15, Ben Dooks wrote: On Wed, Jul 13, 2022 at 05:54:42PM +0100, Ben Dooks wrote: As part of a project we have been looking at using the DesignWare PCIe host. We found a few issues of missing features or small bugs when using this with a recent Linux kernel (v5.17.x) Whilst doing this we also made a start on some tracing events. Hi, has anyone had a chance to review these. If so can this series get applied? If not should anyone else be added to the review list? If it would be easier I can try and find a git tree to publish this branch on if a pull request would be easier. Is there a public spec for the hardware? There isn't anything listed in the source file in the tree. Without the h/w specs it's pretty difficult to review changes. Some of this I did against the Linux driver from the v5.17 kernel which didn't work until these fixes were added (not sure when the Linux kernel got the ATU size autodetection in, etc). I think everything described in this set was from Linux, but I do have access to the Synopsys v5 PCIe core documentation so have not gone to see if there is anything publicly available. -- Ben
Re: updates for designware pci-host
On Wed, Jul 13, 2022 at 05:54:42PM +0100, Ben Dooks wrote: > As part of a project we have been looking at using the DesignWare > PCIe host. We found a few issues of missing features or small bugs > when using this with a recent Linux kernel (v5.17.x) > > Whilst doing this we also made a start on some tracing events. Hi, has anyone had a chance to review these. If so can this series get applied? If not should anyone else be added to the review list? If it would be easier I can try and find a git tree to publish this branch on if a pull request would be easier. -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH v3 0/8] m25p80: Add SFDP support
On Fri, Jul 22, 2022 at 09:05:39AM +0200, Cédric Le Goater wrote: > On 7/22/22 08:35, Cédric Le Goater wrote: > > Hello, > > > > This is a refresh of a patchset sent long ago [1] adding support for > > [1] was lost while writing the cover : > > https://lore.kernel.org/qemu-devel/20200902093107.608000-1-...@kaod.org/ Is there a git branch this could be pulled from to have a look at and test on our setup? -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH] gpio: designware gpio driver
On 18/07/2022 11:15, Peter Maydell wrote: On Mon, 18 Jul 2022 at 11:05, Ben Dooks wrote: On 13/07/2022 18:20, Ben Dooks wrote: A model for the DesignWare GPIO (v1) block. Is there anyone else who should be reviewing these that was missed off the original list? I'd like to get an idea if there is any work to do. I've got a couple more drivers to submit and was waiting on feedback from this before getting these submitted. My overall feedback is: this isn't a pluggable device (PCI, etc), so what's it intended to be used by? Generally we don't take device models except when there's a board model that's using them. I have a board file, but that's currently under NDA, so we're not allowed to release it at the moment. However we've done a few drivers which we'd like to get out of our development tree which other people might find useful (GPIO, SPI, I2C). -- Ben
Re: [PATCH] gpio: designware gpio driver
On 13/07/2022 18:20, Ben Dooks wrote: A model for the DesignWare GPIO (v1) block. Is there anyone else who should be reviewing these that was missed off the original list? I'd like to get an idea if there is any work to do. I've got a couple more drivers to submit and was waiting on feedback from this before getting these submitted. -- Ben
Re: [PATCH 1/7] pci: designware: add 64-bit viewport limit
On 13/07/2022 17:54, Ben Dooks wrote:
Versions 4 and above add support for 64-bit viewport
limit. Add support for the DESIGNWARE_PCIE_ATU_UPPER_LIMIT
regiser where supported.
Signed-off-by: Ben Dooks
Whoops, just noticed this was my old ct address.
---
hw/pci-host/designware.c | 22 +-
include/hw/pci-host/designware.h | 2 +-
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index bde3a343a2..296f1b9760 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -54,6 +54,7 @@
#define DESIGNWARE_PCIE_ATU_BUS(x) (((x) >> 24) & 0xff)
#define DESIGNWARE_PCIE_ATU_DEVFN(x) (((x) >> 16) & 0xff)
#define DESIGNWARE_PCIE_ATU_UPPER_TARGET 0x91C
+#define DESIGNWARE_PCIE_ATU_UPPER_LIMIT0x924
#define DESIGNWARE_PCIE_IRQ_MSI3
@@ -196,6 +197,10 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
val = viewport->target >> 32;
break;
+case DESIGNWARE_PCIE_ATU_UPPER_LIMIT:
+val = viewport->limit >> 32;
+break;
+
case DESIGNWARE_PCIE_ATU_LIMIT:
val = viewport->limit;
break;
@@ -269,7 +274,7 @@ static void
designware_pcie_update_viewport(DesignwarePCIERoot *root,
{
const uint64_t target = viewport->target;
const uint64_t base = viewport->base;
-const uint64_t size = (uint64_t)viewport->limit - base + 1;
+const uint64_t size = viewport->limit - base + 1;
const bool enabled= viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
MemoryRegion *current, *other;
@@ -363,14 +368,21 @@ static void designware_pcie_root_config_write(PCIDevice
*d, uint32_t address,
viewport->target |= val;
break;
+case DESIGNWARE_PCIE_ATU_UPPER_LIMIT:
+viewport->limit &= 0xUL;
+viewport->limit |= (uint64_t)val << 32;
+break;
+
case DESIGNWARE_PCIE_ATU_LIMIT:
-viewport->limit = val;
+viewport->limit = 0xULL;
+viewport->limit |= val;
break;
case DESIGNWARE_PCIE_ATU_CR1:
viewport->cr[0] = val;
break;
case DESIGNWARE_PCIE_ATU_CR2:
+//printf("CR2: value %08x\n", val);
viewport->cr[1] = val;
designware_pcie_update_viewport(root, viewport);
break;
@@ -429,7 +441,7 @@ static void designware_pcie_root_realize(PCIDevice *dev,
Error **errp)
viewport->inbound = true;
viewport->base= 0xULL;
viewport->target = 0xULL;
-viewport->limit = UINT32_MAX;
+viewport->limit = UINT64_MAX-1;
viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM;
source = >pci.address_space_root;
@@ -453,7 +465,7 @@ static void designware_pcie_root_realize(PCIDevice *dev,
Error **errp)
viewport->inbound = false;
viewport->base= 0xULL;
viewport->target = 0xULL;
-viewport->limit = UINT32_MAX;
+viewport->limit = UINT64_MAX-1;
viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM;
destination = >pci.memory;
@@ -560,7 +572,7 @@ static const VMStateDescription
vmstate_designware_pcie_viewport = {
.fields = (VMStateField[]) {
VMSTATE_UINT64(base, DesignwarePCIEViewport),
VMSTATE_UINT64(target, DesignwarePCIEViewport),
-VMSTATE_UINT32(limit, DesignwarePCIEViewport),
+VMSTATE_UINT64(limit, DesignwarePCIEViewport),
VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2),
VMSTATE_END_OF_LIST()
}
diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index 6d9b51ae67..bd4dd49aec 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -44,7 +44,7 @@ typedef struct DesignwarePCIEViewport {
uint64_t base;
uint64_t target;
-uint32_t limit;
+uint64_t limit;
uint32_t cr[2];
bool inbound;
[PATCH 4/7] pci: designware: ignore new bits in ATU CR1
In version 4 and anver ATU CR1 has more bits in it than just the
viewport type. Make a guess at masking these out to avoid issues
where Linux writes these bits and fails to enable memory ATUs.
Signed-off-by: Ben Dooks
---
hw/pci-host/designware.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 6403416634..947547d153 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -276,10 +276,10 @@ static void
designware_pcie_update_viewport(DesignwarePCIERoot *root,
const uint64_t base = viewport->base;
const uint64_t size = viewport->limit - base + 1;
const bool enabled= viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
-
+uint32_t cr0 = viewport->cr[0];
MemoryRegion *current, *other;
-if (viewport->cr[0] == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
+if ((cr0 & 0xFF) == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
current = >mem;
other = >cfg;
memory_region_set_alias_offset(current, target);
--
2.35.1
[PATCH 5/7] pci: designware: move msi to entry 5
The driver should leave irq[0..3] for INT[A..D] but seems to put the
MSI IRQ at entry 3 which should also be INT_D. Extend the irqs[] array
to 5 entires and put the MSI at entry irqs[4].
Signed-off-by: Ben Dooks
---
hw/pci-host/designware.c | 2 +-
include/hw/pci-host/designware.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 947547d153..b5d5b2b8a5 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -56,7 +56,7 @@
#define DESIGNWARE_PCIE_ATU_UPPER_TARGET 0x91C
#define DESIGNWARE_PCIE_ATU_UPPER_LIMIT0x924
-#define DESIGNWARE_PCIE_IRQ_MSI3
+#define DESIGNWARE_PCIE_IRQ_MSI4
static DesignwarePCIEHost *
designware_pcie_root_to_host(DesignwarePCIERoot *root)
diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index bd4dd49aec..37f90c5000 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -90,7 +90,7 @@ struct DesignwarePCIEHost {
MemoryRegion memory;
MemoryRegion io;
-qemu_irq irqs[4];
+qemu_irq irqs[5];
} pci;
MemoryRegion mmio;
--
2.35.1
[PATCH 1/7] pci: designware: add 64-bit viewport limit
Versions 4 and above add support for 64-bit viewport
limit. Add support for the DESIGNWARE_PCIE_ATU_UPPER_LIMIT
regiser where supported.
Signed-off-by: Ben Dooks
---
hw/pci-host/designware.c | 22 +-
include/hw/pci-host/designware.h | 2 +-
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index bde3a343a2..296f1b9760 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -54,6 +54,7 @@
#define DESIGNWARE_PCIE_ATU_BUS(x) (((x) >> 24) & 0xff)
#define DESIGNWARE_PCIE_ATU_DEVFN(x) (((x) >> 16) & 0xff)
#define DESIGNWARE_PCIE_ATU_UPPER_TARGET 0x91C
+#define DESIGNWARE_PCIE_ATU_UPPER_LIMIT0x924
#define DESIGNWARE_PCIE_IRQ_MSI3
@@ -196,6 +197,10 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t
address, int len)
val = viewport->target >> 32;
break;
+case DESIGNWARE_PCIE_ATU_UPPER_LIMIT:
+val = viewport->limit >> 32;
+break;
+
case DESIGNWARE_PCIE_ATU_LIMIT:
val = viewport->limit;
break;
@@ -269,7 +274,7 @@ static void
designware_pcie_update_viewport(DesignwarePCIERoot *root,
{
const uint64_t target = viewport->target;
const uint64_t base = viewport->base;
-const uint64_t size = (uint64_t)viewport->limit - base + 1;
+const uint64_t size = viewport->limit - base + 1;
const bool enabled= viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
MemoryRegion *current, *other;
@@ -363,14 +368,21 @@ static void designware_pcie_root_config_write(PCIDevice
*d, uint32_t address,
viewport->target |= val;
break;
+case DESIGNWARE_PCIE_ATU_UPPER_LIMIT:
+viewport->limit &= 0xUL;
+viewport->limit |= (uint64_t)val << 32;
+break;
+
case DESIGNWARE_PCIE_ATU_LIMIT:
-viewport->limit = val;
+viewport->limit = 0xULL;
+viewport->limit |= val;
break;
case DESIGNWARE_PCIE_ATU_CR1:
viewport->cr[0] = val;
break;
case DESIGNWARE_PCIE_ATU_CR2:
+//printf("CR2: value %08x\n", val);
viewport->cr[1] = val;
designware_pcie_update_viewport(root, viewport);
break;
@@ -429,7 +441,7 @@ static void designware_pcie_root_realize(PCIDevice *dev,
Error **errp)
viewport->inbound = true;
viewport->base= 0xULL;
viewport->target = 0xULL;
-viewport->limit = UINT32_MAX;
+viewport->limit = UINT64_MAX-1;
viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM;
source = >pci.address_space_root;
@@ -453,7 +465,7 @@ static void designware_pcie_root_realize(PCIDevice *dev,
Error **errp)
viewport->inbound = false;
viewport->base= 0xULL;
viewport->target = 0xULL;
-viewport->limit = UINT32_MAX;
+viewport->limit = UINT64_MAX-1;
viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM;
destination = >pci.memory;
@@ -560,7 +572,7 @@ static const VMStateDescription
vmstate_designware_pcie_viewport = {
.fields = (VMStateField[]) {
VMSTATE_UINT64(base, DesignwarePCIEViewport),
VMSTATE_UINT64(target, DesignwarePCIEViewport),
-VMSTATE_UINT32(limit, DesignwarePCIEViewport),
+VMSTATE_UINT64(limit, DesignwarePCIEViewport),
VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2),
VMSTATE_END_OF_LIST()
}
diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index 6d9b51ae67..bd4dd49aec 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -44,7 +44,7 @@ typedef struct DesignwarePCIEViewport {
uint64_t base;
uint64_t target;
-uint32_t limit;
+uint64_t limit;
uint32_t cr[2];
bool inbound;
--
2.35.1
[PATCH 2/7] pci: designware: fix DESIGNWARE_PCIE_ATU_UPPER_TARGET
By inspection DESIGNWARE_PCIE_ATU_UPPER_TARGET should be writing to the upper 32-bits of viewport->target, so fix this by shifting the 32-bit value before the or. Signed-off-by: Ben Dooks --- hw/pci-host/designware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index 296f1b9760..d213d7324c 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -365,7 +365,7 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, case DESIGNWARE_PCIE_ATU_UPPER_TARGET: viewport->target &= 0xULL; -viewport->target |= val; +viewport->target |= (uint64_t)val << 32; break; case DESIGNWARE_PCIE_ATU_UPPER_LIMIT: -- 2.35.1
[PATCH 3/7] pci: designware: clamp viewport index
The current Linux driver for this assumes it can write the 255 into this register and then read back the value to work out how many viewports are supported. Clamp the value so that the probe works and does not cause memory corruption as the value is not well clamped elsewhere in the driver. Signed-off-by: Ben Dooks --- hw/pci-host/designware.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index d213d7324c..6403416634 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -345,6 +345,10 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, break; case DESIGNWARE_PCIE_ATU_VIEWPORT: +/* clamp this value, linux uses it to calculate the + * available number of viewports */ +if (val >= DESIGNWARE_PCIE_NUM_VIEWPORTS) +val = DESIGNWARE_PCIE_NUM_VIEWPORTS-1; root->atu_viewport = val; break; -- 2.35.1
updates for designware pci-host
As part of a project we have been looking at using the DesignWare PCIe host. We found a few issues of missing features or small bugs when using this with a recent Linux kernel (v5.17.x) Whilst doing this we also made a start on some tracing events.
[PATCH 6/7] pci: designware: correct host's class_id
This is a host to pcie bridge, so use PCI_CLASS_BRIDGE_HOST for the class. Signed-off-by: Ben Dooks --- hw/pci-host/designware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index b5d5b2b8a5..a47ae48071 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -615,7 +615,7 @@ static void designware_pcie_root_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_SYNOPSYS; k->device_id = 0xABCD; k->revision = 0; -k->class_id = PCI_CLASS_BRIDGE_PCI; +k->class_id = PCI_CLASS_BRIDGE_HOST; k->is_bridge = true; k->exit = pci_bridge_exitfn; k->realize = designware_pcie_root_realize; -- 2.35.1
[PATCH 7/7] pci: designware: add initial tracing events
Add a couple of tracing events for internal driver updates
Signed-off-by: Ben Dooks
---
hw/pci-host/designware.c | 4
hw/pci-host/trace-events | 4
2 files changed, 8 insertions(+)
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index a47ae48071..489959513f 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -30,6 +30,7 @@
#include "migration/vmstate.h"
#include "hw/irq.h"
#include "hw/pci-host/designware.h"
+#include "trace.h"
#define DESIGNWARE_PCIE_PORT_LINK_CONTROL 0x710
#define DESIGNWARE_PCIE_PHY_DEBUG_R1 0x72C
@@ -112,6 +113,7 @@ static void
designware_pcie_root_update_msi_mapping(DesignwarePCIERoot *root)
const uint64_t base = root->msi.base;
const bool enable = root->msi.intr[0].enable;
+trace_dw_pcie_msi_update(base, enable);
memory_region_set_address(mem, base);
memory_region_set_enabled(mem, enable);
}
@@ -279,6 +281,8 @@ static void
designware_pcie_update_viewport(DesignwarePCIERoot *root,
uint32_t cr0 = viewport->cr[0];
MemoryRegion *current, *other;
+trace_dw_pcie_viewport_update(target, base, size, cr0, enabled);
+
if ((cr0 & 0xFF) == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
current = >mem;
other = >cfg;
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index 437e66ff50..6b064d3c74 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -3,6 +3,10 @@
# bonito.c
bonito_spciconf_small_access(uint64_t addr, unsigned size) "PCI config address
is smaller then 32-bit, addr: 0x%"PRIx64", size: %u"
+# designware.c
+dw_pcie_msi_update(uint64_t base, int enable) "base 0x%" PRIx64 " enable %d"
+dw_pcie_viewport_update(uint64_t target, uint64_t base, uint64_t limit,
uint32_t cr0, int enabled) "target 0x%" PRIx64 " base 0x%" PRIx64 " limit 0x%"
PRIx64 " cr0 0x%" PRIx32 " enabled %d"
+
# grackle.c
grackle_set_irq(int irq_num, int level) "set_irq num %d level %d"
--
2.35.1
[PATCH] gpio: designware gpio driver
A model for the DesignWare GPIO (v1) block.
Signed-off-by: Ben Dooks
---
hw/gpio/Kconfig | 3 +
hw/gpio/designware_gpio.c | 327 ++
hw/gpio/meson.build | 1 +
hw/gpio/trace-events | 7 +
include/hw/gpio/designware_gpio.h | 89
5 files changed, 427 insertions(+)
create mode 100644 hw/gpio/designware_gpio.c
create mode 100644 include/hw/gpio/designware_gpio.h
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index f0e7405f6e..e5883d9763 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -13,3 +13,6 @@ config GPIO_PWR
config SIFIVE_GPIO
bool
+
+config DESIGNWARE_GPIO
+bool
\ No newline at end of file
diff --git a/hw/gpio/designware_gpio.c b/hw/gpio/designware_gpio.c
new file mode 100644
index 00..417bccd630
--- /dev/null
+++ b/hw/gpio/designware_gpio.c
@@ -0,0 +1,327 @@
+/*
+ * Synopsys Desgignware general purpose input/output register definition
+ *
+ * Based on sifive_gpio.c and imx_gpio.c
+ *
+ * Copyright 2022 Sifive, Inc.
+ * Copyright 2018 Steffen Görtz
+ *
+ * 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 "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/gpio/designware_gpio.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+/* only bank A can provide interrupts */
+static void update_output_irqs(DESIGNWAREGPIOState *s)
+{
+struct DESIGNWAREGPIOBank *bank = >bank[0];
+uint32_t level_irqs, edge_irqs = 0;
+
+/* re-calculate interrupts for raw_int_status */
+level_irqs = bank->dr_val ^ s->int_polarity;
+level_irqs &= ~s->int_level;
+
+edge_irqs = bank->dr_val ^ bank->last_dr_val;
+edge_irqs &= s->int_level;
+bank->last_dr_val = bank->dr_val;
+
+/* update irq from raw-status and the mask */
+s->int_status_raw = level_irqs | edge_irqs;
+s->int_status = s->int_status_raw & s->int_mask;
+
+qemu_set_irq(s->irq, s->int_status ? 1 : 0);
+trace_designware_gpio_update_output_irq(s->int_status);
+}
+
+static void update_state(DESIGNWAREGPIOState *s)
+{
+struct DESIGNWAREGPIOBank *bank;
+int banknr, basenr, nr;
+
+for (banknr = 0; banknr < DESIGNWARE_GPIO_BANKS; banknr++) {
+basenr = banknr * DESIGNWARE_GPIO_NR_PER_BANK;
+bank = >bank[banknr];
+
+/* check for data-direction differences */
+if (bank->ddr & bank->in) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "GPIO bank %d: pins shorted, DDR=%x, IN=%x,
overlap=%x\n",
+ banknr, bank->ddr, bank->in, bank->ddr & bank->in);
+}
+
+bank->dr_val = (bank->dr & bank->ddr) | (bank->in & ~bank->ddr);
+
+/* update any outputs marked as outputs */
+for (nr = 0; nr < DESIGNWARE_GPIO_NR_PER_BANK; nr++) {
+if (!extract32(bank->ddr, nr, 1))
+continue;
+qemu_set_irq(s->output[basenr+nr], extract32(bank->dr_val, nr, 1));
+}
+}
+
+update_output_irqs(s);
+}
+
+
+static uint64_t designware_gpio_read(void *opaque, hwaddr offset, unsigned int
size)
+{
+struct DESIGNWAREGPIOState *s = DESIGNWARE_GPIO(opaque);
+struct DESIGNWAREGPIOBank *bank;
+hwaddr banknr, reg;
+uint64_t r = 0;
+bool handled = true;
+
+if (offset < (REG_SWPORTD_DDR + 4)) {
+banknr = offset / REG_SWPORT_DR_STRIDE;
+reg = offset % REG_SWPORT_DR_STRIDE;
+bank = >bank[banknr];
+
+switch (reg) {
+case REG_SWPORTA_DR:
+r = bank->dr;
+break;
+case REG_SWPORTA_DDR:
+r = bank->ddr;
+break;
+default:
+handled = false;
+}
+} else {
+switch (offset) {
+case REG_INTEN:
+r= s->int_en;
+break;
+case REG_INTMASK:
+r = s->int_mask;
+break;
+case REG_INTTYPE_LEVEL:
+r = s->int_level;
+break;
+case REG_INT_POLARITY:
+r = s->int_polarity;
+break;
+case REG_INTSTATUS:
+r = s->int_status;
+break;
+case REG_INTSTATUS_RAW:
+r = s->int_status_raw;
+break;
+case REG_PORTA_DEBOUNCE:
+r = s->porta_debounce;
+break;
+case REG_PORTA_EOI:
+r = 0x0;/* write only */
+break;
+case REG_EXT_PORTA:
+r = s->bank[0].dr_val;
+break;
+case REG_EXT_PORTB:
+r = s->bank[1].dr_val;
+break;
+case REG_EXT_PORTC:
+
Re: [PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
On Mon, Jun 20, 2022 at 04:47:44PM +1000, Alistair Francis wrote: > On Sun, Jun 19, 2022 at 6:14 AM Ben Dooks wrote: > > > > Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify > > the code. > > > > Signed-off-by: Ben Dooks > > Do you mind updating the other machines as well? I can do, I was wondering if anyone had a good guide how to use cocci to do it automatiically as I have been finding it really difficult to work out how to automate API changes. -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
[PATCH 3/4] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
--
v2:
- fix node/path in comment
---
include/sysemu/device_tree.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 79ce009a22..28b68bacaf 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,21 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+do {\
+static const char * const __strs[] = { __VA_ARGS__ }; \
+qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+} while(0)
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
[v2] pair of device-tree helpers
I've been doing a bit of looking at riscv and dt creation, and was thinking the following two helper functions would be useful so implemented qemu_fdt_setprop_reg64_map() and qemu_fdt_setprop_strings() and then applied them to the hw/riscv/sifive_u.c machine.
[PATCH 1/4] device_tree: add qemu_fdt_setprop_reg64_map helper
Add a macro qemu_fdt_setprop_reg64_map() to set the given node's reg property directly from the memory map entry to avoid open coding of the following: qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x0, memmap[SIFIVE_U_DEV_OTP].base, 0x0, memmap[SIFIVE_U_DEV_OTP].size); Signed-off-by: Ben Dooks -- v2: - changed qemu_fdt_setprop_reg64 to qemu_fdt_setprop_reg64_map --- include/sysemu/device_tree.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index ef060a9759..79ce009a22 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -135,6 +135,21 @@ int qemu_fdt_add_path(void *fdt, const char *path); sizeof(qdt_tmp));\ } while (0) +/** + * qemu_fdt_setprop_reg64_map: + * @fdt: the device tree path + * @node_path: node to set property on + * @map: the map entry to set the reg from + * + * A helper tp set the 'reg' node on the specified node from the given map + * entry. + */ +#define qemu_fdt_setprop_reg64_map(fdt, path, map) \ +qemu_fdt_setprop_cells(fdt, path, "reg",\ + (map)->base >> 32, (map)->base, \ + (map)->size >> 32, (map)->size) + + void qemu_fdt_dumpdtb(void *fdt, int size); /** -- 2.35.1
[PATCH 2/4] hw/riscv: use qemu_fdt_setprop_reg64_map() in sifive_u.c
Use the qemu_fdt_setprop_reg64_map() to replace the code
that sets the property manually.
Signed-off-by: Ben Dooks
--
v2:
- changed to qemu_fdt_setprop_reg64_map() from previous
---
hw/riscv/sifive_u.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..89d7aa2a52 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -223,9 +223,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_CLINT].base,
-0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
g_free(cells);
@@ -235,9 +233,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_OTP].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "fuse-count", SIFIVE_U_OTP_REG_SIZE);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_OTP].base,
-0x0, memmap[SIFIVE_U_DEV_OTP].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_OTP]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-otp");
g_free(nodename);
@@ -250,9 +246,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x1);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
hfclk_phandle, rtcclk_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PRCI].base,
-0x0, memmap[SIFIVE_U_DEV_PRCI].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_PRCI]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-prci");
g_free(nodename);
@@ -284,9 +278,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PLIC].base,
-0x0, memmap[SIFIVE_U_DEV_PLIC].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_PLIC]);
qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
qemu_fdt_setprop_cell(fdt, nodename, "phandle", plic_phandle);
plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
@@ -304,9 +296,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop_cell(fdt, nodename, "#gpio-cells", 2);
qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_GPIO].base,
-0x0, memmap[SIFIVE_U_DEV_GPIO].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_GPIO]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts", SIFIVE_U_GPIO_IRQ0,
SIFIVE_U_GPIO_IRQ1, SIFIVE_U_GPIO_IRQ2, SIFIVE_U_GPIO_IRQ3,
SIFIVE_U_GPIO_IRQ4, SIFIVE_U_GPIO_IRQ5, SIFIVE_U_GPIO_IRQ6,
@@ -342,9 +332,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/cache-controller@%lx",
(long)memmap[SIFIVE_U_DEV_L2CC].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_L2CC].base,
-0x0, memmap[SIFIVE_U_DEV_L2CC].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_L2CC]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts",
SIFIVE_U_L2CC_IRQ0, SIFIVE_U_L2CC_IRQ1, SIFIVE_U_L2CC_IRQ2);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
@@ -366,9 +354,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
prci_phandle, PRCI_CLK_TLCLK);
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI2_IRQ);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_QSPI2].base,
-0x0, memmap[SIFIVE_U_DEV_QSPI2].size);
+qemu_fdt_setprop_reg64
[PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify
the code.
Signed-off-by: Ben Dooks
---
hw/riscv/sifive_u.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 89d7aa2a52..16b18d90bd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,8 +214,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
@@ -273,8 +266,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -410,8 +403,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
--
2.35.1
Re: [PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
On Fri, Apr 22, 2022 at 10:19:34AM +0800, Bin Meng wrote: > On Mon, Apr 18, 2022 at 5:13 AM Ben Dooks wrote: > > > > Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify > > the code. > > > > Signed-off-by; Ben Dooks > > ; should be replaced to : > > Not sure how you did that, but you can do with "git commit -s" and git > will take care of the SoB tag. I'm used to adding them manually to git commit messages. -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
[PATCH v2] MAINTAINERS: change Ben Widawsky's email address
ben.widaw...@intel.com will stop working on 2022-06-20, change it to my personal email address. Update .mailmap to handle previously authored commits. Acked-by: Jonathan Cameron Signed-off-by: Ben Widawsky --- v2: Fix typo in commit message change author to b...@bwidawsk.net from @intel.com Swap mailmap direction (Jonathan) --- .mailmap| 1 + MAINTAINERS | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 8c326709cfab..e92e268b9230 100644 --- a/.mailmap +++ b/.mailmap @@ -54,6 +54,7 @@ Aleksandar Rikalo Aleksandar Rikalo Alexander Graf Anthony Liguori Anthony Liguori +Ben Widawsky Christian Borntraeger Filip Bozuta Frederic Konrad diff --git a/MAINTAINERS b/MAINTAINERS index 5580a36b68e1..89da5755116b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2574,7 +2574,7 @@ F: qapi/transaction.json T: git https://repo.or.cz/qemu/armbru.git block-next Compute Express Link -M: Ben Widawsky +M: Ben Widawsky M: Jonathan Cameron S: Supported F: hw/cxl/ base-commit: 9b1f58854959c5a9bdb347e3e04c252ab7fc9ef5 -- 2.36.1
Re: [PATCH] MAINTAINERS: change Ben Widawsky's email address
On 22-06-07 17:50:35, Jonathan Cameron wrote: > On Tue, 7 Jun 2022 09:26:28 -0700 > Ben Widawsky wrote: > > > ben@widaw...@intel.com will stop working on 2022-06-20, change it to my > > personal email address. > > > > Update .mailmap to handle previously authored commits. > > > > Signed-off-by: Ben Widawsky > > With below question addressed, > Acked-by: Jonathan Cameron > > Probably cc Michael Tsirkin as well given he picked up the > patch that introduced this maintainers entry recently. Okay. Luckily I had a typo in the commit message anyway, so it needed a respin. > > Thanks, > > Jonathan > > > > --- > > .mailmap| 1 + > > MAINTAINERS | 2 +- > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/.mailmap b/.mailmap > > index 8c326709cfab..0dec7b156999 100644 > > --- a/.mailmap > > +++ b/.mailmap > > @@ -54,6 +54,7 @@ Aleksandar Rikalo > > > > Aleksandar Rikalo > > > > Alexander Graf > > Anthony Liguori Anthony Liguori > > > > +Ben Widawsky > > Is this backwards as you will (I think) want scripts to output > b...@bwidawsk.net > as your canonical email address going forwards? I guess? I simply read the comment to determine order. I should have gone with what I knew rather than tried to figure out what was meant. "# Next, replace old addresses by a more recent one." > > > Christian Borntraeger > > Filip Bozuta > > Frederic Konrad > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 5580a36b68e1..89da5755116b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2574,7 +2574,7 @@ F: qapi/transaction.json > > T: git https://repo.or.cz/qemu/armbru.git block-next > > > > Compute Express Link > > -M: Ben Widawsky > > +M: Ben Widawsky > > M: Jonathan Cameron > > S: Supported > > F: hw/cxl/ > > > > base-commit: 9b1f58854959c5a9bdb347e3e04c252ab7fc9ef5 >
[PATCH] MAINTAINERS: change Ben Widawsky's email address
ben@widaw...@intel.com will stop working on 2022-06-20, change it to my personal email address. Update .mailmap to handle previously authored commits. Signed-off-by: Ben Widawsky --- .mailmap| 1 + MAINTAINERS | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 8c326709cfab..0dec7b156999 100644 --- a/.mailmap +++ b/.mailmap @@ -54,6 +54,7 @@ Aleksandar Rikalo Aleksandar Rikalo Alexander Graf Anthony Liguori Anthony Liguori +Ben Widawsky Christian Borntraeger Filip Bozuta Frederic Konrad diff --git a/MAINTAINERS b/MAINTAINERS index 5580a36b68e1..89da5755116b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2574,7 +2574,7 @@ F: qapi/transaction.json T: git https://repo.or.cz/qemu/armbru.git block-next Compute Express Link -M: Ben Widawsky +M: Ben Widawsky M: Jonathan Cameron S: Supported F: hw/cxl/ base-commit: 9b1f58854959c5a9bdb347e3e04c252ab7fc9ef5 -- 2.36.1
Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
On 22-06-07 17:37:02, Jonathan Cameron wrote:
> On Tue, 7 Jun 2022 09:19:28 -0700
> Ben Widawsky wrote:
>
> > On 22-06-07 17:07:47, Jonathan Cameron wrote:
> > > Without being able to write these registers, no interleaving is possible.
> > > More refined checks of HDM register state on commit to follow.
> > >
> > > Signed-off-by: Jonathan Cameron
> > > ---
> > > v3: Actually pass the parameter to the call...
> > > v2: (Ben Widawsky)
> > > - Correctly set a tighter write mask for the endpoint devices where this
> > > register has a different use.
> > >
> > > hw/cxl/cxl-component-utils.c | 11 +--
> > > 1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > > index 7985c9bfca..2208284ee6 100644
> > > --- a/hw/cxl/cxl-component-utils.c
> > > +++ b/hw/cxl/cxl-component-utils.c
> > > @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state,
> > > uint32_t *write_msk)
> > > reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
> > > }
> > >
> > > -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> > > +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> > > +enum reg_type type)
> > > {
> > > int decoder_count = 1;
> > > int i;
> > > @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state,
> > > uint32_t *write_msk)
> > > write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf000;
> > > write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0x;
> > > write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> > > +if (type == CXL2_DEVICE) {
> > > +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] =
> > > 0xf000;
> > > +} else {
> > > +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] =
> > > 0x;
> > > +}
> > > +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] =
> > > 0x;
> >
> > Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?
>
> Good point, but also for consistency I think we need
> type == CXL2_LOGICAL_DEVICE as well.
I was looking at this and I am not sure, but I defer to you.
>
> We will only exercise the match to CXL2_TYPE3_DEVICE currently
> as we don't have any emulation for MLDs (and hence LD) or type 1/2 devices
> (CXL2_DEVICE).
>
> I'll send a v4 out tomorrow.
>
Sounds good, feel free to keep the r-b tag.
> >
> > Otherwise,
> > Reviewed-by: Ben Widawsky
> >
> > > }
> > > }
> > >
> > > @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t
> > > *reg_state, uint32_t *write_msk
> > > }
> > >
> > > init_cap_reg(HDM, 5, 1);
> > > -hdm_init_common(reg_state, write_msk);
> > > +hdm_init_common(reg_state, write_msk, type);
> > >
> > > if (caps < 5) {
> > > return;
> > > --
> > > 2.32.0
> > >
>
Re: [PATCH v3] hw/cxl: Fix missing write mask for HDM decoder target list registers
On 22-06-07 17:07:47, Jonathan Cameron wrote:
> Without being able to write these registers, no interleaving is possible.
> More refined checks of HDM register state on commit to follow.
>
> Signed-off-by: Jonathan Cameron
> ---
> v3: Actually pass the parameter to the call...
> v2: (Ben Widawsky)
> - Correctly set a tighter write mask for the endpoint devices where this
> register has a different use.
>
> hw/cxl/cxl-component-utils.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index 7985c9bfca..2208284ee6 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c
> @@ -154,7 +154,8 @@ static void ras_init_common(uint32_t *reg_state, uint32_t
> *write_msk)
> reg_state[R_CXL_RAS_ERR_CAP_CTRL] = 0x00;
> }
>
> -static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk)
> +static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> +enum reg_type type)
> {
> int decoder_count = 1;
> int i;
> @@ -174,6 +175,12 @@ static void hdm_init_common(uint32_t *reg_state,
> uint32_t *write_msk)
> write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf000;
> write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0x;
> write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff;
> +if (type == CXL2_DEVICE) {
> +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] =
> 0xf000;
> +} else {
> +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] =
> 0x;
> +}
> +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0x;
Should it be (type == CXL2_DEVICE || type == CXL2_TYPE3_DEVICE) ?
Otherwise,
Reviewed-by: Ben Widawsky
> }
> }
>
> @@ -239,7 +246,7 @@ void cxl_component_register_init_common(uint32_t
> *reg_state, uint32_t *write_msk
> }
>
> init_cap_reg(HDM, 5, 1);
> -hdm_init_common(reg_state, write_msk);
> +hdm_init_common(reg_state, write_msk, type);
>
> if (caps < 5) {
> return;
> --
> 2.32.0
>
Re: [PATCH] hw/cxl: Fix missing write mask for HDM decoder target list registers
On 22-05-31 13:39:53, Jonathan Cameron wrote: > Without being able to write these registers, no interleaving is possible. > More refined checks of HDM register state on commit to follow. > > Signed-off-by: Jonathan Cameron > --- > hw/cxl/cxl-component-utils.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c > index 7985c9bfca..993248b5c0 100644 > --- a/hw/cxl/cxl-component-utils.c > +++ b/hw/cxl/cxl-component-utils.c > @@ -174,6 +174,8 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t > *write_msk) > write_msk[R_CXL_HDM_DECODER0_SIZE_LO + i * 0x20] = 0xf000; > write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * 0x20] = 0x; > write_msk[R_CXL_HDM_DECODER0_CTRL + i * 0x20] = 0x13ff; > +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * 0x20] = 0x; > +write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_HI + i * 0x20] = 0x; I wonder if this should be 0. It will be weird for endpoints to have a skip value of 0xff. > } > } > > -- > 2.32.0 >
Re: [PATCH v2 0/8] hw/cxl: Move CXL emulation options and state to machines.
On 22-06-01 17:42:27, Jonathan Cameron wrote: > Changes since v1 (thanks to Paolo Bonzini) > * Update 'description' of cxl-fmw as suggested to mention it's an array. > * Add a wrapper cxl_hook_up_pxb_registers() to cxl-host.c as it'll be common > for all machines using CXL with PXB. > > Run through the CI at: > https://gitlab.com/jic23/qemu/-/pipelines/553257456 > > V1 Cover letter: > > Currently only machine with CXL support upstream is i386/pc but arm/virt > patches have been posted and once this is merged an updated series will > follow. Switch support is queued behind this as well because they both > include documentation updates. > > Paolo Bonzini highlighted a couple of issues with the current CXL > emulation code. > > * Top level parameter rather than machine for fixed memory windows > > The --cxl-fixed-memory-window top level command line parameters won't play > well with efforts to make it possible to instantiate entire machines via > RPC. Better to move these to be machine configuration. This change is > relatively straight forward, but does result in very long command lines > (cannot break fixed window setup into multiple -M entries). > > * Move all CXL stuff to machine specific code and helpers > > To simplify the various interactions between machine setup and host > bridges etc, currently various CXL steps are called from the generic > core/machine.c and softmmu/vl.c + there are CXL elements in MachineState. > > Much of this is straight forward to do with one exception: > The CXL pci_expander_bridge host bridges require MMIO register space. > This series does this by walking the bus and filling the register space > in via the machine_done callback. This is similar to the walk done for > identifying host bridges in the ACPI building code but it is rather ugly > and postpones rejection of PXB_CXL instances where cxl=off (default). > > All comments welcome, but the first patch at least changes the command-line > so to avoid have to add backwards compatibility code, it would be great > to merge that before 7.1 is released. > LGTM overall. I'm not thrilled with introducing another [sub]scronym "fmw", but otherwise, no complaints. Series is: Reviewed-by: Ben Widawsky > Thanks, > > Jonathan > > Jonathan Cameron (8): > hw/cxl: Make the CXL fixed memory window setup a machine parameter. > hw/acpi/cxl: Pass in the CXLState directly rather than MachineState > hw/cxl: Push linking of CXL targets into i386/pc rather than in > machine.c > tests/acpi: Allow modification of q35 CXL CEDT table. > pci/pci_expander_bridge: For CXL HB delay the HB register memory > region setup. > tests/acpi: Update q35/CEDT.cxl for new memory addresses. > hw/cxl: Move the CXLState from MachineState to machine type specific > state. > hw/machine: Drop cxl_supported flag as no longer useful > > docs/system/devices/cxl.rst | 4 +- > hw/acpi/cxl.c | 9 +- > hw/core/machine.c | 28 -- > hw/cxl/cxl-host-stubs.c | 9 +- > hw/cxl/cxl-host.c | 100 ++-- > hw/i386/acpi-build.c| 8 +- > hw/i386/pc.c| 31 +++--- > hw/pci-bridge/meson.build | 5 +- > hw/pci-bridge/pci_expander_bridge.c | 32 --- > hw/pci-bridge/pci_expander_bridge_stubs.c | 14 +++ > include/hw/acpi/cxl.h | 5 +- > include/hw/boards.h | 3 +- > include/hw/cxl/cxl.h| 9 +- > include/hw/cxl/cxl_host.h | 23 + > include/hw/i386/pc.h| 2 + > include/hw/pci-bridge/pci_expander_bridge.h | 12 +++ > qapi/machine.json | 13 +++ > softmmu/vl.c| 46 - > tests/data/acpi/q35/CEDT.cxl| Bin 184 -> 184 bytes > tests/qtest/bios-tables-test.c | 4 +- > tests/qtest/cxl-test.c | 4 +- > 21 files changed, 222 insertions(+), 139 deletions(-) > create mode 100644 hw/pci-bridge/pci_expander_bridge_stubs.c > create mode 100644 include/hw/cxl/cxl_host.h > create mode 100644 include/hw/pci-bridge/pci_expander_bridge.h > > -- > 2.32.0 >
Re: [PATCH 1/8] hw/cxl: Make the CXL fixed memory window setup a machine parameter.
On 22-05-31 09:26:27, Paolo Bonzini wrote: > On 5/30/22 15:45, Jonathan Cameron via wrote: > > +object_property_add(obj, "cxl-fmw", "CXLFixedMemoryWindow", > > +machine_get_cfmw, machine_set_cfmw, > > +NULL, state); > > +object_property_set_description(obj, "cxl-fmw", > > +"CXL Fixed Memory Window"); > > Perhaps "CML fixed memory windows (array)" or something like that? > > Paolo I had a mail which I apparently never sent. I'd like to see 'fmw' renamed, since that has no decoder ring in any spec that I'm aware of. Why not keep cfmws nomenclature? It's well defined. Ben
[PATCH] Updating gdbstub to allow safe multithreading in usermode emulation
I was testing some multi-threaded code in qemu's usermode and ran into
issues with the gdbstub because the user mode qemu emulation spawns new
threads when the process tries to make a new thread but the gdbstub does
not handle the threads well. The current gdbstub has a single global
struct which contains all the state data, and multiple threads can write
to this struct simultaneously, causing gdb packets to be corrupted. The
different threads can also try to read off the gdb socket at the same
time causing the packet to be devided between threads. This patch is
designed to add a single separate thread for the usermode gdbstub which
will handle all the gdb comms and avoid the multithreading issues.
To demonstrate that the mutlithreading was not working properly before
and that it hopefully works properlly now, I wrote a small test program
with some gdb scripts that can be found here:
http://url6163.thefarbeyond.com/ls/click?upn=dUoerjbXT3NK9PiRMHEMD5Fm1RmJf0eUWTDkIDLODGZSRXu94ynuiGwQ-2FCFcimw5rndUfJbozecGKoOE5zbdfRVgadbVSeCrgP5IlB2UqPU-3DUBT-_E8SO-2FEypfU855L0ybQoiQY4Xaj8Z6NYzBoBK89OH-2BiIJOE8-2BoeErelzsKKZyRONZN5M7Gzw0Zs4K0tnG4gxJSOWW89OLEjRW7ISHPWO2lT6fJJUM88-2BLL6sh4BfexcL-2FKt7KhnEpzqyb9bd5UZ-2FR2iPVIjp7zfshwPtjJEHAHaIGeNZbI4nFw81hhs0N1tt9sAcC1ALVazbgnC5E-2F5ZChA-3D-3D
Signed-off-by: Ben Cohen
---
gdbstub.c | 105 +
include/exec/gdbstub.h | 13 +
linux-user/signal.c| 4 ++
3 files changed, 122 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index a3ff8702ce..11a76da575 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -29,6 +29,7 @@
#include "qemu/ctype.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
+#include "qemu/thread.h"
#include "trace/trace-root.h"
#include "exec/gdbstub.h"
#ifdef CONFIG_USER_ONLY
@@ -370,8 +371,103 @@ typedef struct GDBState {
int sstep_flags;
int supported_sstep_flags;
} GDBState;
+typedef struct GDBSignal {
+CPUState *cpu;
+int sig;
+} GDBSignal;
static GDBState gdbserver_state;
+#ifdef CONFIG_USER_ONLY
+static GDBSignal *waiting_signal;
+static QemuMutex signal_wait_mutex;
+static QemuMutex signal_done_mutex;
+static QemuMutex another_thread_is_stepping;
+static int signal_result;
+#endif
+
+static void *gdb_signal_handler_loop(void* args)
+{
+while (TRUE) {
+if (NULL != waiting_signal) {
+signal_result = gdb_handlesig(waiting_signal->cpu,
+waiting_signal->sig);
+waiting_signal = NULL;
+qemu_mutex_unlock(_done_mutex);
+}
+}
+/* Should never return */
+return NULL;
+}
+
+int gdb_thread_handle_signal(CPUState *cpu, int sig)
+{
+GDBSignal signal = {
+.cpu = cpu,
+.sig = sig
+};
+if (!cpu->singlestep_enabled) {
+/*
+ * This mutex is locked by all threads that are not stepping to allow
+ * only the thread that is currently stepping to handle signals until
+ * it finished stepping. Without this, pending signals that are queued
+ * up while the stepping thread handles its signal will race with the
+ * stepping thread to get the next signal handled. This is bad, because
+ * the gdb client expects the stepping thread to be the first response
+ * back to the step command that was sent.
+ */
+qemu_mutex_lock(_thread_is_stepping);
+}
+/*
+ * This mutex is locked to allow only one thread at a time to be
+ * handling a signal. This is necessary, because otherwise multiple
+ * threads will try to talk to the gdb client simultaneously and can
+ * race each other sending and receiving packets, potentially going
+ * out of order or simulatenously reading off of the same socket.
+ */
+qemu_mutex_lock(_wait_mutex);
+{
+/*
+ * This mutex is first locked here to ensure that it is in a locked
+ * state before the gdb_signal_handler_loop handles the next signal
+ * and unlocks it.
+ */
+qemu_mutex_lock(_done_mutex);
+waiting_signal =
+/*
+ * The thread locks this mutex again to wait until the
+ * gdb_signal_handler_loop is finished handling the signal and has
+ * unlocked the mutex.
+ */
+qemu_mutex_lock(_done_mutex);
+/*
+ * Finally, unlock this mutex in preparation for the next call to
+ * this function
+ */
+qemu_mutex_unlock(_done_mutex);
+sig = signal_result;
+if (!cpu->singlestep_enabled) {
+/*
+ * If this thread is not stepping and is handling its signal
+ * then it can always safely unlock this mutex.
+ */
+qemu_mutex_unlock(_thread_is_stepping);
+} else {
+/*
+ * If this thread was already stepping it will already be holding
+
Re: [RFC PATCH 2/2] arm/virt: Add aspeed-i2c controller and MCTP EP to enable MCTP testing
On 22-05-20 18:01:28, Jonathan Cameron wrote:
> As the only I2C emulation in QEMU that supports being both
> a master and a slave, suitable for MCTP over i2c is aspeed-i2c
> add this controller to the arm virt model and hook up our new
> i2c_mctp_cxl_fmapi device.
>
> The current Linux driver for aspeed-i2c has a hard requirement on
> a reset controller. Throw down the simplest reset controller
> I could find so as to avoid need to make any chance to the kernel
> code.
s/chance/change
>
> Patch also builds appropriate device tree. Unfortunately for CXL
> we need to use ACPI (no DT bindings yet defined). Enabling this will
> either require appropriate support for MCTP on an i2c master that
> has ACPI bindings, or modifications of the kernel driver to support
> ACPI with aspeed-i2c (which might be a little controversial ;)
I'm naive to what DT defines, but I assume what's there already is insufficient
to make the bindings for CXL. I say this because I believe it wouldn't be too
bad at all to make a cxl_dt.ko, and it's certainly less artificial than
providing ACPI support for things which don't naturally have ACPI support.
>
> Signed-off-by: Jonathan Cameron
> ---
> hw/arm/Kconfig| 1 +
> hw/arm/virt.c | 77 +++
> include/hw/arm/virt.h | 2 ++
> 3 files changed, 80 insertions(+)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 219262a8da..4a733298cd 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -30,6 +30,7 @@ config ARM_VIRT
> select ACPI_VIOT
> select VIRTIO_MEM_SUPPORTED
> select ACPI_CXL
> +select I2C_MCTP_CXL_FMAPI
>
> config CHEETAH
> bool
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d818131b57..ea04279515 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -80,6 +80,9 @@
> #include "hw/char/pl011.h"
> #include "hw/cxl/cxl.h"
> #include "qemu/guest-random.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/i2c/aspeed_i2c.h"
> +#include "hw/misc/i2c_mctp_cxl_fmapi.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -156,6 +159,8 @@ static const MemMapEntry base_memmap[] = {
> [VIRT_PVTIME] = { 0x090a, 0x0001 },
> [VIRT_SECURE_GPIO] ={ 0x090b, 0x1000 },
> [VIRT_MMIO] = { 0x0a00, 0x0200 },
> +[VIRT_I2C] ={ 0x0b00, 0x4000 },
> +[VIRT_RESET_FAKE] = { 0x0b004000, 0x0010 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size
> */
> [VIRT_PLATFORM_BUS] = { 0x0c00, 0x0200 },
> [VIRT_SECURE_MEM] = { 0x0e00, 0x0100 },
> @@ -192,6 +197,7 @@ static const int a15irqmap[] = {
> [VIRT_GPIO] = 7,
> [VIRT_SECURE_UART] = 8,
> [VIRT_ACPI_GED] = 9,
> +[VIRT_I2C] = 10,
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -1996,6 +2002,75 @@ static void virt_cpu_post_init(VirtMachineState *vms,
> MemoryRegion *sysmem)
> }
> }
>
> +static void create_mctp_test(MachineState *ms)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(ms);
> +MemoryRegion *sysmem = get_system_memory();
> +AspeedI2CState *aspeedi2c;
> +struct DeviceState *dev;
> +char *nodename_i2c_master;
> +char *nodename_i2c_sub;
> +char *nodename_reset;
> +uint32_t clk_phandle, reset_phandle;
> +MemoryRegion *sysmem2;
> +
> +dev = qdev_new("aspeed.i2c-ast2600");
> +aspeedi2c = ASPEED_I2C(dev);
> +object_property_set_link(OBJECT(dev), "dram", OBJECT(ms->ram),
> _fatal);
> +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_I2C].base);
> +sysbus_connect_irq(SYS_BUS_DEVICE(>busses[0]), 0,
> qdev_get_gpio_in(vms->gic, vms->irqmap[VIRT_I2C]));
> +
> +/* I2C bus DT */
> +reset_phandle = qemu_fdt_alloc_phandle(ms->fdt);
> +nodename_reset = g_strdup_printf("/reset@%" PRIx64,
> vms->memmap[VIRT_RESET_FAKE].base);
> +qemu_fdt_add_subnode(ms->fdt, nodename_reset);
> +qemu_fdt_setprop_string(ms->fdt, nodename_reset, "compatible",
> "snps,dw-low-reset");
> +qemu_fdt_setprop_sized_cells(ms->fdt, nodename_reset, "reg",
> + 2, vms->memmap[VIRT_RESET_FAKE].base,
> + 2, vms->memmap[VIRT_RESET_FAKE].size);
> +qemu_fdt_setprop_cell(ms->fdt, nodename_reset, "#reset-cells", 0x1);
> +qemu_fdt_setprop_cell(ms->fdt, nodename_reset, "phandle", reset_phandle);
> +sysmem2 = g_new(MemoryRegion, 1);
> +memory_region_init_ram(sysmem2, NULL, "reset",
> vms->memmap[VIRT_RESET_FAKE].size, NULL);
> +memory_region_add_subregion(sysmem, vms->memmap[VIRT_RESET_FAKE].base,
> sysmem2);
> +
> +
Re: [PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
On Sat, Apr 16, 2022 at 08:30:34PM +0100, Ben Dooks wrote:
> Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify
> the code.
>
> Signed-off-by; Ben Dooks
> ---
> hw/riscv/sifive_u.c | 20 +++-
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 1fe364cbb0..b00086d86e 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const
> MemMapEntry *memmap,
> char *nodename;
> uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
> uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
> -static const char * const ethclk_names[2] = { "pclk", "hclk" };
> -static const char * const clint_compat[2] = {
> -"sifive,clint0", "riscv,clint0"
> -};
> -static const char * const plic_compat[2] = {
> -"sifive,plic-1.0.0", "riscv,plic0"
> -};
>
> if (ms->dtb) {
> fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
> @@ -221,8 +214,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
> *memmap,
> nodename = g_strdup_printf("/soc/clint@%lx",
> (long)memmap[SIFIVE_U_DEV_CLINT].base);
> qemu_fdt_add_subnode(fdt, nodename);
> -qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -(char **)_compat, ARRAY_SIZE(clint_compat));
> +qemu_fdt_setprop_strings(fdt, nodename, "compatible",
> + "sifive,clint0", "riscv,clint0");
> qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
> qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
> cells, ms->smp.cpus * sizeof(uint32_t) * 4);
> @@ -273,8 +266,10 @@ static void create_fdt(SiFiveUState *s, const
> MemMapEntry *memmap,
> (long)memmap[SIFIVE_U_DEV_PLIC].base);
> qemu_fdt_add_subnode(fdt, nodename);
> qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
> -qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -(char **)_compat, ARRAY_SIZE(plic_compat));
> +//qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> +//(char **)_compat, ARRAY_SIZE(plic_compat));
Whoops, will fix this conversion, should have removed the original
instead of commenting it out.
--
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
[PATCH 2/4] hw/riscv: use qemu_fdt_setprop_reg64() in sifive_u.c
Use the qemu_fdt_setprop_reg64() to replace the code
that sets the property manually.
Signed-off-by: Ben Dooks
---
hw/riscv/sifive_u.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7fbc7dea42..1fe364cbb0 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -223,9 +223,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_CLINT].base,
-0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
g_free(cells);
@@ -235,9 +233,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_OTP].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "fuse-count", SIFIVE_U_OTP_REG_SIZE);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_OTP].base,
-0x0, memmap[SIFIVE_U_DEV_OTP].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_OTP]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-otp");
g_free(nodename);
@@ -250,9 +246,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x1);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
hfclk_phandle, rtcclk_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PRCI].base,
-0x0, memmap[SIFIVE_U_DEV_PRCI].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_PRCI]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-prci");
g_free(nodename);
@@ -284,9 +278,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PLIC].base,
-0x0, memmap[SIFIVE_U_DEV_PLIC].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_PLIC]);
qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
qemu_fdt_setprop_cell(fdt, nodename, "phandle", plic_phandle);
plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
@@ -304,9 +296,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop_cell(fdt, nodename, "#gpio-cells", 2);
qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_GPIO].base,
-0x0, memmap[SIFIVE_U_DEV_GPIO].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_GPIO]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts", SIFIVE_U_GPIO_IRQ0,
SIFIVE_U_GPIO_IRQ1, SIFIVE_U_GPIO_IRQ2, SIFIVE_U_GPIO_IRQ3,
SIFIVE_U_GPIO_IRQ4, SIFIVE_U_GPIO_IRQ5, SIFIVE_U_GPIO_IRQ6,
@@ -342,9 +332,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/cache-controller@%lx",
(long)memmap[SIFIVE_U_DEV_L2CC].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_L2CC].base,
-0x0, memmap[SIFIVE_U_DEV_L2CC].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_L2CC]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts",
SIFIVE_U_L2CC_IRQ0, SIFIVE_U_L2CC_IRQ1, SIFIVE_U_L2CC_IRQ2);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
@@ -366,9 +354,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
prci_phandle, PRCI_CLK_TLCLK);
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI2_IRQ);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_QSPI2].base,
-0x0, memmap[SIFIVE_U_DEV_QSPI2].size);
+qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_QSPI2]);
qemu_fdt_setprop_string(fdt, nodename, &quo
[PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify
the code.
Signed-off-by; Ben Dooks
---
hw/riscv/sifive_u.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 1fe364cbb0..b00086d86e 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,8 +214,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_reg64(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
@@ -273,8 +266,10 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+//qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
+//(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -410,8 +405,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
--
2.35.1
[PATCH 3/4] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
---
include/sysemu/device_tree.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 28352e7fcb..6ad09564d7 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,21 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @name: node name
+ * @prop: property array
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+do {\
+static const char * const __strs[] = { __VA_ARGS__ }; \
+qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+} while(0)
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
A couple of new device-tree helpers.
I've been doing a bit of looking at riscv and dt creation, and was thinking the following two helper functions would be useful so implemented qemu_fdt_setprop_reg64() and qemu_fdt_setprop_strings() and then applied them to the hw/riscv/sifive_u.c machine. I thought I should get a review in before I continued on looking at more possible helpers to make the dtb creation code leaner.
[PATCH 1/4] device_tree: add qemu_fdt_setprop_reg64 helper
Add a macro qemu_fdt_setprop_reg64() to set the given node's reg property directly from the memory map entry to avoid open coding of the following: qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x0, memmap[SIFIVE_U_DEV_OTP].base, 0x0, memmap[SIFIVE_U_DEV_OTP].size); Signed-off-by: Ben Dooks --- include/sysemu/device_tree.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index ef060a9759..28352e7fcb 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -135,6 +135,21 @@ int qemu_fdt_add_path(void *fdt, const char *path); sizeof(qdt_tmp));\ } while (0) +/** + * qemu_fdt_setprop_reg64: + * @fdt: the device tree path + * @node_path: node to set property on + * @map: the map entry to set the reg from + * + * A helper tp set the 'reg' node on the specified node from the given map + * entry. + */ +#define qemu_fdt_setprop_reg64(fdt, path, map) \ +qemu_fdt_setprop_cells(fdt, path, "reg",\ + (map)->base >> 32, (map)->base, \ + (map)->size >> 32, (map)->size) + + void qemu_fdt_dumpdtb(void *fdt, int size); /** -- 2.35.1
Account creation on QEMU Wiki
Hello, I'd like to create an account on the QEMU Wiki, but as it says on the main page, account creation is currently disabled to reduce spam. It says to ask someone with an existing account to create one for me, but I don't know anyone who has one, so I'm asking here. Thanks, -- Ben Westover OpenPGP_signature Description: OpenPGP digital signature
qemu crash 100% CPU with Ubuntu10.04 guest (solved)
Hi All, I'm cross-posting this from Reddit qemu_kvm, in case it helps in some way. I know my setup is ancient and unique; let me know if you would like more info. Symptoms: 1. Ubuntu10.04 32-bit guest locks up randomly between 0 and 30 days. 2. The console shows a CPU trace dump, nothing else logged on the guest or host. 3. Host system (Ubuntu20.04) 100% CPU for qemu process. Solution: When using virt-install, always use the "--os-variant" parameter! e.g. --os-variant ubuntu10.04 >From the man page "--os-variant... Optimize the guest configuration for a specific operating system". In this case, "optimize" apparently means "stop the crashing". I was deliberately avoiding the option because the VM was already performing much better than expected and I didn't want to complicate the configuration. This was very, very painful to troubleshoot; Involving spinning up 60 VMs simultaneously, waiting for a failure, changing one parameter, repeat. :(
Re: [PATCH v5 20/43] hw/cxl/device: Add a memory device (8.2.8.5)
On 22-02-11 16:45:19, Jonathan Cameron wrote:
> On Fri, 11 Feb 2022 07:50:00 -0800
> Ben Widawsky wrote:
>
> > On 22-02-02 14:10:14, Jonathan Cameron wrote:
> > > From: Ben Widawsky
> > >
> > > A CXL memory device (AKA Type 3) is a CXL component that contains some
> > > combination of volatile and persistent memory. It also implements the
> > > previously defined mailbox interface as well as the memory device
> > > firmware interface.
> > >
> > > Although the memory device is configured like a normal PCIe device, the
> > > memory traffic is on an entirely separate bus conceptually (using the
> > > same physical wires as PCIe, but different protocol).
> > >
> > > Once the CXL topology is fully configure and address decoders committed,
> > > the guest physical address for the memory device is part of a larger
> > > window which is owned by the platform. The creation of these windows
> > > is later in this series.
> > >
> > > The following example will create a 256M device in a 512M window:
> > > -object
> > > "memory-backend-file,id=cxl-mem1,share,mem-path=cxl-type3,size=512M"
> > > -device "cxl-type3,bus=rp0,memdev=cxl-mem1,id=cxl-pmem0"
> > >
> > > Note: Dropped PCDIMM info interfaces for now. They can be added if
> > > appropriate at a later date.
> > >
> > > Signed-off-by: Ben Widawsky
> > > Signed-off-by: Jonathan Cameron
> > > ---
> > > hw/cxl/cxl-mailbox-utils.c | 47 ++
> > > hw/mem/Kconfig | 5 ++
> > > hw/mem/cxl_type3.c | 170 +
> > > hw/mem/meson.build | 1 +
> > > include/hw/cxl/cxl.h | 1 +
> > > include/hw/cxl/cxl_pci.h | 22 +
> > > include/hw/pci/pci_ids.h | 1 +
> > > 7 files changed, 247 insertions(+)
> > > create mode 100644 hw/mem/cxl_type3.c
> > >
> > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > > index 16bb998735..808faec114 100644
> > > --- a/hw/cxl/cxl-mailbox-utils.c
> > > +++ b/hw/cxl/cxl-mailbox-utils.c
> > > @@ -50,6 +50,8 @@ enum {
> > > LOGS= 0x04,
> > > #define GET_SUPPORTED 0x0
> > > #define GET_LOG 0x1
> > > +IDENTIFY= 0x40,
> > > +#define MEMORY_DEVICE 0x0
> > > };
> > >
> > > /* 8.2.8.4.5.1 Command Return Codes */
> > > @@ -216,6 +218,48 @@ static ret_code cmd_logs_get_log(struct cxl_cmd *cmd,
> > > return CXL_MBOX_SUCCESS;
> > > }
> > >
> > > +/* 8.2.9.5.1.1 */
> > > +static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd,
> > > + CXLDeviceState *cxl_dstate,
> > > + uint16_t *len)
> > > +{
> > > +struct {
> > > +char fw_revision[0x10];
> > > +uint64_t total_capacity;
> > > +uint64_t volatile_capacity;
> > > +uint64_t persistent_capacity;
> > > +uint64_t partition_align;
> > > +uint16_t info_event_log_size;
> > > +uint16_t warning_event_log_size;
> > > +uint16_t failure_event_log_size;
> > > +uint16_t fatal_event_log_size;
> > > +uint32_t lsa_size;
> > > +uint8_t poison_list_max_mer[3];
> > > +uint16_t inject_poison_limit;
> > > +uint8_t poison_caps;
> > > +uint8_t qos_telemetry_caps;
> > > +} __attribute__((packed)) *id;
> > > +_Static_assert(sizeof(*id) == 0x43, "Bad identify size");
> > > +
> > > +uint64_t size = cxl_dstate->pmem_size;
> > > +
> > > +if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> > > +return CXL_MBOX_INTERNAL_ERROR;
> > > +}
> > > +
> > > +id = (void *)cmd->payload;
> > > +memset(id, 0, sizeof(*id));
> > > +
> > > +/* PMEM only */
> > > +snprintf(id->fw_revision, 0x10, "BWFW VERSION %02d", 0);
> > > +
> > > +id->total_capacity = size / (256 << 20);
> > > +id->persistent_capacity = size / (256 << 20);
> > > +
> > > +*len = sizeof(*id);
> > > +return CXL_MBOX_SUCCESS;
> > > +}
> > > +
> > > #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> &g
Re: [PATCH v5 20/43] hw/cxl/device: Add a memory device (8.2.8.5)
On 22-02-02 14:10:14, Jonathan Cameron wrote:
> From: Ben Widawsky
>
> A CXL memory device (AKA Type 3) is a CXL component that contains some
> combination of volatile and persistent memory. It also implements the
> previously defined mailbox interface as well as the memory device
> firmware interface.
>
> Although the memory device is configured like a normal PCIe device, the
> memory traffic is on an entirely separate bus conceptually (using the
> same physical wires as PCIe, but different protocol).
>
> Once the CXL topology is fully configure and address decoders committed,
> the guest physical address for the memory device is part of a larger
> window which is owned by the platform. The creation of these windows
> is later in this series.
>
> The following example will create a 256M device in a 512M window:
> -object "memory-backend-file,id=cxl-mem1,share,mem-path=cxl-type3,size=512M"
> -device "cxl-type3,bus=rp0,memdev=cxl-mem1,id=cxl-pmem0"
>
> Note: Dropped PCDIMM info interfaces for now. They can be added if
> appropriate at a later date.
>
> Signed-off-by: Ben Widawsky
> Signed-off-by: Jonathan Cameron
> ---
> hw/cxl/cxl-mailbox-utils.c | 47 ++
> hw/mem/Kconfig | 5 ++
> hw/mem/cxl_type3.c | 170 +
> hw/mem/meson.build | 1 +
> include/hw/cxl/cxl.h | 1 +
> include/hw/cxl/cxl_pci.h | 22 +
> include/hw/pci/pci_ids.h | 1 +
> 7 files changed, 247 insertions(+)
> create mode 100644 hw/mem/cxl_type3.c
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 16bb998735..808faec114 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -50,6 +50,8 @@ enum {
> LOGS= 0x04,
> #define GET_SUPPORTED 0x0
> #define GET_LOG 0x1
> +IDENTIFY= 0x40,
> +#define MEMORY_DEVICE 0x0
> };
>
> /* 8.2.8.4.5.1 Command Return Codes */
> @@ -216,6 +218,48 @@ static ret_code cmd_logs_get_log(struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/* 8.2.9.5.1.1 */
> +static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd,
> + CXLDeviceState *cxl_dstate,
> + uint16_t *len)
> +{
> +struct {
> +char fw_revision[0x10];
> +uint64_t total_capacity;
> +uint64_t volatile_capacity;
> +uint64_t persistent_capacity;
> +uint64_t partition_align;
> +uint16_t info_event_log_size;
> +uint16_t warning_event_log_size;
> +uint16_t failure_event_log_size;
> +uint16_t fatal_event_log_size;
> +uint32_t lsa_size;
> +uint8_t poison_list_max_mer[3];
> +uint16_t inject_poison_limit;
> +uint8_t poison_caps;
> +uint8_t qos_telemetry_caps;
> +} __attribute__((packed)) *id;
> +_Static_assert(sizeof(*id) == 0x43, "Bad identify size");
> +
> +uint64_t size = cxl_dstate->pmem_size;
> +
> +if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> +return CXL_MBOX_INTERNAL_ERROR;
> +}
> +
> +id = (void *)cmd->payload;
> +memset(id, 0, sizeof(*id));
> +
> +/* PMEM only */
> +snprintf(id->fw_revision, 0x10, "BWFW VERSION %02d", 0);
> +
> +id->total_capacity = size / (256 << 20);
> +id->persistent_capacity = size / (256 << 20);
> +
> +*len = sizeof(*id);
> +return CXL_MBOX_SUCCESS;
> +}
> +
> #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> #define IMMEDIATE_POLICY_CHANGE (1 << 3)
> #define IMMEDIATE_LOG_CHANGE (1 << 4)
> @@ -233,8 +277,11 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
> [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8,
> IMMEDIATE_POLICY_CHANGE },
> [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported,
> 0, 0 },
> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
> +[IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
> +cmd_identify_memory_device, 0, 0 },
> };
>
> +
> void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
> {
> uint16_t ret = CXL_MBOX_SUCCESS;
> diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
> index 03dbb3c7df..73c5ae8ad9 100644
> --- a/hw/mem/Kconfig
> +++ b/hw/mem/Kconfig
> @@ -11,3 +11,8 @@ config NVDIMM
>
> config SPARSE_MEM
> bool
> +
> +config CXL_MEM_DEVICE
> +bool
> +default y if CXL
> +select MEM_DEVICE
> diff --git a/hw/mem/cxl_type3.
Re: [PATCH v4 00/42] CXl 2.0 emulation Support
On 22-01-25 11:18:08, Ben Widawsky wrote: > Really awesome work Jonathan. Dan and I are wrapping up some of the kernel > bits, > so all I'll do for now is try to run this, but I hope to be able to review the > parts I'm familiar with at least. > > On 22-01-24 17:16:23, Jonathan Cameron wrote: > > Previous version was RFC v3: CXL 2.0 Support. > > No longer an RFC as I would consider the vast majority of this > > to be ready for detailed review. There are still questions called > > out in some patches however. > > > > Looking in particular for: > > * Review of the PCI interactions > > * x86 and ARM machine interactions (particularly the memory maps) > > * Review of the interleaving approach - is the basic idea > > acceptable? > > * Review of the command line interface. > > * CXL related review welcome but much of that got reviewed > > in earlier versions and hasn't changed substantially. > > > > Main changes: > > * The CXL fixed memory windows are now instantiated via a > > -cxl-fixed-memory-window command line option. As they are host level > > entities, not associated with a particular hardware entity a top > > level parameter seems the most natural way to describe them. > > This is also much closer to how it works on a real host than the > > previous assignment of a physical address window to all components > > along the CXL path. > > Excellent. > > > * Dynamic host memory physical address space allocation both for > > the CXL host bridge MMIO space and the CFMWS windows. > > I thought I had done the host bridge MMIO, but perhaps I was mistaken. Either > way, this is an important step to support all platforms more generally. > > > * Interleaving support (based loosely on Philippe Mathieu-Daudé's > > earlier work on an interleaved memory device). Note this is rudimentary > > and low performance but it may be sufficient for test purposes. > > I'll have to look at this further. I had some thoughts about how we might make > this fast, but it would be more of fake interleaving. How low is "low"? > > > * Additional PCI and memory related utility functions needed for the > > interleaving. > > * Various minor cleanup and increase in scope of tests. > > * For now dropped the support for presenting CXL type 3 devices > > as memory devices in various QEMU interfaces. > > What are the downsides to this? I only used the memory interface originally > because it seemed like a natural fit, but looking back I'm not sure we gain > much (though my memory is very lossy). > > > * Dropped the patch letting UID be different from bus_nr. Whilst > > it may be a useful thing to have, we don't need it for this series > > and so should be handled separately. > > > > I've called out patches with major changes by marking them as > > co-developed or introducing them as new patches. The original > > memory window code has been dropped > > > > After discussions at plumbers and more recently on the mailing list > > it was clear that there was interest in getting emulation for CXL 2.0 > > upstream in QEMU. This version resolves many of the outstanding issues > > and enables the following features: > > > > * Support on both x86/pc and ARM/virt with relevant ACPI tables > > generated in QEMU. > > * Host bridge based on the existing PCI Expander Bridge PXB. > > * CXL fixed memory windows, allowing host to describe interleaving > > across multiple CXL host bridges. > > * pxb-cxl CXL host bridge support including MMIO region for control > > and HDM (Host manage device memory - basically interleaving / routing) > > decoder configuration. > > * Basic CXL Root port support. > > * CXL Type 3 device support with persistent memory regions (backed by > > hostmem backend). > > * Pulled MAINTAINERS entry out to a separate patch and add myself as > > a co-maintainer at Ben's suggestion. > > > > Big TODOs: > > > > * Volatile memory devices (easy but it's more code so left for now). > > * Switch support. > > * Hotplug? May not need much but it's not tested yet! > > * More tests and tighter verification that values written to hardware > > are actually valid - stuff that real hardware would check. > > * Main host bridge support (not a priority for me...) > > I originally cared about this for the sake of making a system more realistic. > I > now believe we should drop this entirely. > > > * Testing, testing and more testing. I have been running a basic > > set of ARM and x86 tests on th
Re: [PATCH v4 00/42] CXl 2.0 emulation Support
Really awesome work Jonathan. Dan and I are wrapping up some of the kernel bits, so all I'll do for now is try to run this, but I hope to be able to review the parts I'm familiar with at least. On 22-01-24 17:16:23, Jonathan Cameron wrote: > Previous version was RFC v3: CXL 2.0 Support. > No longer an RFC as I would consider the vast majority of this > to be ready for detailed review. There are still questions called > out in some patches however. > > Looking in particular for: > * Review of the PCI interactions > * x86 and ARM machine interactions (particularly the memory maps) > * Review of the interleaving approach - is the basic idea > acceptable? > * Review of the command line interface. > * CXL related review welcome but much of that got reviewed > in earlier versions and hasn't changed substantially. > > Main changes: > * The CXL fixed memory windows are now instantiated via a > -cxl-fixed-memory-window command line option. As they are host level > entities, not associated with a particular hardware entity a top > level parameter seems the most natural way to describe them. > This is also much closer to how it works on a real host than the > previous assignment of a physical address window to all components > along the CXL path. Excellent. > * Dynamic host memory physical address space allocation both for > the CXL host bridge MMIO space and the CFMWS windows. I thought I had done the host bridge MMIO, but perhaps I was mistaken. Either way, this is an important step to support all platforms more generally. > * Interleaving support (based loosely on Philippe Mathieu-Daudé's > earlier work on an interleaved memory device). Note this is rudimentary > and low performance but it may be sufficient for test purposes. I'll have to look at this further. I had some thoughts about how we might make this fast, but it would be more of fake interleaving. How low is "low"? > * Additional PCI and memory related utility functions needed for the > interleaving. > * Various minor cleanup and increase in scope of tests. > * For now dropped the support for presenting CXL type 3 devices > as memory devices in various QEMU interfaces. What are the downsides to this? I only used the memory interface originally because it seemed like a natural fit, but looking back I'm not sure we gain much (though my memory is very lossy). > * Dropped the patch letting UID be different from bus_nr. Whilst > it may be a useful thing to have, we don't need it for this series > and so should be handled separately. > > I've called out patches with major changes by marking them as > co-developed or introducing them as new patches. The original > memory window code has been dropped > > After discussions at plumbers and more recently on the mailing list > it was clear that there was interest in getting emulation for CXL 2.0 > upstream in QEMU. This version resolves many of the outstanding issues > and enables the following features: > > * Support on both x86/pc and ARM/virt with relevant ACPI tables > generated in QEMU. > * Host bridge based on the existing PCI Expander Bridge PXB. > * CXL fixed memory windows, allowing host to describe interleaving > across multiple CXL host bridges. > * pxb-cxl CXL host bridge support including MMIO region for control > and HDM (Host manage device memory - basically interleaving / routing) > decoder configuration. > * Basic CXL Root port support. > * CXL Type 3 device support with persistent memory regions (backed by > hostmem backend). > * Pulled MAINTAINERS entry out to a separate patch and add myself as > a co-maintainer at Ben's suggestion. > > Big TODOs: > > * Volatile memory devices (easy but it's more code so left for now). > * Switch support. > * Hotplug? May not need much but it's not tested yet! > * More tests and tighter verification that values written to hardware > are actually valid - stuff that real hardware would check. > * Main host bridge support (not a priority for me...) I originally cared about this for the sake of making a system more realistic. I now believe we should drop this entirely. > * Testing, testing and more testing. I have been running a basic > set of ARM and x86 tests on this, but there is always room for > more tests and greater automation. > > Why do we want QEMU emulation of CXL? > > As Ben stated in V3, QEMU support has been critical to getting OS > software written given lack of availability of hardware supporting the > latest CXL features (coupled with very high demand for support being > ready in a timely fashion). What has become clear since Ben's v3 > is that situation is a continuous one. Whilst we can't talk about > them yet, CXL 3.0 features an
Re: Follow-up on the CXL discussion at OFTC
On 21-11-30 13:09:56, Jonathan Cameron wrote: > On Mon, 29 Nov 2021 18:28:43 + > Alex Bennée wrote: > > > Ben Widawsky writes: > > > > > On 21-11-26 12:08:08, Alex Bennée wrote: > > >> > > >> Ben Widawsky writes: > > >> > > >> > On 21-11-19 02:29:51, Shreyas Shah wrote: > > >> >> Hi Ben > > >> >> > > >> >> Are you planning to add the CXL2.0 switch inside QEMU or already > > >> >> added in one of the version? > > >> >> > > >> > > > >> > From me, there are no plans for QEMU anything until/unless upstream > > >> > thinks it > > >> > will merge the existing patches, or provide feedback as to what it > > >> > would take to > > >> > get them merged. If upstream doesn't see a point in these patches, > > >> > then I really > > >> > don't see much value in continuing to further them. Once hardware > > >> > comes out, the > > >> > value proposition is certainly less. > > >> > > >> I take it: > > >> > > >> Subject: [RFC PATCH v3 00/31] CXL 2.0 Support > > >> Date: Mon, 1 Feb 2021 16:59:17 -0800 > > >> Message-Id: <20210202005948.241655-1-ben.widaw...@intel.com> > > >> > > >> is the current state of the support? I saw there was a fair amount of > > >> discussion on the thread so assumed there would be a v4 forthcoming at > > >> some point. > > > > > > Hi Alex, > > > > > > There is a v4, however, we never really had a solid plan for the primary > > > issue > > > which was around handling CXL memory expander devices properly (both from > > > an > > > interleaving standpoint as well as having a device which hosts multiple > > > memory > > > capacities, persistent and volatile). I didn't feel it was worth sending > > > a v4 > > > unless someone could say > > > > > > 1. we will merge what's there and fix later, or > > > 2. you must have a more perfect emulation in place, or > > > 3. we want to see usages for a real guest > > > > I think 1. is acceptable if the community is happy there will be ongoing > > development and it's not just a code dump. Given it will have a > > MAINTAINERS entry I think that is demonstrated. > > My thought is also 1. There are a few hacks we need to clean out but > nothing that should take too long. I'm sure it'll take a rev or two more. > Right now for example, I've added support to arm-virt and maybe need to > move that over to a different machine model... > The most annoying thing about rebasing it is passing the ACPI tests. They keep changing upstream. Being able to at least merge up to there would be huge. > > > > What's the current use case? Testing drivers before real HW comes out? > > Will it still be useful after real HW comes out for people wanting to > > debug things without HW? > > CXL is continuing to expand in scope and capabilities and I don't see that > reducing any time soon (My guess is 3 years+ to just catch up with what is > under discussion today). So I see two long term use cases: > > 1) Automated verification that we haven't broken things. I suspect no > one person is going to have a test farm covering all the corner cases. > So we'll need emulation + firmware + kernel based testing. > Does this exist in other forms? AFAICT for x86, there isn't much example of this. > 2) New feature prove out. We have already used it for some features that > will appear in the next spec version. Obviously I can't say what or > send that code out yet. Its very useful and the spec draft has changed > in various ways a result. I can't commit others, but Huawei will be > doing more of this going forwards. For that we need a stable base to > which we add the new stuff once spec publication allows it. > I can't commit for Intel but I will say there's more latitude now to work on projects like this compared to when I first wrote the patches. I have interesting in continuing to develop this as well. I'm very interested in supporting interleave and hotplug specifically. > > > > > > > > I had hoped we could merge what was there mostly as is and fix it up as > > > we go. > > > It's useful in the state it is now, and as time goes on, we find more > > > usecases > > > for it in a VMM, and not just driver development. > > > > > >> > > >&
Re: Follow-up on the CXL discussion at OFTC
On 21-11-26 12:08:08, Alex Bennée wrote: > > Ben Widawsky writes: > > > On 21-11-19 02:29:51, Shreyas Shah wrote: > >> Hi Ben > >> > >> Are you planning to add the CXL2.0 switch inside QEMU or already added in > >> one of the version? > >> > > > > From me, there are no plans for QEMU anything until/unless upstream thinks > > it > > will merge the existing patches, or provide feedback as to what it would > > take to > > get them merged. If upstream doesn't see a point in these patches, then I > > really > > don't see much value in continuing to further them. Once hardware comes > > out, the > > value proposition is certainly less. > > I take it: > > Subject: [RFC PATCH v3 00/31] CXL 2.0 Support > Date: Mon, 1 Feb 2021 16:59:17 -0800 > Message-Id: <20210202005948.241655-1-ben.widaw...@intel.com> > > is the current state of the support? I saw there was a fair amount of > discussion on the thread so assumed there would be a v4 forthcoming at > some point. Hi Alex, There is a v4, however, we never really had a solid plan for the primary issue which was around handling CXL memory expander devices properly (both from an interleaving standpoint as well as having a device which hosts multiple memory capacities, persistent and volatile). I didn't feel it was worth sending a v4 unless someone could say 1. we will merge what's there and fix later, or 2. you must have a more perfect emulation in place, or 3. we want to see usages for a real guest I had hoped we could merge what was there mostly as is and fix it up as we go. It's useful in the state it is now, and as time goes on, we find more usecases for it in a VMM, and not just driver development. > > Adding new subsystems to QEMU does seem to be a pain point for new > contributors. Patches tend to fall through the cracks of existing > maintainers who spend most of their time looking at stuff that directly > touches their files. There is also a reluctance to merge large chunks of > functionality without an identified maintainer (and maybe reviewers) who > can be the contact point for new patches. So in short you need: > > - Maintainer Reviewed-by/Acked-by on patches that touch other sub-systems This is the challenging one. I have Cc'd the relevant maintainers (hw/pci and hw/mem are the two) in the past, but I think there interest is lacking (and reasonably so, it is an entirely different subsystem). > - Reviewed-by tags on the new sub-system patches from anyone who understands > CXL I have/had those from Jonathan. > - Some* in-tree testing (so it doesn't quietly bitrot) We had this, but it's stale now. We can bring this back up. > - A patch adding the sub-system to MAINTAINERS with identified people That was there too. Since the original posting, I'd be happy to sign Jonathan up to this if he's willing. > > * Some means at least ensuring qtest can instantiate the device and not > fall over. Obviously more testing is better but it can always be > expanded on in later series. This was in the patch series. It could use more testing for sure, but I had basic functional testing in place via qtest. > > Is that the feedback you were looking for? You validated my assumptions as to what's needed, but your first bullet is the one I can't seem to pin down. Thanks. Ben
Re: Follow-up on the CXL discussion at OFTC
On 21-11-19 18:53:43, Jonathan Cameron wrote: > On Thu, 18 Nov 2021 17:52:07 -0800 > Ben Widawsky wrote: > > > On 21-11-18 15:20:34, Saransh Gupta1 wrote: > > > Hi Ben and Jonathan, > > > > > > Thanks for your replies. I'm looking forward to the patches. > > > > > > For QEMU, I see hotplug support as an item on the list and would like to > > > start working on it. It would be great if you can provide some pointers > > > about how I should go about it. > > > > It's been a while, so I can't recall what's actually missing. I think it > > should > > mostly behave like a normal PCIe endpoint. > > > > > Also, which version of kernel and QEMU (maybe Jonathan's upcoming > > > version) > > > would be a good starting point for it? > > > > If he rebased and claims it works I have no reason to doubt it :-). I have a > > small fix on my v4 branch if you want to use the latest port patches. > > Thanks. I'd missed that one. Now pushed down into the original patch. > > It occurred to me that technically I only know my rebase works on Arm64... > Fingers crossed for x86. > > Anyhow, I'll run more tests on it next week (possibly even including x86), > > Available at: > https://github.com/hisilicon/qemu/tree/cxl-hacks > > For arm64 the description at > https://people.kernel.org/jic23/ will almost work with this. > There is a bug however that I need to track down which currently means you > need to set the pxb uid to the same as the bus number. Shouldn't take > long to fix but it's Friday evening... > (add uid=0x80 to the options for pxb-cxl) > > I dropped the CMA patch from Avery from this tree as need to improve > the way it's getting hold of some parts of libSPDM and move to the current > version of that library (rather than the old openSPDM) > > Ben, if you don't mind me trying to push this forwards, I'll do a bit > of cleanup and reordering then make use of the QEMU folks we have / know and > try and start getting your hard work upstream. I don't mind at all. > > Whilst I've not poked the various interfaces yet, this is working with > a kernel tree that is current cxl/next + Ira's DOE series and Ben's region > series > + (for fun) my SPDM series. That tree's a franken monster so I'm not planning > to share it unless anyone has particular need of it. Hopefully the various > parts will move forwards this cycle anyway so I can stop having to spend > as much time on rebases! > > Jonathan > > > > > > > > > Thanks, > > > Saransh > > > > > > > > > > > > From: "Jonathan Cameron" > > > To: "Ben Widawsky" > > > Cc: "Saransh Gupta1" , , > > > > > > Date: 11/17/2021 09:32 AM > > > Subject:[EXTERNAL] Re: Follow-up on the CXL discussion at OFTC > > > > > > > > > > > > On Wed, 17 Nov 2021 08:57:19 -0800 > > > Ben Widawsky wrote: > > > > > > > Hi Saransh. Please add the list for these kind of questions. I've > > > converted your > > > > HTML mail, but going forward, the list will eat it, so please use text > > > > > > > only. > > > > > > > > On 21-11-16 00:14:33, Saransh Gupta1 wrote: > > > > >Hi Ben, > > > > > > > > > >This is Saransh from IBM. Sorry to have (unintentionally) dropped > > > > > > > > out > > > > >of the conversion on OFTC, I'm new to IRC. > > > > >Just wanted to follow-up on the discussion there. We discussed > > > about > > > > >helping with linux patches reviews. On that front, I have > > > identified > > > > >some colleague(s) who can help me with this. Let me know if/how you > > > > >want to proceed with that. > > > > > > > > Currently the ball is in my court to re-roll the RFC v2 patches [1] > > > based on > > > > feedback from Dan. I've implemented all/most of it, but I'm still > > > debugging some > > > > issues with the result. > > > > > > > > > > > > > >Maybe not urgently, but my team would also like to get an > > > understanding > > > > >of the missing pieces in QEMU. Initially our focus is on type3 > > > memory > > > > >access and hotplug support. Most of the work that my team does is > > &g
Re: Follow-up on the CXL discussion at OFTC
On 21-11-19 02:29:51, Shreyas Shah wrote: > Hi Ben > > Are you planning to add the CXL2.0 switch inside QEMU or already added in one > of the version? > >From me, there are no plans for QEMU anything until/unless upstream thinks it will merge the existing patches, or provide feedback as to what it would take to get them merged. If upstream doesn't see a point in these patches, then I really don't see much value in continuing to further them. Once hardware comes out, the value proposition is certainly less. Having said that, once I get the port/region patches merged for the Linux driver, I do intend to go back and try to implement a basic switch so that we can test those flows. I admit, I'm curious why you're interested in switches. > Regards, > Shreyas > > -Original Message- > From: Ben Widawsky > Sent: Thursday, November 18, 2021 5:48 PM > To: Shreyas Shah > Cc: Saransh Gupta1 ; Jonathan Cameron > ; linux-...@vger.kernel.org; > qemu-devel@nongnu.org > Subject: Re: Follow-up on the CXL discussion at OFTC > > On 21-11-18 22:52:56, Shreyas Shah wrote: > > Hello Folks, > > > > Any plan to add CXL2.0 switch ports in QEMU? > > What's your definition of plan? > > > > > Regards, > > Shreyas > > [snip]
Re: Follow-up on the CXL discussion at OFTC
On 21-11-18 15:20:34, Saransh Gupta1 wrote: > Hi Ben and Jonathan, > > Thanks for your replies. I'm looking forward to the patches. > > For QEMU, I see hotplug support as an item on the list and would like to > start working on it. It would be great if you can provide some pointers > about how I should go about it. It's been a while, so I can't recall what's actually missing. I think it should mostly behave like a normal PCIe endpoint. > Also, which version of kernel and QEMU (maybe Jonathan's upcoming version) > would be a good starting point for it? If he rebased and claims it works I have no reason to doubt it :-). I have a small fix on my v4 branch if you want to use the latest port patches. > > Thanks, > Saransh > > > > From: "Jonathan Cameron" > To: "Ben Widawsky" > Cc: "Saransh Gupta1" , , > > Date: 11/17/2021 09:32 AM > Subject:[EXTERNAL] Re: Follow-up on the CXL discussion at OFTC > > > > On Wed, 17 Nov 2021 08:57:19 -0800 > Ben Widawsky wrote: > > > Hi Saransh. Please add the list for these kind of questions. I've > converted your > > HTML mail, but going forward, the list will eat it, so please use text > only. > > > > On 21-11-16 00:14:33, Saransh Gupta1 wrote: > > >Hi Ben, > > > > > >This is Saransh from IBM. Sorry to have (unintentionally) dropped > out > > >of the conversion on OFTC, I'm new to IRC. > > >Just wanted to follow-up on the discussion there. We discussed > about > > >helping with linux patches reviews. On that front, I have > identified > > >some colleague(s) who can help me with this. Let me know if/how you > > >want to proceed with that. > > > > Currently the ball is in my court to re-roll the RFC v2 patches [1] > based on > > feedback from Dan. I've implemented all/most of it, but I'm still > debugging some > > issues with the result. > > > > > > > >Maybe not urgently, but my team would also like to get an > understanding > > >of the missing pieces in QEMU. Initially our focus is on type3 > memory > > >access and hotplug support. Most of the work that my team does is > > >open-source, so contributing to the QEMU effort is another possible > > >line of collaboration. > > > > If you haven't seen it already, check out my LPC talk [2]. The QEMU > patches > > could use a lot of love. Mostly, I have little/no motivation until > upstream > > shows an interest because I don't have time currently to make sure I > don't break > > vs. upstream. If you want more details here, I can provide them, and I > will Cc > > the qemu-devel mailing list; the end of the LPC talk [2] does have a > list. > Hi Ben, Saransh > > I have a forward port of the series + DOE etc to near current QEMU that is > lightly tested, > and can look to push that out publicly later this week. > > I'd also like to push QEMU support forwards and to start getting this > upstream in QEMU > + fill in some of the missing parts. > > Was aiming to make progress on this a few weeks ago, but as ever other > stuff > got in the way. > > +CC qemu-devel in case anyone else also looking at this. > > Jonathan > > > > > > > > > > >Thanks for your help and guidance! > > > > > >Best, > > >Saransh Gupta > > >Research Staff Member, IBM Research > > > > [1]: > https://lore.kernel.org/linux-cxl/20211022183709.1199701-1-ben.widaw...@intel.com/T/#t > > > > [2]: > https://www.youtube.com/watch?v=g89SLjt5Bd4=PLVsQ_xZBEyN3wA8Ej4BUjudXFbXuxhnfc=49 > > > > > > >
Re: Follow-up on the CXL discussion at OFTC
On 21-11-18 22:52:56, Shreyas Shah wrote: > Hello Folks, > > Any plan to add CXL2.0 switch ports in QEMU? What's your definition of plan? > > Regards, > Shreyas [snip]
Re: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails with assertion
Thanks Dave. Samarth, Easiest is to just use our run_qemu and figure out the diffs (--cmdline will print the qemu commandline): https://github.com/pmem/run_qemu If you're not able to figure it out after that, please let me know. On 21-08-10 17:38:16, Samarth Saxena wrote: > Thanks Dave, > > The Qemu version is qemu-6.0.50. > > I am trying to capture the stack and will place it ASAP. > > Regards, > Samarth > > > -Original Message- > From: Dr. David Alan Gilbert > Sent: Tuesday, August 10, 2021 4:58 PM > To: Samarth Saxena ; ben.widaw...@intel.com > Cc: qemu-devel@nongnu.org > Subject: Re: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails > with assertion > > EXTERNAL MAIL > > > * Samarth Saxena (samar...@cadence.com) wrote: > > Hi All, > > > > I am trying the following command to run Qemu with Kernel 5.14. > > cc'ing in Ben who I think owns the CXL stuff. > > > qemu-system-x86_64 -M q35,accel=kvm,nvdimm=on,cxl=on,hmat=on -m > > 8448M,slots=2,maxmem=16G -smp 8,sockets=2,cores=2,threads=2 -hda > > /lan/dscratch/singhabh/shradha/ubuntu-20.10-64_with_orig_driver_backup > > .qcow2 -enable-kvm -usbdevice tablet -L $VB_ROOT/etc/vm/common/ > > -object memory-backend-ram,id=cxl-ram,share=on,size=256M -device > > "pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52,uid=0,len-window-base=1,window- > > base[0]=0x4c000,memdev[0]=cxl-ram" -device > > cxl-rp,id=rp0,bus=cxl.0,addr=0.0,chassis=0,slot=0 -device > > cxl-type3,bus=rp0,memdev=cxl-ram,id=cxl-vmem0,size=256M -numa > > node,nodeid=0,memdev=cxl-ram -object > > memory-backend-ram,id=other-ram,size=8G,prealloc=n,share=off -numa > > node,nodeid=1,memdev=other-ram,initiator=0 -numa > > cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 > > You probably need to state which qemu tree and version you're using here. > > > I get the following crash before the machine starts to boot. > > > > qemu-system-x86_64: ../softmmu/memory.c:2443: > > memory_region_add_subregion_common: Assertion `!subregion->container' > > failed. > > It's probably best to check with Ben, but you'll want a backtrace and figure > out which subregion and region you're dealing with. > > Dave > > > > > Please help me find what's wrong here. > > > > Regards, > > [CadenceLogoRed185Regcopy1583174817new51584636989.png]<https://www.cad > > ence.com/en_US/home.html> > > Samarth Saxena > > Sr Principal Software Engineer > > T: +911204308300 > > [UIcorrectsize1583179003.png]<https://www.cadence.com/en_US/home.html> > > [16066EmailSignatureFortune100Best2021White92x1271617625037.png] > ://www.cadence.com/en_US/home/company/careers.html> > > > > > > > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK >
Re: CXL 2.0 memory device design
On 21-03-19 18:07:05, Igor Mammedov wrote: > On Wed, 17 Mar 2021 14:40:58 -0700 > Ben Widawsky wrote: > > > Phil, Igor, Markus > > > > TL;DR: What to do about multiple capacities in a single device, and what to > > do > > about interleave? > > > > I've hacked together a basic CXL 2.0 implementation which exposes a CXL > > "Type 3" > > memory device (CXL 2.0 Chapter 2.3). For what we were trying to do this was > > sufficient. There are two main capabilities that CXL spec exposes which > > I've not > > implemented that I'd like to start working toward and am realizing that I > > what I > > have so far might not be able to carry forward to that next milestone. > > > > Capability 1. A CXL memory device may have both a volatile, and a persistent > > capacity. https://bwidawsk.net/HDM_decoders.svg (lower right > > side). The current work only supports a single persistent > > capacity. > > Capability 2. CXL topologies can be interleaved. Basic example: > > https://bwidawsk.net/HDM_decoders.svg (lower left side) > > > > Memory regions are configured via a CXL spec defined HDM decoder. The HDM > > decoder which is minimally implemented supports all the functionality > > mentioned > > above (base, size, interleave, type, etc.). A CXL component may have up to > > 10 > > HDMs. > > > > What I have today: https://bwidawsk.net/QEMU_objects.svg > > There's a single memory backend device for each host bridge. That backend is > > passed to any CXL component that is part of the hierarchy underneath that > > hostbridge. In the case of a Type 3 device memory capacity a subregion is > > created for that capacity from within the main backend. The device itself > > implements the TYPE_MEMORY_DEVICE interface. This allows me to utilize the > > existing memory region code to determine the next free address, and warn on > > overlaps. It hopefully will help when I'm ready to support hotplug. > > As was mentioned on IRC (and maybe on my first attempt to review your patches) > > Backends are for managing host resource (RAM/file/fd) and its properties. > A backend should match a corresponding device model (frontend/emulated hw, > i.e. CXL type 3 device), > the later should manage how it looks to guest. > > i.e. in CXL case I'd imagine CLI adding memory look like: > > -machine cxl=on \ > -device cxl-host-bridge,id=foo \ > -device cxl-rp,id=rp0,bus="foo" ] > -object memory-backend-file,mem-path=somefile,id=mem1 \ > -device cxl-mem,backend=mem1[,bus=rp0] > > if you need to add CXL memory you add pair memory-backend-file + cxl-mem > (which practically reflects what would happen on real hw) Conceptually this is fine with me and I agree it more accurately reflects real hardware. The issue has been more around how to implement that model. > > Sharing a single backend between several CXL devices as a means to implement > interleaving, looks to me as abusing backend concept. > (that's not how it's done on real hw, memory chips (backend) that belong to a > CXL memory > card are not shared with other CXL devices). It's probably address space > that gets partitioned in small chunks to map them to one or another CXL > memory dev. Yes, it is an address space that gets partitioned. Is the recommendation then to create a new address space for each of these regions? > > I'd suggest to forget about interleaving for now and implement > a simplified variant without it. That's fine for me, I'm just hoping if we ever get to the point of implementing interleave, we don't have to start entirely over. > > > Where I've gotten stuck: A Memory Device expects only to have one region of > > memory. Trying to add a second breaks pretty much everything. > > Memory device has very simplistic rules to map devices in address space > (we basically open-coded part of 'memory controller' into machine code > to do address allocation/mapping, due to PC machine historically not having > it implemented properly). > > > I'm hoping to start the discussion about what the right way to emulate this > > in > > QEMU. Ideally something upstreamable would be great. I think adding a > > secondary > > (or more) capacity to a memory class device is doable, but probably not the > > right approach. > > Also earlier you mentioned that it's guest who programs where CXL memory is > mapped, > that isn't compatible with simplistic Memory device interface where guest > has no say where memory is mapped, in Memory Device case, machine code picks > the next free gap in fixed hotplug region and maps it
