Il sab 7 dic 2024, 16:38 Zhao Liu <[email protected]> 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
>
>
>
