[Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Peter Maydell]

[I've cc'd a fairly random selection of people who I thought
might be interested or have an opinion.]

It's fairly common to have a setup where we have a QOM container
object (like an SoC) which in turn instantiates a lot of child
objects (for all the devices). The neat way of doing this looks
like hw/arm/xlnx-zynqmp.c -- in the container's init function,
we use object_initialize() to init all the child objects. If the
container exposes some properties that are really just being
forwarded to one of its children it can set those up in init
with object_property_add_alias(). Finally, in realize the container
realizes all its children.

Unfortunately, this pattern interacts badly with the idea that
you might want to use a QOM property to determine aspects of
the container that affect what child objects it creates.
(Examples would include wanting a "which CPU is this" property
on an SoC object, or if the SoC has a couple of variants which
maybe have extra devices.)

One current approach to that is that instead of init'ing those
child objects in the container init, we postpone that to
container realize. This looks pretty ugly, and it also means
that you can't do "forward this property" using add_alias if the
target is the late-inited child (instead you have to have a
real property on the container and set the property on the child
manually after it's inited). You can see an example of this kind
of thing in hw/arm/armv7m.c.

Another approach is that instead of having a "what CPU" or "what
SoC variant" property on the container, we create one container
type per variation. Then instead of "create container, set QOM
property to specify variant" the user creates the correct container
type for the variant. hw/arm/aspeed_soc.c has an example of this.
That looks pretty nice code-wise, but if there are a lot of
possible options for the variants it could result in a large
number of QOM types.

The other popular approach to this is "don't let the container
be as configurable as it ideally ought to be"...

So, does anybody have a view on what the best way to structure
this kind of container object is? I feel like I'm running into
the annoyances of the approach that armv7m.c is currently taking,
so if there's a better way I'd like to do that instead.

thanks
-- PMM

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Eduardo Habkost]

On Tue, Feb 06, 2018 at 06:18:25PM +0000, Peter Maydell wrote:
> [I've cc'd a fairly random selection of people who I thought
> might be interested or have an opinion.]
> 
> It's fairly common to have a setup where we have a QOM container
> object (like an SoC) which in turn instantiates a lot of child
> objects (for all the devices). The neat way of doing this looks
> like hw/arm/xlnx-zynqmp.c -- in the container's init function,
> we use object_initialize() to init all the child objects. If the
> container exposes some properties that are really just being
> forwarded to one of its children it can set those up in init
> with object_property_add_alias(). Finally, in realize the container
> realizes all its children.
> 
> Unfortunately, this pattern interacts badly with the idea that
> you might want to use a QOM property to determine aspects of
> the container that affect what child objects it creates.
> (Examples would include wanting a "which CPU is this" property
> on an SoC object, or if the SoC has a couple of variants which
> maybe have extra devices.)
> 
> One current approach to that is that instead of init'ing those
> child objects in the container init, we postpone that to
> container realize. This looks pretty ugly, and it also means
> that you can't do "forward this property" using add_alias if the
> target is the late-inited child (instead you have to have a
> real property on the container and set the property on the child
> manually after it's inited). You can see an example of this kind
> of thing in hw/arm/armv7m.c.

My rule of thumb is: if something is configurable (even if it's
just slightly), it belongs to realize.  instance_init is reserved
for stuff that don't take any external input.  If your container
contents are not static, creating the contents is not a task for
instance_init.

But I would like to understand the drawbacks of this approach
better.  So, if the object didn't have any "forward this
property" aliases, would you see other problems with this
approach?

Why exactly those boards need the aliases?  Who sets those alias
properties?  Can we provide helpers that make this task easier?


> 
> Another approach is that instead of having a "what CPU" or "what
> SoC variant" property on the container, we create one container
> type per variation. Then instead of "create container, set QOM
> property to specify variant" the user creates the correct container
> type for the variant. hw/arm/aspeed_soc.c has an example of this.
> That looks pretty nice code-wise, but if there are a lot of
> possible options for the variants it could result in a large
> number of QOM types.

Yeah, this may be reasonable in some cases (e.g. some CPU types),
but it won't always work.

> 
> The other popular approach to this is "don't let the container
> be as configurable as it ideally ought to be"...

I like this approach.  :)

