mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Shameer Kolothum <skolothumtho@nvidia.com>
To: <kvm@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Cc: <alex@shazbot.org>, <jgg@ziepe.ca>, <kevin.tian@intel.com>,
	<kbusch@meta.com>, <michal.winiarski@intel.com>,
	<satyanarayana.k.v.p@intel.com>, <sonangp@nvidia.com>,
	<ankita@nvidia.com>, <nathanc@nvidia.com>, <mochs@nvidia.com>,
	<skolothumtho@nvidia.com>
Subject: [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses
Date: Thu, 12 Nov 2026 14:16:16 +0000	[thread overview]
Message-ID: <20261112141632.918954-5-skolothumtho@nvidia.com> (raw)
In-Reply-To: <20261112141632.918954-1-skolothumtho@nvidia.com>

Config-space handlers can touch hardware during recovery. Run them
through vfio_pci_core_access() and keep userspace copies outside SRCU
so page faults cannot delay recovery.

Defer D0 requests until the config handler has left SRCU. Select the
power-state operation under memory_lock and use the bus try-lock helper
for D0. Return transition errors to userspace.

Blocked power operations reject requests. Open and close continue to
use the internal power helper.

Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 drivers/vfio/pci/vfio_pci_priv.h   |   5 ++
 drivers/vfio/pci/vfio_pci_config.c | 122 +++++++++++++++++++++++------
 drivers/vfio/pci/vfio_pci_core.c   |   8 ++
 3 files changed, 109 insertions(+), 26 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 2a15a8956205..0ea7c710dc38 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -41,6 +41,9 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 				  char __user *buf, size_t count, loff_t *ppos,
 				  bool iswrite);
 
+int vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
+				  pci_power_t state);
+
 ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 			size_t count, loff_t *ppos, bool iswrite);
 
@@ -85,6 +88,8 @@ struct vfio_pci_access_ops {
 	/* Select and call within SRCU. The callback receives kernel buffers. */
 	long (*execute)(struct vfio_pci_core_device *vdev,
 			vfio_pci_access_fn fn, void *data);
+	/* Caller holds memory_lock. */
+	int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
 };
 
 const struct vfio_pci_access_ops *
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 9914f3ac69ae..1e54e2e8b2ef 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -111,8 +111,13 @@ 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);
+	/*
+	 * Set power_up for D0 requests. The caller performs the transition
+	 * after leaving SRCU.
+	 */
 	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 +205,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 +278,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 +306,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 +571,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 +622,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);
@@ -707,11 +717,13 @@ 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().
+ * variables and then invokes the selected power-state operation.
  */
-static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
-					  pci_power_t state)
+int vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
+				  pci_power_t state)
 {
+	int ret;
+
 	if (state >= PCI_D3hot) {
 		vfio_pci_zap_and_down_write_memory_lock(vdev);
 		vfio_pci_dma_buf_move(vdev, true);
@@ -719,27 +731,36 @@ static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
 		down_write(&vdev->memory_lock);
 	}
 
-	vfio_pci_set_power_state(vdev, state);
+	ret = vfio_pci_access_ops(vdev)->set_power_state(vdev, state);
 	if (__vfio_pci_memory_enabled(vdev))
 		vfio_pci_dma_buf_move(vdev, false);
 	up_write(&vdev->memory_lock);
+
+	return ret;
 }
 
 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 leaving access_srcu. Otherwise, the ASPM update can
+			 * wait for the bus lock while AER waits for this SRCU reader.
+			 */
+			*power_up = true;
+			return count;
 		case 1:
 			state = PCI_D1;
 			break;
@@ -751,7 +772,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 +822,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 +836,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 +906,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 +995,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 +1197,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;
 
@@ -1881,6 +1912,28 @@ static size_t vfio_pci_cap_remaining_dword(struct vfio_pci_core_device *vdev,
 	return i;
 }
 
