Message ID | 20130109121027.GA18244@redhat.com |
---|---|
State | New |
Headers | show |
Il 09/01/2013 13:10, Michael S. Tsirkin ha scritto: > OK you mean like this this? Completely untested - if you > and Anthony think we need this change please let me know > and I'll test and repost. > > --> > > pci: make secondary bus reset explicit > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Yes, but this can only be done after looking at all PCI devices that have buses below (of which the bridge is a special case). And Anthony also metioned using a new method of PCIDevice instead of device_reset. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 09/01/2013 13:10, Michael S. Tsirkin ha scritto: >> OK you mean like this this? Completely untested - if you >> and Anthony think we need this change please let me know >> and I'll test and repost. >> >> --> >> >> pci: make secondary bus reset explicit >> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Yes, but this can only be done after looking at all PCI devices that > have buses below (of which the bridge is a special case). And Anthony > also metioned using a new method of PCIDevice instead of device_reset. Sorry, what's the bug here? Is this a reset bug with cold reset or with a form of warm reset? Regards, Anthony Liguori > > Paolo
Il 09/01/2013 21:40, Anthony Liguori ha scritto: >> > Yes, but this can only be done after looking at all PCI devices that >> > have buses below (of which the bridge is a special case). And Anthony >> > also metioned using a new method of PCIDevice instead of device_reset. > Sorry, what's the bug here? Is this a reset bug with cold reset or with > a form of warm reset? Warm reset (virtio status register reset) of virtio-scsi wasn't propagated down to the SCSI bus. I fixed it using qdev_reset_all, but now I'll do it manually in the HBA. I'm fine with that as long as all buses move away from device_reset. Paolo
On Wed, Jan 09, 2013 at 10:22:58PM +0100, Paolo Bonzini wrote: > Il 09/01/2013 21:40, Anthony Liguori ha scritto: > >> > Yes, but this can only be done after looking at all PCI devices that > >> > have buses below (of which the bridge is a special case). And Anthony > >> > also metioned using a new method of PCIDevice instead of device_reset. > > Sorry, what's the bug here? Is this a reset bug with cold reset or with > > a form of warm reset? > > Warm reset (virtio status register reset) of virtio-scsi wasn't > propagated down to the SCSI bus. I fixed it using qdev_reset_all, but > now I'll do it manually in the HBA. I'm fine with that as long as all > buses move away from device_reset. > > Paolo You also pointed out some bug related to status register handling in virtio on s390, right?
Il 09/01/2013 22:40, Michael S. Tsirkin ha scritto: > > Warm reset (virtio status register reset) of virtio-scsi wasn't > > propagated down to the SCSI bus. I fixed it using qdev_reset_all, but > > now I'll do it manually in the HBA. I'm fine with that as long as all > > buses move away from device_reset. > > You also pointed out some bug related to status register handling in virtio > on s390, right? Yes, that would be a separate patch. A large part of this series can also be kept. Paolo
On Thu, Jan 10, 2013 at 09:31:05AM +0100, Paolo Bonzini wrote: > Il 09/01/2013 22:40, Michael S. Tsirkin ha scritto: > > > Warm reset (virtio status register reset) of virtio-scsi wasn't > > > propagated down to the SCSI bus. I fixed it using qdev_reset_all, but > > > now I'll do it manually in the HBA. I'm fine with that as long as all > > > buses move away from device_reset. > > > > You also pointed out some bug related to status register handling in virtio > > on s390, right? > > Yes, that would be a separate patch. A large part of this series can > also be kept. > > Paolo Let's start with bugfixes, no need to bundle them in a series.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 94840c4..ad81040 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -172,7 +172,7 @@ void pci_device_reset(PCIDevice *dev) { int r; - qdev_reset_all(&dev->qdev); + device_reset(&dev->qdev); dev->irq_state = 0; pci_update_irq_status(dev); diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 995842a..a08b479 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -258,10 +258,12 @@ void pci_bridge_disable_base_limit(PCIDevice *dev) pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0); } -/* reset bridge specific configuration registers */ +/* reset bridge specific configuration registers. + * Propagate reset on the secondary bus. */ void pci_bridge_reset(DeviceState *qdev) { PCIDevice *dev = PCI_DEVICE(qdev); + PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev); uint8_t *conf = dev->config; conf[PCI_PRIMARY_BUS] = 0; @@ -295,6 +297,8 @@ void pci_bridge_reset(DeviceState *qdev) pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0); pci_set_word(conf + PCI_BRIDGE_CONTROL, 0); + + pci_bus_reset(&br->sec_bus); } /* default qdev initialization function for PCI-to-PCI bridge */