qemu-rust
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] rust: add BQL-enforcing Cell variant


From: Paolo Bonzini
Subject: Re: [PATCH 1/2] rust: add BQL-enforcing Cell variant
Date: Wed, 27 Nov 2024 09:31:28 +0100

Il mer 27 nov 2024, 07:17 Zhao Liu <zhao1.liu@intel.com> ha scritto:
> and a single "regs: BqlRefCell<PL011Registers>" in PL011State.

Here I have a possibly common question about the choice of BqlCell and
future BqlRefCell. Pls refer my comment below...

I understand we need BqlRefCell instead of BqlCell for registers since
there may be more than one reference, e.g., callbacks from CharBackend
and MemoryRegion may hold multiple references at the same time, right?

If right, like HPET, with only MemoryRegion callbacks, reads and writes
I guess which should not be able to happen at the same time, so BqlCell
is also feasible, as is Irq?

Choose between BqlCell/BqlRefCell like you would choose between Cell/RefCell. They result in different code so pick what looks better.

That said for HPET you have the array of timers structs, and these structs are not Copy, and therefore BqlRefCell is almost the only choice for that array. With BqlRefCell used for the timers you might as well put the other registers in the same BqlRefCell.

(This piece of the thread model is a bit more complex, and I'm fully
aware that the right TYPE relies a lot on understanding the underlying
mechanism, which I'm not yet familiar enough with :-) ).

It's not too complex—the point is to make it as similar as possible to normal single threaded Rust.

However, all in all, using BqlRefCell for register fields looks to be
always better than BqlCell.

Yep :)

Thank you! I'll change the current code to support this. Instead of
implementing a register space like PL011, I continue to handle registers
with u64 variables and bit macro definitions like the C version

That's fine. Experimenting in different directions makes it easier for future developers to evaluate the tradeoffs.

Also related to the above question, I'm a bit hesitant to use BqlCell directly
or RefCell for such u64 fields...

Do whatever looks best to you!

> Plus, speaking in general, "it does something in a different way than the
> pl011 device model" is a good reason to merge the HPET model earlier too. :)

There must be a lot more opens :-(, such as the memattrs/timer binding,
which I hope to discuss with you at the later RFC!

Yes, but stuff like "correctly uses interior mutability" is a very good reason to include the HPET early.

I will send the next version and include BqlRefCell once I incorporate Junjie's feedback, in the meanwhile I will send you the BqlRefCell patch off list.

Paolo 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]