On Wed, 8 Jul 2015 19:44:16 +0200 Paolo Bonzini <pbonz...@redhat.com> wrote:
> > > On 07/07/2015 10:08, Igor Mammedov wrote: > > > Some devices, like IPMI, need to add ACPI table entries to report > > > their presence. Add a method for adding these entries. > > > > I think that it's not up to device to define in which table/scope > > it's entries should be but rather upto a board/platform to decide. > > > > I'd prefer the old way of adding device's ACPI AML into existing > > SSDT > > This was suggested by Michael, so I think you should read the reviews > of earlier versions first. That is basically the same as hooks in v1 only the other way around with all drawbacks attached. Just dropping this universal way to scatter ACPI code all over QEMU and calling ipmi_encode_one_acpi() "[15/16] ipmi: Add ACPI table entries" directly from build_ssdt() would simplify series quite a bit. I'd do the same for SMBIOS entries as well, i.e. drop similar universal way for devices to supply SMBIOS info (it's not their business) and just call ipmi_encode_one_smbios() to add ipmi specific entry if device is present. Again simplifying series even more. that roughly would save us 300 lines of not really necessary code and keep things consistent with a way we are managing ssdt now. > > Paolo > > , like we do for every other device that needs it > > (for example: pvpanic). > > That allows to keep ACPI code in one place and for each platform > > to decide to which table put a device description and where exactly > > it should be. > > > > So I'd drop this patch and add function that returns non NULL > > "IPMIFwInfo *" if IPMI device is present and use slightly modified > > 15/16 to add device entry into the existing SSDT. >