Re: [PATCHv3 RFC] virtio-pci: flexible configuration layout
On Tue, 22 Nov 2011 20:36:22 +0200, "Michael S. Tsirkin" wrote: > Here's an updated vesion. > I'm alternating between updating the spec and the driver, > spec update to follow. Don't touch the spec yet, we have a long way to go :( I want the ability for driver to set the ring size, and the device to set the alignment. That's a bigger change than you have here. I imagine it almost rips the driver into two completely different drivers. This is the kind of thing I had in mind, for the header. Want me to code up the rest? (I've avoided adding the constants for the new layout: a struct is more compact and more descriptive). Cheers, Rusty. diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h --- a/include/linux/virtio_pci.h +++ b/include/linux/virtio_pci.h @@ -42,56 +42,74 @@ #include /* A 32-bit r/o bitmask of the features supported by the host */ -#define VIRTIO_PCI_HOST_FEATURES 0 +#define VIRTIO_PCI_LEGACY_HOST_FEATURES0 /* A 32-bit r/w bitmask of features activated by the guest */ -#define VIRTIO_PCI_GUEST_FEATURES 4 +#define VIRTIO_PCI_LEGACY_GUEST_FEATURES 4 /* A 32-bit r/w PFN for the currently selected queue */ -#define VIRTIO_PCI_QUEUE_PFN 8 +#define VIRTIO_PCI_LEGACY_QUEUE_PFN8 /* A 16-bit r/o queue size for the currently selected queue */ -#define VIRTIO_PCI_QUEUE_NUM 12 +#define VIRTIO_PCI_LEGACY_QUEUE_NUM12 /* A 16-bit r/w queue selector */ -#define VIRTIO_PCI_QUEUE_SEL 14 +#define VIRTIO_PCI_LEGACY_QUEUE_SEL14 /* A 16-bit r/w queue notifier */ -#define VIRTIO_PCI_QUEUE_NOTIFY16 +#define VIRTIO_PCI_LEGACY_QUEUE_NOTIFY 16 /* An 8-bit device status register. */ -#define VIRTIO_PCI_STATUS 18 +#define VIRTIO_PCI_LEGACY_STATUS 18 /* An 8-bit r/o interrupt status register. Reading the value will return the * current contents of the ISR and will also clear it. This is effectively * a read-and-acknowledge. */ -#define VIRTIO_PCI_ISR 19 - -/* The bit of the ISR which indicates a device configuration change. */ -#define VIRTIO_PCI_ISR_CONFIG 0x2 +#define VIRTIO_PCI_LEGACY_ISR 19 /* MSI-X registers: only enabled if MSI-X is enabled. */ /* A 16-bit vector for configuration changes. */ -#define VIRTIO_MSI_CONFIG_VECTOR20 +#define VIRTIO_MSI_LEGACY_CONFIG_VECTOR20 /* A 16-bit vector for selected queue notifications. */ -#define VIRTIO_MSI_QUEUE_VECTOR 22 -/* Vector value used to disable MSI for queue */ -#define VIRTIO_MSI_NO_VECTOR0x +#define VIRTIO_MSI_LEGACY_QUEUE_VECTOR 22 /* The remaining space is defined by each driver as the per-driver * configuration space */ -#define VIRTIO_PCI_CONFIG(dev) ((dev)->msix_enabled ? 24 : 20) +#define VIRTIO_PCI_LEGACY_CONFIG(dev) ((dev)->msix_enabled ? 24 : 20) + +/* How many bits to shift physical queue address written to QUEUE_PFN. + * 12 is historical, and due to x86 page size. */ +#define VIRTIO_PCI_LEGACY_QUEUE_ADDR_SHIFT 12 + +/* The alignment to use between consumer and producer parts of vring. + * x86 pagesize again. */ +#define VIRTIO_PCI_LEGACY_VRING_ALIGN 4096 + +#ifndef __KERNEL__ +/* Don't break compile of old userspace code. These will go away. */ +#define VIRTIO_PCI_HOST_FEATURES VIRTIO_PCI_LEGACY_HOST_FEATURES +#define VIRTIO_PCI_GUEST_FEATURES VIRTIO_PCI_LEGACY_GUEST_FEATURES +#define VIRTIO_PCI_LEGACY_QUEUE_PFN VIRTIO_PCI_QUEUE_PFN +#define VIRTIO_PCI_LEGACY_QUEUE_NUM VIRTIO_PCI_QUEUE_NUM +#define VIRTIO_PCI_LEGACY_QUEUE_SEL VIRTIO_PCI_QUEUE_SEL +#define VIRTIO_PCI_LEGACY_QUEUE_NOTIFY VIRTIO_PCI_QUEUE_NOTIFY +#define VIRTIO_PCI_LEGACY_STATUS VIRTIO_PCI_STATUS +#define VIRTIO_PCI_LEGACY_ISR VIRTIO_PCI_ISR +#define VIRTIO_MSI_LEGACY_CONFIG_VECTOR VIRTIO_MSI_CONFIG_VECTOR +#define VIRTIO_MSI_LEGACY_QUEUE_VECTOR VIRTIO_MSI_QUEUE_VECTOR +#define VIRTIO_PCI_LEGACY_CONFIG(dev) VIRTIO_PCI_CONFIG(dev) +#define VIRTIO_PCI_LEGACY_QUEUE_ADDR_SHIFT VIRTIO_PCI_QUEUE_ADDR_SHIFT +#define VIRTIO_PCI_LEGACY_VRING_ALIGN VIRTIO_PCI_VRING_ALIGN +#endif /* ...!KERNEL */ /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0 -/* How many bits to shift physical queue address written to QUEUE_PFN. - * 12 is historical, and due to x86 page size. */ -#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12 +/* Vector value used to disable MSI for queue */ +#define VIRTIO_MSI_NO_VECTOR0x -/* The alignment to use between consumer and producer parts of vring. - * x86 pagesize again. */ -#define VIRTIO_PCI_VRING_ALIGN 4096 +/* The bit of the ISR which indicates a device configuration change. */ +#define VIRTIO_PCI_ISR_CONFIG 0x2 /* * Layout for Virtio PCI vendor specific capability (little-endian): @@ -133,4 +151,20 @@ #define VIRTIO_PCI_CAP_CFG_OFF_MASK0x #define VIRTIO_P
Re: [PATCH 5 of 5] virtio: expose added descriptors immediately
On Tue, 22 Nov 2011 08:29:08 +0200, "Michael S. Tsirkin"
wrote:
> On Tue, Nov 22, 2011 at 11:03:04AM +1030, Rusty Russell wrote:
> > - /* If you haven't kicked in this long, you're probably doing something
> > -* wrong. */
> > - WARN_ON(vq->num_added > vq->vring.num);
> > + /* This is very unlikely, but theoretically possible. Kick
> > +* just in case. */
> > + if (unlikely(vq->num_added == 65535))
>
> This is 0x but why use the decimal notation?
Interesting. Why use hex? Feels more like binary?
But I've changed it to "(1 << 16) - 1" to be clear.
> > + virtqueue_kick(_vq);
> >
> > pr_debug("Added buffer head %i to %p\n", head, vq);
> > END_USE(vq);
>
> We also still need to reset vq->num_added, right?
virtqueue_kick does that for us.
Cheers,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCHv3 RFC] virtio-pci: flexible configuration layout
Here's an updated vesion.
I'm alternating between updating the spec and the driver,
spec update to follow.
Compiled only. Posting here for early feedback, and to allow Sasha to
proceed with his "kvm tool" work.
Changes from v2:
address comments by Rusty
bugfixes by Sasha
Changes from v1:
Updated to match v3 of the spec, see:
todo:
split core changes out
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/Kconfig | 22 +
drivers/virtio/virtio_pci.c | 203 ---
include/asm-generic/io.h|4 +
include/asm-generic/iomap.h | 11 +++
include/linux/pci_regs.h|4 +
include/linux/virtio_pci.h | 41 +
lib/iomap.c | 41 -
7 files changed, 307 insertions(+), 19 deletions(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 816ed08..465245e 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -25,6 +25,28 @@ config VIRTIO_PCI
If unsure, say M.
+config VIRTIO_PCI_LEGACY
+ bool "Compatibility with Legacy PIO"
+ default y
+ depends on VIRTIO_PCI
+ ---help---
+ Look out into your driveway. Do you have a flying car? If
+ so, you can happily disable this option and virtio will not
+ break. Otherwise, leave it set. Unless you're testing what
+ life will be like in The Future.
+
+ In other words:
+
+ Support compatibility with legacy PIO mapping in hypervisors.
+ As of Nov 2011, this is required by all hypervisors without
+ exception, so for now, disabling this option is only useful for
+ testing. In future hypervisors, it will be possible to disable
+ this option and get a slightly smaller kernel.
+ You know that your hypervisor is new enough if you disable this
+ option and the device initialization passes.
+
+ If unsure, say Y.
+
config VIRTIO_BALLOON
tristate "Virtio balloon driver (EXPERIMENTAL)"
select VIRTIO
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1bf41..681347b 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -37,8 +37,14 @@ struct virtio_pci_device
struct virtio_device vdev;
struct pci_dev *pci_dev;
- /* the IO mapping for the PCI config space */
+ /* the IO address for the common PCI config space */
void __iomem *ioaddr;
+ /* the IO address for device specific config */
+ void __iomem *ioaddr_device;
+ /* the IO address to use for notifications operations */
+ void __iomem *ioaddr_notify;
+ /* the IO address to use for reading ISR */
+ void __iomem *ioaddr_isr;
/* a list of queues so we can dispatch IRQs */
spinlock_t lock;
@@ -57,8 +63,175 @@ struct virtio_pci_device
unsigned msix_used_vectors;
/* Whether we have vector per vq */
bool per_vq_vectors;
+
+ /* Various IO mappings: used for resource tracking only. */
+
+#ifdef CONFIG_VIRTIO_PCI_LEGACY
+ /* Legacy BAR0: typically PIO. */
+ void __iomem *legacy_map;
+#endif
+
+ /* Mappings specified by device capabilities: typically in MMIO */
+ void __iomem *isr_map;
+ void __iomem *notify_map;
+ void __iomem *common_map;
+ void __iomem *device_map;
};
+#ifdef CONFIG_VIRTIO_PCI_LEGACY
+static void __iomem *virtio_pci_set_legacy_map(struct virtio_pci_device
*vp_dev)
+{
+ return vp_dev->legacy_map = pci_iomap(vp_dev->pci_dev, 0, 256);
+}
+
+static void __iomem *virtio_pci_legacy_map(struct virtio_pci_device *vp_dev)
+{
+ return vp_dev->legacy_map;
+}
+#else
+static void __iomem *virtio_pci_set_legacy_map(struct virtio_pci_device
*vp_dev)
+{
+ return NULL;
+}
+
+static void __iomem *virtio_pci_legacy_map(struct virtio_pci_device *vp_dev)
+{
+ return NULL;
+}
+#endif
+
+/*
+ * With PIO, device-specific config moves as MSI-X is enabled/disabled.
+ * Use this accessor to keep pointer to that config in sync.
+ */
+static void virtio_pci_set_msix_enabled(struct virtio_pci_device *vp_dev, int
enabled)
+{
+ void __iomem* m;
+ vp_dev->msix_enabled = enabled;
+ m = virtio_pci_legacy_map(vp_dev);
+ if (m)
+ vp_dev->ioaddr_device = m + VIRTIO_PCI_CONFIG(vp_dev);
+ else
+ vp_dev->ioaddr_device = vp_dev->device_map;
+}
+
+static void __iomem *virtio_pci_map_cfg(struct virtio_pci_device *vp_dev, u8
cap_id,
+ u32 align)
+{
+u32 size;
+u32 offset;
+u8 bir;
+u8 cap_len;
+ int pos;
+ struct pci_dev *dev = vp_dev->pci_dev;
+ void __iomem *p;
+
+ for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
+pos > 0;
+pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
+ u8 id;
+ pci_read_config_byte
Re: [RFC] kvm tools: Implement multiple VQ for virtio-net
I have been playing with userspace-rcu which has a number of neat lockless routines for queuing and hashing. But there aren't kernel versions and several of them may require cmpxchg to work. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
