| Message ID | 20170821091614.28251-3-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 21/08/2017 11:16, Cornelia Huck wrote: > The msi routing code in kvm calls some pci functions: provide > some stubs to enable builds without pci. > > Also, to make this more obvious, guard them via a pci_available boolean > (which also can be reused in other places). > > 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> > --- > accel/kvm/kvm-all.c | 6 +++--- > hw/pci/pci-stub.c | 15 +++++++++++++++ > hw/pci/pci.c | 2 ++ > include/hw/pci/pci.h | 2 ++ > 4 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 46ce479dc3..f85553a851 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) > int virq; > MSIMessage msg = {0, 0}; > > - if (dev) { > + if (pci_available && dev) { > msg = pci_get_msi_message(dev, vector); > } Hi Conny, I did not find a case where pci_available is false and dev is true. but anyway, sure is sure. > > @@ -1271,7 +1271,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) > kroute.u.msi.address_lo = (uint32_t)msg.address; > kroute.u.msi.address_hi = msg.address >> 32; > kroute.u.msi.data = le32_to_cpu(msg.data); > - if (kvm_msi_devid_required()) { > + if (pci_available && kvm_msi_devid_required()) { > kroute.flags = KVM_MSI_VALID_DEVID; > kroute.u.msi.devid = pci_requester_id(dev); > } > @@ -1309,7 +1309,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg, > kroute.u.msi.address_lo = (uint32_t)msg.address; > kroute.u.msi.address_hi = msg.address >> 32; > kroute.u.msi.data = le32_to_cpu(msg.data); > - if (kvm_msi_devid_required()) { > + if (pci_available && kvm_msi_devid_required()) { > kroute.flags = KVM_MSI_VALID_DEVID; > kroute.u.msi.devid = pci_requester_id(dev); > } > diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c > index ecad664946..ace43821ca 100644 > --- a/hw/pci/pci-stub.c > +++ b/hw/pci/pci-stub.c > @@ -23,10 +23,12 @@ > #include "monitor/monitor.h" > #include "qapi/qmp/qerror.h" > #include "hw/pci/pci.h" > +#include "hw/pci/msi.h" I think you forgot that... > #include "qmp-commands.h" > #include "hw/pci/msi.h" ...you already have it included here. Didn't you ? > > bool msi_nonbroken; > +bool pci_available; > > PciInfoList *qmp_query_pci(Error **errp) > { > @@ -38,3 +40,16 @@ 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) > +{ > + g_assert(false); > + return (MSIMessage){}; > +} > + > +uint16_t pci_requester_id(PCIDevice *dev) > +{ > + g_assert(false); > + return 0; > +} > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 258fbe51e2..26f346db2c 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -49,6 +49,8 @@ > # define PCI_DPRINTF(format, ...) do { } while (0) > #endif > > +bool pci_available = true; > + > static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent); > static char *pcibus_get_dev_path(DeviceState *dev); > static char *pcibus_get_fw_dev_path(DeviceState *dev); > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index e598b095eb..8bb6449dd7 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -10,6 +10,8 @@ > > #include "hw/pci/pcie.h" > > +extern bool pci_available; > + > /* PCI bus */ > > #define PCI_DEVFN(slot, func) ((((slot) & 0x1f) << 3) | ((func) & 0x07)) > otherwise LGTM Regards, Pierre
On Mon, 21 Aug 2017 18:02:12 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > On 21/08/2017 11:16, Cornelia Huck wrote: > > The msi routing code in kvm calls some pci functions: provide > > some stubs to enable builds without pci. > > > > Also, to make this more obvious, guard them via a pci_available boolean > > (which also can be reused in other places). > > > > 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> > > --- > > accel/kvm/kvm-all.c | 6 +++--- > > hw/pci/pci-stub.c | 15 +++++++++++++++ > > hw/pci/pci.c | 2 ++ > > include/hw/pci/pci.h | 2 ++ > > 4 files changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > > index 46ce479dc3..f85553a851 100644 > > --- a/accel/kvm/kvm-all.c > > +++ b/accel/kvm/kvm-all.c > > @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) > > int virq; > > MSIMessage msg = {0, 0}; > > > > - if (dev) { > > + if (pci_available && dev) { > > msg = pci_get_msi_message(dev, vector); > > } > > Hi Conny, > > I did not find a case where pci_available is false and dev is true. > but anyway, sure is sure. It makes things more obvious, I think. > > diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c > > index ecad664946..ace43821ca 100644 > > --- a/hw/pci/pci-stub.c > > +++ b/hw/pci/pci-stub.c > > @@ -23,10 +23,12 @@ > > #include "monitor/monitor.h" > > #include "qapi/qmp/qerror.h" > > #include "hw/pci/pci.h" > > +#include "hw/pci/msi.h" > > I think you forgot that... > > > #include "qmp-commands.h" > > #include "hw/pci/msi.h" > > ...you already have it included here. Didn't you ? Hum, once should really be enough. > otherwise LGTM Thanks. Can I translate that into a tag? :)
On 22/08/2017 11:04, Cornelia Huck wrote: > On Mon, 21 Aug 2017 18:02:12 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > >> On 21/08/2017 11:16, Cornelia Huck wrote: >>> The msi routing code in kvm calls some pci functions: provide >>> some stubs to enable builds without pci. >>> >>> Also, to make this more obvious, guard them via a pci_available boolean >>> (which also can be reused in other places). >>> >>> 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> >>> --- >>> accel/kvm/kvm-all.c | 6 +++--- >>> hw/pci/pci-stub.c | 15 +++++++++++++++ >>> hw/pci/pci.c | 2 ++ >>> include/hw/pci/pci.h | 2 ++ >>> 4 files changed, 22 insertions(+), 3 deletions(-) >>> >>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>> index 46ce479dc3..f85553a851 100644 >>> --- a/accel/kvm/kvm-all.c >>> +++ b/accel/kvm/kvm-all.c >>> @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) >>> int virq; >>> MSIMessage msg = {0, 0}; >>> >>> - if (dev) { >>> + if (pci_available && dev) { >>> msg = pci_get_msi_message(dev, vector); >>> } >> >> Hi Conny, >> >> I did not find a case where pci_available is false and dev is true. >> but anyway, sure is sure. > > It makes things more obvious, I think. > >>> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c >>> index ecad664946..ace43821ca 100644 >>> --- a/hw/pci/pci-stub.c >>> +++ b/hw/pci/pci-stub.c >>> @@ -23,10 +23,12 @@ >>> #include "monitor/monitor.h" >>> #include "qapi/qmp/qerror.h" >>> #include "hw/pci/pci.h" >>> +#include "hw/pci/msi.h" >> >> I think you forgot that... >> >>> #include "qmp-commands.h" >>> #include "hw/pci/msi.h" >> >> ...you already have it included here. Didn't you ? > > Hum, once should really be enough. > >> otherwise LGTM > > Thanks. Can I translate that into a tag? :) > Yes, Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 46ce479dc3..f85553a851 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) int virq; MSIMessage msg = {0, 0}; - if (dev) { + if (pci_available && dev) { msg = pci_get_msi_message(dev, vector); } @@ -1271,7 +1271,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev) kroute.u.msi.address_lo = (uint32_t)msg.address; kroute.u.msi.address_hi = msg.address >> 32; kroute.u.msi.data = le32_to_cpu(msg.data); - if (kvm_msi_devid_required()) { + if (pci_available && kvm_msi_devid_required()) { kroute.flags = KVM_MSI_VALID_DEVID; kroute.u.msi.devid = pci_requester_id(dev); } @@ -1309,7 +1309,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg, kroute.u.msi.address_lo = (uint32_t)msg.address; kroute.u.msi.address_hi = msg.address >> 32; kroute.u.msi.data = le32_to_cpu(msg.data); - if (kvm_msi_devid_required()) { + if (pci_available && kvm_msi_devid_required()) { kroute.flags = KVM_MSI_VALID_DEVID; kroute.u.msi.devid = pci_requester_id(dev); } diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c index ecad664946..ace43821ca 100644 --- a/hw/pci/pci-stub.c +++ b/hw/pci/pci-stub.c @@ -23,10 +23,12 @@ #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" bool msi_nonbroken; +bool pci_available; PciInfoList *qmp_query_pci(Error **errp) { @@ -38,3 +40,16 @@ 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) +{ + g_assert(false); + return (MSIMessage){}; +} + +uint16_t pci_requester_id(PCIDevice *dev) +{ + g_assert(false); + return 0; +} diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 258fbe51e2..26f346db2c 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -49,6 +49,8 @@ # define PCI_DPRINTF(format, ...) do { } while (0) #endif +bool pci_available = true; + static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent); static char *pcibus_get_dev_path(DeviceState *dev); static char *pcibus_get_fw_dev_path(DeviceState *dev); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index e598b095eb..8bb6449dd7 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -10,6 +10,8 @@ #include "hw/pci/pcie.h" +extern bool pci_available; + /* PCI bus */ #define PCI_DEVFN(slot, func) ((((slot) & 0x1f) << 3) | ((func) & 0x07))
The msi routing code in kvm calls some pci functions: provide some stubs to enable builds without pci. Also, to make this more obvious, guard them via a pci_available boolean (which also can be reused in other places). 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> --- accel/kvm/kvm-all.c | 6 +++--- hw/pci/pci-stub.c | 15 +++++++++++++++ hw/pci/pci.c | 2 ++ include/hw/pci/pci.h | 2 ++ 4 files changed, 22 insertions(+), 3 deletions(-)