| Message ID | 20170725153330.14966-5-cohuck@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/25/2017 05:33 PM, Cornelia Huck wrote: > Only set the zpci feature bit on builds that actually support pci. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> I like this variant. Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390x/s390-pci-bus.c | 5 +++++ > hw/s390x/s390-pci-bus.h | 1 + > hw/s390x/s390-pci-stub.c | 4 ++++ > target/s390x/kvm.c | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index c57f6ebae0..7b30d4c7bd 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -34,6 +34,11 @@ > } \ > } while (0) > > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > + set_bit(S390_FEAT_ZPCI, model->features); > +} > + > S390pciState *s390_get_phb(void) > { > static S390pciState *phb; > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h > index 5df6292509..d8796536b0 100644 > --- a/hw/s390x/s390-pci-bus.h > +++ b/hw/s390x/s390-pci-bus.h > @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > S390PCIBusDevice *pbdev); > > +void pci_enable_zpci_feature(S390CPUModel *model); > #endif > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > index cc7278a865..8ceaf482e7 100644 > --- a/hw/s390x/s390-pci-stub.c > +++ b/hw/s390x/s390-pci-stub.c > @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) > { > return NULL; > } > + > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > +} > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index c4c5791d27..866ac3d414 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > } > > /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ > - set_bit(S390_FEAT_ZPCI, model->features); > + pci_enable_zpci_feature(model); > set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); > > if (s390_known_cpu_type(cpu_type)) { >
On 25.07.2017 17:33, Cornelia Huck wrote: > Only set the zpci feature bit on builds that actually support pci. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/s390x/s390-pci-bus.c | 5 +++++ > hw/s390x/s390-pci-bus.h | 1 + > hw/s390x/s390-pci-stub.c | 4 ++++ > target/s390x/kvm.c | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index c57f6ebae0..7b30d4c7bd 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -34,6 +34,11 @@ > } \ > } while (0) > > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > + set_bit(S390_FEAT_ZPCI, model->features); > +} > + > S390pciState *s390_get_phb(void) > { > static S390pciState *phb; > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h > index 5df6292509..d8796536b0 100644 > --- a/hw/s390x/s390-pci-bus.h > +++ b/hw/s390x/s390-pci-bus.h > @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > S390PCIBusDevice *pbdev); > > +void pci_enable_zpci_feature(S390CPUModel *model); > #endif > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > index cc7278a865..8ceaf482e7 100644 > --- a/hw/s390x/s390-pci-stub.c > +++ b/hw/s390x/s390-pci-stub.c > @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) > { > return NULL; > } > + > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > +} > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index c4c5791d27..866ac3d414 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > } > > /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ > - set_bit(S390_FEAT_ZPCI, model->features); > + pci_enable_zpci_feature(model); > set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); > > if (s390_known_cpu_type(cpu_type)) { Reviewed-by: Thomas Huth <thuth@redhat.com>
Good. This patch resolves the problem I mentioned in previous verion. Thanks for your work. 在 2017/7/25 下午11:33, Cornelia Huck 写道: > Only set the zpci feature bit on builds that actually support pci. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/s390x/s390-pci-bus.c | 5 +++++ > hw/s390x/s390-pci-bus.h | 1 + > hw/s390x/s390-ci-stub.c | 4 ++++ > target/s390x/kvm.c | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index c57f6ebae0..7b30d4c7bd 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -34,6 +34,11 @@ > } \ > } while (0) > > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > + set_bit(S390_FEAT_ZPCI, model->features); > +} > + > S390pciState *s390_get_phb(void) > { > static S390pciState *phb; > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h > index 5df6292509..d8796536b0 100644 > --- a/hw/s390x/s390-pci-bus.h > +++ b/hw/s390x/s390-pci-bus.h > @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > S390PCIBusDevice *pbdev); > > +void pci_enable_zpci_feature(S390CPUModel *model); > #endif > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > index cc7278a865..8ceaf482e7 100644 > --- a/hw/s390x/s390-pci-stub.c > +++ b/hw/s390x/s390-pci-stub.c > @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) > { > return NULL; > } > + > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > +} > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index c4c5791d27..866ac3d414 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > } > > /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ > - set_bit(S390_FEAT_ZPCI, model->features); > + pci_enable_zpci_feature(model); > set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); > > if (s390_known_cpu_type(cpu_type)) {
On 25.07.2017 17:33, Cornelia Huck wrote: > Only set the zpci feature bit on builds that actually support pci. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/s390x/s390-pci-bus.c | 5 +++++ > hw/s390x/s390-pci-bus.h | 1 + > hw/s390x/s390-pci-stub.c | 4 ++++ > target/s390x/kvm.c | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index c57f6ebae0..7b30d4c7bd 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -34,6 +34,11 @@ > } \ > } while (0) > > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > + set_bit(S390_FEAT_ZPCI, model->features); > +} > + > S390pciState *s390_get_phb(void) > { > static S390pciState *phb; > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h > index 5df6292509..d8796536b0 100644 > --- a/hw/s390x/s390-pci-bus.h > +++ b/hw/s390x/s390-pci-bus.h > @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > S390PCIBusDevice *pbdev); > > +void pci_enable_zpci_feature(S390CPUModel *model); > #endif > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > index cc7278a865..8ceaf482e7 100644 > --- a/hw/s390x/s390-pci-stub.c > +++ b/hw/s390x/s390-pci-stub.c > @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) > { > return NULL; > } > + > +void pci_enable_zpci_feature(S390CPUModel *model) > +{ > +} > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index c4c5791d27..866ac3d414 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > } > > /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ > - set_bit(S390_FEAT_ZPCI, model->features); > + pci_enable_zpci_feature(model); While I see how this solves the problem, I don't really like it. If there is a function "save_the_world()" I expect it to save the world in all scenarios ;) What about the same approach but rather if(pci_configured()) set_bit(S390_FEAT_ZPCI, model->features); Or of course zpci_configured(), pci_zpci_bus_available() ... > set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); > > if (s390_known_cpu_type(cpu_type)) { >
On Wed, 26 Jul 2017 11:28:17 +0200 David Hildenbrand <david@redhat.com> wrote: > On 25.07.2017 17:33, Cornelia Huck wrote: > > Only set the zpci feature bit on builds that actually support pci. > > > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > hw/s390x/s390-pci-bus.c | 5 +++++ > > hw/s390x/s390-pci-bus.h | 1 + > > hw/s390x/s390-pci-stub.c | 4 ++++ > > target/s390x/kvm.c | 2 +- > > 4 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > > index c57f6ebae0..7b30d4c7bd 100644 > > --- a/hw/s390x/s390-pci-bus.c > > +++ b/hw/s390x/s390-pci-bus.c > > @@ -34,6 +34,11 @@ > > } \ > > } while (0) > > > > +void pci_enable_zpci_feature(S390CPUModel *model) > > +{ > > + set_bit(S390_FEAT_ZPCI, model->features); > > +} > > + > > S390pciState *s390_get_phb(void) > > { > > static S390pciState *phb; > > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h > > index 5df6292509..d8796536b0 100644 > > --- a/hw/s390x/s390-pci-bus.h > > +++ b/hw/s390x/s390-pci-bus.h > > @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); > > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > > S390PCIBusDevice *pbdev); > > > > +void pci_enable_zpci_feature(S390CPUModel *model); > > #endif > > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > > index cc7278a865..8ceaf482e7 100644 > > --- a/hw/s390x/s390-pci-stub.c > > +++ b/hw/s390x/s390-pci-stub.c > > @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) > > { > > return NULL; > > } > > + > > +void pci_enable_zpci_feature(S390CPUModel *model) > > +{ > > +} > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > > index c4c5791d27..866ac3d414 100644 > > --- a/target/s390x/kvm.c > > +++ b/target/s390x/kvm.c > > @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) > > } > > > > /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ > > - set_bit(S390_FEAT_ZPCI, model->features); > > + pci_enable_zpci_feature(model); > > While I see how this solves the problem, I don't really like it. If > there is a function "save_the_world()" I expect it to save the world in > all scenarios ;) What, you did not read the fine print? ;) > > What about the same approach but rather > > if(pci_configured()) > set_bit(S390_FEAT_ZPCI, model->features); > > Or of course zpci_configured(), pci_zpci_bus_available() ... I'm not sure that would help with readability. OTOH, it would be usable to check for pci support in other places as well... I can play with this a bit. > > > set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); > > > > if (s390_known_cpu_type(cpu_type)) { > > > >
On 26.07.2017 11:28, David Hildenbrand wrote: > On 25.07.2017 17:33, Cornelia Huck wrote: >> Only set the zpci feature bit on builds that actually support pci. >> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> hw/s390x/s390-pci-bus.c | 5 +++++ >> hw/s390x/s390-pci-bus.h | 1 + >> hw/s390x/s390-pci-stub.c | 4 ++++ >> target/s390x/kvm.c | 2 +- >> 4 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c >> index c57f6ebae0..7b30d4c7bd 100644 >> --- a/hw/s390x/s390-pci-bus.c >> +++ b/hw/s390x/s390-pci-bus.c >> @@ -34,6 +34,11 @@ >> } \ >> } while (0) >> >> +void pci_enable_zpci_feature(S390CPUModel *model) >> +{ >> + set_bit(S390_FEAT_ZPCI, model->features); >> +} >> + >> S390pciState *s390_get_phb(void) >> { >> static S390pciState *phb; >> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h >> index 5df6292509..d8796536b0 100644 >> --- a/hw/s390x/s390-pci-bus.h >> +++ b/hw/s390x/s390-pci-bus.h >> @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); >> S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, >> S390PCIBusDevice *pbdev); >> >> +void pci_enable_zpci_feature(S390CPUModel *model); >> #endif >> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c >> index cc7278a865..8ceaf482e7 100644 >> --- a/hw/s390x/s390-pci-stub.c >> +++ b/hw/s390x/s390-pci-stub.c >> @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) >> { >> return NULL; >> } >> + >> +void pci_enable_zpci_feature(S390CPUModel *model) >> +{ >> +} >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index c4c5791d27..866ac3d414 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) >> } >> >> /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ >> - set_bit(S390_FEAT_ZPCI, model->features); >> + pci_enable_zpci_feature(model); > > While I see how this solves the problem, I don't really like it. If > there is a function "save_the_world()" I expect it to save the world in > all scenarios ;) > > What about the same approach but rather > > if(pci_configured()) > set_bit(S390_FEAT_ZPCI, model->features); Or maybe something like this (without introducing new functions): if (object_class_by_name(TYPE_S390_PCI_HOST_BRIDGE)) { set_bit(S390_FEAT_ZPCI, model->features); } ? I haven't tested this, though, but I think it should work since the S390_PCI classes are only compiled in if CONFIG_PCI=y. Thomas
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index c57f6ebae0..7b30d4c7bd 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -34,6 +34,11 @@ } \ } while (0) +void pci_enable_zpci_feature(S390CPUModel *model) +{ + set_bit(S390_FEAT_ZPCI, model->features); +} + S390pciState *s390_get_phb(void) { static S390pciState *phb; diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h index 5df6292509..d8796536b0 100644 --- a/hw/s390x/s390-pci-bus.h +++ b/hw/s390x/s390-pci-bus.h @@ -333,4 +333,5 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid); S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, S390PCIBusDevice *pbdev); +void pci_enable_zpci_feature(S390CPUModel *model); #endif diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c index cc7278a865..8ceaf482e7 100644 --- a/hw/s390x/s390-pci-stub.c +++ b/hw/s390x/s390-pci-stub.c @@ -72,3 +72,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) { return NULL; } + +void pci_enable_zpci_feature(S390CPUModel *model) +{ +} diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index c4c5791d27..866ac3d414 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -2662,7 +2662,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp) } /* We emulate a zPCI bus and AEN, therefore we don't need HW support */ - set_bit(S390_FEAT_ZPCI, model->features); + pci_enable_zpci_feature(model); set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features); if (s390_known_cpu_type(cpu_type)) {
Only set the zpci feature bit on builds that actually support pci. Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- hw/s390x/s390-pci-bus.c | 5 +++++ hw/s390x/s390-pci-bus.h | 1 + hw/s390x/s390-pci-stub.c | 4 ++++ target/s390x/kvm.c | 2 +- 4 files changed, 11 insertions(+), 1 deletion(-)