> 
> So, does anybody have a view on what the best way to structure
> this kind of container object is? I feel like I'm running into
> the annoyances of the approach that armv7m.c is currently taking,
> so if there's a better way I'd like to do that instead.

-- 
Eduardo

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Peter Maydell]

On 6 February 2018 at 19:04, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Feb 06, 2018 at 06:18:25PM +0000, Peter Maydell wrote:
>> One current approach to that is that instead of init'ing those
>> child objects in the container init, we postpone that to
>> container realize. This looks pretty ugly, and it also means
>> that you can't do "forward this property" using add_alias if the
>> target is the late-inited child (instead you have to have a
>> real property on the container and set the property on the child
>> manually after it's inited). You can see an example of this kind
>> of thing in hw/arm/armv7m.c.
>
> My rule of thumb is: if something is configurable (even if it's
> just slightly), it belongs to realize.  instance_init is reserved
> for stuff that don't take any external input.  If your container
> contents are not static, creating the contents is not a task for
> instance_init.
>
> But I would like to understand the drawbacks of this approach
> better.  So, if the object didn't have any "forward this
> property" aliases, would you see other problems with this
> approach?
>
> Why exactly those boards need the aliases?  Who sets those alias
> properties?  Can we provide helpers that make this task easier?

The "forwarding properties" bit is the one I'm hitting at the
moment. armv7m creates the CPU object, but it's really the
SoC which creates armv7m that wants to set various properties
on the CPU. (The CPU properties being set are things like
"initial vector address" which in real hardware are set by
the SoC hard-wiring various config signals on the core to 1 or 0.)
So I'd end up needing to manually forward a lot of properties
which are implemented in the cpu object but exposed to the
rest of the system via the armv7m wrapper.

It's also a bit weird because in the limit case you end up
doing nothing in init and postponing it all to realize,
at which point we're back to single-phase init. I don't
much like the "needed to configure this thing" design looking
different from the "simple nonconfigurable thing" design.
If nothing else, it makes it harder to convert from one to
the other when we add a new SoC or whatever.

thanks
-- PMM

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Eduardo Habkost]

On Tue, Feb 06, 2018 at 07:27:17PM +0000, Peter Maydell wrote:
> On 6 February 2018 at 19:04, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Tue, Feb 06, 2018 at 06:18:25PM +0000, Peter Maydell wrote:
> >> One current approach to that is that instead of init'ing those
> >> child objects in the container init, we postpone that to
> >> container realize. This looks pretty ugly, and it also means
> >> that you can't do "forward this property" using add_alias if the
> >> target is the late-inited child (instead you have to have a
> >> real property on the container and set the property on the child
> >> manually after it's inited). You can see an example of this kind
> >> of thing in hw/arm/armv7m.c.
> >
> > My rule of thumb is: if something is configurable (even if it's
> > just slightly), it belongs to realize.  instance_init is reserved
> > for stuff that don't take any external input.  If your container
> > contents are not static, creating the contents is not a task for
> > instance_init.
> >
> > But I would like to understand the drawbacks of this approach
> > better.  So, if the object didn't have any "forward this
> > property" aliases, would you see other problems with this
> > approach?
> >
> > Why exactly those boards need the aliases?  Who sets those alias
> > properties?  Can we provide helpers that make this task easier?
> 
> The "forwarding properties" bit is the one I'm hitting at the
> moment. armv7m creates the CPU object, but it's really the
> SoC which creates armv7m that wants to set various properties
> on the CPU. (The CPU properties being set are things like
> "initial vector address" which in real hardware are set by
> the SoC hard-wiring various config signals on the core to 1 or 0.)
> So I'd end up needing to manually forward a lot of properties
> which are implemented in the cpu object but exposed to the
> rest of the system via the armv7m wrapper.
> 
> It's also a bit weird because in the limit case you end up
> doing nothing in init and postponing it all to realize,
> at which point we're back to single-phase init. [...]

I disagree.  A single-phase init inside realize would be able to
take external input, and that's a huge difference from
instance_init (which can't take any input).

>                                           [...] I don't
> much like the "needed to configure this thing" design looking
> different from the "simple nonconfigurable thing" design.
> If nothing else, it makes it harder to convert from one to
> the other when we add a new SoC or whatever.

instance_init can't take any external input, so it can't be used
by configurable things.  So if we really want both configurable
and non-configurable cases to look alike, it would mean not using
instance_init on the non-configurable cases.

If doing everything on realize is not practical, we need to
understand why and fix the limitations on our APIs.

---

Just for clarity, the assumptions I have here are:

A) Initialization has 3 steps:
   1) telling QEMU what's the input we can get (registering
      properties);
   2) get input from the outside (setting properties); and
   3) act on input set by (2).

