Re: [Qemu-devel] [PULL 00/43] pci, pc, acpi fixes, enhancements

[Anthony Liguori]

On Tue, Oct 15, 2013 at 6:43 AM, Gerd Hoffmann <address@hidden> wrote:
> On Mo, 2013-10-14 at 15:42 -0700, Anthony Liguori wrote:
>> "Michael S. Tsirkin" <address@hidden> writes:
>>
>> > Anthony, I know you wanted to review some of the patches,
>> > since you didn't respond either all's well or you
>> > could not find the time.
>> > I think we are better off merging them for 1.7 and then - worst case,
>> > if major issues surface - disabling the functionality at the last minute
>> > than delaying the merge even more.
>>
>> There is no way I'll pull this for 1.7.  Changes like this aren't going
>> to get merged at the last minute.
>
> Hmm?  Patches are discussed and tested for months, with the core not
> having seen no big changes since weeks.  Recent revisions of the series
> are only fixing the bugs which showed up in testing and some finishing
> touches.  It certainly isn't something new poping out of the blue the
> last minute.
>
> Why do you ignore the patches and discussions until things are settled
> and the pull request comes in?

Sorry, I shouldn't mix in general complaining with why I am not happy
with this pull request.  I already said I would take this change given
a clear use-case and I would have merged it if the series was in a
better state.  I am sympathetic to not wanting to maintain this stuff
in SeaBIOS.

But I am not happy with the state of the pull request for reasons I
explained in another note.

Regards,

Anthony Liguori

>
>> A good chunk of the series lacks
>> any Reviewed-bys including the actual hotplug behind a pci bridge bits
>> which is the whole point of the series.
>
> pci bridge hotplug is only a part of the whole picture.
>
> It is about using an existing standard (ACPI) to communicate hardware
> config information between qemu and the guest OS.  Without requiring the
> middle man (seabios or other firmware) knowing details it doesn't need
> to know for its own job.  And avoid creating one paravirtual interface
> after the other to give the firmware the information it needs to
> generate the acpi tables.
>
> It is also about having *one* instance (qemu) generates the acpi tables
> instead of expecting each firmware duplicate that functionality.  It
> makes live a lot easier for alternative firmwares such as ovmf and
> coreboot.  For coreboot the patch series (with the complementary
> coreboot patches to load the tables from qemu) is a big step forward to
> feature parity with seabios.
>
> And, yes, implementing features like pci bridge hotplug and memory
> hotplug (oh, and lets not forget pvpanic) on top of the acpi generation
> series is alot easier:
>
>  * You implement it in qemu, and you are done.
>
>> This is a huge series and I still am not convinced this is the right
>> path forward.  The alternative to this series is a small set of changes
>> to SeaBIOS to support PCI bridge hotplug, no?
>
> No.  The alternative is:
>
>  * You create a paravirt interface to communicate the
>    config information for $newfeature.
>  * You implement that in qemu.
>  * You implement that in seabios.
>  * You implement that in OVMF.
>  * You implement that in coreboot.
>
>> Or 10k SLOC of code into QEMU that includes breaking migration
>> compatibility.
>
> On the plus side we can stop maintaining those 10k SLOC in seabios.
>
> The bits will stay there for a while for compatibility with older qemu
> versions, but don't need much care any more as all new stuff will go
> into qemu instead.
>
> cheers,
>   Gerd
>
>
>