From: Zhao Liu <[email protected]> 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<HPETRegisters> 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 <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Paolo Bonzini <[email protected]> --- rust/hw/timer/hpet/src/device.rs | 60 ++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs index bf9f4936718..da938f356e9 100644 --- a/rust/hw/timer/hpet/src/device.rs +++ b/rust/hw/timer/hpet/src/device.rs @@ -280,7 +280,10 @@ fn get_state(&self) -> &HPETState { } 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 @@ fn calculate_cmp64(&self, cur_tick: u64, target: u64) -> u64 { } 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 @@ fn get_int_route(&self) -> usize { 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 @@ fn set_tn_cfg_reg(&mut self, shift: u32, len: u32, val: u64) { 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 @@ fn set_tn_cmp_reg(&mut self, shift: u32, len: u32, val: u64) { } 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 @@ const fn has_msi_flag(&self) -> bool { 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 @@ fn get_ns(&self, tick: u64) -> u64 { } 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 @@ fn set_int_status_reg(&self, shift: u32, _len: u32, val: u64) { /// 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 @@ fn set_counter_reg(&self, shift: u32, len: u32, val: u64) { // 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 @@ fn read(&self, addr: hwaddr, size: u32) -> u64 { 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 @@ fn write(&self, addr: hwaddr, value: u64, size: u32) { } 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 @@ fn is_rtc_irq_level_needed(&self) -> bool { } 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 { -- 2.52.0
