On Wed, Jul 24, 2013 at 04:52:05PM +0200, Andreas Färber wrote: > Hi Gerd, > > Am 24.07.2013 16:42, schrieb Gerd Hoffmann: > >>> This does not satisfy the "should use QOM properties" requirement that > >>> we discussed in the RFC thread. > >> > >> I don't know which part of the RFC thread still applied and > >> which doesn't: at that point you were rejecting the whole > >> approach. > >> > >> I found a mail where you said: > >> I'd be a lot happier if we were passing more information to this routine > >> and not hard coding it. For instance, the PCI interrupt assignments, > >> the APIC ids, the number of available CPUs, etc. > >> > >> So this is exactly what this code does. > >> What, exactly, would you like to see instead? > >> Create a guest info QOM object, and encode all information used by ACPI > >> generation as properties of this object? > > > > Don't touch device code for this. > > > >>>> -void pvpanic_init(ISABus *bus) > >>>> +void pvpanic_init(ISABus *bus, PcGuestInfo *guest_info) > >>>> { > >>>> ISADevice *dev; > >>>> - FWCfgState *fw_cfg = fw_cfg_find(); > >>>> + FWCfgState *fw_cfg = guest_info->fw_cfg; > >>>> if (!fw_cfg) { > >>>> return; > >>>> } > >>>> dev = isa_create_simple (bus, TYPE_ISA_PVPANIC_DEVICE); > >>>> - pvpanic_fw_cfg(dev, fw_cfg); > >>>> + pvpanic_guest_info(dev, guest_info); > >>>> } > > > > To pick this one as example: Instead of patching pvpanic code to stuff > > config info into GuestInfo you should (1) search the device object tree > > for a pvpanic device and (b) if present read the ioport property to > > figure the base address. > > Yeah, the above does not feel so nice from a QOM view (didn't review the > ACPI series yet).
If you do please review v3 which was just posted. It should address your comment - though long term, it would be nicer if we had multiple inheritance for interfaces, then we could expose a generic panic interface with the io port number, avoid need to poke at specific device. > > /me suggests to check out qmp_qom_get() in qmp.c. Some qom aequivalent > > for qdev_find_recursive would be handy, dunno whenever such a thing > > exists already, Andreas? > > Not sure what's needed here? object_resolve_path() and > object_foreach_child() come to mind... > > Regards, > Andreas > > > I'd tend to accept GuestInfo as temporary thing for stuff which can't be > > figured using qom properties today. Anthony might disagree though. > > > > cheers, > > Gerd > > > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg