On Wed, Oct 10, 2012 at 08:36:12AM -0500, Anthony Liguori wrote: > Rusty Russell <rusty@xxxxxxxxxxxxxxx> writes: > > > Gerd Hoffmann <kraxel@xxxxxxxxxx> writes: > >> So how about this: > >> > >> (1) Add a vendor specific pci capability for new-style virtio. > >> Specifies the pci bar used for new-style virtio registers. > >> Guests can use it to figure whenever new-style virtio is > >> supported and to map the correct bar (which will probably > >> be bar 1 in most cases). > > > > This was closer to the original proposal[1], which I really liked (you > > can layout bars however you want). Anthony thought that vendor > > capabilities were a PCI-e feature, but it seems they're blessed in PCI > > 2.3. > > 2.3 was standardized in 2002. Are we confident that vendor extensions > play nice with pre-2.3 OSes like Win2k, WinXP, etc? > > I still think it's a bad idea to rely on something so "new" in something > as fundamental as virtio-pci unless we have to. > > Regards, > > Anthony Liguori Pre 2.3 it was the case that *all* space outside the capability linked list, and any capability not recognized by space, was considered vendor specific. So there's no problem really. > > > > So let's return to that proposal, giving something like this: > > > > /* IDs for different capabilities. Must all exist. */ > > /* FIXME: Do we win from separating ISR, NOTIFY and COMMON? */ > > /* Common configuration */ > > #define VIRTIO_PCI_CAP_COMMON_CFG 1 > > /* Notifications */ > > #define VIRTIO_PCI_CAP_NOTIFY_CFG 2 > > /* ISR access */ > > #define VIRTIO_PCI_CAP_ISR_CFG 3 > > /* Device specific confiuration */ > > #define VIRTIO_PCI_CAP_DEVICE_CFG 4 > > > > /* This is the PCI capability header: */ > > struct virtio_pci_cap { > > u8 cap_vndr; /* Generic PCI field: PCI_CAP_ID_VNDR */ > > u8 cap_next; /* Generic PCI field: next ptr. */ > > u8 cap_len; /* Generic PCI field: sizeof(struct virtio_pci_cap). */ > > u8 cfg_type; /* One of the VIRTIO_PCI_CAP_*_CFG. */ > > u8 bar; /* Where to find it. */ > > u8 unused; > > __le16 offset; /* Offset within bar. */ > > __le32 length; /* Length. */ > > }; > > > > This means qemu can point the isr_cfg into the legacy area if it wants. > > In fact, it can put everything in BAR0 if it wants. > > > > Thoughts? > > Rusty. > > -- > > 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 -- 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