Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Dec 16, 2012 at 08:31:16PM +0100, Paolo Bonzini wrote:
> Il 16/12/2012 18:04, Michael S. Tsirkin ha scritto:
> > On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote:
> >> Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto:
> >>>>> Maybe it's obvious to you that qdev_reset_all(x)
> >>>>> does a soft reset and what soft reset means
> >>>>> for each bus type
> >>>>
> >>>> We can define soft reset to be *independent* of the bus type.  As you
> >>>> said, you access it with a device register.
> >>>
> >>> I think qemu has one type of reset ATM which is the hard reset.
> >>
> >> A hard reset would kill BARs and configuration space too.
> >> qdev_reset_all doesn't.  Ergo, it is not a hard reset.
> >>
> >> But hey, I'm not wed to names.  Let's call it device-level reset and
> >> bus-level reset.  Whatever.
> > 
> > It's not a question of a name.
> > 
> > ATM qemu supports one kind of reset because it's a kind of reset all
> > hardware supports.  The moment we start inventing
> > our own one we need to document exactly what it means.
> 
> We have two, DeviceClass's and BusClass's reset members.
> 
> I'll make a patch to document them.
> 
> > You have a point about a problem.
> > 
> > My problem is with the solution, this solution depends on the exact
> > modeling for correctness and that I don't want to do since we seem to be
> > re-shuffling what inherits what so often.
> > 
> > For example I could not figure out how the reset function for virtio pci
> > (which clears pending msix vectors so is required) was called by this.
> 
> The same way a PCI bus reset clears pending MSIX vectors.
> 
> qdev_reset_all(pci_dev)
>    -> qdev_walk_children(pci_dev, qdev_reset_one, qbus_reset_one, NULL);
>    -> qdev_reset_one(pci_dev, NULL);
>    -> device_reset(pci_dev);
>    -> calls dc->reset member set for virtio-*-pci, i.e. virtio_pci_reset
> 
> > Another thing that bothers me is that during regular PCI bus reset
> > virtio does not invoke qdev_reset_all but with this reset, it does.
> > Inconsistent.
> 
> It does.
> 
> A PCI bus reset (or FLR) calls pci_device_reset which does this
> 
> void pci_device_reset(PCIDevice *dev)
> {
>     int r;
> 
>     qdev_reset_all(&dev->qdev);
>     ...
> }
> 
> This is exactly how a PCI bus reset clears pending MSIX vectors.
> 
> Paolo

Yes but I mean virtio-pci device is not a bus.
virtio could be a bus.
Our modeling seems incorrect and relying on it for correctness,
looks a bit wrong.

-- 
MST