On Fri, 5 Aug 2022, Peter Maydell wrote:
On Fri, 5 Aug 2022 at 13:55, BALATON Zoltan <bala...@eik.bme.hu> wrote:
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.
I agree that QOM's boilerplate is not nice at all, but if
you do something other than the "standard QOM boilerplate"
solution to a problem then you make it harder for people who
at least know what the standard QOM approach is to figure out
why the code is doing what it does.
True, unless what we do instead is simpler so it's obvious what it's
doing. Also you've said that needing a link is often a sign that there's
something wrong with the modeling so maybe it can be avoided changing how
we model things. I think that's the case here. If we had:
struct PPC4xxMachineState { /* abstract */
MachineState parent_obj;
PPC4xxSoc soc;
}
which we use for all 4xx machnies that use the devices QOMified here and
struct PPC4xxSocState { /* abstract */
DeviceState parent_obj;
PowerPCCPU cpu;
/* other common devices shared by 405 and 440
* (probably most of them), may need to add int ram_banks to allow
* different size ram_memories arrays, etc. but this can be done later
* when doing 440 SoC state and only have the cpu here for now */
}
struct PPC405SocState {
PPC4xxSocState parent_obj;
/* devices specific to 405 same as proposed Ppc405SoCState */
}
and likewise for PPC440SocState which could be done in a different series
taking care of 440 machines later. Then we could more cleanly model the
sharing of code between 4xx SoCs (this series only considered 405 but this
is enangled with 440 so we should take into account that too), This also
allows to get the cpu without a link with something like:
PPC4XX_MACHINE(current_machine /* or qdev_get_machine() */)->soc.cpu
This is pretty clear if you look at the object class definitions and
avoids needing to link things together by hand.
If this is not clear yet or Cédric does not want to do it now I may try it
once he publishes the latest version of this series or as a follow up once
it's merged but if I could get across what I mean and not too much changes
maybe it could be considered so we don't have too many iterations on this.
I understand Cédric may not want to touch bamboo or sam460ex and mostly
cares for 405 to add hotfoot but what I propose does not need going all
the way with the 440 machines, only introduce the QOM classes now so that
it could be used later bur not break the 440 machines now. We may even get
away with just adding a PowerPCCPU *cpu; to PPC4xxMachineState for now
which can be set in the machine init func that creates the cpu before
other devices, then we may not need PPC4xxSocState abstract class for now
but maybe it's clearer with the abstract SoC class that can be filled in
later with common stuff shared by all 4xx machines.
Regards,
BALATON Zoltan
