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. > Unless Michael pointed this patch out, it would have broken (at least) > the kvm tool in a non obvious way which would require a rather long > session of 'git bisect' to figure out whats wrong. > > -- > > 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
