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