Re: [Qemu-devel] [SeaBIOS] [PATCH v2 3/4] i386: generate pc guest info

["Michael S. Tsirkin" <mst@redhat.com>]

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