On Wed, 19 Mar 2025 at 20:51, Paolo Bonzini <pbonz...@redhat.com> wrote:
>
> On 3/19/25 20:25, Peter Maydell wrote:
> > Hi -- this commit seems to have broken use of the PL011 in
> > boards/SoCs that directly embed it in their state structs, so
> > "qemu-system-arm -M raspi2b -display none" now asserts on startup.
> >
> > The Rust PL011's state struct size is now larger than the
> > C state struct size, so it trips the assert in the QOM code
> > that we didn't try to initialize a type into less memory than
> > it needs.
> The reason why it changes is that it switches the imported symbol from
> bindings::CharBackend (the C struct) to chardev::CharBackend which has
> two extra values in it (a count and some debugging info to provide
> better backtraces on error). It is guaranteed to _start_ with a
> bindings::CharBackend, which is helpful for the qdev property, but it's
> bigger.
>
> I don't think there's a good fix other than not using an embedded PL011,
> since you probably would not have that option available when using a
> device without a Rust equivalent.
(do you mean "without a C equivalent" there?)
Hmm. The embedded-struct approach to devices is extremely common,
especially in more recent QEMU C code. Generally those users of
an embedded struct don't actually poke around inside the struct,
so we don't need to have the Rust code match the C layout, but
we do at least need the size to be no bigger than the C version.
(There are some exceptions for some devices, not including the PL011,
where people do poke around inside the struct of a device they've
created...)
There has been discussion that we ought to move (back!) to a
"users of the device just get a pointer to it and the struct
is opaque to them" design pattern -- command line generation
and wiring up of machines would also make that a more natural
approach -- but that's a long term process we'd need to plan
and go through.
I had some ideas a long time ago for making "code outside the
C device implementation touched a field in the device struct"
be a compile error using attribute("deprecated") hidden via macros,
so we could resurrect that as a way to confirm that nobody is
trying to do dubious things before we do a C-to-rust conversion
of a device model.
For the moment, I guess the expedient thing to do would be to add
an extra 16 bytes of padding to the C PL011State struct ?
thanks
-- PMM
