Re: [PATCH v2 8/9] vfio: Check compatibility of CPU and IOMMU address space width
Hi, > > Is there some simple way to figure what the iommu width is (inside the > > guest)? > > If the guest firmware is exposing a DMAR table (VT-d), there's a host > address width field in that table. Otherwise there are capability > registers on the DRHD units that could be queried. AMD-Vi seems to > have similar information in the IVinfo table linked from the IVRS > table. Maybe an iterative solution is ok, starting with the most > common IOMMU types and assuming others are 64-bits wide until proven > otherwise. Hmm. The guest firmware simply exposes the tables it gets from qemu (without parsing it). Also the acpi tables are loaded after the firmware created the address space layout, because qemu adjusts them according to the programming done by the firmware (set pci bus ranges according to bridge windows etc). So checking ACPI tables for that information doesn't look very attractive. The firmware would have to load the tables twice (once early to parse DMAR, once late for the final tables). Going for the capability registers might be possible. Can the hardware be probed for safely? Has AMD capability registers too? Third option would be using another channel to communicate the information from qemu to firmware, where using 'guest-phys-bits' would be one candidate. > Right, as Cédric notes that's sort of what happens here, but my concern > was that the we're kind of incorrectly linking the cpu address width to > the platform address width, which isn't specifically the requirement. There is a separate 'guest-phys-bits' property for that reason. The phys-bits of the CPU are not changed. take care, Gerd
Re: [PATCH 07/10] rust: qdev: make ObjectImpl a supertrait of DeviceImpl
On 17/1/25 20:40, Paolo Bonzini wrote: In practice it has to be implemented always in order to access an implementation of ClassInitImpl. Make the relationship explicit in the code. Signed-off-by: Paolo Bonzini --- rust/qemu-api/src/qdev.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 07/17] hw/misc: Add support for NPCM8XX GCR
Hi Hao,
On 6/2/25 02:30, Hao Wu wrote:
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/misc/npcm_gcr.c | 131 -
include/hw/misc/npcm_gcr.h | 6 +-
2 files changed, 134 insertions(+), 3 deletions(-)
+NPCM8XX_GCR_WD0RCRBLK,
+NPCM8XX_GCR_WD1RCRBLK,
+NPCM8XX_GCR_WD2RCRBLK,
+NPCM8XX_GCR_SWRSTC1BLK,
+NPCM8XX_GCR_SWRSTC2BLK,
+NPCM8XX_GCR_SWRSTC3BLK,
+NPCM8XX_GCR_TIPRSTCBLK,
+NPCM8XX_GCR_CORSTCBLK,
+/* 64 scratch pad registers start here. 0xe00 ~ 0xefc */
+NPCM8XX_GCR_SCRPAD_00 = 0xe00 / sizeof(uint32_t),
+/* 32 semaphore registers start here. 0xf00 ~ 0xf7c */
+NPCM8XX_GCR_GP_SEMFR_00 = 0xf00 / sizeof(uint32_t),
Alternatively:
NPCM8XX_GCR_GP_SEMFR_31 = 0xf7c ...
+NPCM8XX_GCR_REGS_END= 0xf80 / sizeof(uint32_t),
Then no need for NPCM8XX_GCR_REGS_END, we have NPCM8XX_GCR_NR_REGS.
+};
+
+static const uint32_t npcm8xx_cold_reset_values[NPCM8XX_GCR_NR_REGS] = {
+[NPCM8XX_GCR_PDID] = 0x04a35850, /* Arbel A1 */
+[NPCM8XX_GCR_MISCPE]= 0x,
+[NPCM8XX_GCR_A35_MODE] = 0xfff4ff30,
+[NPCM8XX_GCR_SPSWC] = 0x0003,
+[NPCM8XX_GCR_INTCR] = 0x0010035e,
+[NPCM8XX_GCR_HIFCR] = 0x004e,
+[NPCM8XX_GCR_SD2SUR1] = 0xfdc8,
+[NPCM8XX_GCR_SD2SUR2] = 0x5200b130,
+[NPCM8XX_GCR_INTCR2]= (1U << 19), /* DDR initialized */
+[NPCM8XX_GCR_RESSR] = 0x8000,
+[NPCM8XX_GCR_DAVCLVLR] = 0x5a00f3cf,
+[NPCM8XX_GCR_INTCR3]= 0x5e001002,
+[NPCM8XX_GCR_VSRCR] = 0x4800,
+[NPCM8XX_GCR_SCRPAD]= 0x0008,
+[NPCM8XX_GCR_USB1PHYCTL]= 0x034730e4,
+[NPCM8XX_GCR_USB2PHYCTL]= 0x034730e4,
+[NPCM8XX_GCR_USB3PHYCTL]= 0x034730e4,
+/* All 32 semaphores should be initialized to 1. */
+[NPCM8XX_GCR_GP_SEMFR_00...NPCM8XX_GCR_REGS_END - 1] = 0x0001,
[NPCM8XX_GCR_GP_SEMFR_00 ...
NPCM8XX_GCR_GP_SEMFR_31] = 1;
+};
+
static uint64_t npcm_gcr_read(void *opaque, hwaddr offset, unsigned size)
{
uint32_t reg = offset / sizeof(uint32_t);
@@ -263,6 +375,18 @@ static void npcm7xx_gcr_class_init(ObjectClass *klass,
void *data)
c->cold_reset_values = npcm7xx_cold_reset_values;
}
+static void npcm8xx_gcr_class_init(ObjectClass *klass, void *data)
+{
+NPCMGCRClass *c = NPCM_GCR_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+QEMU_BUILD_BUG_ON(NPCM8XX_GCR_REGS_END > NPCM_GCR_MAX_NR_REGS);
+QEMU_BUILD_BUG_ON(NPCM8XX_GCR_REGS_END != NPCM8XX_GCR_NR_REGS);
Not sure these checks are useful, but as you prefer.
+dc->desc = "NPCM8xx System Global Control Registers";
+c->nr_regs = NPCM8XX_GCR_NR_REGS;
+c->cold_reset_values = npcm8xx_cold_reset_values;
+}
@@ -54,8 +54,9 @@
* Number of registers in our device state structure. Don't change this
without
* incrementing the version_id in the vmstate.
*/
-#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS
+#define NPCM_GCR_MAX_NR_REGS NPCM8XX_GCR_NR_REGS
#define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t))
+#define NPCM8XX_GCR_NR_REGS (0xf80 / sizeof(uint32_t))
typedef struct NPCMGCRState {
SysBusDevice parent;
@@ -78,6 +79,7 @@ typedef struct NPCMGCRClass {
#define TYPE_NPCM_GCR "npcm-gcr"
#define TYPE_NPCM7XX_GCR "npcm7xx-gcr"
+#define TYPE_NPCM8XX_GCR "npcm8xx-gcr"
OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR)
#endif /* NPCM_GCR_H */
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
On 2/6/25 10:15, Zhao Liu wrote:
Throughout the entire holiday, I couldn't think of a better way to
express this. I find it particularly useful when wrapping multiple
callbacks. In the future, I want to explore more use cases where this
pattern can be applied.
Thanks very much. Despite having written most of the bindings code, I
don't want to give the impression that everything that I send to the
mailing list is gold. So it matters to me that you send serious
reviews, and that you can learn something but also suggest
clarifications and improvements.
I find even if I drop the `inline` attribution, the test.rs can still be
compiled (by `make check`), I think it's because test.rs hasn't involved
memory related tests so that do_init_io isn't linked into test.rs.
Hmm, maybe it's only with some linkers. I don't remember.
This is ugly because do_init_io exists *exactly* to extract the part
that is not generic. (See
https://users.rust-lang.org/t/soft-question-significantly-improve-rust-compile-time-via-minimizing-generics/103632/8
for an example of this; I think there's even a procedural macro crate
that does that for you, but I can't find it right now).
Thanks! I see. I agree to keep `inline` anyway.
In the meanwhile I found it, it's
https://github.com/llogiq/momo/blob/master/wasm/src/lib.rs. QEMU could
add something like that macro to qemu_api_macros, so that
#[qemu_api_macros::upcast]
fn foo(t: &T) where T: IsA {
}
could be converted to
#[inline(always)]
fn foo(&self) where T: IsA {
fn inner_foo(u: &U) {
}
inner_foo(self.upcast())
}
Paolo
[PATCH] rust: pl011: convert pl011_create to safe Rust
Not a major change but, as a small but significant step in creating
qdev bindings, show how pl011_create can be written without "unsafe"
calls (apart from converting pointers to references).
This also provides a starting point for creating Error** bindings.
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/src/device.rs | 39
rust/qemu-api/src/sysbus.rs | 34 +---
2 files changed, 50 insertions(+), 23 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 22f3ca3b4e8..7e936b50eb0 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,14 +10,12 @@
use qemu_api::{
bindings::{
-error_fatal, qdev_prop_set_chr, qemu_chr_fe_accept_input,
qemu_chr_fe_ioctl,
-qemu_chr_fe_set_handlers, qemu_chr_fe_write_all, qemu_irq,
sysbus_connect_irq,
-sysbus_mmio_map, sysbus_realize, CharBackend, Chardev, QEMUChrEvent,
-CHR_IOCTL_SERIAL_SET_BREAK,
+qemu_chr_fe_accept_input, qemu_chr_fe_ioctl, qemu_chr_fe_set_handlers,
+qemu_chr_fe_write_all, CharBackend, QEMUChrEvent,
CHR_IOCTL_SERIAL_SET_BREAK,
},
chardev::Chardev,
-c_str, impl_vmstate_forward,
-irq::InterruptSource,
+impl_vmstate_forward,
+irq::{IRQState, InterruptSource},
memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType,
ResettablePhasesImpl},
@@ -698,26 +696,27 @@ pub fn post_load(&self, _version_id: u32) -> Result<(),
()> {
/// # Safety
///
-/// We expect the FFI user of this function to pass a valid pointer for `chr`.
+/// We expect the FFI user of this function to pass a valid pointer for `chr`
+/// and `irq`.
#[no_mangle]
pub unsafe extern "C" fn pl011_create(
addr: u64,
-irq: qemu_irq,
+irq: *mut IRQState,
chr: *mut Chardev,
) -> *mut DeviceState {
-let pl011 = PL011State::new();
-unsafe {
-let dev = pl011.as_mut_ptr::();
-qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
+// SAFETY: The callers promise that they have owned references.
+// They do not gift them to pl011_create, so use `Owned::from`.
+let irq = unsafe { Ownedfrom(&*irq) };
+let chr = unsafe { Ownedfrom(&*chr) };
-let sysbus = pl011.as_mut_ptr::();
-sysbus_realize(sysbus, addr_of_mut!(error_fatal));
-sysbus_mmio_map(sysbus, 0, addr);
-sysbus_connect_irq(sysbus, 0, irq);
-
-// return the pointer, which is kept alive by the QOM tree; drop owned
ref
-pl011.as_mut_ptr()
-}
+let dev = PL011State::new();
+dev.prop_set_chr("chardev", &chr);
+dev.realize();
+dev.mmio_map(0, addr);
+dev.connect_irq(0, &irq);
+// SAFETY: return the pointer, which has to be mutable and is kept alive
+// by the QOM tree; drop owned ref
+unsafe { dev.as_mut_ptr() }
}
#[repr(C)]
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index c27dbf79e43..1f071706ce8 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,18 +2,18 @@
// Author(s): Paolo Bonzini
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::ffi::CStr;
+use std::{ffi::CStr, ptr::addr_of_mut};
pub use bindings::{SysBusDevice, SysBusDeviceClass};
use crate::{
bindings,
cell::bql_locked,
-irq::InterruptSource,
+irq::{IRQState, InterruptSource},
memory::MemoryRegion,
prelude::*,
qdev::{DeviceClass, DeviceState},
-qom::ClassInitImpl,
+qom::{ClassInitImpl, Owned},
};
unsafe impl ObjectType for SysBusDevice {
@@ -60,6 +60,34 @@ fn init_irq(&self, irq: &InterruptSource) {
bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr());
}
}
+
+// TODO: do we want a type like GuestAddress here?
+fn mmio_map(&self, id: u32, addr: u64) {
+assert!(bql_locked());
+let id: i32 = id.try_into().unwrap();
+unsafe {
+bindings::sysbus_mmio_map(self.as_mut_ptr(), id, addr);
+}
+}
+
+// Owned<> is used here because sysbus_connect_irq (via
+// object_property_set_link) adds a reference to the IRQState,
+// which can prolong its life
+fn connect_irq(&self, id: u32, irq: &Owned) {
+assert!(bql_locked());
+let id: i32 = id.try_into().unwrap();
+unsafe {
+bindings::sysbus_connect_irq(self.as_mut_ptr(), id,
irq.as_mut_ptr());
+}
+}
+
+fn realize(&self) {
+// TODO: return an Error
+assert!(bql_locked());
+unsafe {
+bindings::sysbus_realize(self.as_mut_ptr(),
addr_of_mut!(bindings::error_fatal));
+}
+}
}
impl SysBusDeviceMethods for R where R::Target:
IsA {}
--
2.48.1
[PATCH] rust: add --rust-target option for bindgen
Without it, recent bindgen will give an error
error: extern block cannot be declared unsafe
if rustc is not new enough to support the "unsafe extern" construct.
Cc: qemu-r...@nongnu.org
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
---
meson.build | 3 +++
1 file changed, 3 insertions(+)
diff --git a/meson.build b/meson.build
index 2c9ac9cfe1e..131b2225ab6 100644
--- a/meson.build
+++ b/meson.build
@@ -4054,6 +4054,9 @@ if have_rust
bindgen_args += ['--formatter', 'none']
endif
endif
+ if bindgen.version().version_compare('>=0.66.0')
+bindgen_args += ['--rust-target', '1.59']
+ endif
if bindgen.version().version_compare('<0.61.0')
# default in 0.61+
bindgen_args += ['--size_t-is-usize']
--
2.48.1
[PATCH v4 01/16] hw/intc/xilinx_intc: Make device endianness configurable
Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness for each machine using the device.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/intc/xilinx_intc.c| 52 +---
hw/microblaze/petalogix_ml605_mmu.c | 1 +
hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 6930f83907a..cd79ac4d4ff 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -3,6 +3,9 @@
*
* Copyright (c) 2009 Edgar E. Iglesias.
*
+ * https://docs.amd.com/v/u/en-US/xps_intc
+ * DS572: LogiCORE IP XPS Interrupt Controller (v2.01a)
+ *
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
* in the Software without restriction, including without limitation the rights
@@ -49,6 +52,7 @@ struct XpsIntc
{
SysBusDevice parent_obj;
+bool little_endian_model;
MemoryRegion mmio;
qemu_irq parent_irq;
@@ -140,18 +144,29 @@ static void pic_write(void *opaque, hwaddr addr,
update_irq(p);
}
-static const MemoryRegionOps pic_ops = {
-.read = pic_read,
-.write = pic_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.impl = {
-.min_access_size = 4,
-.max_access_size = 4,
+static const MemoryRegionOps pic_ops[2] = {
+[0 ... 1] = {
+.read = pic_read,
+.write = pic_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.impl = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.valid = {
+/*
+ * All XPS INTC registers are accessed through the PLB interface.
+ * The base address for these registers is provided by the
+ * configuration parameter, C_BASEADDR. Each register is 32 bits
+ * although some bits may be unused and is accessed on a 4-byte
+ * boundary offset from the base address.
+ */
+.min_access_size = 4,
+.max_access_size = 4,
+},
},
-.valid = {
-.min_access_size = 4,
-.max_access_size = 4
-}
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static void irq_handler(void *opaque, int irq, int level)
@@ -174,13 +189,21 @@ static void xilinx_intc_init(Object *obj)
qdev_init_gpio_in(DEVICE(obj), irq_handler, 32);
sysbus_init_irq(SYS_BUS_DEVICE(obj), &p->parent_irq);
-
-memory_region_init_io(&p->mmio, obj, &pic_ops, p, "xlnx.xps-intc",
- R_MAX * 4);
sysbus_init_mmio(SYS_BUS_DEVICE(obj), &p->mmio);
}
+static void xilinx_intc_realize(DeviceState *dev, Error **errp)
+{
+XpsIntc *p = XILINX_INTC(dev);
+
+memory_region_init_io(&p->mmio, OBJECT(dev),
+ &pic_ops[p->little_endian_model],
+ p, "xlnx.xps-intc",
+ R_MAX * 4);
+}
+
static const Property xilinx_intc_properties[] = {
+DEFINE_PROP_BOOL("little-endian", XpsIntc, little_endian_model, true),
DEFINE_PROP_UINT32("kind-of-intr", XpsIntc, c_kind_of_intr, 0),
};
@@ -188,6 +211,7 @@ static void xilinx_intc_class_init(ObjectClass *klass, void
*data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+dc->realize = xilinx_intc_realize;
device_class_set_props(dc, xilinx_intc_properties);
}
diff --git a/hw/microblaze/petalogix_ml605_mmu.c
b/hw/microblaze/petalogix_ml605_mmu.c
index 8b44be75a22..cf3b9574db3 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -111,6 +111,7 @@ petalogix_ml605_init(MachineState *machine)
dev = qdev_new("xlnx.xps-intc");
+qdev_prop_set_bit(dev, "little-endian", true);
qdev_prop_set_uint32(dev, "kind-of-intr", 1 << TIMER_IRQ);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR);
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 2c0d8c34cd2..0506497ad0a 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -95,6 +95,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
64 * KiB, 1, 0x89, 0x18, 0x, 0x0, 1);
dev = qdev_new("xlnx.xps-intc");
+qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
qdev_prop_set_uint32(dev, "kind-of-intr",
1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
--
2.47.1
[PATCH v4 07/16] target/microblaze: Explode MO_TExx -> MO_TE | MO_xx
Extract the implicit MO_TE definition in order to replace
it by runtime variable in the next commit.
Mechanical change using:
$ for n in UW UL UQ UO SW SL SQ; do \
sed -i -e "s/MO_TE$n/MO_TE | MO_$n/" \
$(git grep -l MO_TE$n target/microblaze); \
done
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
---
target/microblaze/translate.c | 36 +--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 24005f05b21..86efabb83b5 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -780,13 +780,13 @@ static bool trans_lbui(DisasContext *dc, arg_typeb *arg)
static bool trans_lhu(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
}
static bool trans_lhur(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, true);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
}
static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
@@ -798,26 +798,26 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_lhui(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_load(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
}
static bool trans_lw(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
}
static bool trans_lwr(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, true);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
}
static bool trans_lwea(DisasContext *dc, arg_typea *arg)
@@ -829,14 +829,14 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TEUL, MMU_NOMMU_IDX, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_lwi(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_load(dc, arg->rd, addr, MO_TEUL, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
}
static bool trans_lwx(DisasContext *dc, arg_typea *arg)
@@ -846,7 +846,7 @@ static bool trans_lwx(DisasContext *dc, arg_typea *arg)
/* lwx does not throw unaligned access errors, so force alignment */
tcg_gen_andi_tl(addr, addr, ~3);
-tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TEUL);
+tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TE | MO_UL);
tcg_gen_mov_tl(cpu_res_addr, addr);
if (arg->rd) {
@@ -930,13 +930,13 @@ static bool trans_sbi(DisasContext *dc, arg_typeb *arg)
static bool trans_sh(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
}
static bool trans_shr(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, true);
+return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
}
static bool trans_shea(DisasContext *dc, arg_typea *arg)
@@ -948,26 +948,26 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TEUW, MMU_NOMMU_IDX, false);
+return do_store(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_shi(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_store(dc, arg->rd, addr, MO_TEUW, dc->mem_index, false);
+return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
}
static bool trans_sw(DisasContext *dc, arg
[PATCH v4 08/16] target/microblaze: Set MO_TE once in do_load() / do_store()
All callers of do_load() / do_store() set MO_TE flag.
Set it once in the callees.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/microblaze/translate.c | 36 +++
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 86efabb83b5..0d51b2c468c 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -713,6 +713,8 @@ static bool do_load(DisasContext *dc, int rd, TCGv addr,
MemOp mop,
{
MemOp size = mop & MO_SIZE;
+mop |= MO_TE;
+
/*
* When doing reverse accesses we need to do two things.
*
@@ -780,13 +782,13 @@ static bool trans_lbui(DisasContext *dc, arg_typeb *arg)
static bool trans_lhu(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
}
static bool trans_lhur(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
+return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, true);
}
static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
@@ -798,26 +800,26 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
+return do_load(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_lhui(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
}
static bool trans_lw(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
}
static bool trans_lwr(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, true);
+return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, true);
}
static bool trans_lwea(DisasContext *dc, arg_typea *arg)
@@ -829,14 +831,14 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UL, MMU_NOMMU_IDX, false);
+return do_load(dc, arg->rd, addr, MO_UL, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_lwi(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_load(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+return do_load(dc, arg->rd, addr, MO_UL, dc->mem_index, false);
}
static bool trans_lwx(DisasContext *dc, arg_typea *arg)
@@ -863,6 +865,8 @@ static bool do_store(DisasContext *dc, int rd, TCGv addr,
MemOp mop,
{
MemOp size = mop & MO_SIZE;
+mop |= MO_TE;
+
/*
* When doing reverse accesses we need to do two things.
*
@@ -930,13 +934,13 @@ static bool trans_sbi(DisasContext *dc, arg_typeb *arg)
static bool trans_sh(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
}
static bool trans_shr(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, true);
+return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, true);
}
static bool trans_shea(DisasContext *dc, arg_typea *arg)
@@ -948,26 +952,26 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
return true;
#else
TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TE | MO_UW, MMU_NOMMU_IDX, false);
+return do_store(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
#endif
}
static bool trans_shi(DisasContext *dc, arg_typeb *arg)
{
TCGv addr = compute_ldst_addr_typeb(dc, arg->ra, arg->imm);
-return do_store(dc, arg->rd, addr, MO_TE | MO_UW, dc->mem_index, false);
+return do_store(dc, arg->rd, addr, MO_UW, dc->mem_index, false);
}
static bool trans_sw(DisasContext *dc, arg_typea *arg)
{
TCGv addr = compute_ldst_addr_typea(dc, arg->ra, arg->rb);
-return do_store(dc, arg->rd, addr, MO_TE | MO_UL, dc->mem_index, false);
+return do_store(dc, arg->rd, ad
[PATCH v4 04/16] hw/char/xilinx_uartlite: Make device endianness configurable
Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness on the single machine using the
device.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/xilinx_uartlite.c| 27 ++--
hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index 56955e0d74a..948da4263b9 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -57,6 +57,7 @@
struct XilinxUARTLite {
SysBusDevice parent_obj;
+bool little_endian_model;
MemoryRegion mmio;
CharBackend chr;
qemu_irq irq;
@@ -166,17 +167,21 @@ uart_write(void *opaque, hwaddr addr,
uart_update_irq(s);
}
-static const MemoryRegionOps uart_ops = {
-.read = uart_read,
-.write = uart_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
-.min_access_size = 1,
-.max_access_size = 4
-}
+static const MemoryRegionOps uart_ops[2] = {
+[0 ... 1] = {
+.read = uart_read,
+.write = uart_write,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 4,
+},
+},
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static const Property xilinx_uartlite_properties[] = {
+DEFINE_PROP_BOOL("little-endian", XilinxUARTLite, little_endian_model,
true),
DEFINE_PROP_CHR("chardev", XilinxUARTLite, chr),
};
@@ -214,6 +219,9 @@ static void xilinx_uartlite_realize(DeviceState *dev, Error
**errp)
{
XilinxUARTLite *s = XILINX_UARTLITE(dev);
+memory_region_init_io(&s->mmio, OBJECT(dev),
+ &uart_ops[s->little_endian_model],
+ s, "xlnx.xps-uartlite", R_MAX * 4);
qemu_chr_fe_set_handlers(&s->chr, uart_can_rx, uart_rx,
uart_event, NULL, s, NULL, true);
}
@@ -223,9 +231,6 @@ static void xilinx_uartlite_init(Object *obj)
XilinxUARTLite *s = XILINX_UARTLITE(obj);
sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
-memory_region_init_io(&s->mmio, obj, &uart_ops, s,
- "xlnx.xps-uartlite", R_MAX * 4);
sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
}
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9d4316b4036..96aed4ed1a3 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -107,6 +107,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
}
dev = qdev_new(TYPE_XILINX_UARTLITE);
+qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
qdev_prop_set_chr(dev, "chardev", serial_hd(0));
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
--
2.47.1
[PATCH v4 03/16] hw/timer/xilinx_timer: Make device endianness configurable
Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness for each machine using the device.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/microblaze/petalogix_ml605_mmu.c | 1 +
hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
hw/ppc/virtex_ml507.c| 1 +
hw/timer/xilinx_timer.c | 35 +++-
4 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/hw/microblaze/petalogix_ml605_mmu.c
b/hw/microblaze/petalogix_ml605_mmu.c
index cf3b9574db3..bbda70aa93b 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -127,6 +127,7 @@ petalogix_ml605_init(MachineState *machine)
/* 2 timers at irq 2 @ 100 Mhz. */
dev = qdev_new("xlnx.xps-timer");
+qdev_prop_set_bit(dev, "little-endian", true);
qdev_prop_set_uint32(dev, "one-timer-only", 0);
qdev_prop_set_uint32(dev, "clock-frequency", 100 * 100);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index fbf52ba8f2f..9d4316b4036 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -114,6 +114,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
/* 2 timers at irq 2 @ 62 Mhz. */
dev = qdev_new("xlnx.xps-timer");
+qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
qdev_prop_set_uint32(dev, "one-timer-only", 0);
qdev_prop_set_uint32(dev, "clock-frequency", 62 * 100);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 23238119273..f87c221d076 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -230,6 +230,7 @@ static void virtex_init(MachineState *machine)
/* 2 timers at irq 2 @ 62 Mhz. */
dev = qdev_new("xlnx.xps-timer");
+qdev_prop_set_bit(dev, "little-endian", false);
qdev_prop_set_uint32(dev, "one-timer-only", 0);
qdev_prop_set_uint32(dev, "clock-frequency", 62 * 100);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 6595cf5f517..d942ac226e6 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -3,6 +3,9 @@
*
* Copyright (c) 2009 Edgar E. Iglesias.
*
+ * DS573: https://docs.amd.com/v/u/en-US/xps_timer
+ * LogiCORE IP XPS Timer/Counter (v1.02a)
+ *
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
* in the Software without restriction, including without limitation the rights
@@ -69,6 +72,7 @@ struct XpsTimerState
{
SysBusDevice parent_obj;
+bool little_endian_model;
MemoryRegion mmio;
qemu_irq irq;
uint8_t one_timer_only;
@@ -189,18 +193,21 @@ timer_write(void *opaque, hwaddr addr,
timer_update_irq(t);
}
-static const MemoryRegionOps timer_ops = {
-.read = timer_read,
-.write = timer_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.impl = {
-.min_access_size = 4,
-.max_access_size = 4,
+static const MemoryRegionOps timer_ops[2] = {
+[0 ... 1] = {
+.read = timer_read,
+.write = timer_write,
+.impl = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
},
-.valid = {
-.min_access_size = 4,
-.max_access_size = 4
-}
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static void timer_hit(void *opaque)
@@ -233,8 +240,9 @@ static void xilinx_timer_realize(DeviceState *dev, Error
**errp)
ptimer_transaction_commit(xt->ptimer);
}
-memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "xlnx.xps-timer",
- R_MAX * 4 * num_timers(t));
+memory_region_init_io(&t->mmio, OBJECT(t),
+ &timer_ops[t->little_endian_model], t,
+ "xlnx.xps-timer", R_MAX * 4 * num_timers(t));
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
}
@@ -247,6 +255,7 @@ static void xilinx_timer_init(Object *obj)
}
static const Property xilinx_timer_properties[] = {
+DEFINE_PROP_BOOL("little-endian", XpsTimerState, little_endian_model,
true),
DEFINE_PROP_UINT32("clock-frequency", XpsTimerState, freq_hz, 62 *
100),
DEFINE_PROP_UINT8("one-timer-only", XpsTimerState, one_timer_only, 0),
};
--
2.47.1
[PATCH v4 10/16] target/microblaze: Consider endianness while translating code
Consider the CPU ENDI bit, swap instructions when the CPU
endianness doesn't match the binary one.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/microblaze/cpu.h | 7 +++
target/microblaze/translate.c | 5 +++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index f6879eee352..e44ddd53078 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -414,6 +414,13 @@ void mb_translate_code(CPUState *cs, TranslationBlock *tb,
/* Ensure there is no overlap between the two masks. */
QEMU_BUILD_BUG_ON(MSR_TB_MASK & IFLAGS_TB_MASK);
+static inline bool mb_cpu_is_big_endian(CPUState *cs)
+{
+MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+
+return !cpu->cfg.endi;
+}
+
static inline void cpu_get_tb_cpu_state(CPUMBState *env, vaddr *pc,
uint64_t *cs_base, uint32_t *flags)
{
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index b5389d65b2e..b54e5ac4b2f 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -710,7 +710,7 @@ static void record_unaligned_ess(DisasContext *dc, int rd,
static inline MemOp mo_endian(DisasContext *dc)
{
-return MO_TE;
+return dc->cfg->endi ? MO_LE : MO_BE;
}
static bool do_load(DisasContext *dc, int rd, TCGv addr, MemOp mop,
@@ -1647,7 +1647,8 @@ static void mb_tr_translate_insn(DisasContextBase *dcb,
CPUState *cs)
dc->tb_flags_to_set = 0;
-ir = translator_ldl(cpu_env(cs), &dc->base, dc->base.pc_next);
+ir = translator_ldl_swap(cpu_env(cs), &dc->base, dc->base.pc_next,
+ mb_cpu_is_big_endian(cs) != TARGET_BIG_ENDIAN);
if (!decode(dc, ir)) {
trap_illegal(dc, true);
}
--
2.47.1
[PATCH v4 05/16] hw/ssi/xilinx_spi: Make device endianness configurable
Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness on the single machine using the
device.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/microblaze/petalogix_ml605_mmu.c | 1 +
hw/ssi/xilinx_spi.c | 24 +++-
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/hw/microblaze/petalogix_ml605_mmu.c
b/hw/microblaze/petalogix_ml605_mmu.c
index bbda70aa93b..a795c6385b4 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -175,6 +175,7 @@ petalogix_ml605_init(MachineState *machine)
SSIBus *spi;
dev = qdev_new("xlnx.xps-spi");
+qdev_prop_set_bit(dev, "little-endian", true);
qdev_prop_set_uint8(dev, "num-ss-bits", NUM_SPI_FLASHES);
busdev = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(busdev, &error_fatal);
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index fd1ff12eb1d..299004ff36d 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -83,6 +83,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XilinxSPI, XILINX_SPI)
struct XilinxSPI {
SysBusDevice parent_obj;
+bool little_endian_model;
MemoryRegion mmio;
qemu_irq irq;
@@ -313,14 +314,17 @@ done:
xlx_spi_update_irq(s);
}
-static const MemoryRegionOps spi_ops = {
-.read = spi_read,
-.write = spi_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
-.min_access_size = 4,
-.max_access_size = 4
-}
+static const MemoryRegionOps spi_ops[2] = {
+[0 ... 1] = {
+.read = spi_read,
+.write = spi_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+},
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static void xilinx_spi_realize(DeviceState *dev, Error **errp)
@@ -339,7 +343,8 @@ static void xilinx_spi_realize(DeviceState *dev, Error
**errp)
sysbus_init_irq(sbd, &s->cs_lines[i]);
}
-memory_region_init_io(&s->mmio, OBJECT(s), &spi_ops, s,
+memory_region_init_io(&s->mmio, OBJECT(s),
+ &spi_ops[s->little_endian_model], s,
"xilinx-spi", R_MAX * 4);
sysbus_init_mmio(sbd, &s->mmio);
@@ -362,6 +367,7 @@ static const VMStateDescription vmstate_xilinx_spi = {
};
static const Property xilinx_spi_properties[] = {
+DEFINE_PROP_BOOL("little-endian", XilinxSPI, little_endian_model, true),
DEFINE_PROP_UINT8("num-ss-bits", XilinxSPI, num_cs, 1),
};
--
2.47.1
[PATCH v4 16/16] tests/functional: Run cross-endian microblaze tests
Ensure microblaze machines can run cross-endianness by
running all tests on all machines.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/functional/test_microblaze_s3adsp1800.py | 6 ++
tests/functional/test_microblazeel_s3adsp1800.py | 6 ++
2 files changed, 12 insertions(+)
diff --git a/tests/functional/test_microblaze_s3adsp1800.py
b/tests/functional/test_microblaze_s3adsp1800.py
index 0447097c048..f955f6f4021 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -62,5 +62,11 @@ class MicroblazeBigEndianMachine(MicroblazeMachine):
def test_microblaze_s3adsp1800_legacy_be(self):
self.do_ballerina_be_test('petalogix-s3adsp1800')
+def test_microblaze_s3adsp1800_be(self):
+self.do_ballerina_be_test('petalogix-s3adsp1800-be')
+
+def test_microblaze_s3adsp1800_le(self):
+self.do_xmaton_le_test('petalogix-s3adsp1800-le')
+
if __name__ == '__main__':
QemuSystemTest.main()
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py
b/tests/functional/test_microblazeel_s3adsp1800.py
index 56645bd0bb2..b10944bbb0c 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -16,5 +16,11 @@ class MicroblazeLittleEndianMachine(MicroblazeMachine):
def test_microblaze_s3adsp1800_legacy_le(self):
self.do_xmaton_le_test('petalogix-s3adsp1800')
+def test_microblaze_s3adsp1800_le(self):
+self.do_xmaton_le_test('petalogix-s3adsp1800-le')
+
+def test_microblaze_s3adsp1800_be(self):
+self.do_ballerina_be_test('petalogix-s3adsp1800-be')
+
if __name__ == '__main__':
QemuSystemTest.main()
--
2.47.1
[PATCH v4 14/16] tests/functional: Have microblaze tests inherit common parent class
Have the MicroblazeMachine class being common to both
MicroblazeBigEndianMachine and MicroblazeLittleEndianMachine
classes.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/functional/test_microblaze_s3adsp1800.py | 2 ++
tests/functional/test_microblazeel_s3adsp1800.py | 5 ++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/functional/test_microblaze_s3adsp1800.py
b/tests/functional/test_microblaze_s3adsp1800.py
index c4226f49cf3..2eff31f13a7 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -15,6 +15,8 @@ class MicroblazeMachine(QemuSystemTest):
timeout = 90
+class MicroblazeBigEndianMachine(MicroblazeMachine):
+
ASSET_IMAGE_BE = Asset(
('https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/'
'day17.tar.xz'),
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py
b/tests/functional/test_microblazeel_s3adsp1800.py
index 715ef3f79ac..a3e8d7ca48e 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -12,10 +12,9 @@
from qemu_test import QemuSystemTest, Asset
from qemu_test import wait_for_console_pattern
+from test_microblaze_s3adsp1800 import MicroblazeMachine
-class MicroblazeelMachine(QemuSystemTest):
-
-timeout = 90
+class MicroblazeLittleEndianMachine(MicroblazeMachine):
ASSET_IMAGE_LE = Asset(
('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
--
2.47.1
[PATCH v4 15/16] tests/functional: Move microblaze tests to common parent class
Move the xmaton and ballerina tests to the parent class.
Signed-off-by: Philippe Mathieu-Daudé
---
.../functional/test_microblaze_s3adsp1800.py | 27 +--
.../test_microblazeel_s3adsp1800.py | 26 +-
2 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/tests/functional/test_microblaze_s3adsp1800.py
b/tests/functional/test_microblaze_s3adsp1800.py
index 2eff31f13a7..0447097c048 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -7,6 +7,8 @@
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+import time
+from qemu_test import exec_command, exec_command_and_wait_for_pattern
from qemu_test import QemuSystemTest, Asset
from qemu_test import wait_for_console_pattern
@@ -15,13 +17,15 @@ class MicroblazeMachine(QemuSystemTest):
timeout = 90
-class MicroblazeBigEndianMachine(MicroblazeMachine):
-
ASSET_IMAGE_BE = Asset(
('https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/'
'day17.tar.xz'),
'3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
+ASSET_IMAGE_LE = Asset(
+('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
+'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
+
def do_ballerina_be_test(self, machine):
self.set_machine(machine)
self.archive_extract(self.ASSET_IMAGE_BE)
@@ -36,6 +40,25 @@ def do_ballerina_be_test(self, machine):
# message, that's why we don't test for a later string here. This
# needs some investigation by a microblaze wizard one day...
+def do_xmaton_le_test(self, machine):
+self.require_netdev('user')
+self.set_machine(machine)
+self.archive_extract(self.ASSET_IMAGE_LE)
+self.vm.set_console()
+self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
+tftproot = self.scratch_file('day13')
+self.vm.add_args('-nic', f'user,tftp={tftproot}')
+self.vm.launch()
+wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
+time.sleep(0.1)
+exec_command(self, 'root')
+time.sleep(0.1)
+exec_command_and_wait_for_pattern(self,
+'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
+'821cd3cab8efd16ad6ee5acc3642a8ea')
+
+class MicroblazeBigEndianMachine(MicroblazeMachine):
+
def test_microblaze_s3adsp1800_legacy_be(self):
self.do_ballerina_be_test('petalogix-s3adsp1800')
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py
b/tests/functional/test_microblazeel_s3adsp1800.py
index a3e8d7ca48e..56645bd0bb2 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -7,36 +7,12 @@
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
-import time
-from qemu_test import exec_command, exec_command_and_wait_for_pattern
-from qemu_test import QemuSystemTest, Asset
-from qemu_test import wait_for_console_pattern
+from qemu_test import QemuSystemTest
from test_microblaze_s3adsp1800 import MicroblazeMachine
class MicroblazeLittleEndianMachine(MicroblazeMachine):
-ASSET_IMAGE_LE = Asset(
-('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
-'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
-
-def do_xmaton_le_test(self, machine):
-self.require_netdev('user')
-self.set_machine(machine)
-self.archive_extract(self.ASSET_IMAGE_LE)
-self.vm.set_console()
-self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
-tftproot = self.scratch_file('day13')
-self.vm.add_args('-nic', f'user,tftp={tftproot}')
-self.vm.launch()
-wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
-time.sleep(0.1)
-exec_command(self, 'root')
-time.sleep(0.1)
-exec_command_and_wait_for_pattern(self,
-'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
-'821cd3cab8efd16ad6ee5acc3642a8ea')
-
def test_microblaze_s3adsp1800_legacy_le(self):
self.do_xmaton_le_test('petalogix-s3adsp1800')
--
2.47.1
[PATCH v4 06/16] hw/arm/xlnx-zynqmp: Use &error_abort for programming errors
When a property value is static (not provided by QMP or CLI),
error shouldn't happen, otherwise it is a programming error.
Therefore simplify and use &error_abort as this can't fail.
Reported-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Anton Johansson
---
hw/arm/xlnx-zynqmp.c | 38 --
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index bd5b0dd5e76..d6022ff2d3d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -689,16 +689,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
* - SDIO Specification Version 3.0
* - eMMC Specification Version 4.51
*/
-if (!object_property_set_uint(sdhci, "sd-spec-version", 3, errp)) {
-return;
-}
-if (!object_property_set_uint(sdhci, "capareg", SDHCI_CAPABILITIES,
- errp)) {
-return;
-}
-if (!object_property_set_uint(sdhci, "uhs", UHS_I, errp)) {
-return;
-}
+object_property_set_uint(sdhci, "sd-spec-version", 3, &error_abort);
+object_property_set_uint(sdhci, "capareg", SDHCI_CAPABILITIES,
+ &error_abort);
+object_property_set_uint(sdhci, "uhs", UHS_I, &error_abort);
if (!sysbus_realize(SYS_BUS_DEVICE(sdhci), errp)) {
return;
}
@@ -763,14 +757,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
xlnx_zynqmp_create_unimp_mmio(s);
for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA_CH; i++) {
-if (!object_property_set_uint(OBJECT(&s->gdma[i]), "bus-width", 128,
- errp)) {
-return;
-}
-if (!object_property_set_link(OBJECT(&s->gdma[i]), "dma",
- OBJECT(system_memory), errp)) {
-return;
-}
+object_property_set_uint(OBJECT(&s->gdma[i]), "bus-width", 128,
+ &error_abort);
+object_property_set_link(OBJECT(&s->gdma[i]), "dma",
+ OBJECT(system_memory), &error_abort);
if (!sysbus_realize(SYS_BUS_DEVICE(&s->gdma[i]), errp)) {
return;
}
@@ -811,10 +801,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0,
qdev_get_gpio_in(DEVICE(&s->qspi_irq_orgate), 0));
-if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
- OBJECT(&s->qspi_dma), errp)) {
- return;
-}
+object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
+ OBJECT(&s->qspi_dma), &error_abort);
if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
return;
}
@@ -833,10 +821,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
}
for (i = 0; i < XLNX_ZYNQMP_NUM_USB; i++) {
-if (!object_property_set_link(OBJECT(&s->usb[i].sysbus_xhci), "dma",
- OBJECT(system_memory), errp)) {
-return;
-}
+object_property_set_link(OBJECT(&s->usb[i].sysbus_xhci), "dma",
+ OBJECT(system_memory), &error_abort);
qdev_prop_set_uint32(DEVICE(&s->usb[i].sysbus_xhci), "intrs", 4);
qdev_prop_set_uint32(DEVICE(&s->usb[i].sysbus_xhci), "slots", 2);
--
2.47.1
[PATCH v4 13/16] tests/functional: Allow microblaze tests to take a machine name argument
Make microblaze tests a bit more generic.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/functional/test_microblaze_s3adsp1800.py | 7 +--
tests/functional/test_microblazeel_s3adsp1800.py | 7 +--
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/tests/functional/test_microblaze_s3adsp1800.py
b/tests/functional/test_microblaze_s3adsp1800.py
index fac364b1ea9..c4226f49cf3 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -20,8 +20,8 @@ class MicroblazeMachine(QemuSystemTest):
'day17.tar.xz'),
'3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
-def test_microblaze_s3adsp1800_be(self):
-self.set_machine('petalogix-s3adsp1800')
+def do_ballerina_be_test(self, machine):
+self.set_machine(machine)
self.archive_extract(self.ASSET_IMAGE_BE)
self.vm.set_console()
self.vm.add_args('-kernel',
@@ -34,5 +34,8 @@ def test_microblaze_s3adsp1800_be(self):
# message, that's why we don't test for a later string here. This
# needs some investigation by a microblaze wizard one day...
+def test_microblaze_s3adsp1800_legacy_be(self):
+self.do_ballerina_be_test('petalogix-s3adsp1800')
+
if __name__ == '__main__':
QemuSystemTest.main()
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py
b/tests/functional/test_microblazeel_s3adsp1800.py
index 5d353dba5d2..715ef3f79ac 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -21,9 +21,9 @@ class MicroblazeelMachine(QemuSystemTest):
('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
-def test_microblazeel_s3adsp1800_le(self):
+def do_xmaton_le_test(self, machine):
self.require_netdev('user')
-self.set_machine('petalogix-s3adsp1800')
+self.set_machine(machine)
self.archive_extract(self.ASSET_IMAGE_LE)
self.vm.set_console()
self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
@@ -38,5 +38,8 @@ def test_microblazeel_s3adsp1800_le(self):
'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
'821cd3cab8efd16ad6ee5acc3642a8ea')
+def test_microblaze_s3adsp1800_legacy_le(self):
+self.do_xmaton_le_test('petalogix-s3adsp1800')
+
if __name__ == '__main__':
QemuSystemTest.main()
--
2.47.1
[PATCH v4 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines
Introduce an abstract machine parent class which defines
the 'little_endian' property. Duplicate the current machine,
which endian is tied to the binary endianness, to one big
endian and a little endian machine; updating the machine
description. Keep the current default machine for each binary.
'petalogix-s3adsp1800' machine is aliased as:
- 'petalogix-s3adsp1800-be' on big-endian binary,
- 'petalogix-s3adsp1800-le' on little-endian one.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/microblaze/petalogix_s3adsp1800_mmu.c | 62 +++-
1 file changed, 51 insertions(+), 11 deletions(-)
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 96aed4ed1a3..aea727eb7ee 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -55,8 +55,17 @@
#define ETHLITE_IRQ 1
#define UARTLITE_IRQ3
+typedef struct PetalogixS3adsp1800MachineClass {
+MachineClass parent_obj;
+
+bool little_endian;
+} PetalogixS3adsp1800MachineClass;
+
#define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
-MACHINE_TYPE_NAME("petalogix-s3adsp1800")
+MACHINE_TYPE_NAME("petalogix-s3adsp1800-common")
+DECLARE_CLASS_CHECKERS(PetalogixS3adsp1800MachineClass,
+ PETALOGIX_S3ADSP1800_MACHINE,
+ TYPE_PETALOGIX_S3ADSP1800_MACHINE)
static void
petalogix_s3adsp1800_init(MachineState *machine)
@@ -71,11 +80,13 @@ petalogix_s3adsp1800_init(MachineState *machine)
MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
qemu_irq irq[32];
MemoryRegion *sysmem = get_system_memory();
+PetalogixS3adsp1800MachineClass *pmc;
+pmc = PETALOGIX_S3ADSP1800_MACHINE_GET_CLASS(machine);
cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
object_property_set_bool(OBJECT(cpu), "little-endian",
- !TARGET_BIG_ENDIAN, &error_abort);
+ pmc->little_endian, &error_abort);
qdev_realize(DEVICE(cpu), NULL, &error_abort);
/* Attach emulated BRAM through the LMB. */
@@ -95,7 +106,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
64 * KiB, 1, 0x89, 0x18, 0x, 0x0, 1);
dev = qdev_new("xlnx.xps-intc");
-qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
qdev_prop_set_uint32(dev, "kind-of-intr",
1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -107,7 +118,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
}
dev = qdev_new(TYPE_XILINX_UARTLITE);
-qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
qdev_prop_set_chr(dev, "chardev", serial_hd(0));
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
@@ -115,7 +126,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
/* 2 timers at irq 2 @ 62 Mhz. */
dev = qdev_new("xlnx.xps-timer");
-qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
qdev_prop_set_uint32(dev, "one-timer-only", 0);
qdev_prop_set_uint32(dev, "clock-frequency", 62 * 100);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -123,7 +134,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
dev = qdev_new("xlnx.xps-ethernetlite");
-qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
+qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
qemu_configure_nic_device(dev, true, NULL);
qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
@@ -133,26 +144,55 @@ petalogix_s3adsp1800_init(MachineState *machine)
create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x1);
-microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
+microblaze_load_kernel(cpu, pmc->little_endian, ddr_base, ram_size,
machine->initrd_filename,
BINARY_DEVICE_TREE_FILE,
NULL);
}
-static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void
*data)
+static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
+bool little_endian)
{
MachineClass *mc = MACHINE_CLASS(oc);
+PetalogixS3adsp1800MachineClass *pmc =
PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
-mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
mc->init = petalogix_s3adsp1800_init;
Re: [PATCH] rust: add --rust-target option for bindgen
On Thu, Feb 6, 2025 at 12:37 PM Philippe Mathieu-Daudé
wrote:
> > if bindgen.version().version_compare('<0.61.0')
> > # default in 0.61+
> > bindgen_args += ['--size_t-is-usize']
>
> Should this be merged directly on master as build-fix?
If it's breaking CI I can send a pull request later. I wasn't sure if
it was just my branch with other Rust changes.
Paolo
[PATCH v4 02/16] hw/net/xilinx_ethlite: Make device endianness configurable
Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness on the single machine using the
device.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
hw/net/xilinx_ethlite.c | 20 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 0506497ad0a..fbf52ba8f2f 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -121,6 +121,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
dev = qdev_new("xlnx.xps-ethernetlite");
+qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
qemu_configure_nic_device(dev, true, NULL);
qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 14bf2b2e17a..103e53831a7 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -90,6 +90,7 @@ struct XlnxXpsEthLite
NICState *nic;
NICConf conf;
+bool little_endian_model;
uint32_t c_tx_pingpong;
uint32_t c_rx_pingpong;
unsigned int port_index; /* dual port RAM index */
@@ -183,10 +184,10 @@ static void port_tx_write(void *opaque, hwaddr addr,
uint64_t value,
}
}
-static const MemoryRegionOps eth_porttx_ops = {
+static const MemoryRegionOps eth_porttx_ops[2] = {
+[0 ... 1] = {
.read = port_tx_read,
.write = port_tx_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
.impl = {
.min_access_size = 4,
.max_access_size = 4,
@@ -195,6 +196,9 @@ static const MemoryRegionOps eth_porttx_ops = {
.min_access_size = 4,
.max_access_size = 4,
},
+},
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
@@ -232,10 +236,10 @@ static void port_rx_write(void *opaque, hwaddr addr,
uint64_t value,
}
}
-static const MemoryRegionOps eth_portrx_ops = {
+static const MemoryRegionOps eth_portrx_ops[2] = {
+[0 ... 1] = {
.read = port_rx_read,
.write = port_rx_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
.impl = {
.min_access_size = 4,
.max_access_size = 4,
@@ -244,6 +248,9 @@ static const MemoryRegionOps eth_portrx_ops = {
.min_access_size = 4,
.max_access_size = 4,
},
+},
+[0].endianness = DEVICE_BIG_ENDIAN,
+[1].endianness = DEVICE_LITTLE_ENDIAN,
};
static bool eth_can_rx(NetClientState *nc)
@@ -328,7 +335,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error
**errp)
BUFSZ_MAX, &error_abort);
memory_region_add_subregion(&s->container, 0x0800 * i,
&s->port[i].txbuf);
memory_region_init_io(&s->port[i].txio, OBJECT(dev),
- ð_porttx_ops, s,
+ ð_porttx_ops[s->little_endian_model], s,
i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
4 * TX_MAX);
memory_region_add_subregion(&s->container, i ? A_TX_BASE1 : A_TX_BASE0,
@@ -340,7 +347,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error
**errp)
memory_region_add_subregion(&s->container, 0x1000 + 0x0800 * i,
&s->port[i].rxbuf);
memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
- ð_portrx_ops, s,
+ ð_portrx_ops[s->little_endian_model], s,
i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
4 * RX_MAX);
memory_region_add_subregion(&s->container, i ? A_RX_BASE1 : A_RX_BASE0,
@@ -363,6 +370,7 @@ static void xilinx_ethlite_init(Object *obj)
}
static const Property xilinx_ethlite_properties[] = {
+DEFINE_PROP_BOOL("little-endian", XlnxXpsEthLite, little_endian_model,
true),
DEFINE_PROP_UINT32("tx-ping-pong", XlnxXpsEthLite, c_tx_pingpong, 1),
DEFINE_PROP_UINT32("rx-ping-pong", XlnxXpsEthLite, c_rx_pingpong, 1),
DEFINE_NIC_PROPERTIES(XlnxXpsEthLite, conf),
--
2.47.1
[PATCH v4 12/16] tests/functional: Explicit endianness of microblaze assets
The archive used in test_microblaze_s3adsp1800.py (testing a
big-endian target) contains a big-endian kernel. Rename using
the _BE suffix.
Similarly, the archive in test_microblazeel_s3adsp1800 (testing
a little-endian target) contains a little-endian kernel. Rename
using _LE suffix.
These changes will help when adding cross-endian kernel tests.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Thomas Huth
---
tests/functional/test_microblaze_s3adsp1800.py | 6 +++---
tests/functional/test_microblazeel_s3adsp1800.py | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/functional/test_microblaze_s3adsp1800.py
b/tests/functional/test_microblaze_s3adsp1800.py
index 2c4464bd05a..fac364b1ea9 100755
--- a/tests/functional/test_microblaze_s3adsp1800.py
+++ b/tests/functional/test_microblaze_s3adsp1800.py
@@ -15,14 +15,14 @@ class MicroblazeMachine(QemuSystemTest):
timeout = 90
-ASSET_IMAGE = Asset(
+ASSET_IMAGE_BE = Asset(
('https://qemu-advcal.gitlab.io/qac-best-of-multiarch/download/'
'day17.tar.xz'),
'3ba7439dfbea7af4876662c97f8e1f0cdad9231fc166e4861d17042489270057')
-def test_microblaze_s3adsp1800(self):
+def test_microblaze_s3adsp1800_be(self):
self.set_machine('petalogix-s3adsp1800')
-self.archive_extract(self.ASSET_IMAGE)
+self.archive_extract(self.ASSET_IMAGE_BE)
self.vm.set_console()
self.vm.add_args('-kernel',
self.scratch_file('day17', 'ballerina.bin'))
diff --git a/tests/functional/test_microblazeel_s3adsp1800.py
b/tests/functional/test_microblazeel_s3adsp1800.py
index c382afe6bfa..5d353dba5d2 100755
--- a/tests/functional/test_microblazeel_s3adsp1800.py
+++ b/tests/functional/test_microblazeel_s3adsp1800.py
@@ -17,14 +17,14 @@ class MicroblazeelMachine(QemuSystemTest):
timeout = 90
-ASSET_IMAGE = Asset(
+ASSET_IMAGE_LE = Asset(
('http://www.qemu-advent-calendar.org/2023/download/day13.tar.gz'),
'b9b3d43c5dd79db88ada495cc6e0d1f591153fe41355e925d791fbf44de50c22')
-def test_microblazeel_s3adsp1800(self):
+def test_microblazeel_s3adsp1800_le(self):
self.require_netdev('user')
self.set_machine('petalogix-s3adsp1800')
-self.archive_extract(self.ASSET_IMAGE)
+self.archive_extract(self.ASSET_IMAGE_LE)
self.vm.set_console()
self.vm.add_args('-kernel', self.scratch_file('day13', 'xmaton.bin'))
tftproot = self.scratch_file('day13')
--
2.47.1
Re: [PATCH v4 19/33] migration: Add save_live_complete_precopy_thread handler
On 5.02.2025 16:55, Peter Xu wrote:
On Wed, Feb 05, 2025 at 12:53:21PM +0100, Maciej S. Szmigiero wrote:
On 4.02.2025 21:34, Peter Xu wrote:
On Tue, Feb 04, 2025 at 08:32:15PM +0100, Maciej S. Szmigiero wrote:
On 4.02.2025 18:54, Peter Xu wrote:
On Thu, Jan 30, 2025 at 11:08:40AM +0100, Maciej S. Szmigiero wrote:
+static int multifd_device_state_save_thread(void *opaque)
+{
+struct MultiFDDSSaveThreadData *data = opaque;
+int ret;
+
+ret = data->hdlr(data->idstr, data->instance_id, &send_threads_abort,
+ data->handler_opaque);
I thought we discussed somewhere and the plan was we could use Error** here
to report errors. Would that still make sense, or maybe I lost some
context?
That was about *load* threads, here these are *save* threads.
Ah OK.
Save handlers do not return an Error value, neither save_live_iterate, nor
save_live_complete_precopy or save_state does so.
Let's try to make new APIs work with Error* if possible.
Let's assume that these threads return an Error object.
What's qemu_savevm_state_complete_precopy_iterable() supposed to do with it?
IIUC it's not about qemu_savevm_state_complete_precopy_iterable() in this
context, as the Error* will be used in one of the thread of the pool, not
migration thread.
The goal is to be able to set Error* with migrate_set_error(), so that when
migration failed, query-migrate can return the error to libvirt, so
migration always tries to remember the 1st error hit if ever possible.
It's multifd_device_state_save_thread() to do migrate_set_error(), not in
migration thread. qemu_savevm_state_complete_*() are indeed not ready to
pass Errors, but it's not in the discussed stack.
I understand what are you proposing now - you haven't written about using
migrate_set_error() for save threads earlier, just about returning an Error
object.
While this might work it has tendency to uncover errors in other parts of
the migration core - much as using it in the load threads case uncovered
the TLS session error.
(Speaking of which, could you please respond to the issue at the bottom of
this message from 2 days ago?:
https://lore.kernel.org/qemu-devel/150a9741-daab-4724-add0-f35257e86...@maciej.szmigiero.name/
It is blocking rework of the TLS session EOF handling in this patch set.
Thanks.)
But I can try this migrate_set_error() approach here and see if something
breaks.
(..)
Meanwhile, I still feel uneasy on having these globals (send_threads_abort,
send_threads_ret). Can we make MultiFDDSSaveThreadData the only interface
between migration and the threads impl? So I wonder if it can be:
ret = data->hdlr(data);
With extended struct like this (I added thread_error and thread_quit):
struct MultiFDDSSaveThreadData {
SaveLiveCompletePrecopyThreadHandler hdlr;
char *idstr;
uint32_t instance_id;
void *handler_opaque;
/*
* Should be NULL when struct passed over to thread, the thread should
* set this if the handler would return false. It must be kept NULL if
* the handler returned true / success.
*/
Error *thread_error;
As I mentioned above, these handlers do not generally return Error type,
so this would need to be an *int;
/*
* Migration core would set this when it wants to notify thread to
* quit, for example, when error occured in other threads, or migration
is
* cancelled by the user.
*/
bool thread_quit;
^ I guess that was supposed to be a pointer too (*thread_quit).
It's my intention to make this bool, to make everything managed per-thread.
But that's unnecessary since this flag is common to all these threads.
One bool would be enough, but you'll need to export another API for VFIO to
use otherwise. I suppose that's ok too.
Some context of multifd threads and how that's done there..
We started with one "quit" per thread struct, but then we switched to one
bool exactly as you said, see commit 15f3f21d598148.
If you want to stick with one bool, it's okay too, you can export something
similar in misc.h, e.g. multifd_device_state_save_thread_quitting(), then
we can avoid passing in the "quit" either as handler parameter, or
per-thread flag.
Of course I can "export" this flag via a getter function rather than passing
it as a parameter to SaveLiveCompletePrecopyThreadHandler.
It's actually what we do with multifd, these are a bunch of extra threads
to differeciate from the "IO threads" / "multifd threads".
};
Then if any multifd_device_state_save_thread() failed, for example, it
should notify all threads to quit by setting thread_quit, instead of
relying on yet another global variable to show migration needs to quit.
multifd_abort_device_state_save_threads() needs to access
send_threads_abort too.
This may need to become something like:
QLIST_FOREACH() {
MultiFDDSSaveThreadData *data = ...;
data->thread_quit = true;
}
Re: [PATCH v2] qom: reverse order of instance_post_init calls
On 6/2/25 10:58, Paolo Bonzini wrote: Currently, the instance_post_init calls are performed from the leaf class and all the way up to Object. This is incorrect because the leaf class cannot observe property values applied by the superclasses; for example, a compat property will be set on a device *after* the class's post_init callback has run. In particular this makes it impossible for implementations of accel_cpu_instance_init() to operate based on the actual values of the properties, though it seems that cxl_dsp_instance_post_init and rp_instance_post_init might have similar issues. Follow instead the same order as instance_init, starting with Object and running the child class's instance_post_init after the parent. Signed-off-by: Paolo Bonzini --- include/qom/object.h | 3 ++- qom/object.c | 8 2 files changed, 6 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] rust: add --rust-target option for bindgen
On 6/2/25 12:38, Paolo Bonzini wrote:
On Thu, Feb 6, 2025 at 12:37 PM Philippe Mathieu-Daudé
wrote:
if bindgen.version().version_compare('<0.61.0')
# default in 0.61+
bindgen_args += ['--size_t-is-usize']
Should this be merged directly on master as build-fix?
If it's breaking CI I can send a pull request later. I wasn't sure if
it was just my branch with other Rust changes.
No, this is now affecting the main repository, so also all forks.
Re: [PATCH] rust: add --rust-target option for bindgen
On 6/2/25 12:15, Paolo Bonzini wrote:
Without it, recent bindgen will give an error
error: extern block cannot be declared unsafe
if rustc is not new enough to support the "unsafe extern" construct.
Cc: qemu-r...@nongnu.org
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
---
meson.build | 3 +++
1 file changed, 3 insertions(+)
diff --git a/meson.build b/meson.build
index 2c9ac9cfe1e..131b2225ab6 100644
--- a/meson.build
+++ b/meson.build
@@ -4054,6 +4054,9 @@ if have_rust
bindgen_args += ['--formatter', 'none']
endif
endif
+ if bindgen.version().version_compare('>=0.66.0')
+bindgen_args += ['--rust-target', '1.59']
+ endif
if bindgen.version().version_compare('<0.61.0')
# default in 0.61+
bindgen_args += ['--size_t-is-usize']
Should this be merged directly on master as build-fix?
Re: [PATCH v6 0/7] target/riscv: Add support for Control Transfer Records Ext.
On Thu, Feb 6, 2025 at 5:39 AM Alistair Francis wrote: > > On Wed, Feb 5, 2025 at 9:21 PM Rajnesh Kanwal wrote: > > > > This series enables Control Transfer Records extension support on riscv > > platform. This extension is similar to Arch LBR in x86 and BRBE in ARM. > > The Extension has been ratified and this series is based on v1.0 [0] > > > > CTR extension depends on both the implementation of S-mode and Sscsrind > > extension v1.0.0 [1]. CTR access ctrsource, ctrtartget and ctrdata CSRs > > using > > sscsrind extension. > > > > The series is based on Smcdeleg/Ssccfg counter delegation extension [2] > > patches [3]. CTR itself doesn't depend on counter delegation support. This > > rebase is basically to include the Smcsrind patches. > > > > Here is the link to a quick start guide [4] to setup and run a basic perf > > demo > > on Linux to use CTR Ext. > > > > Qemu patches can be found here: > > https://github.com/rajnesh-kanwal/qemu/tree/b4/ctr_upstream_v6 > > > > Opensbi patch can be found here: > > https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream_v2 > > > > Linux kernel patches can be found here: > > https://github.com/rajnesh-kanwal/linux/tree/b4/ctr_upstream_v2 > > > > [0]: > > https://github.com/riscv/riscv-control-transfer-records/releases/tag/v1.0 > > [1]: > > https://github.com/riscvarchive/riscv-indirect-csr-access/releases/tag/v1.0.0 > > [2]: > > https://github.com/riscvarchive/riscv-smcdeleg-ssccfg/releases/tag/v1.0.0 > > [3]: > > https://lore.kernel.org/qemu-riscv/20241203-counter_delegation-v4-0-c12a89bae...@rivosinc.com/ > > [4]: > > https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine > > > > Signed-off-by: Rajnesh Kanwal > > --- > > Changelog: > > v6: Rebased on latest riscv-to-apply.for-upstream. > > It should be rebased on > https://github.com/alistair23/qemu/tree/riscv-to-apply.next > > I applied the first 6 patches as they apply cleanly > > Alistair > Sorry for the inconvenience. Strangely cherry-pick, patch and git am -3 all seem to work fine but git am seems to be failing. I looked into the conflict and it looks like am expects ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12), but gets this in cpu.c. ISA_EXT_DATA_ENTRY(ssu64xl, PRIV_VERSION_1_12_0, has_priv_1_12), I have tried following cmds on top of riscv-to-apply.next and these fetch the last patch only and apply the last patch perfectly fine. b4 am -i -P="-1" 20250205-b4-ctr_upstream_v6-v6-0-439d8e06c...@rivosinc.com git am -3 ./v6_20250205_rkanwal_target_riscv_add_support_for_control_transfer_records_ext.mbx If you want I can send v7 as well rebased on top of riscv-to-apply.next. Thanks Rajnesh > > > > v5: Improvements based on Richard Henderson's feedback. > > - Fixed code gen logic to use gen_update_pc() instead of > > tcg_constant_tl(). > > - Some function renaming. > > - Rebased onto v4 of counter delegation series. > > - > > https://lore.kernel.org/qemu-riscv/20241205-b4-ctr_upstream_v3-v5-0-60b993aa5...@rivosinc.com/ > > > > v4: Improvements based on Richard Henderson's feedback. > > - Refactored CTR related code generation to move more code into > > translation side and avoid unnecessary code execution in generated > > code. > > - Added missing code in machine.c to migrate the new state. > > - > > https://lore.kernel.org/r/20241204-b4-ctr_upstream_v3-v4-0-d3ce6bef9...@rivosinc.com > > > > v3: Improvements based on Jason Chien and Frank Chang's feedback. > > - Created single set of MACROs for CTR CSRs in cpu_bit.h > > - Some fixes in riscv_ctr_add_entry. > > - Return zero for vs/sireg4-6 for CTR 0x200 to 0x2ff range. > > - Improved extension dependency check. > > - Fixed invalid ctrctl csr selection bug in riscv_ctr_freeze. > > - Added implied rules for Smctr and Ssctr. > > - Added missing SMSTATEEN0_CTR bit in mstateen0 and hstateen0 write ops. > > - Some more cosmetic changes. > > - > > https://lore.kernel.org/qemu-riscv/20241104-b4-ctr_upstream_v3-v3-0-32fd3c482...@rivosinc.com/ > > > > v2: Lots of improvements based on Jason Chien's feedback including: > > - Added CTR recording for cm.jalt, cm.jt, cm.popret, cm.popretz. > > - Fixed and added more CTR extension enable checks. > > - Fixed CTR CSR predicate functions. > > - Fixed external trap xTE bit checks. > > - One fix in freeze function for VS-mode. > > - Lots of minor code improvements. > > - Added checks in sctrclr instruction helper. > > - > > https://lore.kernel.org/qemu-riscv/20240619152708.135991-1-rkan...@rivosinc.com/ > > > > v1: > > - > > https://lore.kernel.org/qemu-riscv/20240529160950.132754-1-rkan...@rivosinc.com/ > > > > --- > > Rajnesh Kanwal (7): > > target/riscv: Remove obsolete sfence.vm instruction > > target/riscv: Add Control Transfer Records CSR definitions. > > target/riscv: Add support for Control Transfer Records extension CSRs. > > target/riscv: Add suppor
Re: [PATCH 2/2] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands (8.2.9.9.5.3)
On 24/01/25 03:19PM, Jonathan Cameron wrote:
On Thu, 23 Jan 2025 10:39:03 +0530
Vinayak Holikatti wrote:
CXL spec 3.1 section 8.2.9.9.5.3 describes media operations commands.
CXL devices supports media operations Sanitize and Write zero command.
As before, don't indent this.
Signed-off-by: Vinayak Holikatti
---
hw/cxl/cxl-mailbox-utils.c | 217 ++--
include/hw/cxl/cxl_device.h | 11 ++
2 files changed, 220 insertions(+), 8 deletions(-)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 2315d07fb1..89847ddd9d 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1722,6 +1722,145 @@ static CXLRetCode cmd_sanitize_overwrite(const struct
cxl_cmd *cmd,
return CXL_MBOX_BG_STARTED;
}
+#define DPA_RANGE_GRANULARITY 64
Could use existing CXL_CACHELINE_SIZE definition for this, though I guess
strictly speaking it could be unrelated.
+struct dpa_range_list_entry {
+uint64_t starting_dpa;
+uint64_t length;
+};
+
+static int validate_dpa_addr(CXLType3Dev *ct3d, uint64_t dpa_addr,
+ size_t length)
+{
+MemoryRegion *vmr = NULL, *pmr = NULL, *dc_mr = NULL;
+uint64_t vmr_size = 0, pmr_size = 0, dc_size = 0;
+int rc = 0;
+
+if ((dpa_addr % DPA_RANGE_GRANULARITY) ||
+ (length % DPA_RANGE_GRANULARITY)) {
Probably makes sense to also check for length 0 here as that would
be very odd if sent.
+return -EINVAL;
+}
+
+if (ct3d->hostvmem) {
+vmr = host_memory_backend_get_memory(ct3d->hostvmem);
+vmr_size = memory_region_size(vmr);
+}
+if (ct3d->hostpmem) {
+pmr = host_memory_backend_get_memory(ct3d->hostpmem);
+pmr_size = memory_region_size(pmr);
+}
+if (ct3d->dc.host_dc) {
+dc_mr = host_memory_backend_get_memory(ct3d->dc.host_dc);
+dc_size = memory_region_size(dc_mr);
+}
+
+if (!vmr && !pmr && !dc_mr) {
+return -ENODEV;
+}
+
+if (dpa_addr >= vmr_size + pmr_size + dc_size ||
+(dpa_addr + length) > vmr_size + pmr_size + dc_size) {
If length is checked as non zero above, only the second test is needed.
+return -EINVAL;
+}
+
+if (dpa_addr > vmr_size + pmr_size) {
+if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) {
+return -ENODEV;
+}
+}
+
+
+return rc;
return 0. rc is never set to anything else.
+}
+
+static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length,
+ uint8_t fill_value)
+{
+
+MemoryRegion *vmr = NULL, *pmr = NULL;
+uint64_t vmr_size = 0, pmr_size = 0;
+AddressSpace *as = NULL;
+MemTxAttrs mem_attrs = {0};
+
+if (ct3d->hostvmem) {
+vmr = host_memory_backend_get_memory(ct3d->hostvmem);
+vmr_size = memory_region_size(vmr);
+}
+if (ct3d->hostpmem) {
+pmr = host_memory_backend_get_memory(ct3d->hostpmem);
+pmr_size = memory_region_size(pmr);
+}
+
+if (dpa_addr < vmr_size) {
+as = &ct3d->hostvmem_as;
+} else if (dpa_addr < vmr_size + pmr_size) {
+as = &ct3d->hostpmem_as;
+} else {
+if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) {
+return -ENODEV;
+}
+as = &ct3d->dc.host_dc_as;
+}
You could factor out everything down to here and then use that
for the validate_dpa_addr() as finding an address space means
we also checked the address is valid. Otherwise it does not match.
Didnt get what you meant, validate_dpa_addr is meant for
checking valid dpa address and sanitize_range is
get the address space handle to do actual sanitize
of dpa address, so two are different purposes,
+
+return address_space_set(as, dpa_addr,
odd spacing after return. Should just be one space.
+ fill_value, length, mem_attrs);
+}
+
+/* Perform the actual device zeroing */
+static void __do_sanitize(CXLType3Dev *ct3d)
+{
+struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize;
Single space only before *san_info
+int dpa_range_count = san_info->dpa_range_count;
+int rc = 0;
+
+for (int i = 0; i < dpa_range_count; i++) {
+rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa,
+san_info->dpa_range_list[i].length, san_info->fill_value);
+if (rc) {
+goto exit;
+}
+}
+cxl_discard_all_event_records(&ct3d->cxl_dstate);
Add a comment on why we are deleting event records when sanitizing a small
part of memory?
+exit:
+g_free(ct3d->media_op_sanitize);
+ct3d->media_op_sanitize = NULL;
+return;
+}
+
+static int get_sanitize_duration(uint64_t total_mem)
Where did this come from? Factor out the existing code
in cmd_santize_overwrite() instead of duplicating this stack
of if/else if
Ideally do that as a precursor patch as it's just code movement.
+{
+int secs = 0;
+
+if
Re: [PATCH 2/2] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands (8.2.9.9.5.3)
On 03/02/25 05:02PM, Adam Manzanares wrote:
On Mon, Feb 03, 2025 at 11:33:54AM +, Jonathan Cameron wrote:
> >
> > > +int dpa_range_count = san_info->dpa_range_count;
> > > +int rc = 0;
> > > +
> > > +for (int i = 0; i < dpa_range_count; i++) {
> > > +rc = sanitize_range(ct3d,
san_info->dpa_range_list[i].starting_dpa,
> > > +san_info->dpa_range_list[i].length,
san_info->fill_value);
> > > +if (rc) {
> > > +goto exit;
> > > +}
> > > +}
> > > +cxl_discard_all_event_records(&ct3d->cxl_dstate);
> >
> > Add a comment on why we are deleting event records when sanitizing a small
> > part of memory?
> >
>
> See response below for disabling the media state. Same section referenced
> below, 8.2.10.9.5.1 states all event logs should be deleted. Outcome
> depends on how we interpret "follow the method described in 8.2.10.9.5.1".
>
This also sounds like reading too much into that comment.
Agreed, Vinayak let's drop the discard of all event records
from this patch.
Ok Adam will drop the discard of all event records
> > > +}
> > > +
> > > +start_bg:
> > > +/* EBUSY other bg cmds as of now */
> > > +cci->bg.runtime = secs * 1000UL;
> > > +*len_out = 0;
> > > +/* sanitize when done */
> > > +cxl_dev_disable_media(&ct3d->cxl_dstate);
> > Why? This is santizing part of the device. As I undestand it the
> > main aim is to offload cleanup when the device is in use. Definitely
> > don't want to disable media. If I'm missing a reason please give
> > a spec reference.
>
> Table 8-164, sanitize description mentions to follow method
> in 8.2.10.9.5.1, which does call out placing device in disabled
> state, but I'm not sure if method refers to all text in 8.2.10.9.5.1
> or the method devices uses to sanitize internally.
I think it is meant to just be the method of sanitizing.
Ok, let's use this interpretation. Vinayak, can you remove this as well
and then we put a comment in the patch that media op sanitize is targeted
so no need to disable media or clear event logs.
ok Adam, will remove this as well and comment as needed
>
> I would imagine since sanitize is destructive we would not want to return
> any data from device ranges impacted by sanitize. I believe a simple
> way to achieve this is to disable entire device.
Hmm. That rather destroys the main use case I'm aware of for this
(unlike the general santize commands from earlier CXL versions)/
Superficially sounds like we need a spec clarification as
clearly not super clear!
For this series, let's drive the work with the use case you have in
mind. We will start a thread with the consortium, but I don't think
that should delay this work.
>
Jonathan
Vinayak Holikatti
Re: [RFC v2 3/5] i386/kvm: Support event with select & umask format in KVM PMU filter
On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
> Date: Wed, 05 Feb 2025 11:07:10 +0100
> From: Markus Armbruster
> Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
> format in KVM PMU filter
>
> Zhao Liu writes:
>
> > The select&umask is the common way for x86 to identify the PMU event,
> > so support this way as the "x86-default" format in kvm-pmu-filter
> > object.
>
> So, format 'raw' lets you specify the PMU event code as a number, wheras
> 'x86-default' lets you specify it as select and umask, correct?
Yes!
> Why do we want both?
This 2 formats are both wildly used in x86(for example, in perf tool).
x86 documents usually specify the umask and select fields.
But raw format could also be applied for ARM since ARM just uses a number
to encode event.
> > Signed-off-by: Zhao Liu
>
> [...]
>
> > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > index d51aeeba7cd8..93b869e3f90c 100644
> > --- a/qapi/kvm.json
> > +++ b/qapi/kvm.json
> > @@ -27,11 +27,13 @@
> > #
> > # @raw: the encoded event code that KVM can directly consume.
> > #
> > +# @x86-default: standard x86 encoding format with select and umask.
>
> Why is this named -default?
Intel and AMD both use umask+select to encode events, but this format
doesn't have a name... so I call it `default`, or what about
"x86-umask-select"?
> > +#
> > # Since 10.0
> > ##
> > { 'enum': 'KVMPMUEventEncodeFmt',
> >'prefix': 'KVM_PMU_EVENT_FMT',
> > - 'data': ['raw'] }
> > + 'data': ['raw', 'x86-default'] }
> >
> > ##
> > # @KVMPMURawEvent:
> > @@ -46,6 +48,25 @@
> > { 'struct': 'KVMPMURawEvent',
> >'data': { 'code': 'uint64' } }
> >
> > +##
> > +# @KVMPMUX86DefalutEvent:
>
> Default, I suppose.
Thanks!
> > +#
> > +# x86 PMU event encoding with select and umask.
> > +# raw_event = ((select & 0xf00UL) << 24) | \
> > +# (select) & 0xff) | \
> > +# ((umask) & 0xff) << 8)
>
> Sphinx rejects this with "Unexpected indentation."
>
> Is the formula needed here?
I tried to explain the relationship between raw format and umask+select.
Emm, where do you think is the right place to put the document like
this?
...
> > +##
> > +# @KVMPMUX86DefalutEventVariant:
> > +#
> > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> > +# the numeric value.
> > +#
> > +# @select: x86 PMU event select field. This field is a 12-bit
> > +# unsigned number string.
> > +#
> > +# @umask: x86 PMU event umask field. This field is a uint8 string.
>
> Why are these strings? How are they parsed into numbers?
In practice, the values associated with PMU events (code for arm, select&
umask for x86) are often expressed in hexadecimal. Further, from linux
perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
s390 uses decimal value.
Therefore, it is necessary to support hexadecimal in order to honor PMU
conventions.
However, unfortunately, standard JSON (RFC 8259) does not support
hexadecimal numbers. So I can only consider using the numeric string in
the QAPI and then parsing it to a number.
I parse this string into number by qemu_strtou64().
Re: [PATCH 5/5] tests/functional: skip mem addr test on 32-bit hosts
On 05/02/2025 19.25, Richard Henderson wrote: On 2/5/25 08:53, Daniel P. Berrangé wrote: +Decorator to skip execution of a test on 32-bit targets +Example: + + @skipIf32BitTarget() +''' +def skipIf32BitTarget(): + enoughBits = sys.maxsize > 2**32 + return skipUnless(enoughBits, + 'Test requires a host with 64-bit address space') skipIf32BitHost? I don't mind either way. Definitely host. +1 for host. Otherwise this gets confused with "qemu-system-i386" etc. Thomas
Re: [PATCH 5/5] tests/functional: skip mem addr test on 32-bit hosts
On Wed, Feb 05, 2025 at 10:24:08AM -0800, Richard Henderson wrote: > On 2/5/25 07:59, Daniel P. Berrangé wrote: > > + > > +''' > > +Decorator to skip execution of a test on 32-bit targets > > +Example: > > + > > + @skipIf32BitTarget() > > +''' > > +def skipIf32BitTarget(): > > +enoughBits = sys.maxsize > 2**32 > > This will work for true 32-bit hosts, and possibly for containers running > emulation, but it won't work for cross-compilation (x86_64 to i686 or > aarch64 to arm). > > Perhaps "file qemu-system-foo" | grep "ELF 32-bit" ? > I don't know that we've actually selected the executable at this point > though... The QEMU_TEST_QEMU_BINARY env variable exists at all times, though grepping for ELF format feels a bit icky. We also know the location of the build directory, so was wondering if anything there tells us whether the host target is 64-bit, but it appears not be the case. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v7 RESEND 1/5] hw/core/machine: Reject thread level cache
On 10/1/25 15:51, Zhao Liu wrote: Currently, neither i386 nor ARM have real hardware support for per- thread cache, and there is no clear demand for this specific cache topology. Additionally, since ARM even can't support this special cache topology in device tree, it is unnecessary to support it at this moment, even though per-thread cache might have potential scheduling benefits for VMs without CPU affinity. Therefore, disable thread-level cache topology in the general machine part. At present, i386 has not enabled SMP cache, so disabling the thread parameter does not pose compatibility issues. In the future, if there is a clear demand for this feature, the correct approach would be to add a new control field in MachineClass.smp_props and enable it only for the machines that require it. Signed-off-by: Zhao Liu --- Changes since Patch v6: * New commit to reject "thread" parameter when parse smp-cache. --- hw/core/machine-smp.c | 7 +++ 1 file changed, 7 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 2/4] qdev-properties: Accept bool for OnOffAuto
Daniel P. Berrangé writes:
> On Wed, Jan 08, 2025 at 03:17:51PM +0900, Akihiko Odaki wrote:
>> Accept bool literals for OnOffAuto properties for consistency with bool
>> properties. This enables users to set the "on" or "off" value in a
>> uniform syntax without knowing whether the "auto" value is accepted.
>> This behavior is especially useful when converting an existing bool
>> property to OnOffAuto or vice versa.
>
> Again, to repeat my previous feedback, OnOffAuto is a well defined
> QAPI type - making it secretly accept other values/types behind
> the scenes which are not visible in QAPI scheme is not acceptable.
>
> Effectively this is a backdoor impl of a QAPI alternate
>
> { 'alternate': 'OnOffAutoOrBool',
> 'data': {
> 'o': 'OnOffAuto',
> 'b': 'bool'
> }
> }
>
> except this isn't permitted as the QAPI generator explicitly blocks
> use of alternate when the two branches are 'bool' and 'enum'.
>
> I'm assuming this is because in the QemuOpts scenario, it cannot
> guess upfront whether the input is a bool or enum. This is unfortunate
> though, because at the JSON visitor level it is unambiguous.
>
> I wonder if the QAPI generator could be relaxed in any viable way ?
Discussed in review of a related prior patch:
https://lore.kernel.org/qemu-devel/87h6c4fqz6@pond.sub.org/
Here's the relevant part for your convenience:
>> parse the value as enum, and if that fails, as uint32_t. QAPI already
>> provides a way to express "either this type or that type": alternate.
>> Like this:
>>
>> { 'alternate': 'OnOffAutoUint32',
>>'data': { 'sym': 'OnOffAuto',
>> 'uint': 'uint32' } }
>>
>> Unfortunately, such alternates don't work on the command line due to
>> keyval visitor restrictions. These cannot be lifted entirely, but we
>> might be able to lift them sufficiently to make this alternate work.
>
> The keyval visitor cannot implement alternates because the command line
> input does not have type information. For example, you cannot
> distinguish string "0" and integer 0.
Correct.
For alternate types, an input visitor picks the branch based on the
QType.
With JSON, we have scalar types null, number, string, and bool.
With keyval, we only have string. Alternates with more than one scalar
branch don't work.
They could be made to work to some degree, though. Observe:
* Any value can be a string.
* "frob" can be nothing else.
* "on" and "off" can also be bool.
* "123" and "1e3" can also be number or enum.
Instead of picking the branch based on the QType, we could pick based on
QType and value, where the value part is delegated to a visitor method.
Re: Call for GSoC internship project ideas
On Tue, Jan 28, 2025 at 11:16:43AM -0500, Stefan Hajnoczi wrote: > Dear QEMU and KVM communities, > QEMU will apply for the Google Summer of Code internship > program again this year. Regular contributors can submit project > ideas that they'd like to mentor by replying to this email by > February 7th. > > About Google Summer of Code > - > GSoC (https://summerofcode.withgoogle.com/) offers paid open > source remote work internships to eligible people wishing to participate > in open source development. QEMU has been doing internship for > many years. Our mentors have enjoyed helping talented interns make > their first open source contributions and some former interns continue > to participate today. > > Who can mentor > -- > Regular contributors to QEMU and KVM can participate as mentors. > Mentorship involves about 5 hours of time commitment per week to > communicate with the intern, review their patches, etc. Time is also > required during the intern selection phase to communicate with > applicants. Being a mentor is an opportunity to help someone get > started in open source development, will give you experience with > managing a project in a low-stakes environment, and a chance to > explore interesting technical ideas that you may not have time to > develop yourself. > > How to propose your idea > -- > Reply to this email with the following project idea template filled in: > > === TITLE === > > '''Summary:''' Short description of the project > > Detailed description of the project that explains the general idea, > including a list of high-level tasks that will be completed by the > project, and provides enough background for someone unfamiliar with > the code base to research the idea. Typically 2 or 3 paragraphs. > > '''Links:''' > * Links to mailing lists threads, git repos, or web sites > > '''Details:''' > * Skill level: beginner or intermediate or advanced > * Language: C/Python/Rust/etc > > More information > -- > You can find out about the process we follow here: > Video: https://www.youtube.com/watch?v=xNVCX7YMUL8 > Slides (PDF): https://vmsplice.net/~stefan/stefanha-kvm-forum-2016.pdf > > The QEMU wiki page for GSoC 2024 is now available: > https://wiki.qemu.org/Google_Summer_of_Code_2025 > > What about Outreachy? > --- > We have struggled to find sponsors for the Outreachy internship > program (https://www.outreachy.org/) in recent years. If you or your > organization would like to sponsor an Outreachy internship, please get > in touch. > > Thanks, > Stefan === Adding Kani proofs for Virtqueues in Rust-vmm === '''Summary:''' Verify conformance of the virtqueue implementation in rust-vmm to the VirtIO specification. In the rust-vmm project, devices rely on the virtqueue implementation provided by the `vm-virtio` crate. This implementation is based on the VirtIO specification, which defines the behavior and requirements for virtqueues. To ensure that the implementation meets these specifications, we have been relying on unit tests that check the output of the code given specific inputs. However, writing unit tests can be incomplete, as it's challenging to cover all possible scenarios and edge cases. During this internship, we propose a more comprehensive approach: using Kani proofs to verify that the virtqueue implementation conforms to the VirtIO specification. Kani allows us to write exhaustive checks for all possible values, going beyond what unit tests can achieve. By writing Kani proofs, we can confirm that our implementation meets the requirements of the VirtIO specification. If a proof passes, it provides strong evidence that the virtqueue implementation is correct and conformant. During the internship, we propose the following tasks: - Get familiar with Kani proofs written for Firecraker - Finish current PR that adds a proof for the notification suppression mechanism (see [2]) - Port add_used() proof (see [5]) - Port verify_prepare_kick() proof (see [6]) '''Links:''' * [1] Kani source code - https://github.com/model-checking/kani/ * [2] Notification suppression mechanism PR - https://github.com/rust-vmm/vm-virtio/pull/324 * [3] LPC Talk about how we may check conformance in the VirtIO specification - https://www.youtube.com/watch?v=w7BAR228344 * [4] FOSDEM'25 talk current effort to use Kani - https://fosdem.org/2025/schedule/event/fosdem-2025-5930-hunting-virtio-specification-violations/ * [5] https://github.com/firecracker-microvm/firecracker/blob/4bbbec06ee0d529add07807f75d923cc3d3cd210/src/vmm/src/devices/virtio/queue.rs#L1006 * [6] https://github.com/firecracker-microvm/firecracker/blob/4bbbec06ee0d529add07807f75d923cc3d3cd210/src/vmm/src/devices/virtio/queue.rs#L966 '''Details:''' * Skill level: intermediate * Language: Rust
Re: [PATCH 0/2] qemu/timer: Clarify QEMUTimer new/free API
Cc'ing more developers. On 4/2/25 22:44, Philippe Mathieu-Daudé wrote: ping? On 25/1/25 19:24, Philippe Mathieu-Daudé wrote: Update few QEMUTimer docstring and add a sanity check during timer initialization. Noticed trying to understand leaks in QDev Realize -> Unrealize -> Realize transition. Philippe Mathieu-Daudé (2): qemu/timer: Clarify timer_new*() must be freed with timer_free() qemu/timer: Sanity check timer_list in timer_init_full() include/qemu/timer.h | 12 +++- util/qemu-timer.c | 1 + 2 files changed, 12 insertions(+), 1 deletion(-)
Re: [PATCH 08/10] rust: qdev: switch from legacy reset to Resettable
On Fri, Jan 17, 2025 at 08:40:01PM +0100, Paolo Bonzini wrote: > Date: Fri, 17 Jan 2025 20:40:01 +0100 > From: Paolo Bonzini > Subject: [PATCH 08/10] rust: qdev: switch from legacy reset to Resettable > X-Mailer: git-send-email 2.47.1 > > Signed-off-by: Paolo Bonzini > --- > meson.build | 1 + > rust/hw/char/pl011/src/device.rs | 10 ++- > rust/qemu-api/src/qdev.rs| 116 --- > rust/qemu-api/tests/tests.rs | 5 +- > 4 files changed, 102 insertions(+), 30 deletions(-) > Add my R/b tag, Reviewed-by: Zhao Liu
Re: [PATCH v4 2/4] qdev-properties: Accept bool for OnOffAuto
Akihiko Odaki writes:
> On 2025/02/06 0:29, Markus Armbruster wrote:
>> Akihiko Odaki writes:
>>
>>> Accept bool literals for OnOffAuto properties for consistency with bool
>>> properties. This enables users to set the "on" or "off" value in a
>>> uniform syntax without knowing whether the "auto" value is accepted.
>>> This behavior is especially useful when converting an existing bool
>>> property to OnOffAuto or vice versa.
>>>
>>> Signed-off-by: Akihiko Odaki
>>> ---
>>> hw/core/qdev-properties.c | 17 -
>>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>>> index 434a76f5036e..0081d79f9b7b 100644
>>> --- a/hw/core/qdev-properties.c
>>> +++ b/hw/core/qdev-properties.c
>>> @@ -491,6 +491,21 @@ const PropertyInfo qdev_prop_string = {
>>> .set = set_string,
>>> };
>>>
>>> +static void set_on_off_auto(Object *obj, Visitor *v, const char *name,
>>> +void *opaque, Error **errp)
>>> +{
>>> +Property *prop = opaque;
>>> +int *ptr = object_field_prop_ptr(obj, prop);
>>> +bool value;
>>> +
>>> +if (visit_type_bool(v, name, &value, NULL)) {
>>> +*ptr = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
>>> +return;
>>> +}
>>> +
>>> +qdev_propinfo_set_enum(obj, v, name, opaque, errp);
>>> +}
>>> +
>>> /* --- on/off/auto --- */
>>>
>>> const PropertyInfo qdev_prop_on_off_auto = {
>>> @@ -498,7 +513,7 @@ const PropertyInfo qdev_prop_on_off_auto = {
>>> .description = "on/off/auto",
>>> .enum_table = &OnOffAuto_lookup,
>>> .get = qdev_propinfo_get_enum,
>>> -.set = qdev_propinfo_set_enum,
>>> +.set = set_on_off_auto,
>>> .set_default_value = qdev_propinfo_set_default_value_enum,
>>> };
>>
>> The qdev properties defined with DEFINE_PROP_ON_OFF_AUTO() now
>> additionally accept bool.
>>
>> The commit message tries to explain why this change is useful, but it
>> leaves me confused.
>>
>> Does this solve a problem with existing properties? If yes, what
>> exactly is the problem?
>>
>> Or does this enable new uses of DEFINE_PROP_ON_OFF_AUTO()?
>>
>> I'm trying to understand, but my gut feeling is "bad idea".
>>
>> Having multiple ways to express the same thing is generally undesirable.
>> In this case, "foo": "on" and "foo": true, as well as "foo": "off" and
>> "foo": false.
>>
>> Moreover, OnOffAuto then has two meanings: straightfoward enum as
>> defined in the QAPI schema, and the funny qdev property. This is
>> definitely a bad idea. DEFINE_PROP_T(), where T is some QAPI type,
>> should accept *exactly* the values of T. If these properties need to
>> accept something else, use another name to not invite confusion.
>>
>> If I understand the cover letter correctly, you want to make certain
>> bool properties tri-state for some reason. I haven't looked closely
>> enough to judge whether that makes sense. But do you really have to
>> change a whole bunch of unrelated properties to solve your problem?
>> This is going to be a very hard sell.
>>
>
> I change various virtio properties because they all have a common
> problem. The problem is, when the host does not support a virtio
> capability, virtio devices automatically set capability properties false
> even if the user explicitly sets them true.
I understand we have something like this:
* true: on if possible, else off
* false: off (always possible)
Which one is the default?
There is no way to reliably configure "on", i.e. fail if it's not
possible. I agree that's a problem.
> This problem can be solved
> using an existing mechanism, OnOffAuto, which differentiates the "auto"
> state and explicit the "on" state.
I guess you're proposing something like this:
* auto: on if possible, else off
* on: on if possible, else error
* off: off (always possible)
Which one is the default?
> However, converting bool to OnOffAuto surfaces another problem: they
> disagree how "on" and "off" should be written. Please note that the
> disagreement already exists and so it is nice to solve anyway.
Yes, converting bool to OnOffAuto is an incompatible change.
> This patch tries to solve it by tolerating bool values for OnOffAuto. As
> you pointed out, this approach has a downside: it makes OnOffAuto more
> complicated by having multiple ways to express the same thing.
It also affects existing uses of OnOffAuto, where such a change is
unnecessary and undesirable.
> Another approach is to have one unified way to express "on"/"off" for
> bool and OnOffAuto. This will give three options in total:
>
> 1. Let OnOffAuto accept JSON bool and "on"/"off" (what this patch does)
The parenthesis is inaccurate. This patch only affects qdev properties.
It does not affect use of OnOffAuto elsewhere, e.g. QOM object
"sev-guest" property "legacy-vm-type", or QMP command blockdev-add
argument "locking" with driver "f
Re: [RFC v2 3/5] i386/kvm: Support event with select & umask format in KVM PMU filter
On Thu, Feb 06, 2025 at 05:54:32PM +0800, Zhao Liu wrote:
> On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
> > Date: Wed, 05 Feb 2025 11:07:10 +0100
> > From: Markus Armbruster
> > Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
> > format in KVM PMU filter
> >
> > Zhao Liu writes:
> >
> > > The select&umask is the common way for x86 to identify the PMU event,
> > > so support this way as the "x86-default" format in kvm-pmu-filter
> > > object.
> >
> > So, format 'raw' lets you specify the PMU event code as a number, wheras
> > 'x86-default' lets you specify it as select and umask, correct?
>
> Yes!
>
> > Why do we want both?
>
> This 2 formats are both wildly used in x86(for example, in perf tool).
>
> x86 documents usually specify the umask and select fields.
>
> But raw format could also be applied for ARM since ARM just uses a number
> to encode event.
>
> > > Signed-off-by: Zhao Liu
> >
> > [...]
> >
> > > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > > index d51aeeba7cd8..93b869e3f90c 100644
> > > --- a/qapi/kvm.json
> > > +++ b/qapi/kvm.json
> > > @@ -27,11 +27,13 @@
> > > #
> > > # @raw: the encoded event code that KVM can directly consume.
> > > #
> > > +# @x86-default: standard x86 encoding format with select and umask.
> >
> > Why is this named -default?
>
> Intel and AMD both use umask+select to encode events, but this format
> doesn't have a name... so I call it `default`, or what about
> "x86-umask-select"?
>
> > > +#
> > > # Since 10.0
> > > ##
> > > { 'enum': 'KVMPMUEventEncodeFmt',
> > >'prefix': 'KVM_PMU_EVENT_FMT',
> > > - 'data': ['raw'] }
> > > + 'data': ['raw', 'x86-default'] }
> > >
> > > ##
> > > # @KVMPMURawEvent:
> > > @@ -46,6 +48,25 @@
> > > { 'struct': 'KVMPMURawEvent',
> > >'data': { 'code': 'uint64' } }
> > >
> > > +##
> > > +# @KVMPMUX86DefalutEvent:
> >
> > Default, I suppose.
>
> Thanks!
>
> > > +#
> > > +# x86 PMU event encoding with select and umask.
> > > +# raw_event = ((select & 0xf00UL) << 24) | \
> > > +# (select) & 0xff) | \
> > > +# ((umask) & 0xff) << 8)
> >
> > Sphinx rejects this with "Unexpected indentation."
> >
> > Is the formula needed here?
>
> I tried to explain the relationship between raw format and umask+select.
>
> Emm, where do you think is the right place to put the document like
> this?
>
> ...
>
> > > +##
> > > +# @KVMPMUX86DefalutEventVariant:
Typo s/Defalut/Default/ - repeated many times in this patch.
> > > +#
> > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> > > +# the numeric value.
> > > +#
> > > +# @select: x86 PMU event select field. This field is a 12-bit
> > > +# unsigned number string.
> > > +#
> > > +# @umask: x86 PMU event umask field. This field is a uint8 string.
> >
> > Why are these strings? How are they parsed into numbers?
>
> In practice, the values associated with PMU events (code for arm, select&
> umask for x86) are often expressed in hexadecimal. Further, from linux
> perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
> arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
> s390 uses decimal value.
>
> Therefore, it is necessary to support hexadecimal in order to honor PMU
> conventions.
IMHO having a data format that matches an arbitrary external tool is not
a goal for QMP. It should be neutral and exclusively use the normal JSON
encoding, ie base-10 decimal. Yes, this means a user/client may have to
convert from hex to dec before sending data over QMP. This is true of
many areas of QMP/QEMU config though and thus normal/expected behaviour.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v4 09/16] target/microblaze: Introduce mo_endian() helper
mo_endian() returns the target endianness, currently static.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/microblaze/translate.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 0d51b2c468c..b5389d65b2e 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -708,12 +708,17 @@ static void record_unaligned_ess(DisasContext *dc, int rd,
}
#endif
+static inline MemOp mo_endian(DisasContext *dc)
+{
+return MO_TE;
+}
+
static bool do_load(DisasContext *dc, int rd, TCGv addr, MemOp mop,
int mem_index, bool rev)
{
MemOp size = mop & MO_SIZE;
-mop |= MO_TE;
+mop |= mo_endian(dc);
/*
* When doing reverse accesses we need to do two things.
@@ -848,7 +853,8 @@ static bool trans_lwx(DisasContext *dc, arg_typea *arg)
/* lwx does not throw unaligned access errors, so force alignment */
tcg_gen_andi_tl(addr, addr, ~3);
-tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index, MO_TE | MO_UL);
+tcg_gen_qemu_ld_i32(cpu_res_val, addr, dc->mem_index,
+mo_endian(dc) | MO_UL);
tcg_gen_mov_tl(cpu_res_addr, addr);
if (arg->rd) {
@@ -865,7 +871,7 @@ static bool do_store(DisasContext *dc, int rd, TCGv addr,
MemOp mop,
{
MemOp size = mop & MO_SIZE;
-mop |= MO_TE;
+mop |= mo_endian(dc);
/*
* When doing reverse accesses we need to do two things.
@@ -1019,7 +1025,7 @@ static bool trans_swx(DisasContext *dc, arg_typea *arg)
tcg_gen_atomic_cmpxchg_i32(tval, cpu_res_addr, cpu_res_val,
reg_for_write(dc, arg->rd),
- dc->mem_index, MO_TE | MO_UL);
+ dc->mem_index, mo_endian(dc) | MO_UL);
tcg_gen_brcond_i32(TCG_COND_NE, cpu_res_val, tval, swx_fail);
--
2.47.1
Re: [PATCH 0/2] hw/char/sh_serial: QOM housekeeping
On 24/1/25 18:50, Philippe Mathieu-Daudé wrote: - Parity in realize / unrealize - Define TypeInfo structure Philippe Mathieu-Daudé (2): hw/char/sh_serial: Delete fifo_timeout_timer in DeviceUnrealize hw/char/sh_serial: Convert to TypeInfo hw/char/sh_serial.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) ping?
Re: [PATCH 1/2] rust: remove unnecessary Cargo.toml metadata
On Wed, Jan 29, 2025 at 09:37:03AM +0100, Paolo Bonzini wrote: > Date: Wed, 29 Jan 2025 09:37:03 +0100 > From: Paolo Bonzini > Subject: [PATCH 1/2] rust: remove unnecessary Cargo.toml metadata > X-Mailer: git-send-email 2.48.1 > > Some items of Cargo.toml (readme, homepage, repository) are > only present because of clippy::cargo warnings being enabled in > rust/hw/char/pl011/src/lib.rs. But these items are not > particularly useful and would be all the same for all Cargo.toml > files in the QEMU workspace. Clean them up. > > Signed-off-by: Paolo Bonzini > --- > rust/hw/char/pl011/Cargo.toml | 3 --- > rust/hw/char/pl011/README.md| 31 --- > rust/hw/char/pl011/src/lib.rs | 14 ++ > rust/qemu-api-macros/Cargo.toml | 3 --- > rust/qemu-api-macros/README.md | 1 - > 5 files changed, 6 insertions(+), 46 deletions(-) > delete mode 100644 rust/hw/char/pl011/README.md > delete mode 100644 rust/qemu-api-macros/README.md > Reviewed-by: Zhao Liu
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Zhao Liu writes:
> On Wed, Feb 05, 2025 at 11:03:51AM +0100, Markus Armbruster wrote:
>> Date: Wed, 05 Feb 2025 11:03:51 +0100
>> From: Markus Armbruster
>> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>>
>> Quick & superficial review for now.
>
> Thanks!
>
>> > diff --git a/qapi/kvm.json b/qapi/kvm.json
>> > new file mode 100644
>> > index ..d51aeeba7cd8
>> > --- /dev/null
>> > +++ b/qapi/kvm.json
>> > @@ -0,0 +1,116 @@
>> > +# -*- Mode: Python -*-
>> > +# vim: filetype=python
>> > +
>> > +##
>> > +# = KVM based feature API
>>
>> This is a top-level section. It ends up between sections "QMP
>> introspection" and "QEMU Object Model (QOM)". Is this what we want? Or
>> should it be a sub-section of something? Or next to something else?
>
> Do you mean it's not in the right place in the qapi-schema.json?
>
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index b1581988e4eb..742818d16e45 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -64,6 +64,7 @@
> { 'include': 'compat.json' }
> { 'include': 'control.json' }
> { 'include': 'introspect.json' }
> +{ 'include': 'kvm.json' }
> { 'include': 'qom.json' }
> { 'include': 'qdev.json' }
> { 'include': 'machine-common.json' }
>
> Because qom.json includes kvm.json, so I have to place it before
> qom.json.
>
> It doesn't have any dependencies itself, so placing it in the previous
> position should be fine, where do you prefer?
Let's ignore how to place it for now, and focus on where we would *like*
to place it.
Is it related to anything other than ObjectType / ObjectOptions in the
QMP reference manual?
I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
KVM PMU filter. Should we have a section for accelerator-specific
stuff, with subsections for the various accelerators?
[...]
[PATCH 4/4] vfio/igd: sync GPU generation with i915 kernel driver
From: Corvin Köhne
We're currently missing some GPU IDs already supported by the i915
kernel driver. Additionally, we've treated IvyBridge as gen 6 in the
past. According to i915 it's gen 7 [1]. It shouldn't cause any issues
yet because we treat gen 6 and gen 7 the same way. Nevertheless, we
should use the correct generation to avoid any confusion.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/i915_pci.c?h=v6.13#n330
Signed-off-by: Corvin Köhne
---
hw/vfio/igd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e5d7006ce2..7bbf018efc 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -64,7 +64,7 @@ struct igd_device {
static const struct igd_device igd_devices[] = {
INTEL_SNB_IDS(IGD_DEVICE, 6),
-INTEL_IVB_IDS(IGD_DEVICE, 6),
+INTEL_IVB_IDS(IGD_DEVICE, 7),
INTEL_HSW_IDS(IGD_DEVICE, 7),
INTEL_VLV_IDS(IGD_DEVICE, 7),
INTEL_BDW_IDS(IGD_DEVICE, 8),
@@ -73,8 +73,10 @@ static const struct igd_device igd_devices[] = {
INTEL_BXT_IDS(IGD_DEVICE, 9),
INTEL_KBL_IDS(IGD_DEVICE, 9),
INTEL_CFL_IDS(IGD_DEVICE, 9),
+INTEL_WHL_IDS(IGD_DEVICE, 9),
INTEL_CML_IDS(IGD_DEVICE, 9),
INTEL_GLK_IDS(IGD_DEVICE, 9),
+INTEL_CNL_IDS(IGD_DEVICE, 9),
INTEL_ICL_IDS(IGD_DEVICE, 11),
INTEL_EHL_IDS(IGD_DEVICE, 11),
INTEL_JSL_IDS(IGD_DEVICE, 11),
@@ -86,6 +88,8 @@ static const struct igd_device igd_devices[] = {
INTEL_RPLS_IDS(IGD_DEVICE, 12),
INTEL_RPLU_IDS(IGD_DEVICE, 12),
INTEL_RPLP_IDS(IGD_DEVICE, 12),
+INTEL_ARL_IDS(IGD_DEVICE, 12),
+INTEL_MTL_IDS(IGD_DEVICE, 12),
};
/*
--
2.48.1
[PATCH 0/4] vfio/igd: sync PCI IDs with i915
From: Corvin Köhne Hi, we're currently maintaining an own list of PCI IDs to match the generation of Intels integrated graphic devices. Linux maintains a list too. It's list is more recent, contains the full PCI ID of all devices and ships some macros to easily match them. This patch series imports the PCI ID list from Linux and reuses it. Best regards, Corvin Corvin Köhne (4): include/standard-headers: add PCI IDs for Intel GPUs scripts/update-linux-headers: include PCI ID header for Intel GPUs vfio/igd: use PCI ID defines to detect IGD gen vfio/igd: sync GPU generation with i915 kernel driver hw/vfio/igd.c | 81 +- include/standard-headers/drm/intel/pciids.h | 834 scripts/update-linux-headers.sh | 6 + 3 files changed, 886 insertions(+), 35 deletions(-) create mode 100644 include/standard-headers/drm/intel/pciids.h -- 2.48.1
[PATCH 3/4] vfio/igd: use PCI ID defines to detect IGD gen
From: Corvin Köhne
We've recently imported the PCI ID list of knwon Intel GPU devices from
Linux. It allows us to properly match GPUs to their generation without
maintaining an own list of PCI IDs.
Signed-off-by: Corvin Köhne
---
hw/vfio/igd.c | 77 ---
1 file changed, 42 insertions(+), 35 deletions(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 0740a5dd8c..e5d7006ce2 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -18,6 +18,7 @@
#include "hw/hw.h"
#include "hw/nvram/fw_cfg.h"
#include "pci.h"
+#include "standard-headers/drm/intel/pciids.h"
#include "trace.h"
/*
@@ -51,6 +52,42 @@
* headless setup is desired, the OpRegion gets in the way of that.
*/
+struct igd_device {
+const uint32_t device_id;
+const int gen;
+};
+
+#define IGD_DEVICE(_id, _gen) { \
+.device_id = (_id), \
+.gen = (_gen), \
+}
+
+static const struct igd_device igd_devices[] = {
+INTEL_SNB_IDS(IGD_DEVICE, 6),
+INTEL_IVB_IDS(IGD_DEVICE, 6),
+INTEL_HSW_IDS(IGD_DEVICE, 7),
+INTEL_VLV_IDS(IGD_DEVICE, 7),
+INTEL_BDW_IDS(IGD_DEVICE, 8),
+INTEL_CHV_IDS(IGD_DEVICE, 8),
+INTEL_SKL_IDS(IGD_DEVICE, 9),
+INTEL_BXT_IDS(IGD_DEVICE, 9),
+INTEL_KBL_IDS(IGD_DEVICE, 9),
+INTEL_CFL_IDS(IGD_DEVICE, 9),
+INTEL_CML_IDS(IGD_DEVICE, 9),
+INTEL_GLK_IDS(IGD_DEVICE, 9),
+INTEL_ICL_IDS(IGD_DEVICE, 11),
+INTEL_EHL_IDS(IGD_DEVICE, 11),
+INTEL_JSL_IDS(IGD_DEVICE, 11),
+INTEL_TGL_IDS(IGD_DEVICE, 12),
+INTEL_RKL_IDS(IGD_DEVICE, 12),
+INTEL_ADLS_IDS(IGD_DEVICE, 12),
+INTEL_ADLP_IDS(IGD_DEVICE, 12),
+INTEL_ADLN_IDS(IGD_DEVICE, 12),
+INTEL_RPLS_IDS(IGD_DEVICE, 12),
+INTEL_RPLU_IDS(IGD_DEVICE, 12),
+INTEL_RPLP_IDS(IGD_DEVICE, 12),
+};
+
/*
* This presumes the device is already known to be an Intel VGA device, so we
* take liberties in which device ID bits match which generation. This should
@@ -60,42 +97,12 @@
*/
static int igd_gen(VFIOPCIDevice *vdev)
{
-/*
- * Device IDs for Broxton/Apollo Lake are 0x0a84, 0x1a84, 0x1a85, 0x5a84
- * and 0x5a85, match bit 11:1 here
- * Prefix 0x0a is taken by Haswell, this rule should be matched first.
- */
-if ((vdev->device_id & 0xffe) == 0xa84) {
-return 9;
-}
+for (int i = 0; i < ARRAY_SIZE(igd_devices); i++) {
+if (igd_devices[i].device_id != vdev->device_id) {
+continue;
+}
-switch (vdev->device_id & 0xff00) {
-case 0x0100:/* SandyBridge, IvyBridge */
-return 6;
-case 0x0400:/* Haswell */
-case 0x0a00:/* Haswell */
-case 0x0c00:/* Haswell */
-case 0x0d00:/* Haswell */
-case 0x0f00:/* Valleyview/Bay Trail */
-return 7;
-case 0x1600:/* Broadwell */
-case 0x2200:/* Cherryview */
-return 8;
-case 0x1900:/* Skylake */
-case 0x3100:/* Gemini Lake */
-case 0x5900:/* Kaby Lake */
-case 0x3e00:/* Coffee Lake */
-case 0x9B00:/* Comet Lake */
-return 9;
-case 0x8A00:/* Ice Lake */
-case 0x4500:/* Elkhart Lake */
-case 0x4E00:/* Jasper Lake */
-return 11;
-case 0x9A00:/* Tiger Lake */
-case 0x4C00:/* Rocket Lake */
-case 0x4600:/* Alder Lake */
-case 0xA700:/* Raptor Lake */
-return 12;
+return igd_devices[i].gen;
}
/*
--
2.48.1
[PATCH 2/4] scripts/update-linux-headers: include PCI ID header for Intel GPUs
From: Corvin Köhne We've recently imported the PCI ID header for Intel GPUs into our tree. Add it to our helper script to make it easier for us to sync this file in the future. Signed-off-by: Corvin Köhne --- scripts/update-linux-headers.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index 8913e4fb99..a4ff5a8fe9 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -269,6 +269,12 @@ mkdir -p "$output/include/standard-headers/drm" cp_portable "$hdrdir/include/drm/drm_fourcc.h" \ "$output/include/standard-headers/drm" +# Linux does not install the PCI IDs header for i915 devices, so we have to +# pick it up from the source tree itself. +mkdir -p "$output/include/standard-headers/drm/intel" +cp_portable "$linux/include/drm/intel/pciids.h" \ +"$output/include/standard-headers/drm/intel" + cat <$output/include/standard-headers/linux/types.h /* For QEMU all types are already defined via osdep.h, so this * header does not need to do anything. -- 2.48.1
Re: [PATCH v2] hw/i386/amd_iommu: Allow migration
On Thu, Feb 06, 2025 at 05:18:56AM +, Suravee Suthikulpanit wrote:
> Add migration support for AMD IOMMU model by saving necessary AMDVIState
> parameters for MMIO registers, device table, command buffer, and event
> buffers.
>
> Signed-off-by: Suravee Suthikulpanit
> ---
> Changes from v1:
> (https://lore.kernel.org/all/9ecffa7a-f4c6-45a5-a066-84826ccb5...@amd.com/T/)
> * Include ppr_log, pprlog_len, pprlog_head, pprlog_tail per Joao.
>
> hw/i386/amd_iommu.c | 43 ++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 13af7211e1..a1940a0ab3 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1673,7 +1673,48 @@ static Property amdvi_properties[] = {
>
> static const VMStateDescription vmstate_amdvi_sysbus = {
> .name = "amd-iommu",
> -.unmigratable = 1
> +.version_id = 1,
IMHO we should not remove the 'unmigratable = 1' setting, until we
have fixed the design problem with this device, to split the creation
of the 'AMDVI-PCI' device off from the 'amd-iommu' device, so that the
former is user creatable.
As it stands now, there's no mechanism to guarantee that the internally
created 'AMDVI-PCI' device gets the same PCI address properties on every
boot. Thus it isn't safe to claim this device is migratable yet IMHO.
> +.minimum_version_id = 1,
> +.priority = MIG_PRI_IOMMU,
> +.fields = (VMStateField[]) {
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v7 RESEND 0/5] i386: Support SMP Cache Topology
Hi Paolo,
A kindly ping. (I dropped the cache per thread; do you think this version
is ok?)
Thanks,
Zhao
On Fri, Jan 10, 2025 at 10:51:10PM +0800, Zhao Liu wrote:
> Date: Fri, 10 Jan 2025 22:51:10 +0800
> From: Zhao Liu
> Subject: [PATCH v7 RESEND 0/5] i386: Support SMP Cache Topology
> X-Mailer: git-send-email 2.34.1
>
> Hi folks,
>
> This is my v7 resend version (updated the commit message of origin
> v7's Patch 1).
>
> Compared with v6 [1], v7 dropped the "thread" level cache topology
> (cache per thread):
>
> - Patch 1 is the new patch to reject "thread" parameter for smp-cache.
> - Ptach 2 dropped cache per thread support.
> (Others remain unchanged.)
>
> There're several reasons:
>
> * Currently, neither i386 nor ARM have real hardware support for per-
>thread cache.
> * ARM can't support thread level cache in device tree. [2].
>
> So it is unnecessary to support it at this moment, even though per-
> thread cache might have potential scheduling benefits for VMs without
> CPU affinity.
>
> In the future, if there is a clear demand for this feature, the correct
> approach would be to add a new control field in MachineClass.smp_props
> and enable it only for the machines that require it.
>
>
> This series is based on the master branch at commit aa3a285b5bc5 ("Merge
> tag 'mem-2024-12-21' of https://github.com/davidhildenbrand/qemu into
> staging").
>
> Smp-cache support of ARM side can be found at [3].
>
>
> Background
> ==
>
> The x86 and ARM (RISCV) need to allow user to configure cache properties
> (current only topology):
> * For x86, the default cache topology model (of max/host CPU) does not
>always match the Host's real physical cache topology. Performance can
>increase when the configured virtual topology is closer to the
>physical topology than a default topology would be.
> * For ARM, QEMU can't get the cache topology information from the CPU
>registers, then user configuration is necessary. Additionally, the
>cache information is also needed for MPAM emulation (for TCG) to
>build the right PPTT. (Originally from Jonathan)
>
>
> About smp-cache
> ===
>
> The API design has been discussed heavily in [4].
>
> Now, smp-cache is implemented as a array integrated in -machine. Though
> -machine currently can't support JSON format, this is the one of the
> directions of future.
>
> An example is as follows:
>
> smp_cache=smp-cache.0.cache=l1i,smp-cache.0.topology=core,smp-cache.1.cache=l1d,smp-cache.1.topology=core,smp-cache.2.cache=l2,smp-cache.2.topology=module,smp-cache.3.cache=l3,smp-cache.3.topology=die
>
> "cache" specifies the cache that the properties will be applied on. This
> field is the combination of cache level and cache type. Now it supports
> "l1d" (L1 data cache), "l1i" (L1 instruction cache), "l2" (L2 unified
> cache) and "l3" (L3 unified cache).
>
> "topology" field accepts CPU topology levels including "core", "module",
> "cluster", "die", "socket", "book", "drawer" and a special value
> "default". (Note, now, in v7, smp-cache doesn't support "thread".)
>
> The "default" is introduced to make it easier for libvirt to set a
> default parameter value without having to care about the specific
> machine (because currently there is no proper way for machine to
> expose supported topology levels and caches).
>
> If "default" is set, then the cache topology will follow the
> architecture's default cache topology model. If other CPU topology level
> is set, the cache will be shared at corresponding CPU topology level.
>
> [1]: Patch v6:
> https://lore.kernel.org/qemu-devel/20241219083237.265419-1-zhao1@intel.com/
> [2]: Gap of cache per thread for ARM:
> https://lore.kernel.org/qemu-devel/20250110114100.2...@huawei.com/T/#m50c37fa5d372feac8e607c279cd446da3e22a12c
> [3]: ARM smp-cache:
> https://lore.kernel.org/qemu-devel/20250102152012.1049-1-alireza.san...@huawei.com/
> [4]: API disscussion:
> https://lore.kernel.org/qemu-devel/8734ndj33j@pond.sub.org/
>
> Thanks and Best Regards,
> Zhao
> ---
> Alireza Sanaee (1):
> i386/cpu: add has_caches flag to check smp_cache configuration
>
> Zhao Liu (4):
> hw/core/machine: Reject thread level cache
> i386/cpu: Support module level cache topology
> i386/cpu: Update cache topology with machine's configuration
> i386/pc: Support cache topology in -machine for PC machine
>
> hw/core/machine-smp.c | 9 ++
> hw/i386/pc.c | 4 +++
> include/hw/boards.h | 3 ++
> qemu-options.hx | 30 +-
> target/i386/cpu.c | 71 ++-
> 5 files changed, 115 insertions(+), 2 deletions(-)
>
> --
> 2.34.1
>
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
On 6/2/25 03:34, bibo mao wrote:
On 2025/2/5 下午8:06, Xianglai Li wrote:
When the KVM_REG_LOONGARCH_VCPU_RESET command word
is sent to the kernel through the kvm_set_one_reg interface,
the parameter source needs to be a legal address,
otherwise the kernel will return an error and the command word
will fail to be sent.
Hi Xianglai,
Good catch, it is actually one problem and thanks for reporting it.
Signed-off-by: Xianglai Li
---
Cc: Bibo Mao
Cc: Song Gao
target/loongarch/kvm/kvm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index a3f55155b0..01cddb7012 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
void kvm_arch_reset_vcpu(CPUState *cs)
{
CPULoongArchState *env = cpu_env(cs);
+ uint64_t val;
How about set initial value here although it is not used? such as
uint64_t val = 0;
Oruint64_t unused = 0;
env->mp_state = KVM_MP_STATE_RUNNABLE;
- kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
+ kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
Can we add return value checking here? such as
ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
if (ret) {
error_report("... %s", strerror(errno));
}
Regards
Bibo Mao
}
static int kvm_loongarch_get_mpstate(CPUState *cs)
Re: [PATCH 1/5] tests/functional: skip test if QEMU_TEST_QEMU_BINARY is not set
On 05/02/2025 16.59, Daniel P. Berrangé wrote:
If QEMU_TEST_QEMU_BINARY is not set we currently assert in the setUp
function, resulting in a big traceback:
TAP version 13
Traceback (most recent call last):
File
"/var/home/berrange/src/virt/qemu/tests/functional/qemu_test/testcase.py", line
280, in setUp
super().setUp('qemu-system-')
~
File
"/var/home/berrange/src/virt/qemu/tests/functional/qemu_test/testcase.py", line
196, in setUp
self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be
set')
AssertionError: unexpectedly None : QEMU_TEST_QEMU_BINARY must be set
not ok 1 test_ppc_405.Ppc405Machine.test_ppc_ref405ep
1..1
For every other test pre-requisite that's missing we will mark the test
as skipped. This does the same for missing QEMU_TEST_QEMU_BINARY, such
that we get
TAP version 13
ok 1 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest # SKIP
QEMU_TEST_QEMU_BINARY env variable is not set
ok 2 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_noapic_nomsi #
SKIP QEMU_TEST_QEMU_BINARY env variable is not set
ok 3 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_nomsi # SKIP
QEMU_TEST_QEMU_BINARY env variable is not set
ok 4 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_novector # SKIP
QEMU_TEST_QEMU_BINARY env variable is not set
ok 5 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_novector_noapic #
SKIP QEMU_TEST_QEMU_BINARY env variable is not set
ok 6 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_novector_nomsi #
SKIP QEMU_TEST_QEMU_BINARY env variable is not set
ok 7 test_x86_64_kvm_xen.KVMXenGuest.test_kvm_xen_guest_vapic # SKIP
QEMU_TEST_QEMU_BINARY env variable is not set
1..7
Not sure whether this is the right approach, since a missing
QEMU_TEST_QEMU_BINARY is a real error, and if we just skip, then the problem
might go unnoticed if the user does not look closely.
But to ease the situation: We could maybe add some auto-detection logic that
tries to guess the right qemu-system-$TARGET by looking at the file name of
the test and/or the test function name? We already encode the target
architecture in most of these... WDYT?
Thomas
Re: [PATCH v3 03/17] hw/ssi: Make flash size a property in NPCM7XX FIU
Hi Hao,
On 6/2/25 02:30, Hao Wu wrote:
This allows different FIUs to have different flash sizes, useful
in NPCM8XX which has multiple different sized FIU modules.
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/arm/npcm7xx.c | 6 ++
hw/ssi/npcm7xx_fiu.c | 11 +++
include/hw/ssi/npcm7xx_fiu.h | 1 +
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 386b2c35e9..2d6e08b72b 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -292,17 +292,21 @@ static const struct {
hwaddr regs_addr;
int cs_count;
const hwaddr *flash_addr;
+size_t flash_size;
} npcm7xx_fiu[] = {
{
.name = "fiu0",
.regs_addr = 0xfb00,
.cs_count = ARRAY_SIZE(npcm7xx_fiu0_flash_addr),
.flash_addr = npcm7xx_fiu0_flash_addr,
+.flash_size = 128 * MiB,
+
}, {
.name = "fiu3",
.regs_addr = 0xc000,
.cs_count = ARRAY_SIZE(npcm7xx_fiu3_flash_addr),
.flash_addr = npcm7xx_fiu3_flash_addr,
+.flash_size = 128 * MiB,
},
};
@@ -735,6 +739,8 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(sbd), "cs-count",
npcm7xx_fiu[i].cs_count, &error_abort);
+object_property_set_int(OBJECT(sbd), "flash-size",
+npcm7xx_fiu[i].flash_size, &error_abort);
sysbus_realize(sbd, &error_abort);
sysbus_mmio_map(sbd, 0, npcm7xx_fiu[i].regs_addr);
diff --git a/hw/ssi/npcm7xx_fiu.c b/hw/ssi/npcm7xx_fiu.c
index 21fc489038..ccdce67fa9 100644
--- a/hw/ssi/npcm7xx_fiu.c
+++ b/hw/ssi/npcm7xx_fiu.c
@@ -28,9 +28,6 @@
#include "trace.h"
-/* Up to 128 MiB of flash may be accessed directly as memory. */
-#define NPCM7XX_FIU_FLASH_WINDOW_SIZE (128 * MiB)
-
/* Each module has 4 KiB of register space. Only a fraction of it is used. */
#define NPCM7XX_FIU_CTRL_REGS_SIZE (4 * KiB)
@@ -507,6 +504,11 @@ static void npcm7xx_fiu_realize(DeviceState *dev, Error **errp)
return;
}
+if (s->flash_size == 0) {
+error_setg(errp, "%s: flash size must be set", dev->canonical_path);
+return;
+}
+
s->spi = ssi_create_bus(dev, "spi");
s->cs_lines = g_new0(qemu_irq, s->cs_count);
qdev_init_gpio_out_named(DEVICE(s), s->cs_lines, "cs", s->cs_count);
@@ -525,7 +527,7 @@ static void npcm7xx_fiu_realize(DeviceState *dev, Error
**errp)
flash->fiu = s;
memory_region_init_io(&flash->direct_access, OBJECT(s),
&npcm7xx_fiu_flash_ops, &s->flash[i], "flash",
- NPCM7XX_FIU_FLASH_WINDOW_SIZE);
+ s->flash_size);
Per the comment, this is the device aperture.
Either add a check whether size is always <= 128 * MiB, or use
MIN(s->flash_size, NPCM7XX_FIU_FLASH_WINDOW_SIZE)?
sysbus_init_mmio(sbd, &flash->direct_access);
}
}
@@ -543,6 +545,7 @@ static const VMStateDescription vmstate_npcm7xx_fiu = {
static const Property npcm7xx_fiu_properties[] = {
DEFINE_PROP_INT32("cs-count", NPCM7xxFIUState, cs_count, 0),
+DEFINE_PROP_SIZE("flash-size", NPCM7xxFIUState, flash_size, 0),
};
static void npcm7xx_fiu_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/ssi/npcm7xx_fiu.h b/include/hw/ssi/npcm7xx_fiu.h
index a3a1704289..1785ea16f4 100644
--- a/include/hw/ssi/npcm7xx_fiu.h
+++ b/include/hw/ssi/npcm7xx_fiu.h
@@ -60,6 +60,7 @@ struct NPCM7xxFIUState {
int32_t cs_count;
int32_t active_cs;
qemu_irq *cs_lines;
+size_t flash_size;
NPCM7xxFIUFlash *flash;
SSIBus *spi;
Re: [PATCH v3 09/17] hw/misc: Support 8-bytes memop in NPCM GCR module
On 6/2/25 02:30, Hao Wu wrote:
The NPCM8xx GCR device can be accessed with 64-bit memory operations.
This patch supports that.
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/misc/npcm_gcr.c | 94 +---
hw/misc/trace-events | 4 +-
2 files changed, 74 insertions(+), 24 deletions(-)
diff --git a/hw/misc/npcm_gcr.c b/hw/misc/npcm_gcr.c
index 820b730606..654e048048 100644
--- a/hw/misc/npcm_gcr.c
+++ b/hw/misc/npcm_gcr.c
@@ -201,6 +201,7 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr offset,
unsigned size)
uint32_t reg = offset / sizeof(uint32_t);
NPCMGCRState *s = opaque;
NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
+uint64_t value;
if (reg >= c->nr_regs) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -209,9 +210,21 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr offset,
unsigned size)
return 0;
}
-trace_npcm_gcr_read(offset, s->regs[reg]);
+switch (size) {
+case 4:
+value = s->regs[reg];
+break;
+
+case 8:
Should we assert(!(reg & 1)) in case someone want to enable unaligned
accesses?
+value = deposit64(s->regs[reg], 32, 32, s->regs[reg + 1]);
+break;
+
+default:
+g_assert_not_reached();
+}
-return s->regs[reg];
+trace_npcm_gcr_read(offset, value);
+return value;
}
static void npcm_gcr_write(void *opaque, hwaddr offset,
@@ -231,29 +244,65 @@ static void npcm_gcr_write(void *opaque, hwaddr offset,
return;
}
-switch (reg) {
-case NPCM7XX_GCR_PDID:
-case NPCM7XX_GCR_PWRON:
-case NPCM7XX_GCR_INTSR:
-qemu_log_mask(LOG_GUEST_ERROR,
- "%s: register @ 0x%04" HWADDR_PRIx " is read-only\n",
- __func__, offset);
-return;
-
-case NPCM7XX_GCR_RESSR:
-case NPCM7XX_GCR_CP2BST:
-/* Write 1 to clear */
-value = s->regs[reg] & ~value;
+switch (size) {
+case 4:
+switch (reg) {
+case NPCM7XX_GCR_PDID:
+case NPCM7XX_GCR_PWRON:
+case NPCM7XX_GCR_INTSR:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: register @ 0x%04" HWADDR_PRIx " is read-only\n",
+ __func__, offset);
+return;
+
+case NPCM7XX_GCR_RESSR:
+case NPCM7XX_GCR_CP2BST:
+/* Write 1 to clear */
+value = s->regs[reg] & ~value;
+break;
+
+case NPCM7XX_GCR_RLOCKR1:
+case NPCM7XX_GCR_MDLR:
+/* Write 1 to set */
+value |= s->regs[reg];
+break;
+};
+s->regs[reg] = value;
break;
-case NPCM7XX_GCR_RLOCKR1:
-case NPCM7XX_GCR_MDLR:
-/* Write 1 to set */
-value |= s->regs[reg];
+case 8:
Ditto.
+s->regs[reg] = value;
+s->regs[reg + 1] = extract64(v, 32, 32);
break;
-};
-s->regs[reg] = value;
+default:
+g_assert_not_reached();
+}
+}
+
+static bool npcm_gcr_check_mem_op(void *opaque, hwaddr offset,
+ unsigned size, bool is_write,
+ MemTxAttrs attrs)
+{
+NPCMGCRClass *c = NPCM_GCR_GET_CLASS(opaque);
+
+if (offset >= c->nr_regs * sizeof(uint32_t)) {
+return false;
+}
+
+switch (size) {
+case 4:
+return true;
+case 8:
+if (offset >= NPCM8XX_GCR_SCRPAD_00 * sizeof(uint32_t) &&
+offset < (NPCM8XX_GCR_NR_REGS - 1) * sizeof(uint32_t)) {
+return true;
+} else {
+return false;
+}
+default:
+return false;
+}
}
static const struct MemoryRegionOps npcm_gcr_ops = {
@@ -262,7 +311,8 @@ static const struct MemoryRegionOps npcm_gcr_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.valid = {
.min_access_size= 4,
-.max_access_size= 4,
+.max_access_size= 8,
+.accepts= npcm_gcr_check_mem_op,
.unaligned = false,
},
};
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 0f7204a237..f25dbd6030 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -135,8 +135,8 @@ npcm7xx_clk_read(uint64_t offset, uint32_t value) " offset:
0x%04" PRIx64 " valu
npcm7xx_clk_write(uint64_t offset, uint32_t value) "offset: 0x%04" PRIx64 " value:
0x%08" PRIx32
# npcm_gcr.c
-npcm_gcr_read(uint64_t offset, uint32_t value) " offset: 0x%04" PRIx64 " value:
0x%08" PRIx32
-npcm_gcr_write(uint64_t offset, uint32_t value) "offset: 0x%04" PRIx64 " value:
0x%08" PRIx32
+npcm_gcr_read(uint64_t offset, uint64_t value) " offset: 0x%04" PRIx64 " value:
0x%08" PRIx64
+npcm_gcr_write(uint64_t offset, uint64_t value) "offset: 0x%04" PRIx64 " value:
0x%08" PRIx64
# npcm7xx_mft.c
npcm7xx_mft_read(const char *name, uint64_t offset, uint16_t value) "%s: offset: 0
Re: [PATCH 2/5] tests/functional: remove unused 'bin_prefix' variable
On 05/02/2025 16.59, Daniel P. Berrangé wrote:
This was copied over from avocado but has not been used in the new
functional tests.
Signed-off-by: Daniel P. Berrangé
---
tests/functional/qemu_test/testcase.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/functional/qemu_test/testcase.py
b/tests/functional/qemu_test/testcase.py
index 94541e8bfb..332c782ebc 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -192,7 +192,7 @@ def assets_available(self):
return False
return True
-def setUp(self, bin_prefix):
+def setUp(self):
if self.qemu_bin is None:
self.skipTest("QEMU_TEST_QEMU_BINARY env variable is not set")
@@ -256,7 +256,7 @@ def main():
class QemuUserTest(QemuBaseTest):
def setUp(self):
-super().setUp('qemu-')
+super().setUp()
self._ldpath = []
def add_ldpath(self, ldpath):
@@ -279,7 +279,7 @@ class QemuSystemTest(QemuBaseTest):
def setUp(self):
self._vms = {}
-super().setUp('qemu-system-')
+super().setUp()
We might still need it in case we try to add auto-detection of the QEMU
binary again, but yes, for the time being:
Reviewed-by: Thomas Huth
Re: [PATCH 3/5] tests/functional: set 'qemu_bin' as an object level field
On 05/02/2025 16.59, Daniel P. Berrangé wrote: The 'qemu_bin' field is currently set on the class, despite being accessed as if it were an object instance field with 'self.qemu_bin'. This is no obvious need to have it as a class field, so move it into the object instance. Signed-off-by: Daniel P. Berrangé --- docs/devel/testing/functional.rst | 2 +- tests/functional/qemu_test/testcase.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth
Re: [PATCH v3 13/17] hw/misc: Support NPCM8XX CLK Module Registers
On 6/2/25 02:31, Hao Wu wrote:
NPCM8XX adds a few new registers and have a different set of reset
values to the CLK modules. This patch supports them.
This patch doesn't support the new clock values generated by these
registers. Currently no modules use these new clock values so they
are not necessary at this point.
Implementation of these clocks might be required when implementing
these modules.
Reviewed-by: Titus Rwantare
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/misc/npcm_clk.c | 113 -
include/hw/misc/npcm_clk.h | 10 +++-
2 files changed, 120 insertions(+), 3 deletions(-)
+enum NPCM8xxCLKRegisters {
+NPCM8XX_CLK_CLKEN1,
+NPCM8XX_CLK_CLKSEL,
+NPCM8XX_CLK_CLKDIV1,
+NPCM8XX_CLK_PLLCON0,
+NPCM8XX_CLK_PLLCON1,
+NPCM8XX_CLK_SWRSTR,
+NPCM8XX_CLK_IPSRST1 = 0x20 / sizeof(uint32_t),
+NPCM8XX_CLK_IPSRST2,
+NPCM8XX_CLK_CLKEN2,
+NPCM8XX_CLK_CLKDIV2,
+NPCM8XX_CLK_CLKEN3,
+NPCM8XX_CLK_IPSRST3,
+NPCM8XX_CLK_WD0RCR,
+NPCM8XX_CLK_WD1RCR,
+NPCM8XX_CLK_WD2RCR,
+NPCM8XX_CLK_SWRSTC1,
+NPCM8XX_CLK_SWRSTC2,
+NPCM8XX_CLK_SWRSTC3,
+NPCM8XX_CLK_TIPRSTC,
+NPCM8XX_CLK_PLLCON2,
+NPCM8XX_CLK_CLKDIV3,
+NPCM8XX_CLK_CORSTC,
+NPCM8XX_CLK_PLLCONG,
+NPCM8XX_CLK_AHBCKFI,
+NPCM8XX_CLK_SECCNT,
+NPCM8XX_CLK_CNTR25M,
+/* Registers unique to NPCM8XX SoC */
+NPCM8XX_CLK_CLKEN4,
+NPCM8XX_CLK_IPSRST4,
+NPCM8XX_CLK_BUSTO,
+NPCM8XX_CLK_CLKDIV4,
+NPCM8XX_CLK_WD0RCRB,
+NPCM8XX_CLK_WD1RCRB,
+NPCM8XX_CLK_WD2RCRB,
+NPCM8XX_CLK_SWRSTC1B,
+NPCM8XX_CLK_SWRSTC2B,
+NPCM8XX_CLK_SWRSTC3B,
+NPCM8XX_CLK_TIPRSTCB,
+NPCM8XX_CLK_CORSTCB,
+NPCM8XX_CLK_IPSRSTDIS1,
+NPCM8XX_CLK_IPSRSTDIS2,
+NPCM8XX_CLK_IPSRSTDIS3,
+NPCM8XX_CLK_IPSRSTDIS4,
+NPCM8XX_CLK_CLKENDIS1,
+NPCM8XX_CLK_CLKENDIS2,
+NPCM8XX_CLK_CLKENDIS3,
+NPCM8XX_CLK_CLKENDIS4,
+NPCM8XX_CLK_THRTL_CNT,
+NPCM8XX_CLK_REGS_END,
I suppose by '_END' you mean '_COUNT'. Better define
unrelated to the enum, to avoid confusing static
analyzers:
#define NPCM8XX_CLK_REGS_COUNT (NPCM8XX_CLK_THRTL_CNT + 1)
+};
Re: [PATCH 4/5] tests/functional: remove all class level fields
On 05/02/2025 16.59, Daniel P. Berrangé wrote: A number of fields are set at the class level on QemuBaseTest, even though the exact same named field is then set at the object level later in most cases. Signed-off-by: Daniel P. Berrangé --- tests/functional/qemu_test/testcase.py | 6 -- 1 file changed, 6 deletions(-) diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py index 574c1942f2..531d6393ad 100644 --- a/tests/functional/qemu_test/testcase.py +++ b/tests/functional/qemu_test/testcase.py @@ -33,12 +33,6 @@ class QemuBaseTest(unittest.TestCase): -arch = None - -workdir = None -log = None -logdir = None That's what you get when a Python ignorant like me tries to write object oriented Python code ;-) Thanks for cleaning up my mess! Reviewed-by: Thomas Huth
Re: [PATCH v3 16/17] hw/arm: Add NPCM845 Evaluation board
On 6/2/25 02:31, Hao Wu wrote:
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/arm/meson.build | 2 +-
hw/arm/npcm8xx_boards.c | 253 +++
include/hw/arm/npcm8xx.h | 20
3 files changed, 274 insertions(+), 1 deletion(-)
create mode 100644 hw/arm/npcm8xx_boards.c
diff --git a/include/hw/arm/npcm8xx.h b/include/hw/arm/npcm8xx.h
index 1f7e3d8116..f465d1eeb5 100644
--- a/include/hw/arm/npcm8xx.h
+++ b/include/hw/arm/npcm8xx.h
@@ -52,6 +52,26 @@
#define NPCM8XX_NR_PWM_MODULES 3
+typedef struct NPCM8xxMachine {
+MachineStateparent;
Please use docs/devel/style.rst recommendations:
'parent_obj'
+/*
+ * PWM fan splitter. each splitter connects to one PWM output and
+ * multiple MFT inputs.
+ */
+SplitIRQfan_splitter[NPCM8XX_NR_PWM_MODULES *
+ NPCM7XX_PWM_PER_MODULE];
+} NPCM8xxMachine;
+
+
+typedef struct NPCM8xxMachineClass {
+MachineClassparent;
'parent_class'
+
+const char *soc_type;
+} NPCM8xxMachineClass;
+
+#define TYPE_NPCM8XX_MACHINE MACHINE_TYPE_NAME("npcm8xx")
+OBJECT_DECLARE_TYPE(NPCM8xxMachine, NPCM8xxMachineClass, NPCM8XX_MACHINE)
+
typedef struct NPCM8xxState {
DeviceState parent;
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
On 2025/2/6 下午5:02, Philippe Mathieu-Daudé wrote:
On 6/2/25 03:34, bibo mao wrote:
On 2025/2/5 下午8:06, Xianglai Li wrote:
When the KVM_REG_LOONGARCH_VCPU_RESET command word
is sent to the kernel through the kvm_set_one_reg interface,
the parameter source needs to be a legal address,
otherwise the kernel will return an error and the command word
will fail to be sent.
Hi Xianglai,
Good catch, it is actually one problem and thanks for reporting it.
Signed-off-by: Xianglai Li
---
Cc: Bibo Mao
Cc: Song Gao
target/loongarch/kvm/kvm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index a3f55155b0..01cddb7012 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
void kvm_arch_reset_vcpu(CPUState *cs)
{
CPULoongArchState *env = cpu_env(cs);
+ uint64_t val;
How about set initial value here although it is not used? such as
uint64_t val = 0;
Or uint64_t unused = 0;
yes, that is better and easier to understand -:)
env->mp_state = KVM_MP_STATE_RUNNABLE;
- kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
+ kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
Can we add return value checking here? such as
ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
if (ret) {
error_report("... %s", strerror(errno));
}
Regards
Bibo Mao
}
static int kvm_loongarch_get_mpstate(CPUState *cs)
Re: [PATCH 5/5] tests/functional: skip mem addr test on 32-bit hosts
On 06/02/2025 10.35, Daniel P. Berrangé wrote: On Wed, Feb 05, 2025 at 10:24:08AM -0800, Richard Henderson wrote: On 2/5/25 07:59, Daniel P. Berrangé wrote: + +''' +Decorator to skip execution of a test on 32-bit targets +Example: + + @skipIf32BitTarget() +''' +def skipIf32BitTarget(): +enoughBits = sys.maxsize > 2**32 This will work for true 32-bit hosts, and possibly for containers running emulation, but it won't work for cross-compilation (x86_64 to i686 or aarch64 to arm). Perhaps "file qemu-system-foo" | grep "ELF 32-bit" ? I don't know that we've actually selected the executable at this point though... The QEMU_TEST_QEMU_BINARY env variable exists at all times, though grepping for ELF format feels a bit icky. We also know the location of the build directory, so was wondering if anything there tells us whether the host target is 64-bit, but it appears not be the case. Maybe it's sufficient to wait for Richard's patch to get merged: https://lore.kernel.org/qemu-devel/20250204215359.1238808-11-richard.hender...@linaro.org/ ? Thomas
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
Hi Paolo,
On 17/1/25 20:40, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/src/device.rs | 43 +++---
rust/hw/char/pl011/src/lib.rs| 1 -
rust/hw/char/pl011/src/memory_ops.rs | 36 -
rust/qemu-api/meson.build| 1 +
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/memory.rs | 191 +++
rust/qemu-api/src/sysbus.rs | 7 +-
rust/qemu-api/src/zeroable.rs| 12 ++
8 files changed, 234 insertions(+), 58 deletions(-)
delete mode 100644 rust/hw/char/pl011/src/memory_ops.rs
create mode 100644 rust/qemu-api/src/memory.rs
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 259efacb046..294394c6e82 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,7 +2,7 @@
// Author(s): Manos Pitsidianakis
// SPDX-License-Identifier: GPL-2.0-or-later
-use core::ptr::{addr_of_mut, NonNull};
+use core::ptr::{addr_of, addr_of_mut, NonNull};
use std::{
ffi::CStr,
os::raw::{c_int, c_void},
@@ -12,14 +12,14 @@
bindings::{self, *},
c_str, impl_vmstate_forward,
irq::InterruptSource,
+memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
-qdev::{Clock, ClockEvent, DeviceImpl, ResettablePhasesImpl, ResetType},
+qdev::{Clock, ClockEvent, DeviceImpl, ResetType, ResettablePhasesImpl},
qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
};
use crate::{
device_class,
-memory_ops::PL011_OPS,
registers::{self, Interrupt},
RegisterOffset,
};
@@ -490,20 +490,24 @@ impl PL011State {
/// location/instance. All its fields are expected to hold unitialized
/// values with the sole exception of `parent_obj`.
unsafe fn init(&mut self) {
+static PL011_OPS: MemoryRegionOps =
MemoryRegionOpsBuildernew()
+.read(&PL011State::read)
+.write(&PL011State::write)
+.native_endian()
Could we always make .valid_sizes() explicit?
+.impl_sizes(4, 4)
+.build();
+
// SAFETY:
//
// self and self.iomem are guaranteed to be valid at this point since
callers
// must make sure the `self` reference is valid.
-unsafe {
-memory_region_init_io(
-addr_of_mut!(self.iomem),
-addr_of_mut!(*self).cast::(),
-&PL011_OPS,
-addr_of_mut!(*self).cast::(),
-Self::TYPE_NAME.as_ptr(),
-0x1000,
-);
-}
+MemoryRegion::init_io(
+unsafe { &mut *addr_of_mut!(self.iomem) },
+addr_of_mut!(*self),
+&PL011_OPS,
+"pl011",
+0x1000,
+);
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 300c732ae1d..5622e974cbc 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -29,7 +29,6 @@
mod device;
mod device_class;
-mod memory_ops;
pub use device::pl011_create;
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
deleted file mode 100644
index 95b4df794e4..000
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright 2024, Linaro Limited
-// Author(s): Manos Pitsidianakis
-// SPDX-License-Identifier: GPL-2.0-or-later
-
-use core::ptr::NonNull;
-use std::os::raw::{c_uint, c_void};
-
-use qemu_api::{bindings::*, zeroable::Zeroable};
-
-use crate::device::PL011State;
-
-pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps {
-read: Some(pl011_read),
-write: Some(pl011_write),
-read_with_attrs: None,
-write_with_attrs: None,
-endianness: device_endian::DEVICE_NATIVE_ENDIAN,
-valid: Zeroable::ZERO,
-impl_: MemoryRegionOps__bindgen_ty_2 {
-min_access_size: 4,
-max_access_size: 4,
-..Zeroable::ZERO
-},
-};
Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
On Thu, Feb 06, 2025 at 10:02:25AM +, Shameerali Kolothum Thodi wrote: > Hi Daniel, > > > -Original Message- > > From: Daniel P. Berrangé > > Sent: Friday, January 31, 2025 9:42 PM > > To: Shameerali Kolothum Thodi > > Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; > > eric.au...@redhat.com; peter.mayd...@linaro.org; j...@nvidia.com; > > nicol...@nvidia.com; ddut...@redhat.com; Linuxarm > > ; Wangzhou (B) ; > > jiangkunkun ; Jonathan Cameron > > ; zhangfei@linaro.org > > Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable > > nested SMMUv3 > > > > On Thu, Jan 30, 2025 at 06:09:24PM +, Shameerali Kolothum Thodi > > wrote: > > > > > > Each "arm-smmuv3-nested" instance, when the first device gets attached > > > to it, will create a S2 HWPT and a corresponding SMMUv3 domain in > > kernel > > > SMMUv3 driver. This domain will have a pointer representing the physical > > > SMMUv3 that the device belongs. And any other device which belongs to > > > the same physical SMMUv3 can share this S2 domain. > > > > Ok, so given two guest SMMUv3s, A and B, and two host SMMUv3s, > > C and D, we could end up with A&C and B&D paired, or we could > > end up with A&D and B&C paired, depending on whether we plug > > the first VFIO device into guest SMMUv3 A or B. > > > > This is bad. Behaviour must not vary depending on the order > > in which we create devices. > > > > An guest SMMUv3 is paired to a guest PXB. A guest PXB is liable > > to be paired to a guest NUMA node. A guest NUMA node is liable > > to be paired to host NUMA node. The guest/host SMMU pairing > > must be chosen such that it makes conceptual sense wrt to the > > guest PXB NUMA to host NUMA pairing. > > > > If the kernel picks guest<->host SMMU pairings on a first-device > > first-paired basis, this can end up with incorrect guest NUMA > > configurations. > > Ok. I am trying to understand how this can happen as I assume the > Guest PXB numa node is picked up by whatever device we are > attaching to it and based on which numa_id that device belongs to > in physical host. > > And the physical smmuv3 numa id will be the same to that of the > device numa_id it is associated with. Isn't it? > > For example I have a system here, that has 8 phys SMMUv3s and numa > assignments on this is something like below, > > Phys SMMUv3.0 --> node 0 > \..dev1 --> node0 > Phys SMMUv3.1 --> node 0 > \..dev2 -->node0 > Phys SMMUv3.2 --> node 0 > Phys SMMUv3.3 --> node 0 > > Phys SMMUv3.4 --> node 1 > Phys SMMUv3.5 --> node 1 > \..dev5 --> node1 > Phys SMMUv3.6 --> node 1 > Phys SMMUv3.7 --> node 1 > > > If I have to assign say dev 1, 2 and 5 to a Guest, we need to specify 3 > "arm-smmuv3-accel" instances as they belong to different phys SMMUv3s. > > -device pxb-pcie,id=pcie.1,bus_nr=1,bus=pcie.0,numa_id=0 \ > -device pxb-pcie,id=pcie.2,bus_nr=2,bus=pcie.0,numa_id=0 \ > -device pxb-pcie,id=pcie.3,bus_nr=3,bus=pcie.0,numa_id=1 \ > -device arm-smmuv3-accel,id=smmuv1,bus=pcie.1 \ > -device arm-smmuv3-accel,id=smmuv2,bus=pcie.2 \ > -device arm-smmuv3-accel,id=smmuv3,bus=pcie.3 \ > -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \ > -device pcie-root-port,id=pcie.port2,bus=pcie.3,chassis=2 \ > -device pcie-root-port,id=pcie.port3,bus=pcie.2,chassis=3 \ > -device vfio-pci,host=:dev1,bus=pcie.port1,iommufd=iommufd0 \ > -device vfio-pci,host=: dev2,bus=pcie.port2,iommufd=iommufd0 \ > -device vfio-pci,host=: dev5,bus=pcie.port3,iommufd=iommufd0 > > So I guess even if we don't specify the physical SMMUv3 association > explicitly, the kernel will check that based on the devices the Guest > SMMUv3 is attached to (and hence the Numa association), right? It isn't about checking the devices, it is about the guest SMMU getting differing host SMMU associations. > In other words how an explicit association helps us here? > > Or is it that the Guest PXB numa_id allocation is not always based > on device numa_id? Lets simplify to 2 SMMUs for shorter CLIs. So to start with we assume physical host with two SMMUs, and two PCI devices we want to assign :dev1 - associated with host SMMU 1, and host NUMA node 0 :dev2 - associated with host SMMU 2, and host NUMA node 1 So now we configure QEMU like this: -device pxb-pcie,id=pcie.1,bus_nr=1,bus=pcie.0,numa_id=0 -device pxb-pcie,id=pcie.2,bus_nr=2,bus=pcie.0,numa_id=1 -device arm-smmuv3-accel,id=smmuv1,bus=pcie.1 -device arm-smmuv3-accel,id=smmuv2,bus=pcie.2 -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 -device pcie-root-port,id=pcie.port2,bus=pcie.2,chassis=2 -device vfio-pci,host=:dev1,bus=pcie.port1,iommufd=iommufd0 -device vfio-pci,host=:dev2,bus=pcie.port2,iommufd=iommufd0 For brevity I'm not going to show the config for host/guest NUMA mappings, but assume that guest NUMA node 0 has been configured to map to host NUMA node 0 and guest node 1 to host node 1. In this order of QEMU CLI args we get VFIO device :dev1 cau
Re: [PATCH 1/2] qemu/timer: Clarify timer_new*() must be freed with timer_free()
On Sat, Jan 25, 2025 at 07:24:24PM +0100, Philippe Mathieu-Daudé wrote: > There was not mention QEMUTimer created with timer_new*() must > be released with timer_free() instead of g_free(), because then > active timers are removed from the active list. Update the > documentation mentioning timer_free(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/qemu/timer.h | 10 ++ > 1 file changed, 10 insertions(+) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
RE: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
Hi Nicolin, > -Original Message- > From: Nicolin Chen > Sent: Wednesday, February 5, 2025 12:09 AM > To: Shameerali Kolothum Thodi > ; Eric Auger > > Cc: ddut...@redhat.com; Peter Maydell ; Jason > Gunthorpe ; Daniel P. Berrangé ; > qemu-...@nongnu.org; qemu-devel@nongnu.org; Linuxarm > ; Wangzhou (B) ; > jiangkunkun ; Jonathan Cameron > ; zhangfei@linaro.org > Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable > nested SMMUv3 > > On Tue, Feb 04, 2025 at 06:49:15PM +0100, Eric Auger wrote: > > > In summary, we will have the following series: > > > 1) HWPT uAPI patches in backends/iommufd.c (Zhenzhong or Shameer) > > >https://lore.kernel.org/qemu- > devel/sj0pr11mb6744943702eb5798ec9b3b9992...@sj0pr11mb6744.nam > prd11.prod.outlook.com/ > > > 2) vIOMMU uAPI patches in backends/iommufd.c (I will rebase/send) > > > for 1 and 2, are you taking about the "Add VIOMMU infrastructure > support > > " series in Shameer's branch: private-smmuv3-nested-dev-rfc-v1. > > Sorry I may instead refer to NVidia or Intel's branch but I am not sure > > about the last ones. > > That "vIOMMU infrastructure" is for 2, yes. > > For 1, it's inside the Intel's series: > "cover-letter: intel_iommu: Enable stage-1 translation for passthrough > device" > > So, we need to extract them out and make it separately.. > > > > 3) vSMMUv3 patches for HW-acc/nesting (Hoping Don/you could take > over) > > We can start sending it upstream assuming we have a decent test > environment. > > > > However in > > > https://lore.kernel.org/all/329445b2f68a47269292aefb34584375@huawei.c > om/ > > > > Shameer suggested he may include it in his SMMU multi instance series. > > What do you both prefer? > > Sure, I think it's good to include those patches, One of the feedback I received on my series was to rename "arm-smmuv3-nested" to "arm-smmuv3-accel" and possibly rename function names to include "accel' as well and move those functions to a separate "smmuv3-accel.c" file. I suppose that applies to the " Add HW accelerated nesting support for arm SMMUv3" series as well. Is that fine with you? Thanks, Shameer
Re: [PATCH 2/2] qemu/timer: Sanity check timer_list in timer_init_full()
On Sat, Jan 25, 2025 at 07:24:25PM +0100, Philippe Mathieu-Daudé wrote: > Ensure we are not re-initializing a QEMUTimer already added > to an active list. timer_init*() functions expect either > a recently created and zeroed QEMUTimer, or one previously > free'd with timer_free(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/qemu/timer.h | 2 +- > util/qemu-timer.c| 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 2/7] guest_memfd: Introduce an object to manage the guest-memfd with RamDiscardManager
On Thu, Jan 30, 2025 at 11:28:11AM -0500, Peter Xu wrote:
> On Sun, Jan 26, 2025 at 11:34:29AM +0800, Xu Yilun wrote:
> > > Definitely not suggesting to install an invalid pointer anywhere. The
> > > mapped pointer will still be valid for gmem for example, but the fault
> > > isn't. We need to differenciate two things (1) virtual address mapping,
> > > then (2) permission and accesses on the folios / pages of the mapping.
> > > Here I think it's okay if the host pointer is correctly mapped.
> > >
> > > For your private MMIO use case, my question is if there's no host pointer
> > > to be mapped anyway, then what's the benefit to make the MR to be ram=on?
> > > Can we simply make it a normal IO memory region? The only benefit of a
> >
> > The guest access to normal IO memory region would be emulated by QEMU,
> > while private assigned MMIO requires guest direct access via Secure EPT.
> >
> > Seems the existing code doesn't support guest direct access if
> > mr->ram == false:
>
> Ah it's about this, ok.
>
> I am not sure what's the best approach, but IMHO it's still better we stick
> with host pointer always available when ram=on. OTOH, VFIO private regions
> may be able to provide a special mark somewhere, just like when romd_mode
> was done previously as below (qemu 235e8982ad39), so that KVM should still
> apply these MRs even if they're not RAM.
Also good to me.
>
> >
> > static void kvm_set_phys_mem(KVMMemoryListener *kml,
> > MemoryRegionSection *section, bool add)
> > {
> > [...]
> >
> > if (!memory_region_is_ram(mr)) {
> > if (writable || !kvm_readonly_mem_allowed) {
> > return;
> > } else if (!mr->romd_mode) {
> > /* If the memory device is not in romd_mode, then we actually
> > want
> > * to remove the kvm memory slot so all accesses will trap. */
> > add = false;
> > }
> > }
> >
> > [...]
> >
> > /* register the new slot */
> > do {
> >
> > [...]
> >
> > err = kvm_set_user_memory_region(kml, mem, true);
> > }
> > }
> >
> > > ram=on MR is, IMHO, being able to be accessed as RAM-like. If there's no
> > > host pointer at all, I don't yet understand how that helps private MMIO
> > > from working.
> >
> > I expect private MMIO not accessible from host, but accessible from
> > guest so has kvm_userspace_memory_region2 set. That means the resolving
> > of its PFN during EPT fault cannot depends on host pointer.
> >
> > https://lore.kernel.org/all/20250107142719.179636-1-yilun...@linux.intel.com/
>
> I'll leave this to KVM experts, but I actually didn't follow exactly on why
> mmu notifier is an issue to make , as I thought that was per-mm anyway, and
> KVM
> should logically be able to skip all VFIO private MMIO regions if affected.
I think this creates logical inconsistency. You builds the private MMIO
EPT mapping on fault based on the HVA<->HPA mapping, but doesn't follow
the HVA<->HPA mapping change. Why KVM believes the mapping on fault time
but doesn't on mmu notify time?
> This is a comment to this part of your commit message:
>
> Rely on userspace mapping also means private MMIO mapping should
> follow userspace mapping change via mmu_notifier. This conflicts
> with the current design that mmu_notifier never impacts private
> mapping. It also makes no sense to support mmu_notifier just for
> private MMIO, private MMIO mapping should be fixed when CoCo-VM
> accepts the private MMIO, any following mapping change without
> guest permission should be invalid.
>
> So I don't yet see a hard-no of reusing userspace mapping even if they're
> not faultable as of now - what if they can be faultable in the future? I
The first commit of guest_memfd emphasize a lot on the benifit of
decoupling KVM mapping from host mapping. My understanding is even if
guest memfd can be faultable later, KVM should still work in a way
without userspace mapping.
> am not sure..
>
> OTOH, I also don't think we need KVM_SET_USER_MEMORY_REGION3 anyway.. The
> _REGION2 API is already smart enough to leave some reserved fields:
>
> /* for KVM_SET_USER_MEMORY_REGION2 */
> struct kvm_userspace_memory_region2 {
> __u32 slot;
> __u32 flags;
> __u64 guest_phys_addr;
> __u64 memory_size;
> __u64 userspace_addr;
> __u64 guest_memfd_offset;
> __u32 guest_memfd;
> __u32 pad1;
> __u64 pad2[14];
> };
>
> I think we _could_ reuse some pad*? Reusing guest_memfd field sounds error
> prone to me.
It truly is. I'm expecting some suggestions here.
Thanks,
Yilun
>
> Not sure it could be easier if it's not guest_memfd* but fd + fd_offset
> since the start. But I guess when introducing _REGION2 we didn't expect
> MMIO private regions come so soon..
>
> Thanks,
>
> --
> Peter Xu
>
Re: [PATCH 2/2] rust: include rust_version in Cargo.toml
> diff --git a/rust/hw/char/pl011/src/device_class.rs
> b/rust/hw/char/pl011/src/device_class.rs
> index 8a157a663fb..dbef93f6cb3 100644
> --- a/rust/hw/char/pl011/src/device_class.rs
> +++ b/rust/hw/char/pl011/src/device_class.rs
> @@ -12,7 +12,6 @@
>
> use crate::device::{PL011Registers, PL011State};
>
> -#[allow(clippy::missing_const_for_fn)]
It seems like a rebase nit since the commit 7d0520398f7f ("rust: prefer
NonNull::new to assertions"), which is not worth an extra commit either.
> extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
> let state = NonNull::new(opaque).unwrap().cast::();
> unsafe { state.as_ref().migrate_clock }
Reviewed-by: Zhao Liu
Re: [PATCH v2 4/8] hw/boards: Remove all invalid uses of auto_create_sdcard=true
On 5/2/25 08:03, Markus Armbruster wrote: Philippe Mathieu-Daudé writes: MachineClass::auto_create_sdcard is only useful to automatically create a SD card, attach a IF_SD block drive to it and plug the card onto a SD bus. Only the ARM and RISCV targets use such feature: $ git grep -wl IF_SD hw | cut -d/ -f-2 | sort -u hw/arm hw/riscv $ Remove all other uses. Signed-off-by: Philippe Mathieu-Daudé Impact? As far as I can tell, this stops creation of the if=sd default drive these machines don't actually use. Correct? Yes, since these machines don't expose a SD-bus, the drive can not be attached and always triggers the same error: $ qemu-system-hppa -sd /bin/sh qemu-system-hppa: -sd /bin/sh: machine type does not support if=sd,bus=0,unit=0
Re: [PATCH v4 2/4] qdev-properties: Accept bool for OnOffAuto
On 2025/02/06 18:48, Markus Armbruster wrote:
Akihiko Odaki writes:
On 2025/02/06 0:29, Markus Armbruster wrote:
Akihiko Odaki writes:
Accept bool literals for OnOffAuto properties for consistency with bool
properties. This enables users to set the "on" or "off" value in a
uniform syntax without knowing whether the "auto" value is accepted.
This behavior is especially useful when converting an existing bool
property to OnOffAuto or vice versa.
Signed-off-by: Akihiko Odaki
---
hw/core/qdev-properties.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 434a76f5036e..0081d79f9b7b 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -491,6 +491,21 @@ const PropertyInfo qdev_prop_string = {
.set = set_string,
};
+static void set_on_off_auto(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Property *prop = opaque;
+int *ptr = object_field_prop_ptr(obj, prop);
+bool value;
+
+if (visit_type_bool(v, name, &value, NULL)) {
+*ptr = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
+return;
+}
+
+qdev_propinfo_set_enum(obj, v, name, opaque, errp);
+}
+
/* --- on/off/auto --- */
const PropertyInfo qdev_prop_on_off_auto = {
@@ -498,7 +513,7 @@ const PropertyInfo qdev_prop_on_off_auto = {
.description = "on/off/auto",
.enum_table = &OnOffAuto_lookup,
.get = qdev_propinfo_get_enum,
-.set = qdev_propinfo_set_enum,
+.set = set_on_off_auto,
.set_default_value = qdev_propinfo_set_default_value_enum,
};
The qdev properties defined with DEFINE_PROP_ON_OFF_AUTO() now
additionally accept bool.
The commit message tries to explain why this change is useful, but it
leaves me confused.
Does this solve a problem with existing properties? If yes, what
exactly is the problem?
Or does this enable new uses of DEFINE_PROP_ON_OFF_AUTO()?
I'm trying to understand, but my gut feeling is "bad idea".
Having multiple ways to express the same thing is generally undesirable.
In this case, "foo": "on" and "foo": true, as well as "foo": "off" and
"foo": false.
Moreover, OnOffAuto then has two meanings: straightfoward enum as
defined in the QAPI schema, and the funny qdev property. This is
definitely a bad idea. DEFINE_PROP_T(), where T is some QAPI type,
should accept *exactly* the values of T. If these properties need to
accept something else, use another name to not invite confusion.
If I understand the cover letter correctly, you want to make certain
bool properties tri-state for some reason. I haven't looked closely
enough to judge whether that makes sense. But do you really have to
change a whole bunch of unrelated properties to solve your problem?
This is going to be a very hard sell.
I change various virtio properties because they all have a common
problem. The problem is, when the host does not support a virtio
capability, virtio devices automatically set capability properties false
even if the user explicitly sets them true.
First, I'd like to thank you for your detailed reply.
I understand we have something like this:
* true: on if possible, else off
* false: off (always possible)
Which one is the default?
It depends. Some properties have true by default. The others have false.
There is no way to reliably configure "on", i.e. fail if it's not
possible. I agree that's a problem.
This problem can be solved
using an existing mechanism, OnOffAuto, which differentiates the "auto"
state and explicit the "on" state.
I guess you're proposing something like this:
* auto: on if possible, else off
* on: on if possible, else error
* off: off (always possible)
Which one is the default?
I converted on to auto and off to false in a following patch.
However, converting bool to OnOffAuto surfaces another problem: they
disagree how "on" and "off" should be written. Please note that the
disagreement already exists and so it is nice to solve anyway.
Yes, converting bool to OnOffAuto is an incompatible change.
Not just about conversion, but this inconsistency require users to know
whether a property is bool or OnOffAuto and change how the values are
written in JSON accordingly. This somewhat hurts usability.
This patch tries to solve it by tolerating bool values for OnOffAuto. As
you pointed out, this approach has a downside: it makes OnOffAuto more
complicated by having multiple ways to express the same thing.
It also affects existing uses of OnOffAuto, where such a change is
unnecessary and undesirable.
Another approach is to have one unified way to express "on"/"off" for
bool and OnOffAuto. This will give three options in total:
1. Let OnOffAuto accept JSON bool and "on"/"off" (what this patch does)
The parenthesis is inaccurate. This patch only affects qdev properti
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
On 2/6/25 11:02, Philippe Mathieu-Daudé wrote: Could we always make .valid_sizes() explicit? Yes (for example build() could even fail to compile if you don't have impl_sizes/valid_sizes set), but why do you want that? I'm not even sure that all cases of .valid.max_access_size=4 are correct... Exactly for that :) Not have implicit default values, so correct values are reviewed when models are added. But I wouldn't bet that those that we have in C are reviewed and correct... They are incorrect if the hardware accepts 8-byte writes, either discarding the top 4 bytes (then impl must both be 8) or writing to both registers (then impl must be 4). Paolo
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
On 6/2/25 09:46, Paolo Bonzini wrote:
On Thu, Feb 6, 2025 at 9:40 AM Philippe Mathieu-Daudé wrote:
Hi Paolo,
On 17/1/25 20:40, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/src/device.rs | 43 +++---
rust/hw/char/pl011/src/lib.rs| 1 -
rust/hw/char/pl011/src/memory_ops.rs | 36 -
rust/qemu-api/meson.build| 1 +
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/memory.rs | 191 +++
rust/qemu-api/src/sysbus.rs | 7 +-
rust/qemu-api/src/zeroable.rs| 12 ++
8 files changed, 234 insertions(+), 58 deletions(-)
delete mode 100644 rust/hw/char/pl011/src/memory_ops.rs
create mode 100644 rust/qemu-api/src/memory.rs
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 259efacb046..294394c6e82 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,7 +2,7 @@
// Author(s): Manos Pitsidianakis
// SPDX-License-Identifier: GPL-2.0-or-later
-use core::ptr::{addr_of_mut, NonNull};
+use core::ptr::{addr_of, addr_of_mut, NonNull};
use std::{
ffi::CStr,
os::raw::{c_int, c_void},
@@ -12,14 +12,14 @@
bindings::{self, *},
c_str, impl_vmstate_forward,
irq::InterruptSource,
+memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
-qdev::{Clock, ClockEvent, DeviceImpl, ResettablePhasesImpl, ResetType},
+qdev::{Clock, ClockEvent, DeviceImpl, ResetType, ResettablePhasesImpl},
qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
};
use crate::{
device_class,
-memory_ops::PL011_OPS,
registers::{self, Interrupt},
RegisterOffset,
};
@@ -490,20 +490,24 @@ impl PL011State {
/// location/instance. All its fields are expected to hold unitialized
/// values with the sole exception of `parent_obj`.
unsafe fn init(&mut self) {
+static PL011_OPS: MemoryRegionOps =
MemoryRegionOpsBuildernew()
+.read(&PL011State::read)
+.write(&PL011State::write)
+.native_endian()
Could we always make .valid_sizes() explicit?
Yes (for example build() could even fail to compile if you don't have
impl_sizes/valid_sizes set), but why do you want that? I'm not even
sure that all cases of .valid.max_access_size=4 are correct...
Exactly for that :) Not have implicit default values, so correct
values are reviewed when models are added.
Re: [RFC v2 3/5] i386/kvm: Support event with select & umask format in KVM PMU filter
Daniel P. Berrangé writes:
> On Thu, Feb 06, 2025 at 05:54:32PM +0800, Zhao Liu wrote:
>> On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
>> > Date: Wed, 05 Feb 2025 11:07:10 +0100
>> > From: Markus Armbruster
>> > Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
>> > format in KVM PMU filter
>> >
>> > Zhao Liu writes:
>> >
>> > > The select&umask is the common way for x86 to identify the PMU event,
>> > > so support this way as the "x86-default" format in kvm-pmu-filter
>> > > object.
>> >
>> > So, format 'raw' lets you specify the PMU event code as a number, wheras
>> > 'x86-default' lets you specify it as select and umask, correct?
>>
>> Yes!
>>
>> > Why do we want both?
>>
>> This 2 formats are both wildly used in x86(for example, in perf tool).
>>
>> x86 documents usually specify the umask and select fields.
>>
>> But raw format could also be applied for ARM since ARM just uses a number
>> to encode event.
Is it really too much to ask of the client to compute a raw value from
umask and select values?
>> > > Signed-off-by: Zhao Liu
>> >
>> > [...]
>> >
>> > > diff --git a/qapi/kvm.json b/qapi/kvm.json
>> > > index d51aeeba7cd8..93b869e3f90c 100644
>> > > --- a/qapi/kvm.json
>> > > +++ b/qapi/kvm.json
>> > > @@ -27,11 +27,13 @@
>> > > #
>> > > # @raw: the encoded event code that KVM can directly consume.
>> > > #
>> > > +# @x86-default: standard x86 encoding format with select and umask.
>> >
>> > Why is this named -default?
>>
>> Intel and AMD both use umask+select to encode events, but this format
>> doesn't have a name... so I call it `default`, or what about
>> "x86-umask-select"?
Works for me.
>> > > +#
>> > > # Since 10.0
>> > > ##
>> > > { 'enum': 'KVMPMUEventEncodeFmt',
>> > >'prefix': 'KVM_PMU_EVENT_FMT',
>> > > - 'data': ['raw'] }
>> > > + 'data': ['raw', 'x86-default'] }
>> > >
>> > > ##
>> > > # @KVMPMURawEvent:
>> > > @@ -46,6 +48,25 @@
>> > > { 'struct': 'KVMPMURawEvent',
>> > >'data': { 'code': 'uint64' } }
>> > >
>> > > +##
>> > > +# @KVMPMUX86DefalutEvent:
>> >
>> > Default, I suppose.
>>
>> Thanks!
>>
>> > > +#
>> > > +# x86 PMU event encoding with select and umask.
>> > > +# raw_event = ((select & 0xf00UL) << 24) | \
>> > > +# (select) & 0xff) | \
>> > > +# ((umask) & 0xff) << 8)
>> >
>> > Sphinx rejects this with "Unexpected indentation."
>> >
>> > Is the formula needed here?
>>
>> I tried to explain the relationship between raw format and umask+select.
>>
>> Emm, where do you think is the right place to put the document like
>> this?
Do users need to know how to compute the raw event value from @select
and @umask?
If yes, is C code the best way?
Here's another way:
bits 0..7 : bits 0..7 of @select
bits 8..15: @umask
bits 24..27: bits 8..11 of @select
all other bits: zero
>> ...
>>
>> > > +##
>> > > +# @KVMPMUX86DefalutEventVariant:
>
> Typo s/Defalut/Default/ - repeated many times in this patch.
>
>> > > +#
>> > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
>> > > +# the numeric value.
>> > > +#
>> > > +# @select: x86 PMU event select field. This field is a 12-bit
>> > > +# unsigned number string.
>> > > +#
>> > > +# @umask: x86 PMU event umask field. This field is a uint8 string.
>> >
>> > Why are these strings? How are they parsed into numbers?
>>
>> In practice, the values associated with PMU events (code for arm, select&
>> umask for x86) are often expressed in hexadecimal. Further, from linux
>> perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
>> arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
>> s390 uses decimal value.
>>
>> Therefore, it is necessary to support hexadecimal in order to honor PMU
>> conventions.
>
> IMHO having a data format that matches an arbitrary external tool is not
> a goal for QMP. It should be neutral and exclusively use the normal JSON
> encoding, ie base-10 decimal. Yes, this means a user/client may have to
> convert from hex to dec before sending data over QMP. This is true of
> many areas of QMP/QEMU config though and thus normal/expected behaviour.
Concur.
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
On Thu, Feb 6, 2025 at 9:40 AM Philippe Mathieu-Daudé wrote:
>
> Hi Paolo,
>
> On 17/1/25 20:40, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini
> > ---
> > rust/hw/char/pl011/src/device.rs | 43 +++---
> > rust/hw/char/pl011/src/lib.rs| 1 -
> > rust/hw/char/pl011/src/memory_ops.rs | 36 -
> > rust/qemu-api/meson.build| 1 +
> > rust/qemu-api/src/lib.rs | 1 +
> > rust/qemu-api/src/memory.rs | 191 +++
> > rust/qemu-api/src/sysbus.rs | 7 +-
> > rust/qemu-api/src/zeroable.rs| 12 ++
> > 8 files changed, 234 insertions(+), 58 deletions(-)
> > delete mode 100644 rust/hw/char/pl011/src/memory_ops.rs
> > create mode 100644 rust/qemu-api/src/memory.rs
> >
> > diff --git a/rust/hw/char/pl011/src/device.rs
> > b/rust/hw/char/pl011/src/device.rs
> > index 259efacb046..294394c6e82 100644
> > --- a/rust/hw/char/pl011/src/device.rs
> > +++ b/rust/hw/char/pl011/src/device.rs
> > @@ -2,7 +2,7 @@
> > // Author(s): Manos Pitsidianakis
> > // SPDX-License-Identifier: GPL-2.0-or-later
> >
> > -use core::ptr::{addr_of_mut, NonNull};
> > +use core::ptr::{addr_of, addr_of_mut, NonNull};
> > use std::{
> > ffi::CStr,
> > os::raw::{c_int, c_void},
> > @@ -12,14 +12,14 @@
> > bindings::{self, *},
> > c_str, impl_vmstate_forward,
> > irq::InterruptSource,
> > +memory::{hwaddr, MemoryRegion, MemoryRegionOps,
> > MemoryRegionOpsBuilder},
> > prelude::*,
> > -qdev::{Clock, ClockEvent, DeviceImpl, ResettablePhasesImpl, ResetType},
> > +qdev::{Clock, ClockEvent, DeviceImpl, ResetType, ResettablePhasesImpl},
> > qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
> > };
> >
> > use crate::{
> > device_class,
> > -memory_ops::PL011_OPS,
> > registers::{self, Interrupt},
> > RegisterOffset,
> > };
> > @@ -490,20 +490,24 @@ impl PL011State {
> > /// location/instance. All its fields are expected to hold unitialized
> > /// values with the sole exception of `parent_obj`.
> > unsafe fn init(&mut self) {
> > +static PL011_OPS: MemoryRegionOps =
> > MemoryRegionOpsBuildernew()
> > +.read(&PL011State::read)
> > +.write(&PL011State::write)
> > +.native_endian()
>
> Could we always make .valid_sizes() explicit?
Yes (for example build() could even fail to compile if you don't have
impl_sizes/valid_sizes set), but why do you want that? I'm not even
sure that all cases of .valid.max_access_size=4 are correct...
Paolo
Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
On Fri, Jan 31, 2025 at 05:08:28PM +0100, Eric Auger wrote: > Hi, > > > On 1/31/25 4:23 PM, Shameerali Kolothum Thodi wrote: > > > >> -Original Message- > >> From: Jason Gunthorpe > >> Sent: Friday, January 31, 2025 2:54 PM > >> To: Shameerali Kolothum Thodi > >> Cc: Daniel P. Berrangé ; qemu-...@nongnu.org; > >> qemu-devel@nongnu.org; eric.au...@redhat.com; > >> peter.mayd...@linaro.org; nicol...@nvidia.com; ddut...@redhat.com; > >> Linuxarm ; Wangzhou (B) > >> ; jiangkunkun ; > >> Jonathan Cameron ; > >> zhangfei@linaro.org; Nathan Chen > >> Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable > >> nested SMMUv3 > >> > >> On Fri, Jan 31, 2025 at 02:39:53PM +, Shameerali Kolothum Thodi > >> wrote: > >> > > And Qemu does some checking to make sure that the device is indeed > associated > > with the specified phys-smmuv3. This can be done going through the > sysfs path checking > > which is what I guess libvirt is currently doing to populate the > >> topology. > So basically > > Qemu is just replicating that to validate again. > I would prefer that iommufd users not have to go out to sysfs.. > > > Or another option is extending the IOMMU_GET_HW_INFO IOCTL to > return the phys > > smmuv3 base address which can avoid going through the sysfs. > It also doesn't seem great to expose a physical address. But we could > have an 'iommu instance id' that was a unique small integer? > >>> Ok. But how the user space can map that to the device? > >> Why does it need to? > >> > >> libvirt picks some label for the vsmmu instance, it doesn't matter > >> what the string is. > >> > >> qemu validates that all of the vsmmu instances are only linked to PCI > >> device that have the same iommu ID. This is already happening in the > >> kernel, it will fail attaches to mismatched instances. > >> > >> Nothing further is needed? > > -device pxb-pcie,id=pcie.1,bus_nr=8,bus=pcie.0 \ > > -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \ > > -device arm-smmuv3-accel,bus=pcie.1,id=smmuv1 \ > I don't get what is the point of adding such an id if it is not > referenced anywhere? Every QDev device instance has an 'id' property - if you don't set one explicitly, QEMU will generate one internally. Libvirt will always set the 'id' property to avoid the internal auto- generated IDs, as it wants full knowledge of naming. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
On Wed, Feb 05, 2025 at 12:53:42PM -0800, Nathan Chen wrote: > > > On 1/31/2025 8:08 AM, Eric Auger wrote: > > > > > > > And Qemu does some checking to make sure that the device is indeed > > > > > > associated > > > > > > > with the specified phys-smmuv3. This can be done going through > > > > > > > the > > > > > > sysfs path checking > > > > > > > which is what I guess libvirt is currently doing to populate the > > > > topology. > > > > > > So basically > > > > > > > Qemu is just replicating that to validate again. > > > > > > I would prefer that iommufd users not have to go out to sysfs.. > > > > > > > > > > > > > Or another option is extending the IOMMU_GET_HW_INFO IOCTL to > > > > > > return the phys > > > > > > > smmuv3 base address which can avoid going through the sysfs. > > > > > > It also doesn't seem great to expose a physical address. But we > > > > > > could > > > > > > have an 'iommu instance id' that was a unique small integer? > > > > > Ok. But how the user space can map that to the device? > > > > Why does it need to? > > > > > > > > libvirt picks some label for the vsmmu instance, it doesn't matter > > > > what the string is. > > > > > > > > qemu validates that all of the vsmmu instances are only linked to PCI > > > > device that have the same iommu ID. This is already happening in the > > > > kernel, it will fail attaches to mismatched instances. > > > > > > > > Nothing further is needed? > > > -device pxb-pcie,id=pcie.1,bus_nr=8,bus=pcie.0 \ > > > -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \ > > > -device arm-smmuv3-accel,bus=pcie.1,id=smmuv1 \ > > I don't get what is the point of adding such an id if it is not > > referenced anywhere? > > > > Eric > > Daniel mentions that the host-to-guest SMMU pairing must be chosen such that > it makes conceptual sense w.r.t. the guest NUMA to host NUMA pairing [0]. > The current implementation allows for incorrect host to guest numa node > pairings, e.g. pSMMU has affinity to host numa node 0, but it’s paired with > a vSMMU paired with a guest numa node pinned to host numa node 1. > > By specifying the host SMMU id, we can explicitly pair a host SMMU with a > guest SMMU associated with the correct PXB NUMA node, vs. implying the > host-to-guest SMMU pairing based on what devices are attached to the PXB. > While it would not completely prevent the incorrect pSMMU/vSMMU pairing > w.r.t. host to guest numa node pairings, specifying the pSMMU id would make > the implications of host to guest numa node pairings more clear when > specifying a vSMMU instance. You've not specified any host SMMU id in the above CLI args though, only the PXB association. It needs something like -device arm-smmuv3-accel,bus=pcie.1,id=smmuv1,host-smmu=X where '' is some value to identify the host SMMU With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
> > > +pub struct MemoryRegionOps(
> > > +bindings::MemoryRegionOps,
> > > +// Note: quite often you'll see PhantomData mentioned when
> > > discussing
> > > +// covariance and contravariance; you don't need any of those to
> > > understand
> > > +// this usage of PhantomData. Quite simply, MemoryRegionOps
> > > *logically*
> > > +// holds callbacks that take an argument of type &T, except the type
> > > is erased
> > > +// before the callback is stored in the bindings::MemoryRegionOps
> > > field.
> > > +// The argument of PhantomData is a function pointer in order to
> > > represent
> > > +// that relationship; while that will also provide desirable and
> > > safe variance
> > > +// for T, variance is not the point but just a consequence.
> > > +PhantomData,
> > > +);
> >
> > Wow, it can be wrapped like this!
>
> I like your enthusiasm but I'm not sure what you refer to. ;) Maybe
> it's worth documenting this pattern, so please tell me more (after
> your holidays).
Throughout the entire holiday, I couldn't think of a better way to
express this. I find it particularly useful when wrapping multiple
callbacks. In the future, I want to explore more use cases where this
pattern can be applied.
> > > +impl MemoryRegion {
> > > +// inline to ensure that it is not included in tests, which only
> > > +// link to hwcore and qom. FIXME: inlining is actually the opposite
> > > +// of what we want, since this is the type-erased version of the
> > > +// init_io function below. Look into splitting the qemu_api crate.
> >
> > Ah, I didn't understand the issue described in this comment. Why would
> > inlining affect the linking of tests?
>
> If you don't inline it, do_init_io will always be linked into the
> tests because it is a non-generic function. The tests then fail to
> link, because memory_region_init_io is undefined.
I find even if I drop the `inline` attribution, the test.rs can still be
compiled (by `make check`), I think it's because test.rs hasn't involved
memory related tests so that do_init_io isn't linked into test.rs.
> This is ugly because do_init_io exists *exactly* to extract the part
> that is not generic. (See
> https://users.rust-lang.org/t/soft-question-significantly-improve-rust-compile-time-via-minimizing-generics/103632/8
> for an example of this; I think there's even a procedural macro crate
> that does that for you, but I can't find it right now).
Thanks! I see. I agree to keep `inline` anyway.
Re: [PATCH v7 RESEND 0/5] i386: Support SMP Cache Topology
On Wed, Feb 05, 2025 at 01:32:19PM +0100, Markus Armbruster wrote: > Date: Wed, 05 Feb 2025 13:32:19 +0100 > From: Markus Armbruster > Subject: Re: [PATCH v7 RESEND 0/5] i386: Support SMP Cache Topology > > Zhao Liu writes: > > > Hi folks, > > > > This is my v7 resend version (updated the commit message of origin > > v7's Patch 1). > > If anything changed, even if it's just a commit message, make it a new > version, not a resend, to avoid confusion. Next time :) > > [...] Thanks Markus! I'll keep in my mind about this :-).
[PATCH v2 3/6] hw/intc/aspeed: Add object type name to trace events for better debugging
Currently, these trace events only refer to INTC. To simplify the INTC model,
both INTC(CPU Die) and INTC_IO(IO Die) will share the same helper functions.
However, it is difficult to recognize whether these trace events are comes from
INTC or INTC_IO. To make these trace events more readable, adds object type name
to the INTC trace events.
Update trace events to include the "name" field for better identification.
Signed-off-by: Jamin Lin
---
hw/intc/aspeed_intc.c | 32 +++-
hw/intc/trace-events | 24
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 8b1f83c878..e1e4a9fe59 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -39,6 +39,7 @@ REG32(GICINT136_STATUS, 0x1804)
static void aspeed_intc_update(AspeedINTCState *s, int irq, int level)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+const char *name = object_get_typename(OBJECT(s));
if (irq >= aic->num_ints) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
@@ -46,7 +47,7 @@ static void aspeed_intc_update(AspeedINTCState *s, int irq,
int level)
return;
}
-trace_aspeed_intc_update_irq(irq, level);
+trace_aspeed_intc_update_irq(name, irq, level);
qemu_set_irq(s->output_pins[irq], level);
}
@@ -60,6 +61,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int
level)
{
AspeedINTCState *s = (AspeedINTCState *)opaque;
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+const char *name = object_get_typename(OBJECT(s));
uint32_t status_addr = GICINT_STATUS_BASE + ((0x100 * irq) >> 2);
uint32_t select = 0;
uint32_t enable;
@@ -71,7 +73,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int
level)
return;
}
-trace_aspeed_intc_set_irq(irq, level);
+trace_aspeed_intc_set_irq(name, irq, level);
enable = s->enable[irq];
if (!level) {
@@ -90,7 +92,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int
level)
return;
}
-trace_aspeed_intc_select(select);
+trace_aspeed_intc_select(name, select);
if (s->mask[irq] || s->regs[status_addr]) {
/*
@@ -102,14 +104,14 @@ static void aspeed_intc_set_irq(void *opaque, int irq,
int level)
* save source interrupt to pending variable.
*/
s->pending[irq] |= select;
-trace_aspeed_intc_pending_irq(irq, s->pending[irq]);
+trace_aspeed_intc_pending_irq(name, irq, s->pending[irq]);
} else {
/*
* notify firmware which source interrupt are coming
* by setting status register
*/
s->regs[status_addr] = select;
-trace_aspeed_intc_trigger_irq(irq, s->regs[status_addr]);
+trace_aspeed_intc_trigger_irq(name, irq, s->regs[status_addr]);
aspeed_intc_update(s, irq, 1);
}
}
@@ -118,6 +120,7 @@ static void aspeed_intc_enable_handler(AspeedINTCState *s,
hwaddr offset,
uint64_t data)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
uint32_t old_enable;
uint32_t change;
@@ -148,7 +151,7 @@ static void aspeed_intc_enable_handler(AspeedINTCState *s,
hwaddr offset,
/* enable new source interrupt */
if (old_enable != s->enable[irq]) {
-trace_aspeed_intc_enable(s->enable[irq]);
+trace_aspeed_intc_enable(name, s->enable[irq]);
s->regs[addr] = data;
return;
}
@@ -157,10 +160,10 @@ static void aspeed_intc_enable_handler(AspeedINTCState
*s, hwaddr offset,
change = s->regs[addr] ^ data;
if (change & data) {
s->mask[irq] &= ~change;
-trace_aspeed_intc_unmask(change, s->mask[irq]);
+trace_aspeed_intc_unmask(name, change, s->mask[irq]);
} else {
s->mask[irq] |= change;
-trace_aspeed_intc_mask(change, s->mask[irq]);
+trace_aspeed_intc_mask(name, change, s->mask[irq]);
}
s->regs[addr] = data;
@@ -170,6 +173,7 @@ static void aspeed_intc_status_handler(AspeedINTCState *s,
hwaddr offset,
uint64_t data)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
uint32_t irq;
@@ -196,7 +200,7 @@ static void aspeed_intc_status_handler(AspeedINTCState *s,
hwaddr offset,
/* All source ISR execution are done */
if (!s->regs[addr]) {
-trace_aspeed_intc_all_isr_done(irq);
+trace_aspeed_intc_all_isr_done(name, irq);
if (s->pending[irq]) {
/*
* handle pending source interrupt
@@ -205,11 +209,11 @@ static void aspeed_intc_status_handler(AspeedINTCState
*s, hwaddr offset,
*/
s->regs[addr] = s->pending[irq
[PATCH v2 2/6] hw/intc/aspeed: Introduce helper functions for enable and status registers
The behavior of the enable and status registers is almost identical between
INTC(CPU Die) and INTC_IO(IO Die). To reduce duplicated code, adds
"aspeed_intc_enable_handler" functions to handle enable register write
behavior and "aspeed_intc_status_handler" functions to handle status
register write behavior.
Signed-off-by: Jamin Lin
---
hw/intc/aspeed_intc.c | 185 +++---
1 file changed, 103 insertions(+), 82 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 316885a27a..8b1f83c878 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -114,6 +114,107 @@ static void aspeed_intc_set_irq(void *opaque, int irq,
int level)
}
}
+static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
+ uint64_t data)
+{
+AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+uint32_t addr = offset >> 2;
+uint32_t old_enable;
+uint32_t change;
+uint32_t irq;
+
+irq = (offset & 0x0f00) >> 8;
+
+if (irq >= aic->num_ints) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
+ __func__, irq);
+return;
+}
+
+/*
+ * The enable registers are used to enable source interrupts.
+ * They also handle masking and unmasking of source interrupts
+ * during the execution of the source ISR.
+ */
+
+/* disable all source interrupt */
+if (!data && !s->enable[irq]) {
+s->regs[addr] = data;
+return;
+}
+
+old_enable = s->enable[irq];
+s->enable[irq] |= data;
+
+/* enable new source interrupt */
+if (old_enable != s->enable[irq]) {
+trace_aspeed_intc_enable(s->enable[irq]);
+s->regs[addr] = data;
+return;
+}
+
+/* mask and unmask source interrupt */
+change = s->regs[addr] ^ data;
+if (change & data) {
+s->mask[irq] &= ~change;
+trace_aspeed_intc_unmask(change, s->mask[irq]);
+} else {
+s->mask[irq] |= change;
+trace_aspeed_intc_mask(change, s->mask[irq]);
+}
+
+s->regs[addr] = data;
+}
+
+static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset,
+ uint64_t data)
+{
+AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+uint32_t addr = offset >> 2;
+uint32_t irq;
+
+irq = (offset & 0x0f00) >> 8;
+
+if (irq >= aic->num_ints) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
+ __func__, irq);
+return;
+}
+
+/* clear status */
+s->regs[addr] &= ~data;
+
+/*
+ * These status registers are used for notify sources ISR are executed.
+ * If one source ISR is executed, it will clear one bit.
+ * If it clear all bits, it means to initialize this register status
+ * rather than sources ISR are executed.
+ */
+if (data == 0x) {
+return;
+}
+
+/* All source ISR execution are done */
+if (!s->regs[addr]) {
+trace_aspeed_intc_all_isr_done(irq);
+if (s->pending[irq]) {
+/*
+ * handle pending source interrupt
+ * notify firmware which source interrupt are pending
+ * by setting status register
+ */
+s->regs[addr] = s->pending[irq];
+s->pending[irq] = 0;
+trace_aspeed_intc_trigger_irq(irq, s->regs[addr]);
+aspeed_intc_update(s, irq, 1);
+} else {
+/* clear irq */
+trace_aspeed_intc_clear_irq(irq, 0);
+aspeed_intc_update(s, irq, 0);
+}
+}
+}
+
static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int
size)
{
AspeedINTCState *s = ASPEED_INTC(opaque);
@@ -140,9 +241,6 @@ static void aspeed_intc_write(void *opaque, hwaddr offset,
uint64_t data,
AspeedINTCState *s = ASPEED_INTC(opaque);
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
uint32_t addr = offset >> 2;
-uint32_t old_enable;
-uint32_t change;
-uint32_t irq;
if (offset >= aic->reg_size) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -163,45 +261,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset,
uint64_t data,
case R_GICINT134_EN:
case R_GICINT135_EN:
case R_GICINT136_EN:
-irq = (offset & 0x0f00) >> 8;
-
-if (irq >= aic->num_ints) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number:
%d\n",
- __func__, irq);
-return;
-}
-
-/*
- * These registers are used for enable sources interrupt and
- * mask and unmask source interrupt while executing source ISR.
- */
-
-/* disable all source interrupt */
-if (!data && !s->enable[irq]) {
-s->regs[addr] = data;
-return;
-}
-
-old_enable = s->enable[irq];
-s->enable[irq] |= data;
[PATCH v2 5/6] hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
To improve readability, sort the IRQ table by IRQ number.
Signed-off-by: Jamin Lin
---
hw/arm/aspeed_ast27x0.c | 50 -
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 39567fcab9..6a8487fee6 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -73,27 +73,13 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
static const int aspeed_soc_ast2700a0_irqmap[] = {
-[ASPEED_DEV_UART0] = 132,
-[ASPEED_DEV_UART1] = 132,
-[ASPEED_DEV_UART2] = 132,
-[ASPEED_DEV_UART3] = 132,
-[ASPEED_DEV_UART4] = 8,
-[ASPEED_DEV_UART5] = 132,
-[ASPEED_DEV_UART6] = 132,
-[ASPEED_DEV_UART7] = 132,
-[ASPEED_DEV_UART8] = 132,
-[ASPEED_DEV_UART9] = 132,
-[ASPEED_DEV_UART10]= 132,
-[ASPEED_DEV_UART11]= 132,
-[ASPEED_DEV_UART12]= 132,
-[ASPEED_DEV_FMC] = 131,
[ASPEED_DEV_SDMC] = 0,
-[ASPEED_DEV_SCU] = 12,
-[ASPEED_DEV_ADC] = 130,
+[ASPEED_DEV_HACE] = 4,
[ASPEED_DEV_XDMA] = 5,
-[ASPEED_DEV_EMMC] = 15,
-[ASPEED_DEV_GPIO] = 130,
+[ASPEED_DEV_UART4] = 8,
+[ASPEED_DEV_SCU] = 12,
[ASPEED_DEV_RTC] = 13,
+[ASPEED_DEV_EMMC] = 15,
[ASPEED_DEV_TIMER1]= 16,
[ASPEED_DEV_TIMER2]= 17,
[ASPEED_DEV_TIMER3]= 18,
@@ -102,19 +88,33 @@ static const int aspeed_soc_ast2700a0_irqmap[] = {
[ASPEED_DEV_TIMER6]= 21,
[ASPEED_DEV_TIMER7]= 22,
[ASPEED_DEV_TIMER8]= 23,
-[ASPEED_DEV_WDT] = 131,
-[ASPEED_DEV_PWM] = 131,
+[ASPEED_DEV_DP]= 28,
[ASPEED_DEV_LPC] = 128,
[ASPEED_DEV_IBT] = 128,
+[ASPEED_DEV_KCS] = 128,
+[ASPEED_DEV_ADC] = 130,
+[ASPEED_DEV_GPIO] = 130,
[ASPEED_DEV_I2C] = 130,
-[ASPEED_DEV_PECI] = 133,
+[ASPEED_DEV_FMC] = 131,
+[ASPEED_DEV_WDT] = 131,
+[ASPEED_DEV_PWM] = 131,
+[ASPEED_DEV_I3C] = 131,
+[ASPEED_DEV_UART0] = 132,
+[ASPEED_DEV_UART1] = 132,
+[ASPEED_DEV_UART2] = 132,
+[ASPEED_DEV_UART3] = 132,
+[ASPEED_DEV_UART5] = 132,
+[ASPEED_DEV_UART6] = 132,
+[ASPEED_DEV_UART7] = 132,
+[ASPEED_DEV_UART8] = 132,
+[ASPEED_DEV_UART9] = 132,
+[ASPEED_DEV_UART10]= 132,
+[ASPEED_DEV_UART11]= 132,
+[ASPEED_DEV_UART12]= 132,
[ASPEED_DEV_ETH1] = 132,
[ASPEED_DEV_ETH2] = 132,
[ASPEED_DEV_ETH3] = 132,
-[ASPEED_DEV_HACE] = 4,
-[ASPEED_DEV_KCS] = 128,
-[ASPEED_DEV_DP]= 28,
-[ASPEED_DEV_I3C] = 131,
+[ASPEED_DEV_PECI] = 133,
[ASPEED_DEV_SDHCI] = 133,
};
--
2.34.1
[PATCH v2 0/2] Minor mhpmevent related fixes
Here are two small fixes around mhpmevent encoding and reset value. The first patch is picked from the platform specific event encoding series[1]. [1] https://lore.kernel.org/qemu-devel/20241009-pmu_event_machine-v1-0-dcbd7a60e...@rivosinc.com/ Signed-off-by: Atish Patra --- Changes in v2: - Replace GENMASK_ULL with MAKE_64BIT_MASK - Applied RB/AB tags. - Link to v1: https://lore.kernel.org/r/20250115-pmu_minor_fixes-v1-0-c32388def...@rivosinc.com --- Atish Patra (2): target/riscv: Fix the hpmevent mask target/riscv: Mask out upper sscofpmf bits during validation target/riscv/cpu_bits.h | 5 ++--- target/riscv/pmu.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) --- base-commit: 3f26a7a370c11c7dff68dab19c4e0de901e4 change-id: 20250115-pmu_minor_fixes-7a2b8e3658e4 -- Regards, Atish patra
[PATCH v2 1/2] target/riscv: Fix the hpmevent mask
As per the latest privilege specification v1.13[1], the sscofpmf
only reserves first 8 bits of hpmeventX. Update the corresponding
masks accordingly.
[1]https://github.com/riscv/riscv-isa-manual/issues/1578
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Atish Patra
---
target/riscv/cpu_bits.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index f97c48a3943f..74859c4bc8ff 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -933,9 +933,8 @@ typedef enum RISCVException {
MHPMEVENTH_BIT_VSINH | \
MHPMEVENTH_BIT_VUINH)
-#define MHPMEVENT_SSCOF_MASK _ULL(0x)
-#define MHPMEVENT_IDX_MASK 0xF
-#define MHPMEVENT_SSCOF_RESVD 16
+#define MHPMEVENT_SSCOF_MASK MAKE_64BIT_MASK(63, 56)
+#define MHPMEVENT_IDX_MASK (~MHPMEVENT_SSCOF_MASK)
/* RISC-V-specific interrupt pending bits. */
#define CPU_INTERRUPT_RNMI CPU_INTERRUPT_TGT_EXT_0
--
2.43.0
[PATCH v2 6/6] hw/intc/aspeed: Support different memory region ops
The previous implementation set the "aspeed_intc_ops" struct, containing read
and write callbacks, to be used when I/O is performed on the INTC region.
Both "aspeed_intc_read" and "aspeed_intc_write" callback functions were used
for INTC (CPU Die).
To support the INTC_IO (IO Die) model, introduces a new "reg_ops" class
attribute. This allows setting different memory region operations to support
different INTC models.
Will introduce "aspeed_intc_io_read" and "aspeed_intc_io_write" callback
functions are used for INTC_IO.
Signed-off-by: Jamin Lin
---
hw/intc/aspeed_intc.c | 5 -
include/hw/intc/aspeed_intc.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index e1e4a9fe59..e0e843201a 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -335,7 +335,7 @@ static void aspeed_intc_realize(DeviceState *dev, Error
**errp)
sysbus_init_mmio(sbd, &s->iomem_container);
-memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
+memory_region_init_io(&s->iomem, OBJECT(s), aic->reg_ops, s,
TYPE_ASPEED_INTC ".regs", aic->reg_size);
memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
@@ -353,11 +353,14 @@ static void aspeed_intc_realize(DeviceState *dev, Error
**errp)
static void aspeed_intc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+AspeedINTCClass *aic = ASPEED_INTC_CLASS(klass);
dc->desc = "ASPEED INTC Controller";
dc->realize = aspeed_intc_realize;
device_class_set_legacy_reset(dc, aspeed_intc_reset);
dc->vmsd = NULL;
+
+aic->reg_ops = &aspeed_intc_ops;
}
static const TypeInfo aspeed_intc_info = {
diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index ecaeb15aea..749d7c55be 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -43,6 +43,7 @@ struct AspeedINTCClass {
uint32_t num_ints;
uint64_t mem_size;
uint64_t reg_size;
+const MemoryRegionOps *reg_ops;
};
#endif /* ASPEED_INTC_H */
--
2.34.1
[PATCH v2] qom: reverse order of instance_post_init calls
Currently, the instance_post_init calls are performed from the leaf
class and all the way up to Object. This is incorrect because the
leaf class cannot observe property values applied by the superclasses;
for example, a compat property will be set on a device *after*
the class's post_init callback has run.
In particular this makes it impossible for implementations of
accel_cpu_instance_init() to operate based on the actual values of
the properties, though it seems that cxl_dsp_instance_post_init and
rp_instance_post_init might have similar issues.
Follow instead the same order as instance_init, starting with Object
and running the child class's instance_post_init after the parent.
Signed-off-by: Paolo Bonzini
---
include/qom/object.h | 3 ++-
qom/object.c | 8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 9192265db76..c87a392259d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -445,7 +445,8 @@ struct Object
* class will have already been initialized so the type is only responsible
* for initializing its own members.
* @instance_post_init: This function is called to finish initialization of
- * an object, after all @instance_init functions were called.
+ * an object, after all @instance_init functions were called, as well as
+ * @instance_post_init functions for the parent classes.
* @instance_finalize: This function is called during object destruction. This
* is called before the parent @instance_finalize function has been called.
* An object should only free the members that are unique to its type in this
diff --git a/qom/object.c b/qom/object.c
index ec447f14a78..9b03da22cce 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -431,13 +431,13 @@ static void object_init_with_type(Object *obj, TypeImpl
*ti)
static void object_post_init_with_type(Object *obj, TypeImpl *ti)
{
-if (ti->instance_post_init) {
-ti->instance_post_init(obj);
-}
-
if (type_has_parent(ti)) {
object_post_init_with_type(obj, type_get_parent(ti));
}
+
+if (ti->instance_post_init) {
+ti->instance_post_init(obj);
+}
}
bool object_apply_global_props(Object *obj, const GPtrArray *props,
--
2.48.1
[PATCH v2 2/2] target/riscv: Mask out upper sscofpmf bits during validation
As per the ISA definition, the upper 8 bits in hpmevent are defined
by Sscofpmf for privilege mode filtering and overflow bits while the
lower 56 bits are desginated for platform specific hpmevent values.
For the reset case, mhpmevent value should have zero in lower 56 bits.
Software may set the OF bit to indicate disable interrupt.
Ensure that correct value is checked after masking while clearing the
event encodings.
Reviewed-by: Daniel Henrique Barboza
Acked-by: Alistair Francis
Signed-off-by: Atish Patra
---
target/riscv/pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index cf713663ee56..0408f96e6af8 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -390,7 +390,7 @@ int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t
value,
* Expected mhpmevent value is zero for reset case. Remove the current
* mapping.
*/
-if (!value) {
+if (!(value & MHPMEVENT_IDX_MASK)) {
g_hash_table_foreach_remove(cpu->pmu_event_ctr_map,
pmu_remove_event_map,
GUINT_TO_POINTER(ctr_idx));
--
2.43.0
Re: [PATCH] hw/sd/sdhci: Set reset value of interrupt registers
On 15/1/25 20:04, BALATON Zoltan wrote:
The interrupt enable registers are not reset to 0 but some bits are
enabled on reset. At least some U-Boot versions seem to expect this
and not initialise these registers before expecting interrupts. The
numbers in this patch match what QorIQ P1022 has on reset and fix
U-Boot for this SoC and should not break other drivers that initialise
(and thus overwrite) these reset values.
Signed-off-by: BALATON Zoltan
---
I've also noticed that the work around marked with an XXX comment near
line 600 breaks the U-Boot I've tested so I need to disable it:
if ((s->sdmasysad % boundary_chk) == 0) {
-page_aligned = true;
+//page_aligned = true;
}
What should this hack fix and could it be now removed or somehow
restricted to cases where it's needed?
Cc'ing Jamin for
https://lore.kernel.org/qemu-devel/20241213031205.641009-2-jamin_...@aspeedtech.com/
hw/sd/sdhci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 58375483e3..88eb0bfcb2 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -303,6 +303,8 @@ static void sdhci_reset(SDHCIState *s)
s->data_count = 0;
s->stopped_state = sdhc_not_stopped;
s->pending_insert_state = false;
+s->norintstsen = 0x013f;
+s->errintstsen = 0x117f;
I guess the problem is earlier:
/*
* Set all registers to 0. Capabilities/Version registers are not
cleared
* and assumed to always preserve their value, given to them during
* initialization
*/
memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg -
(uintptr_t)&s->sdmasysad);
Not all registers have to be reset.
Re: [PATCH v3 12/17] hw/misc: Add nr_regs and cold_reset_values to NPCM CLK
On 6/2/25 02:31, Hao Wu wrote:
These 2 values are different between NPCM7XX and NPCM8XX
CLKs. So we add them to the class and assign different values
to them.
Reviewed-by: Peter Maydell
Signed-off-by: Hao Wu
---
hw/misc/npcm_clk.c | 19 +--
include/hw/misc/npcm_clk.h | 9 -
2 files changed, 21 insertions(+), 7 deletions(-)
@@ -870,10 +872,11 @@ static const struct MemoryRegionOps npcm_clk_ops = {
static void npcm_clk_enter_reset(Object *obj, ResetType type)
{
NPCMCLKState *s = NPCM_CLK(obj);
+NPCMCLKClass *c = NPCM_CLK_GET_CLASS(s);
-QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
-
-memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
+g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values));
Equivalent to:
g_assert(sizeof(s->regs) >= sizeof(uintptr_t));
Not very useful IMHO.
+g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
+memcpy(s->regs, c->cold_reset_values, sizeof(s->regs));
s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
npcm7xx_clk_update_all_clocks(s);
/*
@@ -1045,11 +1048,14 @@ static void npcm_clk_class_init(ObjectClass *klass,
void *data)
static void npcm7xx_clk_class_init(ObjectClass *klass, void *data)
{
+NPCMCLKClass *c = NPCM_CLK_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
QEMU_BUILD_BUG_ON(NPCM7XX_CLK_REGS_END > NPCM_CLK_MAX_NR_REGS);
QEMU_BUILD_BUG_ON(NPCM7XX_CLK_REGS_END != NPCM7XX_CLK_NR_REGS);
dc->desc = "NPCM7xx Clock Control Registers";
+c->nr_regs = NPCM7XX_CLK_NR_REGS;
+c->cold_reset_values = npcm7xx_cold_reset_values;
}
static const TypeInfo npcm7xx_clk_pll_info = {
@@ -1081,6 +1087,7 @@ static const TypeInfo npcm_clk_info = {
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(NPCMCLKState),
.instance_init = npcm_clk_init,
+.class_size = sizeof(NPCMCLKClass),
.class_init = npcm_clk_class_init,
.abstract = true,
};
diff --git a/include/hw/misc/npcm_clk.h b/include/hw/misc/npcm_clk.h
index db03b46a52..f47614ac8d 100644
--- a/include/hw/misc/npcm_clk.h
+++ b/include/hw/misc/npcm_clk.h
@@ -175,8 +175,15 @@ struct NPCMCLKState {
Clock *clkref;
};
+typedef struct NPCMCLKClass {
+SysBusDeviceClass parent;
+
+size_t nr_regs;
+const uint32_t *cold_reset_values;
+} NPCMCLKClass;
[PATCH v2 1/6] hw/intc/aspeed: Support setting different memory and register size
According to the AST2700 datasheet, the INTC (CPU DIE) controller has 16KB
(0x4000) of register space, and the INTC_IO (I/O DIE) controller has 1KB (0x400)
of register space.
Introduced a new class attribute "mem_size" to set different memory sizes for
the INTC models in AST2700.
Introduced a new class attribute "reg_size" to set different register sizes for
the INTC models in AST2700.
Signed-off-by: Jamin Lin
---
hw/intc/aspeed_intc.c | 17 +
include/hw/intc/aspeed_intc.h | 4
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 126b711b94..316885a27a 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -117,10 +117,11 @@ static void aspeed_intc_set_irq(void *opaque, int irq,
int level)
static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int
size)
{
AspeedINTCState *s = ASPEED_INTC(opaque);
+AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
uint32_t addr = offset >> 2;
uint32_t value = 0;
-if (addr >= ASPEED_INTC_NR_REGS) {
+if (offset >= aic->reg_size) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
__func__, offset);
@@ -143,7 +144,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset,
uint64_t data,
uint32_t change;
uint32_t irq;
-if (addr >= ASPEED_INTC_NR_REGS) {
+if (offset >= aic->reg_size) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
__func__, offset);
@@ -302,10 +303,16 @@ static void aspeed_intc_realize(DeviceState *dev, Error
**errp)
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
int i;
+memory_region_init(&s->iomem_container, OBJECT(s),
+TYPE_ASPEED_INTC ".container", aic->mem_size);
+
+sysbus_init_mmio(sbd, &s->iomem_container);
+
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
- TYPE_ASPEED_INTC ".regs", ASPEED_INTC_NR_REGS << 2);
+ TYPE_ASPEED_INTC ".regs", aic->reg_size);
+
+memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
-sysbus_init_mmio(sbd, &s->iomem);
qdev_init_gpio_in(dev, aspeed_intc_set_irq, aic->num_ints);
for (i = 0; i < aic->num_ints; i++) {
@@ -344,6 +351,8 @@ static void aspeed_2700_intc_class_init(ObjectClass *klass,
void *data)
dc->desc = "ASPEED 2700 INTC Controller";
aic->num_lines = 32;
aic->num_ints = 9;
+aic->mem_size = 0x4000;
+aic->reg_size = 0x2000;
}
static const TypeInfo aspeed_2700_intc_info = {
diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index 18cb43476c..ecaeb15aea 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -25,6 +25,8 @@ struct AspeedINTCState {
/*< public >*/
MemoryRegion iomem;
+MemoryRegion iomem_container;
+
uint32_t regs[ASPEED_INTC_NR_REGS];
OrIRQState orgates[ASPEED_INTC_NR_INTS];
qemu_irq output_pins[ASPEED_INTC_NR_INTS];
@@ -39,6 +41,8 @@ struct AspeedINTCClass {
uint32_t num_lines;
uint32_t num_ints;
+uint64_t mem_size;
+uint64_t reg_size;
};
#endif /* ASPEED_INTC_H */
--
2.34.1
[PATCH v2 0/6] INTC model cleanup
v2: To streamline the review process, split the following patch series into three parts. https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.2465942-1-jamin_...@aspeedtech.com/ This patch series focuses on cleaning up the INTC model to facilitate future support for the INTC_IO model. Jamin Lin (6): hw/intc/aspeed: Support setting different memory and register size hw/intc/aspeed: Introduce helper functions for enable and status registers hw/intc/aspeed: Add object type name to trace events for better debugging hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0 hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number hw/intc/aspeed: Support different memory region ops hw/arm/aspeed.c | 8 +- hw/arm/aspeed_ast27x0.c | 58 - hw/intc/aspeed_intc.c | 227 -- hw/intc/trace-events | 24 ++-- include/hw/intc/aspeed_intc.h | 5 + 5 files changed, 183 insertions(+), 139 deletions(-) -- 2.34.1
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
On Wed, Feb 05, 2025 at 11:03:51AM +0100, Markus Armbruster wrote:
> Date: Wed, 05 Feb 2025 11:03:51 +0100
> From: Markus Armbruster
> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>
> Quick & superficial review for now.
Thanks!
> > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > new file mode 100644
> > index ..d51aeeba7cd8
> > --- /dev/null
> > +++ b/qapi/kvm.json
> > @@ -0,0 +1,116 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +
> > +##
> > +# = KVM based feature API
>
> This is a top-level section. It ends up between sections "QMP
> introspection" and "QEMU Object Model (QOM)". Is this what we want? Or
> should it be a sub-section of something? Or next to something else?
Do you mean it's not in the right place in the qapi-schema.json?
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index b1581988e4eb..742818d16e45 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -64,6 +64,7 @@
{ 'include': 'compat.json' }
{ 'include': 'control.json' }
{ 'include': 'introspect.json' }
+{ 'include': 'kvm.json' }
{ 'include': 'qom.json' }
{ 'include': 'qdev.json' }
{ 'include': 'machine-common.json' }
Because qom.json includes kvm.json, so I have to place it before
qom.json.
It doesn't have any dependencies itself, so placing it in the previous
position should be fine, where do you prefer?
> > +##
> > +
> > +##
> > +# @KVMPMUFilterAction:
> > +#
> > +# Actions that KVM PMU filter supports.
> > +#
> > +# @deny: disable the PMU event/counter in KVM PMU filter.
> > +#
> > +# @allow: enable the PMU event/counter in KVM PMU filter.
> > +#
> > +# Since 10.0
> > +##
> > +{ 'enum': 'KVMPMUFilterAction',
> > + 'prefix': 'KVM_PMU_FILTER_ACTION',
> > + 'data': ['allow', 'deny'] }
> > +
> > +##
> > +# @KVMPMUEventEncodeFmt:
>
> Please don't abbreviate Format to Fmt. We use Format elsewhere, and
> consistency is desirable.
OK, will fix.
> > ##
> > # = QEMU Object Model (QOM)
> > @@ -1108,6 +1109,7 @@
> >'if': 'CONFIG_LINUX' },
> > 'iommufd',
> > 'iothread',
> > +'kvm-pmu-filter',
> > 'main-loop',
> > { 'name': 'memory-backend-epc',
> >'if': 'CONFIG_LINUX' },
> > @@ -1183,6 +1185,7 @@
> >'if': 'CONFIG_LINUX' },
> >'iommufd':'IOMMUFDProperties',
> >'iothread': 'IothreadProperties',
> > + 'kvm-pmu-filter': 'KVMPMUFilterPropertyVariant',
>
> The others are like
>
> 'mumble': 'MumbleProperties'
>
> Let's stick to that, and also avoid running together multiple
> capitalized acronyms: KvmPmuFilterProperties.
IIUC, then I should use the name "KvmPmuFilterProperties" (string version
for QAPI), and the name "KvmPmuFilterPropertiesVariant" (numeric version
in codes), do you agree?
> >'main-loop': 'MainLoopProperties',
> >'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties',
> >'if': 'CONFIG_LINUX' },
>
RE: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
Hi Daniel, > -Original Message- > From: Daniel P. Berrangé > Sent: Friday, January 31, 2025 9:42 PM > To: Shameerali Kolothum Thodi > Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; > eric.au...@redhat.com; peter.mayd...@linaro.org; j...@nvidia.com; > nicol...@nvidia.com; ddut...@redhat.com; Linuxarm > ; Wangzhou (B) ; > jiangkunkun ; Jonathan Cameron > ; zhangfei@linaro.org > Subject: Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable > nested SMMUv3 > > On Thu, Jan 30, 2025 at 06:09:24PM +, Shameerali Kolothum Thodi > wrote: > > > > Each "arm-smmuv3-nested" instance, when the first device gets attached > > to it, will create a S2 HWPT and a corresponding SMMUv3 domain in > kernel > > SMMUv3 driver. This domain will have a pointer representing the physical > > SMMUv3 that the device belongs. And any other device which belongs to > > the same physical SMMUv3 can share this S2 domain. > > Ok, so given two guest SMMUv3s, A and B, and two host SMMUv3s, > C and D, we could end up with A&C and B&D paired, or we could > end up with A&D and B&C paired, depending on whether we plug > the first VFIO device into guest SMMUv3 A or B. > > This is bad. Behaviour must not vary depending on the order > in which we create devices. > > An guest SMMUv3 is paired to a guest PXB. A guest PXB is liable > to be paired to a guest NUMA node. A guest NUMA node is liable > to be paired to host NUMA node. The guest/host SMMU pairing > must be chosen such that it makes conceptual sense wrt to the > guest PXB NUMA to host NUMA pairing. > > If the kernel picks guest<->host SMMU pairings on a first-device > first-paired basis, this can end up with incorrect guest NUMA > configurations. Ok. I am trying to understand how this can happen as I assume the Guest PXB numa node is picked up by whatever device we are attaching to it and based on which numa_id that device belongs to in physical host. And the physical smmuv3 numa id will be the same to that of the device numa_id it is associated with. Isn't it? For example I have a system here, that has 8 phys SMMUv3s and numa assignments on this is something like below, Phys SMMUv3.0 --> node 0 \..dev1 --> node0 Phys SMMUv3.1 --> node 0 \..dev2 -->node0 Phys SMMUv3.2 --> node 0 Phys SMMUv3.3 --> node 0 Phys SMMUv3.4 --> node 1 Phys SMMUv3.5 --> node 1 \..dev5 --> node1 Phys SMMUv3.6 --> node 1 Phys SMMUv3.7 --> node 1 If I have to assign say dev 1, 2 and 5 to a Guest, we need to specify 3 "arm-smmuv3-accel" instances as they belong to different phys SMMUv3s. -device pxb-pcie,id=pcie.1,bus_nr=1,bus=pcie.0,numa_id=0 \ -device pxb-pcie,id=pcie.2,bus_nr=2,bus=pcie.0,numa_id=0 \ -device pxb-pcie,id=pcie.3,bus_nr=3,bus=pcie.0,numa_id=1 \ -device arm-smmuv3-accel,id=smmuv1,bus=pcie.1 \ -device arm-smmuv3-accel,id=smmuv2,bus=pcie.2 \ -device arm-smmuv3-accel,id=smmuv3,bus=pcie.3 \ -device pcie-root-port,id=pcie.port1,bus=pcie.1,chassis=1 \ -device pcie-root-port,id=pcie.port2,bus=pcie.3,chassis=2 \ -device pcie-root-port,id=pcie.port3,bus=pcie.2,chassis=3 \ -device vfio-pci,host=:dev1,bus=pcie.port1,iommufd=iommufd0 \ -device vfio-pci,host=: dev2,bus=pcie.port2,iommufd=iommufd0 \ -device vfio-pci,host=: dev5,bus=pcie.port3,iommufd=iommufd0 So I guess even if we don't specify the physical SMMUv3 association explicitly, the kernel will check that based on the devices the Guest SMMUv3 is attached to (and hence the Numa association), right? In other words how an explicit association helps us here? Or is it that the Guest PXB numa_id allocation is not always based on device numa_id? (May be I am missing something here. Sorry) Thanks, Shameer > The mgmt apps needs to be able to tell QEMU exactly which > host SMMU to pair with each guest SMMU, and QEMU needs to > then tell the kernel. > > > And as I mentioned in cover letter, Qemu will report, > > > > " > > Attempt to add the HNS VF to a different SMMUv3 will result in, > > > > -device vfio-pci,host=:7d:02.2,bus=pcie.port3,iommufd=iommufd0: > Unable to attach viommu > > -device vfio-pci,host=:7d:02.2,bus=pcie.port3,iommufd=iommufd0: > vfio :7d:02.2: > >Failed to set iommu_device: [iommufd=29] error attach :7d:02.2 > (38) to id=11: Invalid argument > > > > At present Qemu is not doing any extra validation other than the above > > failure to make sure the user configuration is correct or not. The > > assumption is libvirt will take care of this. > > " > > So in summary, if the libvirt gets it wrong, Qemu will fail with error. > > That's good error checking, and required, but also insufficient > as illustrated above IMHO. > > > If a more explicit association is required, some help from kernel is > required > > to identify the physical SMMUv3 associated with the device. > > Yep, I think SMMUv3 info for devices needs to be exposed to userspace, > as well as a mechanism for QEMU to tell the kernel the SMMU mapping. > > > With regards, > Daniel > -- > |: h
[PATCH v2 4/6] hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
Currently, AST2700 SoC only supports A0. To support AST2700 A1, rename its IRQ
table and machine name.
Signed-off-by: Jamin Lin
---
hw/arm/aspeed.c | 8
hw/arm/aspeed_ast27x0.c | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d9418e2b9f..6ddfdbdeba 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1654,12 +1654,12 @@ static void ast2700_evb_i2c_init(AspeedMachineState
*bmc)
TYPE_TMP105, 0x4d);
}
-static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
+static void aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void
*data)
{
MachineClass *mc = MACHINE_CLASS(oc);
AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
-mc->desc = "Aspeed AST2700 EVB (Cortex-A35)";
+mc->desc = "Aspeed AST2700 A0 EVB (Cortex-A35)";
amc->soc_name = "ast2700-a0";
amc->hw_strap1 = AST2700_EVB_HW_STRAP1;
amc->hw_strap2 = AST2700_EVB_HW_STRAP2;
@@ -1795,9 +1795,9 @@ static const TypeInfo aspeed_machine_types[] = {
.class_init = aspeed_minibmc_machine_ast1030_evb_class_init,
#ifdef TARGET_AARCH64
}, {
-.name = MACHINE_TYPE_NAME("ast2700-evb"),
+.name = MACHINE_TYPE_NAME("ast2700a0-evb"),
.parent= TYPE_ASPEED_MACHINE,
-.class_init= aspeed_machine_ast2700_evb_class_init,
+.class_init= aspeed_machine_ast2700a0_evb_class_init,
#endif
}, {
.name = TYPE_ASPEED_MACHINE,
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 4114e15ddd..39567fcab9 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -72,7 +72,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
#define AST2700_MAX_IRQ 256
/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
-static const int aspeed_soc_ast2700_irqmap[] = {
+static const int aspeed_soc_ast2700a0_irqmap[] = {
[ASPEED_DEV_UART0] = 132,
[ASPEED_DEV_UART1] = 132,
[ASPEED_DEV_UART2] = 132,
@@ -740,7 +740,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev,
Error **errp)
create_unimplemented_device("ast2700.io", 0x0, 0x400);
}
-static void aspeed_soc_ast2700_class_init(ObjectClass *oc, void *data)
+static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
{
static const char * const valid_cpu_types[] = {
ARM_CPU_TYPE_NAME("cortex-a35"),
@@ -763,7 +763,7 @@ static void aspeed_soc_ast2700_class_init(ObjectClass *oc,
void *data)
sc->uarts_num= 13;
sc->num_cpus = 4;
sc->uarts_base = ASPEED_DEV_UART0;
-sc->irqmap = aspeed_soc_ast2700_irqmap;
+sc->irqmap = aspeed_soc_ast2700a0_irqmap;
sc->memmap = aspeed_soc_ast2700_memmap;
sc->get_irq = aspeed_soc_ast2700_get_irq;
}
@@ -778,7 +778,7 @@ static const TypeInfo aspeed_soc_ast27x0_types[] = {
.name = "ast2700-a0",
.parent = TYPE_ASPEED27X0_SOC,
.instance_init = aspeed_soc_ast2700_init,
-.class_init = aspeed_soc_ast2700_class_init,
+.class_init = aspeed_soc_ast2700a0_class_init,
},
};
--
2.34.1
Re: [RFC v2 3/5] i386/kvm: Support event with select & umask format in KVM PMU filter
> > > > +## > > > > +# @KVMPMUX86DefalutEventVariant: > > Typo s/Defalut/Default/ - repeated many times in this patch. My bad! Will fix! > > > > +# > > > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than > > > > +# the numeric value. > > > > +# > > > > +# @select: x86 PMU event select field. This field is a 12-bit > > > > +# unsigned number string. > > > > +# > > > > +# @umask: x86 PMU event umask field. This field is a uint8 string. > > > > > > Why are these strings? How are they parsed into numbers? > > > > In practice, the values associated with PMU events (code for arm, select& > > umask for x86) are often expressed in hexadecimal. Further, from linux > > perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/ > > arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only > > s390 uses decimal value. > > > > Therefore, it is necessary to support hexadecimal in order to honor PMU > > conventions. > > IMHO having a data format that matches an arbitrary external tool is not > a goal for QMP. It should be neutral and exclusively use the normal JSON > encoding, ie base-10 decimal. Yes, this means a user/client may have to > convert from hex to dec before sending data over QMP. This is true of > many areas of QMP/QEMU config though and thus normal/expected behaviour. > Thanks! This will simplify the code a lot.
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
> > > @@ -1183,6 +1185,7 @@ > > >'if': 'CONFIG_LINUX' }, > > >'iommufd':'IOMMUFDProperties', > > >'iothread': 'IothreadProperties', > > > + 'kvm-pmu-filter': 'KVMPMUFilterPropertyVariant', > > > > The others are like > > > > 'mumble': 'MumbleProperties' > > > > Let's stick to that, and also avoid running together multiple > > capitalized acronyms: KvmPmuFilterProperties. > > IIUC, then I should use the name "KvmPmuFilterProperties" (string version > for QAPI), and the name "KvmPmuFilterPropertiesVariant" (numeric version > in codes), do you agree? > Thanks to Daniel's feedback, pmu filter doesn't need the string version anymore. So there's only 1 "KvmPmuFilterProperties" structure in QAPI.
[PATCH 1/4] include/standard-headers: add PCI IDs for Intel GPUs
From: Corvin Köhne
Intels integrated graphics devices do require many quirks to pass them
to a VM as passthrough device. Unfortunately, those quirks are device
specific and we have to check the device IDs to apply quirks properly.
In the past, we've maintained an own list of PCI IDs. However, it's
incomplete. Fortunately, Linux already maintains a list of known PCI
IDs, so we can copy it.
The file was obtained from the v6.13 tag of Linux.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/drm/intel/pciids.h?h=v6.13
Signed-off-by: Corvin Köhne
---
include/standard-headers/drm/intel/pciids.h | 834
1 file changed, 834 insertions(+)
create mode 100644 include/standard-headers/drm/intel/pciids.h
diff --git a/include/standard-headers/drm/intel/pciids.h
b/include/standard-headers/drm/intel/pciids.h
new file mode 100644
index 00..32480b5563
--- /dev/null
+++ b/include/standard-headers/drm/intel/pciids.h
@@ -0,0 +1,834 @@
+/*
+ * Copyright 2013 Intel Corporation
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#ifndef __PCIIDS_H__
+#define __PCIIDS_H__
+
+#ifdef __KERNEL__
+#define INTEL_VGA_DEVICE(_id, _info) { \
+ PCI_DEVICE(PCI_VENDOR_ID_INTEL, (_id)), \
+ .class = PCI_BASE_CLASS_DISPLAY << 16, .class_mask = 0xff << 16, \
+ .driver_data = (kernel_ulong_t)(_info), \
+}
+
+#define INTEL_QUANTA_VGA_DEVICE(_info) { \
+ .vendor = PCI_VENDOR_ID_INTEL, .device = 0x16a, \
+ .subvendor = 0x152d, .subdevice = 0x8990, \
+ .class = PCI_BASE_CLASS_DISPLAY << 16, .class_mask = 0xff << 16, \
+ .driver_data = (kernel_ulong_t)(_info), \
+}
+#endif
+
+#define INTEL_I810_IDS(MACRO__, ...) \
+ MACRO__(0x7121, ## __VA_ARGS__), /* I810 */ \
+ MACRO__(0x7123, ## __VA_ARGS__), /* I810_DC100 */ \
+ MACRO__(0x7125, ## __VA_ARGS__) /* I810_E */
+
+#define INTEL_I815_IDS(MACRO__, ...) \
+ MACRO__(0x1132, ## __VA_ARGS__) /* I815*/
+
+#define INTEL_I830_IDS(MACRO__, ...) \
+ MACRO__(0x3577, ## __VA_ARGS__)
+
+#define INTEL_I845G_IDS(MACRO__, ...) \
+ MACRO__(0x2562, ## __VA_ARGS__)
+
+#define INTEL_I85X_IDS(MACRO__, ...) \
+ MACRO__(0x3582, ## __VA_ARGS__), /* I855_GM */ \
+ MACRO__(0x358e, ## __VA_ARGS__)
+
+#define INTEL_I865G_IDS(MACRO__, ...) \
+ MACRO__(0x2572, ## __VA_ARGS__) /* I865_G */
+
+#define INTEL_I915G_IDS(MACRO__, ...) \
+ MACRO__(0x2582, ## __VA_ARGS__), /* I915_G */ \
+ MACRO__(0x258a, ## __VA_ARGS__) /* E7221_G */
+
+#define INTEL_I915GM_IDS(MACRO__, ...) \
+ MACRO__(0x2592, ## __VA_ARGS__) /* I915_GM */
+
+#define INTEL_I945G_IDS(MACRO__, ...) \
+ MACRO__(0x2772, ## __VA_ARGS__) /* I945_G */
+
+#define INTEL_I945GM_IDS(MACRO__, ...) \
+ MACRO__(0x27a2, ## __VA_ARGS__), /* I945_GM */ \
+ MACRO__(0x27ae, ## __VA_ARGS__) /* I945_GME */
+
+#define INTEL_I965G_IDS(MACRO__, ...) \
+ MACRO__(0x2972, ## __VA_ARGS__), /* I946_GZ */ \
+ MACRO__(0x2982, ## __VA_ARGS__),/* G35_G */ \
+ MACRO__(0x2992, ## __VA_ARGS__),/* I965_Q */ \
+ MACRO__(0x29a2, ## __VA_ARGS__) /* I965_G */
+
+#define INTEL_G33_IDS(MACRO__, ...) \
+ MACRO__(0x29b2, ## __VA_ARGS__), /* Q35_G */ \
+ MACRO__(0x29c2, ## __VA_ARGS__),/* G33_G */ \
+ MACRO__(0x29d2, ## __VA_ARGS__) /* Q33_G */
+
+#define INTEL_I965GM_IDS(MACRO__, ...) \
+ MACRO__(0x2a02, ## __VA_ARGS__),/* I965_GM */ \
+ MACRO__(0x2a12, ## __VA_ARGS__) /* I965_GME */
+
+#define INTEL_GM45_IDS(MACRO__, ...) \
+ MACRO__(0x2a42, ## __VA_ARGS__) /* GM45_G */
+
+#define INTEL_G45_IDS(MACRO__, ...) \
+ MACRO__(0x2e02, ## __VA_ARGS__), /* IGD_E_G */ \
+ MACRO__(0x2e12, ## __VA_ARGS__), /* Q45_G */ \
+ MACRO__(0x2e22, ## __VA_ARGS__), /* G45_G */ \
+ MACRO__(0x2e32, ## __VA_ARGS__), /* G41_G */ \
+
Re: [RFC PATCH 0/5] hw/arm/virt: Add support for user-creatable nested SMMUv3
On Thu, Feb 06, 2025 at 10:34:15AM +, Shameerali Kolothum Thodi wrote: > > -Original Message- > > From: Nicolin Chen > > On Tue, Feb 04, 2025 at 06:49:15PM +0100, Eric Auger wrote: > > > However in > > > > > > Shameer suggested he may include it in his SMMU multi instance series. > > > What do you both prefer? > > > > Sure, I think it's good to include those patches, > > One of the feedback I received on my series was to rename "arm-smmuv3-nested" > to "arm-smmuv3-accel" and possibly rename function names to include "accel' > as well > and move those functions to a separate "smmuv3-accel.c" file. I suppose that > applies to > the " Add HW accelerated nesting support for arm SMMUv3" series as well. > > Is that fine with you? Oh, no problem. If you want to rename the whole thing, please feel free. I do see the naming conflict between the "nested" stage and the "nested" HW feature, which are both supported by the vSMMU now. Thanks Nicolin
Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines
(sorry, posted too quick) On 6/2/25 19:24, Philippe Mathieu-Daudé wrote: +Michal On 6/2/25 19:06, Daniel P. Berrangé wrote: On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote: On 6/2/25 18:12, Daniel P. Berrangé wrote: On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote: On 6/2/25 15:31, Daniel P. Berrangé wrote: On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé wrote: Hi Daniel, On 6/2/25 14:20, Daniel P. Berrangé wrote: On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé wrote: Introduce an abstract machine parent class which defines the 'little_endian' property. Duplicate the current machine, which endian is tied to the binary endianness, to one big endian and a little endian machine; updating the machine description. Keep the current default machine for each binary. 'petalogix-s3adsp1800' machine is aliased as: - 'petalogix-s3adsp1800-be' on big-endian binary, - 'petalogix-s3adsp1800-le' on little-endian one. Does it makes sense to expose these as different machine types ? If all the HW is identical in both cases, it feels like the endianness could just be a bool property of the machine type, rather than a new machine type. Our test suites expect "qemu-system-foo -M bar" to work out of the box, we can not have non-default properties. (This is related to the raspberry pi discussion in https://lore.kernel.org/qemu-devel/20250204002240.97830-1- phi...@linaro.org/). My plan is to deprecate 'petalogix-s3adsp1800', so once we remove it we can merge both qemu-system-microblaze and qemu-system-microblazeel into a single binary. If you don't want to add more machines, what should be the endianness of the 'petalogix-s3adsp1800' machine in a binary with no particular endianness? Either we add for explicit endianness (fixing test suites) or we add one machine for each endianness; I fail to see other options not too confusing for our users. We would pick an arbitrary endianness of our choosing I guess. How does this work in physical machines ? Is the choice of endianess a firmware setting, or a choice by the vendor when manufacturing in some way ? Like MIPS*, SH4* and Xtensa*, it is a jumper on the board (wired to a CPU pin which is sampled once at cold reset). That makes me thing even more it is just a machine type property. I'm happy with a machine property, this was even my first approach using OnOffAuto until I ran the test-suite and have qom-introspection failing. What should be the default? Per the SH4 datasheet: Bit 31—Endian Flag (ENDIAN): Samples the value of the endian specification external pin (MD5) in a power-on reset by the RESET pin. The endian mode of all spaces is determined by this bit. ENDIAN is a read-only bit. There is no default per the spec, and I believe using one is a mistake. If it is left as an unspecified choice in the spec, then I would presume HW vendors are picking an option based on what they expect "most" common usage to be amongst their customers. IOW, if we know of typically used guest OS prefer big or little, that could influence our choice. Please have a look at this thread: https://lore.kernel.org/qemu-devel/ d3346a55-584b-427b-8c87-32f7411a9...@amd.com/ Per Michal: Truth is that I am not aware about anybody configuring MB to big endian and we are on AXI for quite a long time. There is still code in Linux kernel for it but I can't see any reason to keep it around. I don't think that make sense to keep big endian configurations alive at all. So with that in mind, using a default of little-endian=true for MicroBlaze seems safe as of 2025. For MIPS, big-endian was the default but as the industry shifted, it stayed big-endian on low-end 32-bit cores but mostly became little-endian on high-end 64-bit cores. For Renesas RX, the "Endian" section in the "CPU" chapter of the hardware manual mentions: Both big and little endian are supported for the treatment of data, and the endian is switched by changing the setting on a mode pin (MDE) at the time of a power-on reset. SH-4 "is bi-endian, running in either big-endian or little-endian byte ordering."
Re: [PULL 0/7] 9p queue 2025-02-06
On Thu, Feb 6, 2025 at 11:49 AM Christian Schoenebeck wrote: > > The following changes since commit d922088eb4ba6bc31a99f17b32cf75e59dd306cd: > > Merge tag 'ui-pull-request' of https://gitlab.com/marcandre.lureau/qemu > into staging (2025-02-03 13:42:02 -0500) > > are available in the Git repository at: > > https://github.com/cschoenebeck/qemu.git tags/pull-9p-20250206 > > for you to fetch changes up to bfa7bf02782dbd996201c90f850ca11730041af1: > > MAINTAINERS: Mark me as reviewer only for 9pfs (2025-02-06 17:10:46 +0100) > > > > * Greg Kurz steps back as maintainer of 9pfs. > > * Make multidevs=remap default option (instead of multidevs=warn) > and update documentation related to this option. > > * Improve tracing (i.e. usefulness of log output content). > > * Add test cases for accessing a directory after removal. > > > Christian Schoenebeck (6): > 9pfs: improve v9fs_walk() tracing > 9pfs: make multidevs=remap default > 9pfs: improve v9fs_open() tracing > tests/9p: rename test use_after_unlink -> use_file_after_unlink > tests/9p: add use_dir_after_unlink test > tests/9p: extend use_dir_after_unlink test with Treaddir The following test failure occurred in the CI system: 12/65 qemu:qtest+qtest-x86_64 / qtest-x86_64/qos-test ERROR 14.74s killed by signal 6 SIGABRT ― ✀ ― stderr: Received response 7 (RLERROR) instead of 77 (RUNLINKAT) Rlerror has errno 22 (Invalid argument) ** ERROR:../tests/qtest/libqos/virtio-9p-client.c:276:v9fs_req_recv: assertion failed (hdr.id == id): (7 == 77) (test program exited with status code -6) https://gitlab.com/qemu-project/qemu/-/jobs/9065429175 Please take a look. Thanks! Stefan > > Greg Kurz (1): > MAINTAINERS: Mark me as reviewer only for 9pfs > > MAINTAINERS | 3 +-- > hw/9pfs/9p-local.c | 3 +++ > hw/9pfs/9p-util-generic.c| 50 > > hw/9pfs/9p-util.h| 6 ++ > hw/9pfs/9p.c | 45 +-- > hw/9pfs/meson.build | 1 + > hw/9pfs/trace-events | 4 ++-- > qemu-options.hx | 49 --- > tests/qtest/virtio-9p-test.c | 50 > > 9 files changed, 175 insertions(+), 36 deletions(-) > create mode 100644 hw/9pfs/9p-util-generic.c >
[PATCH 06/61] tcg: Split out tcg_gen_gvec_3_var
Signed-off-by: Richard Henderson
---
include/tcg/tcg-op-gvec-common.h | 4 ++
tcg/tcg-op-gvec.c| 102 +++
2 files changed, 68 insertions(+), 38 deletions(-)
diff --git a/include/tcg/tcg-op-gvec-common.h b/include/tcg/tcg-op-gvec-common.h
index 877871c101..6e8fccad01 100644
--- a/include/tcg/tcg-op-gvec-common.h
+++ b/include/tcg/tcg-op-gvec-common.h
@@ -236,6 +236,10 @@ void tcg_gen_gvec_2i(uint32_t dofs, uint32_t aofs,
uint32_t oprsz,
uint32_t maxsz, int64_t c, const GVecGen2i *);
void tcg_gen_gvec_2s(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
uint32_t maxsz, TCGv_i64 c, const GVecGen2s *);
+void tcg_gen_gvec_3_var(TCGv_ptr dbase, uint32_t dofs,
+TCGv_ptr abase, uint32_t aofs,
+TCGv_ptr bbase, uint32_t bofs,
+uint32_t oprsz, uint32_t maxsz, const GVecGen3 *);
void tcg_gen_gvec_3(uint32_t dofs, uint32_t aofs, uint32_t bofs,
uint32_t oprsz, uint32_t maxsz, const GVecGen3 *);
void tcg_gen_gvec_3i(uint32_t dofs, uint32_t aofs, uint32_t bofs,
diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 08019421f2..3e53e43354 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -178,9 +178,11 @@ void tcg_gen_gvec_2i_ool(uint32_t dofs, uint32_t aofs,
TCGv_i64 c,
}
/* Generate a call to a gvec-style helper with three vector operands. */
-void tcg_gen_gvec_3_ool(uint32_t dofs, uint32_t aofs, uint32_t bofs,
-uint32_t oprsz, uint32_t maxsz, int32_t data,
-gen_helper_gvec_3 *fn)
+static void expand_3_ool(TCGv_ptr dbase, uint32_t dofs,
+ TCGv_ptr abase, uint32_t aofs,
+ TCGv_ptr bbase, uint32_t bofs,
+ uint32_t oprsz, uint32_t maxsz,
+ int32_t data, gen_helper_gvec_3 *fn)
{
TCGv_ptr a0, a1, a2;
TCGv_i32 desc = tcg_constant_i32(simd_desc(oprsz, maxsz, data));
@@ -189,9 +191,9 @@ void tcg_gen_gvec_3_ool(uint32_t dofs, uint32_t aofs,
uint32_t bofs,
a1 = tcg_temp_ebb_new_ptr();
a2 = tcg_temp_ebb_new_ptr();
-tcg_gen_addi_ptr(a0, tcg_env, dofs);
-tcg_gen_addi_ptr(a1, tcg_env, aofs);
-tcg_gen_addi_ptr(a2, tcg_env, bofs);
+tcg_gen_addi_ptr(a0, dbase, dofs);
+tcg_gen_addi_ptr(a1, abase, aofs);
+tcg_gen_addi_ptr(a2, bbase, bofs);
fn(a0, a1, a2, desc);
@@ -200,6 +202,14 @@ void tcg_gen_gvec_3_ool(uint32_t dofs, uint32_t aofs,
uint32_t bofs,
tcg_temp_free_ptr(a2);
}
+void tcg_gen_gvec_3_ool(uint32_t dofs, uint32_t aofs, uint32_t bofs,
+uint32_t oprsz, uint32_t maxsz, int32_t data,
+gen_helper_gvec_3 *fn)
+{
+expand_3_ool(tcg_env, dofs, tcg_env, aofs, tcg_env, bofs,
+ oprsz, maxsz, data, fn);
+}
+
/* Generate a call to a gvec-style helper with four vector operands. */
void tcg_gen_gvec_4_ool(uint32_t dofs, uint32_t aofs, uint32_t bofs,
uint32_t cofs, uint32_t oprsz, uint32_t maxsz,
@@ -789,8 +799,10 @@ static void expand_2s_i32(uint32_t dofs, uint32_t aofs,
uint32_t oprsz,
}
/* Expand OPSZ bytes worth of three-operand operations using i32 elements. */
-static void expand_3_i32(uint32_t dofs, uint32_t aofs,
- uint32_t bofs, uint32_t oprsz, bool load_dest,
+static void expand_3_i32(TCGv_ptr dbase, uint32_t dofs,
+ TCGv_ptr abase, uint32_t aofs,
+ TCGv_ptr bbase, uint32_t bofs,
+ uint32_t oprsz, bool load_dest,
void (*fni)(TCGv_i32, TCGv_i32, TCGv_i32))
{
TCGv_i32 t0 = tcg_temp_new_i32();
@@ -799,13 +811,13 @@ static void expand_3_i32(uint32_t dofs, uint32_t aofs,
uint32_t i;
for (i = 0; i < oprsz; i += 4) {
-tcg_gen_ld_i32(t0, tcg_env, aofs + i);
-tcg_gen_ld_i32(t1, tcg_env, bofs + i);
+tcg_gen_ld_i32(t0, abase, aofs + i);
+tcg_gen_ld_i32(t1, bbase, bofs + i);
if (load_dest) {
-tcg_gen_ld_i32(t2, tcg_env, dofs + i);
+tcg_gen_ld_i32(t2, dbase, dofs + i);
}
fni(t2, t0, t1);
-tcg_gen_st_i32(t2, tcg_env, dofs + i);
+tcg_gen_st_i32(t2, dbase, dofs + i);
}
tcg_temp_free_i32(t2);
tcg_temp_free_i32(t1);
@@ -953,8 +965,10 @@ static void expand_2s_i64(uint32_t dofs, uint32_t aofs,
uint32_t oprsz,
}
/* Expand OPSZ bytes worth of three-operand operations using i64 elements. */
-static void expand_3_i64(uint32_t dofs, uint32_t aofs,
- uint32_t bofs, uint32_t oprsz, bool load_dest,
+static void expand_3_i64(TCGv_ptr dbase, uint32_t dofs,
+ TCGv_ptr abase, uint32_t aofs,
+ TCGv_ptr bbase, uint32_t bofs,
+ uint32_t oprsz, bool load_dest,
void (*fni)(TCGv_i64, TCGv_i64, TC
[PATCH 13/61] target/arm: Add ZT0
This is a 512-bit array introduced with SME2.
Save it only when ZA is in use.
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 3 +++
target/arm/machine.c | 21 +
2 files changed, 24 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 938c990854..091a517a93 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -734,6 +734,9 @@ typedef struct CPUArchState {
uint64_t scxtnum_el[4];
struct {
+/* SME2 ZT0 -- 512 bit array, with data ordered like ARMVectorReg. */
+uint64_t zt0[512 / 64] QEMU_ALIGNED(16);
+
/*
* SME ZA storage -- 256 x 256 byte array, with bytes in host
* word order, as we do with vfp.zregs[]. This corresponds to
diff --git a/target/arm/machine.c b/target/arm/machine.c
index d41da414b3..416fe1b7be 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -320,6 +320,26 @@ static const VMStateDescription vmstate_za = {
VMSTATE_END_OF_LIST()
}
};
+
+static bool zt0_needed(void *opaque)
+{
+ARMCPU *cpu = opaque;
+
+return za_needed(cpu) && cpu_isar_feature(aa64_sme2, cpu);
+}
+
+static const VMStateDescription vmstate_zt0 = {
+.name = "cpu/zt0",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = zt0_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64_ARRAY(env.za_state.zt0, ARMCPU,
+ ARRAY_SIZE(((CPUARMState *)0)->za_state.zt0)),
+VMSTATE_END_OF_LIST()
+}
+};
+
#endif /* AARCH64 */
static bool serror_needed(void *opaque)
@@ -1104,6 +1124,7 @@ const VMStateDescription vmstate_arm_cpu = {
#ifdef TARGET_AARCH64
&vmstate_sve,
&vmstate_za,
+&vmstate_zt0,
#endif
&vmstate_serror,
&vmstate_irq_line_state,
--
2.43.0
