qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 06/10] rust: pl011: extract PL011Registers


From: Paolo Bonzini
Subject: [PATCH 06/10] rust: pl011: extract PL011Registers
Date: Fri, 17 Jan 2025 10:26:53 +0100

Pull all the mutable fields of PL011State into a separate struct.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       | 251 ++++++++++++++-----------
 rust/hw/char/pl011/src/device_class.rs |  46 +++--
 2 files changed, 168 insertions(+), 129 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 67c3e63baa1..476abe765a9 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -78,11 +78,8 @@ fn index(&self, idx: u32) -> &Self::Output {
 }
 
 #[repr(C)]
-#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)]
-/// PL011 Device Model in QEMU
-pub struct PL011State {
-    pub parent_obj: ParentField<SysBusDevice>,
-    pub iomem: MemoryRegion,
+#[derive(Debug, Default, qemu_api_macros::offsets)]
+pub struct PL011Registers {
     #[doc(alias = "fr")]
     pub flags: registers::Flags,
     #[doc(alias = "lcr")]
@@ -102,8 +99,17 @@ pub struct PL011State {
     pub read_pos: u32,
     pub read_count: u32,
     pub read_trigger: u32,
+}
+
+#[repr(C)]
+#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)]
+/// PL011 Device Model in QEMU
+pub struct PL011State {
+    pub parent_obj: ParentField<SysBusDevice>,
+    pub iomem: MemoryRegion,
     #[doc(alias = "chr")]
     pub char_backend: CharBackend,
+    pub regs: PL011Registers,
     /// QEMU interrupts
     ///
     /// ```text
@@ -161,61 +167,8 @@ fn vmsd() -> Option<&'static VMStateDescription> {
     const RESET: Option<fn(&mut Self)> = Some(Self::reset);
 }
 
-impl PL011State {
-    /// Initializes a pre-allocated, unitialized instance of `PL011State`.
-    ///
-    /// # Safety
-    ///
-    /// `self` must point to a correctly sized and aligned location for the
-    /// `PL011State` type. It must not be called more than once on the same
-    /// location/instance. All its fields are expected to hold unitialized
-    /// values with the sole exception of `parent_obj`.
-    unsafe fn init(&mut self) {
-        const CLK_NAME: &CStr = c_str!("clk");
-
-        // 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::<Object>(),
-                &PL011_OPS,
-                addr_of_mut!(*self).cast::<c_void>(),
-                Self::TYPE_NAME.as_ptr(),
-                0x1000,
-            );
-        }
-
-        // SAFETY:
-        //
-        // self.clock is not initialized at this point; but since `NonNull<_>` 
is Copy,
-        // we can overwrite the undefined value without side effects. This is
-        // safe since all PL011State instances are created by QOM code which
-        // calls this function to initialize the fields; therefore no code is
-        // able to access an invalid self.clock value.
-        unsafe {
-            let dev: &mut DeviceState = self.upcast_mut();
-            self.clock = NonNull::new(qdev_init_clock_in(
-                dev,
-                CLK_NAME.as_ptr(),
-                None, /* pl011_clock_update */
-                addr_of_mut!(*self).cast::<c_void>(),
-                ClockEvent::ClockUpdate.0,
-            ))
-            .unwrap();
-        }
-    }
-
-    fn post_init(&self) {
-        self.init_mmio(&self.iomem);
-        for irq in self.interrupts.iter() {
-            self.init_irq(irq);
-        }
-    }
-
-    fn regs_read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, u32> {
+impl PL011Registers {
+    pub(self) fn read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, 
u32> {
         use RegisterOffset::*;
 
         std::ops::ControlFlow::Break(match offset {
@@ -257,7 +210,12 @@ fn regs_read(&mut self, offset: RegisterOffset) -> 
ControlFlow<u32, u32> {
         })
     }
 
-    fn regs_write(&mut self, offset: RegisterOffset, value: u32) -> bool {
+    pub(self) fn write(
+        &mut self,
+        offset: RegisterOffset,
+        value: u32,
+        char_backend: *mut CharBackend,
+    ) -> bool {
         // eprintln!("write offset {offset} value {value}");
         use RegisterOffset::*;
         match offset {
@@ -267,10 +225,10 @@ fn regs_write(&mut self, offset: RegisterOffset, value: 
u32) -> bool {
                 // XXX this blocks entire thread. Rewrite to use
                 // qemu_chr_fe_write and background I/O callbacks
 
-                // SAFETY: self.char_backend is a valid CharBackend instance 
after it's been
+                // SAFETY: char_backend is a valid CharBackend instance after 
it's been
                 // initialized in realize().
                 unsafe {
-                    qemu_chr_fe_write_all(addr_of_mut!(self.char_backend), 
&ch, 1);
+                    qemu_chr_fe_write_all(char_backend, &ch, 1);
                 }
                 // interrupts always checked
                 let _ = self.loopback_tx(value);
@@ -305,7 +263,7 @@ fn regs_write(&mut self, offset: RegisterOffset, value: 
u32) -> bool {
                     // initialized in realize().
                     unsafe {
                         qemu_chr_fe_ioctl(
-                            addr_of_mut!(self.char_backend),
+                            char_backend,
                             CHR_IOCTL_SERIAL_SET_BREAK as i32,
                             addr_of_mut!(break_enable).cast::<c_void>(),
                         );
@@ -424,23 +382,6 @@ fn set_read_trigger(&mut self) {
         self.read_trigger = 1;
     }
 
-    pub fn realize(&mut self) {
-        // SAFETY: self.char_backend has the correct size and alignment for a
-        // CharBackend object, and its callbacks are of the correct types.
-        unsafe {
-            qemu_chr_fe_set_handlers(
-                addr_of_mut!(self.char_backend),
-                Some(pl011_can_receive),
-                Some(pl011_receive),
-                Some(pl011_event),
-                None,
-                addr_of_mut!(*self).cast::<c_void>(),
-                core::ptr::null_mut(),
-                true,
-            );
-        }
-    }
-
     pub fn reset(&mut self) {
         self.line_control.reset();
         self.receive_status_error_clear.reset();
@@ -473,26 +414,6 @@ pub fn reset_tx_fifo(&mut self) {
         self.flags.set_transmit_fifo_empty(true);
     }
 
-    pub fn can_receive(&self) -> bool {
-        // trace_pl011_can_receive(s->lcr, s->read_count, r);
-        self.read_count < self.fifo_depth()
-    }
-
-    pub fn receive(&mut self, ch: u32) {
-        if !self.loopback_enabled() && self.put_fifo(ch) {
-            self.update();
-        }
-    }
-
-    pub fn event(&mut self, event: QEMUChrEvent) {
-        if event == bindings::QEMUChrEvent::CHR_EVENT_BREAK && 
!self.loopback_enabled() {
-            let update = self.put_fifo(registers::Data::BREAK.into());
-            if update {
-                self.update();
-            }
-        }
-    }
-
     #[inline]
     pub fn fifo_enabled(&self) -> bool {
         self.line_control.fifos_enabled() == registers::Mode::FIFO
@@ -531,14 +452,7 @@ pub fn put_fifo(&mut self, value: u32) -> bool {
         false
     }
 
-    pub fn update(&self) {
-        let flags = self.int_level & self.int_enabled;
-        for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
-            irq.set(flags & i != 0);
-        }
-    }
-
-    pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
+    pub fn post_load(&mut self) -> Result<(), ()> {
         /* Sanity-check input state */
         if self.read_pos >= self.read_fifo.len() || self.read_count > 
self.read_fifo.len() {
             return Err(());
@@ -558,8 +472,66 @@ pub fn post_load(&mut self, _version_id: u32) -> 
Result<(), ()> {
 
         Ok(())
     }
+}
+
+impl PL011State {
+    /// Initializes a pre-allocated, unitialized instance of `PL011State`.
+    ///
+    /// # Safety
+    ///
+    /// `self` must point to a correctly sized and aligned location for the
+    /// `PL011State` type. It must not be called more than once on the same
+    /// location/instance. All its fields are expected to hold unitialized
+    /// values with the sole exception of `parent_obj`.
+    unsafe fn init(&mut self) {
+        const CLK_NAME: &CStr = c_str!("clk");
+
+        // 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::<Object>(),
+                &PL011_OPS,
+                addr_of_mut!(*self).cast::<c_void>(),
+                Self::TYPE_NAME.as_ptr(),
+                0x1000,
+            );
+        }
+
+        self.regs = Default::default();
+
+        // SAFETY:
+        //
+        // self.clock is not initialized at this point; but since `NonNull<_>` 
is Copy,
+        // we can overwrite the undefined value without side effects. This is
+        // safe since all PL011State instances are created by QOM code which
+        // calls this function to initialize the fields; therefore no code is
+        // able to access an invalid self.clock value.
+        unsafe {
+            let dev: &mut DeviceState = self.upcast_mut();
+            self.clock = NonNull::new(qdev_init_clock_in(
+                dev,
+                CLK_NAME.as_ptr(),
+                None, /* pl011_clock_update */
+                addr_of_mut!(*self).cast::<c_void>(),
+                ClockEvent::ClockUpdate.0,
+            ))
+            .unwrap();
+        }
+    }
+
+    fn post_init(&self) {
+        self.init_mmio(&self.iomem);
+        for irq in self.interrupts.iter() {
+            self.init_irq(irq);
+        }
+    }
 
     pub fn read(&mut self, offset: hwaddr, _size: u32) -> ControlFlow<u64, 
u64> {
+        let regs = &mut self.regs;
         match RegisterOffset::try_from(offset) {
             Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => {
                 let device_id = self.get_class().device_id;
@@ -569,7 +541,7 @@ pub fn read(&mut self, offset: hwaddr, _size: u32) -> 
ControlFlow<u64, u64> {
                 // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 
0x%x\n", (int)offset);
                 ControlFlow::Break(0)
             }
-            Ok(field) => match self.regs_read(field) {
+            Ok(field) => match regs.read(field) {
                 ControlFlow::Break(value) => ControlFlow::Break(value.into()),
                 ControlFlow::Continue(value) => {
                     self.update();
@@ -580,14 +552,71 @@ pub fn read(&mut self, offset: hwaddr, _size: u32) -> 
ControlFlow<u64, u64> {
     }
 
     pub fn write(&mut self, offset: hwaddr, value: u64) {
+        let regs = &mut self.regs;
         if let Ok(field) = RegisterOffset::try_from(offset) {
-            if self.regs_write(field, value as u32) {
+            if regs.write(field, value as u32, &mut self.char_backend) {
                 self.update();
             }
         } else {
             eprintln!("write bad offset {offset} value {value}");
         }
     }
+
+    pub fn can_receive(&self) -> bool {
+        // trace_pl011_can_receive(s->lcr, s->read_count, r);
+        let regs = &self.regs;
+        regs.read_count < regs.fifo_depth()
+    }
+
+    pub fn receive(&mut self, ch: u32) {
+        let regs = &mut self.regs;
+        if !regs.loopback_enabled() && regs.put_fifo(ch) {
+            self.update();
+        }
+    }
+
+    pub fn event(&mut self, event: QEMUChrEvent) {
+        let regs = &mut self.regs;
+        if event == bindings::QEMUChrEvent::CHR_EVENT_BREAK && 
!regs.loopback_enabled() {
+            let update = regs.put_fifo(registers::Data::BREAK.into());
+            if update {
+                self.update()
+            }
+        }
+    }
+
+    pub fn realize(&mut self) {
+        // SAFETY: self.char_backend has the correct size and alignment for a
+        // CharBackend object, and its callbacks are of the correct types.
+        unsafe {
+            qemu_chr_fe_set_handlers(
+                addr_of_mut!(self.char_backend),
+                Some(pl011_can_receive),
+                Some(pl011_receive),
+                Some(pl011_event),
+                None,
+                addr_of_mut!(*self).cast::<c_void>(),
+                core::ptr::null_mut(),
+                true,
+            );
+        }
+    }
+
+    pub fn reset(&mut self) {
+        self.regs.reset();
+    }
+
+    pub fn update(&self) {
+        let regs = &self.regs;
+        let flags = regs.int_level & regs.int_enabled;
+        for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
+            irq.set(flags & i != 0);
+        }
+    }
+
+    pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
+        self.regs.post_load()
+    }
 }
 
 /// Which bits in the interrupt status matter for each outbound IRQ line ?
diff --git a/rust/hw/char/pl011/src/device_class.rs 
b/rust/hw/char/pl011/src/device_class.rs
index 2fd805fd12d..e25645ceb0d 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -6,11 +6,11 @@
 use std::os::raw::{c_int, c_void};
 
 use qemu_api::{
-    bindings::*, c_str, vmstate_clock, vmstate_fields, vmstate_of, 
vmstate_subsections,
-    vmstate_unused, zeroable::Zeroable,
+    bindings::*, c_str, vmstate_clock, vmstate_fields, vmstate_of, 
vmstate_struct,
+    vmstate_subsections, vmstate_unused, zeroable::Zeroable,
 };
 
-use crate::device::PL011State;
+use crate::device::{PL011Registers, PL011State};
 
 extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
     unsafe {
@@ -45,6 +45,30 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, 
version_id: c_int) -> c_int {
     }
 }
 
+static VMSTATE_PL011_REGS: VMStateDescription = VMStateDescription {
+    name: c_str!("pl011").as_ptr(),
+    version_id: 2,
+    minimum_version_id: 2,
+    fields: vmstate_fields! {
+        vmstate_of!(PL011Registers, flags),
+        vmstate_of!(PL011Registers, line_control),
+        vmstate_of!(PL011Registers, receive_status_error_clear),
+        vmstate_of!(PL011Registers, control),
+        vmstate_of!(PL011Registers, dmacr),
+        vmstate_of!(PL011Registers, int_enabled),
+        vmstate_of!(PL011Registers, int_level),
+        vmstate_of!(PL011Registers, read_fifo),
+        vmstate_of!(PL011Registers, ilpr),
+        vmstate_of!(PL011Registers, ibrd),
+        vmstate_of!(PL011Registers, fbrd),
+        vmstate_of!(PL011Registers, ifl),
+        vmstate_of!(PL011Registers, read_pos),
+        vmstate_of!(PL011Registers, read_count),
+        vmstate_of!(PL011Registers, read_trigger),
+    },
+    ..Zeroable::ZERO
+};
+
 pub static VMSTATE_PL011: VMStateDescription = VMStateDescription {
     name: c_str!("pl011").as_ptr(),
     version_id: 2,
@@ -52,21 +76,7 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, 
version_id: c_int) -> c_int {
     post_load: Some(pl011_post_load),
     fields: vmstate_fields! {
         vmstate_unused!(core::mem::size_of::<u32>()),
-        vmstate_of!(PL011State, flags),
-        vmstate_of!(PL011State, line_control),
-        vmstate_of!(PL011State, receive_status_error_clear),
-        vmstate_of!(PL011State, control),
-        vmstate_of!(PL011State, dmacr),
-        vmstate_of!(PL011State, int_enabled),
-        vmstate_of!(PL011State, int_level),
-        vmstate_of!(PL011State, read_fifo),
-        vmstate_of!(PL011State, ilpr),
-        vmstate_of!(PL011State, ibrd),
-        vmstate_of!(PL011State, fbrd),
-        vmstate_of!(PL011State, ifl),
-        vmstate_of!(PL011State, read_pos),
-        vmstate_of!(PL011State, read_count),
-        vmstate_of!(PL011State, read_trigger),
+        vmstate_struct!(PL011State, regs, &VMSTATE_PL011_REGS, PL011Registers),
     },
     subsections: vmstate_subsections! {
         VMSTATE_PL011_CLOCK
-- 
2.47.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]