| Message ID | 20170823155458.19601-4-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/23/2017 05:54 PM, Cornelia Huck wrote: > Some non-pci code calls into zpci code. Provide some stubs for builds > without pci. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/s390x/Makefile.objs | 3 +- > hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 76 insertions(+), 1 deletion(-) > create mode 100644 hw/s390x/s390-pci-stub.c > > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs > index b2aade2466..7ee19d3abc 100644 > --- a/hw/s390x/Makefile.objs > +++ b/hw/s390x/Makefile.objs > @@ -11,7 +11,8 @@ obj-y += 3270-ccw.o > obj-y += virtio-ccw.o > obj-y += css-bridge.o > obj-y += ccw-device.o > -obj-y += s390-pci-bus.o s390-pci-inst.o > +obj-$(CONFIG_PCI) += s390-pci-bus.o s390-pci-inst.o > +obj-$(call lnot,$(CONFIG_PCI)) += s390-pci-stub.o > obj-y += s390-skeys.o > obj-y += s390-stattrib.o > obj-$(CONFIG_KVM) += s390-skeys-kvm.o > diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c > new file mode 100644 > index 0000000000..2e7e42a2af > --- /dev/null > +++ b/hw/s390x/s390-pci-stub.c > @@ -0,0 +1,74 @@ > +/* stubs for non-pci builds */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "cpu.h" > +#include "s390-pci-inst.h" > +#include "s390-pci-bus.h" > + > +/* target/s390x/ioinst.c */ > +int chsc_sei_nt2_get_event(void *res) > +{ > + return 1; > +} > + > +int chsc_sei_nt2_have_event(void) > +{ > + return 0; > +} > + > +/* hw/s390x/sclp.c */ > +void s390_pci_sclp_configure(SCCB *sccb) > +{ > +} > + > +void s390_pci_sclp_deconfigure(SCCB *sccb) > +{ > +} shouldnt these function set an error code in the sccb, e.g. something like sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
On 08/24/2017 09:38 AM, Christian Borntraeger wrote: > > > On 08/23/2017 05:54 PM, Cornelia Huck wrote: >> Some non-pci code calls into zpci code. Provide some stubs for builds >> without pci. >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> hw/s390x/Makefile.objs | 3 +- >> hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 76 insertions(+), 1 deletion(-) >> create mode 100644 hw/s390x/s390-pci-stub.c >> >> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs >> index b2aade2466..7ee19d3abc 100644 >> --- a/hw/s390x/Makefile.objs >> +++ b/hw/s390x/Makefile.objs >> @@ -11,7 +11,8 @@ obj-y += 3270-ccw.o >> obj-y += virtio-ccw.o >> obj-y += css-bridge.o >> obj-y += ccw-device.o >> -obj-y += s390-pci-bus.o s390-pci-inst.o >> +obj-$(CONFIG_PCI) += s390-pci-bus.o s390-pci-inst.o >> +obj-$(call lnot,$(CONFIG_PCI)) += s390-pci-stub.o >> obj-y += s390-skeys.o >> obj-y += s390-stattrib.o >> obj-$(CONFIG_KVM) += s390-skeys-kvm.o >> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c >> new file mode 100644 >> index 0000000000..2e7e42a2af >> --- /dev/null >> +++ b/hw/s390x/s390-pci-stub.c >> @@ -0,0 +1,74 @@ >> +/* stubs for non-pci builds */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> +#include "cpu.h" >> +#include "s390-pci-inst.h" >> +#include "s390-pci-bus.h" >> + >> +/* target/s390x/ioinst.c */ >> +int chsc_sei_nt2_get_event(void *res) >> +{ >> + return 1; >> +} >> + >> +int chsc_sei_nt2_have_event(void) >> +{ >> + return 0; >> +} >> + >> +/* hw/s390x/sclp.c */ >> +void s390_pci_sclp_configure(SCCB *sccb) >> +{ >> +} >> + >> +void s390_pci_sclp_deconfigure(SCCB *sccb) >> +{ >> +} > > shouldnt these function set an error code in the sccb, e.g. > something like > > sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > > > Oh you have something like that in patch 7. Maybe move?
On Thu, 24 Aug 2017 09:43:48 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 08/24/2017 09:38 AM, Christian Borntraeger wrote: > > > > > > On 08/23/2017 05:54 PM, Cornelia Huck wrote: > >> Some non-pci code calls into zpci code. Provide some stubs for builds > >> without pci. > >> > >> Reviewed-by: Thomas Huth <thuth@redhat.com> > >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >> --- > >> hw/s390x/Makefile.objs | 3 +- > >> hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 76 insertions(+), 1 deletion(-) > >> create mode 100644 hw/s390x/s390-pci-stub.c > >> +/* hw/s390x/sclp.c */ > >> +void s390_pci_sclp_configure(SCCB *sccb) > >> +{ > >> +} > >> + > >> +void s390_pci_sclp_deconfigure(SCCB *sccb) > >> +{ > >> +} > > > > shouldnt these function set an error code in the sccb, e.g. > > something like > > > > sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > > > > > > > > Oh you have something like that in patch 7. Maybe move? Does not really change anything in practice, but I can move it.
On 08/24/2017 11:09 AM, Cornelia Huck wrote: > On Thu, 24 Aug 2017 09:43:48 +0200 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 08/24/2017 09:38 AM, Christian Borntraeger wrote: >>> >>> >>> On 08/23/2017 05:54 PM, Cornelia Huck wrote: >>>> Some non-pci code calls into zpci code. Provide some stubs for builds >>>> without pci. >>>> >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>>> --- >>>> hw/s390x/Makefile.objs | 3 +- >>>> hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 76 insertions(+), 1 deletion(-) >>>> create mode 100644 hw/s390x/s390-pci-stub.c > >>>> +/* hw/s390x/sclp.c */ >>>> +void s390_pci_sclp_configure(SCCB *sccb) >>>> +{ >>>> +} >>>> + >>>> +void s390_pci_sclp_deconfigure(SCCB *sccb) >>>> +{ >>>> +} >>> >>> shouldnt these function set an error code in the sccb, e.g. >>> something like >>> >>> sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); >>> >>> >>> >> >> Oh you have something like that in patch 7. Maybe move? > > Does not really change anything in practice, but I can move it. > You mean these stubs are not supposed to be reachable and are just for making the linker happy, or? If that's the case I would prefer having that expressed by something like assert(false) or even #define NOT_REACHABLE assert(false). Otherwise patch looks good, but I did not a full review on it, so let's try this: Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
On Thu, 24 Aug 2017 11:50:54 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 08/24/2017 11:09 AM, Cornelia Huck wrote: > > On Thu, 24 Aug 2017 09:43:48 +0200 > > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > >> On 08/24/2017 09:38 AM, Christian Borntraeger wrote: > >>> > >>> > >>> On 08/23/2017 05:54 PM, Cornelia Huck wrote: > >>>> Some non-pci code calls into zpci code. Provide some stubs for builds > >>>> without pci. > >>>> > >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> > >>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >>>> --- > >>>> hw/s390x/Makefile.objs | 3 +- > >>>> hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> 2 files changed, 76 insertions(+), 1 deletion(-) > >>>> create mode 100644 hw/s390x/s390-pci-stub.c > > > >>>> +/* hw/s390x/sclp.c */ > >>>> +void s390_pci_sclp_configure(SCCB *sccb) > >>>> +{ > >>>> +} > >>>> + > >>>> +void s390_pci_sclp_deconfigure(SCCB *sccb) > >>>> +{ > >>>> +} > >>> > >>> shouldnt these function set an error code in the sccb, e.g. > >>> something like > >>> > >>> sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > >>> > >>> > >>> > >> > >> Oh you have something like that in patch 7. Maybe move? > > > > Does not really change anything in practice, but I can move it. > > > > You mean these stubs are not supposed to be reachable and are just > for making the linker happy, or? If that's the case I would prefer > having that expressed by something like assert(false) or even > #define NOT_REACHABLE assert(false). Without the last patch that makes virtio-pci depend on pci for s390x, you can't compile without pci anyway ;) > > Otherwise patch looks good, but I did not a full review on it, > so let's try this: > Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com> > Thanks!
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs index b2aade2466..7ee19d3abc 100644 --- a/hw/s390x/Makefile.objs +++ b/hw/s390x/Makefile.objs @@ -11,7 +11,8 @@ obj-y += 3270-ccw.o obj-y += virtio-ccw.o obj-y += css-bridge.o obj-y += ccw-device.o -obj-y += s390-pci-bus.o s390-pci-inst.o +obj-$(CONFIG_PCI) += s390-pci-bus.o s390-pci-inst.o +obj-$(call lnot,$(CONFIG_PCI)) += s390-pci-stub.o obj-y += s390-skeys.o obj-y += s390-stattrib.o obj-$(CONFIG_KVM) += s390-skeys-kvm.o diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c new file mode 100644 index 0000000000..2e7e42a2af --- /dev/null +++ b/hw/s390x/s390-pci-stub.c @@ -0,0 +1,74 @@ +/* stubs for non-pci builds */ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "cpu.h" +#include "s390-pci-inst.h" +#include "s390-pci-bus.h" + +/* target/s390x/ioinst.c */ +int chsc_sei_nt2_get_event(void *res) +{ + return 1; +} + +int chsc_sei_nt2_have_event(void) +{ + return 0; +} + +/* hw/s390x/sclp.c */ +void s390_pci_sclp_configure(SCCB *sccb) +{ +} + +void s390_pci_sclp_deconfigure(SCCB *sccb) +{ +} + +/* target/s390x/kvm.c */ +int clp_service_call(S390CPU *cpu, uint8_t r2) +{ + return -1; +} + +int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) +{ + return -1; +} + +int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) +{ + return -1; +} + +int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar) +{ + return -1; +} + +int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) +{ + return -1; +} + +int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr, + uint8_t ar) +{ + return -1; +} + +int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar) +{ + return -1; +} + +S390pciState *s390_get_phb(void) +{ + return NULL; +} + +S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx) +{ + return NULL; +}