Il sab 7 dic 2024, 16:38 Zhao Liu <zhao1....@intel.com> ha scritto: > Thanks for pointing that out, I really didn't think of that, I > understand how that would break the atomicity of the BQL lock, right? >
Yes, but also the function seems unnecessary. > impl fw_cfg_config { > > pub(crate) fn assign_hpet_id() -> usize { > > assert!(bql_locked()); > > // SAFETY: all accesses go through these methods, which guarantee > // that the accesses are protected by the BQL. > > let fw_cfg = unsafe { &mut *hpet_fw_cfg }; > > Nice idea! > > > if self.count == u8::MAX { > > // first instance > > fw_cfg.count = 0; > > } > > Will something like “anything that releases bql_lock” happen here? No, there are no function calls even. There seems to be no atomicity guarantee here. > It's not beautiful but it's guaranteed to be atomic. For the rare case of static mut, which is unsafe anyway, it makes sense. > > > if fw_cfg.count == 8 { > > // TODO: Add error binding: error_setg() > > panic!("Only 8 instances of HPET is allowed"); > > } > > > > let id: usize = fw_cfg.count.into(); > > fw_cfg.count += 1; > > id > > } > > } > > > > and you can assert bql_locked by hand instead of using the BqlCell. > > Thanks! I can also add a line of doc for bql_locked that it can be used > directly without BqlCell if necessary. > Good idea! And if you also agree the Phillipe's idea, I also need to add BqlCell > for fw_cfg field in HPETClass :-). > No, that also breaks compilation with CONFIG_HPET=n. The idea is nice but it doesn't work. ¯\_(ツ)_/¯ Paolo > Regards, > Zhao > > >