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

[Michael S. Tsirkin]

On Wed, Jul 24, 2013 at 04:42:08PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >> 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.
> 
> /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?
> 
> 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


That's exactly what I implemented, with APIs so that we don't
expose structure internals and path names to all the world.
Will post soon.