+struct vfio_pci_config_access {
+	struct perm_bits *perm;
+	loff_t pos;
+	int count;
+	int offset;
+	__le32 val;
+	bool power_up;
+	bool iswrite;
+};
+
+static long vfio_pci_access_config(struct vfio_pci_core_device *vdev, void *data)
+{
+	struct vfio_pci_config_access *access = data;
+	struct perm_bits *perm = access->perm;
+
+	if (access->iswrite)
+		return perm->writefn(vdev, access->pos, access->count, perm,
+				     access->offset, access->val, &access->power_up);
+	return perm->readfn(vdev, access->pos, access->count, perm,
+			    access->offset, &access->val);
+}
+
 ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 				  char __user *buf, size_t count, loff_t *ppos,
 				  bool iswrite)
@@ -1889,6 +1942,7 @@ 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;
+	struct vfio_pci_config_access access = {};
 	u8 cap_id;
 	ssize_t ret;
 
@@ -1949,6 +2003,12 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 	WARN_ON(cap_start > *ppos);
 
 	offset = *ppos - cap_start;
+	access.perm = perm;
+	access.pos = *ppos;
+	access.count = count;
+	access.offset = offset;
+	access.iswrite = iswrite;
+
 
 	if (iswrite) {
 		if (!perm->writefn)
@@ -1957,11 +2017,21 @@ 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);
+		access.val = val;
+		ret = vfio_pci_core_access(vdev, vfio_pci_access_config, &access);
+		if (ret < 0)
+			return ret;
+		if (access.power_up) {
+			int err = vfio_lock_and_set_power_state(vdev, PCI_D0);
+
+			if (err)
+				return err;
+		}
 	} else {
 		if (perm->readfn) {
-			ret = perm->readfn(vdev, *ppos, count,
-					   perm, offset, &val);
+			access.val = val;
+			ret = vfio_pci_core_access(vdev, vfio_pci_access_config, &access);
+			val = access.val;
 			if (ret < 0)
 				return ret;
 		}
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 6529eab7865b..21c0dda37dd1 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -324,12 +324,20 @@ static long vfio_pci_access_blocked(struct vfio_pci_core_device *vdev,
 	return -EIO;
 }
 
+static int vfio_pci_power_blocked(struct vfio_pci_core_device *vdev,
+				  pci_power_t state)
+{
+	return -EIO;
+}
+
 static const struct vfio_pci_access_ops vfio_pci_live_ops = {
 	.execute = vfio_pci_access_live,
+	.set_power_state = vfio_pci_try_set_power_state,
 };
 
 static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
 	.execute = vfio_pci_access_blocked,
+	.set_power_state = vfio_pci_power_blocked,
 };
 
 /*
-- 
2.43.0


  parent reply	other threads:[~2026-11-12 14:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 01/20] vfio/pci: Add SRCU-protected access ops Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 02/20] PCI/PM: Add a power-state helper that tries the bus lock Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 03/20] vfio/pci: Avoid waiting for the bus lock when preparing D0 Shameer Kolothum
2026-11-12 14:16 ` Shameer Kolothum [this message]
2026-11-12 14:16 ` [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 06/20] vfio/pci: Use access ops for BAR and ROM accesses Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 07/20] vfio/pci: Use access ops when mapping BAR pages Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 08/20] vfio/pci: Use access ops for hardware IRQ operations Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 09/20] vfio/pci: Use access ops for function reset Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 10/20] vfio/pci: Use access ops for runtime-PM requests Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 12/20] vfio/pci: Use access ops for DMA-BUF export and restoration Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 13/20] vfio/pci: Track PCI recovery state Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 16/20] vfio/pci: Restore device state from slot_reset() Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 17/20] vfio/pci: Complete recovery in resume() Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 19/20] vfio/pci: Add VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints Shameer Kolothum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261112141632.918954-5-skolothumtho@nvidia.com \
    --to=skolothumtho@nvidia.com \
    --cc=alex@shazbot.org \
    --cc=ankita@nvidia.com \
    --cc=jgg@ziepe.ca \
    --cc=kbusch@meta.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=michal.winiarski@intel.com \
    --cc=mochs@nvidia.com \
    --cc=nathanc@nvidia.com \
    --cc=satyanarayana.k.v.p@intel.com \
    --cc=sonangp@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®