[PULL 057/153] rust/hpet: Make global register accessors as methods of HPETRegisters

Paolo Bonzini posted 153 patches 1 week, 4 days ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Yonggang Luo <luoyonggang@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Magnus Kulke <magnus.kulke@linux.microsoft.com>, Wei Liu <wei.liu@kernel.org>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, David Hildenbrand <david@kernel.org>, Igor Mammedov <imammedo@redhat.com>, Alistair Francis <alistair.francis@wdc.com>, Stefan Berger <stefanb@linux.vnet.ibm.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Aarushi Mehta <mehta.aaru20@gmail.com>, Julia Suvorova <jusual@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Stefan Weil <sw@weilnetz.de>, Samuel Thibault <samuel.thibault@ens-lyon.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Peter Maydell <peter.maydell@linaro.org>, Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Luc Michel <luc@lmichel.fr>, Damien Hedde <damien.hedde@dahe.fr>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Markus Armbruster <armbru@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Ani Sinha <anisinha@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Greg Kurz <groug@kaod.org>, "Michael S. Tsirkin" <mst@redhat.com>, Dongjiu Geng <gengdongjiu1@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Niek Linnenbank <nieklinnenbank@gmail.com>, Samuel Tardieu <sam@rfc1149.net>, Antony Pavlov <antonynpavlov@gmail.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Andrey Smirnov <andrew.smirnov@gmail.com>, Bernhard Beschow <shentey@gmail.com>, Rob Herring <robh@kernel.org>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Felipe Balbi <balbi@kernel.org>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <leif.lindholm@oss.qualcomm.com>, Eric Auger <eric.auger@redhat.com>, Alexandre Iooss <erdnaxe@crans.org>, Shannon Zhao <shannon.zhaosl@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Michael Rolnik <mrolnik@gmail.com>, Raphael Norwitz <raphael@enfabrica.net>, Helge Deller <deller@gmx.de>, "Clément Chigot" <chigot@adacore.com>, Frederic Konrad <konrad.frederic@yahoo.fr>, Alberto Garcia <berto@igalia.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Vijai Kumar K <vijai@behindbytes.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Collin L. Walling" <walling@linux.ibm.com>, Amit Shah <amit@kernel.org>, Yanan Wang <wangyanan55@huawei.com>, Riku Voipio <riku.voipio@iki.fi>, Paul Burton <paulburton@kernel.org>, Aleksandar Rikalo <arikalo@gmail.com>, Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, "Hervé Poussineau" <hpoussin@reactos.org>, BALATON Zoltan <balaton@eik.bme.hu>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Ninad Palsule <ninad@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, Corey Minyard <cminyard@mvista.com>, Patrick Leis <venture@google.com>, Alejandro Jimenez <alejandro.j.jimenez@oracle.com>, Sairaj Kodilkar <sarunkod@amd.com>, Jason Wang <jasowang@redhat.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, David Woodhouse <dwmw2@infradead.org>, Sergio Lopez <slp@redhat.com>, Alexander Graf <graf@amazon.com>, Dorjoy Chowdhury <dorjoychy111@gmail.com>, Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Huacai Chen <chenhuacai@kernel.org>, Jia Liu <proljc@gmail.com>, Gautam Menghani <gautam@linux.ibm.com>, Aditya Gupta <adityag@linux.ibm.com>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Gustavo Romero <gustavo.romero@linaro.org>, Francisco Iglesias <francisco.iglesias@amd.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Vikram Garhwal <vikram.garhwal@bytedance.com>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Max Filippov <jcmvbkbc@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Sven Schnelle <svens@stackframe.org>, Stafford Horne <shorne@gmail.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, Chinmay Rath <rathc@linux.ibm.com>, Sai Pavan Boddu <sai.pavan.boddu@amd.com>, Ran Wang <wangran@bosc.ac.cn>, Anup Patel <anup.patel@wdc.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Farhan Ali <alifm@linux.ibm.com>, Nina Schoetterl-Glausch <nsg@linux.ibm.com>, Jared Rossi <jrossi@linux.ibm.com>, Zhuoying Cai <zycai@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Hannes Reinecke <hare@suse.com>, Bin Meng <bmeng.cn@gmail.com>, Titus Rwantare <titusr@google.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Jeuk Kim <jeuk20.kim@samsung.com>, "Hongren (Zenithal) Zheng" <i@zenithal.me>, "Canokeys.org" <contact@canokeys.org>, John Levon <john.levon@nutanix.com>, Thanos Makatos <thanos.makatos@nutanix.com>, Alex Williamson <alex@shazbot.org>, Tomita Moeko <tomitamoeko@gmail.com>, Viresh Kumar <viresh.kumar@linaro.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Eric Blake <eblake@redhat.com>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>, Huai-Cheng Kuo <hchkuo@avery-design.com.tw>, Chris Browy <cbrowy@avery-design.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, "Dr. David Alan Gilbert" <dave@treblig.org>, Zhang Chen <zhangckid@gmail.com>, Li Zhijian <lizhijian@fujitsu.com>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Cleber Rosa <crosa@redhat.com>, Michael Roth <michael.roth@amd.com>, Maksim Davydov <davydov-max@yandex-team.ru>, David Gibson <david@gibson.dropbear.id.au>, Hyman Huang <yong.huang@smartx.com>, Brian Cain <brian.cain@oss.qualcomm.com>, Marcelo Tosatti <mtosatti@redhat.com>, Pedro Barbuda <pbarbuda@microsoft.com>, Mohamed Mediouni <mohamed@unpredictable.fr>, Coiby Xu <Coiby.Xu@gmail.com>
[PULL 057/153] rust/hpet: Make global register accessors as methods of HPETRegisters
Posted by Paolo Bonzini 1 week, 4 days ago
From: Zhao Liu <zhao1.liu@intel.com>

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 <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20251113051937.4017675-14-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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