[ipxe-devel] [PATCH v3 3/3] [virtio] Enable and restore PCI device correctly

[Ladi Prosek]

The driver enabled both memory and I/O access even if they were
not usable, e.g. BAR not mapped by BIOS. This commit fixes it by
enabling only the BAR types actually used. The device is also
restored to the original state (in terms of the command PCI
register) on removal.

Reviewed-by: Marcel Apfelbaum <marcel at redhat.com>
Signed-off-by: Ladi Prosek <lprosek at redhat.com>
---
 src/drivers/bus/virtio-pci.c | 10 ++++++++++
 src/drivers/net/virtio-net.c | 41 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/drivers/bus/virtio-pci.c b/src/drivers/bus/virtio-pci.c
index a1c805a..0b196b5 100644
--- a/src/drivers/bus/virtio-pci.c
+++ b/src/drivers/bus/virtio-pci.c
@@ -387,6 +387,16 @@ int vpm_find_vqs(struct virtio_pci_modern_device *vdev,
         if (err) {
             goto err_map_notify;
         }
+
+        /* enable memory or I/O access if not already enabled */
+        switch (vq->notification.flags & VIRTIO_PCI_REGION_TYPE_MASK) {
+            case VIRTIO_PCI_REGION_PORT:
+                pci_enable_device(vdev->pci, PCI_COMMAND_IO, 0);
+                break;
+            case VIRTIO_PCI_REGION_MEMORY:
+                pci_enable_device(vdev->pci, PCI_COMMAND_MEM, 0);
+                break;
+        }
     }
 
     /* Select and activate all queues. Has to be done last: once we do
diff --git a/src/drivers/net/virtio-net.c b/src/drivers/net/virtio-net.c
index fe0fd4b..5b15836 100644
--- a/src/drivers/net/virtio-net.c
+++ b/src/drivers/net/virtio-net.c
@@ -92,6 +92,9 @@ struct virtnet_nic {
 	/** 0 for legacy, 1 for virtio 1.0 */
 	int virtio_version;
 
+	/** Content of the command register before we initialized the device */
+	unsigned old_pci_command;
+
 	/** Virtio 1.0 device data */
 	struct virtio_pci_modern_device vdev;
 
@@ -463,7 +466,8 @@ static int virtnet_probe_legacy ( struct pci_device *pci ) {
 	       virtnet, pci->dev.name, ioaddr, pci->irq );
 
 	/* Enable PCI bus master and reset NIC */
-	adjust_pci_device ( pci );
+	virtnet->old_pci_command = pci_enable_device ( pci,
+		PCI_COMMAND_MASTER | PCI_COMMAND_IO, 0 );
 	vp_reset ( ioaddr );
 
 	/* Load MAC address */
@@ -487,12 +491,34 @@ static int virtnet_probe_legacy ( struct pci_device *pci ) {
 	unregister_netdev ( netdev );
  err_register_netdev:
 	vp_reset ( ioaddr );
+	pci_restore_device ( pci, virtnet->old_pci_command );
 	netdev_nullify ( netdev );
 	netdev_put ( netdev );
 	return rc;
 }
 
 /**
+ * Determine if the device uses at least one region of the given type
+ *
+ * @v vdev	Virtio-net device data
+ * @v type	The region type to test
+ * @ret result	Non-zero if the type is used, zero otherwise
+ */
+static int virtnet_uses_region_type ( struct virtio_pci_modern_device *vdev,
+				      unsigned type ) {
+	if ( ( vdev->common.flags & VIRTIO_PCI_REGION_TYPE_MASK ) == type ) {
+		return 1;
+	}
+	if ( ( vdev->isr.flags & VIRTIO_PCI_REGION_TYPE_MASK ) == type ) {
+		return 1;
+	}
+	if ( ( vdev->device.flags & VIRTIO_PCI_REGION_TYPE_MASK ) == type ) {
+		return 1;
+	}
+	return 0;
+}
+
+/**
  * Probe PCI device, modern virtio 1.0
  *
  * @v pci	PCI device
@@ -504,6 +530,7 @@ static int virtnet_probe_modern ( struct pci_device *pci, int *found_dev ) {
 	struct virtnet_nic *virtnet;
 	u64 features;
 	int rc, common, isr, notify, config, device;
+	unsigned pci_command;
 
 	common = virtio_pci_find_capability ( pci, VIRTIO_PCI_CAP_COMMON_CFG );
 	if ( ! common ) {
@@ -562,7 +589,14 @@ static int virtnet_probe_modern ( struct pci_device *pci, int *found_dev ) {
 	}
 
 	/* Enable the PCI device */
-	adjust_pci_device ( pci );
+	pci_command = PCI_COMMAND_MASTER;
+	if ( virtnet_uses_region_type ( &virtnet->vdev, VIRTIO_PCI_REGION_PORT ) ) {
+		pci_command |= PCI_COMMAND_IO;
+	}
+	if ( virtnet_uses_region_type ( &virtnet->vdev, VIRTIO_PCI_REGION_MEMORY ) ) {
+		pci_command |= PCI_COMMAND_MEM;
+	}
+	virtnet->old_pci_command = pci_enable_device ( pci, pci_command, 0 );
 
 	/* Reset the device and set initial status bits */
 	vpm_reset ( &virtnet->vdev );
@@ -601,6 +635,7 @@ static int virtnet_probe_modern ( struct pci_device *pci, int *found_dev ) {
 err_register_netdev:
 err_mac_address:
 	vpm_reset ( &virtnet->vdev );
+	pci_restore_device ( pci, virtnet->old_pci_command );
 	netdev_nullify ( netdev );
 	netdev_put ( netdev );
 
@@ -638,6 +673,8 @@ static void virtnet_remove ( struct pci_device *pci ) {
 	struct net_device *netdev = pci_get_drvdata ( pci );
 	struct virtnet_nic *virtnet = netdev->priv;
 
+	pci_restore_device ( pci, virtnet->old_pci_command );
+
 	virtio_pci_unmap_capability ( &virtnet->vdev.device );
 	virtio_pci_unmap_capability ( &virtnet->vdev.isr );
 	virtio_pci_unmap_capability ( &virtnet->vdev.common );
-- 
2.5.5