Re: [PATCH v2 12/20] ppc/ppc405: QOM'ify EBC

[Cédric Le Goater]

On 8/4/22 21:26, BALATON Zoltan wrote:
> On Thu, 4 Aug 2022, Peter Maydell wrote:
> > On Thu, 4 Aug 2022 at 19:03, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> > > I was trying to find out how to do it but I don't understand QOM enough to
> > > answer the simple question of how to get the cpu object from QOM. My
> > > guesses are:
> > >
> > > object_resolve_path_type("/machine", TYPE_POWERPC_CPU, NULL)
>
> Out of curiosity would this work though to get the cpu or if not why not and 
> what would be a preferred way? I could not find this out from reading the 
> object.h comments, the docs/deve/qom.rst, nor searching the code.

You could scan the object topology using object_child_foreach_recursive()
and use object_dynamic_cast() to find a POWERPC CPU object. A link is
much faster !

> > > or maybe
> > >
> > > object_resolve_path_at(OBJECT(dev)->parent, "cpu")
> > >
> > > or how do these functions work and what is the preferred way to retrieve
> > > an object from the QOM tree? This is what I hoped someone with more
> > > understanding of QOM could answer.
> >
> > The standard approach that we use elsewhere in the tree for handling
> > "this device needs to have a pointer to a CPU object or whatever"
> > is "the device has a QOM link property, and the SoC sets that
> > property when it creates the device".
> >
> > There are other ways it could in theory be done, but there is
> > benefit in consistency, and "define and set the property" is
>
> If this is the preferred way then so be it, I just don't like it because I 
> think this is too many boilerplate code that could be avoided. This series:
>
>   9 files changed, 894 insertions(+), 652 deletions(-)
>
>   and that's including removing all of the taihu machine; the file where the 
> QOMification is done:
>
>   hw/ppc/ppc405_uc.c              | 799 +++++++++++++++++++-------------

Yes. You should consider also that this code is > 15 years old and
serious shortcuts have been taken to "make things work". I think QOM
clarifies the models and represents better the HW topology. There is
a price in being explicit.

> Ideally introducing QOM should make it simpler not more complex. Four of the 
> QOMified devices only have a property defined at all because of this cpu link 
> that's only used once in the realize method to register DCRs. This is about 10 
> lines of code each. If there was a simple way to get the cpu object from these 
> realize methods then we could get rid of all these properties and save about 
> 40-50 lines and make these simpler.

I tried several approaches and found this one was the simplest and not
too verbose really.

The DCRs are accessed by software through the use of the mtdcr and mfdcr
instructions. These are converted in transactions on a side band bus,
the DCR bus, which connects the on-SoC devices to the CPU. The "cpu" link
should be considered as modeling this little piece of HW logic connecting
the device to the DCR bus.

Thanks,

C.


> > straightforward. It also means the device object doesn't have
> > to know anything about the way the SoC container is laid out.
>
> We only need the cpu object so we don't need to know the soc container if 
> there's a way to get it otherwise I just don't know how QOM works and was 
> trying to find a way to get to the cpu object. Maybe it's simpler than that.
>
> If there's no simple way or you and Cédric think it isn't worth the effort then 
> I'm also OK with it but if there's a way to make this simpler I'd be happy to 
> get rid of things that make it harder to read and understand code or allow 
> making mistakes more easily. I take whatever decision you make so won't say 
> this again, I think I've explained my point now.
>
> Regards,
> BALATON Zoltan
>
> > (It's usually worth looking at whether there are cleanups
> > that could mean the device doesn't have to have a pointer to
> > that other object at all -- but that isn't always the case,
> > or the cleanups would be a big job in their own right that
> > are better not tangled up with QOMification.)
> >
> > thanks
> > -- PMM