virtio_pci: modern driver
Subsystems (5)
Michael S. Tsirkin
maintainercover virtio_pci: modern driver
Based on Rusty's patches.
Coding style and funny jokes are his.
Bugs and a star wars reference (should be easy to spot) are mine. Untested, but useful as basis for beginning the qemu work.
TODO:
= simplify probing: use a common probe function, probe with modern driver
first, if that fails - probe with legacy driver.
BUGS: ATM legacy driver can win and drive a transitional device
Until this is fixed, to test, disable transitional mode in device
More ideas (optional):
= allow disabling legacy driver
= support shared IRQ for config, read ISR
= allocate VQ ring in chunks, good for large rings
= use less meory for small VQ ring in chunks, good for large rings
General TODOs:
= add config generation support
= move alloc_virtqueue_pages to virtio core, reuse in e.g. ccw
Michael S Tsirkin (1):
pci: add pci_iomap_range
Michael S. Tsirkin (2):
virtio_pci: add VIRTIO_PCI_NO_LEGACY
virtio_pci: modern driver
Rusty Russell (2):
virtio-pci: define layout for virtio 1.0
virtio_pci: macros for PCI layout offsets.
drivers/virtio/virtio_pci_common.h | 23 +- include/asm-generic/pci_iomap.h | 5 + include/uapi/linux/virtio_pci.h | 106 ++++++- drivers/virtio/virtio_pci_modern.c | 621 +++++++++++++++++++++++++++++++++++++ lib/pci_iomap.c | 46 ++- drivers/virtio/Makefile | 2 +- 6 files changed, 788 insertions(+), 15 deletions(-) create mode 100644 drivers/virtio/virtio_pci_modern.c
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 1/5 virtio_pci: add VIRTIO_PCI_NO_LEGACY
Add macro to disable all legacy register defines.
Helpful to make sure legacy macros don't leak
through into modern code.
--- include/uapi/linux/virtio_pci.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
include/uapi/linux/virtio_pci.h +10 -5
@@ -41,6 +41,8 @@#include <linux/virtio_config.h> +#ifndef VIRTIO_PCI_NO_LEGACY +/* A 32-bit r/o bitmask of the features supported by the host */ #define VIRTIO_PCI_HOST_FEATURES 0
@@ -67,16 +69,11 @@* 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 -/* MSI-X registers: only enabled if MSI-X is enabled. */ /* A 16-bit vector for configuration changes. */ #define VIRTIO_MSI_CONFIG_VECTOR 20/* 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_VECTOR 0xffff/* The remaining space is defined by each driver as the per-driver * configuration space */
@@ -94,4 +91,12 @@/* The alignment to use between consumer and producer parts of vring. * x86 pagesize again. */ #define VIRTIO_PCI_VRING_ALIGN 4096 + +#endif /* VIRTIO_PCI_NO_LEGACY */ + +/* The bit of the ISR which indicates a device configuration change. */ +#define VIRTIO_PCI_ISR_CONFIG 0x2 +/* Vector value used to disable MSI for queue */ +#define VIRTIO_MSI_NO_VECTOR 0xffff +#endif
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 2/5 virtio-pci: define layout for virtio 1.0
In reply to Michael S. TsirkinFrom: Rusty Russell <[email protected]>
Based on patches by Michael S. Tsirkin <[email protected]>, but I found it hard to follow so changed to use structures which are more self-documenting.
--- include/uapi/linux/virtio_pci.h | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
include/uapi/linux/virtio_pci.h +62 -0
@@ -99,4 +99,66 @@/* Vector value used to disable MSI for queue */ #define VIRTIO_MSI_NO_VECTOR 0xffff +#ifndef VIRTIO_PCI_NO_MODERN + +/* IDs for different capabilities. Must all exist. */ + +/* 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: capability length */ +__u8 type_and_bar;/* Upper 3 bits: bar. +* Lower 3 is VIRTIO_PCI_CAP_*_CFG. */ +__le32 offset;/* Offset within bar. */ +__le32 length;/* Length. */ +}; + +#define VIRTIO_PCI_CAP_BAR_SHIFT 5 +#define VIRTIO_PCI_CAP_BAR_MASK 0x7 +#define VIRTIO_PCI_CAP_TYPE_SHIFT 0 +#define VIRTIO_PCI_CAP_TYPE_MASK 0x7 + +struct virtio_pci_notify_cap { +struct virtio_pci_cap cap; +__le32 notify_off_multiplier;/* Multiplier for queue_notify_off. */ +}; + +/* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */ +struct virtio_pci_common_cfg { +/* About the whole device. */ +__le32 device_feature_select;/* read-write */ +__le32 device_feature;/* read-only */ +__le32 guest_feature_select;/* read-write */ +__le32 guest_feature;/* read-write */ +__le16 msix_config;/* read-write */ +__le16 num_queues;/* read-only */ +__u8 device_status;/* read-write */ +__u8 unused1; + +/* About a specific virtqueue. */ +__le16 queue_select;/* read-write */ +__le16 queue_size;/* read-write, power of 2. */ +__le16 queue_msix_vector;/* read-write */ +__le16 queue_enable;/* read-write */ +__le16 queue_notify_off;/* read-only */ +__le32 queue_desc_lo;/* read-write */ +__le32 queue_desc_hi;/* read-write */ +__le32 queue_avail_lo;/* read-write */ +__le32 queue_avail_hi;/* read-write */ +__le32 queue_used_lo;/* read-write */ +__le32 queue_used_hi;/* read-write */ +}; + +#endif /* VIRTIO_PCI_NO_MODERN */ +#endif
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 2/5 virtio-pci: define layout for virtio 1.0
In reply to Michael S. TsirkinFrom: Rusty Russell <[email protected]>
Based on patches by Michael S. Tsirkin <[email protected]>, but I found it hard to follow so changed to use structures which are more self-documenting.
--- include/uapi/linux/virtio_pci.h | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
include/uapi/linux/virtio_pci.h +62 -0
@@ -99,4 +99,66 @@/* Vector value used to disable MSI for queue */ #define VIRTIO_MSI_NO_VECTOR 0xffff +#ifndef VIRTIO_PCI_NO_MODERN + +/* IDs for different capabilities. Must all exist. */ + +/* 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: capability length */ +__u8 type_and_bar;/* Upper 3 bits: bar. +* Lower 3 is VIRTIO_PCI_CAP_*_CFG. */ +__le32 offset;/* Offset within bar. */ +__le32 length;/* Length. */ +}; + +#define VIRTIO_PCI_CAP_BAR_SHIFT 5 +#define VIRTIO_PCI_CAP_BAR_MASK 0x7 +#define VIRTIO_PCI_CAP_TYPE_SHIFT 0 +#define VIRTIO_PCI_CAP_TYPE_MASK 0x7 + +struct virtio_pci_notify_cap { +struct virtio_pci_cap cap; +__le32 notify_off_multiplier;/* Multiplier for queue_notify_off. */ +}; + +/* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */ +struct virtio_pci_common_cfg { +/* About the whole device. */ +__le32 device_feature_select;/* read-write */ +__le32 device_feature;/* read-only */ +__le32 guest_feature_select;/* read-write */ +__le32 guest_feature;/* read-write */ +__le16 msix_config;/* read-write */ +__le16 num_queues;/* read-only */ +__u8 device_status;/* read-write */ +__u8 unused1; + +/* About a specific virtqueue. */ +__le16 queue_select;/* read-write */ +__le16 queue_size;/* read-write, power of 2. */ +__le16 queue_msix_vector;/* read-write */ +__le16 queue_enable;/* read-write */ +__le16 queue_notify_off;/* read-only */ +__le32 queue_desc_lo;/* read-write */ +__le32 queue_desc_hi;/* read-write */ +__le32 queue_avail_lo;/* read-write */ +__le32 queue_avail_hi;/* read-write */ +__le32 queue_used_lo;/* read-write */ +__le32 queue_used_hi;/* read-write */ +}; + +#endif /* VIRTIO_PCI_NO_MODERN */ +#endif
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 3/5 pci: add pci_iomap_range
In reply to Michael S. TsirkinFrom: Michael S Tsirkin <[email protected]>
Virtio drivers should map the part of the range they need, not necessarily all of it. They also need non-cacheable mapping even for prefetchable BARs.
--- include/asm-generic/pci_iomap.h | 5 +++++ lib/pci_iomap.c | 46 +++++++++++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 6 deletions(-)
include/asm-generic/pci_iomap.h +5 -0
@@ -15,6 +15,11 @@ struct pci_dev;#ifdef CONFIG_PCI /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */ extern void __iomem *pci_iomap (struct pci_dev *dev,int bar,unsigned long max); +extern void __iomem *pci_iomap_range (struct pci_dev *dev,int bar, +unsigned offset, +unsigned long minlen, +unsigned long maxlen, +bool force_nocache);/* Create a virtual mapping cookie for a port on a given PCI device. * Do not call this directly, it exists to make it easier for architectures * to override */
lib/pci_iomap.c +40 -6
78 lines not on this page · raw message
Signature · 1 line
MST
Arnd Bergmann
maintainerRe: [PATCH RFC 3/5] pci: add pci_iomap_range
lib/pci_iomap.c line 56
Quoted 7 lines · On Thursday 11 December 2014 21:37:34 Michael S. Tsirkin wrote:
if (flags & IORESOURCE_MEM) {lib/pci_iomap.c +1
-1-if (flags &IORESOURCE_CACHEABLE ) +if (!force_nocache && (flags &IORESOURCE_CACHEABLE ))return ioremap (start, len);return ioremap_nocache (start, len); }
ioremap is the same as ioremap_nocache, so this doesn't really make any sense. IORESOURCE_CACHEABLE is practically only set for ROM bars, which we rarely map.
Arnd
Michael S. Tsirkin
maintainerRe: [PATCH RFC 3/5] pci: add pci_iomap_range
Quoted 9 lines · Arnd Bergmann
lib/pci_iomap.c line 56
Quoted 7 lines · On Thursday 11 December 2014 21:37:34 Michael S. Tsirkin wrote:
if (flags & IORESOURCE_MEM) {lib/pci_iomap.c +1
-1-if (flags &IORESOURCE_CACHEABLE ) +if (!force_nocache && (flags &IORESOURCE_CACHEABLE ))return ioremap (start, len);return ioremap_nocache (start, len); }
Quoted 5 lines
ioremap is the same as
ioremap_nocache, so this doesn't really make any sense.IORESOURCE_CACHEABLEis practically only set for ROM bars, which we rarely map.Arnd
Hmm I think you are right.
It's an old patch - I think at one point in time we did set it when we now set IORESOURCE_PREFETCH.
I'll drop this parameter.
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 3/5 pci: add pci_iomap_range
In reply to Michael S. TsirkinFrom: Michael S Tsirkin <[email protected]>
Virtio drivers should map the part of the range they need, not necessarily all of it. They also need non-cacheable mapping even for prefetchable BARs.
--- include/asm-generic/pci_iomap.h | 5 +++++ lib/pci_iomap.c | 46 +++++++++++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 6 deletions(-)
include/asm-generic/pci_iomap.h +5 -0
@@ -15,6 +15,11 @@ struct pci_dev;#ifdef CONFIG_PCI /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */ extern void __iomem *pci_iomap (struct pci_dev *dev,int bar,unsigned long max); +extern void __iomem *pci_iomap_range (struct pci_dev *dev,int bar, +unsigned offset, +unsigned long minlen, +unsigned long maxlen, +bool force_nocache);/* Create a virtual mapping cookie for a port on a given PCI device. * Do not call this directly, it exists to make it easier for architectures * to override */
lib/pci_iomap.c +40 -6
78 lines not on this page · raw message
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 4/5 virtio_pci: modern driver
In reply to Michael S. TsirkinIt compiles! Must be perfect.
--- drivers/virtio/virtio_pci_common.h | 23 +- include/uapi/linux/virtio_pci.h | 29 ++ drivers/virtio/virtio_pci_modern.c | 560 +++++++++++++++++++++++++++++++++++++ drivers/virtio/Makefile | 2 +- 4 files changed, 610 insertions(+), 4 deletions(-) create mode 100644 drivers/virtio/virtio_pci_modern.c
drivers/virtio/virtio_pci_common.h +20 -3
@@ -53,12 +53,29 @@ struct virtio_pci_device {struct virtio_device vdev;struct pci_dev *pci_dev; +/* In legacy mode, these two point to within ->legacy. */ +/* Where to read and clear interrupt */ +u8 __iomem *isr; + +/* Modern only fields */ +/* The IO mapping for the PCI config space (non-legacy mode) */ +struct virtio_pci_common_cfg __iomem *common; +/* Device-specific data (non-legacy mode) */ +void __iomem *device; +/* Base of vq notifications (non-legacy mode). */ +void __iomem *notify_base; + +/* So we can sanity-check accesses. */ +size_t notify_len; +size_t device_len; + +/* Multiply queue_notify_off by this value. (non-legacy mode). */ +u32 notify_offset_multiplier; + +/* Legacy only field */ /* the IO mapping for the PCI config space */ void __iomem *ioaddr; -/* the IO mapping for ISR operation */ -void __iomem *isr; -/* a list of queues so we can dispatch IRQs */ spinlock_t lock;struct list_head virtqueues;
include/uapi/linux/virtio_pci.h +29 -0
@@ -159,6 +159,35 @@ struct virtio_pci_common_cfg {__le32 queue_used_hi;/* read-write */ }; +/* Macro versions of offsets for the Old Timers! */ +#define VIRTIO_PCI_CAP_VNDR 0 +#define VIRTIO_PCI_CAP_NEXT 1 +#define VIRTIO_PCI_CAP_LEN 2 +#define VIRTIO_PCI_CAP_TYPE_AND_BAR 3 +#define VIRTIO_PCI_CAP_OFFSET 4 +#define VIRTIO_PCI_CAP_LENGTH 8 + +#define VIRTIO_PCI_NOTIFY_CAP_MULT 12 + +#define VIRTIO_PCI_COMMON_DFSELECT 0 +#define VIRTIO_PCI_COMMON_DF 4 +#define VIRTIO_PCI_COMMON_GFSELECT 8 +#define VIRTIO_PCI_COMMON_GF 12 +#define VIRTIO_PCI_COMMON_MSIX 16 +#define VIRTIO_PCI_COMMON_NUMQ 18 +#define VIRTIO_PCI_COMMON_STATUS 20 +#define VIRTIO_PCI_COMMON_Q_SELECT 22 +#define VIRTIO_PCI_COMMON_Q_SIZE 24 +#define VIRTIO_PCI_COMMON_Q_MSIX 26 +#define VIRTIO_PCI_COMMON_Q_ENABLE 28 +#define VIRTIO_PCI_COMMON_Q_NOFF 30 +#define VIRTIO_PCI_COMMON_Q_DESCLO 32 +#define VIRTIO_PCI_COMMON_Q_DESCHI 36 +#define VIRTIO_PCI_COMMON_Q_AVAILLO 40 +#define VIRTIO_PCI_COMMON_Q_AVAILHI 44 +#define VIRTIO_PCI_COMMON_Q_USEDLO 48 +#define VIRTIO_PCI_COMMON_Q_USEDHI 52 +#endif /* VIRTIO_PCI_NO_MODERN */ #endif
drivers/virtio/virtio_pci_modern.c new file +560 -0
560 lines not on this page · raw message
drivers/virtio/Makefile +1 -1
@@ -1,5 +1,5 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o -virtio_pci-y := virtio_pci_legacy.o virtio_pci_common.o +virtio_pci-y := virtio_pci_modern.o virtio_pci_legacy.o virtio_pci_common.o obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRFC PATCH 5/5 virtio_pci: macros for PCI layout offsets.
In reply to Michael S. TsirkinFrom: Rusty Russell <[email protected]>
QEMU wants it, so why not? Trust, but verify.
--- drivers/virtio/virtio_pci_modern.c | 61 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
drivers/virtio/virtio_pci_modern.c +61 -0
@@ -406,6 +406,65 @@ static void __iomem *map_capability(struct pci_dev *dev, int off, size_t minlen,return p; } +/* This is part of the ABI. Don't screw with it. */ +static inline void check_offsets (void ) +{ +/* Note: disk space was harmed in compilation of this function. */ +BUILD_BUG_ON (VIRTIO_PCI_CAP_VNDR != + offsetof(struct virtio_pci_cap , cap_vndr)); +BUILD_BUG_ON (VIRTIO_PCI_CAP_NEXT != + offsetof(struct virtio_pci_cap , cap_next)); +BUILD_BUG_ON (VIRTIO_PCI_CAP_LEN != + offsetof(struct virtio_pci_cap , cap_len)); +BUILD_BUG_ON (VIRTIO_PCI_CAP_TYPE_AND_BAR != + offsetof(struct virtio_pci_cap , type_and_bar)); +BUILD_BUG_ON (VIRTIO_PCI_CAP_OFFSET != + offsetof(struct virtio_pci_cap , offset)); +BUILD_BUG_ON (VIRTIO_PCI_CAP_LENGTH != + offsetof(struct virtio_pci_cap , length)); +BUILD_BUG_ON (VIRTIO_PCI_NOTIFY_CAP_MULT != + offsetof(struct virtio_pci_notify_cap , + notify_off_multiplier)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_DFSELECT != + offsetof(struct virtio_pci_common_cfg , + device_feature_select)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_DF != + offsetof(struct virtio_pci_common_cfg , device_feature)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_GFSELECT != + offsetof(struct virtio_pci_common_cfg , + guest_feature_select)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_GF != + offsetof(struct virtio_pci_common_cfg , guest_feature)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_MSIX != + offsetof(struct virtio_pci_common_cfg , msix_config)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_NUMQ != + offsetof(struct virtio_pci_common_cfg , num_queues)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_STATUS != + offsetof(struct virtio_pci_common_cfg , device_status)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_SELECT != + offsetof(struct virtio_pci_common_cfg , queue_select)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_SIZE != + offsetof(struct virtio_pci_common_cfg , queue_size)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_MSIX != + offsetof(struct virtio_pci_common_cfg , queue_msix_vector)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_ENABLE != + offsetof(struct virtio_pci_common_cfg , queue_enable)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_NOFF != + offsetof(struct virtio_pci_common_cfg , queue_notify_off)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_DESCLO != + offsetof(struct virtio_pci_common_cfg , queue_desc_lo)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_DESCHI != + offsetof(struct virtio_pci_common_cfg , queue_desc_hi)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_AVAILLO != + offsetof(struct virtio_pci_common_cfg , queue_avail_lo)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_AVAILHI != + offsetof(struct virtio_pci_common_cfg , queue_avail_hi)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_USEDLO != + offsetof(struct virtio_pci_common_cfg , queue_used_lo)); +BUILD_BUG_ON (VIRTIO_PCI_COMMON_Q_USEDHI != + offsetof(struct virtio_pci_common_cfg , queue_used_hi)); +} +/* the PCI probing function */ static int virtio_pci_probe (struct pci_dev *pci_dev,const struct pci_device_id *id)
@@ -413,6 +472,8 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,struct virtio_pci_device *vp_dev;int err, common, isr, notify, device; +check_offsets (); +/* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */ if (pci_dev->device <0x1000 || pci_dev->device >0x107f )return -ENODEV ;
Signature · 1 line
MST