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

[Michael S. Tsirkin]

On Tue, Oct 15, 2013 at 06:53:54AM -0700, Anthony Liguori wrote:
> 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

Is v2 better?

I think I have addressed your complaints by
1. dropping bridge code for now
2. fixing the commit log issue for the patch with generated code
   that you noticed

The use case is coreboot support.

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