| Message ID | 20170401004624.30886-3-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 31 Mar 2017 21:46:07 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > TYPE_S390_PCI_HOST_BRIDGE has user_creatable=false but has > no comment explaining why. Add a FIXME to document that. > > Cc: Frank Blaschka <frank.blaschka@de.ibm.com> > Cc: Cornelia Huck <cornelia.huck@de.ibm.com> > Cc: Christian Borntraeger <borntraeger@de.ibm.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Richard Henderson <rth@twiddle.net> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/s390x/s390-pci-bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index 1ec30c45ce..2c3b960bdf 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -867,7 +867,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); > > - dc->user_creatable = false; > + dc->user_creatable = false; /*FIXME: explain why */ > dc->reset = s390_pcihost_reset; > k->init = s390_pcihost_init; > hc->plug = s390_pcihost_hot_plug; (adding some more possibly interested parties) We currently have one master s390 phb (and it's been that way since s390 pci was introduced). Recently, there has been some remodelling going on to make this more similar to what sPAPR does. I think we could make this even more similar to sPAPR and have this user createable; but I'm currently not sure it's worth the effort. Opinions?
On Mon, Apr 03, 2017 at 10:55:38AM +0200, Cornelia Huck wrote: > On Fri, 31 Mar 2017 21:46:07 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > TYPE_S390_PCI_HOST_BRIDGE has user_creatable=false but has > > no comment explaining why. Add a FIXME to document that. > > > > Cc: Frank Blaschka <frank.blaschka@de.ibm.com> > > Cc: Cornelia Huck <cornelia.huck@de.ibm.com> > > Cc: Christian Borntraeger <borntraeger@de.ibm.com> > > Cc: Alexander Graf <agraf@suse.de> > > Cc: Richard Henderson <rth@twiddle.net> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > hw/s390x/s390-pci-bus.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > > index 1ec30c45ce..2c3b960bdf 100644 > > --- a/hw/s390x/s390-pci-bus.c > > +++ b/hw/s390x/s390-pci-bus.c > > @@ -867,7 +867,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data) > > DeviceClass *dc = DEVICE_CLASS(klass); > > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); > > > > - dc->user_creatable = false; > > + dc->user_creatable = false; /*FIXME: explain why */ > > dc->reset = s390_pcihost_reset; > > k->init = s390_pcihost_init; > > hc->plug = s390_pcihost_hot_plug; > > (adding some more possibly interested parties) > > We currently have one master s390 phb (and it's been that way since > s390 pci was introduced). Recently, there has been some remodelling > going on to make this more similar to what sPAPR does. I think we could > make this even more similar to sPAPR and have this user createable; but > I'm currently not sure it's worth the effort. Opinions? It looks -device s390-pcihost was never possible, anyway, because no s390x machine has has_dynamic_sysbus=1, and TYPE_PCI_HOST_BRIDGE is a sys-bus-device. Also, patch 03/19 on this series would make the explicit user_creatable=false assignment in s390_pcihost_class_init() unnecessary. I don't think it is worth the effort to change that, unless you have a specific use case that would benefit from it.
On Mon, 3 Apr 2017 16:20:11 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Apr 03, 2017 at 10:55:38AM +0200, Cornelia Huck wrote: > > On Fri, 31 Mar 2017 21:46:07 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > TYPE_S390_PCI_HOST_BRIDGE has user_creatable=false but has > > > no comment explaining why. Add a FIXME to document that. > > > > > > Cc: Frank Blaschka <frank.blaschka@de.ibm.com> > > > Cc: Cornelia Huck <cornelia.huck@de.ibm.com> > > > Cc: Christian Borntraeger <borntraeger@de.ibm.com> > > > Cc: Alexander Graf <agraf@suse.de> > > > Cc: Richard Henderson <rth@twiddle.net> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > hw/s390x/s390-pci-bus.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > > > index 1ec30c45ce..2c3b960bdf 100644 > > > --- a/hw/s390x/s390-pci-bus.c > > > +++ b/hw/s390x/s390-pci-bus.c > > > @@ -867,7 +867,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data) > > > DeviceClass *dc = DEVICE_CLASS(klass); > > > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); > > > > > > - dc->user_creatable = false; > > > + dc->user_creatable = false; /*FIXME: explain why */ > > > dc->reset = s390_pcihost_reset; > > > k->init = s390_pcihost_init; > > > hc->plug = s390_pcihost_hot_plug; > > > > (adding some more possibly interested parties) > > > > We currently have one master s390 phb (and it's been that way since > > s390 pci was introduced). Recently, there has been some remodelling > > going on to make this more similar to what sPAPR does. I think we could > > make this even more similar to sPAPR and have this user createable; but > > I'm currently not sure it's worth the effort. Opinions? > > It looks -device s390-pcihost was never possible, anyway, because > no s390x machine has has_dynamic_sysbus=1, and TYPE_PCI_HOST_BRIDGE > is a sys-bus-device. Yes. > > Also, patch 03/19 on this series would make the explicit > user_creatable=false assignment in s390_pcihost_class_init() > unnecessary. If we switch to "sysbus devices are never user creatable, except for a select few" anyway, I think we can just get rid of this. > > I don't think it is worth the effort to change that, unless you > have a specific use case that would benefit from it. Not really. We're building a highly artificical "topology" that does not exist on real hardware (and is not seen by the guest) here, so we can basically do whatever works best. We _might_ want to be more similar to sPAPR, so that we're not the complete oddball, but it seems that we can make everything work with the current setup already.
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 1ec30c45ce..2c3b960bdf 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -867,7 +867,7 @@ static void s390_pcihost_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); - dc->user_creatable = false; + dc->user_creatable = false; /*FIXME: explain why */ dc->reset = s390_pcihost_reset; k->init = s390_pcihost_init; hc->plug = s390_pcihost_hot_plug;
TYPE_S390_PCI_HOST_BRIDGE has user_creatable=false but has no comment explaining why. Add a FIXME to document that. Cc: Frank Blaschka <frank.blaschka@de.ibm.com> Cc: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Alexander Graf <agraf@suse.de> Cc: Richard Henderson <rth@twiddle.net> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/s390x/s390-pci-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)