Re: [PATCH v7 2/2] hw/riscv/virt: Add IOPMP support
On Wed, Jun 12, 2024 at 1:25 PM Ethan Chen via wrote:
>
> If a requestor device is connected to the IOPMP device, its memory access will
> be checked by the IOPMP rule.
>
> - Add 'iopmp=on' option to add an iopmp device and make the Generic PCI
> Express
> Bridge connect to IOPMP.
I have only had a chance to have a quick look at this series and the spec.
But the IOPMP spec applies to all devices right, but this series seems
to only work with PCI. Am I missing something?
Alistair
>
> Signed-off-by: Ethan Chen
> ---
> docs/system/riscv/virt.rst | 6
> hw/riscv/Kconfig | 1 +
> hw/riscv/virt.c| 57 --
> include/hw/riscv/virt.h| 5 +++-
> 4 files changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
> index 9a06f95a34..3b2576f905 100644
> --- a/docs/system/riscv/virt.rst
> +++ b/docs/system/riscv/virt.rst
> @@ -116,6 +116,12 @@ The following machine-specific options are supported:
>having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified,
>the default number of per-HART VS-level AIA IMSIC pages is 0.
>
> +- iopmp=[on|off]
> +
> + When this option is "on", an IOPMP device is added to machine. It checks
> dma
> + operations from the generic PCIe host bridge. This option is assumed to be
> + "off".
> +
> Running Linux kernel
>
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index a2030e3a6f..0b45a5ade2 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -56,6 +56,7 @@ config RISCV_VIRT
> select PLATFORM_BUS
> select ACPI
> select ACPI_PCI
> +select RISCV_IOPMP
>
> config SHAKTI_C
> bool
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4fdb660525..53a1b71c71 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -55,6 +55,7 @@
> #include "hw/acpi/aml-build.h"
> #include "qapi/qapi-visit-common.h"
> #include "hw/virtio/virtio-iommu.h"
> +#include "hw/misc/riscv_iopmp.h"
>
> /* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU.
> */
> static bool virt_use_kvm_aia(RISCVVirtState *s)
> @@ -82,6 +83,7 @@ static const MemMapEntry virt_memmap[] = {
> [VIRT_UART0] ={ 0x1000, 0x100 },
> [VIRT_VIRTIO] = { 0x10001000,0x1000 },
> [VIRT_FW_CFG] = { 0x1010, 0x18 },
> +[VIRT_IOPMP] ={ 0x1020, 0x10 },
> [VIRT_FLASH] ={ 0x2000, 0x400 },
> [VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE },
> [VIRT_IMSIC_S] = { 0x2800, VIRT_IMSIC_MAX_SIZE },
> @@ -1006,6 +1008,24 @@ static void create_fdt_virtio_iommu(RISCVVirtState *s,
> uint16_t bdf)
> bdf + 1, iommu_phandle, bdf + 1, 0x - bdf);
> }
>
> +static void create_fdt_iopmp(RISCVVirtState *s, const MemMapEntry *memmap,
> + uint32_t irq_mmio_phandle) {
> +g_autofree char *name = NULL;
> +MachineState *ms = MACHINE(s);
> +
> +name = g_strdup_printf("/soc/iopmp@%lx", (long)memmap[VIRT_IOPMP].base);
> +qemu_fdt_add_subnode(ms->fdt, name);
> +qemu_fdt_setprop_string(ms->fdt, name, "compatible", "riscv_iopmp");
> +qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0x0,
> memmap[VIRT_IOPMP].base,
> +0x0, memmap[VIRT_IOPMP].size);
> +qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
> irq_mmio_phandle);
> +if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> +qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", IOPMP_IRQ);
> +} else {
> +qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", IOPMP_IRQ, 0x4);
> +}
> +}
> +
> static void finalize_fdt(RISCVVirtState *s)
> {
> uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
> @@ -1024,6 +1044,10 @@ static void finalize_fdt(RISCVVirtState *s)
> create_fdt_uart(s, virt_memmap, irq_mmio_phandle);
>
> create_fdt_rtc(s, virt_memmap, irq_mmio_phandle);
> +
> +if (s->have_iopmp) {
> +create_fdt_iopmp(s, virt_memmap, irq_mmio_phandle);
> +}
> }
>
> static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
> @@ -1404,7 +1428,7 @@ static void virt_machine_init(MachineState *machine)
> RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
> MemoryRegion *system_memory = get_system_memory();
> MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> -DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
> +DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip, *gpex_dev;
> int i, base_hartid, hart_count;
> int socket_count = riscv_socket_count(machine);
>
> @@ -1570,7 +1594,7 @@ static void virt_machine_init(MachineState *machine)
> qdev_get_gpio_in(virtio_irqchip, VIRTIO_IRQ + i));
> }
>
> -gpex_pcie_init(system_memory, pcie_irqchip, s);
> +gpex_dev = gpex_pcie_init(system_memory, pcie_irqchip, s);
>
>
Re: [PATCH v4] hw/gpio/aspeed: Add reg_table_count to AspeedGPIOClass
On 6/20/24 4:02 PM, Zheyu Ma wrote:
ASan detected a global-buffer-overflow error in the aspeed_gpio_read()
function. This issue occurred when reading beyond the bounds of the
reg_table.
To enhance the safety and maintainability of the Aspeed GPIO code, this commit
introduces a reg_table_count member to the AspeedGPIOClass structure. This
change ensures that the size of the GPIO register table is explicitly tracked
and initialized, reducing the risk of errors if new register tables are
introduced in the future.
Reproducer:
cat << EOF | qemu-system-aarch64 -display none \
-machine accel=qtest, -m 512M -machine ast1030-evb -qtest stdio
readq 0x7e780272
EOF
ASAN log indicating the issue:
==2602930==ERROR: AddressSanitizer: global-buffer-overflow on address
0x55a5da29e128 at pc 0x55a5d700dc62 bp 0x7fff096c4e90 sp 0x7fff096c4e88
READ of size 2 at 0x55a5da29e128 thread T0
#0 0x55a5d700dc61 in aspeed_gpio_read hw/gpio/aspeed_gpio.c:564:14
#1 0x55a5d933f3ab in memory_region_read_accessor system/memory.c:445:11
#2 0x55a5d92fba40 in access_with_adjusted_size system/memory.c:573:18
#3 0x55a5d92f842c in memory_region_dispatch_read1 system/memory.c:1426:16
#4 0x55a5d92f7b68 in memory_region_dispatch_read system/memory.c:1459:9
#5 0x55a5d9376ad1 in flatview_read_continue_step system/physmem.c:2836:18
#6 0x55a5d9376399 in flatview_read_continue system/physmem.c:2877:19
#7 0x55a5d93775b8 in flatview_read system/physmem.c:2907:12
Signed-off-by: Zheyu Ma
Applied to aspeed-next.
Thanks,
C.
---
Changes in v4:
- Change the variable name to 'reg_table_count'
- Change the 'reg_table_count' type to unsigned
Changes in v3:
- Add the reproducer
---
hw/gpio/aspeed_gpio.c | 17 +
include/hw/gpio/aspeed_gpio.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index c1781e2ba3..6474bb8de5 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -559,6 +559,12 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr
offset, uint32_t size)
return debounce_value;
}
+if (idx >= agc->reg_table_count) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: idx 0x%" PRIx64 " out of bounds\n",
+ __func__, idx);
+return 0;
+}
+
reg = >reg_table[idx];
if (reg->set_idx >= agc->nr_gpio_sets) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
@@ -785,6 +791,12 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset,
uint64_t data,
return;
}
+if (idx >= agc->reg_table_count) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: idx 0x%" PRIx64 " out of bounds\n",
+ __func__, idx);
+return;
+}
+
reg = >reg_table[idx];
if (reg->set_idx >= agc->nr_gpio_sets) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
@@ -1117,6 +1129,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass
*klass, void *data)
agc->nr_gpio_pins = 216;
agc->nr_gpio_sets = 7;
agc->reg_table = aspeed_3_3v_gpios;
+agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
}
static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
@@ -1127,6 +1140,7 @@ static void aspeed_gpio_2500_class_init(ObjectClass
*klass, void *data)
agc->nr_gpio_pins = 228;
agc->nr_gpio_sets = 8;
agc->reg_table = aspeed_3_3v_gpios;
+agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
}
static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
@@ -1137,6 +1151,7 @@ static void
aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_pins = 208;
agc->nr_gpio_sets = 7;
agc->reg_table = aspeed_3_3v_gpios;
+agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
}
static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
@@ -1147,6 +1162,7 @@ static void
aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_pins = 36;
agc->nr_gpio_sets = 2;
agc->reg_table = aspeed_1_8v_gpios;
+agc->reg_table_count = GPIO_1_8V_REG_ARRAY_SIZE;
}
static void aspeed_gpio_1030_class_init(ObjectClass *klass, void *data)
@@ -1157,6 +1173,7 @@ static void aspeed_gpio_1030_class_init(ObjectClass
*klass, void *data)
agc->nr_gpio_pins = 151;
agc->nr_gpio_sets = 6;
agc->reg_table = aspeed_3_3v_gpios;
+agc->reg_table_count = GPIO_3_3V_REG_ARRAY_SIZE;
}
static const TypeInfo aspeed_gpio_info = {
diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index 904eecf62c..90a12ae318 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -75,6 +75,7 @@ struct AspeedGPIOClass {
uint32_t nr_gpio_pins;
uint32_t nr_gpio_sets;
const AspeedGPIOReg *reg_table;
+unsigned reg_table_count;
};
struct AspeedGPIOState {
[PATCH v1] memory tier: consolidate the initialization of memory tiers
If we simply move the set_node_memory_tier() from memory_tier_init() to
late_initcall(), it will result in HMAT not registering the
mt_adistance_algorithm callback function, because set_node_memory_tier()
is not performed during the memory tiering initialization phase,
leading to a lack of correct default_dram information.
Therefore, we introduced a nodemask to pass the information of the
default DRAM nodes. The reason for not choosing to reuse
default_dram_type->nodes is that it is not clean enough. So in the end,
we use a __initdata variable, which is a variable that is released once
initialization is complete, including both CPU and memory nodes for HMAT
to iterate through.
Besides, since default_dram_type may be checked/used during the
initialization process of HMAT and drivers, it is better to keep the
allocation of default_dram_type in memory_tier_init().
Signed-off-by: Ho-Ren (Jack) Chuang
---
Hi all,
The current memory tier initialization process is distributed across two
different functions, memory_tier_init() and memory_tier_late_init(). This
design is hard to maintain. Thus, this patch is proposed to reduce the
possible code paths by consolidating different initialization patches into one.
The earlier discussion with Jonathan and Ying is listed here:
https://lore.kernel.org/lkml/20240405150244.4...@huawei.com/
If we want to put these two initializations together, they must be placed
together in the later function. Because only at that time, the HMAT information
will be ready, adist between nodes can be calculated, and memory tiering can be
established based on the adist. So we position the initialization at
memory_tier_init() to the memory_tier_late_init() call.
Moreover, it's natural to keep memory_tier initialization in drivers at
device_initcall() level.
This patchset is based on commits cf93be18fa1b and a72a30af550c:
[0/2]
https://lkml.kernel.org/r/20240405000707.2670063-1-horenchu...@bytedance.com
[1/2]
https://lkml.kernel.org/r/20240405000707.2670063-2-horenchu...@bytedance.com
[1/2]
https://lkml.kernel.org/r/20240405000707.2670063-3-horenchu...@bytedance.com
Thanks,
Ho-Ren (Jack) Chuang
drivers/acpi/numa/hmat.c | 4 ++-
include/linux/memory-tiers.h | 6
mm/memory-tiers.c| 70 ++--
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 2c8ccc91ebe6..31a77a3324a8 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -939,11 +939,13 @@ static int hmat_set_default_dram_perf(void)
int nid, pxm;
struct memory_target *target;
struct access_coordinate *attrs;
+ nodemask_t default_dram_nodes;
if (!default_dram_type)
return -EIO;
- for_each_node_mask(nid, default_dram_type->nodes) {
+ default_dram_nodes = mt_get_default_dram_nodemask();
+ for_each_node_mask(nid, default_dram_nodes) {
pxm = node_to_pxm(nid);
target = find_mem_target(pxm);
if (!target)
diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
index 0d70788558f4..1567db7bd40e 100644
--- a/include/linux/memory-tiers.h
+++ b/include/linux/memory-tiers.h
@@ -51,6 +51,7 @@ int mt_perf_to_adistance(struct access_coordinate *perf, int
*adist);
struct memory_dev_type *mt_find_alloc_memory_type(int adist,
struct list_head
*memory_types);
void mt_put_memory_types(struct list_head *memory_types);
+nodemask_t mt_get_default_dram_nodemask(void);
#ifdef CONFIG_MIGRATION
int next_demotion_node(int node);
void node_get_allowed_targets(pg_data_t *pgdat, nodemask_t *targets);
@@ -149,5 +150,10 @@ static inline struct memory_dev_type
*mt_find_alloc_memory_type(int adist,
static inline void mt_put_memory_types(struct list_head *memory_types)
{
}
+
+static inline nodemask_t mt_get_default_dram_nodemask(void)
+{
+ return NODE_MASK_NONE;
+}
#endif /* CONFIG_NUMA */
#endif /* _LINUX_MEMORY_TIERS_H */
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
index 6632102bd5c9..7d4b7f53dd8f 100644
--- a/mm/memory-tiers.c
+++ b/mm/memory-tiers.c
@@ -43,6 +43,7 @@ static LIST_HEAD(memory_tiers);
static LIST_HEAD(default_memory_types);
static struct node_memory_type_map node_memory_types[MAX_NUMNODES];
struct memory_dev_type *default_dram_type;
+static nodemask_t default_dram_nodes __initdata = NODE_MASK_NONE;
static const struct bus_type memory_tier_subsys = {
.name = "memory_tiering",
@@ -125,6 +126,11 @@ static inline struct memory_tier *to_memory_tier(struct
device *device)
return container_of(device, struct memory_tier, dev);
}
+nodemask_t __init mt_get_default_dram_nodemask(void)
+{
+ return default_dram_nodes;
+}
+
static __always_inline nodemask_t get_memtier_nodemask(struct memory_tier
*memtier)
{
nodemask_t nodes = NODE_MASK_NONE;
@@ -671,27 +677,38 @@
Re: [PATCH] target/riscv: Fix froundnx.h nanbox check
On Sun, Jun 9, 2024 at 3:32 PM Branislav Brzak wrote:
>
> helper_froundnx_h function mistakenly uses single percision nanbox
> check instead of the half percision one. This patch fixes the issue.
>
> Signed-off-by: Branislav Brzak
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/fpu_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 871a70a316..91b1a56d10 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -676,7 +676,7 @@ uint64_t helper_fround_h(CPURISCVState *env, uint64_t rs1)
>
> uint64_t helper_froundnx_h(CPURISCVState *env, uint64_t rs1)
> {
> -float16 frs1 = check_nanbox_s(env, rs1);
> +float16 frs1 = check_nanbox_h(env, rs1);
> frs1 = float16_round_to_int(frs1, >fp_status);
> return nanbox_h(env, frs1);
> }
> --
> 2.34.1
>
>
Re: [PATCH v3 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
On 6/16/24 23:28, Gustavo Romero wrote: Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they are not confined to use only in gdbstub.c. Signed-off-by: Gustavo Romero --- gdbstub/internals.h| 2 -- include/exec/gdbstub.h | 5 + include/gdbstub/commands.h | 6 ++ 3 files changed, 11 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] mips: pass code of conditional trap
Richard Henderson 于2024年6月21日周五 12:21写道：
>
> On 6/20/24 16:46, YunQiang Su wrote:
> > @@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
> > if (ctx->hflags != ctx->saved_hflags) {
> > tcg_gen_movi_i32(hflags, ctx->hflags);
> > }
> > -generate_exception(ctx, EXCP_TRAP);
> > +generate_exception_with_code(ctx, EXCP_TRAP, code);
> > gen_set_label(l1);
> > }
> > }
>
> There are two instances within gen_trap, one of which *does* store into
> error_code, but
> that gets overwritten by do_raise_exception_err.
>
Ohh, yes. There is another `generate_exception_end` if cond == 0.
> Search for EXCP_TRAP.
>
>
> r~
Re: [PATCH v3 6/9] target/arm: Factor out code for setting MTE TCF0 field
On 6/16/24 23:28, Gustavo Romero wrote: Factor out the code used for setting the MTE TCF0 field from the prctl code into a convenient function. Other subsystems, like gdbstub, need to set this field as well, so keep it as a separate function to avoid duplication and ensure consistency in how this field is set across the board. Signed-off-by: Gustavo Romero --- linux-user/aarch64/target_prctl.h | 22 ++--- target/arm/tcg/mte_user_helper.h | 40 +++ I'm not keen on this placement, because this is specifically linux syscall semantics. I'm not sure what the right thing to do here, because it's not like there are any other OS that support MTE at the moment, and gdb is inheriting linux's ptrace interface. I think it would be less bad if we put the header in linux-user/aarch64/ and have target/arm/gdbstub.c include that if CONFIG_USER_ONLY & CONFIG_LINUX. r~
Re: [PATCH v3 5/9] target/arm: Make some MTE helpers widely available
On 6/16/24 23:28, Gustavo Romero wrote:
@@ -287,7 +256,7 @@ uint64_t HELPER(addsubg)(CPUARMState *env, uint64_t ptr,
return address_with_allocation_tag(ptr + offset, rtag);
}
-static int load_tag1(uint64_t ptr, uint8_t *mem)
+inline int load_tag1(uint64_t ptr, uint8_t *mem)
{
int ofs = extract32(ptr, LOG2_TAG_GRANULE, 1) * 4;
return extract32(*mem, ofs, 4);
@@ -321,7 +290,7 @@ static void check_tag_aligned(CPUARMState *env, uint64_t
ptr, uintptr_t ra)
}
/* For use in a non-parallel context, store to the given nibble. */
-static void store_tag1(uint64_t ptr, uint8_t *mem, int tag)
+inline void store_tag1(uint64_t ptr, uint8_t *mem, int tag)
{
int ofs = extract32(ptr, LOG2_TAG_GRANULE, 1) * 4;
*mem = deposit32(*mem, ofs, 4, tag);
Move these two entirely to the header as static inline.
In general, inline without static doesn't mean what you think.
With that,
Reviewed-by: Richard Henderson
diff --git a/target/arm/tcg/mte_helper.h b/target/arm/tcg/mte_helper.h
new file mode 100644
index 00..69ad8457f8
--- /dev/null
+++ b/target/arm/tcg/mte_helper.h
@@ -0,0 +1,63 @@
+/*
+ * ARM MemTag operation helpers.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef TARGET_ARM_MTE_H
+#define TARGET_ARM_MTE_H
+
+/**
+ * allocation_tag_mem_probe:
+ * @env: the cpu environment
+ * @ptr_mmu_idx: the addressing regime to use for the virtual address
+ * @ptr: the virtual address for which to look up tag memory
+ * @ptr_access: the access to use for the virtual address
+ * @ptr_size: the number of bytes in the normal memory access
+ * @tag_access: the access to use for the tag memory
+ * @probe: true to merely probe, never taking an exception
+ * @ra: the return address for exception handling
+ *
+ * Our tag memory is formatted as a sequence of little-endian nibbles.
+ * That is, the byte at (addr >> (LOG2_TAG_GRANULE + 1)) contains two
+ * tags, with the tag at [3:0] for the lower addr and the tag at [7:4]
+ * for the higher addr.
+ *
+ * Here, resolve the physical address from the virtual address, and return
+ * a pointer to the corresponding tag byte.
+ *
+ * If there is no tag storage corresponding to @ptr, return NULL.
+ *
+ * If the page is inaccessible for @ptr_access, or has a watchpoint, there are
+ * three options:
+ * (1) probe = true, ra = 0 : pure probe -- we return NULL if the page is not
+ * accessible, and do not take watchpoint traps. The calling code must
+ * handle those cases in the right priority compared to MTE traps.
+ * (2) probe = false, ra = 0 : probe, no fault expected -- the caller
guarantees
+ * that the page is going to be accessible. We will take watchpoint traps.
+ * (3) probe = false, ra != 0 : non-probe -- we will take both memory access
+ * traps and watchpoint traps.
+ * (probe = true, ra != 0 is invalid and will assert.)
+ */
+uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx,
+ uint64_t ptr, MMUAccessType ptr_access,
+ int ptr_size, MMUAccessType tag_access,
+ bool probe, uintptr_t ra);
+/**
+ * load_tag1 - Load 1 tag (nibble) from byte
+ * @ptr: The tagged address
+ * @mem: The tag address (packed, 2 tags in byte)
+ */
+int load_tag1(uint64_t ptr, uint8_t *mem);
+
+/**
+ * store_tag1 - Store 1 tag (nibble) into byte
+ * @ptr: The tagged address
+ * @mem: The tag address (packed, 2 tags in byte)
+ * @tag: The tag to be stored in the nibble
+ */
+void store_tag1(uint64_t ptr, uint8_t *mem, int tag);
+
+#endif /* TARGET_ARM_MTE_H */
Re: [PATCH v3 4/9] target/arm: Fix exception case in allocation_tag_mem_probe
On 6/16/24 23:28, Gustavo Romero wrote: If page in 'ptr_access' is inaccessible and probe is 'true' allocation_tag_mem_probe should not throw an exception, but currently it does, so fix it. Signed-off-by: Gustavo Romero Reviewed-by: Alex Bennée --- target/arm/tcg/mte_helper.c | 3 +++ 1 file changed, 3 insertions(+) Oops. Reviewed-by: Richard Henderson r~
Re: [PATCH v3 3/9] gdbstub: Add support for target-specific stubs
On 6/16/24 23:28, Gustavo Romero wrote:
+static char *extended_qsupported_features;
+void gdb_extend_qsupported_features(char *qsupported_features)
+{
+extended_qsupported_features = qsupported_features;
+}
Assert these functions aren't called twice.
That should be good enough until we need something more complicated.
Speaking of more complicated... do we have a plan for gdb when we get to the point where
the board contains multiple cpu types? Not yet, of course, but we're working on it...
r~
Re: [PATCH] target/riscv: Fix froundnx.h nanbox check
On 6/8/24 14:45, Branislav Brzak wrote:
t/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index 871a70a316..91b1a56d10 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -676,7 +676,7 @@ uint64_t helper_fround_h(CPURISCVState *env, uint64_t rs1)
uint64_t helper_froundnx_h(CPURISCVState *env, uint64_t rs1)
{
-float16 frs1 = check_nanbox_s(env, rs1);
+float16 frs1 = check_nanbox_h(env, rs1);
frs1 = float16_round_to_int(frs1, >fp_status);
return nanbox_h(env, frs1);
}
Reviewed-by: Richard Henderson
r~
Re: [PATCH] mips: pass code of conditional trap
On 6/20/24 16:46, YunQiang Su wrote:
@@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
if (ctx->hflags != ctx->saved_hflags) {
tcg_gen_movi_i32(hflags, ctx->hflags);
}
-generate_exception(ctx, EXCP_TRAP);
+generate_exception_with_code(ctx, EXCP_TRAP, code);
gen_set_label(l1);
}
}
There are two instances within gen_trap, one of which *does* store into error_code, but
that gets overwritten by do_raise_exception_err.
Search for EXCP_TRAP.
r~
Re: [PATCH] target/riscv: Fix froundnx.h nanbox check
On Sun, Jun 9, 2024 at 3:32 PM Branislav Brzak wrote:
>
> helper_froundnx_h function mistakenly uses single percision nanbox
> check instead of the half percision one. This patch fixes the issue.
>
> Signed-off-by: Branislav Brzak
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/fpu_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 871a70a316..91b1a56d10 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -676,7 +676,7 @@ uint64_t helper_fround_h(CPURISCVState *env, uint64_t rs1)
>
> uint64_t helper_froundnx_h(CPURISCVState *env, uint64_t rs1)
> {
> -float16 frs1 = check_nanbox_s(env, rs1);
> +float16 frs1 = check_nanbox_h(env, rs1);
> frs1 = float16_round_to_int(frs1, >fp_status);
> return nanbox_h(env, frs1);
> }
> --
> 2.34.1
>
>
Re: [PATCH v2 2/6] target/riscv: Introduce extension implied rule helpers
On Sun, Jun 16, 2024 at 12:48 PM wrote:
>
> From: Frank Chang
>
> Introduce helpers to enable the extensions based on the implied rules.
> The implied extensions are enabled recursively, so we don't have to
> expand all of them manually. This also eliminates the old-fashioned
> ordering requirement. For example, Zvksg implies Zvks, Zvks implies
> Zvksed, etc., removing the need to check the implied rules of Zvksg
> before Zvks.
>
> Signed-off-by: Frank Chang
> Reviewed-by: Jerry Zhang Jian
> Tested-by: Max Chou
> ---
> target/riscv/tcg/tcg-cpu.c | 91 ++
> 1 file changed, 91 insertions(+)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index eb6f7b9d12..f8d6371764 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -36,6 +36,9 @@
> static GHashTable *multi_ext_user_opts;
> static GHashTable *misa_ext_user_opts;
>
> +static GHashTable *misa_implied_rules;
> +static GHashTable *ext_implied_rules;
> +
> static bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
> {
> return g_hash_table_contains(multi_ext_user_opts,
> @@ -836,11 +839,97 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu)
> }
> }
>
> +static void riscv_cpu_init_implied_exts_rules(void)
> +{
> +RISCVCPUImpliedExtsRule *rule;
> +int i;
> +
> +for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
> +g_hash_table_insert(misa_implied_rules, GUINT_TO_POINTER(rule->ext),
> +(gpointer)rule);
> +}
> +
> +for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
> +g_hash_table_insert(ext_implied_rules, GUINT_TO_POINTER(rule->ext),
> +(gpointer)rule);
> +}
> +}
> +
> +static void cpu_enable_implied_rule(RISCVCPU *cpu,
> +RISCVCPUImpliedExtsRule *rule)
> +{
> +CPURISCVState *env = >env;
> +RISCVCPUImpliedExtsRule *ir;
> +bool enabled = false;
> +int i;
> +
> +#ifndef CONFIG_USER_ONLY
> +enabled = qatomic_read(>enabled) & BIT_ULL(cpu->env.mhartid);
enabled is a uint64_t, so this limits us to 64 harts right?
The virt machine currently has a limit of 512, so this won't work right?
Alistair
> +#endif
> +
> +if (!enabled) {
> +/* Enable the implied MISAs. */
> +if (rule->implied_misas) {
> +riscv_cpu_set_misa_ext(env, env->misa_ext | rule->implied_misas);
> +
> +for (i = 0; misa_bits[i] != 0; i++) {
> +if (rule->implied_misas & misa_bits[i]) {
> +ir = g_hash_table_lookup(misa_implied_rules,
> + GUINT_TO_POINTER(misa_bits[i]));
> +
> +if (ir) {
> +cpu_enable_implied_rule(cpu, ir);
> +}
> +}
> +}
> +}
> +
> +/* Enable the implied extensions. */
> +for (i = 0; rule->implied_exts[i] != RISCV_IMPLIED_EXTS_RULE_END;
> i++) {
> +cpu_cfg_ext_auto_update(cpu, rule->implied_exts[i], true);
> +
> +ir = g_hash_table_lookup(ext_implied_rules,
> +
> GUINT_TO_POINTER(rule->implied_exts[i]));
> +
> +if (ir) {
> +cpu_enable_implied_rule(cpu, ir);
> +}
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +qatomic_or(>enabled, BIT_ULL(cpu->env.mhartid));
> +#endif
> +}
> +}
> +
> +static void riscv_cpu_enable_implied_rules(RISCVCPU *cpu)
> +{
> +RISCVCPUImpliedExtsRule *rule;
> +int i;
> +
> +/* Enable the implied MISAs. */
> +for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
> +if (riscv_has_ext(>env, rule->ext)) {
> +cpu_enable_implied_rule(cpu, rule);
> +}
> +}
> +
> +/* Enable the implied extensions. */
> +for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
> +if (isa_ext_is_enabled(cpu, rule->ext)) {
> +cpu_enable_implied_rule(cpu, rule);
> +}
> +}
> +}
> +
> void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> {
> CPURISCVState *env = >env;
> Error *local_err = NULL;
>
> +riscv_cpu_init_implied_exts_rules();
> +riscv_cpu_enable_implied_rules(cpu);
> +
> riscv_cpu_validate_misa_priv(env, _err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> @@ -1346,6 +1435,8 @@ static void riscv_tcg_cpu_instance_init(CPUState *cs)
>
> misa_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
> multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
> +misa_implied_rules = g_hash_table_new(NULL, g_direct_equal);
> +ext_implied_rules = g_hash_table_new(NULL, g_direct_equal);
> riscv_cpu_add_user_properties(obj);
>
> if (riscv_cpu_has_max_extensions(obj)) {
> --
> 2.43.2
>
>
Re: [PATCH v3 qemu 00/11] acpi: NUMA nodes for CXL HB as GP + complex NUMA test
Hi, Jonathan, Jonathan Cameron writes: > v3: Thanks to Richard for help debugging BE issue and to Igor for > finding a bunch of other thing to improve via the context in > the fix patch. > > - Fix the big endian host/little endian guest issue in the HID being > written to the Generic Port Affinity Structure ACPI Device Handle. > - Fix a bug in the ordering of bus vs devfn in the BDF field which is > reversed in the ACPI table wrt to QEMU's internal handling. Note the > fix is minimal and refactored later in the series. > - Move original GI code to hw/acpi/aml-build.c and hw/acpi/pc.c as > no need for a separate file and this keeps the SRAT entry building > all in one place. > - Use properties for the pci bus number and the ACPI UID to avoid > using pci internal implementation details in hw/acpi. > - Drop the GenericNode base object as much less code is unified with > the new approach to the aml building and that approach did not bring > sufficient advantages to be worthwhile after other refactors. > A little more duplication occurs in v3 but the code is easier to read. > > ACPI 6.5 introduced Generic Port Affinity Structures to close a system > description gap that was a problem for CXL memory systems. > It defines an new SRAT Affinity structure (and hence allows creation of an > ACPI Proximity Node which can only be defined via an SRAT structure) > for the boundary between a discoverable fabric and a non discoverable > system interconnects etc. > > The HMAT data on latency and bandwidth is combined with discoverable > information from the CXL bus (link speeds, lane counts) and CXL devices > (switch port to port characteristics and USP to memory, via CDAT tables > read from the device). QEMU has supported the rest of the elements > of this chain for a while but now the kernel has caught up and we need > the missing element of Generic Ports (this code has been used extensively > in testing and debugging that kernel support, some resulting fixes > currently under review). > > Generic Port Affinity Structures are very similar to the recently > added Generic Initiator Affinity Structures (GI) so this series > factors out and reuses much of that infrastructure for reuse > There are subtle differences (beyond the obvious structure ID change). > > - The ACPI spec example (and linux kernel support) has a Generic > Port not as associated with the CXL root port, but rather with > the CXL Host bridge. As a result, an ACPI handle is used (rather > than the PCI SBDF option for GIs). In QEMU the easiest way > to get to this is to target the root bridge PCI Bus, and > conveniently the root bridge bus number is used for the UID allowing > us to construct an appropriate entry. > > A key addition of this series is a complex NUMA topology example that > stretches the QEMU emulation code for GI, GP and nodes with just > CPUS, just memory, just hot pluggable memory, mixture of memory and CPUs. > > A similar test showed up a few NUMA related bugs with fixes applied for > 9.0 (note that one of these needs linux booted to identify that it > rejects the HMAT table and this test is a regression test for the > table generation only). > > https://lore.kernel.org/qemu-devel/2eb6672cfdaea7dacd8e9bb0523887f13b9f85ce.1710282274.git@redhat.com/ > https://lore.kernel.org/qemu-devel/74e2845c5f95b0c139c79233ddb65bb17f2dd679.1710282274.git@redhat.com/ When developing the Linux kernel patchset "[PATCH v3 0/3] cxl/region: Support to calculate memory tier abstract distance" as in [1]. [1] https://lore.kernel.org/linux-cxl/20240618084639.1419629-1-ying.hu...@intel.com/ I use this patchset to test my kernel patchset and it works great! Thanks! Feel free to add my Tested-by: "Huang, Ying" in the future versions. > > Jonathan Cameron (11): > hw/acpi: Fix ordering of BDF in Generic Initiator PCI Device Handle. > hw/acpi/GI: Fix trivial parameter alignment issue. > hw/acpi: Move AML building code for Generic Initiators to aml_build.c > hw/acpi: Rename build_all_acpi_generic_initiators() to > build_acpi_generic_initiator() > hw/pci: Add a bus property to pci_props and use for acpi/gi > acpi/pci: Move Generic Initiator object handling into acpi/pci.* > hw/pci-bridge: Add acpi_uid property to CXL PXB > hw/acpi: Generic Port Affinity Structure support > bios-tables-test: Allow for new acpihmat-generic-x test data. > bios-tables-test: Add complex SRAT / HMAT test for GI GP > bios-tables-test: Add data for complex numa test (GI, GP etc) > > qapi/qom.json | 34 +++ > include/hw/acpi/acpi_generic_initiator.h| 30 +-- > include/hw/acpi/aml-build.h | 8 + > include/hw/acpi/pci.h | 7 + > include/hw/pci/pci_bridge.h | 1 + > hw/acpi/acpi_generic_initiator.c| 132 +--- > hw/acpi/aml-build.c | 84 > hw/acpi/pci.c
Re: Failed to hot-plug device to pxb bridge
Sorry for not replying earlier, I have been busy with other things.
> > Hi Igor, Daniel and all:
> >
> >
> > https://lore.kernel.org/all/20220422135101.65796...@redhat.com/t/#r831d589f243c24334a09995620b74408847a87a0
> >
> > This message discuss hotplug device to pxb bridge. At the end, Igor
> > suggested enable shpc on pxb bridge:
> > pxb_dev_realize_common():
> > qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, true);
>
>
> you can try check if hotplug gets to shpc_device_plug_cb() and what it does
> there
> if it gets to the end but you don't get any reaction from guest OS
> it might be a guest issue.
I checked the guest dmesg and found that failed load shpc driver.
[0.98] shpchp :00:05.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PCI0.S28_)
[0.968238] shpchp :00:05.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PCI0)
[0.969160] shpchp :00:05.0: Cannot get control of SHPC hotplug
[0.969876] shpchp :00:06.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PCI0.S30_)
[0.971454] shpchp :00:06.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PCI0)
[0.972376] shpchp :00:06.0: Cannot get control of SHPC hotplug
[0.973119] shpchp :80:00.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PC80)
[0.974674] shpchp :80:00.0: Cannot get control of SHPC hotplug
[0.979422] shpchp :81:01.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PC80)
[0.980948] shpchp :81:01.0: Cannot get control of SHPC hotplug
[0.981685] shpchp :60:00.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PC60)
[0.994623] shpchp :60:00.0: Cannot get control of SHPC hotplug
[0.995349] shpchp :61:01.0: Requesting control of SHPC hotplug via
OSHP (\_SB_.PC60)
[0.996891] shpchp :61:01.0: Cannot get control of SHPC hotplug
[0.997626] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
Read the shpc driver code, I found that before shpc_init it need check the shpc
capability of bridge(shpc_capable) and
get hotplug control from firmware(acpi_get_hp_hw_control_from_firmware) in
shpc_probe.
Howerver it return fail in acpi_get_hp_hw_control_from_firmware. I dump the
acpid table, not found OSC and OSHP
method in dsdt.dsl.
In the QEMU build_dsdt, not found build osc method in i440fx.
Putting aside this pxb scenario, I suspect that after disable
acpi-pci-hotplug-with-bridge-support,
hot-plug into the PCI bridge via SHPC can be successful in i440fx ?
> > Howerver, I tried it and the guest can't find the device on pxb bridge. Add
> > some log, QEMU use hotplug handler shpc_device_plug_cb.
> >
> > QEMU command line:
> > -device pxb,bus_nr=96,id=pci.1,numa_node=0,bus=pci.0,addr=0x3
> > -device pxb,bus_nr=128,id=pci.2,numa_node=1,bus=pci.0,addr=0x4
> > -device pci-bridge,chassis_nr=3,id=pci.3,bus=pci.0,addr=0x5
> > -device pci-bridge,chassis_nr=4,id=pci.4,bus=pci.0,addr=0x6
> > -device pci-bridge,chassis_nr=5,id=pci.5,bus=pci.1,addr=0x1
> > -device pci-bridge,chassis_nr=6,id=pci.6,bus=pci.2,addr=0x1
> >
> > Hotplug command:
> > {"execute":"human-monitor-command","arguments":{"command-line":"drive_add
> > dummy
> > file=/home/test/data1.img,format=qcow2,if=none,id=drive-virtio-disk1,cache=none"}}
> > {"execute":"device_add","arguments":{"driver":"virtio-blk-pci","scsi":"off","bus":"pci.5","addr":"0x1","drive":"drive-virtio-disk1","id":"virtio-disk1"}}
> >
> > Some info in the guest:
> > [root@localhost ~]# lspci -tv
> > -+-[:80]---00.0-[81-82]01.0-[82]--
> > +-[:60]---00.0-[61-62]01.0-[62]--
> > \-[:00]-+-00.0 Intel Corporation 440FX - 82441FX PMC [Natoma]
> > +-01.0 Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
> > +-01.1 Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> > +-01.2 Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II]
> > +-01.3 Intel Corporation 82371AB/EB/MB PIIX4 ACPI
> > +-02.0 Cirrus Logic GD 5446
> > +-03.0 Red Hat, Inc. QEMU PCI Expander bridge
> > +-04.0 Red Hat, Inc. QEMU PCI Expander bridge
> > +-05.0-[01]--
> > +-06.0-[02]--
> > +-07.0 Red Hat, Inc. Virtio network device
> > +-08.0 Red Hat, Inc. Virtio block device
> > \-09.0 Red Hat, Inc. Virtio memory balloon
> > [root@localhost boot]# grep -i 'shpc' config-3.10.0-1160.83.1.el7.x86_64
> > CONFIG_HOTPLUG_PCI_SHPC=y
> > [root@localhost boot]# uname -r
> > 3.10.0-1160.83.1.el7.x86_64
> >
> > Thanks.
> >
> >
>
Re: [PATCH] hw/riscv/virt.c: Make block devices default to virtio
On Thu, Jun 20, 2024 at 4:48 PM Sunil V L wrote: > > RISC-V virt is currently missing default type for block devices. Without > this being set, proper backend is not created when option like -cdrom > is used. So, make the virt board's default block device type be > IF_VIRTIO similar to other architectures. > > We also need to set no_cdrom to avoid getting a default cdrom device. > > Signed-off-by: Sunil V L Thanks! Applied to riscv-to-apply.next Alistair > --- > hw/riscv/virt.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 8675c3a7d1..b0871b7f81 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1764,6 +1764,8 @@ static void virt_machine_class_init(ObjectClass *oc, > void *data) > mc->init = virt_machine_init; > mc->max_cpus = VIRT_CPUS_MAX; > mc->default_cpu_type = TYPE_RISCV_CPU_BASE; > +mc->block_default_type = IF_VIRTIO; > +mc->no_cdrom = 1; > mc->pci_allow_0_address = true; > mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids; > mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props; > -- > 2.34.1 > >
Re: [PATCH] hw/riscv/virt.c: Make block devices default to virtio
On Thu, Jun 20, 2024 at 4:48 PM Sunil V L wrote: > > RISC-V virt is currently missing default type for block devices. Without > this being set, proper backend is not created when option like -cdrom > is used. So, make the virt board's default block device type be > IF_VIRTIO similar to other architectures. > > We also need to set no_cdrom to avoid getting a default cdrom device. > > Signed-off-by: Sunil V L Reviewed-by: Alistair Francis Alistair > --- > hw/riscv/virt.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 8675c3a7d1..b0871b7f81 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1764,6 +1764,8 @@ static void virt_machine_class_init(ObjectClass *oc, > void *data) > mc->init = virt_machine_init; > mc->max_cpus = VIRT_CPUS_MAX; > mc->default_cpu_type = TYPE_RISCV_CPU_BASE; > +mc->block_default_type = IF_VIRTIO; > +mc->no_cdrom = 1; > mc->pci_allow_0_address = true; > mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids; > mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props; > -- > 2.34.1 > >
Re: [PATCH] mips: pass code of conditional trap
Maciej W. Rozycki 于2024年6月21日周五 08:41写道： > > On Fri, 21 Jun 2024, YunQiang Su wrote: > > > Linux and We use the code of conditional trap instructions to emit > > signals other than simple SIGTRAP. Currently, code 6 (overflow), > > 7 (div by zero) are supported. It means that if code 7 is used with > > a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit. > > > > But when `gen_trap` we didn't pass the code as we use `generate_exception`, > > which has no info about the code. Let's introduce a new function > > `generate_exception_code` for it. > > I haven't touched this stuff for ages, but AFAICT the code is already > passed where applicable via the environment for `do_tr_or_bp' to handle, > so I can't understand why your change is needed. > The error_code in env is always zero, as we need to set it here. > What problem are you trying to solve? > See the talk in GCC mailing list about testsuite/ubsan/overflow-div-3.c Qemu emits SIGTRAP instead of SIGFPE, due to it didn't initialize the code of conditional trap to env. > Maciej
Re: [PATCH v3 1/9] gdbstub: Clean up process_string_cmd
On 6/16/24 23:28, Gustavo Romero wrote: Change 'process_string_cmd' to return true on success and false on failure, instead of 0 and -1. Signed-off-by: Gustavo Romero Reviewed-by: Alex Bennée --- gdbstub/gdbstub.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] mips: pass code of conditional trap
On Fri, 21 Jun 2024, YunQiang Su wrote: > Linux and We use the code of conditional trap instructions to emit > signals other than simple SIGTRAP. Currently, code 6 (overflow), > 7 (div by zero) are supported. It means that if code 7 is used with > a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit. > > But when `gen_trap` we didn't pass the code as we use `generate_exception`, > which has no info about the code. Let's introduce a new function > `generate_exception_code` for it. I haven't touched this stuff for ages, but AFAICT the code is already passed where applicable via the environment for `do_tr_or_bp' to handle, so I can't understand why your change is needed. What problem are you trying to solve? Maciej
Re: [PATCH v4] hw/gpio/aspeed: Add reg_table_count to AspeedGPIOClass
On Thu, 2024-06-20 at 16:02 +0200, Zheyu Ma wrote: > ASan detected a global-buffer-overflow error in the aspeed_gpio_read() > function. This issue occurred when reading beyond the bounds of the > reg_table. > > To enhance the safety and maintainability of the Aspeed GPIO code, this commit > introduces a reg_table_count member to the AspeedGPIOClass structure. This > change ensures that the size of the GPIO register table is explicitly tracked > and initialized, reducing the risk of errors if new register tables are > introduced in the future. > > Reproducer: > cat << EOF | qemu-system-aarch64 -display none \ > -machine accel=qtest, -m 512M -machine ast1030-evb -qtest stdio > readq 0x7e780272 > EOF > > ASAN log indicating the issue: > ==2602930==ERROR: AddressSanitizer: global-buffer-overflow on address > 0x55a5da29e128 at pc 0x55a5d700dc62 bp 0x7fff096c4e90 sp 0x7fff096c4e88 > READ of size 2 at 0x55a5da29e128 thread T0 > #0 0x55a5d700dc61 in aspeed_gpio_read hw/gpio/aspeed_gpio.c:564:14 > #1 0x55a5d933f3ab in memory_region_read_accessor system/memory.c:445:11 > #2 0x55a5d92fba40 in access_with_adjusted_size system/memory.c:573:18 > #3 0x55a5d92f842c in memory_region_dispatch_read1 system/memory.c:1426:16 > #4 0x55a5d92f7b68 in memory_region_dispatch_read system/memory.c:1459:9 > #5 0x55a5d9376ad1 in flatview_read_continue_step system/physmem.c:2836:18 > #6 0x55a5d9376399 in flatview_read_continue system/physmem.c:2877:19 > #7 0x55a5d93775b8 in flatview_read system/physmem.c:2907:12 > > Signed-off-by: Zheyu Ma Reviewed-by: Andrew Jeffery
[PATCH] mips: pass code of conditional trap
Linux and We use the code of conditional trap instructions to emit
signals other than simple SIGTRAP. Currently, code 6 (overflow),
7 (div by zero) are supported. It means that if code 7 is used with
a conditional trap instruction, a SIGFPE instead of SIGTRAP will emit.
But when `gen_trap` we didn't pass the code as we use `generate_exception`,
which has no info about the code. Let's introduce a new function
`generate_exception_code` for it.
---
target/mips/tcg/translate.c | 8 +++-
target/mips/tcg/translate.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 333469b268..e680a1c2f2 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1353,6 +1353,12 @@ void generate_exception(DisasContext *ctx, int excp)
gen_helper_raise_exception(tcg_env, tcg_constant_i32(excp));
}
+void generate_exception_with_code(DisasContext *ctx, int excp, int code)
+{
+gen_helper_raise_exception_err(tcg_env, tcg_constant_i32(excp),
+ tcg_constant_i32(code));
+}
+
void generate_exception_end(DisasContext *ctx, int excp)
{
generate_exception_err(ctx, excp, 0);
@@ -4553,7 +4559,7 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
if (ctx->hflags != ctx->saved_hflags) {
tcg_gen_movi_i32(hflags, ctx->hflags);
}
-generate_exception(ctx, EXCP_TRAP);
+generate_exception_with_code(ctx, EXCP_TRAP, code);
gen_set_label(l1);
}
}
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 2b6646b339..e3d544b478 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -134,6 +134,7 @@ enum {
} while (0)
void generate_exception(DisasContext *ctx, int excp);
+void generate_exception_with_code(DisasContext *ctx, int excp, int code);
void generate_exception_err(DisasContext *ctx, int excp, int err);
void generate_exception_end(DisasContext *ctx, int excp);
void generate_exception_break(DisasContext *ctx, int code);
--
2.39.3 (Apple Git-146)
[RFC PATCH 0/4] hw/display/virtio-gpu: Introducing asynchronous guest display render
From: Dongwon Kim Introducing new virtio-gpu param, 'render_sync' when guest scanout blob is used (blob=true). The new param is used to specify when to start rendering a guest scanout frame. By default (and so far) rendering of the guest frame is started in the draw event to make sure guest display update is sychronized with host's vsync. But this method inevitably brings some extra wait because most of time, the draw event is not happening right after the guest scanout frame is flushed. This delay often makes the guest stuck at certain frame for too long and causes general performance degradation of graphic workloads on the guest's side especially when the display update rate is high. This unwanted perf drop can be reduced if the guest scanout frame is rendered as soon as it is flushed through 'VIRTIO_GPU_CMD_RESOURCE_FLUSH' msg. The gl display pipeline can be unblocked a lot earlier in this case so that guest can move to the next display frame right away. However, this "asynchrounous" render mode may cause some waste of resources as the guest could produce more frames than what are actually displayed on the host display. This is similar as running rendering apps with no vblank or vsync option. This is why this feature should stay as optional. The param 'render_sync' is set to 'true' by default and this is in line with traditional way while setting it to 'false' is basically enabling this asynchronouse mode. Dongwon Kim (4): hw/display/virtio-gpu: Introducing render_sync param ui/egl-helpers: Consolidates create-sync and create-fence ui/gtk-egl: Start rendering of guest blob scanout if render_sync is off ui/gtk-gl-draw: Start rendering of guest blob scanout if render_sync is off include/hw/virtio/virtio-gpu.h | 3 ++ include/ui/dmabuf.h | 4 +- include/ui/egl-helpers.h| 3 +- hw/display/vhost-user-gpu.c | 3 +- hw/display/virtio-gpu-udmabuf.c | 3 +- hw/display/virtio-gpu.c | 2 + hw/vfio/display.c | 3 +- ui/dbus-listener.c | 2 +- ui/dmabuf.c | 11 +++- ui/egl-helpers.c| 27 -- ui/gtk-egl.c| 93 ++--- ui/gtk-gl-area.c| 90 +++ 12 files changed, 146 insertions(+), 98 deletions(-) -- 2.34.1
[RFC PATCH 3/4] ui/gtk-egl: Start rendering of guest blob scanout if render_sync is off
From: Dongwon Kim
Draw (executing glBlitFramebuffer) immediately as soon as the frame
is flushed instead of getting it done in the next draw event if render_sync
flag is reset. With this, the fence will be signaled way ealier so the guest
can be working on the next frame right away.
Cc: Gerd Hoffmann
Cc: Marc-André Lureau
Cc: Vivek Kasireddy
Signed-off-by: Dongwon Kim
---
ui/gtk-egl.c | 88 ++--
1 file changed, 51 insertions(+), 37 deletions(-)
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 55199f8659..2877140c3b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -80,6 +80,12 @@ void gd_egl_draw(VirtualConsole *vc)
ww = gdk_window_get_width(window) * ws;
wh = gdk_window_get_height(window) * ws;
+vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
+vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
+
+eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
+ vc->gfx.esurface, vc->gfx.ectx);
+
if (vc->gfx.scanout_mode) {
#ifdef CONFIG_GBM
if (dmabuf) {
@@ -88,21 +94,9 @@ void gd_egl_draw(VirtualConsole *vc)
} else {
qemu_dmabuf_set_draw_submitted(dmabuf, false);
}
-}
-#endif
-gd_egl_scanout_flush(>gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
-vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
-vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
-
-glFlush();
-#ifdef CONFIG_GBM
-if (dmabuf) {
-fence_fd = egl_dmabuf_create_fence(dmabuf);
-if (fence_fd >= 0) {
-qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
-} else {
-graphic_hw_gl_block(vc->gfx.dcl.con, false);
+if (qemu_dmabuf_get_render_sync(dmabuf)) {
+gd_egl_scanout_flush(>gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
}
}
#endif
@@ -110,19 +104,12 @@ void gd_egl_draw(VirtualConsole *vc)
if (!vc->gfx.ds) {
return;
}
-eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
- vc->gfx.esurface, vc->gfx.ectx);
-
surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
-
-eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
-
-vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
-vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
-
-glFlush();
}
+
+eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
+glFlush();
}
void gd_egl_update(DisplayChangeListener *dcl,
@@ -146,14 +133,20 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+#ifdef CONFIG_GBM
+QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+#endif
+
gd_update_monitor_refresh_rate(
vc, vc->window ? vc->window : vc->gfx.drawing_area);
-if (vc->gfx.guest_fb.dmabuf &&
-qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
+#ifdef CONFIG_GBM
+if (dmabuf && qemu_dmabuf_get_draw_submitted(dmabuf) &&
+qemu_dmabuf_get_render_sync(dmabuf)) {
gd_egl_draw(vc);
return;
}
+#endif
if (!vc->gfx.esurface) {
gd_egl_init(vc);
@@ -166,9 +159,9 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
surface_gl_create_texture(vc->gfx.gls, vc->gfx.ds);
}
#ifdef CONFIG_GBM
-if (vc->gfx.guest_fb.dmabuf) {
-egl_dmabuf_release_texture(vc->gfx.guest_fb.dmabuf);
-gd_egl_scanout_dmabuf(dcl, vc->gfx.guest_fb.dmabuf);
+if (dmabuf) {
+egl_dmabuf_release_texture(dmabuf);
+gd_egl_scanout_dmabuf(dcl, dmabuf);
}
#endif
}
@@ -344,6 +337,11 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
return;
}
if (!vc->gfx.guest_fb.framebuffer) {
+#ifdef CONFIG_GBM
+if (dmabuf) {
+graphic_hw_gl_block(vc->gfx.dcl.con, false);
+}
+#endif
return;
}
@@ -366,7 +364,16 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
egl_fb_blit(>gfx.win_fb, >gfx.guest_fb, !vc->gfx.y0_top);
}
-eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
+#ifdef CONFIG_GBM
+if (dmabuf) {
+fence_fd = egl_dmabuf_create_fence(dmabuf);
+if (fence_fd >= 0) {
+qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
+} else {
+graphic_hw_gl_block(vc->gfx.dcl.con, false);
+}
+}
+#endif
}
void gd_egl_flush(DisplayChangeListener *dcl,
@@ -374,15 +381,22 @@ void gd_egl_flush(DisplayChangeListener *dcl,
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
GtkWidget *area = vc->gfx.drawing_area;
-
-if (vc->gfx.guest_fb.dmabuf &&
-!qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
-
[RFC PATCH 1/4] hw/display/virtio-gpu: Introducing render_sync param
From: Dongwon Kim
Introducing new virtio-gpu param, 'render_sync' when guest scanout blob
is used (blob=true). The new param is used to specify when to start
rendering a guest scanout frame.
By default (and so far) rendering of the guest frame is started in
the draw event to make sure guest display update is sychronized with
host's vsync. But this method inevitably brings some extra wait because
most of time, the draw event is not happening right after the guest
scanout frame is flushed.
This delay often makes the guest stuck at certain frame for too long and
causes general performance degradation of graphic workloads on the guest's
side especially when the display update rate is high. This unwanted perf
drop can be reduced if the guest scanout frame is rendered as soon as it is
flushed through 'VIRTIO_GPU_CMD_RESOURCE_FLUSH' msg. The gl display
pipeline can be unblocked a lot earlier in this case so that guest can
move to the next display frame right away.
However, this "asynchrounous" render mode may cause some waste of resources
as the guest could produce more frames than what are actually displayed
on the host display. This is similar as running rendering apps with no vblank
or vsync option. This is why this feature should stay as optional.
The param 'render_sync' is set to 'true' by default and this is in line
with traditional way while setting it to 'false' is basically enabling
this asynchronouse mode.
Cc: Gerd Hoffmann
Cc: Marc-André Lureau
Cc: Vivek Kasireddy
Signed-off-by: Dongwon Kim
---
include/hw/virtio/virtio-gpu.h | 3 +++
include/ui/dmabuf.h | 4 +++-
hw/display/vhost-user-gpu.c | 3 ++-
hw/display/virtio-gpu-udmabuf.c | 3 ++-
hw/display/virtio-gpu.c | 2 ++
hw/vfio/display.c | 3 ++-
ui/dbus-listener.c | 2 +-
ui/dmabuf.c | 11 ++-
8 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7a59379f5a..9bcc572eab 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -97,6 +97,7 @@ enum virtio_gpu_base_conf_flags {
VIRTIO_GPU_FLAG_EDID_ENABLED,
VIRTIO_GPU_FLAG_DMABUF_ENABLED,
VIRTIO_GPU_FLAG_BLOB_ENABLED,
+VIRTIO_GPU_FLAG_RENDER_SYNC_ENABLED,
VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
};
@@ -111,6 +112,8 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
#define virtio_gpu_blob_enabled(_cfg) \
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_render_sync_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_RENDER_SYNC_ENABLED))
#define virtio_gpu_context_init_enabled(_cfg) \
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
#define virtio_gpu_rutabaga_enabled(_cfg) \
diff --git a/include/ui/dmabuf.h b/include/ui/dmabuf.h
index dc74ba895a..45384e32e3 100644
--- a/include/ui/dmabuf.h
+++ b/include/ui/dmabuf.h
@@ -17,7 +17,8 @@ QemuDmaBuf *qemu_dmabuf_new(uint32_t width, uint32_t height,
uint32_t y, uint32_t backing_width,
uint32_t backing_height, uint32_t fourcc,
uint64_t modifier, int dmabuf_fd,
-bool allow_fences, bool y0_top);
+bool allow_fences, bool y0_top,
+bool render_sync);
void qemu_dmabuf_free(QemuDmaBuf *dmabuf);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuDmaBuf, qemu_dmabuf_free);
@@ -40,6 +41,7 @@ void *qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf);
int32_t qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf);
bool qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf);
bool qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf);
+bool qemu_dmabuf_get_render_sync(QemuDmaBuf *dmabuf);
void qemu_dmabuf_set_texture(QemuDmaBuf *dmabuf, uint32_t texture);
void qemu_dmabuf_set_fence_fd(QemuDmaBuf *dmabuf, int32_t fence_fd);
void qemu_dmabuf_set_sync(QemuDmaBuf *dmabuf, void *sync);
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index e4b398d26c..69fa722c88 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -285,7 +285,8 @@ vhost_user_gpu_handle_display(VhostUserGPU *g,
VhostUserGpuMsg *msg)
m->fd_stride, 0, 0, 0, 0,
m->fd_drm_fourcc, modifier,
fd, false, m->fd_flags &
- VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP);
+ VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP,
+ false);
dpy_gl_scanout_dmabuf(con, dmabuf);
g->dmabuf[m->scanout_id] = dmabuf;
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index c02ec6d37d..8fcc0c3055 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -176,6 +176,7 @@
[RFC PATCH 4/4] ui/gtk-gl-draw: Start rendering of guest blob scanout if render_sync is off
From: Dongwon Kim
Draw (executing glBlitFramebuffer) immediately as soon as the frame
is flushed instead of getting it done in the next draw event if render_sync
flag is reset. With this, the fence will be signaled way ealier so the guest
can be working on the next frame right away.
Cc: Marc-André Lureau
Cc: Vivek Kasireddy
Signed-off-by: Dongwon Kim
---
ui/gtk-gl-area.c | 84 +---
1 file changed, 58 insertions(+), 26 deletions(-)
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 0b11423824..88d4e66a52 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -65,39 +65,36 @@ void gd_gl_area_draw(VirtualConsole *vc)
} else {
qemu_dmabuf_set_draw_submitted(dmabuf, false);
}
-}
-#endif
-glBindFramebuffer(GL_READ_FRAMEBUFFER, vc->gfx.guest_fb.framebuffer);
-/* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
-
-glViewport(0, 0, ww, wh);
-y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
-y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
-glBlitFramebuffer(0, y1, vc->gfx.w, y2,
- 0, 0, ww, wh,
- GL_COLOR_BUFFER_BIT, GL_NEAREST);
-#ifdef CONFIG_GBM
-if (dmabuf) {
-int fence_fd;
-fence_fd = egl_dmabuf_create_fence(dmabuf);
-if (fence_fd >= 0) {
-qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
-return;
+if (qemu_dmabuf_get_render_sync(dmabuf)) {
+int fence_fd;
+glBindFramebuffer(GL_READ_FRAMEBUFFER,
vc->gfx.guest_fb.framebuffer);
+/* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
+
+glViewport(0, 0, ww, wh);
+y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
+y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
+glBlitFramebuffer(0, y1, vc->gfx.w, y2,
+ 0, 0, ww, wh,
+ GL_COLOR_BUFFER_BIT, GL_NEAREST);
+fence_fd = egl_dmabuf_create_fence(dmabuf);
+if (fence_fd >= 0) {
+qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
+} else {
+graphic_hw_gl_block(vc->gfx.dcl.con, false);
+}
}
-graphic_hw_gl_block(vc->gfx.dcl.con, false);
}
#endif
-glFlush();
} else {
if (!vc->gfx.ds) {
return;
}
-gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
}
+glFlush();
}
void gd_gl_area_update(DisplayChangeListener *dcl,
@@ -119,13 +116,19 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+#ifdef CONFIG_GBM
+QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+#endif
+
gd_update_monitor_refresh_rate(vc, vc->window ? vc->window :
vc->gfx.drawing_area);
-if (vc->gfx.guest_fb.dmabuf &&
-qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
+#ifdef CONFIG_GBM
+if (dmabuf && qemu_dmabuf_get_draw_submitted(dmabuf) &&
+qemu_dmabuf_get_render_sync(dmabuf)) {
gd_gl_area_draw(vc);
return;
}
+#endif
if (!vc->gfx.gls) {
if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
@@ -282,13 +285,42 @@ void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
uint32_t x, uint32_t y, uint32_t w, uint32_t h)
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+#ifdef CONFIG_GBM
+QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+int ww, wh, ws, y1, y2;
+
+if (dmabuf && !qemu_dmabuf_get_draw_submitted(dmabuf)) {
+gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
+ws =
gdk_window_get_scale_factor(gtk_widget_get_window(vc->gfx.drawing_area));
+ww = gtk_widget_get_allocated_width(vc->gfx.drawing_area) * ws;
+wh = gtk_widget_get_allocated_height(vc->gfx.drawing_area) * ws;
-if (vc->gfx.guest_fb.dmabuf &&
-!qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
graphic_hw_gl_block(vc->gfx.dcl.con, true);
-qemu_dmabuf_set_draw_submitted(vc->gfx.guest_fb.dmabuf, true);
+qemu_dmabuf_set_draw_submitted(dmabuf, true);
gtk_gl_area_set_scanout_mode(vc, true);
+if (!qemu_dmabuf_get_render_sync(dmabuf)) {
+int fence_fd;
+glBindFramebuffer(GL_READ_FRAMEBUFFER,
vc->gfx.guest_fb.framebuffer);
+/* GtkGLArea sets GL_DRAW_FRAMEBUFFER for us */
+
+glViewport(0, 0, ww, wh);
+y1 = vc->gfx.y0_top ? 0 : vc->gfx.h;
+y2 = vc->gfx.y0_top ? vc->gfx.h : 0;
+glBlitFramebuffer(0, y1, vc->gfx.w, y2,
+ 0, 0,
[RFC PATCH 2/4] ui/egl-helpers: Consolidates create-sync and create-fence
From: Dongwon Kim
There is no reason to split those two operations so combining
two functions - egl_dmabuf_create_sync and egl_dmabuf_create_fence.
Cc: Gerd Hoffmann
Cc: Marc-André Lureau
Cc: Vivek Kasireddy
Signed-off-by: Dongwon Kim
---
include/ui/egl-helpers.h | 3 +--
ui/egl-helpers.c | 27 +++
ui/gtk-egl.c | 19 +++
ui/gtk-gl-area.c | 10 ++
4 files changed, 21 insertions(+), 38 deletions(-)
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 4b8c0d2281..606d6c8288 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -51,8 +51,7 @@ int egl_get_fd_for_texture(uint32_t tex_id, EGLint *stride,
EGLint *fourcc,
void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf);
void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf);
-void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf);
-void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf);
+int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf);
#endif
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 99b2ebbe23..e16f2cb23d 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -371,34 +371,29 @@ void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf)
qemu_dmabuf_set_texture(dmabuf, 0);
}
-void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
+int egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
{
EGLSyncKHR sync;
+int fence_fd = -1;
if (epoxy_has_egl_extension(qemu_egl_display,
"EGL_KHR_fence_sync") &&
epoxy_has_egl_extension(qemu_egl_display,
-"EGL_ANDROID_native_fence_sync")) {
+"EGL_ANDROID_native_fence_sync") &&
+qemu_dmabuf_get_fence_fd(dmabuf) == -1) {
sync = eglCreateSyncKHR(qemu_egl_display,
EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
if (sync != EGL_NO_SYNC_KHR) {
-qemu_dmabuf_set_sync(dmabuf, sync);
+fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
+ sync);
+if (fence_fd >= 0) {
+qemu_dmabuf_set_fence_fd(dmabuf, fence_fd);
+}
+eglDestroySyncKHR(qemu_egl_display, sync);
}
}
-}
-
-void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
-{
-void *sync = qemu_dmabuf_get_sync(dmabuf);
-int fence_fd;
-if (sync) {
-fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
- sync);
-qemu_dmabuf_set_fence_fd(dmabuf, fence_fd);
-eglDestroySyncKHR(qemu_egl_display, sync);
-qemu_dmabuf_set_sync(dmabuf, NULL);
-}
+return fence_fd;
}
#endif /* CONFIG_GBM */
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 9831c10e1b..55199f8659 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -68,7 +68,6 @@ void gd_egl_draw(VirtualConsole *vc)
GdkWindow *window;
#ifdef CONFIG_GBM
QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
-int fence_fd;
#endif
int ww, wh, ws;
@@ -99,13 +98,12 @@ void gd_egl_draw(VirtualConsole *vc)
glFlush();
#ifdef CONFIG_GBM
if (dmabuf) {
-egl_dmabuf_create_fence(dmabuf);
-fence_fd = qemu_dmabuf_get_fence_fd(dmabuf);
+fence_fd = egl_dmabuf_create_fence(dmabuf);
if (fence_fd >= 0) {
qemu_set_fd_handler(fence_fd, gd_hw_gl_flushed, NULL, vc);
-return;
+} else {
+graphic_hw_gl_block(vc->gfx.dcl.con, false);
}
-graphic_hw_gl_block(vc->gfx.dcl.con, false);
}
#endif
} else {
@@ -336,7 +334,11 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
GdkWindow *window;
+#ifdef CONFIG_GBM
+QemuDmaBuf *dmabuf = vc->gfx.guest_fb.dmabuf;
+#endif
int ww, wh, ws;
+int fence_fd;
if (!vc->gfx.scanout_mode) {
return;
@@ -364,12 +366,6 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
egl_fb_blit(>gfx.win_fb, >gfx.guest_fb, !vc->gfx.y0_top);
}
-#ifdef CONFIG_GBM
-if (vc->gfx.guest_fb.dmabuf) {
-egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf);
-}
-#endif
-
eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
}
@@ -387,7 +383,6 @@ void gd_egl_flush(DisplayChangeListener *dcl,
gtk_widget_queue_draw_area(area, x, y, w, h);
return;
}
-
gd_egl_scanout_flush(>gfx.dcl, x, y, w, h);
}
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index b628b35451..0b11423824 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -77,17 +77,10 @@ void gd_gl_area_draw(VirtualConsole *vc)
glBlitFramebuffer(0, y1, vc->gfx.w, y2,
0, 0, ww, wh,
GL_COLOR_BUFFER_BIT, GL_NEAREST);
-#ifdef CONFIG_GBM
-if (dmabuf) {
-egl_dmabuf_create_sync(dmabuf);
-
standardizing i2c device ids
Corey and Peter, My team builds lots of configurations for Qemu boards, and one pain point has been that the qom path for a device depends on the device insertion order, child[0], child[1] and the like. I noticed that the qdev paths for devices also exist by their device id property. By default, this ends up being the device type name. I was wondering if it made sense to override this with the device type plus the smbus address? I did something similar with the i2c mux device, to resolve part of this issue. Patrick
[RFC PATCH 0/7] migration/multifd: Introduce storage slots
Hi folks, First of all, apologies for the roughness of the series. I'm off for the next couple of weeks and wanted to put something together early for your consideration. This series is a refactoring (based on an earlier, off-list attempt[0]), aimed to remove the usage of the MultiFDPages_t type in the multifd core. If we're going to add support for more data types to multifd, we first need to clean that up. This time around this work was prompted by Maciej's series[1]. I see you're having to add a bunch of is_device_state checks to work around the rigidity of the code. Aside from the VFIO work, there is also the intent (coming back from Juan's ideas) to make multifd the default code path for migration, which will have to include the vmstate migration and anything else we put on the stream via QEMUFile. I have long since been bothered by having 'pages' sprinkled all over the code, so I might be coming at this with a bit of a narrow focus, but I believe in order to support more types of payloads in multifd, we need to first allow the scheduling at multifd_send_pages() to be independent of MultiFDPages_t. So here it is. Let me know what you think. (as I said, I'll be off for a couple of weeks, so feel free to incorporate any of this code if it's useful. Or to ignore it completely). CI run: https://gitlab.com/farosas/qemu/-/pipelines/1340992028 0- https://github.com/farosas/qemu/commits/multifd-packet-cleanups/ 1- https://lore.kernel.org/r/cover.1718717584.git.maciej.szmigi...@oracle.com Fabiano Rosas (7): migration/multifd: Reduce access to p->pages migration/multifd: Pass in MultiFDPages_t to file_write_ramblock_iov migration/multifd: Replace p->pages with an opaque pointer migration/multifd: Move pages accounting into multifd_send_zero_page_detect() migration/multifd: Isolate ram pages packet data migration/multifd: Move payload storage out of the channel parameters migration/multifd: Hide multifd slots implementation migration/file.c | 3 +- migration/file.h | 2 +- migration/multifd-qpl.c | 8 +- migration/multifd-uadk.c | 9 +- migration/multifd-zero-page.c | 6 +- migration/multifd-zlib.c | 4 +- migration/multifd-zstd.c | 4 +- migration/multifd.c | 263 -- migration/multifd.h | 28 +++- migration/ram.c | 1 + 10 files changed, 232 insertions(+), 96 deletions(-) base-commit: 79e6ec66ba1067a135394a330fec14b50cf49534 -- 2.35.3
[RFC PATCH 2/7] migration/multifd: Pass in MultiFDPages_t to file_write_ramblock_iov
We want to stop dereferencing 'pages' so it can be replaced by an
opaque pointer in the next patches.
Signed-off-by: Fabiano Rosas
---
migration/file.c| 3 ++-
migration/file.h| 2 +-
migration/multifd.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/migration/file.c b/migration/file.c
index ab18ba505a..07e8648c7d 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -167,12 +167,13 @@ void file_start_incoming_migration(FileMigrationArgs
*file_args, Error **errp)
}
int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
-int niov, RAMBlock *block, Error **errp)
+int niov, MultiFDPages_t *pages, Error **errp)
{
ssize_t ret = 0;
int i, slice_idx, slice_num;
uintptr_t base, next, offset;
size_t len;
+RAMBlock *block = pages->block;
slice_idx = 0;
slice_num = 1;
diff --git a/migration/file.h b/migration/file.h
index 7699c04677..d735b623b0 100644
--- a/migration/file.h
+++ b/migration/file.h
@@ -22,6 +22,6 @@ void file_cleanup_outgoing_migration(void);
bool file_send_channel_create(gpointer opaque, Error **errp);
void file_create_incoming_channels(QIOChannel *ioc, Error **errp);
int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
-int niov, RAMBlock *block, Error **errp);
+int niov, MultiFDPages_t *pages, Error **errp);
int multifd_file_recv_data(MultiFDRecvParams *p, Error **errp);
#endif
diff --git a/migration/multifd.c b/migration/multifd.c
index 506f42e124..58340d9d95 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -953,7 +953,7 @@ static void *multifd_send_thread(void *opaque)
if (migrate_mapped_ram()) {
ret = file_write_ramblock_iov(p->c, p->iov, p->iovs_num,
- pages->block, _err);
+ pages, _err);
} else {
ret = qio_channel_writev_full_all(p->c, p->iov, p->iovs_num,
NULL, 0, p->write_flags,
--
2.35.3
[RFC PATCH 7/7] migration/multifd: Hide multifd slots implementation
The only two things the multifd client needs to access are the active
slot and the active slot size:
The active slot itself is obviously needed because it's where the data
is put.
The slot size is needed only by the ram pages code, because it does
not fill the data slot and sends it in one go, it instead fills the
slot partially at each call of multifd_queue_page(), so the size is
needed to differentiate an empty slot (free or recently consumed) from
the slot that is partially full.
Hide the MultiFDSlots implementation so the client is not tempted to
make use of the free list. That field is there simply because we need
the client to carry a handle to that memory, it's not supposed to be
accessed directly.
Signed-off-by: Fabiano Rosas
---
migration/multifd.c | 26 +++---
migration/multifd.h | 8 +++-
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index f22a1c2e84..9fb719eb0d 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -38,6 +38,11 @@
#define MULTIFD_MAGIC 0x11223344U
#define MULTIFD_VERSION 1
+struct MultiFDSlots {
+MultiFDSendData **free;
+MultiFDSendData *active;
+};
+
typedef struct {
uint32_t magic;
uint32_t version;
@@ -737,7 +742,22 @@ static inline bool multifd_queue_full(MultiFDPages_t
*pages)
static inline void multifd_enqueue(MultiFDPages_t *pages, ram_addr_t offset)
{
pages->offset[pages->num++] = offset;
-multifd_ram_send_slots->active->size += qemu_target_page_size();
+multifd_set_slot_size(multifd_ram_send_slots, qemu_target_page_size());
+}
+
+void *multifd_get_active_slot(MultiFDSlots *multifd_ram_send_slots)
+{
+return multifd_ram_send_slots->active->opaque;
+}
+
+void multifd_set_slot_size(MultiFDSlots *multifd_ram_send_slots, size_t size)
+{
+multifd_ram_send_slots->active->size += size;
+}
+
+bool multifd_slot_has_data(MultiFDSlots *multifd_ram_send_slots)
+{
+return !!multifd_ram_send_slots->active->size;
}
/* Returns true if enqueue successful, false otherwise */
@@ -746,7 +766,7 @@ bool multifd_queue_page(RAMBlock *block, ram_addr_t offset)
MultiFDPages_t *pages;
retry:
-pages = multifd_ram_send_slots->active->opaque;
+pages = multifd_get_active_slot(multifd_ram_send_slots);
/* If the queue is empty, we can already enqueue now */
if (multifd_queue_empty(pages)) {
@@ -951,7 +971,7 @@ int multifd_send_sync_main(void)
return 0;
}
-if (multifd_ram_send_slots->active->size) {
+if (multifd_slot_has_data(multifd_ram_send_slots)) {
if (!multifd_send(multifd_ram_send_slots)) {
error_report("%s: multifd_send_pages fail", __func__);
return -1;
diff --git a/migration/multifd.h b/migration/multifd.h
index 5230729077..8f99fe2652 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -102,15 +102,13 @@ struct MultiFDSendData {
void (*cleanup)(void *);
};
-struct MultiFDSlots {
-MultiFDSendData **free;
-MultiFDSendData *active;
-};
-
MultiFDSlots *multifd_allocate_slots(void *(*alloc_fn)(void),
void (*reset_fn)(void *),
void (*cleanup_fn)(void *));
void multifd_ram_save_setup(void);
+void *multifd_get_active_slot(MultiFDSlots *multifd_ram_send_slots);
+void multifd_set_slot_size(MultiFDSlots *multifd_ram_send_slots, size_t size);
+bool multifd_slot_has_data(MultiFDSlots *multifd_ram_send_slots);
typedef struct {
/* Fields are only written at creating/deletion time */
--
2.35.3
[RFC PATCH 5/7] migration/multifd: Isolate ram pages packet data
While we cannot yet disentangle the multifd packet from page data, we
can make the code a bit cleaner by setting the page-related fields in
a separate function.
Signed-off-by: Fabiano Rosas
---
migration/multifd.c | 104 +---
1 file changed, 68 insertions(+), 36 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index c4a952576d..6fe339b378 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -407,65 +407,64 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
g_free(pages);
}
-void multifd_send_fill_packet(MultiFDSendParams *p)
+static void multifd_ram_fill_packet(MultiFDSendParams *p)
{
MultiFDPacket_t *packet = p->packet;
MultiFDPages_t *pages = p->data->opaque;
-uint64_t packet_num;
uint32_t zero_num = pages->num - pages->normal_num;
-int i;
-packet->flags = cpu_to_be32(p->flags);
packet->pages_alloc = cpu_to_be32(pages->allocated);
packet->normal_pages = cpu_to_be32(pages->normal_num);
packet->zero_pages = cpu_to_be32(zero_num);
-packet->next_packet_size = cpu_to_be32(p->next_packet_size);
-
-packet_num = qatomic_fetch_inc(_send_state->packet_num);
-packet->packet_num = cpu_to_be64(packet_num);
if (pages->block) {
strncpy(packet->ramblock, pages->block->idstr, 256);
}
-for (i = 0; i < pages->num; i++) {
+for (int i = 0; i < pages->num; i++) {
/* there are architectures where ram_addr_t is 32 bit */
uint64_t temp = pages->offset[i];
packet->offset[i] = cpu_to_be64(temp);
}
-p->packets_sent++;
p->total_normal_pages += pages->normal_num;
p->total_zero_pages += zero_num;
+}
-trace_multifd_send(p->id, packet_num, pages->normal_num, zero_num,
+void multifd_send_fill_packet(MultiFDSendParams *p)
+{
+MultiFDPacket_t *packet = p->packet;
+uint64_t packet_num;
+
+memset(packet, 0, p->packet_len);
+
+packet->magic = cpu_to_be32(MULTIFD_MAGIC);
+packet->version = cpu_to_be32(MULTIFD_VERSION);
+
+packet->flags = cpu_to_be32(p->flags);
+packet->next_packet_size = cpu_to_be32(p->next_packet_size);
+
+packet_num = qatomic_fetch_inc(_send_state->packet_num);
+packet->packet_num = cpu_to_be64(packet_num);
+
+p->packets_sent++;
+
+if (p->data) {
+multifd_ram_fill_packet(p);
+}
+
+trace_multifd_send(p->id, packet_num,
+ be32_to_cpu(packet->normal_pages),
+ be32_to_cpu(packet->zero_pages),
p->flags, p->next_packet_size);
}
-static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
+static int multifd_ram_unfill_packet(MultiFDRecvParams *p, Error **errp)
{
MultiFDPacket_t *packet = p->packet;
int i;
-packet->magic = be32_to_cpu(packet->magic);
-if (packet->magic != MULTIFD_MAGIC) {
-error_setg(errp, "multifd: received packet "
- "magic %x and expected magic %x",
- packet->magic, MULTIFD_MAGIC);
-return -1;
-}
-
-packet->version = be32_to_cpu(packet->version);
-if (packet->version != MULTIFD_VERSION) {
-error_setg(errp, "multifd: received packet "
- "version %u and expected version %u",
- packet->version, MULTIFD_VERSION);
-return -1;
-}
-
-p->flags = be32_to_cpu(packet->flags);
-
packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
/*
* If we received a packet that is 100 times bigger than expected
@@ -494,15 +493,9 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
*p, Error **errp)
return -1;
}
-p->next_packet_size = be32_to_cpu(packet->next_packet_size);
-p->packet_num = be64_to_cpu(packet->packet_num);
-p->packets_recved++;
p->total_normal_pages += p->normal_num;
p->total_zero_pages += p->zero_num;
-trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
- p->flags, p->next_packet_size);
-
if (p->normal_num == 0 && p->zero_num == 0) {
return 0;
}
@@ -544,6 +537,45 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
*p, Error **errp)
return 0;
}
+static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
+{
+MultiFDPacket_t *packet = p->packet;
+int ret = 0;
+
+packet->magic = be32_to_cpu(packet->magic);
+if (packet->magic != MULTIFD_MAGIC) {
+error_setg(errp, "multifd: received packet "
+ "magic %x and expected magic %x",
+ packet->magic, MULTIFD_MAGIC);
+return -1;
+}
+
+packet->version = be32_to_cpu(packet->version);
+if (packet->version != MULTIFD_VERSION) {
+error_setg(errp, "multifd: received packet "
+ "version %u and expected version %u",
+ packet->version, MULTIFD_VERSION);
+return -1;
+}
+
+p->flags =
[RFC PATCH 4/7] migration/multifd: Move pages accounting into multifd_send_zero_page_detect()
All references to pages are being removed from the multifd worker threads in order to allow multifd to deal with different payload types. multifd_send_zero_page_detect() is called by all multifd migration paths that deal with pages and is the last spot where zero pages and normal page amounts are adjusted. Move the pages accounting into that function. Signed-off-by: Fabiano Rosas --- migration/multifd-zero-page.c | 4 migration/multifd.c | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c index 1ad77462a4..63a1c24ba8 100644 --- a/migration/multifd-zero-page.c +++ b/migration/multifd-zero-page.c @@ -14,6 +14,7 @@ #include "qemu/cutils.h" #include "exec/ramblock.h" #include "migration.h" +#include "migration-stats.h" #include "multifd.h" #include "options.h" #include "ram.h" @@ -74,6 +75,9 @@ void multifd_send_zero_page_detect(MultiFDSendParams *p) } pages->normal_num = i; + +stat64_add(_stats.normal_pages, pages->normal_num); +stat64_add(_stats.zero_pages, pages->num - pages->normal_num); } void multifd_recv_zero_page_process(MultiFDRecvParams *p) diff --git a/migration/multifd.c b/migration/multifd.c index 55b31c4515..c4a952576d 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -975,8 +975,6 @@ static void *multifd_send_thread(void *opaque) stat64_add(_stats.multifd_bytes, p->next_packet_size + p->packet_len); -stat64_add(_stats.normal_pages, pages->normal_num); -stat64_add(_stats.zero_pages, pages->num - pages->normal_num); multifd_pages_reset(pages); p->next_packet_size = 0; -- 2.35.3
[RFC PATCH 3/7] migration/multifd: Replace p->pages with an opaque pointer
We want multifd to be able to handle more types of data than just ram
pages. To start decoupling multifd from pages, replace p->pages
(MultiFDPages_t) with a new type MultiFDSendData that hides an opaque
pointer.
The general idea here is to isolate functions that *need* to handle
MultiFDPages_t and move them in the future to multifd-ram.c, while
multifd.c will stay with only the core functions that handle
MultiFDSendData/MultiFDRecvData.
Signed-off-by: Fabiano Rosas
---
migration/multifd-zero-page.c | 2 +-
migration/multifd-zlib.c | 2 +-
migration/multifd-zstd.c | 2 +-
migration/multifd.c | 57 +--
migration/multifd.h | 13
5 files changed, 44 insertions(+), 32 deletions(-)
diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
index e1b8370f88..1ad77462a4 100644
--- a/migration/multifd-zero-page.c
+++ b/migration/multifd-zero-page.c
@@ -46,7 +46,7 @@ static void swap_page_offset(ram_addr_t *pages_offset, int a,
int b)
*/
void multifd_send_zero_page_detect(MultiFDSendParams *p)
{
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
RAMBlock *rb = pages->block;
int i = 0;
int j = pages->num - 1;
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 65f8aba5c8..e75e04d2c7 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -123,7 +123,7 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error
**errp)
*/
static int zlib_send_prepare(MultiFDSendParams *p, Error **errp)
{
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
struct zlib_data *z = p->compress_data;
z_stream *zs = >zs;
uint32_t out_size = 0;
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index cb6075a9a5..1ba572a882 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -119,7 +119,7 @@ static void zstd_send_cleanup(MultiFDSendParams *p, Error
**errp)
*/
static int zstd_send_prepare(MultiFDSendParams *p, Error **errp)
{
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
struct zstd_data *z = p->compress_data;
int ret;
uint32_t i;
diff --git a/migration/multifd.c b/migration/multifd.c
index 58340d9d95..55b31c4515 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -49,8 +49,7 @@ typedef struct {
struct {
MultiFDSendParams *params;
-/* array of pages to sent */
-MultiFDPages_t *pages;
+MultiFDSendData *data;
/*
* Global number of generated multifd packets.
*
@@ -110,7 +109,7 @@ void multifd_send_channel_created(void)
static void multifd_set_file_bitmap(MultiFDSendParams *p)
{
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
assert(pages->block);
@@ -164,7 +163,7 @@ static void nocomp_send_cleanup(MultiFDSendParams *p, Error
**errp)
static void multifd_send_prepare_iovs(MultiFDSendParams *p)
{
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
for (int i = 0; i < pages->normal_num; i++) {
p->iov[p->iovs_num].iov_base = pages->block->host + pages->offset[i];
@@ -411,7 +410,7 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
void multifd_send_fill_packet(MultiFDSendParams *p)
{
MultiFDPacket_t *packet = p->packet;
-MultiFDPages_t *pages = p->pages;
+MultiFDPages_t *pages = p->data->opaque;
uint64_t packet_num;
uint32_t zero_num = pages->num - pages->normal_num;
int i;
@@ -591,7 +590,8 @@ static bool multifd_send_pages(void)
int i;
static int next_channel;
MultiFDSendParams *p = NULL; /* make happy gcc */
-MultiFDPages_t *pages = multifd_send_state->pages;
+MultiFDPages_t *channel_pages;
+MultiFDSendData *data = multifd_send_state->data;
if (multifd_send_should_exit()) {
return false;
@@ -626,11 +626,14 @@ static bool multifd_send_pages(void)
* qatomic_store_release() in multifd_send_thread().
*/
smp_mb_acquire();
-assert(!p->pages->num);
-multifd_send_state->pages = p->pages;
-p->pages = pages;
+
+channel_pages = p->data->opaque;
+assert(!channel_pages->num);
+
+multifd_send_state->data = p->data;
+p->data = data;
/*
- * Making sure p->pages is setup before marking pending_job=true. Pairs
+ * Making sure p->data is setup before marking pending_job=true. Pairs
* with the qatomic_load_acquire() in multifd_send_thread().
*/
qatomic_store_release(>pending_job, true);
@@ -660,7 +663,7 @@ bool multifd_queue_page(RAMBlock *block, ram_addr_t offset)
MultiFDPages_t *pages;
retry:
-pages = multifd_send_state->pages;
+pages = multifd_send_state->data->opaque;
/* If the queue is empty, we can already enqueue now */
if (multifd_queue_empty(pages)) {
@@ -790,8 +793,10 @@ static bool
[RFC PATCH 1/7] migration/multifd: Reduce access to p->pages
I'm about to replace the p->pages pointer with an opaque pointer, so
do a cleanup now to reduce direct accesses to p->page, which makes the
next diffs cleaner.
Signed-off-by: Fabiano Rosas
---
migration/multifd-qpl.c | 8 +---
migration/multifd-uadk.c | 9 +
migration/multifd-zlib.c | 2 +-
migration/multifd-zstd.c | 2 +-
migration/multifd.c | 13 +++--
5 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/migration/multifd-qpl.c b/migration/multifd-qpl.c
index 9265098ee7..f8c84c52cf 100644
--- a/migration/multifd-qpl.c
+++ b/migration/multifd-qpl.c
@@ -404,13 +404,14 @@ retry:
static void multifd_qpl_compress_pages_slow_path(MultiFDSendParams *p)
{
QplData *qpl = p->compress_data;
+MultiFDPages_t *pages = p->pages;
uint32_t size = p->page_size;
qpl_job *job = qpl->sw_job;
uint8_t *zbuf = qpl->zbuf;
uint8_t *buf;
-for (int i = 0; i < p->pages->normal_num; i++) {
-buf = p->pages->block->host + p->pages->offset[i];
+for (int i = 0; i < pages->normal_num; i++) {
+buf = pages->block->host + pages->offset[i];
multifd_qpl_prepare_comp_job(job, buf, zbuf, size);
if (qpl_execute_job(job) == QPL_STS_OK) {
multifd_qpl_fill_packet(i, p, zbuf, job->total_out);
@@ -498,6 +499,7 @@ static void multifd_qpl_compress_pages(MultiFDSendParams *p)
static int multifd_qpl_send_prepare(MultiFDSendParams *p, Error **errp)
{
QplData *qpl = p->compress_data;
+MultiFDPages_t *pages = p->pages;
uint32_t len = 0;
if (!multifd_send_prepare_common(p)) {
@@ -505,7 +507,7 @@ static int multifd_qpl_send_prepare(MultiFDSendParams *p,
Error **errp)
}
/* The first IOV is used to store the compressed page lengths */
-len = p->pages->normal_num * sizeof(uint32_t);
+len = pages->normal_num * sizeof(uint32_t);
multifd_qpl_fill_iov(p, (uint8_t *) qpl->zlen, len);
if (qpl->hw_avail) {
multifd_qpl_compress_pages(p);
diff --git a/migration/multifd-uadk.c b/migration/multifd-uadk.c
index d12353fb21..b8ba3cd9c1 100644
--- a/migration/multifd-uadk.c
+++ b/migration/multifd-uadk.c
@@ -174,19 +174,20 @@ static int multifd_uadk_send_prepare(MultiFDSendParams
*p, Error **errp)
uint32_t hdr_size;
uint8_t *buf = uadk_data->buf;
int ret = 0;
+MultiFDPages_t *pages = p->pages;
if (!multifd_send_prepare_common(p)) {
goto out;
}
-hdr_size = p->pages->normal_num * sizeof(uint32_t);
+hdr_size = pages->normal_num * sizeof(uint32_t);
/* prepare the header that stores the lengths of all compressed data */
prepare_next_iov(p, uadk_data->buf_hdr, hdr_size);
-for (int i = 0; i < p->pages->normal_num; i++) {
+for (int i = 0; i < pages->normal_num; i++) {
struct wd_comp_req creq = {
.op_type = WD_DIR_COMPRESS,
-.src = p->pages->block->host + p->pages->offset[i],
+.src = pages->block->host + pages->offset[i],
.src_len = p->page_size,
.dst = buf,
/* Set dst_len to double the src in case compressed out >=
page_size */
@@ -214,7 +215,7 @@ static int multifd_uadk_send_prepare(MultiFDSendParams *p,
Error **errp)
*/
if (!uadk_data->handle || creq.dst_len >= p->page_size) {
uadk_data->buf_hdr[i] = cpu_to_be32(p->page_size);
-prepare_next_iov(p, p->pages->block->host + p->pages->offset[i],
+prepare_next_iov(p, pages->block->host + pages->offset[i],
p->page_size);
buf += p->page_size;
}
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 2ced69487e..65f8aba5c8 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -147,7 +147,7 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error
**errp)
* with compression. zlib does not guarantee that this is safe,
* therefore copy the page before calling deflate().
*/
-memcpy(z->buf, p->pages->block->host + pages->offset[i], p->page_size);
+memcpy(z->buf, pages->block->host + pages->offset[i], p->page_size);
zs->avail_in = p->page_size;
zs->next_in = z->buf;
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index ca17b7e310..cb6075a9a5 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -138,7 +138,7 @@ static int zstd_send_prepare(MultiFDSendParams *p, Error
**errp)
if (i == pages->normal_num - 1) {
flush = ZSTD_e_flush;
}
-z->in.src = p->pages->block->host + pages->offset[i];
+z->in.src = pages->block->host + pages->offset[i];
z->in.size = p->page_size;
z->in.pos = 0;
diff --git a/migration/multifd.c b/migration/multifd.c
index d82885fdbb..506f42e124 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -114,11 +114,11 @@ static void
[RFC PATCH 6/7] migration/multifd: Move payload storage out of the channel parameters
Multifd currently has a simple scheduling mechanism that distributes
work to the various channels by providing the client (producer) with a
memory slot and swapping that slot with free slot from the next idle
channel (consumer). Or graphically:
[] <-- multifd_send_state->pages
[][][][] <-- channels' p->pages pointers
1) client fills the empty slot with data:
[a]
[][][][]
2) multifd_send_pages() finds an idle channel and swaps the pointers:
[a]
[][][][]
^idle
[]
[a][][][]
3) client can immediately fill new slot with more data:
[b]
[a][][][]
4) channel processes the data, the channel slot is now free to use
again:
[b]
[][][][]
This works just fine, except that it doesn't allow different types of
payloads to be processed at the same time in different channels,
i.e. the data type of multifd_send_state->pages needs to be the same
as p->pages. For each new data type different from MultiFDPage_t that
is to be handled, this logic needs to be duplicated by adding new
fields to multifd_send_state and to the channels.
The core of the issue here is that we're using the channel parameters
(MultiFDSendParams) to hold the storage space on behalf of the multifd
client (currently ram.c). This is cumbersome because it forces us to
change multifd_send_pages() to check the data type being handled
before deciding which field to use.
One way to solve this is to detach the storage space from the multifd
channel and put it somewhere else, in control of the multifd
client. That way, multifd_send_pages() can operate on an opaque
pointer without needing to be adapted to each new data type. Implement
this logic with a new "slots" abstraction:
struct MultiFDSendData {
void *opaque;
size_t size;
}
struct MultiFDSlots {
MultiFDSendData **free; <-- what used to be p->pages
MultiFDSendData *active; <-- what used to be multifd_send_state->pages
};
Each multifd client now gets one set of slots to use. The slots are
passed into multifd_send_pages() (renamed to multifd_send). The
channels now only hold a pointer to the generic MultiFDSendData, and
after it's processed that reference can be dropped.
Or graphically:
1) client fills the active slot with data. Channels point to nothing
at this point:
[a] <-- active slot
[][][][] <-- free slots, one per-channel
[][][][] <-- channels' p->data pointers
2) multifd_send() swaps the pointers inside the client slot. Channels
still point to nothing:
[]
[a][][][]
[][][][]
3) multifd_send() finds an idle channel and updates its pointer:
[]
[a][][][]
[a][][][]
^idle
4) a second client calls multifd_send(), but with it's own slots:
[] [b]
[a][][][] [][][][]
[a][][][]
5) multifd_send() does steps 2 and 3 again:
[] []
[a][][][] [][b][][]
[a][b][][]
^idle
6) The channels continue processing the data and lose/acquire the
references as multifd_send() updates them. The free lists of each
client are not affected.
Signed-off-by: Fabiano Rosas
---
migration/multifd.c | 119 +++-
migration/multifd.h | 17 +++
migration/ram.c | 1 +
3 files changed, 102 insertions(+), 35 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index 6fe339b378..f22a1c2e84 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -97,6 +97,30 @@ struct {
MultiFDMethods *ops;
} *multifd_recv_state;
+MultiFDSlots *multifd_allocate_slots(void *(*alloc_fn)(void),
+ void (*reset_fn)(void *),
+ void (*cleanup_fn)(void *))
+{
+int thread_count = migrate_multifd_channels();
+MultiFDSlots *slots = g_new0(MultiFDSlots, 1);
+
+slots->active = g_new0(MultiFDSendData, 1);
+slots->free = g_new0(MultiFDSendData *, thread_count);
+
+slots->active->opaque = alloc_fn();
+slots->active->reset = reset_fn;
+slots->active->cleanup = cleanup_fn;
+
+for (int i = 0; i < thread_count; i++) {
+slots->free[i] = g_new0(MultiFDSendData, 1);
+slots->free[i]->opaque = alloc_fn();
+slots->free[i]->reset = reset_fn;
+slots->free[i]->cleanup = cleanup_fn;
+}
+
+return slots;
+}
+
static bool multifd_use_packets(void)
{
return !migrate_mapped_ram();
@@ -313,8 +337,10 @@ void multifd_register_ops(int method, MultiFDMethods *ops)
}
/* Reset a MultiFDPages_t* object for the next use */
-static void multifd_pages_reset(MultiFDPages_t *pages)
+static void multifd_pages_reset(void *opaque)
{
+MultiFDPages_t *pages = opaque;
+
/*
* We don't need to touch offset[] array, because it will be
* overwritten later when reused.
@@ -388,8 +414,9 @@ static int multifd_recv_initial_packet(QIOChannel *c, Error
**errp)
return msg.id;
}
-static MultiFDPages_t *multifd_pages_init(uint32_t n)
+static void *multifd_pages_init(void)
{
+uint32_t n = MULTIFD_PACKET_SIZE /
Re: [PULL 65/76] hw/mips/loongson3_virt: Wire up loongson_ipi device
在2024年6月20日六月 下午8:50，Philippe Mathieu-Daudé写道：
[...]
> Do you mind posting a patch fixing it?
I'll prepare a series tomorrow with your comments on this patch before.
Thanks
- Jiaxun
>
>> +hwaddr base = ((hwaddr)node << 44) + virt_memmap[VIRT_IPI].base;
>> +base += core * 0x100;
>> +qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
>> +sysbus_mmio_map(SYS_BUS_DEVICE(ipi), i + 2, base);
>> +}
>> +
>> +if (ase_lcsr_available(_CPU(cpu)->env)) {
>> +MemoryRegion *core_iocsr = g_new(MemoryRegion, 1);
>> +g_autofree char *name = g_strdup_printf("core%d_iocsr", i);
>> +memory_region_init_alias(core_iocsr, OBJECT(cpu), name,
>> + iocsr, 0, UINT32_MAX);
>> +memory_region_add_subregion(_CPU(cpu)->env.iocsr.mr,
>> +0, core_iocsr);
>> +}
>> +
>> +if (node > 0) {
>> continue; /* Only node-0 can be connected to LIOINTC */
>> }
>>
>> for (ip = 0; ip < 4 ; ip++) {
>> -int pin = i * 4 + ip;
>> +int pin = core * LOONGSON3_CORE_PER_NODE + ip;
>> sysbus_connect_irq(SYS_BUS_DEVICE(liointc),
>> pin, cpu->env.irq[ip + 2]);
>> }
--
- Jiaxun
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
Hi Peter, thank you for looking. On Thu, Jun 20, 2024 at 12:09 PM Peter Maydell wrote: > I think this is the point where I would say "you're making the > code worse for upstream and the only benefit is to your out-of-tree > downstream code". If you want to build QEMU, use one of the compilers > that QEMU supports. I think there is a value in letting other developers (not just us) to build their code with QEMU. I understand your concern and sent an updated version of the patch, which is only two lines long. > There are lots and lots of places where we > assume the GCC-or-clang feature set over and above plain C. I am not changing this part.
[PATCH v2] exec: don't use void* in pointer arithmetic in headers
void* pointer arithmetic is a GCC extentension
which could not be available in other build
tools (e.g. C++). This changes removes this
assumption.
Google-Bug-Id: 331190993
Change-Id: I5a064853429f627c17a9213910811dea4ced6174
Signed-off-by: Roman Kiryanov
---
v2: renamed from "use char* for pointer arithmetic"
and removed all explicit extra cast with
one typedef in memory.h.
include/exec/memory.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index b1713f30b8..b616338f05 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2795,8 +2795,10 @@ MemTxResult address_space_write_rom(AddressSpace *as,
hwaddr addr,
#define ARG1_DECLAddressSpace *as
#include "exec/memory_ldst_phys.h.inc"
+typedef uint8_t *MemoryRegionCachePtr;
+
struct MemoryRegionCache {
-void *ptr;
+MemoryRegionCachePtr ptr;
hwaddr xlat;
hwaddr len;
FlatView *fv;
--
2.45.2.741.gdbec12cfda-goog
Re: [PATCH v2 6/6] target/riscv: Remove extension auto-update check statements
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
Remove the old-fashioned extension auto-update check statements as
they are replaced by the extension implied rules.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/tcg/tcg-cpu.c | 119 -
1 file changed, 119 deletions(-)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index fe84d4402e..94875c8ec9 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -468,10 +468,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
return;
}
-if (cpu->cfg.ext_zfh) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zfhmin), true);
-}
-
if (cpu->cfg.ext_zfhmin && !riscv_has_ext(env, RVF)) {
error_setg(errp, "Zfh/Zfhmin extensions require F extension");
return;
@@ -493,9 +489,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error
**errp)
error_propagate(errp, local_err);
return;
}
-
-/* The V vector extension depends on the Zve64d extension */
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve64d), true);
}
/* The Zve64d extension depends on the Zve64f extension */
@@ -504,18 +497,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
error_setg(errp, "Zve64d/V extensions require D extension");
return;
}
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve64f), true);
-}
-
-/* The Zve64f extension depends on the Zve64x and Zve32f extensions */
-if (cpu->cfg.ext_zve64f) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve64x), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32f), true);
-}
-
-/* The Zve64x extension depends on the Zve32x extension */
-if (cpu->cfg.ext_zve64x) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32x), true);
}
/* The Zve32f extension depends on the Zve32x extension */
@@ -524,11 +505,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
error_setg(errp, "Zve32f/Zve64f extensions require F extension");
return;
}
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zve32x), true);
-}
-
-if (cpu->cfg.ext_zvfh) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvfhmin), true);
}
if (cpu->cfg.ext_zvfhmin && !cpu->cfg.ext_zve32f) {
@@ -551,11 +527,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
return;
}
-/* Set the ISA extensions, checks should have happened above */
-if (cpu->cfg.ext_zhinx) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zca), true);
-}
-
if ((cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinxmin) && !cpu->cfg.ext_zfinx)
{
error_setg(errp, "Zdinx/Zhinx/Zhinxmin extensions require Zfinx");
return;
@@ -573,27 +544,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
}
}
-if (cpu->cfg.ext_zce) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zca), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcb), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmp), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmt), true);
-if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
-}
-}
-
-/* zca, zcd and zcf has a PRIV 1.12.0 restriction */
-if (riscv_has_ext(env, RVC) && env->priv_ver >= PRIV_VERSION_1_12_0) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zca), true);
-if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
-}
-if (riscv_has_ext(env, RVD)) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcd), true);
-}
-}
-
if (mcc->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
error_setg(errp, "Zcf extension is only relevant to RV32");
return;
@@ -627,52 +577,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
Error **errp)
return;
}
-/*
- * Shorthand vector crypto extensions
- */
-if (cpu->cfg.ext_zvknc) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvkn), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvbc), true);
-}
-
-if (cpu->cfg.ext_zvkng) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvkn), true);
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvkg), true);
-}
-
-if (cpu->cfg.ext_zvkn) {
-cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvkned), true);
-
Re: [PATCH v2 5/6] target/riscv: Add Zc extension implied rule
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
Zc extension has special implied rules that need to be handled separately.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/tcg/tcg-cpu.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index f8d6371764..fe84d4402e 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -902,11 +902,45 @@ static void cpu_enable_implied_rule(RISCVCPU *cpu,
}
}
+/* Zc extension has special implied rules that need to be handled separately. */
+static void cpu_enable_zc_implied_rules(RISCVCPU *cpu)
+{
+RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
+CPURISCVState *env = >env;
+
+if (cpu->cfg.ext_zce) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zca), true);
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcb), true);
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmp), true);
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmt), true);
+
+if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
+}
+}
+
+/* Zca, Zcd and Zcf has a PRIV 1.12.0 restriction */
+if (riscv_has_ext(env, RVC) && env->priv_ver >= PRIV_VERSION_1_12_0) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zca), true);
+
+if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
+}
+
+if (riscv_has_ext(env, RVD)) {
+cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcd), true);
+}
+}
+}
+
static void riscv_cpu_enable_implied_rules(RISCVCPU *cpu)
{
RISCVCPUImpliedExtsRule *rule;
int i;
+/* Enable the implied extensions for Zc. */
+cpu_enable_zc_implied_rules(cpu);
+
/* Enable the implied MISAs. */
for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
if (riscv_has_ext(>env, rule->ext)) {
Re: [PATCH v2 4/6] target/riscv: Add standard extension implied rules
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
Add standard extension implied rules to enable the implied extensions of
the standard extension recursively.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
Acked-by: Alistair Francis
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/cpu.c | 340 +
1 file changed, 340 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d09b5e9e62..1a3b1387e1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2297,12 +2297,352 @@ static RISCVCPUImpliedExtsRule RVV_IMPLIED = {
},
};
+static RISCVCPUImpliedExtsRule ZCB_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zcb),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zca),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZCD_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zcd),
+.implied_misas = RVD,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zca),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZCE_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zce),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zcb), CPU_CFG_OFFSET(ext_zcmp),
+CPU_CFG_OFFSET(ext_zcmt),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZCF_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zcf),
+.implied_misas = RVF,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zca),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZCMP_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zcmp),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zca),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZCMT_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zcmt),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zca), CPU_CFG_OFFSET(ext_zicsr),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZDINX_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zdinx),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zfinx),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZFA_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zfa),
+.implied_misas = RVF,
+.implied_exts = { RISCV_IMPLIED_EXTS_RULE_END },
+};
+
+static RISCVCPUImpliedExtsRule ZFBFMIN_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zfbfmin),
+.implied_misas = RVF,
+.implied_exts = { RISCV_IMPLIED_EXTS_RULE_END },
+};
+
+static RISCVCPUImpliedExtsRule ZFH_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zfh),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zfhmin),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZFHMIN_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zfhmin),
+.implied_misas = RVF,
+.implied_exts = { RISCV_IMPLIED_EXTS_RULE_END },
+};
+
+static RISCVCPUImpliedExtsRule ZFINX_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zfinx),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zicsr),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZHINX_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zhinx),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zhinxmin),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZHINXMIN_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zhinxmin),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zfinx),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZICNTR_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zicntr),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zicsr),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZIHPM_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zihpm),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zicsr),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZK_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zk),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zkn), CPU_CFG_OFFSET(ext_zkr),
+CPU_CFG_OFFSET(ext_zkt),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZKN_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zkn),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zbkb), CPU_CFG_OFFSET(ext_zbkc),
+CPU_CFG_OFFSET(ext_zbkx), CPU_CFG_OFFSET(ext_zkne),
+CPU_CFG_OFFSET(ext_zknd), CPU_CFG_OFFSET(ext_zknh),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZKS_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zks),
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zbkb), CPU_CFG_OFFSET(ext_zbkc),
+CPU_CFG_OFFSET(ext_zbkx), CPU_CFG_OFFSET(ext_zksed),
+CPU_CFG_OFFSET(ext_zksh),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule ZVBB_IMPLIED = {
+.ext = CPU_CFG_OFFSET(ext_zvbb),
+.implied_exts = {
+
Re: [PATCH v2 3/6] target/riscv: Add MISA implied rules
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
Add MISA extension implied rules to enable the implied extensions
of MISA recursively.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
Reviewed-by: Alistair Francis
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/cpu.c | 50 +-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index bacbb32120..d09b5e9e62 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2250,8 +2250,56 @@ RISCVCPUProfile *riscv_profiles[] = {
NULL,
};
+static RISCVCPUImpliedExtsRule RVA_IMPLIED = {
+.is_misa = true,
+.ext = RVA,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zalrsc), CPU_CFG_OFFSET(ext_zaamo),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule RVD_IMPLIED = {
+.is_misa = true,
+.ext = RVD,
+.implied_misas = RVF,
+.implied_exts = { RISCV_IMPLIED_EXTS_RULE_END },
+};
+
+static RISCVCPUImpliedExtsRule RVF_IMPLIED = {
+.is_misa = true,
+.ext = RVF,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zicsr),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule RVM_IMPLIED = {
+.is_misa = true,
+.ext = RVM,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zmmul),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
+static RISCVCPUImpliedExtsRule RVV_IMPLIED = {
+.is_misa = true,
+.ext = RVV,
+.implied_exts = {
+CPU_CFG_OFFSET(ext_zve64d),
+
+RISCV_IMPLIED_EXTS_RULE_END
+},
+};
+
RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[] = {
-NULL
+_IMPLIED, _IMPLIED, _IMPLIED,
+_IMPLIED, _IMPLIED, NULL
};
RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[] = {
Re: [PATCH v2 2/6] target/riscv: Introduce extension implied rule helpers
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
Introduce helpers to enable the extensions based on the implied rules.
The implied extensions are enabled recursively, so we don't have to
expand all of them manually. This also eliminates the old-fashioned
ordering requirement. For example, Zvksg implies Zvks, Zvks implies
Zvksed, etc., removing the need to check the implied rules of Zvksg
before Zvks.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/tcg/tcg-cpu.c | 91 ++
1 file changed, 91 insertions(+)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index eb6f7b9d12..f8d6371764 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -36,6 +36,9 @@
static GHashTable *multi_ext_user_opts;
static GHashTable *misa_ext_user_opts;
+static GHashTable *misa_implied_rules;
+static GHashTable *ext_implied_rules;
+
static bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
{
return g_hash_table_contains(multi_ext_user_opts,
@@ -836,11 +839,97 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu)
}
}
+static void riscv_cpu_init_implied_exts_rules(void)
+{
+RISCVCPUImpliedExtsRule *rule;
+int i;
+
+for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
+g_hash_table_insert(misa_implied_rules, GUINT_TO_POINTER(rule->ext),
+(gpointer)rule);
+}
+
+for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
+g_hash_table_insert(ext_implied_rules, GUINT_TO_POINTER(rule->ext),
+(gpointer)rule);
+}
+}
+
+static void cpu_enable_implied_rule(RISCVCPU *cpu,
+RISCVCPUImpliedExtsRule *rule)
+{
+CPURISCVState *env = >env;
+RISCVCPUImpliedExtsRule *ir;
+bool enabled = false;
+int i;
+
+#ifndef CONFIG_USER_ONLY
+enabled = qatomic_read(>enabled) & BIT_ULL(cpu->env.mhartid);
+#endif
+
+if (!enabled) {
+/* Enable the implied MISAs. */
+if (rule->implied_misas) {
+riscv_cpu_set_misa_ext(env, env->misa_ext | rule->implied_misas);
+
+for (i = 0; misa_bits[i] != 0; i++) {
+if (rule->implied_misas & misa_bits[i]) {
+ir = g_hash_table_lookup(misa_implied_rules,
+ GUINT_TO_POINTER(misa_bits[i]));
+
+if (ir) {
+cpu_enable_implied_rule(cpu, ir);
+}
+}
+}
+}
+
+/* Enable the implied extensions. */
+for (i = 0; rule->implied_exts[i] != RISCV_IMPLIED_EXTS_RULE_END; i++)
{
+cpu_cfg_ext_auto_update(cpu, rule->implied_exts[i], true);
+
+ir = g_hash_table_lookup(ext_implied_rules,
+ GUINT_TO_POINTER(rule->implied_exts[i]));
+
+if (ir) {
+cpu_enable_implied_rule(cpu, ir);
+}
+}
+
+#ifndef CONFIG_USER_ONLY
+qatomic_or(>enabled, BIT_ULL(cpu->env.mhartid));
+#endif
+}
+}
+
+static void riscv_cpu_enable_implied_rules(RISCVCPU *cpu)
+{
+RISCVCPUImpliedExtsRule *rule;
+int i;
+
+/* Enable the implied MISAs. */
+for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
+if (riscv_has_ext(>env, rule->ext)) {
+cpu_enable_implied_rule(cpu, rule);
+}
+}
+
+/* Enable the implied extensions. */
+for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
+if (isa_ext_is_enabled(cpu, rule->ext)) {
+cpu_enable_implied_rule(cpu, rule);
+}
+}
+}
+
void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
{
CPURISCVState *env = >env;
Error *local_err = NULL;
+riscv_cpu_init_implied_exts_rules();
+riscv_cpu_enable_implied_rules(cpu);
+
riscv_cpu_validate_misa_priv(env, _err);
if (local_err != NULL) {
error_propagate(errp, local_err);
@@ -1346,6 +1435,8 @@ static void riscv_tcg_cpu_instance_init(CPUState *cs)
misa_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
+misa_implied_rules = g_hash_table_new(NULL, g_direct_equal);
+ext_implied_rules = g_hash_table_new(NULL, g_direct_equal);
riscv_cpu_add_user_properties(obj);
if (riscv_cpu_has_max_extensions(obj)) {
Re: [PATCH v2 1/6] target/riscv: Introduce extension implied rules definition
On 6/15/24 11:46 PM, frank.ch...@sifive.com wrote:
From: Frank Chang
RISCVCPUImpliedExtsRule is created to store the implied rules.
'is_misa' flag is used to distinguish whether the rule is derived
from the MISA or other extensions.
'ext' stores the MISA bit if 'is_misa' is true. Otherwise, it stores
the offset of the extension defined in RISCVCPUConfig. 'ext' will also
serve as the key of the hash tables to look up the rule in the following
commit.
Signed-off-by: Frank Chang
Reviewed-by: Jerry Zhang Jian
Tested-by: Max Chou
---
Reviewed-by: Daniel Henrique Barboza
target/riscv/cpu.c | 8
target/riscv/cpu.h | 25 +
2 files changed, 33 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4760cb2cc1..bacbb32120 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2250,6 +2250,14 @@ RISCVCPUProfile *riscv_profiles[] = {
NULL,
};
+RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[] = {
+NULL
+};
+
+RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[] = {
+NULL
+};
+
static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 90b8f1b08f..6b31731fa8 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -124,6 +124,31 @@ typedef enum {
EXT_STATUS_DIRTY,
} RISCVExtStatus;
+typedef struct riscv_cpu_implied_exts_rule RISCVCPUImpliedExtsRule;
+
+struct riscv_cpu_implied_exts_rule {
+#ifndef CONFIG_USER_ONLY
+/*
+ * Bitmask indicates the rule enabled status for the harts.
+ * This enhancement is only available in system-mode QEMU,
+ * as we don't have a good way (e.g. mhartid) to distinguish
+ * the SMP cores in user-mode QEMU.
+ */
+uint64_t enabled;
+#endif
+/* True if this is a MISA implied rule. */
+bool is_misa;
+/* ext is MISA bit if is_misa flag is true, else extension offset. */
+const uint32_t ext;
+const uint32_t implied_misas;
+const uint32_t implied_exts[];
+};
+
+extern RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[];
+extern RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[];
+
+#define RISCV_IMPLIED_EXTS_RULE_END -1
+
#define MMU_USER_IDX 3
#define MAX_RISCV_PMPS (16)
Re: [PULL 65/76] hw/mips/loongson3_virt: Wire up loongson_ipi device
Hi Jiaxun,
On 18/6/24 18:00, Philippe Mathieu-Daudé wrote:
From: Jiaxun Yang
Wire up loongson_ipi device for loongson3_virt machine, so we
can have SMP support for TCG backend as well.
Signed-off-by: Jiaxun Yang
Acked-by: Song Gao
Reviewed-by: Philippe Mathieu-Daudé
Message-ID: <20240605-loongson3-ipi-v3-3-ddd2c0e03...@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé
---
hw/mips/loongson3_bootp.h | 3 +++
hw/mips/loongson3_bootp.c | 2 --
hw/mips/loongson3_virt.c | 39 +--
hw/mips/Kconfig | 1 +
4 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/hw/mips/loongson3_bootp.c b/hw/mips/loongson3_bootp.c
index 03a10b63c1..b97b81903b 100644
--- a/hw/mips/loongson3_bootp.c
+++ b/hw/mips/loongson3_bootp.c
@@ -25,8 +25,6 @@
#include "hw/boards.h"
#include "hw/mips/loongson3_bootp.h"
-#define LOONGSON3_CORE_PER_NODE 4
-
static void init_cpu_info(void *g_cpuinfo, uint64_t cpu_freq)
{
struct efi_cpuinfo_loongson *c = g_cpuinfo;
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 440268a074..4ad36f0c5b 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -36,6 +36,7 @@
#include "hw/mips/loongson3_bootp.h"
#include "hw/misc/unimp.h"
#include "hw/intc/i8259.h"
+#include "hw/intc/loongson_ipi.h"
#include "hw/loader.h"
#include "hw/isa/superio.h"
#include "hw/pci/msi.h"
@@ -74,6 +75,7 @@ const MemMapEntry virt_memmap[] = {
[VIRT_PCIE_ECAM] = { 0x1a00, 0x200 },
[VIRT_BIOS_ROM] ={ 0x1fc0, 0x20 },
[VIRT_UART] ={ 0x1fe001e0, 0x8 },
+[VIRT_IPI] = { 0x3ff01000, 0x400 },
[VIRT_LIOINTC] = { 0x3ff01400, 0x64 },
[VIRT_PCIE_MMIO] = { 0x4000,0x4000 },
[VIRT_HIGHMEM] = { 0x8000, 0x0 }, /* Variable */
@@ -485,6 +487,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
Clock *cpuclk;
CPUMIPSState *env;
DeviceState *liointc;
+DeviceState *ipi = NULL;
char *filename;
const char *kernel_cmdline = machine->kernel_cmdline;
const char *kernel_filename = machine->kernel_filename;
@@ -494,6 +497,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
MemoryRegion *ram = g_new(MemoryRegion, 1);
MemoryRegion *bios = g_new(MemoryRegion, 1);
MemoryRegion *iomem = g_new(MemoryRegion, 1);
+MemoryRegion *iocsr = g_new(MemoryRegion, 1);
/* TODO: TCG will support all CPU types */
if (!kvm_enabled()) {
@@ -527,6 +531,19 @@ static void mips_loongson3_virt_init(MachineState *machine)
create_unimplemented_device("mmio fallback 0", 0x1000, 256 * MiB);
create_unimplemented_device("mmio fallback 1", 0x3000, 256 * MiB);
+memory_region_init(iocsr, OBJECT(machine), "loongson3.iocsr", UINT32_MAX);
+
+/* IPI controller is in kernel for KVM */
+if (!kvm_enabled()) {
+ipi = qdev_new(TYPE_LOONGSON_IPI);
+qdev_prop_set_uint32(ipi, "num-cpu", machine->smp.cpus);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), _fatal);
+memory_region_add_subregion(iocsr, SMP_IPI_MAILBOX,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
0));
+memory_region_add_subregion(iocsr, MAIL_SEND_ADDR,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
1));
+}
+
liointc = qdev_new("loongson.liointc");
sysbus_realize_and_unref(SYS_BUS_DEVICE(liointc), _fatal);
@@ -543,6 +560,8 @@ static void mips_loongson3_virt_init(MachineState *machine)
clock_set_hz(cpuclk, DEF_LOONGSON3_FREQ);
for (i = 0; i < machine->smp.cpus; i++) {
+int node = i / LOONGSON3_CORE_PER_NODE;
+int core = i % LOONGSON3_CORE_PER_NODE;
int ip;
/* init CPUs */
@@ -553,12 +572,28 @@ static void mips_loongson3_virt_init(MachineState
*machine)
cpu_mips_clock_init(cpu);
qemu_register_reset(main_cpu_reset, cpu);
-if (i >= 4) {
+if (ipi) {
Coverity mentions a Null pointer dereference (REVERSE_INULL) here
(CID 1547264):
Null-checking "ipi" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
Maybe this silences it:
-if (ipi) {
+if (!kvm_enabled()) {
Still I'd rather using tcg_enabled() in this file.
Do you mind posting a patch fixing it?
+hwaddr base = ((hwaddr)node << 44) + virt_memmap[VIRT_IPI].base;
+base += core * 0x100;
+qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
+sysbus_mmio_map(SYS_BUS_DEVICE(ipi), i + 2, base);
+}
+
+if (ase_lcsr_available(_CPU(cpu)->env)) {
+MemoryRegion *core_iocsr = g_new(MemoryRegion, 1);
+g_autofree char *name = g_strdup_printf("core%d_iocsr", i);
+memory_region_init_alias(core_iocsr,
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
Il gio 20 giu 2024, 20:13 Manos Pitsidianakis <
manos.pitsidiana...@linaro.org> ha scritto:
> On Thu, 20 Jun 2024 16:21, Paolo Bonzini wrote:
> >On 6/19/24 22:13, Manos Pitsidianakis wrote:
> >> Add options for Rust in meson_options.txt, meson.build, configure to
> >> prepare for adding Rust code in the followup commits.
> >>
> >> `rust` is a reserved meson name, so we have to use an alternative.
> >> `with_rust` was chosen.
> >>
> >> A cargo_wrapper.py script is added that is heavily based on the work of
> >> Marc-André Lureau from 2021.
> >>
> >>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> >>
> >> Signed-off-by: Marc-André Lureau
> >> Signed-off-by: Manos Pitsidianakis
> >
> >The cargo_wrapper.py script is not used yet, so it should be
> >delayed until it's used.
>
> That's true, I just wanted to make review easier by splitting it out.
> Can we squash them later or do you think I should I do it for the next
> series version?
>
I guess it depends on what the next version looks like. If you start
working on the workspace/build tree/Kconfig parts, it might not have a lot
of cargo_wrapper.py code surviving.
Feel free to take this patch and add
Signed-off-by: Paolo Bonzini
Paolo
>For the detection of the toolchain, I'd rather do everything in
> >configure since that's where the cross file is built. Something like:
> >
> >diff --git a/configure b/configure
> >index 8b6a2f16ceb..6412a1021c3 100755
> >--- a/configure
> >+++ b/configure
> >@@ -173,6 +173,8 @@ fi
> >
> > # default parameters
> > container_engine="auto"
> >+rust_target_triple=""
> >+with_rust="no"
> > cpu=""
> > cross_compile="no"
> > cross_prefix=""
> >@@ -201,6 +202,8 @@ for opt do
> >--cross-prefix=*) cross_prefix="$optarg"
> > cross_compile="yes"
> >;;
> >+ --cargo=*) CARGO="$optarg"
> >+ ;;
> >--cc=*) CC="$optarg"
> >;;
> >--cxx=*) CXX="$optarg"
> >@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
> > pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
> > sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
> >
> >+cargo="${CARGO-cargo}"
> >+
> > check_define() {
> > cat > $TMPC < > #if !defined($1)
> >@@ -628,6 +635,8 @@ for opt do
> >;;
> >--cross-prefix=*)
> >;;
> >+ --cargo=*)
> >+ ;;
> >--cc=*)
> >;;
> >--host-cc=*) host_cc="$optarg"
> >@@ -755,8 +764,14 @@ for opt do
> >;;
> >--container-engine=*) container_engine="$optarg"
> >;;
> >+ --rust-target-triple=*) rust_target_triple="$optarg"
> >+ ;;
> >--gdb=*) gdb_bin="$optarg"
> >;;
> >+ --with-rust) with_rust=yes
> >+ ;;
> >+ --without-rust) with_rust=no
> >+ ;;
> ># everything else has the same name in configure and meson
> >--*) meson_option_parse "$opt" "$optarg"
> >;;
> >@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
> > Advanced options (experts only):
> >-Dmesonoptname=val passthrough option to meson unmodified
> >--cross-prefix=PREFIXuse PREFIX for compile tools, PREFIX can be
> blank [$cross_prefix]
> >+ --cargo=CARGOuse Cargo binary CARGO [$cargo]
> >--cc=CC use C compiler CC [$cc]
> >--host-cc=CC when cross compiling, use C compiler CC for
> code run
> > at build time [$host_cc]
> >@@ -869,11 +885,13 @@ Advanced options (experts only):
> >--python=PYTHON use specified python [$python]
> >--ninja=NINJAuse specified ninja [$ninja]
> >--static enable static build [$static]
> >- --without-default-features default all --enable-* options to "disabled"
> >- --without-default-devices do not include any device that is not
> needed to
> >+ --rust-target-triple=TRIPLE target for Rust cross compilation
> >+ --without-default-features default all --enable-* options to
> "disabled"
> >+ --without-default-devicesdo not include any device that is not
> needed to
> > start the emulator (only use if you are
> including
> > desired devices in configs/devices/)
> >--with-devices-ARCH=NAME override default configs/devices
> >+ --with-rust enable experimental Rust code
> >--enable-debug enable common debug build options
> >--cpu=CPUBuild for host CPU [$cpu]
> >--disable-containers don't use containers for cross-building
> >@@ -1138,6 +1159,20 @@ EOF
> >fi
> > fi
> >
> >+##
> >+# detect rust triples
> >+
> >+if test "$with_rust" = yes; then
> >+ $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
> >+ if test $? != 0; then
> >+error_exit "could not execute cargo binary \"$CARGO\""
> >+ fi
> >+ rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
> >+ if test "$rust_target_triple" = ""; then
> >+rust_target_triple=$rust_host_triple
> >+ fi
> >+fi
> >+
> >
Re: [PATCH v2 09/12] plugins: add migration blocker
On 6/20/24 08:22, Alex Bennée wrote: If the plugin in controlling time there is some state that might be missing from the plugin tracking it. Migration is unlikely to work in this case so lets put a migration blocker in to let the user know if they try. Signed-off-by: Alex Bennée Suggested-by: "Dr. David Alan Gilbert" --- plugins/api.c | 8 1 file changed, 8 insertions(+) diff --git a/plugins/api.c b/plugins/api.c index 4431a0ea7e..c4239153af 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -47,6 +47,8 @@ #include "disas/disas.h" #include "plugin.h" #ifndef CONFIG_USER_ONLY +#include "qapi/error.h" +#include "migration/blocker.h" #include "exec/ram_addr.h" #include "qemu/plugin-memory.h" #include "hw/boards.h" @@ -589,11 +591,17 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry) * Time control */ static bool has_control; +Error *migration_blocker; static. With that, Reviewed-by: Richard Henderson r~
Re: How to use designware-root-port and designware-root-host devices ?
Thanks for the answers, I could move forward a bit more. I'm going/I need to to create a "virt" machine with designware PCI controller for simulation purposes. Will get back with progress in case anyone is interested in results. Thank you again for your time and support. Arthur On Thu, Jun 20, 2024, 23:05 Peter Maydell wrote: > On Thu, 20 Jun 2024 at 18:34, Thomas Huth wrote: > > > > On 20/06/2024 10.28, Arthur Tumanyan wrote: > > > From the other hand the device is declared as non pluggable: > > > dc->user_creatable = false; > > > > Well, that means that you cannot use those with "-device". They can only > be > > instantiated via the code that creates the machine. > > > > > Can you please help me to use designware-root-host/port devices ? > > > > It seems like the i.MX7 SABRE machine is using this device, so instead of > > "-M virt", you could have a try with "-M mcimx7d-sabre" (and a kernel > that > > supports this machine) instead. > > Right -- these devices are the PCIe controller that's used on the i.MX7 > and i.MX6 SoCs, and they're automatically created when you use a machine > type that uses one of those SoCs. The "virt" board doesn't use that > PCIe controller, it uses the "generic PCIe bridge" TYPE_GPEX_HOST > (and you automatically get a PCIe controller when you use the virt board). > You can't change the PCIe controller type of a QEMU machine from > the command line, you have to configure the guest to use the controller > the machine type provides. > > thanks > -- PMM >
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On 6/20/24 11:36, Manos Pitsidianakis wrote:
On Thu, 20 Jun 2024 17:01, Richard Henderson
wrote:
On 6/19/24 13:13, Manos Pitsidianakis wrote:
+# FIXME: These are the latest stable versions, refine to actual minimum ones.
+msrv = {
+ 'rustc': '1.79.0',
+ 'cargo': '1.79.0',
+ 'bindgen': '0.69.4',
+}
A note for other rust newbies:
These versions are pretty darn close to actual minima. Ubuntu 24.04 packages rust 1.77,
which does not support (but has a warning reserving syntax for)
+ println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
rerun-if-env-changed is not new, I think, but the `cargo::` instead of `cargo:` syntax is.
Is this what the warning is saying?
Source
:
Note: The old invocation prefix cargo: (one colon only) is deprecated
and won’t get any new features. To migrate, use two-colons prefix
cargo::, which was added in Rust 1.77. If you were using
cargo:KEY=VALUE for arbitrary links manifest key-value pairs, it is
encouraged to switch to cargo::metadata=KEY=VALUE. Stick to cargo:
only if the support of Rust version older than 1.77 is required.
But this is not in any way necessary for us, we can ignore cargo's stale build detection
and force it from meson.
Yes indeed. The exact error reported is
error: unsupported output in build script of `pl011 v0.1.0
(/home/rth/qemu/src/rust/pl011)`: `cargo::rerun-if-env-changed=MESON_BUILD_DIR`
Found a `cargo::key=value` build directive which is reserved for future use.
Either change the directive to `cargo:key=value` syntax (note the single `:`) or upgrade
your version of Rust.
r~
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
On Thu, 20 Jun 2024 at 17:24, Roman Kiryanov wrote: > > Hi Daniel and Alex, > > On Thu, Jun 20, 2024 at 8:10 AM Alex Bennée wrote: > > > > Daniel P. Berrangé writes: > > > NB, QEMU is explicitly *NOT* targetting the C standard, we are > > > targetting the C dialect supported by GCC and CLang only. IOW, > > > if they have well defined behaviour for arithmetic on void *, > > > then we are free to use it. > > > > It looks like GNU C does support it: > > GCC does support void* pointer arithmetic as an extension. But if you > decide to use other compilers, you might not have the same luck. We > (and maybe other developers) would like to use the QEMU headers with a > C++ compiler where this extension is not available. I think this is the point where I would say "you're making the code worse for upstream and the only benefit is to your out-of-tree downstream code". If you want to build QEMU, use one of the compilers that QEMU supports. There are lots and lots of places where we assume the GCC-or-clang feature set over and above plain C. thanks -- PMM
Re: How to use designware-root-port and designware-root-host devices ?
On Thu, 20 Jun 2024 at 18:34, Thomas Huth wrote: > > On 20/06/2024 10.28, Arthur Tumanyan wrote: > > From the other hand the device is declared as non pluggable: > > dc->user_creatable = false; > > Well, that means that you cannot use those with "-device". They can only be > instantiated via the code that creates the machine. > > > Can you please help me to use designware-root-host/port devices ? > > It seems like the i.MX7 SABRE machine is using this device, so instead of > "-M virt", you could have a try with "-M mcimx7d-sabre" (and a kernel that > supports this machine) instead. Right -- these devices are the PCIe controller that's used on the i.MX7 and i.MX6 SoCs, and they're automatically created when you use a machine type that uses one of those SoCs. The "virt" board doesn't use that PCIe controller, it uses the "generic PCIe bridge" TYPE_GPEX_HOST (and you automatically get a PCIe controller when you use the virt board). You can't change the PCIe controller type of a QEMU machine from the command line, you have to configure the guest to use the controller the machine type provides. thanks -- PMM
Re: [PATCH] target/arm/helper: Fix timer interrupt masking when HCR_EL2.E2H == 0
On Thu, 20 Jun 2024 at 14:56, Florian Lugou wrote:
>
> On Thu, Jun 20, 2024 at 11:43:17AM +0100, Peter Maydell wrote:
> > For this timer check, we're doing I think the same thing as the
> > pseudocode AArch64.CheckTimerConditions(), which does:
> >
> > if (IsFeatureImplemented(FEAT_RME) && ss IN {SS_Root, SS_Realm} &&
> > CNTHCTL_EL2.CNTPMASK == '1') then
> > imask = '1';
> >
> > so I'm inclined to say that our current implementation in QEMU is correct.
>
> Indeed. I got confused with the specification, my apologies.
>
> I am facing an issue with QEMU freezing waiting for a timer interrupt when
> running with -icount shift=0,sleep=off. Bissection has shown that the issue
> appeared with f6fc36deef6abcee406211f3e2f11ff894b87fa4.
>
> Further testing suggests that the issue may come from gt_recalc_timer. Calling
> gt_update_irq before timer_mod (as it was done before f6fc36deef6a) rather
> than
> at the end of the function solves the issue. Is it possible that timer_mod
> relies on cpu->gt_timer_outputs, which has not been modified at this point to
> reflect the timer triggering?
I don't *think* it ought to care -- timer_mod() tells QEMU's timer
infrastructure when to schedule the next timer callback for,
and the gt_timer_outputs qemu_irqs tell the interrupt controller
that the interrupt lines have changed state.
Do you have a reproduce case?
-- PMM
Re: [PATCH] hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu
On Thu, 20 Jun 2024 at 12:16, Edgar E. Iglesias wrote:
>
> On Thu, Jun 20, 2024 at 12:25:51PM +0200, Philippe Mathieu-Daudé wrote:
> > On 20/6/24 12:10, Peter Maydell wrote:
> > > On Tue, 18 Jun 2024 at 15:51, Philippe Mathieu-Daudé
> > > wrote:
> > > >
> > > > On 18/6/24 16:40, Zheyu Ma wrote:
> > > > > This commit updates the a9_gtimer_get_current_cpu() function to handle
> > > > > cases where QTest is enabled. When QTest is used, it returns 0 instead
> > > > > of dereferencing the current_cpu, which can be NULL. This prevents the
> > > > > program from crashing during QTest runs.
> > > > >
> > > > > Reproducer:
> > > > > cat << EOF | qemu-system-aarch64 -display \
> > > > > none -machine accel=qtest, -m 512M -machine npcm750-evb -qtest stdio
> > > > > writel 0xf03fe20c 0x26d7468c
> > > > > EOF
> > > > >
> > > > > Signed-off-by: Zheyu Ma
> > > > > ---
> > > > >hw/timer/a9gtimer.c | 5 +
> > > > >1 file changed, 5 insertions(+)
> >
> >
> > > > >if (current_cpu->cpu_index >= s->num_cpu) {
> > > >
> > > > That said, such accesses of @current_cpu from hw/ are dubious.
> > >
> > > True, but I'm not sure we ever settled on the right way to avoid
> > > them, did we?
> >
> > No we didn't, it is still in my TODO list; we might discuss it
> > when I post my RFC.
> >
>
> Yeah, this way of getting the core id is a problem when having multiple
> ARM CPU subsystems (and for heterogenous cores).
>
> IIRC, when I looked at what the GIC v2 HW does, the GIC exposes an AMBA
> port for each CPU. In my mental model that would translate to exposing
> multiple Memory Reginos (sysbus_init_mmio) and mapping the appropriate
> device MR to each CPU AddressSpace.
Yeah. The trouble is that doing this requires massively rearranging
how all the GICv2 board models connect up address spaces...
> We could also do it with memory attributes but I don't think the
> master Ids are standardised enough to extract a core-index from
> with out having SoC specific code,, at least not accross Xilinx devices.
...and yes, using the requester ID to specify the CPU in the
MemTxAttrs is the other proposal.
thanks
-- PMM
Re: [RFC PATCH 27/34] accel/tcg: Make translate-all.c target independent
On 18/06/24, Richard Henderson wrote:
> On 6/13/24 02:50, Anton Johansson wrote:
> > On 24/01/24, Richard Henderson wrote:
> > > On 1/20/24 00:40, Anton Johansson wrote:
> > > > Makes translate-all.c independent of softmmu target by switching
> > > >
> > > > TARGET_LONG_BITS-> target_long_bits()
> > > >
> > > > TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words,
> > > > target_insn_start_words(),
> > > >
> > > > TCG_GUEST_DEFAULT_MO-> target_default_memory_order()
> > > >
> > > > MO_TE -> target_endian_memory_order()
> > > >
> > > > Signed-off-by: Anton Johansson
> > > > ---
> > > >accel/tcg/translate-all.c | 38 ++
> > > >1 file changed, 18 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> > > > index 9c981d1750..a3ae0c6910 100644
> > > > --- a/accel/tcg/translate-all.c
> > > > +++ b/accel/tcg/translate-all.c
> > > > @@ -65,7 +65,6 @@
> > > >#include "internal-common.h"
> > > >#include "internal-target.h"
> > > >#include "perf.h"
> > > > -#include "tcg/insn-start-words.h"
> > > >TBContext tb_ctx;
> > > > @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp)
> > > >val |= (int64_t)(byte & 0x7f) << shift;
> > > >shift += 7;
> > > >} while (byte & 0x80);
> > > > -if (shift < TARGET_LONG_BITS && (byte & 0x40)) {
> > > > +if (shift < target_long_bits() && (byte & 0x40)) {
> > >
> > > You just make TARGET_PAGE_SIZE etc target independent, right?
> > > So you don't need this? Or is this because of user-only still.
> >
> > Hi, Richard!
> >
> > I missed this piece of feedback previously. I don't see how
> > TARGET_PAGE_[SIZE|BITS] would be used here. Are the values we're
> > encoding always TARGET_PAGE_BITS in size?
>
> I was obviously tired here, since they're obviously unrelated.
>
> However in this case I think TARGET_LONG_BITS is a red herring, and we
> should be using int64_t not target_long at all, and thus the shift count
> must be less than 64.
>
>
> r~
>
Ah I see, thanks!:) I'll give that a go then.
--
Anton Johansson
rev.ng Labs Srl.
Re: [PATCH 09/13] qapi: convert "Note" sections to plain rST
On Thu, Jun 20, 2024 at 11:46 AM John Snow wrote: > > > On Thu, Jun 20, 2024, 9:35 AM Markus Armbruster wrote: > >> Markus Armbruster writes: >> >> > John Snow writes: >> >> [...] >> >> >> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json >> >> index b3de1fb6b3a..57598331c5c 100644 >> >> --- a/qga/qapi-schema.json >> >> +++ b/qga/qapi-schema.json >> >> [...] >> >> >> @@ -631,8 +632,8 @@ >> >> # Errors: >> >> # - If hybrid suspend is not supported, Unsupported >> >> # >> >> -# Notes: It's strongly recommended to issue the guest-sync command >> >> -# before sending commands when the guest resumes >> >> +# .. note:: It's strongly recommended to issue the guest-sync command >> >> +#before sending commands when the guest resumes. >> >> # >> >> # Since: 1.1 >> >> ## >> >> @@ -1461,16 +1462,15 @@ >> >> # * POSIX: as defined by os-release(5) >> >> # * Windows: contains string "server" or "client" >> >> # >> >> -# Notes: On POSIX systems the fields @id, @name, @pretty-name, >> >> -# @version, @version-id, @variant and @variant-id follow the >> >> -# definition specified in os-release(5). Refer to the manual page >> >> -# for exact description of the fields. Their values are taken >> >> -# from the os-release file. If the file is not present in the >> >> -# system, or the values are not present in the file, the fields >> >> -# are not included. >> >> +# .. note:: On POSIX systems the fields @id, @name, @pretty-name, >> >> +#@version, @version-id, @variant and @variant-id follow the >> >> +#definition specified in os-release(5). Refer to the manual page >> for >> >> +#exact description of the fields. Their values are taken from the >> >> +#os-release file. If the file is not present in the system, or >> the >> >> +#values are not present in the file, the fields are not included. >> >> # >> >> -# On Windows the values are filled from information gathered from >> >> -# the system. >> >> +#On Windows the values are filled from information gathered from >> >> +#the system. >> > >> > Please don't change the indentation here. I get the same output with >> > >> > @@ -1461,7 +1462,7 @@ >> ># * POSIX: as defined by os-release(5) >> ># * Windows: contains string "server" or "client" >> ># >> > -# Notes: On POSIX systems the fields @id, @name, @pretty-name, >> > +# .. note:: On POSIX systems the fields @id, @name, @pretty-name, >> ># @version, @version-id, @variant and @variant-id follow the >> ># definition specified in os-release(5). Refer to the manual page >> ># for exact description of the fields. Their values are taken >> >> I'm blind. Actually, you change indentation of subsequent lines from 4 >> to 3 *everywhere*. I guess you do that to make subsequent lines line up >> with the directive, here "note". >> >> Everywhere else, we indent such lines by 4. Hmm. How terrible would it >> be not to mess with the alignment? >> >> If we want to use 3 for directives, is it worth pointing out in the >> commit message? >> >> [...] >> > > Let me look up some conventions and see what's the most prominent... as > well as testing what emacs does by default (or if emacs can be configured > to do what we want with in-tree style config. Warning: I am functionally > inept at emacs lisp. Warning 2x: [neo]vi[m] users, you're entirely on your > own. I'm sorry.) > > I use three myself by force of habit and have some mild reluctance to > respin for that reason, but ... guess we ought to be consistent if we can. > > (No idea where my habit came from. Maybe it is just because it looks nice > to me and no other reason.) > > ((I have no plans, nor desire, to write any kind of checker to enforce > this, though - sorry.)) > Sphinx doc uses three spaces: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#directives ... but it warns that it's arbitrary; but it seems common to align with the directive. * https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#footnotes footnotes require "at least 3" spaces * https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#directives directives are only required to be "indented" but the amount isn't specified. rst docs use three. I'm happy with three; I don't believe we need to make it consistent with e.g. our home-spun field list syntax (arguments, features) or with code blocks. I think whatever looks good in the source is fine, and I think three looks good for directives. I don't think we need to require this, but I can mention in the commit message that I chose it for the sake of aesthetics and for parity with what most rST docs appear to use. Note: emacs behavior for wrapping paragraphs in our QAPI files does not create an indent if there isn't already one. I think convincing emacs to apply rST rules inside of a JSON file we lie and call a Python file might be beyond my ability to do quickly. The
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On Thu, 20 Jun 2024 17:01, Richard Henderson
wrote:
On 6/19/24 13:13, Manos Pitsidianakis wrote:
+# FIXME: These are the latest stable versions, refine to actual minimum ones.
+msrv = {
+ 'rustc': '1.79.0',
+ 'cargo': '1.79.0',
+ 'bindgen': '0.69.4',
+}
A note for other rust newbies:
These versions are pretty darn close to actual minima. Ubuntu 24.04 packages rust 1.77,
which does not support (but has a warning reserving syntax for)
+println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
rerun-if-env-changed is not new, I think, but the `cargo::` instead of
`cargo:` syntax is. Is this what the warning is saying?
Source
:
Note: The old invocation prefix cargo: (one colon only) is deprecated
and won’t get any new features. To migrate, use two-colons prefix
cargo::, which was added in Rust 1.77. If you were using
cargo:KEY=VALUE for arbitrary links manifest key-value pairs, it is
encouraged to switch to cargo::metadata=KEY=VALUE. Stick to cargo:
only if the support of Rust version older than 1.77 is required.
But this is not in any way necessary for us, we can ignore cargo's stale
build detection and force it from meson.
Re: [PATCH v3 0/6] virtio,vhost: Add VIRTIO_F_IN_ORDER support
On Thu, Jun 20, 2024 at 7:56 PM Jonah Palmer wrote: > > The goal of these patches is to add support to a variety of virtio and > vhost devices for the VIRTIO_F_IN_ORDER transport feature. This feature > indicates that all buffers are used by the device in the same order in > which they were made available by the driver. > > These patches attempt to implement a generalized, non-device-specific > solution to support this feature. > > The core feature behind this solution is a buffer mechanism in the form > of a VirtQueue's used_elems VirtQueueElement array. This allows devices > who always use buffers in-order by default to have a minimal overhead > impact. Devices that may not always use buffers in-order likely will > experience a performance hit. How large that performance hit is will > depend on how frequently elements are completed out-of-order. > > A VirtQueue whose device uses this feature will use its used_elems > VirtQueueElement array to hold used VirtQueueElements. The index that > used elements are placed in used_elems is the same index on the > used/descriptor ring that would satisfy the in-order requirement. In > other words, used elements are placed in their in-order locations on > used_elems and are only written to the used/descriptor ring once the > elements on used_elems are able to continue their expected order. > > To differentiate between a "used" and "unused" element on the used_elems > array (a "used" element being an element that has returned from > processing and an "unused" element being an element that has not yet > been processed), we added a boolean 'in_order_filled' member to the > VirtQueueElement struct. This flag is set to true when the element comes > back from processing (virtqueue_ordered_fill) and then set back to false > once it's been written to the used/descriptor ring > (virtqueue_ordered_flush). > > Testing: > > Testing was done using the dpdk-testpmd application on both the host and > guest using the following configurations. Traffic was generated between > the host and guest after running 'start tx_first' on both the host and > guest dpdk-testpmd applications. Results are below after traffic was > generated for several seconds. > > Relevant Qemu args: > --- > -chardev socket,id=char1,path=/tmp/vhost-user1,server=off > -chardev socket,id=char2,path=/tmp/vhost-user2,server=off > -netdev type=vhost-user,id=net1,chardev=char1,vhostforce=on,queues=1 > -netdev type=vhost-user,id=net2,chardev=char2,vhostforce=on,queues=1 > -device virtio-net-pci,in_order=true,packed=true,netdev=net1, > mac=56:48:4f:53:54:00,mq=on,vectors=4,rx_queue_size=256 > -device virtio-net-pci,in_order=true,packed=true,netdev=net2, > mac=56:48:4f:53:54:01,mq=on,vectors=4,rx_queue_size=256 > Hi Jonah, These tests are great, but others should also be performed. In particular, QEMU should run ok with "tap" netdev with vhost=off instead of vhost-user: -netdev type=tap,id=net1,vhost=off -netdev type=tap,id=net2,vhost=off This way, packets are going through the modified code. With this configuration, QEMU is the one forwarding the packets so testpmd is not needed in the host. It's still needed in the guest as linux guest driver does not support in_order. The guest kernel cmdline and testpmd cmdline should require no changes from the configuration you describe here. And then try with in_order=true,packed=false and in_order=true,packed=off in corresponding virtio-net-pci. Performance comparison between in_order=true and in_order=false is also interesting but we're not batching so I don't think we will get an extreme improvement. Does the plan work for you? Thanks! > Host dpdk-testpmd command: > -- > dpdk-testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 > --vdev 'net_vhost0,iface=/tmp/vhost-user1' > --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- > --portmask=f -i --rxq=1 --txq=1 --nb-cores=4 --forward-mode=io > > Guest dpdk-testpmd command: > --- > dpdk-testpmd -l 0,1 -a :00:02.0 -a :00:03.0 -- --portmask=3 > --rxq=1 --txq=1 --nb-cores=1 --forward-mode=io -i > > Results: > > +++ Accumulated forward statistics for all ports+++ > RX-packets: 79067488 RX-dropped: 0 RX-total: 79067488 > TX-packets: 79067552 TX-dropped: 0 TX-total: 79067552 > > > --- > v3: Drop Tested-by tags until patches are re-tested. > Replace 'prev_avail_idx' with 'vq->last_avail_idx - 1' in > virtqueue_split_pop. > Remove redundant '+vq->vring.num' in 'max_steps' calculation in > virtqueue_ordered_fill. > Add test results to CV. > > v2: Make 'in_order_filled' more descriptive. > Change 'j' to more descriptive var name in virtqueue_split_pop. > Use more definitive search conditional in virtqueue_ordered_fill. > Avoid code duplication in
Re: [PATCH v2 12/12] accel/tcg: Avoid unnecessary call overhead from qemu_plugin_vcpu_mem_cb
On 6/20/24 08:22, Alex Bennée wrote:
From: Max Chou
If there are not any QEMU plugin memory callback functions, checking
before calling the qemu_plugin_vcpu_mem_cb function can reduce the
function call overhead.
Signed-off-by: Max Chou
Reviewed-by: Richard Henderson
Reviewed-by: Frank Chang
Message-Id: <20240613175122.1299212-2-max.c...@sifive.com>
---
accel/tcg/ldst_common.c.inc | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/accel/tcg/ldst_common.c.inc b/accel/tcg/ldst_common.c.inc
index c82048e377..87ceb95487 100644
--- a/accel/tcg/ldst_common.c.inc
+++ b/accel/tcg/ldst_common.c.inc
@@ -125,7 +125,9 @@ void helper_st_i128(CPUArchState *env, uint64_t addr,
Int128 val, MemOpIdx oi)
static void plugin_load_cb(CPUArchState *env, abi_ptr addr, MemOpIdx oi)
{
-qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, oi, QEMU_PLUGIN_MEM_R);
+if (cpu_plugin_mem_cbs_enabled(env_cpu(env))) {
+qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, oi, QEMU_PLUGIN_MEM_R);
+}
}
uint8_t cpu_ldb_mmu(CPUArchState *env, abi_ptr addr, MemOpIdx oi, uintptr_t ra)
@@ -188,7 +190,9 @@ Int128 cpu_ld16_mmu(CPUArchState *env, abi_ptr addr,
static void plugin_store_cb(CPUArchState *env, abi_ptr addr, MemOpIdx oi)
{
-qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, oi, QEMU_PLUGIN_MEM_W);
+if (cpu_plugin_mem_cbs_enabled(env_cpu(env))) {
+qemu_plugin_vcpu_mem_cb(env_cpu(env), addr, oi, QEMU_PLUGIN_MEM_W);
+}
}
void cpu_stb_mmu(CPUArchState *env, abi_ptr addr, uint8_t val,
You might want to add the same thing in: accel/tcg/atomic_common.c.inc
Pierrick
Re: [PATCH] plugins/execlog.c: correct dump of registers values
On 6/20/24 01:38, Frédéric Pétrot wrote:
Register values are dumped as 'sz' chunks of two nibbles in the execlog
plugin, sz was 1 too big.
Signed-off-by: Frédéric Pétrot
---
contrib/plugins/execlog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 371db97eb1..1c1601cc0b 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -101,7 +101,7 @@ static void insn_check_regs(CPU *cpu)
GByteArray *temp = reg->last;
g_string_append_printf(cpu->last_exec, ", %s -> 0x", reg->name);
/* TODO: handle BE properly */
-for (int i = sz; i >= 0; i--) {
+for (int i = sz - 1; i >= 0; i--) {
g_string_append_printf(cpu->last_exec, "%02x",
reg->new->data[i]);
}
Good catch, thanks!
Reviewed-by: Pierrick Bouvier
Pierrick
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On Thu, 20 Jun 2024 15:32, Alex Bennée wrote:
Manos Pitsidianakis writes:
Add mechanism to generate rust hw targets that depend on a custom
bindgen target for rust bindings to C.
This way bindings will be created before the rust crate is compiled.
The bindings will end up in BUILDDIR/{target}-generated.rs and have the same
name
as a target:
ninja aarch64-softmmu-generated.rs
+
+
+rust_targets = {}
+
+cargo_wrapper = [
+ find_program(meson.global_source_root() / 'scripts/cargo_wrapper.py'),
+ '--config-headers', meson.project_build_root() / 'config-host.h',
+ '--meson-build-root', meson.project_build_root(),
+ '--meson-build-dir', meson.current_build_dir(),
+ '--meson-source-dir', meson.current_source_dir(),
+]
I'm unclear what the difference between meson-build-root and
meson-build-dir is?
Build-dir is the subdir of the current subdir(...) meson.build file
So if we are building under qemu/build, meson_build_root is qemu/build
and meson_build_dir is qemu/build/rust
We also end up defining crate-dir and outdir. Aren't these all
derivable from whatever module we are building?
Crate dir is the source directory (i.e. qemu/rust/pl011) that contains
the crate's manifest file Cargo.toml.
Outdir is where to put the final build artifact for meson to find. We
could derive that from the build directories and package names somehow
but I chose to be explicit instead of doing indirect logic to make the
process less magic.
I know it's a lot so I'm open to simplifications. The only problem is
that all of these directories, except the crate source code, are defined
from meson and can change with any refactor we do from the meson side of
things.
Re: [PATCH] docs: add precision about capstone for execlog plugin
On 6/20/24 06:57, Alexandre Iooss wrote: Some people are wondering why they get an empty string as disassembly. Most of the time, they configured QEMU without Capstone support. Let's document this behaviour to help users. Signed-off-by: Alexandre Iooss --- docs/devel/tcg-plugins.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst index 9cc09d8c3d..f7d7b9e3a4 100644 --- a/docs/devel/tcg-plugins.rst +++ b/docs/devel/tcg-plugins.rst @@ -539,7 +539,9 @@ which will output an execution trace following this structure:: 0, 0xd34, 0xf9c8f000, "bl #0x10c8" 0, 0x10c8, 0xfff96c43, "ldr r3, [r0, #0x44]", load, 0x20e4, RAM -the output can be filtered to only track certain instructions or +Please note that you need to configure QEMU with Capstone support to get disassembly. + +The output can be filtered to only track certain instructions or addresses using the ``ifilter`` or ``afilter`` options. You can stack the arguments if required:: Reviewed-by: Pierrick Bouvier
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
On Thu, Jun 20, 2024 at 11:16 AM Paolo Bonzini wrote:
> > > Would it work instead to declare MemoryRegionCache's ptr field as char*?
> >
> > I prefer to use char* only where there are strings. For unstructured data
> > such as
> > MemoryRegionCache, void* is more appropriate.
>
> Or uint8_t*... I agree about char*, but unless casts are needed, I
> find uint8_t and void pointers to be more or less interchangeable.
>
> The problem is that casts are a bit uglier and (while unlikely in this
> particular case) more subject to bit rot.
Will `typedef char *MemoryRegionCachePtr;` or making the ptr field
pointing to `struct MemoryRegionCacheData { char unused; }` work
better?
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On Thu, 20 Jun 2024 15:34, Paolo Bonzini wrote:
On Thu, Jun 20, 2024 at 1:10 PM Alex Bennée wrote:
> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> +msrv = {
> + 'rustc': '1.79.0',
> + 'cargo': '1.79.0',
> + 'bindgen': '0.69.4',
> +}
So for Debian Bookworm this comes out as:
msrv = {
'rustc': '1.79.0',
'cargo': '1.79.0',
'bindgen': '0.69.4',
}
I think it's 0.60.1 bindgen and 1.63.0 rustc/cargo? That means we
don't have generic associated types (1.65), which are nice to have but
not absolutely necessary.
The only other one with an old version is Ubuntu 22.04 (1.58.1), but
it has 1.75.0 in updates
Paolo
1.63 is definitely old at this point but we might still be in luck,
(Except for bindgen). I will try running cargo-msrv[0] which finds the
actual minimum supported version of a codebase with a binary search and
see if we can give up features if necessary.
[0]: https://github.com/foresterre/cargo-msrv
Re: [PATCH 1/2] target/arm: Move initialization of debug ID registers
On 6/20/24 11:13, Gustavo Romero wrote: @@ -1268,7 +1268,10 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64SMFR0, FA64, 1); /* FEAT_SME_FA64 */ cpu->isar.id_aa64smfr0 = t; -/* Replicate the same data to the 32-bit id registers. */ +/* + * Replicate the same values from the 32-bit max CPU to the 32-bit ID + * registers. + */ aa32_max_features(cpu); I think the previous comment is more accurate. There is no separate "32-bit max CPU". There is one "max CPU", which supports both 32-bit and 64-bit modes, and thus has both 32-bit and 64-bit ID registers. The rest of the patch looks good. r~
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
On Thu, 20 Jun 2024 16:41, Alex Bennée wrote:
+summary_info += {'Rust support': with_rust}
+if with_rust and get_option('with_rust_target_triple') != ''
+ summary_info += {'Rust target': get_option('with_rust_target_triple')}
+endif
I wonder if we should display the auto-probed triple here as well, not
just when its been overridden?
I agree, once we straighten out host target / cross target detection
logic the target summary info print should be unconditional.
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
On Thu, Jun 20, 2024 at 8:14 PM Richard Henderson wrote: > > On 6/20/24 11:06, Paolo Bonzini wrote: > > On 6/19/24 00:46, Roman Kiryanov wrote: > >> void* pointer arithmetic is not in the > >> C standard. This change allows using > >> the QEMU headers with a C++ compiler. > >> > >> Google-Bug-Id: 331190993 > >> Change-Id: I5a064853429f627c17a9213910811dea4ced6174 > >> Signed-off-by: Roman Kiryanov > > > > Would it work instead to declare MemoryRegionCache's ptr field as char*? > > I prefer to use char* only where there are strings. For unstructured data > such as > MemoryRegionCache, void* is more appropriate. Or uint8_t*... I agree about char*, but unless casts are needed, I find uint8_t and void pointers to be more or less interchangeable. The problem is that casts are a bit uglier and (while unlikely in this particular case) more subject to bit rot. Paolo
[PATCH 0/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max
Enable FEAT_Debugv8p8 on Arm 32 and 64-bit max CPUs. Gustavo Romero (2): target/arm: Move initialization of debug ID registers target/arm: Enable FEAT_Debugv8p8 for -cpu max target/arm/cpu.h | 2 ++ target/arm/tcg/cpu32.c | 34 +- target/arm/tcg/cpu64.c | 9 ++--- 3 files changed, 37 insertions(+), 8 deletions(-) -- 2.34.1
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
On 6/20/24 11:06, Paolo Bonzini wrote: On 6/19/24 00:46, Roman Kiryanov wrote: void* pointer arithmetic is not in the C standard. This change allows using the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: I5a064853429f627c17a9213910811dea4ced6174 Signed-off-by: Roman Kiryanov Would it work instead to declare MemoryRegionCache's ptr field as char*? I prefer to use char* only where there are strings. For unstructured data such as MemoryRegionCache, void* is more appropriate. r~
[PATCH 2/2] target/arm: Enable FEAT_Debugv8p8 for -cpu max
Enable FEAT_Debugv8p8 for 32-bit and 64-bit max CPUs. This feature is out of scope for QEMU since it concerns the external debug interface for JTAG, but is mandatory in Armv8.8 implementations, hence it is reported as supported in the ID registers. Signed-off-by: Gustavo Romero --- target/arm/tcg/cpu32.c | 6 +++--- target/arm/tcg/cpu64.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c index b155a0136f..a1273a73a3 100644 --- a/target/arm/tcg/cpu32.c +++ b/target/arm/tcg/cpu32.c @@ -82,8 +82,8 @@ void aa32_max_features(ARMCPU *cpu) cpu->isar.id_pfr2 = t; t = cpu->isar.id_dfr0; -t = FIELD_DP32(t, ID_DFR0, COPDBG, 9);/* FEAT_Debugv8p4 */ -t = FIELD_DP32(t, ID_DFR0, COPSDBG, 9); /* FEAT_Debugv8p4 */ +t = FIELD_DP32(t, ID_DFR0, COPDBG, 10); /* FEAT_Debugv8p8 */ +t = FIELD_DP32(t, ID_DFR0, COPSDBG, 10); /* FEAT_Debugv8p8 */ t = FIELD_DP32(t, ID_DFR0, PERFMON, 6); /* FEAT_PMUv3p5 */ cpu->isar.id_dfr0 = t; @@ -93,7 +93,7 @@ void aa32_max_features(ARMCPU *cpu) t = 0x8000; t = FIELD_DP32(t, DBGDIDR, SE_IMP, 1); t = FIELD_DP32(t, DBGDIDR, NSUHD_IMP, 1); -t = FIELD_DP32(t, DBGDIDR, VERSION, 6); /* Armv8 debug */ +t = FIELD_DP32(t, DBGDIDR, VERSION, 10); /* FEAT_Debugv8p8 */ t = FIELD_DP32(t, DBGDIDR, CTX_CMPS, 1); t = FIELD_DP32(t, DBGDIDR, BRPS, 5); t = FIELD_DP32(t, DBGDIDR, WRPS, 3); diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index 7d4b88d787..d011755753 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1253,7 +1253,7 @@ void aarch64_max_tcg_initfn(Object *obj) cpu->isar.id_aa64zfr0 = t; t = cpu->isar.id_aa64dfr0; -t = FIELD_DP64(t, ID_AA64DFR0, DEBUGVER, 9); /* FEAT_Debugv8p4 */ +t = FIELD_DP64(t, ID_AA64DFR0, DEBUGVER, 10); /* FEAT_Debugv8p8 */ t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 6);/* FEAT_PMUv3p5 */ t = FIELD_DP64(t, ID_AA64DFR0, HPMN0, 1); /* FEAT_HPMN0 */ cpu->isar.id_aa64dfr0 = t; -- 2.34.1
[PATCH 1/2] target/arm: Move initialization of debug ID registers
Move the initialization of the debug ID registers to aa32_max_features, which is used to set the 32-bit ID registers for both 32-bit and 64-bit max CPUs. This ensures that the debug ID registers are consistently set for both max CPUs in a single place. Signed-off-by: Gustavo Romero --- target/arm/cpu.h | 2 ++ target/arm/tcg/cpu32.c | 30 +++--- target/arm/tcg/cpu64.c | 7 +-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 3841359d0f..d8eb986a04 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2299,6 +2299,8 @@ FIELD(DBGDEVID, DOUBLELOCK, 20, 4) FIELD(DBGDEVID, AUXREGS, 24, 4) FIELD(DBGDEVID, CIDMASK, 28, 4) +FIELD(DBGDEVID1, PCSROFFSET, 0, 4) + FIELD(MVFR0, SIMDREG, 0, 4) FIELD(MVFR0, FPSP, 4, 4) FIELD(MVFR0, FPDP, 8, 4) diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c index bdd82d912a..b155a0136f 100644 --- a/target/arm/tcg/cpu32.c +++ b/target/arm/tcg/cpu32.c @@ -87,6 +87,33 @@ void aa32_max_features(ARMCPU *cpu) t = FIELD_DP32(t, ID_DFR0, PERFMON, 6); /* FEAT_PMUv3p5 */ cpu->isar.id_dfr0 = t; +/* Debug ID registers. */ + +/* Bit[15] is RES1, Bit[13] and Bits[11:0] are RES0. */ +t = 0x8000; +t = FIELD_DP32(t, DBGDIDR, SE_IMP, 1); +t = FIELD_DP32(t, DBGDIDR, NSUHD_IMP, 1); +t = FIELD_DP32(t, DBGDIDR, VERSION, 6); /* Armv8 debug */ +t = FIELD_DP32(t, DBGDIDR, CTX_CMPS, 1); +t = FIELD_DP32(t, DBGDIDR, BRPS, 5); +t = FIELD_DP32(t, DBGDIDR, WRPS, 3); +cpu->isar.dbgdidr = t; + +t = FIELD_DP32(t, DBGDEVID, PCSAMPLE, 3); +t = FIELD_DP32(t, DBGDEVID, WPADDRMASK, 1); +t = FIELD_DP32(t, DBGDEVID, BPADDRMASK, 15); +t = FIELD_DP32(t, DBGDEVID, VECTORCATCH, 0); +t = FIELD_DP32(t, DBGDEVID, VIRTEXTNS, 1); +t = FIELD_DP32(t, DBGDEVID, DOUBLELOCK, 1); +t = FIELD_DP32(t, DBGDEVID, AUXREGS, 0); +t = FIELD_DP32(t, DBGDEVID, CIDMASK, 0); +cpu->isar.dbgdevid = t; + +/* Bits[31:4] are RES0. */ +t = 0; +t = FIELD_DP32(t, DBGDEVID1, PCSROFFSET, 2); +cpu->isar.dbgdevid1 = t; + t = cpu->isar.id_dfr1; t = FIELD_DP32(t, ID_DFR1, HPMN0, 1); /* FEAT_HPMN0 */ cpu->isar.id_dfr1 = t; @@ -955,9 +982,6 @@ static void arm_max_initfn(Object *obj) cpu->isar.id_isar4 = 0x00011142; cpu->isar.id_isar5 = 0x00011121; cpu->isar.id_isar6 = 0; -cpu->isar.dbgdidr = 0x3516d000; -cpu->isar.dbgdevid = 0x00110f13; -cpu->isar.dbgdevid1 = 0x2; cpu->isar.reset_pmcr_el0 = 0x41013000; cpu->clidr = 0x0a200023; cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */ diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index 0899251eef..7d4b88d787 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1167,7 +1167,7 @@ void aarch64_max_tcg_initfn(Object *obj) t = cpu->isar.id_aa64isar2; t = FIELD_DP64(t, ID_AA64ISAR2, MOPS, 1); /* FEAT_MOPS */ -t = FIELD_DP64(t, ID_AA64ISAR2, BC, 1); /* FEAT_HBC */ +t = FIELD_DP64(t, ID_AA64ISAR2, BC, 1); /* FEAT_HBC */ t = FIELD_DP64(t, ID_AA64ISAR2, WFXT, 2); /* FEAT_WFxT */ cpu->isar.id_aa64isar2 = t; @@ -1268,7 +1268,10 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64SMFR0, FA64, 1); /* FEAT_SME_FA64 */ cpu->isar.id_aa64smfr0 = t; -/* Replicate the same data to the 32-bit id registers. */ +/* + * Replicate the same values from the 32-bit max CPU to the 32-bit ID + * registers. + */ aa32_max_features(cpu); #ifdef CONFIG_USER_ONLY -- 2.34.1
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
On Thu, 20 Jun 2024 16:21, Paolo Bonzini wrote:
On 6/19/24 22:13, Manos Pitsidianakis wrote:
Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.
`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.
A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.
https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
Signed-off-by: Marc-André Lureau
Signed-off-by: Manos Pitsidianakis
The cargo_wrapper.py script is not used yet, so it should be
delayed until it's used.
That's true, I just wanted to make review easier by splitting it out.
Can we squash them later or do you think I should I do it for the next
series version?
For the detection of the toolchain, I'd rather do everything in
configure since that's where the cross file is built. Something like:
diff --git a/configure b/configure
index 8b6a2f16ceb..6412a1021c3 100755
--- a/configure
+++ b/configure
@@ -173,6 +173,8 @@ fi
# default parameters
container_engine="auto"
+rust_target_triple=""
+with_rust="no"
cpu=""
cross_compile="no"
cross_prefix=""
@@ -201,6 +202,8 @@ for opt do
--cross-prefix=*) cross_prefix="$optarg"
cross_compile="yes"
;;
+ --cargo=*) CARGO="$optarg"
+ ;;
--cc=*) CC="$optarg"
;;
--cxx=*) CXX="$optarg"
@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
+cargo="${CARGO-cargo}"
+
check_define() {
cat > $TMPC <
+##
+# detect rust triples
+
+if test "$with_rust" = yes; then
+ $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
+ if test $? != 0; then
+error_exit "could not execute cargo binary \"$CARGO\""
+ fi
+ rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
+ if test "$rust_target_triple" = ""; then
+rust_target_triple=$rust_host_triple
+ fi
+fi
+
##
# functions to probe cross compilers
@@ -1604,6 +1639,10 @@ if test "$container" != no; then
echo "RUNC=$runc" >> $config_host_mak
fi
echo "SUBDIRS=$subdirs" >> $config_host_mak
+if test "$with_rust" = yes; then
+ echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
+ echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
+fi
echo "PYTHON=$python" >> $config_host_mak
echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >>
$config_host_mak
echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >>
$cross
+ if test "$with_rust" = yes; then
+if test "$rust_host_triple" != "$rust_target_triple"; then
+ echo "cargo = [$(meson_quote $cargo --target "$rust_target_triple")]" >>
$cross
+else
+ echo "cargo = [$(meson_quote $cargo)]" >> $cross
+fi
+ fi
Hm that looks better indeed, thanks!
echo "ar = [$(meson_quote $ar)]" >> $cross
echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
echo "nm = [$(meson_quote $nm)]" >> $cross
diff --git a/meson.build b/meson.build
index c5360fbd299..ad7dbc0d641 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,11 @@ foreach lang : all_languages
endif
endforeach
+cargo = not_found
+if 'RUST_TARGET_TRIPLE' in config_host
+ cargo = find_program('cargo', required: true)
+endif
+
# default flags for all hosts
# We use -fwrapv to tell the compiler that we require a C dialect where
# left shift of signed integers is well defined and has the expected
@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
else
summary_info += {'Objective-C compiler': false}
endif
+summary_info += {'Rust support': cargo.found()}
+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) !=
config_host['RUST_HOST_TRIPLE']
+ summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
+endif
option_cflags = (get_option('debug') ? ['-g'] : [])
if get_option('optimization') != 'plain'
option_cflags += ['-O' + get_option('optimization')]
Re: [PATCH 3/3] exec: use char* for pointer arithmetic
On 6/19/24 00:46, Roman Kiryanov wrote: void* pointer arithmetic is not in the C standard. This change allows using the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: I5a064853429f627c17a9213910811dea4ced6174 Signed-off-by: Roman Kiryanov Would it work instead to declare MemoryRegionCache's ptr field as char*? Thanks, Paolo
Re: [PATCH 2/3] exec: avoid using C++ keywords in function parameters
On 6/19/24 00:45, Roman Kiryanov wrote:
to use the QEMU headers with a C++ compiler.
Google-Bug-Id: 331190993
Change-Id: Ic4e49b9c791616bb22c973922772b0494706092c
Signed-off-by: Roman Kiryanov
---
include/exec/memory.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1be58f694c..d7591a60d9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -945,7 +945,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_start)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_stop:
@@ -964,7 +964,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_sync:
Queued, thanks.
Paolo
[PATCH v3 3/6] virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support
Add VIRTIO_F_IN_ORDER feature support for the virtqueue_fill operation.
The goal of the virtqueue_ordered_fill operation when the
VIRTIO_F_IN_ORDER feature has been negotiated is to search for this
now-used element, set its length, and mark the element as filled in
the VirtQueue's used_elems array.
By marking the element as filled, it will indicate that this element has
been processed and is ready to be flushed, so long as the element is
in-order.
Reviewed-by: Eugenio Pérez
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio.c | 44 +++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9cbf75f021..e1dfec4655 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -873,6 +873,46 @@ static void virtqueue_packed_fill(VirtQueue *vq, const
VirtQueueElement *elem,
vq->used_elems[idx].ndescs = elem->ndescs;
}
+static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len)
+{
+unsigned int i, steps, max_steps;
+
+i = vq->used_idx;
+steps = 0;
+/*
+ * We shouldn't need to increase 'i' by more than the distance
+ * between used_idx and last_avail_idx.
+ */
+max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
+
+/* Search for element in vq->used_elems */
+while (steps <= max_steps) {
+/* Found element, set length and mark as filled */
+if (vq->used_elems[i].index == elem->index) {
+vq->used_elems[i].len = len;
+vq->used_elems[i].in_order_filled = true;
+break;
+}
+
+i += vq->used_elems[i].ndescs;
+steps += vq->used_elems[i].ndescs;
+
+if (i >= vq->vring.num) {
+i -= vq->vring.num;
+}
+}
+
+/*
+ * We should be able to find a matching VirtQueueElement in
+ * used_elems. If we don't, this is an error.
+ */
+if (steps >= max_steps) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: %s cannot fill buffer id %u\n",
+ __func__, vdev->name, elem->index);
+}
+}
+
static void virtqueue_packed_fill_desc(VirtQueue *vq,
const VirtQueueElement *elem,
unsigned int idx,
@@ -923,7 +963,9 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement
*elem,
return;
}
-if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_IN_ORDER)) {
+virtqueue_ordered_fill(vq, elem, len);
+} else if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
virtqueue_packed_fill(vq, elem, len, idx);
} else {
virtqueue_split_fill(vq, elem, len, idx);
--
2.43.0
[PATCH v3 5/6] vhost, vhost-user: Add VIRTIO_F_IN_ORDER to vhost feature bits
Add support for the VIRTIO_F_IN_ORDER feature across a variety of vhost
devices.
The inclusion of VIRTIO_F_IN_ORDER in the feature bits arrays for these
devices ensures that the backend is capable of offering and providing
support for this feature, and that it can be disabled if the backend
does not support it.
Acked-by: Eugenio Pérez
Signed-off-by: Jonah Palmer
---
hw/block/vhost-user-blk.c| 1 +
hw/net/vhost_net.c | 2 ++
hw/scsi/vhost-scsi.c | 1 +
hw/scsi/vhost-user-scsi.c| 1 +
hw/virtio/vhost-user-fs.c| 1 +
hw/virtio/vhost-user-vsock.c | 1 +
net/vhost-vdpa.c | 1 +
7 files changed, 8 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9e6bbc6950..1dd0a8ef63 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -51,6 +51,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index fd1a93701a..eb0b1c06e5 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -48,6 +48,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VIRTIO_NET_F_HASH_REPORT,
VHOST_INVALID_FEATURE_BIT
};
@@ -76,6 +77,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VIRTIO_NET_F_RSS,
VIRTIO_NET_F_HASH_REPORT,
VIRTIO_NET_F_GUEST_USO4,
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ae26bc19a4..40e7630191 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -38,6 +38,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index a63b1f4948..1d59951ab7 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -36,6 +36,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index cca2cd41be..9243dbb128 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -33,6 +33,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
index 9431b9792c..cc7e4e47b4 100644
--- a/hw/virtio/vhost-user-vsock.c
+++ b/hw/virtio/vhost-user-vsock.c
@@ -21,6 +21,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_INDIRECT_DESC,
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_IN_ORDER,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 85e73dd6a7..ed3185acfa 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -62,6 +62,7 @@ const int vdpa_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
VIRTIO_F_VERSION_1,
+VIRTIO_F_IN_ORDER,
VIRTIO_NET_F_CSUM,
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
VIRTIO_NET_F_CTRL_MAC_ADDR,
--
2.43.0
[PATCH v3 6/6] virtio: Add VIRTIO_F_IN_ORDER property definition
Extend the virtio device property definitions to include the
VIRTIO_F_IN_ORDER feature.
The default state of this feature is disabled, allowing it to be
explicitly enabled where it's supported.
Acked-by: Eugenio Pérez
Signed-off-by: Jonah Palmer
---
include/hw/virtio/virtio.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 88e70c1ae1..d33345ecc5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -371,7 +371,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
DEFINE_PROP_BIT64("packed", _state, _field, \
VIRTIO_F_RING_PACKED, false), \
DEFINE_PROP_BIT64("queue_reset", _state, _field, \
- VIRTIO_F_RING_RESET, true)
+ VIRTIO_F_RING_RESET, true), \
+DEFINE_PROP_BIT64("in_order", _state, _field, \
+ VIRTIO_F_IN_ORDER, false)
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
--
2.43.0
[PATCH v3 0/6] virtio,vhost: Add VIRTIO_F_IN_ORDER support
The goal of these patches is to add support to a variety of virtio and vhost devices for the VIRTIO_F_IN_ORDER transport feature. This feature indicates that all buffers are used by the device in the same order in which they were made available by the driver. These patches attempt to implement a generalized, non-device-specific solution to support this feature. The core feature behind this solution is a buffer mechanism in the form of a VirtQueue's used_elems VirtQueueElement array. This allows devices who always use buffers in-order by default to have a minimal overhead impact. Devices that may not always use buffers in-order likely will experience a performance hit. How large that performance hit is will depend on how frequently elements are completed out-of-order. A VirtQueue whose device uses this feature will use its used_elems VirtQueueElement array to hold used VirtQueueElements. The index that used elements are placed in used_elems is the same index on the used/descriptor ring that would satisfy the in-order requirement. In other words, used elements are placed in their in-order locations on used_elems and are only written to the used/descriptor ring once the elements on used_elems are able to continue their expected order. To differentiate between a "used" and "unused" element on the used_elems array (a "used" element being an element that has returned from processing and an "unused" element being an element that has not yet been processed), we added a boolean 'in_order_filled' member to the VirtQueueElement struct. This flag is set to true when the element comes back from processing (virtqueue_ordered_fill) and then set back to false once it's been written to the used/descriptor ring (virtqueue_ordered_flush). Testing: Testing was done using the dpdk-testpmd application on both the host and guest using the following configurations. Traffic was generated between the host and guest after running 'start tx_first' on both the host and guest dpdk-testpmd applications. Results are below after traffic was generated for several seconds. Relevant Qemu args: --- -chardev socket,id=char1,path=/tmp/vhost-user1,server=off -chardev socket,id=char2,path=/tmp/vhost-user2,server=off -netdev type=vhost-user,id=net1,chardev=char1,vhostforce=on,queues=1 -netdev type=vhost-user,id=net2,chardev=char2,vhostforce=on,queues=1 -device virtio-net-pci,in_order=true,packed=true,netdev=net1, mac=56:48:4f:53:54:00,mq=on,vectors=4,rx_queue_size=256 -device virtio-net-pci,in_order=true,packed=true,netdev=net2, mac=56:48:4f:53:54:01,mq=on,vectors=4,rx_queue_size=256 Host dpdk-testpmd command: -- dpdk-testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 --vdev 'net_vhost0,iface=/tmp/vhost-user1' --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- --portmask=f -i --rxq=1 --txq=1 --nb-cores=4 --forward-mode=io Guest dpdk-testpmd command: --- dpdk-testpmd -l 0,1 -a :00:02.0 -a :00:03.0 -- --portmask=3 --rxq=1 --txq=1 --nb-cores=1 --forward-mode=io -i Results: +++ Accumulated forward statistics for all ports+++ RX-packets: 79067488 RX-dropped: 0 RX-total: 79067488 TX-packets: 79067552 TX-dropped: 0 TX-total: 79067552 --- v3: Drop Tested-by tags until patches are re-tested. Replace 'prev_avail_idx' with 'vq->last_avail_idx - 1' in virtqueue_split_pop. Remove redundant '+vq->vring.num' in 'max_steps' calculation in virtqueue_ordered_fill. Add test results to CV. v2: Make 'in_order_filled' more descriptive. Change 'j' to more descriptive var name in virtqueue_split_pop. Use more definitive search conditional in virtqueue_ordered_fill. Avoid code duplication in virtqueue_ordered_flush. v1: Move series from RFC to PATCH for submission. Jonah Palmer (6): virtio: Add bool to VirtQueueElement virtio: virtqueue_pop - VIRTIO_F_IN_ORDER support virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support virtio: virtqueue_ordered_flush - VIRTIO_F_IN_ORDER support vhost,vhost-user: Add VIRTIO_F_IN_ORDER to vhost feature bits virtio: Add VIRTIO_F_IN_ORDER property definition hw/block/vhost-user-blk.c| 1 + hw/net/vhost_net.c | 2 + hw/scsi/vhost-scsi.c | 1 + hw/scsi/vhost-user-scsi.c| 1 + hw/virtio/vhost-user-fs.c| 1 + hw/virtio/vhost-user-vsock.c | 1 + hw/virtio/virtio.c | 123 ++- include/hw/virtio/virtio.h | 6 +- net/vhost-vdpa.c | 1 + 9 files changed, 134 insertions(+), 3 deletions(-) -- 2.43.0
[PATCH v3 2/6] virtio: virtqueue_pop - VIRTIO_F_IN_ORDER support
Add VIRTIO_F_IN_ORDER feature support in virtqueue_split_pop and
virtqueue_packed_pop.
VirtQueueElements popped from the available/descritpor ring are added to
the VirtQueue's used_elems array in-order and in the same fashion as
they would be added the used and descriptor rings, respectively.
This will allow us to keep track of the current order, what elements
have been written, as well as an element's essential data after being
processed.
Reviewed-by: Eugenio Pérez
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 893a072c9d..9cbf75f021 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1630,6 +1630,12 @@ static void *virtqueue_split_pop(VirtQueue *vq, size_t
sz)
elem->in_sg[i] = iov[out_num + i];
}
+if (virtio_vdev_has_feature(vdev, VIRTIO_F_IN_ORDER)) {
+vq->used_elems[vq->last_avail_idx - 1].index = elem->index;
+vq->used_elems[vq->last_avail_idx - 1].len = elem->len;
+vq->used_elems[vq->last_avail_idx - 1].ndescs = elem->ndescs;
+}
+
vq->inuse++;
trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
@@ -1758,6 +1764,13 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t
sz)
elem->index = id;
elem->ndescs = (desc_cache == _desc_cache) ? 1 : elem_entries;
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_F_IN_ORDER)) {
+vq->used_elems[vq->last_avail_idx].index = elem->index;
+vq->used_elems[vq->last_avail_idx].len = elem->len;
+vq->used_elems[vq->last_avail_idx].ndescs = elem->ndescs;
+}
+
vq->last_avail_idx += elem->ndescs;
vq->inuse += elem->ndescs;
--
2.43.0
[PATCH v3 4/6] virtio: virtqueue_ordered_flush - VIRTIO_F_IN_ORDER support
Add VIRTIO_F_IN_ORDER feature support for the virtqueue_flush operation.
The goal of the virtqueue_ordered_flush operation when the
VIRTIO_F_IN_ORDER feature has been negotiated is to write elements to
the used/descriptor ring in-order and then update used_idx.
The function iterates through the VirtQueueElement used_elems array
in-order starting at vq->used_idx. If the element is valid (filled), the
element is written to the used/descriptor ring. This process continues
until we find an invalid (not filled) element.
For packed VQs, the first entry (at vq->used_idx) is written to the
descriptor ring last so the guest doesn't see any invalid descriptors.
If any elements were written, the used_idx is updated.
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio.c | 66 +-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e1dfec4655..b0b1b556a2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1024,6 +1024,68 @@ static void virtqueue_packed_flush(VirtQueue *vq,
unsigned int count)
}
}
+static void virtqueue_ordered_flush(VirtQueue *vq)
+{
+unsigned int i = vq->used_idx;
+unsigned int ndescs = 0;
+uint16_t old = vq->used_idx;
+bool packed;
+VRingUsedElem uelem;
+
+packed = virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED);
+
+if (packed) {
+if (unlikely(!vq->vring.desc)) {
+return;
+}
+} else if (unlikely(!vq->vring.used)) {
+return;
+}
+
+/* First expected in-order element isn't ready, nothing to do */
+if (!vq->used_elems[i].in_order_filled) {
+return;
+}
+
+/* Search for filled elements in-order */
+while (vq->used_elems[i].in_order_filled) {
+/*
+ * First entry for packed VQs is written last so the guest
+ * doesn't see invalid descriptors.
+ */
+if (packed && i != vq->used_idx) {
+virtqueue_packed_fill_desc(vq, >used_elems[i], ndescs, false);
+} else if (!packed) {
+uelem.id = vq->used_elems[i].index;
+uelem.len = vq->used_elems[i].len;
+vring_used_write(vq, , i);
+}
+
+vq->used_elems[i].in_order_filled = false;
+ndescs += vq->used_elems[i].ndescs;
+i += ndescs;
+if (i >= vq->vring.num) {
+i -= vq->vring.num;
+}
+}
+
+if (packed) {
+virtqueue_packed_fill_desc(vq, >used_elems[vq->used_idx], 0, true);
+vq->used_idx += ndescs;
+if (vq->used_idx >= vq->vring.num) {
+vq->used_idx -= vq->vring.num;
+vq->used_wrap_counter ^= 1;
+vq->signalled_used_valid = false;
+}
+} else {
+vring_used_idx_set(vq, i);
+if (unlikely((int16_t)(i - vq->signalled_used) < (uint16_t)(i - old)))
{
+vq->signalled_used_valid = false;
+}
+}
+vq->inuse -= ndescs;
+}
+
void virtqueue_flush(VirtQueue *vq, unsigned int count)
{
if (virtio_device_disabled(vq->vdev)) {
@@ -1031,7 +1093,9 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
return;
}
-if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_IN_ORDER)) {
+virtqueue_ordered_flush(vq);
+} else if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
virtqueue_packed_flush(vq, count);
} else {
virtqueue_split_flush(vq, count);
--
2.43.0
[PATCH v3 1/6] virtio: Add bool to VirtQueueElement
Add the boolean 'in_order_filled' member to the VirtQueueElement structure. The use of this boolean will signify whether the element has been processed and is ready to be flushed (so long as the element is in-order). This boolean is used to support the VIRTIO_F_IN_ORDER feature. Reviewed-by: Eugenio Pérez Signed-off-by: Jonah Palmer --- include/hw/virtio/virtio.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 7d5ffdc145..88e70c1ae1 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -69,6 +69,8 @@ typedef struct VirtQueueElement unsigned int ndescs; unsigned int out_num; unsigned int in_num; +/* Element has been processed (VIRTIO_F_IN_ORDER) */ +bool in_order_filled; hwaddr *in_addr; hwaddr *out_addr; struct iovec *in_sg; -- 2.43.0
Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions
On Thu, Jun 20, 2024 at 7:22 PM Richard Henderson wrote: > > I'm not sure this makes sense. The CONFIG_AVX* options are used only > > to validate whether the toolchain has support for this. The QEMU > > code then has a runtime, so it automagically uses AVX2/AVX512 > > if-and-only-if running on a suitably new CPU. IOW, we want this > > enabled always when the toolchain supports it, regardless of what > > x86_version is set. > > Indeed. To me this means they should not be configure options at all. > We should simply detect compiler support, end of. Ok, I'll drop this patch then for now. Paolo
Re: [PATCH v14 00/14] Support blob memory and venus on qemu
On 6/19/24 20:37, Alex Bennée wrote: > So I've been experimenting with Aarch64 TCG with an Intel backend like > this: > > ./qemu-system-aarch64 \ >-M virt -cpu cortex-a76 \ >-device virtio-net-pci,netdev=unet \ >-netdev user,id=unet,hostfwd=tcp::-:22 \ >-m 8192 \ >-object memory-backend-memfd,id=mem,size=8G,share=on \ >-serial mon:stdio \ >-kernel > ~/lsrc/linux.git/builds/arm64.initramfs/arch/arm64/boot/Image \ >-append "console=ttyAMA0" \ >-device qemu-xhci -device usb-kbd -device usb-tablet \ >-device virtio-gpu-gl-pci,blob=true,venus=true,hostmem=4G \ >-display sdl,gl=on -d > plugin,guest_errors,trace:virtio_gpu_cmd_res_create_blob,trace:virtio_gpu_cmd_res_back_\*,trace:virtio_gpu_cmd_res_xfer_toh_3d,trace:virtio_gpu_cmd_res_xfer_fromh_3d,trace:address_space_map > > > And I've noticed a couple of things. First trying to launch vkmark to > run a KMS mode test fails with: > ... > virgl_render_server[1875931]: vkr: failed to import resource: invalid > res_id 5 > virgl_render_server[1875931]: vkr: vkAllocateMemory resulted in CS error > virgl_render_server[1875931]: vkr: ring_submit_cmd: vn_dispatch_command > failed > > More interestingly when shutting stuff down we see weirdness like: > > address_space_map as:0x561b48ec48c0 addr 0x1008ac4b0:18 write:1 attrs:0x1 > > > virgl_render_server[1875931]: vkr: destroying context 3 (vkmark) with a > valid instance > > virgl_render_server[1875931]: vkr: destroying device with valid objects > > > vkr_context_remove_object: -7438602987017907480 > > > vkr_context_remove_object: 7 > > > vkr_context_remove_object: 5 > > which indicates something has gone very wrong. I'm not super familiar > with the memory allocation patterns but should stuff that is done as > virtio_gpu_cmd_res_back_attach() be find-able in the list of resources? This is expected to fail. Vkmark creates shmem virgl GBM FB BO on guest that isn't exportable on host. AFAICT, more code changes should be needed to support this case. Note that "destroying device with valid objects" msg is fine, won't hurt to silence it in Venus to avoid confusion. It will happen every time guest application is closed without explicitly releasing every VK object. > I tried running under RR to further debug but weirdly I can't get > working graphics with that. I did try running under threadsan which > complained about a potential data race: > > vkr_context_add_object: 1 -> 0x7b2c0288 > vkr_context_add_object: 2 -> 0x7b2c0270 > vkr_context_add_object: 3 -> 0x7b387f28 > vkr_context_add_object: 4 -> 0x7b387fa0 > vkr_context_add_object: 5 -> 0x7b48000103f8 > vkr_context_add_object: 6 -> 0x7b48000104a0 > vkr_context_add_object: 7 -> 0x7b4800010440 > virtio_gpu_cmd_res_back_attach res 0x5 > virtio_gpu_cmd_res_back_attach res 0x6 > vkr_context_add_object: 8 -> 0x7b48000103e0 > virgl_render_server[1751430]: vkr: failed to import resource: invalid > res_id 5 > virgl_render_server[1751430]: vkr: vkAllocateMemory resulted in CS error > virgl_render_server[1751430]: vkr: ring_submit_cmd: vn_dispatch_command > failed > == > WARNING: ThreadSanitizer: data race (pid=1751256) > Read of size 8 at 0x7f7fa0ea9138 by main thread (mutexes: write M0): > #0 memcpy (qemu-system-aarch64+0x41fede) (BuildId: > 0bab171e77cb6782341ee3407e44af7267974025) .. > == > SUMMARY: ThreadSanitizer: data race > (/home/alex/lsrc/qemu.git/builds/system.threadsan/qemu-system-aarch64+0x41fede) > (BuildId: 0bab171e77cb6782341ee3407e44af7267974025) in __interceptor_memcpy > > This could be a false positive or it could be a race between the guest > kernel clearing memory while we are still doing > virtio_gpu_ctrl_response. > > What do you think? The memcpy warning looks a bit suspicion, but likely is harmless. I don't see such warning with TSAN and x86 VM. -- Best regards, Dmitry
Re: How to use designware-root-port and designware-root-host devices ?
On 20/06/2024 10.28, Arthur Tumanyan wrote:
Hi all,
My question may sound stupid, however...
Hi Arthur,
no worries, the question how to use which device in QEMU can be quite tricky ;-)
Currently I'm trying to make
available designware-root-{port,host} devices in linux when I run it in qemu.
I try the following way to run:
qemu-system-arm -M virt -m 2G \
-kernel images/Image \
-append "rootwait root=/dev/vda ro" \
-drive file=images/rootfs.ext2,format=raw,id=hd0 \
-device designware-root-port,id=rp0,chassis=1,slot=0,bus=pcie.0,addr=1 \
-device e1000,netdev=net0,mac=52:54:00:12:34:56,bus=rp0,addr=0 \
-netdev user,id=net0
but it seems designware device is not enabled by default: qemu-system-arm:
-device designware-root-port,id=rp0,chassis=1,slot=0,bus=pcie.0,addr=1:
'designware-root-port' is not a valid device model name
Are you sure about the names?
$ grep -r 'designware' *
...
include/hw/pci-host/designware.h:#define TYPE_DESIGNWARE_PCIE_HOST
"designware-pcie-host"
include/hw/pci-host/designware.h:#define TYPE_DESIGNWARE_PCIE_ROOT
"designware-pcie-root"
when I enable it from Kconfig/meson.build it says the device is already
registered and exits with abort().
From the other hand the device is declared as non pluggable:
dc->user_creatable = false;
Well, that means that you cannot use those with "-device". They can only be
instantiated via the code that creates the machine.
Can you please help me to use designware-root-host/port devices ?
It seems like the i.MX7 SABRE machine is using this device, so instead of
"-M virt", you could have a try with "-M mcimx7d-sabre" (and a kernel that
supports this machine) instead.
HTH,
Thomas
Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions
On 6/20/24 08:01, Daniel P. Berrangé wrote:
On Thu, Jun 20, 2024 at 03:02:54PM +0200, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index 54e6b09f4fb..c5360fbd299 100644
--- a/meson.build
+++ b/meson.build
@@ -2863,6 +2863,7 @@ have_cpuid_h = cc.links('''
config_host_data.set('CONFIG_CPUID_H', have_cpuid_h)
config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
+ .enable_auto_if(get_option('x86_version') >= '3') \
.require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX2') \
.require(cc.links('''
#include
@@ -2875,6 +2876,7 @@ config_host_data.set('CONFIG_AVX2_OPT',
get_option('avx2') \
'''), error_message: 'AVX2 not available').allowed())
config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
+ .enable_auto_if(get_option('x86_version') >= '4') \
.require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512BW') \
.require(cc.links('''
#include
I'm not sure this makes sense. The CONFIG_AVX* options are used only
to validate whether the toolchain has support for this. The QEMU
code then has a runtime, so it automagically uses AVX2/AVX512
if-and-only-if running on a suitably new CPU. IOW, we want this
enabled always when the toolchain supports it, regardless of what
x86_version is set.
Indeed. To me this means they should not be configure options at all.
We should simply detect compiler support, end of.
r~
Re: [PATCH v2 09/14] include/hw: temporarily disable deletion of versioned machine types
On 20/06/2024 18.57, Daniel P. Berrangé wrote:
The new deprecation and deletion policy for versioned machine types is
being introduced in QEMU 9.1.0.
Under the new policy a number of old machine types (any prior to 2.12)
would be liable for immediate deletion which would be a violation of our
historical deprecation and removal policy
Thus automatic deletions (by skipping QOM registration) are temporarily
gated on existance of the env variable "QEMU_DELETE_MACHINES" / QEMU
version number >= 10.1.0. This allows opt-in testing of the automatic
deletion logic, while activating it fully in QEMU >= 10.1.0.
This whole commit should be reverted in the 10.1.0 dev cycle or shortly
thereafter.
Signed-off-by: Daniel P. Berrangé
---
include/hw/boards.h | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 187ed76646..ef6f18f2c1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -686,11 +686,28 @@ struct MachineState {
* suitable period of time has passed, it will cause
* execution of the method to return, avoiding registration
* of the machine
+ *
+ * The new deprecation and deletion policy for versioned
+ * machine types was introduced in QEMU 9.1.0.
+ *
+ * Under the new policy a number of old machine types (any
+ * prior to 2.12) would be liable for immediate deletion
+ * which would be a violation of our historical deprecation
+ * and removal policy
+ *
+ * Thus deletions are temporarily gated on existance of
+ * the env variable "QEMU_DELETE_MACHINES" / QEMU version
+ * number >= 10.1.0. This gate can be deleted in the 10.1.0
+ * dev cycle
*/
#define MACHINE_VER_DELETION(...) \
do { \
if (MACHINE_VER_SHOULD_DELETE(__VA_ARGS__)) { \
-return; \
+if (getenv("QEMU_DELETE_MACHINES") || \
+QEMU_VERSION_MAJOR > 10 || (QEMU_VERSION_MAJOR == 10 && \
+QEMU_VERSION_MINOR >= 1)) { \
+return; \
+} \
} \
} while (0)
Reviewed-by: Thomas Huth
Re: [PATCH 4/6] meson: allow configuring the x86-64 baseline
On 6/20/24 06:02, Paolo Bonzini wrote: Signed-off-by: Paolo Bonzini --- meson.build | 41 --- meson_options.txt | 3 +++ scripts/meson-buildoptions.sh | 3 +++ 3 files changed, 39 insertions(+), 8 deletions(-) Acked-by: Richard Henderson For -mneeded, we need gcc 11 and for enforcing GNU_PROPERTY_X86_ISA_1_NEEDED we need glibc 2.33, so: debian 12 fedora 34 ubuntu 2204 suse leap 15.6 or tumbleweed. centos stream 9 I believe the -mneeded option will be accepted by FreeBSD's clang, but the note will not be enforced by the dynamic linker at startup. However, since this is all optional, requiring an explicit configure option, I don't think any of this versioning should stand in the way. r~
Re: [PATCH v2 09/12] plugins: add migration blocker
On 20/06/2024 17.22, Alex Bennée wrote:
If the plugin in controlling time there is some state that might be
missing from the plugin tracking it. Migration is unlikely to work in
this case so lets put a migration blocker in to let the user know if
they try.
Signed-off-by: Alex Bennée
Suggested-by: "Dr. David Alan Gilbert"
---
plugins/api.c | 8
1 file changed, 8 insertions(+)
diff --git a/plugins/api.c b/plugins/api.c
index 4431a0ea7e..c4239153af 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -47,6 +47,8 @@
#include "disas/disas.h"
#include "plugin.h"
#ifndef CONFIG_USER_ONLY
+#include "qapi/error.h"
+#include "migration/blocker.h"
#include "exec/ram_addr.h"
#include "qemu/plugin-memory.h"
#include "hw/boards.h"
@@ -589,11 +591,17 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry)
* Time control
*/
static bool has_control;
+Error *migration_blocker;
const void *qemu_plugin_request_time_control(void)
{
if (!has_control) {
has_control = true;
+#ifdef CONFIG_SOFTMMU
+error_setg(_blocker,
+ "TCG plugin time control does not support migration");
+migrate_add_blocker(_blocker, NULL);
+#endif
return _control;
}
return NULL;
Reviewed-by: Thomas Huth
Re: [PATCH 5/6] meson: remove dead optimization option
On 6/20/24 06:02, Paolo Bonzini wrote: Signed-off-by: Paolo Bonzini --- meson.build | 13 - meson_options.txt | 2 -- 2 files changed, 15 deletions(-) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH] cxl: avoid duplicating report from MCE & device
On Wed, 19 Jun 2024 00:53:10 +0800
Shiyang Ruan wrote:
> Background:
> Since CXL device is a memory device, while CPU consumes a poison page of
> CXL device, it always triggers a MCE by interrupt (INT18), no matter
> which-First path is configured. This is the first report. Then
> currently, in FW-First path, the poison event is transferred according
> to the following process: CXL device -> firmware -> OS:ACPI->APEI->GHES
> -> CPER -> trace report. This is the second one. These two reports
> are indicating the same poisoning page, which is the so-called "duplicate
> report"[1]. And the memory_failure() handling I'm trying to add in
> OS-First path could also be another duplicate report.
>
> Hope the flow below could make it easier to understand:
> CPU accesses bad memory on CXL device, then
> -> MCE (INT18), *always* report (1)
> -> * FW-First (implemented now)
> -> CXL device -> FW
> -> OS:ACPI->APEI->GHES->CPER -> trace report (2.a)
> * OS-First (not implemented yet, I'm working on it)
> -> CXL device -> MSI
> -> OS:CXL driver -> memory_failure() (2.b)
> so, the (1) and (2.a/b) are duplicated.
>
> (I didn't get response in my reply for [1] while I have to make patch to
> solve this problem, so please correct me if my understanding is wrong.)
>
> This patch adds a new notifier_block and MCE_PRIO_CXL, for CXL memdev
> to check whether the current poison page has been reported (if yes,
> stop the notifier chain, won't call the following memory_failure()
> to report), into `x86_mce_decoder_chain`. In this way, if the poison
> page already handled(recorded and reported) in (1) or (2), the other one
> won't duplicate the report. The record could be clear when
> cxl_clear_poison() is called.
>
> [1]
> https://lore.kernel.org/linux-cxl/664d948fb86f0_e8be29...@dwillia2-mobl3.amr.corp.intel.com.notmuch/
>
> Signed-off-by: Shiyang Ruan
So poison can be cleared in a number of ways and a CXL poison clear command
is unfortunately only one of them. Some architectures have instructions
that guarantee to write a whole cacheline and can clear things as well.
I believe x86 does for starters.
+CC linux-edac and related maintainers / reviewers.
linux-mm and hwpoison maintainer.
So I think this needs a more general solution that encompasses
more general cleanup of poison.
Trivial comments inline.
Jonathan
> ---
> arch/x86/include/asm/mce.h | 1 +
> drivers/cxl/core/mbox.c| 130 +
> drivers/cxl/core/memdev.c | 6 +-
> drivers/cxl/cxlmem.h | 3 +
> 4 files changed, 139 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index dfd2e9699bd7..d8109c48e7d9 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -182,6 +182,7 @@ enum mce_notifier_prios {
> MCE_PRIO_NFIT,
> MCE_PRIO_EXTLOG,
> MCE_PRIO_UC,
> + MCE_PRIO_CXL,
> MCE_PRIO_EARLY,
> MCE_PRIO_CEC,
> MCE_PRIO_HIGHEST = MCE_PRIO_CEC
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 2626f3fff201..0eb3c5401e81 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -4,6 +4,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> @@ -880,6 +882,9 @@ void cxl_event_trace_record(const struct cxl_memdev
> *cxlmd,
> if (cxlr)
> hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);
>
> + if (hpa != ULLONG_MAX && cxl_mce_recorded(hpa))
> + return;
> +
> if (event_type == CXL_CPER_EVENT_GEN_MEDIA)
> trace_cxl_general_media(cxlmd, type, cxlr, hpa,
> >gen_media);
> @@ -1408,6 +1413,127 @@ int cxl_poison_state_init(struct cxl_memdev_state
> *mds)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_poison_state_init, CXL);
>
> +struct cxl_mce_record {
> + struct list_head node;
> + u64 hpa;
> +};
> +LIST_HEAD(cxl_mce_records);
> +DEFINE_MUTEX(cxl_mce_mutex);
> +
> +bool cxl_mce_recorded(u64 hpa)
> +{
> + struct cxl_mce_record *cur, *next, *rec;
> + int rc;
> +
> + rc = mutex_lock_interruptible(_mce_mutex);
guard(mutex)(_mce_muted);
> + if (rc)
> + return false;
> +
> + list_for_each_entry_safe(cur, next, _mce_records, node) {
> + if (cur->hpa == hpa) {
> + mutex_unlock(_mce_mutex);
> + return true;
> + }
> + }
> +
> + rec = kmalloc(sizeof(struct cxl_mce_record), GFP_KERNEL);
> + rec->hpa = hpa;
> + list_add(_mce_records, >node);
> +
> + mutex_unlock(_mce_mutex);
> +
> + return false;
> +}
> +
> +void cxl_mce_clear(u64 hpa)
> +{
> + struct cxl_mce_record *cur, *next;
> + int rc;
> +
> + rc = mutex_lock_interruptible(_mce_mutex);
Maybe cond_guard().
> + if (rc)
> +
[PATCH v2 11/14] hw: skip registration of outdated versioned machine types
This calls the MACHINE_VER_DELETION() macro in the machine type
registration method, so that when a versioned machine type reaches
the end of its life, it is no longer registered with QOM and thus
cannot be used.
The actual definition of the machine type should be deleted at
this point, but experience shows that can easily be forgotten.
By skipping registration the manual code deletion task can be
done at any later date.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
---
hw/arm/virt.c | 1 +
hw/m68k/virt.c | 1 +
hw/ppc/spapr.c | 1 +
hw/s390x/s390-virtio-ccw.c | 1 +
include/hw/i386/pc.h | 1 +
5 files changed, 5 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ef6591d914..ab4a0d9ed6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -123,6 +123,7 @@ static void arm_virt_compat_set(MachineClass *mc)
}; \
static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
{ \
+MACHINE_VER_DELETION(__VA_ARGS__); \
type_register_static(_VER_SYM(info, virt, __VA_ARGS__)); \
} \
type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__));
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 37bb36b385..cda199af8f 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -356,6 +356,7 @@ type_init(virt_machine_register_types)
}; \
static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
{ \
+MACHINE_VER_DELETION(__VA_ARGS__); \
type_register_static(_VER_SYM(info, virt, __VA_ARGS__)); \
} \
type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__));
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 55268489d3..044e6a8d9d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4824,6 +4824,7 @@ static void
spapr_machine_latest_class_options(MachineClass *mc)
}; \
static void MACHINE_VER_SYM(register, spapr, __VA_ARGS__)(void) \
{\
+MACHINE_VER_DELETION(__VA_ARGS__); \
type_register(_VER_SYM(info, spapr, __VA_ARGS__)); \
}\
type_init(MACHINE_VER_SYM(register, spapr, __VA_ARGS__))
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4cc7567872..0cb8c595a2 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -846,6 +846,7 @@ static const TypeInfo ccw_machine_info = {
};\
static void MACHINE_VER_SYM(register, ccw, __VA_ARGS__)(void) \
{ \
+MACHINE_VER_DELETION(__VA_ARGS__);\
type_register_static(_VER_SYM(info, ccw, __VA_ARGS__)); \
} \
type_init(MACHINE_VER_SYM(register, ccw, __VA_ARGS__))
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 83d2e66498..4e55d7ef6e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -340,6 +340,7 @@ extern const size_t pc_compat_2_3_len;
}; \
static void MACHINE_VER_SYM(register, namesym, __VA_ARGS__)(void) \
{ \
+MACHINE_VER_DELETION(__VA_ARGS__); \
type_register(_VER_SYM(info, namesym, __VA_ARGS__)); \
} \
type_init(MACHINE_VER_SYM(register, namesym, __VA_ARGS__));
--
2.43.0
[PATCH v2 14/14] docs: document special exception for machine type deprecation & removal
This extends the deprecation policy to indicate that versioned machine types will be marked deprecated after 3 years, and then subject to removal after a further 3 years has passed. Reviewed-by: Thomas Huth Signed-off-by: Daniel P. Berrangé --- docs/about/deprecated.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ff3da68208..bba12d1641 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -11,6 +11,19 @@ releases, the feature is liable to be removed. Deprecated features may also generate warnings on the console when QEMU starts up, or if activated via a monitor command, however, this is not a mandatory requirement. +As a special exception to this general timeframe, rather than have an +indefinite lifetime, versioned machine types are only intended to be +supported for a period of 6 years, equivalent to 18 QEMU releases. All +versioned machine types will be automatically marked deprecated after an +initial 3 years (9 QEMU releases) has passed, and will then be deleted after +a further 3 year period has passed. It is recommended that a deprecated +machine type is only used for incoming migrations and restore of saved state, +for pre-existing VM deployments. They should be scheduled for updating to a +newer machine type during an appropriate service window. Newly deployed VMs +should exclusively use a non-deprecated machine type, with use of the most +recent version highly recommended. Non-versioned machine types follow the +general feature deprecation policy. + Prior to the 2.10.0 release there was no official policy on how long features would be deprecated prior to their removal, nor any documented list of which features were deprecated. Thus -- 2.43.0
[PATCH v2 05/14] hw/m68k: convert 'virt' machine definitions to use new macros
This changes the DEFINE_VIRT_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.
A DEFINE_VIRT_MACHINE_AS_LATEST helper is added so that it
is not required to pass 'false' for every single historical
machine type.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
---
hw/m68k/virt.c | 51 --
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 09bc9bdfef..cd6ee692f7 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -335,99 +335,106 @@ static void virt_machine_register_types(void)
type_init(virt_machine_register_types)
-#define DEFINE_VIRT_MACHINE(major, minor, latest) \
-static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
-void *data) \
+#define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \
+static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \
+ObjectClass *oc, \
+void *data) \
{ \
MachineClass *mc = MACHINE_CLASS(oc); \
-virt_machine_##major##_##minor##_options(mc); \
-mc->desc = "QEMU " # major "." # minor " M68K Virtual Machine"; \
+MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \
+mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " M68K Virtual
Machine"; \
if (latest) { \
mc->alias = "virt"; \
} \
} \
-static const TypeInfo machvirt_##major##_##minor##_info = { \
-.name = MACHINE_TYPE_NAME("virt-" # major "." # minor), \
+static const TypeInfo MACHINE_VER_SYM(info, virt, __VA_ARGS__) = \
+{ \
+.name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \
.parent = MACHINE_TYPE_NAME("virt"), \
-.class_init = virt_##major##_##minor##_class_init, \
+.class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \
}; \
-static void machvirt_machine_##major##_##minor##_init(void) \
+static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
{ \
-type_register_static(_##major##_##minor##_info); \
+type_register_static(_VER_SYM(info, virt, __VA_ARGS__)); \
} \
-type_init(machvirt_machine_##major##_##minor##_init);
+type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__));
+
+#define DEFINE_VIRT_MACHINE_AS_LATEST(major, minor) \
+DEFINE_VIRT_MACHINE_IMPL(true, major, minor)
+#define DEFINE_VIRT_MACHINE(major, minor) \
+DEFINE_VIRT_MACHINE_IMPL(false, major, minor)
static void virt_machine_9_1_options(MachineClass *mc)
{
}
-DEFINE_VIRT_MACHINE(9, 1, true)
+DEFINE_VIRT_MACHINE_AS_LATEST(9, 1)
static void virt_machine_9_0_options(MachineClass *mc)
{
virt_machine_9_1_options(mc);
compat_props_add(mc->compat_props, hw_compat_9_0, hw_compat_9_0_len);
}
-DEFINE_VIRT_MACHINE(9, 0, false)
+DEFINE_VIRT_MACHINE(9, 0)
static void virt_machine_8_2_options(MachineClass *mc)
{
virt_machine_9_0_options(mc);
compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len);
}
-DEFINE_VIRT_MACHINE(8, 2, false)
+DEFINE_VIRT_MACHINE(8, 2)
static void virt_machine_8_1_options(MachineClass *mc)
{
virt_machine_8_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_8_1, hw_compat_8_1_len);
}
-DEFINE_VIRT_MACHINE(8, 1, false)
+DEFINE_VIRT_MACHINE(8, 1)
static void virt_machine_8_0_options(MachineClass *mc)
{
virt_machine_8_1_options(mc);
compat_props_add(mc->compat_props, hw_compat_8_0, hw_compat_8_0_len);
}
-DEFINE_VIRT_MACHINE(8, 0, false)
+DEFINE_VIRT_MACHINE(8, 0)
static void virt_machine_7_2_options(MachineClass *mc)
{
virt_machine_8_0_options(mc);
compat_props_add(mc->compat_props, hw_compat_7_2, hw_compat_7_2_len);
}
-DEFINE_VIRT_MACHINE(7, 2, false)
+DEFINE_VIRT_MACHINE(7, 2)
static void virt_machine_7_1_options(MachineClass *mc)
{
virt_machine_7_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_7_1, hw_compat_7_1_len);
}
-DEFINE_VIRT_MACHINE(7, 1, false)
+DEFINE_VIRT_MACHINE(7, 1)
static void virt_machine_7_0_options(MachineClass *mc)
{
virt_machine_7_1_options(mc);
compat_props_add(mc->compat_props, hw_compat_7_0, hw_compat_7_0_len);
}
-DEFINE_VIRT_MACHINE(7, 0, false)
+DEFINE_VIRT_MACHINE(7, 0)
static void virt_machine_6_2_options(MachineClass *mc)
{
virt_machine_7_0_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_2, hw_compat_6_2_len);
}
-DEFINE_VIRT_MACHINE(6, 2, false)
+DEFINE_VIRT_MACHINE(6, 2)
static void virt_machine_6_1_options(MachineClass *mc)
{
virt_machine_6_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
}
-DEFINE_VIRT_MACHINE(6, 1, false)
+DEFINE_VIRT_MACHINE(6, 1)
static void virt_machine_6_0_options(MachineClass *mc)
{
virt_machine_6_1_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_0,
[PATCH v2 10/14] hw: set deprecation info for all versioned machine types
This calls the MACHINE_VER_DEPRECATION() macro in the definition of
all machine type classes which support versioning. This ensures
that they will automatically get deprecation info set when they
reach the appropriate point in their lifecycle.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
---
hw/arm/virt.c | 1 +
hw/m68k/virt.c | 1 +
hw/ppc/spapr.c | 1 +
hw/s390x/s390-virtio-ccw.c | 1 +
include/hw/i386/pc.h | 1 +
5 files changed, 5 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a326aa24db..ef6591d914 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -110,6 +110,7 @@ static void arm_virt_compat_set(MachineClass *mc)
arm_virt_compat_set(mc); \
MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \
mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " ARM Virtual
Machine"; \
+MACHINE_VER_DEPRECATION(__VA_ARGS__); \
if (latest) { \
mc->alias = "virt"; \
} \
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index cd6ee692f7..37bb36b385 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -343,6 +343,7 @@ type_init(virt_machine_register_types)
MachineClass *mc = MACHINE_CLASS(oc); \
MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \
mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " M68K Virtual
Machine"; \
+MACHINE_VER_DEPRECATION(__VA_ARGS__); \
if (latest) { \
mc->alias = "virt"; \
} \
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2785b6b303..55268489d3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4811,6 +4811,7 @@ static void
spapr_machine_latest_class_options(MachineClass *mc)
{\
MachineClass *mc = MACHINE_CLASS(oc);\
MACHINE_VER_SYM(class_options, spapr, __VA_ARGS__)(mc); \
+MACHINE_VER_DEPRECATION(__VA_ARGS__);\
if (latest) {\
spapr_machine_latest_class_options(mc); \
}\
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index efed539bc6..4cc7567872 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -825,6 +825,7 @@ static const TypeInfo ccw_machine_info = {
MachineClass *mc = MACHINE_CLASS(oc); \
MACHINE_VER_SYM(class_options, ccw, __VA_ARGS__)(mc); \
mc->desc = "Virtual s390x machine (version "
MACHINE_VER_STR(__VA_ARGS__) ")"; \
+MACHINE_VER_DEPRECATION(__VA_ARGS__); \
if (latest) { \
mc->alias = "s390-ccw-virtio";\
mc->is_default = true;\
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 027c6f29f7..83d2e66498 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -330,6 +330,7 @@ extern const size_t pc_compat_2_3_len;
MachineClass *mc = MACHINE_CLASS(oc); \
MACHINE_VER_SYM(options, namesym, __VA_ARGS__)(mc); \
mc->init = MACHINE_VER_SYM(init, namesym, __VA_ARGS__); \
+MACHINE_VER_DEPRECATION(__VA_ARGS__); \
} \
static const TypeInfo MACHINE_VER_SYM(info, namesym, __VA_ARGS__) = \
{ \
--
2.43.0
[PATCH v2 12/14] hw/ppc: remove obsolete manual deprecation reason string of spapr machines
The automatic deprecation mechanism introduced in the preceeding patches will mark every spapr machine upto and including 2.12 as deprecated. As such we can revert the manually added deprecation which was a subset: commit 1392617d35765d5d912625fbb5cab1ffbed8e140 Author: Cédric Le Goater Date: Tue Jan 23 16:37:02 2024 +1000 spapr: Tag pseries-2.1 - 2.11 machines as deprecated Reviewed-by: Thomas Huth Signed-off-by: Daniel P. Berrangé --- hw/ppc/spapr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 044e6a8d9d..98fa3aa6a8 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -5156,7 +5156,6 @@ static void spapr_machine_2_11_class_options(MachineClass *mc) spapr_machine_2_12_class_options(mc); smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_ON; compat_props_add(mc->compat_props, hw_compat_2_11, hw_compat_2_11_len); -mc->deprecation_reason = "old and not maintained - use a 2.12+ version"; } DEFINE_SPAPR_MACHINE(2, 11); -- 2.43.0
[PATCH v2 07/14] hw/i386: convert 'q35' machine definitions to use new macros
This changes the DEFINE_Q35_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.
The added benefit is that it avoids the need to repeat the
version number thrice in three different formats in the calls
to DEFINE_Q35_MACHINE.
Due to the odd-ball '4.0.1' machine type version, this
commit introduces a DEFINE_Q35_BUGFIX helper, to allow
defining of "bugfix" machine types which have a three
digit version.
Signed-off-by: Daniel P. Berrangé
---
hw/i386/pc_q35.c | 215 ---
1 file changed, 90 insertions(+), 125 deletions(-)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index bd7db4abac..71d3c6d122 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -331,17 +331,11 @@ static void pc_q35_init(MachineState *machine)
}
}
-#define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
-static void pc_init_##suffix(MachineState *machine) \
-{ \
-void (*compat)(MachineState *m) = (compatfn); \
-if (compat) { \
-compat(machine); \
-} \
-pc_q35_init(machine); \
-} \
-DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
+#define DEFINE_Q35_MACHINE(major, minor) \
+DEFINE_PC_VER_MACHINE(pc_q35, "pc-q35", pc_q35_init, major, minor);
+#define DEFINE_Q35_MACHINE_BUGFIX(major, minor, micro) \
+DEFINE_PC_VER_MACHINE(pc_q35, "pc-q35", pc_q35_init, major, minor, micro);
static void pc_q35_machine_options(MachineClass *m)
{
@@ -367,32 +361,30 @@ static void pc_q35_machine_options(MachineClass *m)
pc_q35_compat_defaults, pc_q35_compat_defaults_len);
}
-static void pc_q35_9_1_machine_options(MachineClass *m)
+static void pc_q35_machine_9_1_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
-DEFINE_Q35_MACHINE(v9_1, "pc-q35-9.1", NULL,
- pc_q35_9_1_machine_options);
+DEFINE_Q35_MACHINE(9, 1);
-static void pc_q35_9_0_machine_options(MachineClass *m)
+static void pc_q35_machine_9_0_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-pc_q35_9_1_machine_options(m);
+pc_q35_machine_9_1_options(m);
m->alias = NULL;
compat_props_add(m->compat_props, hw_compat_9_0, hw_compat_9_0_len);
compat_props_add(m->compat_props, pc_compat_9_0, pc_compat_9_0_len);
pcmc->isa_bios_alias = false;
}
-DEFINE_Q35_MACHINE(v9_0, "pc-q35-9.0", NULL,
- pc_q35_9_0_machine_options);
+DEFINE_Q35_MACHINE(9, 0);
-static void pc_q35_8_2_machine_options(MachineClass *m)
+static void pc_q35_machine_8_2_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-pc_q35_9_0_machine_options(m);
+pc_q35_machine_9_0_options(m);
m->max_cpus = 1024;
compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len);
compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len);
@@ -400,26 +392,24 @@ static void pc_q35_8_2_machine_options(MachineClass *m)
pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_64;
}
-DEFINE_Q35_MACHINE(v8_2, "pc-q35-8.2", NULL,
- pc_q35_8_2_machine_options);
+DEFINE_Q35_MACHINE(8, 2);
-static void pc_q35_8_1_machine_options(MachineClass *m)
+static void pc_q35_machine_8_1_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-pc_q35_8_2_machine_options(m);
+pc_q35_machine_8_2_options(m);
pcmc->broken_32bit_mem_addr_check = true;
compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len);
compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len);
}
-DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
- pc_q35_8_1_machine_options);
+DEFINE_Q35_MACHINE(8, 1);
-static void pc_q35_8_0_machine_options(MachineClass *m)
+static void pc_q35_machine_8_0_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-pc_q35_8_1_machine_options(m);
+pc_q35_machine_8_1_options(m);
compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
@@ -428,132 +418,120 @@ static void pc_q35_8_0_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
- pc_q35_8_0_machine_options);
+DEFINE_Q35_MACHINE(8, 0);
-static void pc_q35_7_2_machine_options(MachineClass *m)
+static void pc_q35_machine_7_2_options(MachineClass *m)
{
-pc_q35_8_0_machine_options(m);
+pc_q35_machine_8_0_options(m);
compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
}
-DEFINE_Q35_MACHINE(v7_2, "pc-q35-7.2", NULL,
- pc_q35_7_2_machine_options);
+DEFINE_Q35_MACHINE(7, 2);
-static void pc_q35_7_1_machine_options(MachineClass *m)
+static void
[PATCH v2 13/14] hw/i386: remove obsolete manual deprecation reason string of i440fx machines
The automatic deprecation mechanism introduced in the preceeding patches
will mark every i440fx machine upto and including 2.12 as deprecated. As
such we can revert the manually added deprecation introduced in:
commit 792b4fdd4eb8197bd6eb9e80a1dfaf0cb3b54aeb
Author: Philippe Mathieu-Daudé
Date: Wed Feb 28 10:34:35 2024 +0100
hw/i386/pc: Deprecate 2.4 to 2.12 pc-i440fx machines
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
---
hw/i386/pc_piix.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5705d6e155..9445b07b4f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -688,7 +688,6 @@ DEFINE_I440FX_MACHINE(3, 0);
static void pc_i440fx_machine_2_12_options(MachineClass *m)
{
pc_i440fx_machine_3_0_options(m);
-m->deprecation_reason = "old and unattended - use a newer version instead";
compat_props_add(m->compat_props, hw_compat_2_12, hw_compat_2_12_len);
compat_props_add(m->compat_props, pc_compat_2_12, pc_compat_2_12_len);
}
--
2.43.0
[PATCH v2 03/14] hw/s390x: convert 'ccw' machine definitions to use new macros
This changes the DEFINE_CCW_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.
The added benefit is that it avoids the need to repeat the
version number twice in two different formats in the calls
to DEFINE_CCW_MACHINE.
A DEFINE_CCW_MACHINE_AS_LATEST helper is added so that it
is not required to pass 'false' for every single historical
machine type.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
---
hw/s390x/s390-virtio-ccw.c | 96 +-
1 file changed, 53 insertions(+), 43 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..efed539bc6 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -15,6 +15,7 @@
#include "qapi/error.h"
#include "exec/ram_addr.h"
#include "exec/confidential-guest-support.h"
+#include "hw/boards.h"
#include "hw/s390x/s390-virtio-hcall.h"
#include "hw/s390x/sclp.h"
#include "hw/s390x/s390_flic.h"
@@ -816,35 +817,44 @@ static const TypeInfo ccw_machine_info = {
},
};
-#define DEFINE_CCW_MACHINE(suffix, verstr, latest)\
-static void ccw_machine_##suffix##_class_init(ObjectClass *oc,\
- void *data) \
+#define DEFINE_CCW_MACHINE_IMPL(latest, ...) \
+static void MACHINE_VER_SYM(class_init, ccw, __VA_ARGS__)(\
+ObjectClass *oc, \
+void *data) \
{ \
MachineClass *mc = MACHINE_CLASS(oc); \
-ccw_machine_##suffix##_class_options(mc); \
-mc->desc = "Virtual s390x machine (version " verstr ")"; \
+MACHINE_VER_SYM(class_options, ccw, __VA_ARGS__)(mc); \
+mc->desc = "Virtual s390x machine (version "
MACHINE_VER_STR(__VA_ARGS__) ")"; \
if (latest) { \
mc->alias = "s390-ccw-virtio";\
mc->is_default = true;\
} \
} \
-static void ccw_machine_##suffix##_instance_init(Object *obj) \
+static void MACHINE_VER_SYM(instance_init, ccw, __VA_ARGS__)(Object *obj) \
{ \
MachineState *machine = MACHINE(obj); \
-current_mc = S390_CCW_MACHINE_CLASS(MACHINE_GET_CLASS(machine));
\
-ccw_machine_##suffix##_instance_options(machine); \
+current_mc = S390_CCW_MACHINE_CLASS(MACHINE_GET_CLASS(machine)); \
+MACHINE_VER_SYM(instance_options, ccw, __VA_ARGS__)(machine); \
} \
-static const TypeInfo ccw_machine_##suffix##_info = { \
-.name = MACHINE_TYPE_NAME("s390-ccw-virtio-" verstr), \
+static const TypeInfo MACHINE_VER_SYM(info, ccw, __VA_ARGS__) = \
+{ \
+.name = MACHINE_VER_TYPE_NAME("s390-ccw-virtio", __VA_ARGS__),\
.parent = TYPE_S390_CCW_MACHINE, \
-.class_init = ccw_machine_##suffix##_class_init, \
-.instance_init = ccw_machine_##suffix##_instance_init,\
+.class_init = MACHINE_VER_SYM(class_init, ccw, __VA_ARGS__), \
+.instance_init = MACHINE_VER_SYM(instance_init, ccw, __VA_ARGS__),\
};\
-static void ccw_machine_register_##suffix(void) \
+static void MACHINE_VER_SYM(register, ccw, __VA_ARGS__)(void) \
{ \
-type_register_static(_machine_##suffix##_info); \
+type_register_static(_VER_SYM(info, ccw, __VA_ARGS__)); \
} \
-type_init(ccw_machine_register_##suffix)
+type_init(MACHINE_VER_SYM(register, ccw, __VA_ARGS__))
+
+#define DEFINE_CCW_MACHINE_AS_LATEST(major, minor) \
+DEFINE_CCW_MACHINE_IMPL(true, major, minor)
+
+#define DEFINE_CCW_MACHINE(major, minor) \
+DEFINE_CCW_MACHINE_IMPL(false, major, minor)
[PATCH v2 09/14] include/hw: temporarily disable deletion of versioned machine types
The new deprecation and deletion policy for versioned machine types is
being introduced in QEMU 9.1.0.
Under the new policy a number of old machine types (any prior to 2.12)
would be liable for immediate deletion which would be a violation of our
historical deprecation and removal policy
Thus automatic deletions (by skipping QOM registration) are temporarily
gated on existance of the env variable "QEMU_DELETE_MACHINES" / QEMU
version number >= 10.1.0. This allows opt-in testing of the automatic
deletion logic, while activating it fully in QEMU >= 10.1.0.
This whole commit should be reverted in the 10.1.0 dev cycle or shortly
thereafter.
Signed-off-by: Daniel P. Berrangé
---
include/hw/boards.h | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 187ed76646..ef6f18f2c1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -686,11 +686,28 @@ struct MachineState {
* suitable period of time has passed, it will cause
* execution of the method to return, avoiding registration
* of the machine
+ *
+ * The new deprecation and deletion policy for versioned
+ * machine types was introduced in QEMU 9.1.0.
+ *
+ * Under the new policy a number of old machine types (any
+ * prior to 2.12) would be liable for immediate deletion
+ * which would be a violation of our historical deprecation
+ * and removal policy
+ *
+ * Thus deletions are temporarily gated on existance of
+ * the env variable "QEMU_DELETE_MACHINES" / QEMU version
+ * number >= 10.1.0. This gate can be deleted in the 10.1.0
+ * dev cycle
*/
#define MACHINE_VER_DELETION(...) \
do { \
if (MACHINE_VER_SHOULD_DELETE(__VA_ARGS__)) { \
-return; \
+if (getenv("QEMU_DELETE_MACHINES") || \
+QEMU_VERSION_MAJOR > 10 || (QEMU_VERSION_MAJOR == 10 && \
+QEMU_VERSION_MINOR >= 1)) { \
+return; \
+} \
} \
} while (0)
--
2.43.0
