| Message ID | 1381762577-12526-1-git-send-email-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 14/10/2013 16:57, Michael S. Tsirkin ha scritto: > pci, pc, acpi fixes, enhancements > > This includes some pretty big changes: > - pci master abort support by Marcel > - pci IRQ API rework by Marcel > - acpi generation and pci bridge hotplug support by myself > > Everything has gone through several revisions, latest versions have been on > list for a while without any more comments, tested by several > people. > > Please pull for 1.7. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Are you going to do another pull request with the virtio and bus-reset fixes? Paolo
On Mon, Oct 14, 2013 at 04:57:19PM +0200, Paolo Bonzini wrote: > Il 14/10/2013 16:57, Michael S. Tsirkin ha scritto: > > pci, pc, acpi fixes, enhancements > > > > This includes some pretty big changes: > > - pci master abort support by Marcel > > - pci IRQ API rework by Marcel > > - acpi generation and pci bridge hotplug support by myself > > > > Everything has gone through several revisions, latest versions have been on > > list for a while without any more comments, tested by several > > people. > > > > Please pull for 1.7. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Are you going to do another pull request with the virtio and bus-reset > fixes? > > Paolo I missed that you did the testing of the post-order changed. I'll put them on my branch but I'd rather this kind of change went through a bit more testing so - next pull. For virtio I thought you said Andreas wants to merge it?
Il 14/10/2013 17:12, Michael S. Tsirkin ha scritto: > > Are you going to do another pull request with the virtio and bus-reset > > fixes? > > I missed that you did the testing of the post-order changed. > I'll put them on my branch but I'd rather this kind of > change went through a bit more testing so - next pull. Ok. As long as they're on your radar, I don't mind if you want to delay them to 1.8. > For virtio I thought you said Andreas wants to merge it? No, there are conflicts with one of his series, but he is fine with mine going in first and he'd rather not handle it himself. With your (and perhaps Andreas and Alex's) Acked-by I can send the pull request myself. Paolo
"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. 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. 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? Or 10k SLOC of code into QEMU that includes breaking migration compatibility. Regards, Anthony Liguori > The following changes since commit e26d3e734650640fabd7d95ace4f3a6f88725e0b: > > smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony > > for you to fetch changes up to 6cab1e7000021fa6a487f67e1dba986f68fee30d: > > acpi-build: enable hotplug for PCI bridges (2013-10-14 17:48:58 +0300) > > ---------------------------------------------------------------- > pci, pc, acpi fixes, enhancements > > This includes some pretty big changes: > - pci master abort support by Marcel > - pci IRQ API rework by Marcel > - acpi generation and pci bridge hotplug support by myself > > Everything has gone through several revisions, latest versions have been on > list for a while without any more comments, tested by several > people. > > Please pull for 1.7. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > ---------------------------------------------------------------- > Igor Mammedov (1): > cleanup object.h: include error.h directly > > Marcel Apfelbaum (11): > memory: Change MemoryRegion priorities from unsigned to signed > docs/memory: Explictly state that MemoryRegion priority is signed > hw/pci: partially handle pci master abort > hw/core: Add interface to allocate and free a single IRQ > hw/pci: add pci wrappers for allocating and asserting irqs > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > hw/vmxnet3: set interrupts using pci irq wrappers > hw/vfio: set interrupts using pci irq wrappers > hw: set interrupts using pci irq wrappers > hw/pcie: AER and hot-plug events must use device's interrupt > hw/pci: removed irq field from PCIDevice > > Michael S. Tsirkin (31): > qom: cleanup struct Error references > qom: add pointer to int property helpers > pci: fix up w64 size calculation helper > fw_cfg: interface to trigger callback on read > loader: support for unmapped ROM blobs > pcie_host: expose UNMAPPED macro > pcie_host: expose address format > q35: use macro for MCFG property name > q35: expose mmcfg size as a property > i386: add ACPI table files from seabios > acpi: add rules to compile ASL source > acpi: pre-compiled ASL files > acpi: ssdt pcihp: updat generated file > loader: use file path size from fw_cfg.h > i386: add bios linker/loader > loader: allow adding ROMs in done callbacks > i386: define pc guest info > acpi/piix: add macros for acpi property names > piix: APIs for pc guest info > ich9: APIs for pc guest info > pvpanic: add API to access io port > hpet: add API to find it > acpi: add interface to access user-installed tables > pc: use new api to add builtin tables > i386: ACPI table generation code from seabios > ssdt: fix PBLK length > ssdt-proc: update generated file > pci: add pci_for_each_bus_depth_first > pcihp: generalization of piix4 acpi > piix4: add acpi pci hotplug support > acpi-build: enable hotplug for PCI bridges > > configure | 9 +- > hw/i386/acpi-build.h | 9 + > hw/i386/acpi-defs.h | 331 ++ > hw/i386/bios-linker-loader.h | 27 + > hw/lm32/lm32_hwsetup.h | 2 +- > include/exec/memory.h | 4 +- > include/hw/acpi/acpi.h | 4 + > include/hw/acpi/ich9.h | 2 + > include/hw/acpi/pcihp.h | 72 + > include/hw/acpi/piix4.h | 8 + > include/hw/i386/ich9.h | 2 + > include/hw/i386/pc.h | 27 + > include/hw/irq.h | 7 + > include/hw/loader.h | 8 +- > include/hw/nvram/fw_cfg.h | 8 +- > include/hw/pci-host/q35.h | 2 + > include/hw/pci/pci.h | 40 +- > include/hw/pci/pci_bus.h | 1 + > include/hw/pci/pcie.h | 18 - > include/hw/pci/pcie_host.h | 27 + > include/hw/sysbus.h | 2 +- > include/hw/timer/hpet.h | 2 + > include/qom/object.h | 73 +- > hw/acpi/core.c | 40 + > hw/acpi/ich9.c | 24 + > hw/acpi/pcihp.c | 312 ++ > hw/acpi/piix4.c | 125 +- > hw/audio/ac97.c | 4 +- > hw/audio/es1370.c | 4 +- > hw/audio/intel-hda.c | 2 +- > hw/block/nvme.c | 2 +- > hw/char/serial-pci.c | 5 +- > hw/char/tpci200.c | 8 +- > hw/core/irq.c | 16 + > hw/core/loader.c | 31 +- > hw/core/sysbus.c | 4 +- > hw/display/qxl.c | 2 +- > hw/i386/acpi-build.c | 1420 +++++++ > hw/i386/bios-linker-loader.c | 158 + > hw/i386/pc.c | 25 +- > hw/i386/pc_piix.c | 5 + > hw/i386/pc_q35.c | 3 + > hw/ide/cmd646.c | 2 +- > hw/ide/ich.c | 3 +- > hw/isa/lpc_ich9.c | 40 + > hw/isa/vt82c686.c | 2 +- > hw/misc/ivshmem.c | 2 +- > hw/misc/pvpanic.c | 13 +- > hw/misc/vfio.c | 11 +- > hw/net/e1000.c | 2 +- > hw/net/eepro100.c | 4 +- > hw/net/ne2000.c | 3 +- > hw/net/pcnet-pci.c | 3 +- > hw/net/rtl8139.c | 2 +- > hw/net/vmxnet3.c | 13 +- > hw/nvram/fw_cfg.c | 33 +- > hw/pci-bridge/pci_bridge_dev.c | 2 +- > hw/pci-host/piix.c | 8 + > hw/pci-host/q35.c | 26 +- > hw/pci/pci.c | 100 +- > hw/pci/pcie.c | 4 +- > hw/pci/pcie_aer.c | 4 +- > hw/pci/pcie_host.c | 24 - > hw/pci/shpc.c | 2 +- > hw/scsi/esp-pci.c | 3 +- > hw/scsi/lsi53c895a.c | 2 +- > hw/scsi/megasas.c | 6 +- > hw/scsi/vmw_pvscsi.c | 2 +- > hw/timer/hpet.c | 5 + > hw/usb/hcd-ehci-pci.c | 2 +- > hw/usb/hcd-ohci.c | 2 +- > hw/usb/hcd-uhci.c | 6 +- > hw/usb/hcd-xhci.c | 7 +- > hw/virtio/virtio-pci.c | 4 +- > memory.c | 4 +- > qom/object.c | 60 + > vl.c | 3 + > docs/memory.txt | 4 + > hw/acpi/Makefile.objs | 2 +- > hw/i386/Makefile.objs | 27 + > hw/i386/acpi-dsdt-cpu-hotplug.dsl | 93 + > hw/i386/acpi-dsdt-dbug.dsl | 41 + > hw/i386/acpi-dsdt-hpet.dsl | 51 + > hw/i386/acpi-dsdt-isa.dsl | 117 + > hw/i386/acpi-dsdt-pci-crs.dsl | 105 + > hw/i386/acpi-dsdt.dsl | 341 ++ > hw/i386/acpi-dsdt.hex.generated | 4409 +++++++++++++++++++++ > hw/i386/q35-acpi-dsdt.dsl | 452 +++ > hw/i386/q35-acpi-dsdt.hex.generated | 7346 +++++++++++++++++++++++++++++++++++ > hw/i386/ssdt-misc.dsl | 119 + > hw/i386/ssdt-misc.hex.generated | 386 ++ > hw/i386/ssdt-pcihp.dsl | 50 + > hw/i386/ssdt-pcihp.hex.generated | 108 + > hw/i386/ssdt-proc.dsl | 63 + > hw/i386/ssdt-proc.hex.generated | 134 + > scripts/acpi_extract.py | 362 ++ > scripts/acpi_extract_preprocess.py | 51 + > scripts/update-acpi.sh | 4 + > 98 files changed, 17366 insertions(+), 183 deletions(-) > create mode 100644 hw/i386/acpi-build.h > create mode 100644 hw/i386/acpi-defs.h > create mode 100644 hw/i386/bios-linker-loader.h > create mode 100644 include/hw/acpi/pcihp.h > create mode 100644 include/hw/acpi/piix4.h > create mode 100644 hw/acpi/pcihp.c > create mode 100644 hw/i386/acpi-build.c > create mode 100644 hw/i386/bios-linker-loader.c > create mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl > create mode 100644 hw/i386/acpi-dsdt-dbug.dsl > create mode 100644 hw/i386/acpi-dsdt-hpet.dsl > create mode 100644 hw/i386/acpi-dsdt-isa.dsl > create mode 100644 hw/i386/acpi-dsdt-pci-crs.dsl > create mode 100644 hw/i386/acpi-dsdt.dsl > create mode 100644 hw/i386/acpi-dsdt.hex.generated > create mode 100644 hw/i386/q35-acpi-dsdt.dsl > create mode 100644 hw/i386/q35-acpi-dsdt.hex.generated > create mode 100644 hw/i386/ssdt-misc.dsl > create mode 100644 hw/i386/ssdt-misc.hex.generated > create mode 100644 hw/i386/ssdt-pcihp.dsl > create mode 100644 hw/i386/ssdt-pcihp.hex.generated > create mode 100644 hw/i386/ssdt-proc.dsl > create mode 100644 hw/i386/ssdt-proc.hex.generated > create mode 100755 scripts/acpi_extract.py > create mode 100755 scripts/acpi_extract_preprocess.py > create mode 100644 scripts/update-acpi.sh
On Mon, Oct 14, 2013 at 03:42:37PM -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. Last minute? This has been on list for months. > 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. It isn't. The point is getting ACPI out of seabios. OK what if I drop the bridge hotplug part? > 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, we also get alternative firmwares working correctly with QEMU. > Or 10k SLOC of code into QEMU that includes breaking migration > compatibility. AFAIK it doesn't break migration compatibility. > Regards, > > Anthony Liguori > > > The following changes since commit e26d3e734650640fabd7d95ace4f3a6f88725e0b: > > > > smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300) > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony > > > > for you to fetch changes up to 6cab1e7000021fa6a487f67e1dba986f68fee30d: > > > > acpi-build: enable hotplug for PCI bridges (2013-10-14 17:48:58 +0300) > > > > ---------------------------------------------------------------- > > pci, pc, acpi fixes, enhancements > > > > This includes some pretty big changes: > > - pci master abort support by Marcel > > - pci IRQ API rework by Marcel > > - acpi generation and pci bridge hotplug support by myself > > > > Everything has gone through several revisions, latest versions have been on > > list for a while without any more comments, tested by several > > people. > > > > Please pull for 1.7. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > ---------------------------------------------------------------- > > Igor Mammedov (1): > > cleanup object.h: include error.h directly > > > > Marcel Apfelbaum (11): > > memory: Change MemoryRegion priorities from unsigned to signed > > docs/memory: Explictly state that MemoryRegion priority is signed > > hw/pci: partially handle pci master abort > > hw/core: Add interface to allocate and free a single IRQ > > hw/pci: add pci wrappers for allocating and asserting irqs > > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > > hw/vmxnet3: set interrupts using pci irq wrappers > > hw/vfio: set interrupts using pci irq wrappers > > hw: set interrupts using pci irq wrappers > > hw/pcie: AER and hot-plug events must use device's interrupt > > hw/pci: removed irq field from PCIDevice > > > > Michael S. Tsirkin (31): > > qom: cleanup struct Error references > > qom: add pointer to int property helpers > > pci: fix up w64 size calculation helper > > fw_cfg: interface to trigger callback on read > > loader: support for unmapped ROM blobs > > pcie_host: expose UNMAPPED macro > > pcie_host: expose address format > > q35: use macro for MCFG property name > > q35: expose mmcfg size as a property > > i386: add ACPI table files from seabios > > acpi: add rules to compile ASL source > > acpi: pre-compiled ASL files > > acpi: ssdt pcihp: updat generated file > > loader: use file path size from fw_cfg.h > > i386: add bios linker/loader > > loader: allow adding ROMs in done callbacks > > i386: define pc guest info > > acpi/piix: add macros for acpi property names > > piix: APIs for pc guest info > > ich9: APIs for pc guest info > > pvpanic: add API to access io port > > hpet: add API to find it > > acpi: add interface to access user-installed tables > > pc: use new api to add builtin tables > > i386: ACPI table generation code from seabios > > ssdt: fix PBLK length > > ssdt-proc: update generated file > > pci: add pci_for_each_bus_depth_first > > pcihp: generalization of piix4 acpi > > piix4: add acpi pci hotplug support > > acpi-build: enable hotplug for PCI bridges > > > > configure | 9 +- > > hw/i386/acpi-build.h | 9 + > > hw/i386/acpi-defs.h | 331 ++ > > hw/i386/bios-linker-loader.h | 27 + > > hw/lm32/lm32_hwsetup.h | 2 +- > > include/exec/memory.h | 4 +- > > include/hw/acpi/acpi.h | 4 + > > include/hw/acpi/ich9.h | 2 + > > include/hw/acpi/pcihp.h | 72 + > > include/hw/acpi/piix4.h | 8 + > > include/hw/i386/ich9.h | 2 + > > include/hw/i386/pc.h | 27 + > > include/hw/irq.h | 7 + > > include/hw/loader.h | 8 +- > > include/hw/nvram/fw_cfg.h | 8 +- > > include/hw/pci-host/q35.h | 2 + > > include/hw/pci/pci.h | 40 +- > > include/hw/pci/pci_bus.h | 1 + > > include/hw/pci/pcie.h | 18 - > > include/hw/pci/pcie_host.h | 27 + > > include/hw/sysbus.h | 2 +- > > include/hw/timer/hpet.h | 2 + > > include/qom/object.h | 73 +- > > hw/acpi/core.c | 40 + > > hw/acpi/ich9.c | 24 + > > hw/acpi/pcihp.c | 312 ++ > > hw/acpi/piix4.c | 125 +- > > hw/audio/ac97.c | 4 +- > > hw/audio/es1370.c | 4 +- > > hw/audio/intel-hda.c | 2 +- > > hw/block/nvme.c | 2 +- > > hw/char/serial-pci.c | 5 +- > > hw/char/tpci200.c | 8 +- > > hw/core/irq.c | 16 + > > hw/core/loader.c | 31 +- > > hw/core/sysbus.c | 4 +- > > hw/display/qxl.c | 2 +- > > hw/i386/acpi-build.c | 1420 +++++++ > > hw/i386/bios-linker-loader.c | 158 + > > hw/i386/pc.c | 25 +- > > hw/i386/pc_piix.c | 5 + > > hw/i386/pc_q35.c | 3 + > > hw/ide/cmd646.c | 2 +- > > hw/ide/ich.c | 3 +- > > hw/isa/lpc_ich9.c | 40 + > > hw/isa/vt82c686.c | 2 +- > > hw/misc/ivshmem.c | 2 +- > > hw/misc/pvpanic.c | 13 +- > > hw/misc/vfio.c | 11 +- > > hw/net/e1000.c | 2 +- > > hw/net/eepro100.c | 4 +- > > hw/net/ne2000.c | 3 +- > > hw/net/pcnet-pci.c | 3 +- > > hw/net/rtl8139.c | 2 +- > > hw/net/vmxnet3.c | 13 +- > > hw/nvram/fw_cfg.c | 33 +- > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > hw/pci-host/piix.c | 8 + > > hw/pci-host/q35.c | 26 +- > > hw/pci/pci.c | 100 +- > > hw/pci/pcie.c | 4 +- > > hw/pci/pcie_aer.c | 4 +- > > hw/pci/pcie_host.c | 24 - > > hw/pci/shpc.c | 2 +- > > hw/scsi/esp-pci.c | 3 +- > > hw/scsi/lsi53c895a.c | 2 +- > > hw/scsi/megasas.c | 6 +- > > hw/scsi/vmw_pvscsi.c | 2 +- > > hw/timer/hpet.c | 5 + > > hw/usb/hcd-ehci-pci.c | 2 +- > > hw/usb/hcd-ohci.c | 2 +- > > hw/usb/hcd-uhci.c | 6 +- > > hw/usb/hcd-xhci.c | 7 +- > > hw/virtio/virtio-pci.c | 4 +- > > memory.c | 4 +- > > qom/object.c | 60 + > > vl.c | 3 + > > docs/memory.txt | 4 + > > hw/acpi/Makefile.objs | 2 +- > > hw/i386/Makefile.objs | 27 + > > hw/i386/acpi-dsdt-cpu-hotplug.dsl | 93 + > > hw/i386/acpi-dsdt-dbug.dsl | 41 + > > hw/i386/acpi-dsdt-hpet.dsl | 51 + > > hw/i386/acpi-dsdt-isa.dsl | 117 + > > hw/i386/acpi-dsdt-pci-crs.dsl | 105 + > > hw/i386/acpi-dsdt.dsl | 341 ++ > > hw/i386/acpi-dsdt.hex.generated | 4409 +++++++++++++++++++++ > > hw/i386/q35-acpi-dsdt.dsl | 452 +++ > > hw/i386/q35-acpi-dsdt.hex.generated | 7346 +++++++++++++++++++++++++++++++++++ > > hw/i386/ssdt-misc.dsl | 119 + > > hw/i386/ssdt-misc.hex.generated | 386 ++ > > hw/i386/ssdt-pcihp.dsl | 50 + > > hw/i386/ssdt-pcihp.hex.generated | 108 + > > hw/i386/ssdt-proc.dsl | 63 + > > hw/i386/ssdt-proc.hex.generated | 134 + > > scripts/acpi_extract.py | 362 ++ > > scripts/acpi_extract_preprocess.py | 51 + > > scripts/update-acpi.sh | 4 + > > 98 files changed, 17366 insertions(+), 183 deletions(-) > > create mode 100644 hw/i386/acpi-build.h > > create mode 100644 hw/i386/acpi-defs.h > > create mode 100644 hw/i386/bios-linker-loader.h > > create mode 100644 include/hw/acpi/pcihp.h > > create mode 100644 include/hw/acpi/piix4.h > > create mode 100644 hw/acpi/pcihp.c > > create mode 100644 hw/i386/acpi-build.c > > create mode 100644 hw/i386/bios-linker-loader.c > > create mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl > > create mode 100644 hw/i386/acpi-dsdt-dbug.dsl > > create mode 100644 hw/i386/acpi-dsdt-hpet.dsl > > create mode 100644 hw/i386/acpi-dsdt-isa.dsl > > create mode 100644 hw/i386/acpi-dsdt-pci-crs.dsl > > create mode 100644 hw/i386/acpi-dsdt.dsl > > create mode 100644 hw/i386/acpi-dsdt.hex.generated > > create mode 100644 hw/i386/q35-acpi-dsdt.dsl > > create mode 100644 hw/i386/q35-acpi-dsdt.hex.generated > > create mode 100644 hw/i386/ssdt-misc.dsl > > create mode 100644 hw/i386/ssdt-misc.hex.generated > > create mode 100644 hw/i386/ssdt-pcihp.dsl > > create mode 100644 hw/i386/ssdt-pcihp.hex.generated > > create mode 100644 hw/i386/ssdt-proc.dsl > > create mode 100644 hw/i386/ssdt-proc.hex.generated > > create mode 100755 scripts/acpi_extract.py > > create mode 100755 scripts/acpi_extract_preprocess.py > > create mode 100644 scripts/update-acpi.sh
On Mon, Oct 14, 2013 at 03:42:37PM -0700, Anthony Liguori wrote: > This is a huge series and I still am not convinced this is the right > path forward. Also, this kind of response is quite unexpected after this direction was discussed multiple times and largely agreed upon on the phone meeting, and after multiple people spent lots of time discussing and testing this on list.
On Mon, 14 Oct 2013 15:42:37 -0700 Anthony Liguori <anthony@codemonkey.ws> 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. 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. > > 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? It's also needed for memory hotplug to make any progress, there is no point posting it before ACPI tables are merged. > Or 10k SLOC of code into QEMU that includes breaking migration > compatibility. > > Regards, > > Anthony Liguori > > > The following changes since commit e26d3e734650640fabd7d95ace4f3a6f88725e0b: > > > > smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300) > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony > > > > for you to fetch changes up to 6cab1e7000021fa6a487f67e1dba986f68fee30d: > > > > acpi-build: enable hotplug for PCI bridges (2013-10-14 17:48:58 +0300) > > > > ---------------------------------------------------------------- > > pci, pc, acpi fixes, enhancements > > > > This includes some pretty big changes: > > - pci master abort support by Marcel > > - pci IRQ API rework by Marcel > > - acpi generation and pci bridge hotplug support by myself > > > > Everything has gone through several revisions, latest versions have been on > > list for a while without any more comments, tested by several > > people. > > > > Please pull for 1.7. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > ---------------------------------------------------------------- > > Igor Mammedov (1): > > cleanup object.h: include error.h directly > > > > Marcel Apfelbaum (11): > > memory: Change MemoryRegion priorities from unsigned to signed > > docs/memory: Explictly state that MemoryRegion priority is signed > > hw/pci: partially handle pci master abort > > hw/core: Add interface to allocate and free a single IRQ > > hw/pci: add pci wrappers for allocating and asserting irqs > > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > > hw/vmxnet3: set interrupts using pci irq wrappers > > hw/vfio: set interrupts using pci irq wrappers > > hw: set interrupts using pci irq wrappers > > hw/pcie: AER and hot-plug events must use device's interrupt > > hw/pci: removed irq field from PCIDevice > > > > Michael S. Tsirkin (31): > > qom: cleanup struct Error references > > qom: add pointer to int property helpers > > pci: fix up w64 size calculation helper > > fw_cfg: interface to trigger callback on read > > loader: support for unmapped ROM blobs > > pcie_host: expose UNMAPPED macro > > pcie_host: expose address format > > q35: use macro for MCFG property name > > q35: expose mmcfg size as a property > > i386: add ACPI table files from seabios > > acpi: add rules to compile ASL source > > acpi: pre-compiled ASL files > > acpi: ssdt pcihp: updat generated file > > loader: use file path size from fw_cfg.h > > i386: add bios linker/loader > > loader: allow adding ROMs in done callbacks > > i386: define pc guest info > > acpi/piix: add macros for acpi property names > > piix: APIs for pc guest info > > ich9: APIs for pc guest info > > pvpanic: add API to access io port > > hpet: add API to find it > > acpi: add interface to access user-installed tables > > pc: use new api to add builtin tables > > i386: ACPI table generation code from seabios > > ssdt: fix PBLK length > > ssdt-proc: update generated file > > pci: add pci_for_each_bus_depth_first > > pcihp: generalization of piix4 acpi > > piix4: add acpi pci hotplug support > > acpi-build: enable hotplug for PCI bridges > > > > configure | 9 +- > > hw/i386/acpi-build.h | 9 + > > hw/i386/acpi-defs.h | 331 ++ > > hw/i386/bios-linker-loader.h | 27 + > > hw/lm32/lm32_hwsetup.h | 2 +- > > include/exec/memory.h | 4 +- > > include/hw/acpi/acpi.h | 4 + > > include/hw/acpi/ich9.h | 2 + > > include/hw/acpi/pcihp.h | 72 + > > include/hw/acpi/piix4.h | 8 + > > include/hw/i386/ich9.h | 2 + > > include/hw/i386/pc.h | 27 + > > include/hw/irq.h | 7 + > > include/hw/loader.h | 8 +- > > include/hw/nvram/fw_cfg.h | 8 +- > > include/hw/pci-host/q35.h | 2 + > > include/hw/pci/pci.h | 40 +- > > include/hw/pci/pci_bus.h | 1 + > > include/hw/pci/pcie.h | 18 - > > include/hw/pci/pcie_host.h | 27 + > > include/hw/sysbus.h | 2 +- > > include/hw/timer/hpet.h | 2 + > > include/qom/object.h | 73 +- > > hw/acpi/core.c | 40 + > > hw/acpi/ich9.c | 24 + > > hw/acpi/pcihp.c | 312 ++ > > hw/acpi/piix4.c | 125 +- > > hw/audio/ac97.c | 4 +- > > hw/audio/es1370.c | 4 +- > > hw/audio/intel-hda.c | 2 +- > > hw/block/nvme.c | 2 +- > > hw/char/serial-pci.c | 5 +- > > hw/char/tpci200.c | 8 +- > > hw/core/irq.c | 16 + > > hw/core/loader.c | 31 +- > > hw/core/sysbus.c | 4 +- > > hw/display/qxl.c | 2 +- > > hw/i386/acpi-build.c | 1420 +++++++ > > hw/i386/bios-linker-loader.c | 158 + > > hw/i386/pc.c | 25 +- > > hw/i386/pc_piix.c | 5 + > > hw/i386/pc_q35.c | 3 + > > hw/ide/cmd646.c | 2 +- > > hw/ide/ich.c | 3 +- > > hw/isa/lpc_ich9.c | 40 + > > hw/isa/vt82c686.c | 2 +- > > hw/misc/ivshmem.c | 2 +- > > hw/misc/pvpanic.c | 13 +- > > hw/misc/vfio.c | 11 +- > > hw/net/e1000.c | 2 +- > > hw/net/eepro100.c | 4 +- > > hw/net/ne2000.c | 3 +- > > hw/net/pcnet-pci.c | 3 +- > > hw/net/rtl8139.c | 2 +- > > hw/net/vmxnet3.c | 13 +- > > hw/nvram/fw_cfg.c | 33 +- > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > hw/pci-host/piix.c | 8 + > > hw/pci-host/q35.c | 26 +- > > hw/pci/pci.c | 100 +- > > hw/pci/pcie.c | 4 +- > > hw/pci/pcie_aer.c | 4 +- > > hw/pci/pcie_host.c | 24 - > > hw/pci/shpc.c | 2 +- > > hw/scsi/esp-pci.c | 3 +- > > hw/scsi/lsi53c895a.c | 2 +- > > hw/scsi/megasas.c | 6 +- > > hw/scsi/vmw_pvscsi.c | 2 +- > > hw/timer/hpet.c | 5 + > > hw/usb/hcd-ehci-pci.c | 2 +- > > hw/usb/hcd-ohci.c | 2 +- > > hw/usb/hcd-uhci.c | 6 +- > > hw/usb/hcd-xhci.c | 7 +- > > hw/virtio/virtio-pci.c | 4 +- > > memory.c | 4 +- > > qom/object.c | 60 + > > vl.c | 3 + > > docs/memory.txt | 4 + > > hw/acpi/Makefile.objs | 2 +- > > hw/i386/Makefile.objs | 27 + > > hw/i386/acpi-dsdt-cpu-hotplug.dsl | 93 + > > hw/i386/acpi-dsdt-dbug.dsl | 41 + > > hw/i386/acpi-dsdt-hpet.dsl | 51 + > > hw/i386/acpi-dsdt-isa.dsl | 117 + > > hw/i386/acpi-dsdt-pci-crs.dsl | 105 + > > hw/i386/acpi-dsdt.dsl | 341 ++ > > hw/i386/acpi-dsdt.hex.generated | 4409 +++++++++++++++++++++ > > hw/i386/q35-acpi-dsdt.dsl | 452 +++ > > hw/i386/q35-acpi-dsdt.hex.generated | 7346 +++++++++++++++++++++++++++++++++++ > > hw/i386/ssdt-misc.dsl | 119 + > > hw/i386/ssdt-misc.hex.generated | 386 ++ > > hw/i386/ssdt-pcihp.dsl | 50 + > > hw/i386/ssdt-pcihp.hex.generated | 108 + > > hw/i386/ssdt-proc.dsl | 63 + > > hw/i386/ssdt-proc.hex.generated | 134 + > > scripts/acpi_extract.py | 362 ++ > > scripts/acpi_extract_preprocess.py | 51 + > > scripts/update-acpi.sh | 4 + > > 98 files changed, 17366 insertions(+), 183 deletions(-) > > create mode 100644 hw/i386/acpi-build.h > > create mode 100644 hw/i386/acpi-defs.h > > create mode 100644 hw/i386/bios-linker-loader.h > > create mode 100644 include/hw/acpi/pcihp.h > > create mode 100644 include/hw/acpi/piix4.h > > create mode 100644 hw/acpi/pcihp.c > > create mode 100644 hw/i386/acpi-build.c > > create mode 100644 hw/i386/bios-linker-loader.c > > create mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl > > create mode 100644 hw/i386/acpi-dsdt-dbug.dsl > > create mode 100644 hw/i386/acpi-dsdt-hpet.dsl > > create mode 100644 hw/i386/acpi-dsdt-isa.dsl > > create mode 100644 hw/i386/acpi-dsdt-pci-crs.dsl > > create mode 100644 hw/i386/acpi-dsdt.dsl > > create mode 100644 hw/i386/acpi-dsdt.hex.generated > > create mode 100644 hw/i386/q35-acpi-dsdt.dsl > > create mode 100644 hw/i386/q35-acpi-dsdt.hex.generated > > create mode 100644 hw/i386/ssdt-misc.dsl > > create mode 100644 hw/i386/ssdt-misc.hex.generated > > create mode 100644 hw/i386/ssdt-pcihp.dsl > > create mode 100644 hw/i386/ssdt-pcihp.hex.generated > > create mode 100644 hw/i386/ssdt-proc.dsl > > create mode 100644 hw/i386/ssdt-proc.hex.generated > > create mode 100755 scripts/acpi_extract.py > > create mode 100755 scripts/acpi_extract_preprocess.py > > create mode 100644 scripts/update-acpi.sh
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, 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
On Mon, Oct 14, 2013 at 10:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Oct 14, 2013 at 03:42:37PM -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. > > Last minute? This has been on list for months. It doesn't matter how long the patches have been on the list. We have a very short testing cycle for releases. This pull request lacks any automated testing. Something like this really should come with at least some qtest validation that we are still generating the right ACPI tables but certainly could have simpler unit tests too. There is no statement about what manual testing you actually did. Have you run kvm autotest? Have you tested a variety of Windows guests? The pull request has a patch with a binary diff and a comment of: "update generated file, not sure what changed" And that didn't concern you prior to sending the pull request? This series is not ready to merge. >> 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. > > It isn't. The point is getting ACPI out of seabios. > OK what if I drop the bridge hotplug part? How does that address the above? >> 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, we also get alternative firmwares working correctly with QEMU. > >> Or 10k SLOC of code into QEMU that includes breaking migration >> compatibility. > > AFAIK it doesn't break migration compatibility. From 41/43: "The interface is actually backwards-compatible with existing PIIX4 ACPI (though not migration compatible)." And does "AFAIK" translate to, "I have tested migration from new and old and old and new with this series"? I suspect the answer is no. Regards, Anthony Liguori >> Regards, >> >> Anthony Liguori >> >> > The following changes since commit e26d3e734650640fabd7d95ace4f3a6f88725e0b: >> > >> > smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300) >> > >> > are available in the git repository at: >> > >> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony >> > >> > for you to fetch changes up to 6cab1e7000021fa6a487f67e1dba986f68fee30d: >> > >> > acpi-build: enable hotplug for PCI bridges (2013-10-14 17:48:58 +0300) >> > >> > ---------------------------------------------------------------- >> > pci, pc, acpi fixes, enhancements >> > >> > This includes some pretty big changes: >> > - pci master abort support by Marcel >> > - pci IRQ API rework by Marcel >> > - acpi generation and pci bridge hotplug support by myself >> > >> > Everything has gone through several revisions, latest versions have been on >> > list for a while without any more comments, tested by several >> > people. >> > >> > Please pull for 1.7. >> > >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> > >> > ---------------------------------------------------------------- >> > Igor Mammedov (1): >> > cleanup object.h: include error.h directly >> > >> > Marcel Apfelbaum (11): >> > memory: Change MemoryRegion priorities from unsigned to signed >> > docs/memory: Explictly state that MemoryRegion priority is signed >> > hw/pci: partially handle pci master abort >> > hw/core: Add interface to allocate and free a single IRQ >> > hw/pci: add pci wrappers for allocating and asserting irqs >> > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init >> > hw/vmxnet3: set interrupts using pci irq wrappers >> > hw/vfio: set interrupts using pci irq wrappers >> > hw: set interrupts using pci irq wrappers >> > hw/pcie: AER and hot-plug events must use device's interrupt >> > hw/pci: removed irq field from PCIDevice >> > >> > Michael S. Tsirkin (31): >> > qom: cleanup struct Error references >> > qom: add pointer to int property helpers >> > pci: fix up w64 size calculation helper >> > fw_cfg: interface to trigger callback on read >> > loader: support for unmapped ROM blobs >> > pcie_host: expose UNMAPPED macro >> > pcie_host: expose address format >> > q35: use macro for MCFG property name >> > q35: expose mmcfg size as a property >> > i386: add ACPI table files from seabios >> > acpi: add rules to compile ASL source >> > acpi: pre-compiled ASL files >> > acpi: ssdt pcihp: updat generated file >> > loader: use file path size from fw_cfg.h >> > i386: add bios linker/loader >> > loader: allow adding ROMs in done callbacks >> > i386: define pc guest info >> > acpi/piix: add macros for acpi property names >> > piix: APIs for pc guest info >> > ich9: APIs for pc guest info >> > pvpanic: add API to access io port >> > hpet: add API to find it >> > acpi: add interface to access user-installed tables >> > pc: use new api to add builtin tables >> > i386: ACPI table generation code from seabios >> > ssdt: fix PBLK length >> > ssdt-proc: update generated file >> > pci: add pci_for_each_bus_depth_first >> > pcihp: generalization of piix4 acpi >> > piix4: add acpi pci hotplug support >> > acpi-build: enable hotplug for PCI bridges >> > >> > configure | 9 +- >> > hw/i386/acpi-build.h | 9 + >> > hw/i386/acpi-defs.h | 331 ++ >> > hw/i386/bios-linker-loader.h | 27 + >> > hw/lm32/lm32_hwsetup.h | 2 +- >> > include/exec/memory.h | 4 +- >> > include/hw/acpi/acpi.h | 4 + >> > include/hw/acpi/ich9.h | 2 + >> > include/hw/acpi/pcihp.h | 72 + >> > include/hw/acpi/piix4.h | 8 + >> > include/hw/i386/ich9.h | 2 + >> > include/hw/i386/pc.h | 27 + >> > include/hw/irq.h | 7 + >> > include/hw/loader.h | 8 +- >> > include/hw/nvram/fw_cfg.h | 8 +- >> > include/hw/pci-host/q35.h | 2 + >> > include/hw/pci/pci.h | 40 +- >> > include/hw/pci/pci_bus.h | 1 + >> > include/hw/pci/pcie.h | 18 - >> > include/hw/pci/pcie_host.h | 27 + >> > include/hw/sysbus.h | 2 +- >> > include/hw/timer/hpet.h | 2 + >> > include/qom/object.h | 73 +- >> > hw/acpi/core.c | 40 + >> > hw/acpi/ich9.c | 24 + >> > hw/acpi/pcihp.c | 312 ++ >> > hw/acpi/piix4.c | 125 +- >> > hw/audio/ac97.c | 4 +- >> > hw/audio/es1370.c | 4 +- >> > hw/audio/intel-hda.c | 2 +- >> > hw/block/nvme.c | 2 +- >> > hw/char/serial-pci.c | 5 +- >> > hw/char/tpci200.c | 8 +- >> > hw/core/irq.c | 16 + >> > hw/core/loader.c | 31 +- >> > hw/core/sysbus.c | 4 +- >> > hw/display/qxl.c | 2 +- >> > hw/i386/acpi-build.c | 1420 +++++++ >> > hw/i386/bios-linker-loader.c | 158 + >> > hw/i386/pc.c | 25 +- >> > hw/i386/pc_piix.c | 5 + >> > hw/i386/pc_q35.c | 3 + >> > hw/ide/cmd646.c | 2 +- >> > hw/ide/ich.c | 3 +- >> > hw/isa/lpc_ich9.c | 40 + >> > hw/isa/vt82c686.c | 2 +- >> > hw/misc/ivshmem.c | 2 +- >> > hw/misc/pvpanic.c | 13 +- >> > hw/misc/vfio.c | 11 +- >> > hw/net/e1000.c | 2 +- >> > hw/net/eepro100.c | 4 +- >> > hw/net/ne2000.c | 3 +- >> > hw/net/pcnet-pci.c | 3 +- >> > hw/net/rtl8139.c | 2 +- >> > hw/net/vmxnet3.c | 13 +- >> > hw/nvram/fw_cfg.c | 33 +- >> > hw/pci-bridge/pci_bridge_dev.c | 2 +- >> > hw/pci-host/piix.c | 8 + >> > hw/pci-host/q35.c | 26 +- >> > hw/pci/pci.c | 100 +- >> > hw/pci/pcie.c | 4 +- >> > hw/pci/pcie_aer.c | 4 +- >> > hw/pci/pcie_host.c | 24 - >> > hw/pci/shpc.c | 2 +- >> > hw/scsi/esp-pci.c | 3 +- >> > hw/scsi/lsi53c895a.c | 2 +- >> > hw/scsi/megasas.c | 6 +- >> > hw/scsi/vmw_pvscsi.c | 2 +- >> > hw/timer/hpet.c | 5 + >> > hw/usb/hcd-ehci-pci.c | 2 +- >> > hw/usb/hcd-ohci.c | 2 +- >> > hw/usb/hcd-uhci.c | 6 +- >> > hw/usb/hcd-xhci.c | 7 +- >> > hw/virtio/virtio-pci.c | 4 +- >> > memory.c | 4 +- >> > qom/object.c | 60 + >> > vl.c | 3 + >> > docs/memory.txt | 4 + >> > hw/acpi/Makefile.objs | 2 +- >> > hw/i386/Makefile.objs | 27 + >> > hw/i386/acpi-dsdt-cpu-hotplug.dsl | 93 + >> > hw/i386/acpi-dsdt-dbug.dsl | 41 + >> > hw/i386/acpi-dsdt-hpet.dsl | 51 + >> > hw/i386/acpi-dsdt-isa.dsl | 117 + >> > hw/i386/acpi-dsdt-pci-crs.dsl | 105 + >> > hw/i386/acpi-dsdt.dsl | 341 ++ >> > hw/i386/acpi-dsdt.hex.generated | 4409 +++++++++++++++++++++ >> > hw/i386/q35-acpi-dsdt.dsl | 452 +++ >> > hw/i386/q35-acpi-dsdt.hex.generated | 7346 +++++++++++++++++++++++++++++++++++ >> > hw/i386/ssdt-misc.dsl | 119 + >> > hw/i386/ssdt-misc.hex.generated | 386 ++ >> > hw/i386/ssdt-pcihp.dsl | 50 + >> > hw/i386/ssdt-pcihp.hex.generated | 108 + >> > hw/i386/ssdt-proc.dsl | 63 + >> > hw/i386/ssdt-proc.hex.generated | 134 + >> > scripts/acpi_extract.py | 362 ++ >> > scripts/acpi_extract_preprocess.py | 51 + >> > scripts/update-acpi.sh | 4 + >> > 98 files changed, 17366 insertions(+), 183 deletions(-) >> > create mode 100644 hw/i386/acpi-build.h >> > create mode 100644 hw/i386/acpi-defs.h >> > create mode 100644 hw/i386/bios-linker-loader.h >> > create mode 100644 include/hw/acpi/pcihp.h >> > create mode 100644 include/hw/acpi/piix4.h >> > create mode 100644 hw/acpi/pcihp.c >> > create mode 100644 hw/i386/acpi-build.c >> > create mode 100644 hw/i386/bios-linker-loader.c >> > create mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl >> > create mode 100644 hw/i386/acpi-dsdt-dbug.dsl >> > create mode 100644 hw/i386/acpi-dsdt-hpet.dsl >> > create mode 100644 hw/i386/acpi-dsdt-isa.dsl >> > create mode 100644 hw/i386/acpi-dsdt-pci-crs.dsl >> > create mode 100644 hw/i386/acpi-dsdt.dsl >> > create mode 100644 hw/i386/acpi-dsdt.hex.generated >> > create mode 100644 hw/i386/q35-acpi-dsdt.dsl >> > create mode 100644 hw/i386/q35-acpi-dsdt.hex.generated >> > create mode 100644 hw/i386/ssdt-misc.dsl >> > create mode 100644 hw/i386/ssdt-misc.hex.generated >> > create mode 100644 hw/i386/ssdt-pcihp.dsl >> > create mode 100644 hw/i386/ssdt-pcihp.hex.generated >> > create mode 100644 hw/i386/ssdt-proc.dsl >> > create mode 100644 hw/i386/ssdt-proc.hex.generated >> > create mode 100755 scripts/acpi_extract.py >> > create mode 100755 scripts/acpi_extract_preprocess.py >> > create mode 100644 scripts/update-acpi.sh
On Tue, Oct 15, 2013 at 6:43 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > 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? 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 > > >
Il 15/10/2013 15:51, Anthony Liguori ha scritto: > From 41/43: > > "The interface is actually backwards-compatible with > existing PIIX4 ACPI (though not migration compatible)." > > And does "AFAIK" translate to, "I have tested migration from new and > old and old and new with this series"? I suspect the answer is no. Since when do we support migration from new to old? Paolo
On Tue, 15 Oct 2013 06:51:30 -0700 Anthony Liguori <anthony@codemonkey.ws> wrote: > On Mon, Oct 14, 2013 at 10:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Oct 14, 2013 at 03:42:37PM -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. > > > > Last minute? This has been on list for months. > > It doesn't matter how long the patches have been on the list. We have > a very short testing cycle for releases. > > This pull request lacks any automated testing. Something like this > really should come with at least some qtest validation that we are > still generating the right ACPI tables but certainly could have > simpler unit tests too. > > There is no statement about what manual testing you actually did. > Have you run kvm autotest? Have you tested a variety of Windows > guests? I've manually boot/install tested a bunch of x64 based OSes rhel6/fc18/ws2008/ws2012/ws2003r2/XP There were no regressions so far.
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
On Tue, Oct 15, 2013 at 7:01 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 15/10/2013 15:51, Anthony Liguori ha scritto: >> From 41/43: >> >> "The interface is actually backwards-compatible with >> existing PIIX4 ACPI (though not migration compatible)." >> >> And does "AFAIK" translate to, "I have tested migration from new and >> old and old and new with this series"? I suspect the answer is no. > > Since when do we support migration from new to old? We allow it to break because we only send the newest version of things but we should try our best to avoid that from happening. That's why we have things like subsections. Regards, Anthony Liguori > Paolo
On Tue, Oct 15, 2013 at 06:51:30AM -0700, Anthony Liguori wrote: > On Mon, Oct 14, 2013 at 10:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Oct 14, 2013 at 03:42:37PM -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. > > > > Last minute? This has been on list for months. > > It doesn't matter how long the patches have been on the list. We have > a very short testing cycle for releases. > > This pull request lacks any automated testing. Something like this > really should come with at least some qtest validation that we are > still generating the right ACPI tables but certainly could have > simpler unit tests too. It did go through autotest testing though. > There is no statement about what manual testing you actually did. Manually I loaded tables and verified that they match the bios bit for bit except pointer values. > Have you run kvm autotest? Have you tested a variety of Windows > guests? Yes, both. > The pull request has a patch with a binary diff and a comment of: > > "update generated file, not sure what changed" > > And that didn't concern you prior to sending the pull request? Sorry, I forgot to update the description. V2 has it right: IASL sticks its own version when it builds tables, this is what changed. > This series is not ready to merge. Because a single commit message was out of date? > >> 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. > > > > It isn't. The point is getting ACPI out of seabios. > > OK what if I drop the bridge hotplug part? > > How does that address the above? It addresses the issues you have raised which was with the bridge. > >> 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, we also get alternative firmwares working correctly with QEMU. > > > >> Or 10k SLOC of code into QEMU that includes breaking migration > >> compatibility. > > > > AFAIK it doesn't break migration compatibility. > > >From 41/43: > > "The interface is actually backwards-compatible with > existing PIIX4 ACPI (though not migration compatible)." > > And does "AFAIK" translate to, "I have tested migration from new and > old and old and new with this series"? I suspect the answer is no. > > Regards, > > Anthony Liguori But the code to handle it is there, at least. I will test it but I think minor fixes like this can go in after soft freeze.
On Tue, Oct 15, 2013 at 7:20 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Tue, Oct 15, 2013 at 06:51:30AM -0700, Anthony Liguori wrote: >> On Mon, Oct 14, 2013 at 10:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote: >> > On Mon, Oct 14, 2013 at 03:42:37PM -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. >> > >> > Last minute? This has been on list for months. >> >> It doesn't matter how long the patches have been on the list. We have >> a very short testing cycle for releases. >> >> This pull request lacks any automated testing. Something like this >> really should come with at least some qtest validation that we are >> still generating the right ACPI tables but certainly could have >> simpler unit tests too. > > It did go through autotest testing though. This specific tree or some previous version of the series? A full run or just a restricted run? >> There is no statement about what manual testing you actually did. > > Manually I loaded tables and verified that they match > the bios bit for bit except pointer values. > >> Have you run kvm autotest? Have you tested a variety of Windows >> guests? > > Yes, both. > >> The pull request has a patch with a binary diff and a comment of: >> >> "update generated file, not sure what changed" >> >> And that didn't concern you prior to sending the pull request? > > > Sorry, I forgot to update the description. V2 has it right: > IASL sticks its own version when it builds tables, > this is what changed. > >> This series is not ready to merge. > > Because a single commit message was out of date? > >> >> 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. >> > >> > It isn't. The point is getting ACPI out of seabios. >> > OK what if I drop the bridge hotplug part? >> >> How does that address the above? > > It addresses the issues you have raised which was with > the bridge. > >> >> 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, we also get alternative firmwares working correctly with QEMU. >> > >> >> Or 10k SLOC of code into QEMU that includes breaking migration >> >> compatibility. >> > >> > AFAIK it doesn't break migration compatibility. >> >> >From 41/43: >> >> "The interface is actually backwards-compatible with >> existing PIIX4 ACPI (though not migration compatible)." >> >> And does "AFAIK" translate to, "I have tested migration from new and >> old and old and new with this series"? I suspect the answer is no. >> >> Regards, >> >> Anthony Liguori > > But the code to handle it is there, at least. > I will test it but I think minor fixes like this can go > in after soft freeze. I cannot reasonable revert a series like this before we cut GA. We would have to delay the release until everything was fixed. The release is a month away and most of us will lose at least a week to KVM Forum so our ducks need to be in a row here. Please put together a summary of the testing this series has gone through. I still think there should be automated testing as part of this but if the manual testing is sufficiently thorough I'll reconsider for 1.7. Regards, Anthony Liguori > > -- > MST
On Tue, Oct 15, 2013 at 06:53:54AM -0700, Anthony Liguori wrote: > On Tue, Oct 15, 2013 at 6:43 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > > 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? > > 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 > > > > > >
On Tue, Oct 15, 2013 at 07:17:59AM -0700, Anthony Liguori wrote: > On Tue, Oct 15, 2013 at 7:01 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Il 15/10/2013 15:51, Anthony Liguori ha scritto: > >> From 41/43: > >> > >> "The interface is actually backwards-compatible with > >> existing PIIX4 ACPI (though not migration compatible)." > >> > >> And does "AFAIK" translate to, "I have tested migration from new and > >> old and old and new with this series"? I suspect the answer is no. > > > > Since when do we support migration from new to old? > > We allow it to break because we only send the newest version of things > but we should try our best to avoid that from happening. That's why > we have things like subsections. > > Regards, > > Anthony Liguori > > > Paolo I addressed this using _TEST VMSTATE macros: +static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id) +{ + PIIX4PMState *s = opaque; + return s->use_acpi_pci_hotplug; +} + +static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id) +{ + PIIX4PMState *s = opaque; + return !s->use_acpi_pci_hotplug; +} + use_acpi_pci_hotplug is set only for new machine. Did you miss this during review or is something wrong with this?
On Tue, Oct 15, 2013 at 07:21:34AM -0700, Anthony Liguori wrote: > On Tue, Oct 15, 2013 at 7:20 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Oct 15, 2013 at 06:51:30AM -0700, Anthony Liguori wrote: > >> On Mon, Oct 14, 2013 at 10:28 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > >> > On Mon, Oct 14, 2013 at 03:42:37PM -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. > >> > > >> > Last minute? This has been on list for months. > >> > >> It doesn't matter how long the patches have been on the list. We have > >> a very short testing cycle for releases. > >> > >> This pull request lacks any automated testing. Something like this > >> really should come with at least some qtest validation that we are > >> still generating the right ACPI tables but certainly could have > >> simpler unit tests too. > > > > It did go through autotest testing though. > > This specific tree or some previous version of the series? This specific tree + updated seabios. > A full run > or just a restricted run? All tests I normally use for PCI: install guest, migrate, virtio net+blk. If you want more just give me a list: last I looked full run has lots of known failures, that's one of the issues with autotest. I think that ACPI tables being exactly identical when used with seabios is also a convincing argument. > >> There is no statement about what manual testing you actually did. > > > > Manually I loaded tables and verified that they match > > the bios bit for bit except pointer values. > > > >> Have you run kvm autotest? Have you tested a variety of Windows > >> guests? > > > > Yes, both. > > > >> The pull request has a patch with a binary diff and a comment of: > >> > >> "update generated file, not sure what changed" > >> > >> And that didn't concern you prior to sending the pull request? > > > > > > Sorry, I forgot to update the description. V2 has it right: > > IASL sticks its own version when it builds tables, > > this is what changed. > > > >> This series is not ready to merge. > > > > Because a single commit message was out of date? > > > >> >> 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. > >> > > >> > It isn't. The point is getting ACPI out of seabios. > >> > OK what if I drop the bridge hotplug part? > >> > >> How does that address the above? > > > > It addresses the issues you have raised which was with > > the bridge. > > > >> >> 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, we also get alternative firmwares working correctly with QEMU. > >> > > >> >> Or 10k SLOC of code into QEMU that includes breaking migration > >> >> compatibility. > >> > > >> > AFAIK it doesn't break migration compatibility. > >> > >> >From 41/43: > >> > >> "The interface is actually backwards-compatible with > >> existing PIIX4 ACPI (though not migration compatible)." > >> > >> And does "AFAIK" translate to, "I have tested migration from new and > >> old and old and new with this series"? I suspect the answer is no. > >> > >> Regards, > >> > >> Anthony Liguori > > > > But the code to handle it is there, at least. > > I will test it but I think minor fixes like this can go > > in after soft freeze. > > I cannot reasonable revert a series like this before we cut GA. But we can *very* easily disable the new stuff from being exported to guests. Just tweak machine definitions in i386/pc. > We > would have to delay the release until everything was fixed. The > release is a month away and most of us will lose at least a week to > KVM Forum so our ducks need to be in a row here. > > Please put together a summary of the testing this series has gone > through. I still think there should be automated testing as part of > this but if the manual testing is sufficiently thorough I'll > reconsider for 1.7. OK this goes for bridge as well or just for the infrastructure? > Regards, > > Anthony Liguori > > > > > -- > > MST
On Tue, Oct 15, 2013 at 07:21:34AM -0700, Anthony Liguori wrote: > Please put together a summary of the testing this series has gone > through. I still think there should be automated testing as part of > this but if the manual testing is sufficiently thorough I'll > reconsider for 1.7. > > Regards, > > Anthony Liguori This is the list I have. Note: it's all x86 only: all new code is local to x86. Core ACPI changes: Both piix and q35: KVM only: seabios: boot/install: rhel6/fc18/ws2008/ws2012/ws2003r2/XP seabios + coreboot fedora guest KVM and TCG: boot live CD, dump tables in guest, compare bit by bit with host, analyse differences (I can send diff if required) autotest: piix only, KVM only virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.unattended_ins +tall.cdrom.floppy_ks.aio_native virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.tcp virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.unix virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec.g +zip_exec virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.fd virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.physical_resou +rces_check virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.boot virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.reboot virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.shutdown virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.unattended_ins +tall.cdrom.extra_cdrom_ks.aio_native virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.tcp virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.unix virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec.g +zip_exec virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.fd virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.physical_resou +rces_check virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.boot virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.reboot virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.shutdown boot.0 PCI bridge patches: piix only: KVM and TCG: boot live CD linux, remove/add device by hotplug boot windows 7, remove/add device by hotplug Core ACPI changes have been through much more testing. On the other hand bridge would be much easier to revert. If you think we reasonably can merge the bridge hotplug, please let me know I'll address the minor comments that Paolo and you made, test cross version compatibility and resubmit ASAP.
On Tue, 15 Oct 2013 17:51:28 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Oct 15, 2013 at 07:21:34AM -0700, Anthony Liguori wrote: > > Please put together a summary of the testing this series has gone > > through. I still think there should be automated testing as part of > > this but if the manual testing is sufficiently thorough I'll > > reconsider for 1.7. > > > > Regards, > > > > Anthony Liguori > > This is the list I have. Note: it's all x86 only: all new code > is local to x86. > > > Core ACPI changes: > > Both piix and q35: > > KVM only: > seabios: > boot/install: > rhel6/fc18/ws2008/ws2012/ws2003r2/XP > > seabios + coreboot > fedora guest > > KVM and TCG: > boot live CD, dump tables in guest, compare bit by bit with > host, analyse differences (I can send diff if required) > > > > autotest: > piix only, KVM only > > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.unattended_ins > +tall.cdrom.floppy_ks.aio_native > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.tcp > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.unix > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec.g > +zip_exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.fd > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.physical_resou > +rces_check > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.boot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.reboot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.shutdown > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.unattended_ins > +tall.cdrom.extra_cdrom_ks.aio_native > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.tcp > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.unix > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec.g > +zip_exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.fd > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.physical_resou > +rces_check > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.boot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.reboot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.shutdown > boot.0 > > > > PCI bridge patches: > > piix only: > KVM and TCG: > boot live CD linux, remove/add device by hotplug > > boot windows 7, remove/add device by hotplug + CPU hotplug with RHEL6 and ws2008/ws2012/ws2003r2 guests > > Core ACPI changes have been through much more testing. > On the other hand bridge would be much easier to revert. > > If you think we reasonably can merge the bridge hotplug, please let me know > I'll address the minor comments that Paolo and you made, test cross version > compatibility and resubmit ASAP. >
On Tue, Oct 15, 2013 at 05:51:28PM +0300, Michael S. Tsirkin wrote: > On Tue, Oct 15, 2013 at 07:21:34AM -0700, Anthony Liguori wrote: > > Please put together a summary of the testing this series has gone > > through. I still think there should be automated testing as part of > > this but if the manual testing is sufficiently thorough I'll > > reconsider for 1.7. > > > > Regards, > > > > Anthony Liguori > > This is the list I have. Note: it's all x86 only: all new code > is local to x86. > > > Core ACPI changes: > > Both piix and q35: > > KVM only: > seabios: > boot/install: > rhel6/fc18/ws2008/ws2012/ws2003r2/XP I forgot: boot+hotunplug/hotplug device rhel6/systemrescuecd/win7/winxp > seabios + coreboot > fedora guest > > KVM and TCG: > boot live CD, dump tables in guest, compare bit by bit with > host, analyse differences (I can send diff if required) > > > > autotest: > piix only, KVM only > > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.unattended_ins > +tall.cdrom.floppy_ks.aio_native > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.tcp > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.unix > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.exec.g > +zip_exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.migrate.fd > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.physical_resou > +rces_check > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.boot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.reboot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.Win7.x86_64.sp1.shutdown > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.unattended_ins > +tall.cdrom.extra_cdrom_ks.aio_native > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.tcp > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.unix > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.exec.g > +zip_exec > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.migrate.fd > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.physical_resou > +rces_check > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.boot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.reboot > virt.qemu.qemu-mst-baseline-git.qcow2.virtio_blk.smp2.virtio_net.RHEL.6.4.x86_64.shutdown > boot.0 > > > > PCI bridge patches: > > piix only: > KVM and TCG: > boot live CD linux, remove/add device by hotplug > > boot windows 7, remove/add device by hotplug > > > Core ACPI changes have been through much more testing. > On the other hand bridge would be much easier to revert. > > If you think we reasonably can merge the bridge hotplug, please let me know > I'll address the minor comments that Paolo and you made, test cross version > compatibility and resubmit ASAP. > > -- > MST
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. The following changes since commit e26d3e734650640fabd7d95ace4f3a6f88725e0b: smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony for you to fetch changes up to 6cab1e7000021fa6a487f67e1dba986f68fee30d: acpi-build: enable hotplug for PCI bridges (2013-10-14 17:48:58 +0300) ---------------------------------------------------------------- pci, pc, acpi fixes, enhancements This includes some pretty big changes: - pci master abort support by Marcel - pci IRQ API rework by Marcel - acpi generation and pci bridge hotplug support by myself Everything has gone through several revisions, latest versions have been on list for a while without any more comments, tested by several people. Please pull for 1.7. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> ---------------------------------------------------------------- Igor Mammedov (1): cleanup object.h: include error.h directly Marcel Apfelbaum (11): memory: Change MemoryRegion priorities from unsigned to signed docs/memory: Explictly state that MemoryRegion priority is signed hw/pci: partially handle pci master abort hw/core: Add interface to allocate and free a single IRQ hw/pci: add pci wrappers for allocating and asserting irqs hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init hw/vmxnet3: set interrupts using pci irq wrappers hw/vfio: set interrupts using pci irq wrappers hw: set interrupts using pci irq wrappers hw/pcie: AER and hot-plug events must use device's interrupt hw/pci: removed irq field from PCIDevice Michael S. Tsirkin (31): qom: cleanup struct Error references qom: add pointer to int property helpers pci: fix up w64 size calculation helper fw_cfg: interface to trigger callback on read loader: support for unmapped ROM blobs pcie_host: expose UNMAPPED macro pcie_host: expose address format q35: use macro for MCFG property name q35: expose mmcfg size as a property i386: add ACPI table files from seabios acpi: add rules to compile ASL source acpi: pre-compiled ASL files acpi: ssdt pcihp: updat generated file loader: use file path size from fw_cfg.h i386: add bios linker/loader loader: allow adding ROMs in done callbacks i386: define pc guest info acpi/piix: add macros for acpi property names piix: APIs for pc guest info ich9: APIs for pc guest info pvpanic: add API to access io port hpet: add API to find it acpi: add interface to access user-installed tables pc: use new api to add builtin tables i386: ACPI table generation code from seabios ssdt: fix PBLK length ssdt-proc: update generated file pci: add pci_for_each_bus_depth_first pcihp: generalization of piix4 acpi piix4: add acpi pci hotplug support acpi-build: enable hotplug for PCI bridges configure | 9 +- hw/i386/acpi-build.h | 9 + hw/i386/acpi-defs.h | 331 ++ hw/i386/bios-linker-loader.h | 27 + hw/lm32/lm32_hwsetup.h | 2 +- include/exec/memory.h | 4 +- include/hw/acpi/acpi.h | 4 + include/hw/acpi/ich9.h | 2 + include/hw/acpi/pcihp.h | 72 + include/hw/acpi/piix4.h | 8 + include/hw/i386/ich9.h | 2 + include/hw/i386/pc.h | 27 + include/hw/irq.h | 7 + include/hw/loader.h | 8 +- include/hw/nvram/fw_cfg.h | 8 +- include/hw/pci-host/q35.h | 2 + include/hw/pci/pci.h | 40 +- include/hw/pci/pci_bus.h | 1 + include/hw/pci/pcie.h | 18 - include/hw/pci/pcie_host.h | 27 + include/hw/sysbus.h | 2 +- include/hw/timer/hpet.h | 2 + include/qom/object.h | 73 +- hw/acpi/core.c | 40 + hw/acpi/ich9.c | 24 + hw/acpi/pcihp.c | 312 ++ hw/acpi/piix4.c | 125 +- hw/audio/ac97.c | 4 +- hw/audio/es1370.c | 4 +- hw/audio/intel-hda.c | 2 +- hw/block/nvme.c | 2 +- hw/char/serial-pci.c | 5 +- hw/char/tpci200.c | 8 +- hw/core/irq.c | 16 + hw/core/loader.c | 31 +- hw/core/sysbus.c | 4 +- hw/display/qxl.c | 2 +- hw/i386/acpi-build.c | 1420 +++++++ hw/i386/bios-linker-loader.c | 158 + hw/i386/pc.c | 25 +- hw/i386/pc_piix.c | 5 + hw/i386/pc_q35.c | 3 + hw/ide/cmd646.c | 2 +- hw/ide/ich.c | 3 +- hw/isa/lpc_ich9.c | 40 + hw/isa/vt82c686.c | 2 +- hw/misc/ivshmem.c | 2 +- hw/misc/pvpanic.c | 13 +- hw/misc/vfio.c | 11 +- hw/net/e1000.c | 2 +- hw/net/eepro100.c | 4 +- hw/net/ne2000.c | 3 +- hw/net/pcnet-pci.c | 3 +- hw/net/rtl8139.c | 2 +- hw/net/vmxnet3.c | 13 +- hw/nvram/fw_cfg.c | 33 +- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci-host/piix.c | 8 + hw/pci-host/q35.c | 26 +- hw/pci/pci.c | 100 +- hw/pci/pcie.c | 4 +- hw/pci/pcie_aer.c | 4 +- hw/pci/pcie_host.c | 24 - hw/pci/shpc.c | 2 +- hw/scsi/esp-pci.c | 3 +- hw/scsi/lsi53c895a.c | 2 +- hw/scsi/megasas.c | 6 +- hw/scsi/vmw_pvscsi.c | 2 +- hw/timer/hpet.c | 5 + hw/usb/hcd-ehci-pci.c | 2 +- hw/usb/hcd-ohci.c | 2 +- hw/usb/hcd-uhci.c | 6 +- hw/usb/hcd-xhci.c | 7 +- hw/virtio/virtio-pci.c | 4 +- memory.c | 4 +- qom/object.c | 60 + vl.c | 3 + docs/memory.txt | 4 + hw/acpi/Makefile.objs | 2 +- hw/i386/Makefile.objs | 27 + hw/i386/acpi-dsdt-cpu-hotplug.dsl | 93 + hw/i386/acpi-dsdt-dbug.dsl | 41 + hw/i386/acpi-dsdt-hpet.dsl | 51 + hw/i386/acpi-dsdt-isa.dsl | 117 + hw/i386/acpi-dsdt-pci-crs.dsl | 105 + hw/i386/acpi-dsdt.dsl | 341 ++ hw/i386/acpi-dsdt.hex.generated | 4409 +++++++++++++++++++++ hw/i386/q35-acpi-dsdt.dsl | 452 +++ hw/i386/q35-acpi-dsdt.hex.generated | 7346 +++++++++++++++++++++++++++++++++++ hw/i386/ssdt-misc.dsl | 119 + hw/i386/ssdt-misc.hex.generated | 386 ++ hw/i386/ssdt-pcihp.dsl | 50 + hw/i386/ssdt-pcihp.hex.generated | 108 + hw/i386/ssdt-proc.dsl | 63 + hw/i386/ssdt-proc.hex.generated | 134 + scripts/acpi_extract.py | 362 ++ scripts/acpi_extract_preprocess.py | 51 + scripts/update-acpi.sh | 4 + 98 files changed, 17366 insertions(+), 183 deletions(-) create mode 100644 hw/i386/acpi-build.h create mode 100644 hw/i386/acpi-defs.h create mode 100644 hw/i386/bios-linker-loader.h create mode 100644 include/hw/acpi/pcihp.h create mode 100644 include/hw/acpi/piix4.h create mode 100644 hw/acpi/pcihp.c create mode 100644 hw/i386/acpi-build.c create mode 100644 hw/i386/bios-linker-loader.c create mode 100644 hw/i386/acpi-dsdt-cpu-hotplug.dsl create mode 100644 hw/i386/acpi-dsdt-dbug.dsl create mode 100644 hw/i386/acpi-dsdt-hpet.dsl create mode 100644 hw/i386/acpi-dsdt-isa.dsl create mode 100644 hw/i386/acpi-dsdt-pci-crs.dsl create mode 100644 hw/i386/acpi-dsdt.dsl create mode 100644 hw/i386/acpi-dsdt.hex.generated create mode 100644 hw/i386/q35-acpi-dsdt.dsl create mode 100644 hw/i386/q35-acpi-dsdt.hex.generated create mode 100644 hw/i386/ssdt-misc.dsl create mode 100644 hw/i386/ssdt-misc.hex.generated create mode 100644 hw/i386/ssdt-pcihp.dsl create mode 100644 hw/i386/ssdt-pcihp.hex.generated create mode 100644 hw/i386/ssdt-proc.dsl create mode 100644 hw/i386/ssdt-proc.hex.generated create mode 100755 scripts/acpi_extract.py create mode 100755 scripts/acpi_extract_preprocess.py create mode 100644 scripts/update-acpi.sh