diff mbox series

[05/10] rust: pl011: pull interrupt updates out of read/write ops

Message ID 20250117092657.1051233-6-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series rust: pl011: correctly use interior mutability | expand

Commit Message

Paolo Bonzini Jan. 17, 2025, 9:26 a.m. UTC
qemu_irqs are not part of the vmstate, therefore they will remain in
PL011State.  Update them if needed after regs_read()/regs_write().

Apply #[must_use] to functions that return whether the interrupt state
could have changed, so that it's harder to forget the call to update().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 68 ++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 30 deletions(-)
diff mbox series

Patch

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 2e8707aef97..67c3e63baa1 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -234,7 +234,6 @@  fn regs_read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, u32> {
                 }
                 // Update error bits.
                 self.receive_status_error_clear.set_from_data(c);
-                self.update();
                 // Must call qemu_chr_fe_accept_input, so return Continue:
                 return ControlFlow::Continue(u32::from(c));
             },
@@ -258,7 +257,7 @@  fn regs_read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, u32> {
         })
     }
 
-    fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
+    fn regs_write(&mut self, offset: RegisterOffset, value: u32) -> bool {
         // eprintln!("write offset {offset} value {value}");
         use RegisterOffset::*;
         match offset {
@@ -273,9 +272,10 @@  fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
                 unsafe {
                     qemu_chr_fe_write_all(addr_of_mut!(self.char_backend), &ch, 1);
                 }
-                self.loopback_tx(value);
+                // interrupts always checked
+                let _ = self.loopback_tx(value);
                 self.int_level |= registers::INT_TX;
-                self.update();
+                return true;
             }
             RSR => {
                 self.receive_status_error_clear = 0.into();
@@ -299,7 +299,7 @@  fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
                     self.reset_rx_fifo();
                     self.reset_tx_fifo();
                 }
-                if self.line_control.send_break() ^ new_val.send_break() {
+                let update = (self.line_control.send_break() != new_val.send_break()) && {
                     let mut break_enable: c_int = new_val.send_break().into();
                     // SAFETY: self.char_backend is a valid CharBackend instance after it's been
                     // initialized in realize().
@@ -310,15 +310,16 @@  fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
                             addr_of_mut!(break_enable).cast::<c_void>(),
                         );
                     }
-                    self.loopback_break(break_enable > 0);
-                }
+                    self.loopback_break(break_enable > 0)
+                };
                 self.line_control = new_val;
                 self.set_read_trigger();
+                return update;
             }
             CR => {
                 // ??? Need to implement the enable bit.
                 self.control = value.into();
-                self.loopback_mdmctrl();
+                return self.loopback_mdmctrl();
             }
             FLS => {
                 self.ifl = value;
@@ -326,13 +327,13 @@  fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
             }
             IMSC => {
                 self.int_enabled = value;
-                self.update();
+                return true;
             }
             RIS => {}
             MIS => {}
             ICR => {
                 self.int_level &= !value;
-                self.update();
+                return true;
             }
             DMACR => {
                 self.dmacr = value;
@@ -342,14 +343,12 @@  fn regs_write(&mut self, offset: RegisterOffset, value: u32) {
                 }
             }
         }
+        false
     }
 
     #[inline]
-    fn loopback_tx(&mut self, value: u32) {
-        if !self.loopback_enabled() {
-            return;
-        }
-
+    #[must_use]
+    fn loopback_tx(&mut self, value: u32) -> bool {
         // Caveat:
         //
         // In real hardware, TX loopback happens at the serial-bit level
@@ -367,12 +366,13 @@  fn loopback_tx(&mut self, value: u32) {
         // hardware flow-control is enabled.
         //
         // For simplicity, the above described is not emulated.
-        self.put_fifo(value);
+        self.loopback_enabled() && self.put_fifo(value)
     }
 
-    fn loopback_mdmctrl(&mut self) {
+    #[must_use]
+    fn loopback_mdmctrl(&mut self) -> bool {
         if !self.loopback_enabled() {
-            return;
+            return false;
         }
 
         /*
@@ -413,13 +413,11 @@  fn loopback_mdmctrl(&mut self) {
             il |= Interrupt::RI as u32;
         }
         self.int_level = il;
-        self.update();
+        true
     }
 
-    fn loopback_break(&mut self, enable: bool) {
-        if enable {
-            self.loopback_tx(registers::Data::BREAK.into());
-        }
+    fn loopback_break(&mut self, enable: bool) -> bool {
+        enable && self.loopback_tx(registers::Data::BREAK.into())
     }
 
     fn set_read_trigger(&mut self) {
@@ -481,14 +479,17 @@  pub fn can_receive(&self) -> bool {
     }
 
     pub fn receive(&mut self, ch: u32) {
-        if !self.loopback_enabled() {
-            self.put_fifo(ch)
+        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() {
-            self.put_fifo(registers::Data::BREAK.into());
+            let update = self.put_fifo(registers::Data::BREAK.into());
+            if update {
+                self.update();
+            }
         }
     }
 
@@ -511,7 +512,8 @@  pub fn fifo_depth(&self) -> u32 {
         1
     }
 
-    pub fn put_fifo(&mut self, value: u32) {
+    #[must_use]
+    pub fn put_fifo(&mut self, value: u32) -> bool {
         let depth = self.fifo_depth();
         assert!(depth > 0);
         let slot = (self.read_pos + self.read_count) & (depth - 1);
@@ -524,8 +526,9 @@  pub fn put_fifo(&mut self, value: u32) {
 
         if self.read_count == self.read_trigger {
             self.int_level |= registers::INT_RX;
-            self.update();
+            return true;
         }
+        false
     }
 
     pub fn update(&self) {
@@ -568,14 +571,19 @@  pub fn read(&mut self, offset: hwaddr, _size: u32) -> ControlFlow<u64, u64> {
             }
             Ok(field) => match self.regs_read(field) {
                 ControlFlow::Break(value) => ControlFlow::Break(value.into()),
-                ControlFlow::Continue(value) => ControlFlow::Continue(value.into()),
+                ControlFlow::Continue(value) => {
+                    self.update();
+                    ControlFlow::Continue(value.into())
+                },
             }
         }
     }
 
     pub fn write(&mut self, offset: hwaddr, value: u64) {
         if let Ok(field) = RegisterOffset::try_from(offset) {
-           self.regs_write(field, value as u32);
+            if self.regs_write(field, value as u32) {
+                self.update();
+            }
         } else {
             eprintln!("write bad offset {offset} value {value}");
         }