We have the ability to make memory accesses use a typesafe access width
type in Rust, which the C API currently lacks as it does not use a
newtype wrapper for specifying the amount of bytes a memory access has;
it uses a plain 32-bit integer value instead.
Replace use of u32 size arguments with a Bits enum that has only the
allowed byte sizes as variants and has a u32 representation so that it
can be fed back into C as well.
Signed-off-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
---
rust/hw/char/pl011/src/device.rs | 8 ++++----
rust/hw/timer/hpet/src/device.rs | 14 +++++++-------
rust/qemu-api/src/memory.rs | 34 ++++++++++++++++++++++++----------
3 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index
5b53f2649f161287f40f79075afba47db6d9315c..0c146821fbec4d310963264b90bb2bf2d30b901d
100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,7 +10,7 @@
irq::{IRQState, InterruptSource},
log::Log,
log_mask_ln,
- memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
+ memory::{hwaddr, Bits, MemoryRegion, MemoryRegionOps,
MemoryRegionOpsBuilder},
prelude::*,
qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType,
ResettablePhasesImpl},
qom::{ObjectImpl, Owned, ParentField, ParentInit},
@@ -501,7 +501,7 @@ unsafe fn init(mut this: ParentInit<Self>) {
.read(&PL011State::read)
.write(&PL011State::write)
.native_endian()
- .impl_sizes(4, 4)
+ .impl_sizes(Bits::_32, Bits::_32)
.build();
// SAFETY: this and this.iomem are guaranteed to be valid at this point
@@ -534,7 +534,7 @@ fn post_init(&self) {
}
}
- fn read(&self, offset: hwaddr, _size: u32) -> u64 {
+ fn read(&self, offset: hwaddr, _size: Bits) -> u64 {
match RegisterOffset::try_from(offset) {
Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => {
let device_id = self.get_class().device_id;
@@ -555,7 +555,7 @@ fn read(&self, offset: hwaddr, _size: u32) -> u64 {
}
}
- fn write(&self, offset: hwaddr, value: u64, _size: u32) {
+ fn write(&self, offset: hwaddr, value: u64, _size: Bits) {
let mut update_irq = false;
if let Ok(field) = RegisterOffset::try_from(offset) {
// qemu_chr_fe_write_all() calls into the can_receive
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index
acf7251029e912f18a5690b0d6cf04ea8151c5e1..a94e128e302a57df709ef3643694308833791859
100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -18,7 +18,7 @@
cell::{BqlCell, BqlRefCell},
irq::InterruptSource,
memory::{
- hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder,
MEMTXATTRS_UNSPECIFIED,
+ hwaddr, Bits, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder,
MEMTXATTRS_UNSPECIFIED,
},
prelude::*,
qdev::{DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
@@ -703,8 +703,8 @@ unsafe fn init(mut this: ParentInit<Self>) {
.read(&HPETState::read)
.write(&HPETState::write)
.native_endian()
- .valid_sizes(4, 8)
- .impl_sizes(4, 8)
+ .valid_sizes(Bits::_32, Bits::_64)
+ .impl_sizes(Bits::_32, Bits::_64)
.build();
MemoryRegion::init_io(
@@ -771,9 +771,9 @@ fn reset_hold(&self, _type: ResetType) {
self.rtc_irq_level.set(0);
}
- fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode<'_> {
+ fn decode(&self, mut addr: hwaddr, size: Bits) -> HPETAddrDecode<'_> {
let shift = ((addr & 4) * 8) as u32;
- let len = std::cmp::min(size * 8, 64 - shift);
+ let len = std::cmp::min(size as u32 * 8, 64 - shift);
addr &= !4;
let reg = if (0..=0xff).contains(&addr) {
@@ -796,7 +796,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) ->
HPETAddrDecode<'_> {
HPETAddrDecode { shift, len, reg }
}
- fn read(&self, addr: hwaddr, size: u32) -> u64 {
+ fn read(&self, addr: hwaddr, size: Bits) -> u64 {
// TODO: Add trace point - trace_hpet_ram_read(addr)
let HPETAddrDecode { shift, reg, .. } = self.decode(addr, size);
@@ -823,7 +823,7 @@ fn read(&self, addr: hwaddr, size: u32) -> u64 {
}) >> shift
}
- fn write(&self, addr: hwaddr, value: u64, size: u32) {
+ fn write(&self, addr: hwaddr, value: u64, size: Bits) {
let HPETAddrDecode { shift, len, reg } = self.decode(addr, size);
// TODO: Add trace point - trace_hpet_ram_write(addr, value)
diff --git a/rust/qemu-api/src/memory.rs b/rust/qemu-api/src/memory.rs
index
e40fad6cf19ea7971daf78afdf9ac886308ef5c3..b1907aa01300a3fac8e1f3b69c5d50da631a556d
100644
--- a/rust/qemu-api/src/memory.rs
+++ b/rust/qemu-api/src/memory.rs
@@ -20,6 +20,15 @@
zeroable::Zeroable,
};
+#[derive(Copy, Clone, Debug, Eq, PartialEq, qemu_api_macros::TryInto)]
+#[repr(u32)]
+pub enum Bits {
+ _8 = 1,
+ _16 = 2,
+ _32 = 4,
+ _64 = 8,
+}
+
pub struct MemoryRegionOps<T>(
bindings::MemoryRegionOps,
// Note: quite often you'll see PhantomData<fn(&T)> mentioned when
discussing
@@ -41,32 +50,37 @@ unsafe impl<T: Sync> Sync for MemoryRegionOps<T> {}
#[derive(Clone)]
pub struct MemoryRegionOpsBuilder<T>(bindings::MemoryRegionOps,
PhantomData<fn(&T)>);
-unsafe extern "C" fn memory_region_ops_read_cb<T, F: for<'a> FnCall<(&'a T,
hwaddr, u32), u64>>(
+unsafe extern "C" fn memory_region_ops_read_cb<T, F: for<'a> FnCall<(&'a T,
hwaddr, Bits), u64>>(
opaque: *mut c_void,
addr: hwaddr,
size: c_uint,
) -> u64 {
+ let size = Bits::try_from(size).expect("invalid size argument");
F::call((unsafe { &*(opaque.cast::<T>()) }, addr, size))
}
-unsafe extern "C" fn memory_region_ops_write_cb<T, F: for<'a> FnCall<(&'a T,
hwaddr, u64, u32)>>(
+unsafe extern "C" fn memory_region_ops_write_cb<
+ T,
+ F: for<'a> FnCall<(&'a T, hwaddr, u64, Bits)>,
+>(
opaque: *mut c_void,
addr: hwaddr,
data: u64,
size: c_uint,
) {
+ let size = Bits::try_from(size).expect("invalid size argument");
F::call((unsafe { &*(opaque.cast::<T>()) }, addr, data, size))
}
impl<T> MemoryRegionOpsBuilder<T> {
#[must_use]
- pub const fn read<F: for<'a> FnCall<(&'a T, hwaddr, u32), u64>>(mut self,
_f: &F) -> Self {
+ pub const fn read<F: for<'a> FnCall<(&'a T, hwaddr, Bits), u64>>(mut self,
_f: &F) -> Self {
self.0.read = Some(memory_region_ops_read_cb::<T, F>);
self
}
#[must_use]
- pub const fn write<F: for<'a> FnCall<(&'a T, hwaddr, u64, u32)>>(mut self,
_f: &F) -> Self {
+ pub const fn write<F: for<'a> FnCall<(&'a T, hwaddr, u64, Bits)>>(mut
self, _f: &F) -> Self {
self.0.write = Some(memory_region_ops_write_cb::<T, F>);
self
}
@@ -90,9 +104,9 @@ pub const fn native_endian(mut self) -> Self {
}
#[must_use]
- pub const fn valid_sizes(mut self, min: u32, max: u32) -> Self {
- self.0.valid.min_access_size = min;
- self.0.valid.max_access_size = max;
+ pub const fn valid_sizes(mut self, min: Bits, max: Bits) -> Self {
+ self.0.valid.min_access_size = min as u32;
+ self.0.valid.max_access_size = max as u32;
self
}
@@ -103,9 +117,9 @@ pub const fn valid_unaligned(mut self) -> Self {
}
#[must_use]
- pub const fn impl_sizes(mut self, min: u32, max: u32) -> Self {
- self.0.impl_.min_access_size = min;
- self.0.impl_.max_access_size = max;
+ pub const fn impl_sizes(mut self, min: Bits, max: Bits) -> Self {
+ self.0.impl_.min_access_size = min as u32;
+ self.0.impl_.max_access_size = max as u32;
self
}
--
2.47.2
