Re: [SeaBIOS] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

On 06/10/13 20:58, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> > This adds support for device hotplug behind
> > pci bridges. Bridge devices themselves need
> > to be pre-configured on qemu command line.
> >
> > One of the goals of this project was to
> > demonstrate the advantages of generating
> > ACPI tables in QEMU.
> > In my opinion, it does this successfully.
>
> Since you've gone out of your way to make this difficult to actually
> review...

I haven't tried to review the patch yet, but why would you say such a
thing? Michael wants to convince you. There's no way he could sneak past
you in this patch. You surely won't say "it's too hard to review, I'll
yield", and he's obviously aware.

[snip]
> >    * No need for yet another interface to bios to detect qemu version
> >      to check if it's safe to activate new code,
> >      and to ship multiple table versions:
>
> We only care about supporting one version of SeaBIOS.  SeaBIOS should
> only care about supporting one version of QEMU.  We're not asking it to
> support multiple versions.

Kevin requires cross-compat between {old, new} qemu and {old, new} SeaBIOS:

http://thread.gmane.org/gmane.comp.emulators.qemu/202005/focus=202072
> >      we generated code so we know this is new qemu
> >    * Use of glib's GArray makes it much easier to build
> >      up tables in code without need for iasl and code patching
>
> Adding a dynamic array to SeaBIOS isn't rocket science.

Please take a look at my series for OVMF that adds basic support for
SMBIOS tables. It took me three days of basically uninterrupted coding
and two approaches to throw away until I got something submittable (with
default tables for only type 0 and type 1).

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/3037

I don't want to invite accusations like "this is a strawman argument,
noone was speaking about SMBIOS", but the selective, dynamic patching is
somewhat similar between AML and SMBIOS in any given boot firmware. You
got a big bunch of named offsets that must be mangled individually.

Allowing the firmware to only install blobs verbatim, or maybe patch
them but only in a completely programmatic manner (ie. without specific
field names & offsets in the firmware, which I did try for SMBIOS in
OVMF, and failed, (*)), would be a big help.

((*) because the fw_cfg format of individual SMBIOS fields doesn't
distinguish formatted fields from strings in the unformatted area)

Not rocket science, just churn and busywork.

[snip]
> So TL;DR, you break a bunch of stuff and introduce a mess of code.  It
> would be less code and wouldn't break anything to add this logic to
> SeaBIOS.
>
> How is this even a discussion?

Well it isn't for me any more; count me out. I'll go with the flow.

Cheers,
Laszlo