Re: [Qemu-devel] [PATCH 10/15] smbios: Add a function to directly add an entry

[Paolo Bonzini <pbonzini@redhat.com>]

On 14/04/2015 08:31, Michael S. Tsirkin wrote:
> On Mon, Apr 13, 2015 at 06:40:46PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 13/04/2015 18:34, Corey Minyard wrote:
>>>>> I made this the same as the ACPI code, which you have to have as a
>>>>> callback if you are adding it to a common SSDT.
>>>>
>>>> Not really I think.
>>>
>>> The AML functions require that you have a tree to attach what you are
>>> adding.  If you did your own SSDT, you wouldn't need a callback.  You
>>> could add a binary blob that gets put into the SSDT, but I think that
>>> would require adding some AML functions.
>>
>> I very much prefer the callback idea.  Long term it could be used by
>> more devices and possibly it could be turned into an AMLProvider QOM
>> interface.  Then the ACPI builder could iterate on all QOM devices and
>> just ask which of them can provide some AML.
> 
> Yes, that would make sense. Devices which have a static
> AML could provide a static AML property, with very little code,
> those that have dynamic AML - dynamic AML property with more code.

Why complicate the ACPI hooks unnecessarily?  The generic code is
written to support dynamic AML, why should devices support both?  This
seems like premature optimization.

> I was looking for ways to remove dependencies for this patchset,
> not add them.

I'm not sure which dependencies these are.

>> Also, tables are rebuilt when the firmware loads them, and handing in a
>> blob makes it harder to achieve this on-the-fly modification, compared
>> to callbacks.
> 
> Well that's not true for smbios, is it?

No, it's not.  For smbios, I agree that passing a static table to the
API (i.e. having smbios_register_device_table instead of
smbios_register_device_table_handler, and calling smbios_table_entry_add
directly from smbios_get_tables) is also a valid choice.

Paolo