Skip to contentSkip to conversation

vfio-pci: VGA and legacy MMIO & I/O port access

PATCH New Alex Williamson 1 participant 4 messages Jan 15, 2013

Subsystems (2)
KVM VFIO driver
lore
Share & tools
lore

Alex Williamson

author
#0 lore raw

cover vfio-pci: VGA and legacy MMIO & I/O port access

This series includes a couple cleanups that I'll target for 3.9 plus RFC patch 3/3 that adds device regions for legacy I/O and makes use of the VGA arbiter for enabling VGA assignment. I know the POWER guys don't likely care about VGA support, but Alexey, Ben, and David, I'd appreciate any feedback on the overal incorporation into vfio given your history with the design. VGA is still a work in progress but with this base I'm able to assign a handlful of desktop cards that I have on hand to a qemu guest using standard builtin VGA drivers. Thanks,

Alex

---

Alex Williamson (3):
vfio-pci: Cleanup read/write functions
vfio-pci: Cleanup BAR access
vfio-pci: Add support for legacy MMIO & I/O port towards VGA support

 drivers/vfio/pci/Kconfig            |   10 +
 drivers/vfio/pci/vfio_pci.c         |  106 +++++++++----
 drivers/vfio/pci/vfio_pci_config.c  |    5 -
 drivers/vfio/pci/vfio_pci_private.h |   16 +-
 drivers/vfio/pci/vfio_pci_rdwr.c    |  282 +++++++++++++++++------------------
 include/uapi/linux/vfio.h           |    8 +
 6 files changed, 242 insertions(+), 185 deletions(-)
author
#1 lore raw

PATCH 1/3 vfio-pci: Cleanup read/write functions

The read and write functions are nearly identical, combine them and convert to a switch statement. This also makes it easy to narrow the scope of when we use the io/mem accessors in case new regions are added.

Signed-off-by Alex Williamson <[email protected]>
---
 drivers/vfio/pci/vfio_pci.c |   59 +++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 30 deletions(-)
drivers/vfio/pci/vfio_pci.c +29 -30
index 6c11994..27f7665 100644
@@ -355,8 +355,8 @@ static long vfio_pci_ioctl(void *device_data,
 	return -ENOTTY;
 }
 
