On Thu, 28 Sep 2017 12:41:54 +0200
Christian Borntraeger <borntrae...@de.ibm.com> wrote:
> On 09/28/2017 12:34 PM, Christian Borntraeger wrote:
> >
> >
> > On 09/27/2017 05:03 PM, Cornelia Huck wrote:
> >> On Wed, 27 Sep 2017 15:49:27 +0100
> >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> >>
> >>> * Cornelia Huck (coh...@redhat.com) wrote:
> >>>> On Wed, 27 Sep 2017 15:28:38 +0100
> >>>> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> >>>>
> >>>>> * David Hildenbrand (da...@redhat.com) wrote:
> >>>>>> On 27.09.2017 12:59, Christian Borntraeger wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 09/27/2017 12:56 PM, Cornelia Huck wrote:
> >>>>>>>> On Wed, 27 Sep 2017 18:25:00 +0800
> >>>>>>>> Yi Min Zhao <zyi...@linux.vnet.ibm.com> wrote:
> >>>>>>>>
> >>>>>>>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
> >>>>>>>>>> On Tue, 26 Sep 2017 20:40:25 +0200
> >>>>>>>>>> David Hildenbrand <da...@redhat.com> wrote:
> >>>>>>>>
> >>>>>>>>>>> I'd really really really (did I mention really?) favor something
> >>>>>>>>>>> like a
> >>>>>>>>>>> dummy device, because we could easily handle the !CONFIG_PCI case
> >>>>>>>>>>> then.
> >>>>>>>>>>>
> >>>>>>>>>>> All these compat options and conditions will kill us someday...
> >>>>>>>>>>> we're
> >>>>>>>>>>> already patching around that whole stuff way too much.
> >>>>>>>>>>>
> >>>>>>>>>>> If we ever unconditionally created a device, we should keep doing
> >>>>>>>>>>> so.
> >>>>>>>>>> Yes, that whole thing is horrible, especially interaction with
> >>>>>>>>>> compat
> >>>>>>>>>> machines.
> >>>>>>>>>>
> >>>>>>>>>> Do you have an idea on how to create such a dummy device (without
> >>>>>>>>>> having to effectively copy a lot of configured-out code)?
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>> How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)?
> >>>>>>>>> If no zpci feature, we avoid plugging any pci device.
> >>>>>>>>> Then we could always create phb.
> >>>>>>>>> I think pcibus's vmstate is only data to migrate.
> >>>>>>>>
> >>>>>>>> That's still problematic if CONFIG_PCI is off. I currently don't
> >>>>>>>> have a
> >>>>>>>> better idea than either disallowing compat machines on builds without
> >>>>>>>> pci, or using a dummy device...
> >>>>>>>
> >>>>>>> For this particular case your initial patch might be less problematic
> >>>>>>> than
> >>>>>>> a dummy device, because the code that does the migration is NOT
> >>>>>>> contained
> >>>>>>> in s390 specific code but in common PCI code instead. We would need
> >>>>>>> to keep
> >>>>>>> the dummy device always in a way that it will work with the common PCI
> >>>>>>> code.
> >>>>>>>
> >>>>>>
> >>>>>> Interesting, so how is migration then handled for e.g. x86 or other
> >>>>>> architectures that can work without CONFIG_PCI? I assume their
> >>>>>> migration
> >>>>>> should also break?
> >>>>>
> >>>>> It's tied to machine-type; the x86 i440fx and q35 machine types have
> >>>>> PCI; you can't disable PCI while still having those machine types.
> >>>>> (I don't know if you can disable PCI at all on x86)
> >>>>
> >>>> Ugh, that sounds like we need two machine types on s390x as well
> >>>> (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built
> >>>> conditionally. That whole zpci detanglement is looking worse and
> >>>> worse :(
> >>>
> >>> Well fundamentally the migration expects to migrate something into
> >>> the same shaped hole on the destination; if you've got a lump of PCI
> >>> config on the source there's got to be somewhere for it to fit on the
> >>> destination.
> >>> Now, if PCI is actually pretty rare; then you might be able to make
> >>> the host-pci bridge a normal device and not include it in any
> >>> machine type; that way those who want PCI can just instantiate
> >>> the host-pci bridge, and those who don't want it just stick with
> >>> the base machine type.
> >>
> >> I fear that ship has already sailed; the s390-ccw-virtio machine type
> >> has been instantiating a phb for quite some time, which means we have
> >> to drag this on in the compat machines...
> >
> > In the end that means that you should revert
> >
> > commit d32bd032d8fde41281aae34c16a4aa97e9acfeac
> > Author: Cornelia Huck <coh...@redhat.com>
> > AuthorDate: Thu Jul 6 17:21:52 2017 +0200
> > Commit: Cornelia Huck <coh...@redhat.com>
> > CommitDate: Wed Aug 30 18:23:25 2017 +0200
> >
> > s390x/ccw: create s390 phb conditionally
> >
> > to keep s390-ccw-virtio clean proper.
> >
> > If you want to have PCI disabled, you can do you in a machine like
> too quick ^that^
>
> > s390-rhelx.y.z or whatever.
>
> I think we really must revert this commit.
>
A set of non-pci machines looks more attractive to me. I currently have
the following (not yet tested):
From 5dd282bd6e12dca0ca8252019a4c4c58e729b2c5 Mon Sep 17 00:00:00 2001
From: Cornelia Huck <coh...@redhat.com>
Date: Thu, 28 Sep 2017 14:00:48 +0200
Subject: [PATCH] s390x: set of -nopci machines for non-pci builds
Signed-off-by: Cornelia Huck <coh...@redhat.com>
---
hw/s390x/s390-virtio-ccw.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1bcb7000ab..e032857b6e 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -266,7 +266,7 @@ static void ccw_init(MachineState *machine)
machine->initrd_filename, "s390-ccw.img",
"s390-netboot.img", true);
- if (s390_has_feat(S390_FEAT_ZPCI)) {
+ if (pci_available) {
DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
object_property_add_child(qdev_get_machine(),
TYPE_S390_PCI_HOST_BRIDGE,
@@ -596,9 +596,11 @@ bool css_migration_enabled(void)
{ \
MachineClass *mc = MACHINE_CLASS(oc); \
ccw_machine_##suffix##_class_options(mc); \
- mc->desc = "VirtIO-ccw based S390 machine v" verstr; \
+ mc->desc = pci_available ? "VirtIO-ccw based S390 machine v" verstr : \
+ "VirtIO-ccw based S390 machine (no PCI) v" verstr; \
if (latest) { \
- mc->alias = "s390-ccw-virtio"; \
+ mc->alias = pci_available ? "s390-ccw-virtio" : \
+ "s390-ccw-virtio-nopci"; \
mc->is_default = 1; \
} \
} \
@@ -609,14 +611,24 @@ bool css_migration_enabled(void)
ccw_machine_##suffix##_instance_options(machine); \
} \
static const TypeInfo ccw_machine_##suffix##_info = { \
- .name = MACHINE_TYPE_NAME("s390-ccw-virtio-" verstr), \
+ .name = MACHINE_TYPE_NAME("s390-virtio-ccw-" verstr), \
+ .parent = TYPE_S390_CCW_MACHINE, \
+ .class_init = ccw_machine_##suffix##_class_init, \
+ .instance_init = ccw_machine_##suffix##_instance_init, \
+ }; \
+ static const TypeInfo ccw_machine_nopci_##suffix##_info = { \
+ .name = MACHINE_TYPE_NAME("s390-virtio-ccw-nopci-" verstr), \
.parent = TYPE_S390_CCW_MACHINE, \
.class_init = ccw_machine_##suffix##_class_init, \
.instance_init = ccw_machine_##suffix##_instance_init, \
}; \
static void ccw_machine_register_##suffix(void) \
{ \
- type_register_static(&ccw_machine_##suffix##_info); \
+ if (pci_available) { \
+ type_register_static(&ccw_machine_##suffix##_info); \
+ } else { \
+ type_register_static(&ccw_machine_nopci_##suffix##_info); \
+ } \
} \
type_init(ccw_machine_register_##suffix)
--
2.13.5
