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(). Reviewed-by: Zhao Liu <zhao1....@intel.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- rust/hw/char/pl011/src/device.rs | 84 ++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index de5110038a5..9cac9d352a2 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -242,7 +242,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)); } @@ -266,7 +265,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 { @@ -281,9 +280,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(); @@ -307,7 +307,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(). @@ -318,15 +318,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; @@ -334,13 +335,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; @@ -350,14 +351,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 @@ -375,12 +374,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; } /* @@ -421,13 +421,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) { @@ -489,14 +487,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 == 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(); + } } } @@ -519,7 +520,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); @@ -532,8 +534,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) { @@ -565,7 +568,8 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> { } pub fn read(&mut self, offset: hwaddr, _size: u32) -> ControlFlow<u64, u64> { - match RegisterOffset::try_from(offset) { + let mut update_irq = false; + let result = match RegisterOffset::try_from(offset) { Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => { let device_id = self.get_class().device_id; ControlFlow::Break(u64::from(device_id[(offset - 0xfe0) >> 2])) @@ -574,22 +578,30 @@ 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) => { - let result = self.regs_read(field); - match result { - ControlFlow::Break(value) => ControlFlow::Break(value.into()), - ControlFlow::Continue(value) => ControlFlow::Continue(value.into()), + Ok(field) => match self.regs_read(field) { + ControlFlow::Break(value) => ControlFlow::Break(value.into()), + ControlFlow::Continue(value) => { + update_irq = true; + ControlFlow::Continue(value.into()) } - } + }, + }; + if update_irq { + self.update(); } + result } pub fn write(&mut self, offset: hwaddr, value: u64) { + let mut update_irq = false; if let Ok(field) = RegisterOffset::try_from(offset) { - self.regs_write(field, value as u32); + update_irq = self.regs_write(field, value as u32); } else { eprintln!("write bad offset {offset} value {value}"); } + if update_irq { + self.update(); + } } } -- 2.48.1