-static ssize_t vfio_pci_read(void *device_data, char __user *buf,
-			     size_t count, loff_t *ppos)
+static ssize_t vfio_pci_rw(void *device_data, char __user *buf,
+			   size_t count, loff_t *ppos, bool iswrite)
 {
 	unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
 	struct vfio_pci_device *vdev = device_data;

@@ -365,42 +365,41 @@ static ssize_t vfio_pci_read(void *device_data, char __user *buf,
 	if (index >= VFIO_PCI_NUM_REGIONS)
 		return -EINVAL;
 
-	if (index == VFIO_PCI_CONFIG_REGION_INDEX)
-		return vfio_pci_config_readwrite(vdev, buf, count, ppos, false);
-	else if (index == VFIO_PCI_ROM_REGION_INDEX)
-		return vfio_pci_mem_readwrite(vdev, buf, count, ppos, false);
-	else if (pci_resource_flags(pdev, index) & IORESOURCE_IO)
-		return vfio_pci_io_readwrite(vdev, buf, count, ppos, false);
-	else if (pci_resource_flags(pdev, index) & IORESOURCE_MEM)
+	switch (index) {
+	case VFIO_PCI_CONFIG_REGION_INDEX:
+		return vfio_pci_config_readwrite(vdev, buf, count,
+						 ppos, iswrite);
+	case VFIO_PCI_ROM_REGION_INDEX:
+		if (iswrite)
+			return -EINVAL;
 		return vfio_pci_mem_readwrite(vdev, buf, count, ppos, false);
 
+	case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
+	{
+		unsigned long flags = pci_resource_flags(pdev, index);
+
+		if (flags & IORESOURCE_IO)
+			return vfio_pci_io_readwrite(vdev, buf, count,
+						     ppos, iswrite);
+		if (flags & IORESOURCE_MEM)
+			return vfio_pci_mem_readwrite(vdev, buf, count,
+						      ppos, iswrite);
+	}
+	}
+
 	return -EINVAL;
 }
 
+static ssize_t vfio_pci_read(void *device_data, char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	return vfio_pci_rw(device_data, buf, count, ppos, false);
+}
+
 static ssize_t vfio_pci_write(void *device_data, const char __user *buf,
 			      size_t count, loff_t *ppos)
 {
-	unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
-	struct vfio_pci_device *vdev = device_data;
-	struct pci_dev *pdev = vdev->pdev;
-
-	if (index >= VFIO_PCI_NUM_REGIONS)
-		return -EINVAL;
-
-	if (index == VFIO_PCI_CONFIG_REGION_INDEX)
-		return vfio_pci_config_readwrite(vdev, (char __user *)buf,
-						 count, ppos, true);
-	else if (index == VFIO_PCI_ROM_REGION_INDEX)
-		return -EINVAL;
-	else if (pci_resource_flags(pdev, index) & IORESOURCE_IO)
-		return vfio_pci_io_readwrite(vdev, (char __user *)buf,
-					     count, ppos, true);
-	else if (pci_resource_flags(pdev, index) & IORESOURCE_MEM) {
-		return vfio_pci_mem_readwrite(vdev, (char __user *)buf,
-					      count, ppos, true);
-	}
-
-	return -EINVAL;
+	return vfio_pci_rw(device_data, buf, count, ppos, true);
 }
 
 static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
author
#2 lore raw

PATCH 2/3 vfio-pci: Cleanup BAR access

In reply to Alex Williamson

We can actually handle MMIO and I/O port from the same access function since PCI already does abstraction of this. The ROM BAR only requires a minor difference, so it gets included too. vfio_pci_config_readwrite gets renamed for consistency.

Signed-off-by Alex Williamson <[email protected]>
---
 drivers/vfio/pci/vfio_pci.c         |   26 ++--
 drivers/vfio/pci/vfio_pci_config.c  |    5 -
 drivers/vfio/pci/vfio_pci_private.h |   15 +-
 drivers/vfio/pci/vfio_pci_rdwr.c    |  231 ++++++++++-------------------------
 4 files changed, 88 insertions(+), 189 deletions(-)
drivers/vfio/pci/vfio_pci.c +11 -15
index 27f7665..781f900 100644
@@ -360,31 +360,21 @@ static ssize_t vfio_pci_rw(void *device_data, char __user *buf,
 {
 	unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
 	struct vfio_pci_device *vdev = device_data;
-	struct pci_dev *pdev = vdev->pdev;
 
 	if (index >= VFIO_PCI_NUM_REGIONS)
 		return -EINVAL;
 
 	switch (index) {
 	case VFIO_PCI_CONFIG_REGION_INDEX:
-		return vfio_pci_config_readwrite(vdev, buf, count,
-						 ppos, iswrite);
+		return vfio_pci_config_rw(vdev, buf, count, ppos, iswrite);
+
 	case VFIO_PCI_ROM_REGION_INDEX:
 		if (iswrite)
 			return -EINVAL;
-		return vfio_pci_mem_readwrite(vdev, buf, count, ppos, false);
+		return vfio_pci_bar_rw(vdev, buf, count, ppos, false);
 
 	case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
-	{
-		unsigned long flags = pci_resource_flags(pdev, index);
-
-		if (flags & IORESOURCE_IO)
-			return vfio_pci_io_readwrite(vdev, buf, count,
-						     ppos, iswrite);
-		if (flags & IORESOURCE_MEM)
-			return vfio_pci_mem_readwrite(vdev, buf, count,
-						      ppos, iswrite);
-	}
+		return vfio_pci_bar_rw(vdev, buf, count, ppos, iswrite);
 	}
 
 	return -EINVAL;

@@ -393,13 +383,19 @@ static ssize_t vfio_pci_rw(void *device_data, char __user *buf,
 static ssize_t vfio_pci_read(void *device_data, char __user *buf,
 			     size_t count, loff_t *ppos)
 {
+	if (!count)
+		return 0;
+
 	return vfio_pci_rw(device_data, buf, count, ppos, false);
 }
 
 static ssize_t vfio_pci_write(void *device_data, const char __user *buf,
 			      size_t count, loff_t *ppos)
 {
-	return vfio_pci_rw(device_data, buf, count, ppos, true);
+	if (!count)
+		return 0;
+
+	return vfio_pci_rw(device_data, (char __user *)buf, count, ppos, true);
 }
 
 static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
drivers/vfio/pci/vfio_pci_config.c +2 -3
index 8b8f7d1..6985149 100644
@@ -1501,9 +1501,8 @@ static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
 	return ret;
 }
 
-ssize_t vfio_pci_config_readwrite(struct vfio_pci_device *vdev,
-				  char __user *buf, size_t count,
-				  loff_t *ppos, bool iswrite)
+ssize_t vfio_pci_config_rw(struct vfio_pci_device *vdev, char __user *buf,
+			   size_t count, loff_t *ppos, bool iswrite)
 {
 	size_t done = 0;
 	int ret = 0;
drivers/vfio/pci/vfio_pci_private.h +6 -9
index 611827c..00d19b9 100644
@@ -70,15 +70,12 @@ extern int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev,
 				   uint32_t flags, unsigned index,
 				   unsigned start, unsigned count, void *data);
 
-extern ssize_t vfio_pci_config_readwrite(struct vfio_pci_device *vdev,
-					 char __user *buf, size_t count,
-					 loff_t *ppos, bool iswrite);
-extern ssize_t vfio_pci_mem_readwrite(struct vfio_pci_device *vdev,
-				      char __user *buf, size_t count,
-				      loff_t *ppos, bool iswrite);
-extern ssize_t vfio_pci_io_readwrite(struct vfio_pci_device *vdev,
-				     char __user *buf, size_t count,
-				     loff_t *ppos, bool iswrite);
+extern ssize_t vfio_pci_config_rw(struct vfio_pci_device *vdev,
+				  char __user *buf, size_t count,
+				  loff_t *ppos, bool iswrite);
+
+extern ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, char __user *buf,
+			       size_t count, loff_t *ppos, bool iswrite);
 
 extern int vfio_pci_init_perm_bits(void);
 extern void vfio_pci_uninit_perm_bits(void);
drivers/vfio/pci/vfio_pci_rdwr.c +69 -162

319 lines not on this page · raw message

author
#3 lore raw

RFC PATCH 3/3 vfio-pci: Add support for legacy MMIO & I/O port towards VGA support

In reply to Alex Williamson

Create two new legacy regions, one for MMIO space below 1MB and another for 64k of I/O port space. For devices of PCI class VGA these ranges will be exposed and allow direct access to the device at the PCI defined VGA addresses, 0xa0000, 0x3b0, 0x3c0. VFIO makes use of the host VGA arbiter to manage host chipset config to route each access to the correct device.

Signed-off-by Alex Williamson <[email protected]>
---
 drivers/vfio/pci/Kconfig            |   10 ++++
 drivers/vfio/pci/vfio_pci.c         |   49 ++++++++++++++++++++
 drivers/vfio/pci/vfio_pci_private.h |    9 ++++
 drivers/vfio/pci/vfio_pci_rdwr.c    |   85 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h           |    8 +++
 5 files changed, 160 insertions(+), 1 deletion(-)
drivers/vfio/pci/Kconfig +10 -0
index 5980758..559d807 100644
@@ -6,3 +6,13 @@ config VFIO_PCI
 	  use of PCI drivers using the VFIO framework.
 
 	  If you don't know what to do here, say N.
+
+config VFIO_PCI_VGA
+	bool "VFIO PCI support for VGA devices"
+	depends on VFIO_PCI && X86 && VGA_ARB && EXPERIMENTAL
+	help
+	  Support for VGA extensions to VFIO PCI.  This exposes additional
+	  regions on VGA devices for accessing legacy VGA addresses used
+	  by BIOS and generic video drivers.
+
+	  If you don't know what to do here, say N.
drivers/vfio/pci/vfio_pci.c +48 -1

81 lines not on this page · raw message

drivers/vfio/pci/vfio_pci_private.h +9 -0
index 00d19b9..453ea69 100644
@@ -53,6 +53,7 @@ struct vfio_pci_device {
 	bool			reset_works;
 	bool			extended_caps;
 	bool			bardirty;
+	bool			has_vga;
 	struct pci_saved_state	*pci_saved_state;
 	atomic_t		refcnt;
 };

@@ -77,6 +78,14 @@ extern ssize_t vfio_pci_config_rw(struct vfio_pci_device *vdev,
 extern ssize_t vfio_pci_bar_rw(struct vfio_pci_device *vdev, char __user *buf,
 			       size_t count, loff_t *ppos, bool iswrite);
 
+extern ssize_t vfio_pci_legacy_mem_rw(struct vfio_pci_device *vdev,
+				      char __user *buf, size_t count,
+				      loff_t *ppos, bool iswrite);
+
+extern ssize_t vfio_pci_legacy_io_rw(struct vfio_pci_device *vdev,
+				     char __user *buf, size_t count,
+				     loff_t *ppos, bool iswrite);
+
 extern int vfio_pci_init_perm_bits(void);
 extern void vfio_pci_uninit_perm_bits(void);
drivers/vfio/pci/vfio_pci_rdwr.c +85 -0

94 lines not on this page · raw message

include/uapi/linux/vfio.h +8 -0
index 4758d1b..19a90c0 100644
@@ -147,6 +147,12 @@ struct vfio_device_info {
 	__u32	flags;
 #define VFIO_DEVICE_FLAGS_RESET	(1 << 0)	/* Device supports reset */
 #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
+/* Legacy MMIO 0x0 - 0xfffff via VFIO_PCI_LEGACY_MMIO_REGION_INDEX */
+#define VFIO_DEVICE_FLAGS_PCI_LEGACY_MMIO	(1 << 2)
+/* Legacy I/O Port 0x0 - 0xffff via VFIO_PCI_LEGACY_IOPORT_REGION_INDEX */
+#define VFIO_DEVICE_FLAGS_PCI_LEGACY_IOPORT	(1 << 3)
+/* Supports VGA regions 0xa0000-0xbffff, 0x3b0-0x3bb, and 0x3c0-0x3df */
+#define VFIO_DEVICE_FLAGS_PCI_VGA		(1 << 4)
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };

@@ -303,6 +309,8 @@ enum {
 	VFIO_PCI_BAR5_REGION_INDEX,
 	VFIO_PCI_ROM_REGION_INDEX,
 	VFIO_PCI_CONFIG_REGION_INDEX,
+	VFIO_PCI_LEGACY_MMIO_REGION_INDEX,
+	VFIO_PCI_LEGACY_IOPORT_REGION_INDEX,
 	VFIO_PCI_NUM_REGIONS
 };