There is no need to monkeypatch DeviceId::Luminary into the already-initialized PL011State. Instead, now that we can define a class hierarchy, we can define PL011Class and make device_id a field in there.
There is also no need anymore to have "Arm" as zero, so change DeviceId into a wrapper for the array; all it does is provide an Index<hwaddr> implementation because arrays can only be indexed by usize. Reviewed-by: Zhao Liu <zhao1....@intel.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- rust/hw/char/pl011/src/device.rs | 59 +++++++++++++++----------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 689202f4550..215f94a6e4a 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -5,7 +5,7 @@ use core::ptr::{addr_of_mut, NonNull}; use std::{ ffi::CStr, - os::raw::{c_int, c_uchar, c_uint, c_void}, + os::raw::{c_int, c_uint, c_void}, }; use qemu_api::{ @@ -14,7 +14,7 @@ irq::InterruptSource, prelude::*, qdev::DeviceImpl, - qom::{ObjectImpl, ParentField}, + qom::{ClassInitImpl, ObjectImpl, ParentField}, }; use crate::{ @@ -33,27 +33,20 @@ /// QEMU sourced constant. pub const PL011_FIFO_DEPTH: u32 = 16; -#[derive(Clone, Copy, Debug)] -enum DeviceId { - #[allow(dead_code)] - Arm = 0, - Luminary, -} +#[derive(Clone, Copy)] +struct DeviceId(&'static [u8; 8]); impl std::ops::Index<hwaddr> for DeviceId { - type Output = c_uchar; + type Output = u8; fn index(&self, idx: hwaddr) -> &Self::Output { - match self { - Self::Arm => &Self::PL011_ID_ARM[idx as usize], - Self::Luminary => &Self::PL011_ID_LUMINARY[idx as usize], - } + &self.0[idx as usize] } } impl DeviceId { - const PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1]; - const PL011_ID_LUMINARY: [c_uchar; 8] = [0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]; + const ARM: Self = Self(&[0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1]); + const LUMINARY: Self = Self(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]); } // FIFOs use 32-bit indices instead of usize, for compatibility with @@ -126,17 +119,28 @@ pub struct PL011State { pub clock: NonNull<Clock>, #[doc(alias = "migrate_clk")] pub migrate_clock: bool, - /// The byte string that identifies the device. - device_id: DeviceId, } qom_isa!(PL011State : SysBusDevice, DeviceState, Object); +pub struct PL011Class { + parent_class: <SysBusDevice as ObjectType>::Class, + /// The byte string that identifies the device. + device_id: DeviceId, +} + unsafe impl ObjectType for PL011State { - type Class = <SysBusDevice as ObjectType>::Class; + type Class = PL011Class; const TYPE_NAME: &'static CStr = crate::TYPE_PL011; } +impl ClassInitImpl<PL011Class> for PL011State { + fn class_init(klass: &mut PL011Class) { + klass.device_id = DeviceId::ARM; + <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class); + } +} + impl ObjectImpl for PL011State { type ParentType = SysBusDevice; @@ -214,7 +218,8 @@ pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow<u let value = match RegisterOffset::try_from(offset) { Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => { - u32::from(self.device_id[(offset - 0xfe0) >> 2]) + let device_id = self.get_class().device_id; + u32::from(device_id[(offset - 0xfe0) >> 2]) } Err(_) => { // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset); @@ -648,16 +653,10 @@ pub struct PL011Luminary { parent_obj: ParentField<PL011State>, } -impl PL011Luminary { - /// Initializes a pre-allocated, unitialized instance of `PL011Luminary`. - /// - /// # Safety - /// - /// We expect the FFI user of this function to pass a valid pointer, that - /// has the same size as [`PL011Luminary`]. We also expect the device is - /// readable/writeable from one thread at any time. - unsafe fn init(&mut self) { - self.parent_obj.device_id = DeviceId::Luminary; +impl ClassInitImpl<PL011Class> for PL011Luminary { + fn class_init(klass: &mut PL011Class) { + klass.device_id = DeviceId::LUMINARY; + <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class); } } @@ -670,8 +669,6 @@ unsafe impl ObjectType for PL011Luminary { impl ObjectImpl for PL011Luminary { type ParentType = PL011State; - - const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init); } impl DeviceImpl for PL011Luminary {} -- 2.47.1