Re: [Qemu-arm] [RFC PATCH 0/6] qom: introduce TypeInfo name aliases

[Igor Mammedov]

On Thu, 4 Jan 2018 17:22:03 -0200
Eduardo Habkost <address@hidden> wrote:

> On Thu, Jan 04, 2018 at 11:40:40AM -0300, Philippe Mathieu-Daudé wrote:
> > Hi,
> > 
> > This RFC series is intended to simplify Flattened Device Tree support,
> > in particular the 'compatible' FDT entry, when Linux names mismatches
> > QEMU ones, but this is the same device modelled.
> > 
> > Eventually this might help to remove the QDevAlias qdev_alias_table[]
> > in qdev-monitor.c.
> >   
> 
> Didn't look closely at the patches yet, but this sounds like a
> nice generic way to replace other alias systems.  We have at
> least:
Though it seems easy and trivial, I'm a bit concerned about using
QOM types for the task though.
Also see commit 6acbe4c6f which labels aliases as a bad idea
and says that they are there only for compatibility and shouldn't
be used.
So far I agree with that statement, because it introduces
ambiguity in code used internally and more worrying is that
this ambiguity will increase user visible ABI (think of '-device_add FOO_ALIAS')
that we would need to maintain afterwards.
It would be nice to have unified alias API, but I think it should
be separate one and limited to the same scope (i.e. compat stuff),
and even that won't be easy as different alias impl. we have now
have a different needs.

wrt this series targeted usage, I'd prefer that object_new/initialize
would use real type names when creating devices as it does currently

FDT linux guest specific names wouldn't seep into device model
itself. Firmware (FDT or ACPI) should be separate from device
implementation.

If really there is need to dynamically scan present devices
and build FDT from result, then probably we should introduce
interface that devices could implement if necessary.
(I was thinking about such possibility for ACPI). But so far
it looked to me as too much overhead for what we do now.


> * qdev-monitor.c: qdev_alias_table[] (as mentioned above)
> * chardev/char.c: chardev_alias_table[]
> * target/alpha/cpu.c: alpha_cpu_aliases[]
> * target/ppc/cpu-models.c: ppc_cpu_aliases[]
> * include/hw/boards.h: MachineClass::alias
> 
> Probably there are others I couldn't find.
> 
> 
> > So far this is only a 'proof of concept'.
> > To see how the qtests perform, I only modified 3 devices, 2 used by the
> > Xilinx Zynq machines (Cadence), and the e1000 (used by the PXE test).
> > 
> > Regards,
> > 
> > Phil.
> > 
> > Philippe Mathieu-Daudé (6):
> >   qom: introduce TypeInfo name aliases
> >   hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
> >   hw/char/cadence_uart: add FDT aliases
> >   arm/xlnx-zynq: use FDT names for the Cadence UART
> >   hw/net/cadence_gem: add FDT names as alias
> >   hw/arm/xlnx-zynq: use FDT names for the Cadence GEM
> > 
> >  include/qom/object.h   |  3 +++
> >  hw/arm/xilinx_zynq.c   |  2 ++
> >  hw/arm/xlnx-zynqmp.c   |  4 ++--
> >  hw/char/cadence_uart.c |  7 +++++++
> >  hw/net/cadence_gem.c   |  6 ++++++
> >  hw/net/e1000.c         |  5 ++++-
> >  qom/object.c           | 18 ++++++++++++++++--
> >  7 files changed, 40 insertions(+), 5 deletions(-)
> > 
> > -- 
> > 2.15.1
> >   
>