[RFC PATCH v2 02/16] vfio/pci: Gate config space access

[Shameer Kolothum <skolothumtho@nvidia.com>]

Protect config callbacks with the access gate. Keep user copies outside
SRCU because userfaultfd can stall them indefinitely.

Run D0 transitions after releasing SRCU to avoid waiting for pci_bus_sem
while AER waits for readers to drain. Record blocked D0 requests for
replay in resume(). Return -EIO for blocked D1/D2/D3 requests, which are
not queued for replay. Preserve existing handling of hardware power-state
errors outside recovery.

Add a power_up output parameter to writefn so the PM callback can request
a D0 transition after the caller releases access_srcu.

Assisted-by: LLM
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
Note: The memory_lock/pci_bus_sem dependency remains unresolved. See
the cover letter's "Locking and open questions" section.
---
 include/linux/vfio_pci_core.h      |   2 +
 drivers/vfio/pci/vfio_pci_config.c | 104 +++++++++++++++++++++++------
 2 files changed, 84 insertions(+), 22 deletions(-)

diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index de0993280344..1dc9630dc740 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -138,6 +138,8 @@ struct vfio_pci_core_device {
 	bool			sriov_active;
 	struct pci_saved_state	*pci_saved_state;
 	struct pci_saved_state	*pm_save;
+	/* Deferred D0 request, protected by memory_lock. */
+	bool			power_up_pending;
 	int			ioeventfds_nr;
 	struct vfio_pci_eventfd __rcu *err_trigger;
 	struct vfio_pci_eventfd __rcu *req_trigger;
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 9914f3ac69ae..e9480b573381 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -111,8 +111,14 @@ struct perm_bits {
 	u8	*write;		/* writeable bits */
 	int	(*readfn)(struct vfio_pci_core_device *vdev, int pos, int count,
 			  struct perm_bits *perm, int offset, __le32 *val);
+	/*
+	 * Write callbacks run under access_srcu. Defer operations that take
+	 * pci_bus_sem: AER may hold its read lock while waiting for SRCU,
+	 * and a queued writer can block further read acquisitions.
+	 */
 	int	(*writefn)(struct vfio_pci_core_device *vdev, int pos, int count,
-			   struct perm_bits *perm, int offset, __le32 val);
+			   struct perm_bits *perm, int offset, __le32 val,
+			   bool *power_up);
 };
 
 #define	NO_VIRT		0
