qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: peter.maydell@linaro.org, imammedo@redhat.com,
	marcel.a@redhat.com, qemu-devel@nongnu.org,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PULL 00/43] pci, pc, acpi fixes, enhancements
Date: Tue, 15 Oct 2013 16:14:39 +0200	[thread overview]
Message-ID: <1381846479.3327.0.camel@nilsson.home.kraxel.org> (raw)
In-Reply-To: <87r4bnlbj6.fsf@codemonkey.ws>


On Mo, 2013-10-14 at 15:42 -0700, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> 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?

> 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, it makes implementing features like pci bridge hotplug and
memory hotplug (oh, and lets not forget pvpanic) 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

      parent reply	other threads:[~2013-10-15 14:15 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-14 14:57 [Qemu-devel] [PULL 00/43] pci, pc, acpi fixes, enhancements Michael S. Tsirkin
2013-10-14 14:57 ` Paolo Bonzini
2013-10-14 15:12   ` Michael S. Tsirkin
2013-10-14 15:21     ` Paolo Bonzini
2013-10-14 14:57 ` [Qemu-devel] [PULL 01/43] memory: Change MemoryRegion priorities from unsigned to signed Michael S. Tsirkin
2013-10-14 14:57 ` [Qemu-devel] [PULL 02/43] docs/memory: Explictly state that MemoryRegion priority is signed Michael S. Tsirkin
2013-10-14 14:57 ` [Qemu-devel] [PULL 03/43] hw/pci: partially handle pci master abort Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 04/43] hw/core: Add interface to allocate and free a single IRQ Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 05/43] hw/pci: add pci wrappers for allocating and asserting irqs Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 06/43] hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 07/43] hw/vmxnet3: set interrupts using pci irq wrappers Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 08/43] hw/vfio: " Michael S. Tsirkin
2013-10-14 15:46   ` Alex Williamson
2013-10-14 14:58 ` [Qemu-devel] [PULL 09/43] hw: " Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 10/43] hw/pcie: AER and hot-plug events must use device's interrupt Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 11/43] hw/pci: removed irq field from PCIDevice Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 12/43] cleanup object.h: include error.h directly Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 13/43] qom: cleanup struct Error references Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 14/43] qom: add pointer to int property helpers Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 15/43] pci: fix up w64 size calculation helper Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 16/43] fw_cfg: interface to trigger callback on read Michael S. Tsirkin
2013-10-14 14:58 ` [Qemu-devel] [PULL 17/43] loader: support for unmapped ROM blobs Michael S. Tsirkin
2013-11-08  0:54   ` Alexander Graf
2013-11-09 17:21     ` Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 18/43] pcie_host: expose UNMAPPED macro Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 19/43] pcie_host: expose address format Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 20/43] q35: use macro for MCFG property name Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 21/43] q35: expose mmcfg size as a property Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 22/43] i386: add ACPI table files from seabios Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 23/43] acpi: add rules to compile ASL source Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 24/43] acpi: pre-compiled ASL files Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 25/43] acpi: ssdt pcihp: updat generated file Michael S. Tsirkin
2013-10-14 22:32   ` Anthony Liguori
2013-10-15  5:24     ` Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 26/43] loader: use file path size from fw_cfg.h Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 27/43] i386: add bios linker/loader Michael S. Tsirkin
2013-10-14 14:59 ` [Qemu-devel] [PULL 28/43] loader: allow adding ROMs in done callbacks Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 29/43] i386: define pc guest info Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 30/43] acpi/piix: add macros for acpi property names Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 31/43] piix: APIs for pc guest info Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 32/43] ich9: " Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 33/43] pvpanic: add API to access io port Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 34/43] hpet: add API to find it Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 35/43] acpi: add interface to access user-installed tables Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 36/43] pc: use new api to add builtin tables Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 37/43] i386: ACPI table generation code from seabios Michael S. Tsirkin
2013-10-14 15:00 ` [Qemu-devel] [PULL 38/43] ssdt: fix PBLK length Michael S. Tsirkin
2013-10-14 15:01 ` [Qemu-devel] [PULL 39/43] ssdt-proc: update generated file Michael S. Tsirkin
2013-10-14 15:01 ` [Qemu-devel] [PULL 40/43] pci: add pci_for_each_bus_depth_first Michael S. Tsirkin
2013-10-14 15:01 ` [Qemu-devel] [PULL 41/43] pcihp: generalization of piix4 acpi Michael S. Tsirkin
2013-10-14 22:36   ` Anthony Liguori
2013-10-15  5:23     ` Michael S. Tsirkin
2013-10-14 15:01 ` [Qemu-devel] [PULL 42/43] piix4: add acpi pci hotplug support Michael S. Tsirkin
2013-10-15 14:31   ` Paolo Bonzini
2013-10-15 14:35     ` Michael S. Tsirkin
2013-10-15 14:50       ` Paolo Bonzini
2013-10-15 14:54         ` Michael S. Tsirkin
2013-10-15 14:54           ` Paolo Bonzini
2013-10-15 15:07             ` Michael S. Tsirkin
2013-10-15 15:09               ` Paolo Bonzini
2013-10-15 15:16                 ` Michael S. Tsirkin
2013-10-15 16:27         ` Anthony Liguori
2013-10-15 20:17           ` Michael S. Tsirkin
2013-10-16 15:03             ` Paolo Bonzini
2013-10-16 16:38             ` Anthony Liguori
2013-10-16 18:18               ` Michael S. Tsirkin
2013-10-16 18:18                 ` Anthony Liguori
2013-10-16 18:37                   ` Michael S. Tsirkin
2013-10-16 21:26                     ` Paolo Bonzini
2013-10-16 22:03                       ` Michael S. Tsirkin
2013-10-16 22:25                         ` Paolo Bonzini
2013-10-16 23:52                           ` Anthony Liguori
2013-10-17  5:22                             ` Michael S. Tsirkin
2013-10-17  5:32                           ` Michael S. Tsirkin
2013-10-17  5:48                             ` Gleb Natapov
2013-10-17  5:34                           ` Michael S. Tsirkin
2013-10-17 11:06                             ` Paolo Bonzini
2013-10-17  8:18                     ` Gerd Hoffmann
2013-12-10 11:15   ` Igor Mammedov
2013-10-14 15:01 ` [Qemu-devel] [PULL 43/43] acpi-build: enable hotplug for PCI bridges Michael S. Tsirkin
2013-10-14 22:42 ` [Qemu-devel] [PULL 00/43] pci, pc, acpi fixes, enhancements Anthony Liguori
2013-10-15  5:28   ` Michael S. Tsirkin
2013-10-15 13:51     ` Anthony Liguori
2013-10-15 14:01       ` Paolo Bonzini
2013-10-15 14:17         ` Anthony Liguori
2013-10-15 14:24           ` Michael S. Tsirkin
2013-10-15 14:09       ` Igor Mammedov
2013-10-15 14:20       ` Michael S. Tsirkin
2013-10-15 14:21         ` Anthony Liguori
2013-10-15 14:30           ` Michael S. Tsirkin
2013-10-15 14:37           ` Michael S. Tsirkin
2013-10-15 14:51           ` Michael S. Tsirkin
2013-10-15 15:27             ` Igor Mammedov
2013-10-15 15:37             ` Michael S. Tsirkin
2013-10-15  5:33   ` Michael S. Tsirkin
2013-10-15 11:53   ` Igor Mammedov
2013-10-15 13:43   ` Gerd Hoffmann
2013-10-15 13:53     ` Anthony Liguori
2013-10-15 14:21       ` Michael S. Tsirkin
2013-10-15 14:14   ` Gerd Hoffmann [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1381846479.3327.0.camel@nilsson.home.kraxel.org \
    --to=kraxel@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=imammedo@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).