qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing t


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field
Date: Sun, 16 Dec 2012 19:04:18 +0200

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.


> > Any other kind is device specific now.
> > The fact that qdev has two APIs which are kind of but not
> > exactly the same is a bug not a feature.
> 
> BusState reset and DeviceState reset are not the same because they are
> triggered differently, one by bus-level functionality (e.g. FLR) and the
> other by device-level functionality (e.g. a register).
> 
> That's neither a bug nor a feature.  It's just obvious.
> 
> > And relying on it in generic virtio is just going to
> > confuse things.
> 
> It is going to confuse you perhaps, but it will not confuse whoever will
> write the next virtio device with a bus.  Who knows, virtio-i2c.
> 
> > Further it does not follow that all backends are children
> > of the frontend.
> 
> Backends are not children of the frontend.  Seriously, this is qdev 101.
> 
> virtio-scsi has no backend.  Frontends (disks) are children of
> virtio-scsi.  Each backend (host block device) is connected to a child
> of virtio-scsi.
> 
> virtio-scsi does not need to reset back-ends.  It needs to reset front-ends.
> 
> virtio-serial does not know it needs to reset back-ends.  It knows it
> needs to reset front-ends.  Each reset of the front-end will also
> propagate to a back-end, but virtio-serial need not know that.
> 
> > So please just fix the virtio-scsi bug for now and we can
> > address the bigger issue if any later.
> 
> I'm refusing to fix the bug in virtio-scsi.  It is not a virtio-scsi
> bug, as proved by the virtio-serial bus having to do the same things.
> Two wrongs do not make a right, and here we have three wrongs already:
> virtio-pci, virtio-s390, virtio-serial all reinventing the wheel.
> 
> Paolo

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.

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.

-- 
MST



reply via email to

[Prev in Thread] Current Thread [Next in Thread]