B) Step (2) needs to be done after instance_init.

C) Step (3) can't be done on instance_init because of (B).

D) realize is the only opportunity we have to perform step (3)
   today (but this can be changed).


-- 
Eduardo

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Igor Mammedov]

On Tue, 6 Feb 2018 18:18:25 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> [I've cc'd a fairly random selection of people who I thought
> might be interested or have an opinion.]
> 
> It's fairly common to have a setup where we have a QOM container
> object (like an SoC) which in turn instantiates a lot of child
> objects (for all the devices). The neat way of doing this looks
> like hw/arm/xlnx-zynqmp.c -- in the container's init function,
> we use object_initialize() to init all the child objects. If the
> container exposes some properties that are really just being
> forwarded to one of its children it can set those up in init
> with object_property_add_alias(). Finally, in realize the container
> realizes all its children.
> 
> Unfortunately, this pattern interacts badly with the idea that
> you might want to use a QOM property to determine aspects of
> the container that affect what child objects it creates.
> (Examples would include wanting a "which CPU is this" property
> on an SoC object, or if the SoC has a couple of variants which
> maybe have extra devices.)
[...] 

> Another approach is that instead of having a "what CPU" or "what
> SoC variant" property on the container, we create one container
> type per variation. Then instead of "create container, set QOM
> property to specify variant" the user creates the correct container
> type for the variant. hw/arm/aspeed_soc.c has an example of this.
> That looks pretty nice code-wise, but if there are a lot of
> possible options for the variants it could result in a large
> number of QOM types.
hw/arm/aspeed_soc.c looks rather clean and readable.

This types would be used only by boards and won't
be visible outside of QEMU, aren't they?
If so why would be large number of QOM types of a concern here?

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Paolo Bonzini]

On 06/02/2018 20:27, Peter Maydell wrote:
> armv7m creates the CPU object, but it's really the
> SoC which creates armv7m that wants to set various properties
> on the CPU. (The CPU properties being set are things like
> "initial vector address" which in real hardware are set by
> the SoC hard-wiring various config signals on the core to 1 or 0.)

Just to understand the context better, when you buy a SoC (presumably as
IP) can you also configure it to hardwire different CPU config signals?

Thanks,

Paolo

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Peter Maydell]

On 7 February 2018 at 15:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 06/02/2018 20:27, Peter Maydell wrote:
>> armv7m creates the CPU object, but it's really the
>> SoC which creates armv7m that wants to set various properties
>> on the CPU. (The CPU properties being set are things like
>> "initial vector address" which in real hardware are set by
>> the SoC hard-wiring various config signals on the core to 1 or 0.)
>
> Just to understand the context better, when you buy a SoC (presumably as
> IP) can you also configure it to hardwire different CPU config signals?

Usually if you buy an SoC you're buying the silicon, not
the IP. I think the usual case is that the SoC ties the
CPU config signals to 0 or 1 (part of the SoC designer's
job is configuring the semi-configurable bits of IP the
way they want when they put them all together). It would
certainly be in theory possible for the SoC to instead
route those signals out to pins on the SoC for the board
to tie off, but my impression is that's not a likely design
choice.

thanks
-- PMM

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Paolo Bonzini]

On 07/02/2018 17:22, Peter Maydell wrote:
>> Just to understand the context better, when you buy a SoC (presumably as
>> IP) can you also configure it to hardwire different CPU config signals?
>
> Usually if you buy an SoC you're buying the silicon, not
> the IP. I think the usual case is that the SoC ties the
> CPU config signals to 0 or 1 (part of the SoC designer's
> job is configuring the semi-configurable bits of IP the
> way they want when they put them all together). It would
> certainly be in theory possible for the SoC to instead
> route those signals out to pins on the SoC for the board
> to tie off, but my impression is that's not a likely design
> choice.

