| Message ID | 20121002192147.31100.41594.stgit@bling.home |
|---|---|
| State | New |
| Headers | show |
Alex Williamson <alex.williamson@redhat.com> writes: > In the event that a pci-assign device is added to a chipset that > hasn't yet implemented the INTx routing interface, exit gracefully > instead of killing the VM. I'm sure we'll implement this for q35, > but there's no reason for such a harsh response. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > hw/kvm/pci-assign.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c > index 05b93d9..7ce0f37 100644 > --- a/hw/kvm/pci-assign.c > +++ b/hw/kvm/pci-assign.c > @@ -887,6 +887,12 @@ static int assign_intx(AssignedDevice *dev) > > intx_route = pci_device_route_intx_to_irq(&dev->dev, dev->intpin); > assert(intx_route.mode != PCI_INTX_INVERTED); > + if (intx_route.mode == PCI_INTX_NOROUTE) { > + error_report("pci-assign: chipset provides no INTx routing " > + "information, but device supports INTx interrupt mode.\n"); > + pci_device_set_intx_routing_notifier(&dev->dev, NULL); > + return -ENOTSUP; > + } Please don't use error_report() in new code. Propagate an Error object using error_setg() and friends. Regards, Anthony Liguori > > if (dev->intx_route.mode == intx_route.mode && > dev->intx_route.irq == intx_route.irq) {
On Tue, 2012-10-02 at 14:55 -0500, Anthony Liguori wrote: > Alex Williamson <alex.williamson@redhat.com> writes: > > > In the event that a pci-assign device is added to a chipset that > > hasn't yet implemented the INTx routing interface, exit gracefully > > instead of killing the VM. I'm sure we'll implement this for q35, > > but there's no reason for such a harsh response. > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > --- > > > > hw/kvm/pci-assign.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c > > index 05b93d9..7ce0f37 100644 > > --- a/hw/kvm/pci-assign.c > > +++ b/hw/kvm/pci-assign.c > > @@ -887,6 +887,12 @@ static int assign_intx(AssignedDevice *dev) > > > > intx_route = pci_device_route_intx_to_irq(&dev->dev, dev->intpin); > > assert(intx_route.mode != PCI_INTX_INVERTED); > > + if (intx_route.mode == PCI_INTX_NOROUTE) { > > + error_report("pci-assign: chipset provides no INTx routing " > > + "information, but device supports INTx interrupt mode.\n"); > > + pci_device_set_intx_routing_notifier(&dev->dev, NULL); > > + return -ENOTSUP; > > + } > > Please don't use error_report() in new code. Propagate an Error object > using error_setg() and friends. That doesn't really seem to be an option since this function is called both from the driver initfn and the interrupt routing change notifier. Neither of those give me an Error object... suggestions? Thanks, Alex
Il 02/10/2012 22:12, Alex Williamson ha scritto: > On Tue, 2012-10-02 at 14:55 -0500, Anthony Liguori wrote: >> Alex Williamson <alex.williamson@redhat.com> writes: >> >>> In the event that a pci-assign device is added to a chipset that >>> hasn't yet implemented the INTx routing interface, exit gracefully >>> instead of killing the VM. I'm sure we'll implement this for q35, >>> but there's no reason for such a harsh response. >>> >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>> --- >>> >>> hw/kvm/pci-assign.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c >>> index 05b93d9..7ce0f37 100644 >>> --- a/hw/kvm/pci-assign.c >>> +++ b/hw/kvm/pci-assign.c >>> @@ -887,6 +887,12 @@ static int assign_intx(AssignedDevice *dev) >>> >>> intx_route = pci_device_route_intx_to_irq(&dev->dev, dev->intpin); >>> assert(intx_route.mode != PCI_INTX_INVERTED); >>> + if (intx_route.mode == PCI_INTX_NOROUTE) { >>> + error_report("pci-assign: chipset provides no INTx routing " >>> + "information, but device supports INTx interrupt mode.\n"); >>> + pci_device_set_intx_routing_notifier(&dev->dev, NULL); >>> + return -ENOTSUP; >>> + } >> >> Please don't use error_report() in new code. Propagate an Error object >> using error_setg() and friends. > > That doesn't really seem to be an option since this function is called > both from the driver initfn and the interrupt routing change notifier. > Neither of those give me an Error object... suggestions? Thanks, Both assigned_dev_update_irq_routing and assigned_dev_update_msi would have to print the error themselves (qerror_report_err + error_free). Looks like a waste of time, frankly. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 02/10/2012 22:12, Alex Williamson ha scritto: >> On Tue, 2012-10-02 at 14:55 -0500, Anthony Liguori wrote: >>> Alex Williamson <alex.williamson@redhat.com> writes: >>> >>>> In the event that a pci-assign device is added to a chipset that >>>> hasn't yet implemented the INTx routing interface, exit gracefully >>>> instead of killing the VM. I'm sure we'll implement this for q35, >>>> but there's no reason for such a harsh response. >>>> >>>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>>> --- >>>> >>>> hw/kvm/pci-assign.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c >>>> index 05b93d9..7ce0f37 100644 >>>> --- a/hw/kvm/pci-assign.c >>>> +++ b/hw/kvm/pci-assign.c >>>> @@ -887,6 +887,12 @@ static int assign_intx(AssignedDevice *dev) >>>> >>>> intx_route = pci_device_route_intx_to_irq(&dev->dev, dev->intpin); >>>> assert(intx_route.mode != PCI_INTX_INVERTED); >>>> + if (intx_route.mode == PCI_INTX_NOROUTE) { >>>> + error_report("pci-assign: chipset provides no INTx routing " >>>> + "information, but device supports INTx interrupt mode.\n"); >>>> + pci_device_set_intx_routing_notifier(&dev->dev, NULL); >>>> + return -ENOTSUP; >>>> + } >>> >>> Please don't use error_report() in new code. Propagate an Error object >>> using error_setg() and friends. >> >> That doesn't really seem to be an option since this function is called >> both from the driver initfn and the interrupt routing change notifier. >> Neither of those give me an Error object... suggestions? Thanks, > > Both assigned_dev_update_irq_routing and assigned_dev_update_msi would > have to print the error themselves (qerror_report_err + error_free). > Looks like a waste of time, frankly. That's a pity but I do agree. Regards, Anthony Liguori > > Paolo
diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c index 05b93d9..7ce0f37 100644 --- a/hw/kvm/pci-assign.c +++ b/hw/kvm/pci-assign.c @@ -887,6 +887,12 @@ static int assign_intx(AssignedDevice *dev) intx_route = pci_device_route_intx_to_irq(&dev->dev, dev->intpin); assert(intx_route.mode != PCI_INTX_INVERTED); + if (intx_route.mode == PCI_INTX_NOROUTE) { + error_report("pci-assign: chipset provides no INTx routing " + "information, but device supports INTx interrupt mode.\n"); + pci_device_set_intx_routing_notifier(&dev->dev, NULL); + return -ENOTSUP; + } if (dev->intx_route.mode == intx_route.mode && dev->intx_route.irq == intx_route.irq) {
In the event that a pci-assign device is added to a chipset that hasn't yet implemented the INTx routing interface, exit gracefully instead of killing the VM. I'm sure we'll implement this for q35, but there's no reason for such a harsh response. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/kvm/pci-assign.c | 6 ++++++ 1 file changed, 6 insertions(+)