Re: [Qemu-devel] [PATCH v3 14/16] acpi: Add a way for devices to add ACPI tables

[Corey Minyard <minyard@acm.org>]

On 07/09/2015 03:25 AM, Igor Mammedov wrote:
> On Wed, 8 Jul 2015 22:39:24 +0200
> Paolo Bonzini <pbonzini@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.

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?

-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.