|
| From: | BALATON Zoltan |
| Subject: | Re: [PATCH v2 12/20] ppc/ppc405: QOM'ify EBC |
| Date: | Fri, 5 Aug 2022 14:55:01 +0200 (CEST) |
On Fri, 5 Aug 2022, Cédric Le Goater wrote:
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 toanswer 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 !
Yes that sounds a lot slower, I hoped there's simple easy way to get to the cpu, then we could simplify this a bit.
One more idea, you've introduced the Ppc405MachineState which you can get to casting current_machine and so it's a convenient place to store a cpu pointer or even just use PPC405_MACHINE(current_machine)->soc.cpu. This works for these 405 machines you've changed in this series but other PPC4xx machines don't have this machine and soc states yet. Could these Ppc405MachineState and Ppc405SoCState be more generic as Ppc4xxMachineState and Ppc4xxSoCState considering that these chips are quite similar or maybe we need an abstract PPC4xxSoC class but machine state could be shared? (If you say this is too much cahnges for you now I may try to look at this later once your series is merged but going that way now could save some churn.) If we had a Ppc4xxMachineState that has a cpu pointer then we could easily add that to bamboo and sam460ex now and these QOMified devices could then use PPC4XX_MACHINE(current_machine)->cpu instead of a link property. What do you think?
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.
I know this is a mess curently but QOM is full of boilerplate code which is confusing for new people and makes it hard to undestand the code. So cutting down the boilerplate and making things simpler would help people who want to get started with QEMU development. If adding a property was 3-4 additional lines I wouldn't care but if it makes the code significantly more complex and thus harder to understand at a glance then I'd rather avoid it if possible and stick to simple terms. Verbosity is good if it explains things better but bad if it's hiding the important details between unneeded complexity due to having to use constructs that are not obvious. Also the property needs to be set which is additional lines and possibility for mistakes so if there's a way to avoid that I think that's better.
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.
I rather consider it an implementation detail and trying to find the simplest way. If we don't find anything simpler I'm OK with link properties but I'm not convinced yet there's no simpler solution to this problem that could avoid some of the additional complexity.
Regards, BALATON Zoltan
| [Prev in Thread] | Current Thread | [Next in Thread] |