Yeah, that's why I was wondering if buying an SoC as configurable IP was
a thing at all.  In that case, I would just make things less
configurable and, if needed, hack the configurability with -global.

Paolo

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Peter Maydell]

On 7 February 2018 at 16:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 07/02/2018 17:22, Peter Maydell wrote:
>>> Just to understand the context better, when you buy a SoC (presumably as
>>> IP) can you also configure it to hardwire different CPU config signals?
>>
>> Usually if you buy an SoC you're buying the silicon, not
>> the IP. I think the usual case is that the SoC ties the
>> CPU config signals to 0 or 1 (part of the SoC designer's
>> job is configuring the semi-configurable bits of IP the
>> way they want when they put them all together). It would
>> certainly be in theory possible for the SoC to instead
>> route those signals out to pins on the SoC for the board
>> to tie off, but my impression is that's not a likely design
>> choice.
>
> Yeah, that's why I was wondering if buying an SoC as configurable IP was
> a thing at all.  In that case, I would just make things less
> configurable and, if needed, hack the configurability with -global.

So is that a vote for the aspeed style "create N different
QOM types" ?

It doesn't necessarily help with 'armv7m', which isn't an
SoC, but just part of the way we currently model M profile CPUs.
In real hardware, the CPU includes all of the interrupt
controller, timers, etc, which in QEMU are separate
objects to the QEMU CPU object and wrapped up inside
an armv7m container.

(It would I guess be possible to merge the 'armv7m' container
entirely into the QEMU CPU object, so creating the CPU object
gave you an nvic and the mmio register regions to map and so
on. We don't do anything like that for other cpus right now,
though...)

thanks
-- PMM

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Paolo Bonzini]

On 07/02/2018 17:49, Peter Maydell wrote:
>> Yeah, that's why I was wondering if buying an SoC as configurable IP was
>> a thing at all.  In that case, I would just make things less
>> configurable and, if needed, hack the configurability with -global.
>
> So is that a vote for the aspeed style "create N different
> QOM types" ?

Yeah, many types for the SoCs, each setting different properties on the
CPU/nvic/timers/etc.

> It doesn't necessarily help with 'armv7m', which isn't an
> SoC, but just part of the way we currently model M profile CPUs.

Does armv7m have a fixed or variable number of sub-objects?  If it's
fixed, setting up forwarding properties is easy.  But if even parts of
it are variable, it does get messy.

Thanks,

Paolo

> In real hardware, the CPU includes all of the interrupt
> controller, timers, etc, which in QEMU are separate
> objects to the QEMU CPU object and wrapped up inside
> an armv7m container.
> 
> (It would I guess be possible to merge the 'armv7m' container
> entirely into the QEMU CPU object, so creating the CPU object
> gave you an nvic and the mmio register regions to map and so
> on. We don't do anything like that for other cpus right now,
> though...)

Re: [Qemu-devel] how to handle QOM 'container' objects whose contents depend on QOM properties?

[Peter Maydell]

On 7 February 2018 at 17:28, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 07/02/2018 17:49, Peter Maydell wrote:
>>> Yeah, that's why I was wondering if buying an SoC as configurable IP was
>>> a thing at all.  In that case, I would just make things less
>>> configurable and, if needed, hack the configurability with -global.
>>
>> So is that a vote for the aspeed style "create N different
>> QOM types" ?
>
> Yeah, many types for the SoCs, each setting different properties on the
> CPU/nvic/timers/etc.
>
>> It doesn't necessarily help with 'armv7m', which isn't an
>> SoC, but just part of the way we currently model M profile CPUs.
>
> Does armv7m have a fixed or variable number of sub-objects?  If it's
> fixed, setting up forwarding properties is easy.  But if even parts of
> it are variable, it does get messy.

If the CPU supports the security extensions then it gets an
extra timer device. (That typically doesn't need any properties
forwarded it it, though.)

I'll have a look at refactoring armv7m (again ;-)) into
an armv7m-cortex-m3, armv7m-cortex-m4, and armv7m-cortex-m33.

thanks
-- PMM