On Sun, Apr 08, 2012 at 05:01:36PM +0300, Avi Kivity wrote: > On 04/08/2012 04:53 PM, Michael S. Tsirkin wrote: > > On Sun, Apr 08, 2012 at 04:41:27PM +0300, Avi Kivity wrote: > > > On 04/08/2012 04:30 PM, Michael S. Tsirkin wrote: > > > > On Sun, Apr 08, 2012 at 04:24:11PM +0300, Avi Kivity wrote: > > > > > On 04/08/2012 04:21 PM, Michael S. Tsirkin wrote: > > > > > > On Sun, Apr 08, 2012 at 04:18:29PM +0300, Avi Kivity wrote: > > > > > > > On 04/08/2012 04:17 PM, Michael S. Tsirkin wrote: > > > > > > > > > > > > > > > > > > Don't we FLR the device, which ought to disable MSI on the real device? > > > > > > > > > > > > > > > > AFAIK we call pci_reset, which saves device state, does an FLR > > > > > > > > and then restores the state. I think this might include msi as well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Then that is wrong as well, no? > > > > > > > > > > > > Not as such assuming we disable msi/msix first :) > > > > > > > > > > I think we need to fix both, no? > > > > > > > > Isn't this what this patch does? > > > > > > If we change pci_reset() (or a variant that we call) to reset MSI, and > > > update qemu to synchronize from the device after pci_reset(), then we > > > achieve the same result, in a different way. > > > > MSI vectors are set up by kvm in the host. So we should not > > abruptly drop that by a sysfs write: would need to > > synchronise with kvm. Once we do, there's nothing left > > for pci_reset to do. > > I'm thinking about this flow: > > FLR the device > for each emulated register > read it from the hardware > if different from emulated register: > update the internal model (for example, disabling MSI in kvm if > needed) If we do it this way we get back the problem this patch is trying to solve: MSIX assigned while device memory is disabled would cause unsupported request errors. > set emulated register to hardware value Yes, I see what you are trying to say now. Unfortunately that's not enough: we also need to restore the registers afterwards for device to become useful again. Doing this in kernel seems more robust, otherwise we risk losing the device if qemu gets killed before it has restored the registers. > > > Since reset can change other config space registers, we achieve > > > correctness for more of them. > > > > Which other registers do you have in mind? > > BARs for example. We may have our own reset for this, but isn't copying > the hardware values more trustworthy? BAR values in host and guest are unrelated. If pci_reset didn't restore BAR values we won't be able to operate the device. > -- > error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
