From: Zhao Liu Date: Thu, 13 Nov 2025 05:19:28 +0000 (+0800) Subject: rust/hpet: Make global register accessors as methods of HPETRegisters X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2abee243d6abc114aadfbdf97f18f653d3de2464;p=thirdparty%2Fqemu.git rust/hpet: Make global register accessors as methods of HPETRegisters Implement helper accessors as methods of HPETRegisters. Then HPETRegisters can be accessed without going through HPETState. In subsequent refactoring, coarser-grained BQL lock protection will be implemented. Specifically, BqlRefCell will be borrowed only once during MMIO accesses, and the scope of borrowed `regs` will be extended to cover the entire MMIO access. Consequently, repeated borrow() attempts within function calls will no longer be allowed. Therefore, refactor the accessors of HPETRegisters to bypass HPETState, which help to reduce borrow() in deep function calls. Signed-off-by: Zhao Liu Link: https://lore.kernel.org/r/20251113051937.4017675-14-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs index bf9f493671..da938f356e 100644 --- a/rust/hw/timer/hpet/src/device.rs +++ b/rust/hw/timer/hpet/src/device.rs @@ -280,7 +280,10 @@ impl HPETTimer { } fn is_int_active(&self) -> bool { - self.get_state().is_timer_int_active(self.index.into()) + self.get_state() + .regs + .borrow() + .is_timer_int_active(self.index.into()) } /// calculate next value of the general counter that matches the @@ -299,7 +302,7 @@ impl HPETTimer { } fn get_int_route(&self) -> usize { - if self.index <= 1 && self.get_state().is_legacy_mode() { + if self.index <= 1 && self.get_state().regs.borrow().is_legacy_mode() { // If LegacyReplacement Route bit is set, HPET specification requires // timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC, // timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. @@ -326,7 +329,7 @@ impl HPETTimer { fn set_irq(&self, set: bool) { let route = self.get_int_route(); - if set && self.regs.is_int_enabled() && self.get_state().is_hpet_enabled() { + if set && self.regs.is_int_enabled() && self.get_state().regs.borrow().is_hpet_enabled() { if self.regs.is_fsb_route_enabled() { // SAFETY: // the parameters are valid. @@ -425,7 +428,7 @@ impl HPETTimer { self.period = u64::from(self.period as u32); // truncate! } - if self.get_state().is_hpet_enabled() { + if self.get_state().regs.borrow().is_hpet_enabled() { self.set_timer(); } } @@ -456,7 +459,7 @@ impl HPETTimer { } self.regs.clear_valset(); - if self.get_state().is_hpet_enabled() { + if self.get_state().regs.borrow().is_hpet_enabled() { self.set_timer(); } } @@ -540,6 +543,20 @@ pub struct HPETRegisters { counter: u64, } +impl HPETRegisters { + fn is_legacy_mode(&self) -> bool { + self.config & (1 << HPET_CFG_LEG_RT_SHIFT) != 0 + } + + fn is_hpet_enabled(&self) -> bool { + self.config & (1 << HPET_CFG_ENABLE_SHIFT) != 0 + } + + fn is_timer_int_active(&self, index: usize) -> bool { + self.int_status & (1 << index) != 0 + } +} + /// HPET Event Timer Block Abstraction #[repr(C)] #[derive(qom::Object, hwcore::Device)] @@ -587,18 +604,6 @@ impl HPETState { self.flags & (1 << HPET_FLAG_MSI_SUPPORT_SHIFT) != 0 } - fn is_legacy_mode(&self) -> bool { - self.regs.borrow().config & (1 << HPET_CFG_LEG_RT_SHIFT) != 0 - } - - fn is_hpet_enabled(&self) -> bool { - self.regs.borrow().config & (1 << HPET_CFG_ENABLE_SHIFT) != 0 - } - - fn is_timer_int_active(&self, index: usize) -> bool { - self.regs.borrow().int_status & (1 << index) != 0 - } - fn get_ticks(&self) -> u64 { ns_to_ticks(CLOCK_VIRTUAL.get_ns() + self.hpet_offset.get()) } @@ -608,13 +613,14 @@ impl HPETState { } fn handle_legacy_irq(&self, irq: u32, level: u32) { + let regs = self.regs.borrow(); if irq == HPET_LEGACY_PIT_INT { - if !self.is_legacy_mode() { + if !regs.is_legacy_mode() { self.irqs[0].set(level != 0); } } else { self.rtc_irq_level.set(level); - if !self.is_legacy_mode() { + if !regs.is_legacy_mode() { self.irqs[RTC_ISA_IRQ].set(level != 0); } } @@ -697,7 +703,8 @@ impl HPETState { /// Main Counter Value Register fn set_counter_reg(&self, shift: u32, len: u32, val: u64) { - if self.is_hpet_enabled() { + let mut regs = self.regs.borrow_mut(); + if regs.is_hpet_enabled() { // HPET spec says that writes to this register should only be // done while the counter is halted. So this is an undefined // behavior. There's no need to forbid it, but when HPET is @@ -706,7 +713,6 @@ impl HPETState { // not be changed as well). trace::trace_hpet_ram_write_counter_write_while_enabled(); } - let mut regs = self.regs.borrow_mut(); regs.counter = regs.counter.deposit(shift, len, val); } @@ -825,10 +831,11 @@ impl HPETState { Global(CFG) => self.regs.borrow().config, Global(INT_STATUS) => self.regs.borrow().int_status, Global(COUNTER) => { - let cur_tick = if self.is_hpet_enabled() { + let regs = self.regs.borrow(); + let cur_tick = if regs.is_hpet_enabled() { self.get_ticks() } else { - self.regs.borrow().counter + regs.counter }; trace::trace_hpet_ram_read_reading_counter((addr & 4) as u8, cur_tick); @@ -860,8 +867,9 @@ impl HPETState { } fn pre_save(&self) -> Result<(), migration::Infallible> { - if self.is_hpet_enabled() { - self.regs.borrow_mut().counter = self.get_ticks(); + let mut regs = self.regs.borrow_mut(); + if regs.is_hpet_enabled() { + regs.counter = self.get_ticks(); } /* @@ -896,7 +904,7 @@ impl HPETState { } fn is_offset_needed(&self) -> bool { - self.is_hpet_enabled() && self.hpet_offset_saved + self.regs.borrow().is_hpet_enabled() && self.hpet_offset_saved } fn validate_num_timers(&self, _version_id: u8) -> bool {