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

[Igor Mammedov]

On Wed, 8 Jul 2015 22:39:24 +0200
Paolo Bonzini <address@hidden> 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.

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