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.