On Tue, 2011-11-01 at 14:42 +0200, Michael S. Tsirkin wrote: > On Tue, Nov 01, 2011 at 02:33:33PM +0200, Sasha Levin wrote: > > On Tue, 2011-11-01 at 13:45 +0200, Michael S. Tsirkin wrote: > > > On Tue, Nov 01, 2011 at 10:39:08AM +1030, Rusty Russell wrote: > > > > * [new tag] rusty@xxxxxxxxxxxxxxx-v3.1-7196-gac5be1e -> rusty@xxxxxxxxxxxxxxx-v3.1-7196-gac5be1e > > > > > > > > The following changes since commit 839d8810747bbf39e0a5a7f223b67bffa7945f8d: > > > > > > > > Merge branch 'i2c-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jdelvare/staging (2011-10-30 15:54:59 -0700) > > > > > > > > are available in the git repository at: > > > > > > > > git://github.com/rustyrussell/linux.git master > > > > > > > > Alexey Kardashevskiy (1): > > > > virtio-pci: Use PCI MMIO instead of PIO when available > > > > > > I missed this one - wasn't Cc'd neither me, kvm or virtio mailing lists. > > > > > > It's well known that mmio is much slower than pio on kvm, since > > > mmio needs to be emulated to get at the address. > > > So I'm expecting this will cause a performance regression. > > > IMO we should keep using PIO for VQ and interrupt status access > > > if PIO is available. > > > > > > Another consideration is that in an attempt to pack data > > > densely in the PIO space the layout became messy. > > > It would be better to have common config space and > > > per-device config space in separate pages, possibly > > > with padding between them. > > > > > > So I'd like a bit more discussion on this patch, > > > I'm concerned that if this is released in 3.2 as is we'll > > > have to support this forever. How about a revert for now? > > > > Another thing, the patch tries to map BAR 2 and use it as the > > configuration space. > > > > It's both not documented properly anywhere, and is not fully backwards > > compatible - we currently use BAR 2 as part of our MSIX handling in the > > kvm tool and I'm sure we're not the only ones to assume virtio-pci only > > uses BAR 0. > > > > A proper solution would be for example a configuration in the PIO config > > space which points to the MMIO BAR to use instead. > > I think it makes sense to put the configuration in PCI > configuration space, using vendor-specific capability. > This way we can reuse existing functionality for scanning > capability lists. Yup, I agree. It would also allow dropping the PIO config BAR altogether after some period of backwards compatibility. It was mostly to point out that the patch isn't really backwards compatible :) -- Sasha. -- 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