Re: Configuring onboard devices

[Eduardo Habkost]

On Sun, May 03, 2020 at 11:13:41PM +0100, Mark Cave-Ayland wrote:
> On 02/05/2020 06:47, Markus Armbruster wrote:
> 
> > Mark Cave-Ayland <address@hidden> writes:
> > 
> >> On 30/04/2020 16:20, Markus Armbruster wrote:
> >>
> >>>> Ah I see now, these aliases are for individual properties rather than 
> >>>> objects. What I
> >>>> was trying to ask was if it were possible to have something like this:
> >>>>
> >>>> /machine (SS-5-machine)
> >>>>   /builtin
> >>>>     /nic0 -> link to "lance" device
> >>>>
> >>>> Here nic0 is an alias "published" by the maintainer of the SS-5 machine 
> >>>> which is
> >>>> configured in the machine init() function using 
> >>>> object_property_add_link() or a
> >>>> suitable wrapper. Users can then configure these builtin devices from 
> >>>> the command
> >>>> line using your -machine nic0.netdev=my-netdev-id syntax or similar.
> >>>
> >>> Got it now, thanks!
> >>>
> >>>> Having the default devices under /builtin or other known QOM path would 
> >>>> enable
> >>>> builtin devices to be easily enumerated programatically and/or from the 
> >>>> command line
> >>>> as required.
> >>>
> >>> There are three standard containers under /machine/:
> >>>
> >>> * /machine/peripheral/
> >>>
> >>>   Devices with a user-specified ID go here, as /machine/peripheral/ID.
> >>>   User-specified means -device or device_add.
> >>>
> >>>   /machine/peripheral/ID is effectively a stable interface.  It's just
> >>>   underdocumented (undocumented?).
> >>>
> >>>   To be useful, the stuff below ID/ needed to be stable and documented,
> >>>   too.
> >>>
> >>> * /machine/peripheral-anon/
> >>>
> >>>   Same, but user elected not to give an ID.
> >>>   /machine/peripheral-anon/device[N], where N counts up from zero in
> >>>   creation order.
> >>>
> >>>   N is obviously not stable, but this is a problem of the user's making.
> >>>   If you want to refer to a device, give it an ID.
> >>>
> >>> * /machine/unattached/
> >>>
> >>>   The orphanage.  When a device has no parent when its realized, it gets
> >>>   put here, as /machine/unattached/device[N], where N counts up from
> >>>   zero in realization order.
> >>>
> >>>   N is obviously not stable, and this time we can't blame the
> >>>   victim^Wuser.  You can search for devices of a certain type.
> >>>   Sometimes that's good enough.
> >>>
> >>>   All the onboard devices are here, and much more.  We've fathered a lot
> >>>   of unloved red-headed children, it seems...
> >>>
> >>>   Some of the "much more" is due to sloppy modelling, i.e. neglecting to
> >>>   set the proper parent.
> >>>
> >>>   I figure we could put onboard devices in a nicer place, with nicer
> >>>   names.  Need a convention for the place and the names, then make board
> >>>   code conform to it.
> >>
> >> That's good, it seems that this is already fairly close to how it works 
> >> for -device
> >> at the moment.
> >>
> >> I don't think that it is possible to come up a single place for on-board 
> >> devices to
> >> live directly though. Going back to one of my first examples: wiring up a 
> >> chardev to
> >> a serial port on the macio device. To me it makes sense for that to exist 
> >> in QOM
> >> under /machine/pci-bus/mac-io/escc. In contrast an in-built NIC could live 
> >> under
> >> /machine/pci-bus/in-built/nic, and placing one or both of these devices 
> >> directly
> >> under /machine/foo doesn't feel intuitive.
> > 
> > I'm not familiar with this machine.  You make me suspect the serial
> > thingy is a component of a larger device.
> > 
> > Properly modelled, a composite device has its components as children.
> > These appear below their parent in the QOM composition tree.
> > 
> > Example: a "serial-isa" device has a "serial" component.  When the
> > former is at /machine/unattached/device[28]/, the latter is at
> > /machine/unattached/device[28]/serial/.
> > 
> > I guess that's what you want for macio's serial port.
> > 
> > Counter-example: a "isa-super-io" device has compoenents of type
> > "isa-parallel", "isa-serial", "isa-fdc", "i8042", "isa-ide".
> > Nevertheless, these appear next to their parent in /machine/unattached/.
> > I'm still too much of a QOM ignoramus to explain why that's so.  Paolo,
> > can you?
> 
> FWIW the older machines have a lot of calls to qdev_create(NULL, TYPE_FOO) for
> devices that are part of the machine because they live within the machine 
> address
> space but are not specifically attached to a qbus.
> 
> >> AFAIK as per your ARM virt example I believe it is only possible to 
> >> register an alias
> >> for a property rather than for an Object? The ultimate aim would be for
> >> object_resolve_path("/machine/builtin/nic0") and
> >> object_resolve_path("/machine/pci-bus/in-built/nic") to return the same 
> >> Object, and
> >> for the aliases on built-in devices to be children of /machine/builtin to 
> >> allow easy
> >> iteration and introspection.
> > 
> > Paolo, could link properties achieve that?
> > 
> > Mark, I guess you want the alias / link from builtin/nic0 to the actual
> > place to simplify configuration: the user then needs to know less about
> > the board.  Correct?
> 
> Correct. In a perfect world I'd love to say that Daniel's suggestion to use 
> QOM paths
> would work, however from my experience they change far too much. This is one 
> of the
> reasons that the TYPE_FW_PATH_PROVIDER interface exists so that the 
> generation of
> (boot) paths for the firmware is separate from the QOM/qdev paths.
> 
> I don't feel too strongly whether it's a link property, some kind of alias, or
> perhaps like OpenFirmware just a string property containing the QOM path to 
> the
> device, since ultimately I imagine there would be wrapper functions for 
> machine
> init() to call which hide away the implementation details.

Whatever solution we use to make sure a new interface is stable,
we must have automated test cases to ensure we don't break it in
the future.  Otherwise we will be creating yet another
compatibility-keeping ritual that we fail to follow because
nobody notices when we break it.

I wouldn't like to move from "2 different object paths that can't
be trusted to be stable" to "3 different object paths that can't
be trusted to be stable".

-- 
Eduardo