@@ -200,7 +206,8 @@ static int vfio_default_config_read(struct vfio_pci_core_device *vdev, int pos,
 
 static int vfio_default_config_write(struct vfio_pci_core_device *vdev, int pos,
 				     int count, struct perm_bits *perm,
-				     int offset, __le32 val)
+				     int offset, __le32 val,
+				     bool *power_up)
 {
 	__le32 virt = 0, write = 0;
 
@@ -272,7 +279,8 @@ static int vfio_direct_config_read(struct vfio_pci_core_device *vdev, int pos,
 /* Raw access skips any kind of virtualization */
 static int vfio_raw_config_write(struct vfio_pci_core_device *vdev, int pos,
 				 int count, struct perm_bits *perm,
-				 int offset, __le32 val)
+				 int offset, __le32 val,
+				 bool *power_up)
 {
 	int ret;
 
@@ -299,7 +307,8 @@ static int vfio_raw_config_read(struct vfio_pci_core_device *vdev, int pos,
 /* Virt access uses only virtualization */
 static int vfio_virt_config_write(struct vfio_pci_core_device *vdev, int pos,
 				  int count, struct perm_bits *perm,
-				  int offset, __le32 val)
+				  int offset, __le32 val,
+				  bool *power_up)
 {
 	memcpy(vdev->vconfig + pos, &val, count);
 	return count;
@@ -563,7 +572,8 @@ static bool vfio_need_bar_restore(struct vfio_pci_core_device *vdev)
 
 static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos,
 				   int count, struct perm_bits *perm,
-				   int offset, __le32 val)
+				   int offset, __le32 val,
+				   bool *power_up)
 {
 	struct pci_dev *pdev = vdev->pdev;
 	__le16 *virt_cmd;
@@ -613,7 +623,8 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos,
 			vfio_bar_restore(vdev);
 	}
 
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0) {
 		if (offset == PCI_COMMAND)
 			up_write(&vdev->memory_lock);
@@ -709,8 +720,8 @@ static int __init init_pci_cap_basic_perm(struct perm_bits *perm)
  * It takes all the required locks to protect the access of power related
  * variables and then invokes vfio_pci_set_power_state().
  */
-static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
-					  pci_power_t state)
+static int vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
+					 pci_power_t state)
 {
 	if (state >= PCI_D3hot) {
 		vfio_pci_zap_and_down_write_memory_lock(vdev);
@@ -719,27 +730,51 @@ static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
 		down_write(&vdev->memory_lock);
 	}
 
+	/*
+	 * Defer D0 until recovery completes if access is already blocked.
+	 *
+	 * This check does not prevent a block starting during the transition.
+	 * A lock dependency remains: D0 takes pci_bus_sem under memory_lock,
+	 * while AER takes memory_lock under pci_bus_sem. A queued bus writer
+	 * can block the D0 reader and deadlock both paths.
+	 */
+	if (vdev->pci_recovery_supported && READ_ONCE(vdev->access_blocked)) {
+		if (state == PCI_D0)
+			vdev->power_up_pending = true;
+		up_write(&vdev->memory_lock);
+		return state == PCI_D0 ? 0 : -EIO;
+	}
+
 	vfio_pci_set_power_state(vdev, state);
 	if (__vfio_pci_memory_enabled(vdev))
 		vfio_pci_dma_buf_move(vdev, false);
 	up_write(&vdev->memory_lock);
+
+	return 0;
 }
 
 static int vfio_pm_config_write(struct vfio_pci_core_device *vdev, int pos,
 				int count, struct perm_bits *perm,
-				int offset, __le32 val)
+				int offset, __le32 val,
+				bool *power_up)
 {
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0)
 		return count;
 
 	if (offset == PCI_PM_CTRL) {
 		pci_power_t state;
+		int ret;
 
 		switch (le32_to_cpu(val) & PCI_PM_CTRL_STATE_MASK) {
 		case 0:
-			state = PCI_D0;
-			break;
+			/*
+			 * Request D0 after dropping access_srcu; the ASPM update can
+			 * otherwise deadlock with AER waiting for SRCU readers.
+			 */
+			*power_up = true;
+			return count;
 		case 1:
 			state = PCI_D1;
 			break;
@@ -751,7 +786,9 @@ static int vfio_pm_config_write(struct vfio_pci_core_device *vdev, int pos,
 			break;
 		}
 
-		vfio_lock_and_set_power_state(vdev, state);
+		ret = vfio_lock_and_set_power_state(vdev, state);
+		if (ret)
+			return ret;
 	}
 
 	return count;
@@ -799,7 +836,8 @@ static int __init init_pci_cap_pm_perm(struct perm_bits *perm)
 
 static int vfio_vpd_config_write(struct vfio_pci_core_device *vdev, int pos,
 				 int count, struct perm_bits *perm,
-				 int offset, __le32 val)
+				 int offset, __le32 val,
+				 bool *power_up)
 {
 	struct pci_dev *pdev = vdev->pdev;
 	__le16 *paddr = (__le16 *)(vdev->vconfig + pos - offset + PCI_VPD_ADDR);
@@ -812,7 +850,8 @@ static int vfio_vpd_config_write(struct vfio_pci_core_device *vdev, int pos,
 	 * of PCI_VPD_ADDR, then the PCI_VPD_ADDR_F bit is written and we
 	 * have work to do.
 	 */
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0 || offset > PCI_VPD_ADDR + 1 ||
 	    offset + count <= PCI_VPD_ADDR + 1)
 		return count;
@@ -881,13 +920,15 @@ static int __init init_pci_cap_pcix_perm(struct perm_bits *perm)
 
 static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos,
 				 int count, struct perm_bits *perm,
-				 int offset, __le32 val)
+				 int offset, __le32 val,
+				 bool *power_up)
 {
 	__le16 *ctrl = (__le16 *)(vdev->vconfig + pos -
 				  offset + PCI_EXP_DEVCTL);
 	int readrq = le16_to_cpu(*ctrl) & PCI_EXP_DEVCTL_READRQ;
 
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0)
 		return count;
 
@@ -968,11 +1009,13 @@ static int __init init_pci_cap_exp_perm(struct perm_bits *perm)
 
 static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos,
 				int count, struct perm_bits *perm,
-				int offset, __le32 val)
+				int offset, __le32 val,
+				bool *power_up)
 {
 	u8 *ctrl = vdev->vconfig + pos - offset + PCI_AF_CTRL;
 
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0)
 		return count;
 
@@ -1168,9 +1211,11 @@ static int vfio_msi_config_read(struct vfio_pci_core_device *vdev, int pos,
 
 static int vfio_msi_config_write(struct vfio_pci_core_device *vdev, int pos,
 				 int count, struct perm_bits *perm,
-				 int offset, __le32 val)
+				 int offset, __le32 val,
+				 bool *power_up)
 {
-	count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+	count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+					  power_up);
 	if (count < 0)
 		return count;
 
@@ -1889,6 +1934,8 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 	struct perm_bits *perm;
 	__le32 val = 0;
 	int cap_start = 0, offset;
+	int idx;
+	bool power_up = false;
 	u8 cap_id;
 	ssize_t ret;
 
@@ -1957,11 +2004,24 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 		if (copy_from_user(&val, buf, count))
 			return -EFAULT;
 
-		ret = perm->writefn(vdev, *ppos, count, perm, offset, val);
+		idx = vfio_pci_core_access_begin(vdev);
+		if (idx < 0)
+			return idx;
+		ret = perm->writefn(vdev, *ppos, count, perm, offset, val,
+				    &power_up);
+		vfio_pci_core_access_end(vdev, idx);
+		if (ret < 0)
+			return ret;
+		if (power_up)
+			vfio_lock_and_set_power_state(vdev, PCI_D0);
 	} else {
 		if (perm->readfn) {
+			idx = vfio_pci_core_access_begin(vdev);
+			if (idx < 0)
+				return idx;
 			ret = perm->readfn(vdev, *ppos, count,
 					   perm, offset, &val);
+			vfio_pci_core_access_end(vdev, idx);
 			if (ret < 0)
 				return ret;
 		}
-- 
2.43.0