On Mon, 13 Jul 2015 16:55:29 -0500 Corey Minyard <miny...@acm.org> wrote:
> On 07/09/2015 03:25 AM, Igor Mammedov wrote: > > On Wed, 8 Jul 2015 22:39:24 +0200 > > Paolo Bonzini <pbonz...@redhat.com> wrote: > > > >> > >> On 08/07/2015 21:26, Igor Mammedov wrote: > >>>> 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. > >> On the other hand, it would be much harder to make IPMI optional unless > >> you want to add a bunch of ugly stubs. Same for SMBIOS. Especially > > it's no the case if one uses QOM properties to fetch info. > > it's a bit verbose but works well without need for ugly stubs. > > > >> when you can have the same ACPI and SMBIOS tables in both x86 and ARM > >> virtual machines, it becomes a little more the device business to > >> provide ACPI and SMBIOS information. > > For a generic way for device to supply information to ACPI, > > I was thinking about introducing interface that device might implement > > for example if it should have _CRS in its ACPI description. > > Then APCI core could query for devices that implement interface > > and build AML code based on resource values (i.e. base address, > > size, irq #, e.t.c) it gets from device. > > I'm just getting back from vacation and catching up. > > I would agree that implementing an interface is a more consistent > way to do this than the way it's done now. If it's ok with everyone, > I'll work on implementing this. Implementing interface shouldn't block this series. I'm ok with this series ACPI wise if you drop 14/16 and do as suggested above + address comment on 15/16. It would require minimal changes and get us an interesting device to play with without significant delays. I would do the same with SMBIOS side as well. Implementing interface could be made on top along with conversion of current code that fetches resource info from devices manually. > > I'm not sure that resource values are enough to do this, though. > IPMI has some custom fields (_IFT, _SRV, _STR) and lots of optional > fields (_ADR, _UID, _STA), how would you do these fields? fields names are part of ACPI spec, maybe interface should have methods for returning these fields. _ADR - for example for PCI devices should address of device on bus _SRV - return packed revision number _IFT - return interface number _STR, _UID, _STA - I'd leave them to board/firmware to decide how to name/enumerate/detect presence (i.e. in acpi-build.c) > > -corey > > > > >> So overall I'm happy with the way it was done here. I and Michael > >> disagreed on several details, but not enough to fight over it... > > I won't fight over it either, but it's my opinion how it should be > > done. > > > >> Paolo > >> > >>> 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. >
