| Message ID | 20170725153330.14966-2-cohuck@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25.07.2017 17:33, Cornelia Huck wrote: > The msi routing code in kvm calls some pci functions: provide > some stubs to enable builds without pci. > > Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn") > Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions") > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/pci/pci-stub.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c > index ecad664946..bc228ce91e 100644 > --- a/hw/pci/pci-stub.c > +++ b/hw/pci/pci-stub.c > @@ -23,6 +23,7 @@ > #include "monitor/monitor.h" > #include "qapi/qmp/qerror.h" > #include "hw/pci/pci.h" > +#include "hw/pci/msi.h" > #include "qmp-commands.h" > #include "hw/pci/msi.h" > > @@ -38,3 +39,14 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict) > { > monitor_printf(mon, "PCI devices not supported\n"); > } > + > +/* kvm-all wants this */ > +MSIMessage pci_get_msi_message(PCIDevice *dev, int vector) > +{ > + assert(false); > +} > + > +uint16_t pci_requester_id(PCIDevice *dev) > +{ > + assert(false); > +} Since this missed the 2.10 freeze anyway and we've got some more time to discuss it: I still think it would be much cleaner to move the functions from kvm-all.c that use these PCI functions into a separate file kvm-pci.c instead and compile that only with CONFIG_PCI=y. That way we likely also could prevent that more dependencies on CONFIG_PCI creep into kvm-all.c again at later points in time. I've now had a closer look, and it seems like the only affected functions are kvm_irqchip_add_msi_route() and kvm_irqchip_update_msi_route() ... and these seems only to be used in code we would not care about on s390x with CONFIG_PCI=n. So could you please have at least a try to move these two functions (and other related msi functions) to a new file kvm-pci.c instead to see whether that would work, too? Thanks, Thomas
On Wed, 26 Jul 2017 08:52:44 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 25.07.2017 17:33, Cornelia Huck wrote: > > The msi routing code in kvm calls some pci functions: provide > > some stubs to enable builds without pci. > > > > Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn") > > Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions") > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > hw/pci/pci-stub.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c > > index ecad664946..bc228ce91e 100644 > > --- a/hw/pci/pci-stub.c > > +++ b/hw/pci/pci-stub.c > > @@ -23,6 +23,7 @@ > > #include "monitor/monitor.h" > > #include "qapi/qmp/qerror.h" > > #include "hw/pci/pci.h" > > +#include "hw/pci/msi.h" > > #include "qmp-commands.h" > > #include "hw/pci/msi.h" > > > > @@ -38,3 +39,14 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict) > > { > > monitor_printf(mon, "PCI devices not supported\n"); > > } > > + > > +/* kvm-all wants this */ > > +MSIMessage pci_get_msi_message(PCIDevice *dev, int vector) > > +{ > > + assert(false); > > +} > > + > > +uint16_t pci_requester_id(PCIDevice *dev) > > +{ > > + assert(false); > > +} > > Since this missed the 2.10 freeze anyway and we've got some more time to > discuss it: I still think it would be much cleaner to move the functions > from kvm-all.c that use these PCI functions into a separate file > kvm-pci.c instead and compile that only with CONFIG_PCI=y. That way we > likely also could prevent that more dependencies on CONFIG_PCI creep > into kvm-all.c again at later points in time. > > I've now had a closer look, and it seems like the only affected > functions are kvm_irqchip_add_msi_route() and > kvm_irqchip_update_msi_route() ... and these seems only to be used in > code we would not care about on s390x with CONFIG_PCI=n. So could you > please have at least a try to move these two functions (and other > related msi functions) to a new file kvm-pci.c instead to see whether > that would work, too? I can try, as this missed the 2.10 boat anyway. The code contains some parts that are not relevant in all cases (msi routes if we don't use pci, adapter routes for anything not s390x, ...), but I don't think trying to rip this out would be much of an improvement. I'll try the minimal (well, not quite as minimal as this patch) route instead.
On Wed, 26 Jul 2017 10:26:23 +0200 Cornelia Huck <cohuck@redhat.com> wrote: > On Wed, 26 Jul 2017 08:52:44 +0200 > Thomas Huth <thuth@redhat.com> wrote: > > Since this missed the 2.10 freeze anyway and we've got some more time to > > discuss it: I still think it would be much cleaner to move the functions > > from kvm-all.c that use these PCI functions into a separate file > > kvm-pci.c instead and compile that only with CONFIG_PCI=y. That way we > > likely also could prevent that more dependencies on CONFIG_PCI creep > > into kvm-all.c again at later points in time. > > > > I've now had a closer look, and it seems like the only affected > > functions are kvm_irqchip_add_msi_route() and > > kvm_irqchip_update_msi_route() ... and these seems only to be used in > > code we would not care about on s390x with CONFIG_PCI=n. So could you > > please have at least a try to move these two functions (and other > > related msi functions) to a new file kvm-pci.c instead to see whether > > that would work, too? > > I can try, as this missed the 2.10 boat anyway. > > The code contains some parts that are not relevant in all cases (msi > routes if we don't use pci, adapter routes for anything not > s390x, ...), but I don't think trying to rip this out would be much of > an improvement. I'll try the minimal (well, not quite as minimal as > this patch) route instead. I tried this; but it turned out to be one of those cases where you pull on a stick and then find half a tree attached to it... Much of the code is used by all routing variants, and I ended up exposing so many functions that it was just ugly and unreadable. The variant I'm now going with is keeping the original approach and adding a pci_available variable, which I also use in the patch that exposes the zpci facility patch only when pci is compiled in. I'll try to post this after lunch.
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c index ecad664946..bc228ce91e 100644 --- a/hw/pci/pci-stub.c +++ b/hw/pci/pci-stub.c @@ -23,6 +23,7 @@ #include "monitor/monitor.h" #include "qapi/qmp/qerror.h" #include "hw/pci/pci.h" +#include "hw/pci/msi.h" #include "qmp-commands.h" #include "hw/pci/msi.h" @@ -38,3 +39,14 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict) { monitor_printf(mon, "PCI devices not supported\n"); } + +/* kvm-all wants this */ +MSIMessage pci_get_msi_message(PCIDevice *dev, int vector) +{ + assert(false); +} + +uint16_t pci_requester_id(PCIDevice *dev) +{ + assert(false); +}
The msi routing code in kvm calls some pci functions: provide some stubs to enable builds without pci. Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn") Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions") Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- hw/pci/pci-stub.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)