mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alexander Graf <graf@amazon.com>
To: <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>, <nh-open-source@amazon.com>,
	"The AWS Nitro Enclaves Team"
	<aws-nitro-enclaves-devel@amazon.com>
Subject: [PATCH 05/19] nitro_enclaves: Stop sending commands to a broken device
Date: Wed, 11 Nov 2026 13:49:54 +0000	[thread overview]
Message-ID: <712030a1a6fc70eeca39388a193246ff3d53d15d.1794393487.git.graf@amazon.com> (raw)
In-Reply-To: <cover.1794300000.git.graf@amazon.com>

The Nitro Enclaves device always answers quickly to its commands. Consider
it broken if it does not reply to a command within 120s. Except for
STOP/FREE commands which legitimately may take longer because the
hypervisor performs cleanup work that may take some time. For those,
use a 10 minute timeout.

Rework the request handler logic to enforce these limits. But still keep
processes killable during that time. Except for stop commands, which
are used as part of the slot shutdown sequence and have to block.

If a STOP/FREE command failed to respond in time (10 minutes), the
device is clearly broken. Switch to a mode then where it no longer
accepts any new commands.

Signed-off-by: Alexander Graf <graf@amazon.com>
---
 drivers/virt/nitro_enclaves/ne_misc_dev.c | 20 ++-----
 drivers/virt/nitro_enclaves/ne_pci_dev.c  | 63 ++++++++++++++++++-----
 drivers/virt/nitro_enclaves/ne_pci_dev.h  |  4 ++
 3 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 0e9d6157dcc64..964cb21b27c77 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -1500,29 +1500,17 @@ static int ne_enclave_free_slot(struct ne_pci_dev *ne_pci_dev,
 		rc = ne_do_request(pdev, ENCLAVE_STOP,
 				   &enclave_stop_request, sizeof(enclave_stop_request),
 				   &cmd_reply, sizeof(cmd_reply));
-		if (rc < 0) {
-			dev_err_ratelimited(ne_misc_dev.this_device,
-					    "Error in enclave stop [rc=%d]\n", rc);
-
+		if (rc < 0)
 			return rc;
-		}
 
 		memset(&cmd_reply, 0, sizeof(cmd_reply));
 	}
 
 	slot_free_req.slot_uid = ne_enclave->slot_uid;
 
-	rc = ne_do_request(pdev, SLOT_FREE,
-			   &slot_free_req, sizeof(slot_free_req),
-			   &cmd_reply, sizeof(cmd_reply));
-	if (rc < 0) {
-		dev_err_ratelimited(ne_misc_dev.this_device,
-				    "Error in slot free [rc=%d]\n", rc);
-
-		return rc;
-	}
-
-	return 0;
+	return ne_do_request(pdev, SLOT_FREE,
+			     &slot_free_req, sizeof(slot_free_req),
+			     &cmd_reply, sizeof(cmd_reply));
 }
 
 /**
diff --git a/drivers/virt/nitro_enclaves/ne_pci_dev.c b/drivers/virt/nitro_enclaves/ne_pci_dev.c
index 199c3c7f8c42e..70fda4d29891f 100644
--- a/drivers/virt/nitro_enclaves/ne_pci_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_pci_dev.c
@@ -46,9 +46,10 @@
 #define NE_CMD_RETRY_TOTAL_MSECS	(40000) /* 40 sec */
 
 /**
- * NE_FREE_TIMEOUT_MSECS - How long after a first -EAGAIN ENCLAVE_STOP and
- *			   SLOT_FREE may still go out. Scrubbing a large enclave
- *			   can take minutes.
+ * NE_FREE_TIMEOUT_MSECS - How long the driver waits for the reply to
+ *			   ENCLAVE_STOP or SLOT_FREE, and how long after a first
+ *			   -EAGAIN it sends either again. Scrubbing a large
+ *			   enclave can take minutes.
  */
 #define NE_FREE_TIMEOUT_MSECS		(600000) /* 10 min */
 
@@ -98,13 +99,14 @@ static void ne_retrieve_reply(struct pci_dev *pdev, struct ne_pci_dev_cmd_reply
 /**
  * ne_wait_for_reply() - Wait for a reply of a PCI device command.
  * @pdev:	PCI device for which a reply is waited.
+ * @timeout_msecs:	How long to wait for the reply.
  *
  * Context: Process context. This function is called with the ne_pci_dev mutex held.
  * Return:
  * * 0 on success.
  * * Negative return value on failure.
  */
-static int ne_wait_for_reply(struct pci_dev *pdev)
+static int ne_wait_for_reply(struct pci_dev *pdev, unsigned int timeout_msecs)
 {
 	struct ne_pci_dev *ne_pci_dev = pci_get_drvdata(pdev);
 	int rc = -EINVAL;
@@ -112,10 +114,13 @@ static int ne_wait_for_reply(struct pci_dev *pdev)
 	/*
 	 * TODO: Update to _interruptible and handle interrupted wait event
 	 * e.g. -ERESTARTSYS, incoming signals + update timeout, if needed.
+	 *
+	 * A slot that is being scrubbed holds the reply for minutes, which is
+	 * no hung task, so wait idle.
 	 */
-	rc = wait_event_timeout(ne_pci_dev->cmd_reply_wait_q,
-				atomic_read(&ne_pci_dev->cmd_reply_avail) != 0,
-				msecs_to_jiffies(NE_DEFAULT_TIMEOUT_MSECS));
+	rc = wait_event_idle_timeout(ne_pci_dev->cmd_reply_wait_q,
+				     atomic_read(&ne_pci_dev->cmd_reply_avail) != 0,
+				     msecs_to_jiffies(timeout_msecs));
 	if (!rc)
 		return -ETIMEDOUT;
 
@@ -179,9 +184,23 @@ int ne_do_request(struct pci_dev *pdev, enum ne_pci_dev_cmd_type cmd_type,
 retry:
 	/*
 	 * Use this mutex so that the PCI device handles one command request at
-	 * a time.
+	 * a time. A slot being freed can hold it for minutes, so let a task
+	 * that waits for it be killed, unless the task frees a slot itself:
+	 * release has to do that for a process that is being killed too.
 	 */
-	mutex_lock(&ne_pci_dev->pci_dev_mutex);
+	if (ne_cmd_frees_slot(cmd_type)) {
+		mutex_lock(&ne_pci_dev->pci_dev_mutex);
+	} else {
+		rc = mutex_lock_killable(&ne_pci_dev->pci_dev_mutex);
+		if (rc)
+			return rc;
+	}
+
+	if (ne_pci_dev->failed) {
+		rc = -ENODEV;
+
+		goto unlock_mutex;
+	}
 
 	if (deadline && time_after_eq(jiffies, deadline)) {
 		dev_err_ratelimited(&pdev->dev,
@@ -197,10 +216,18 @@ int ne_do_request(struct pci_dev *pdev, enum ne_pci_dev_cmd_type cmd_type,
 
 	ne_submit_request(pdev, cmd_type, cmd_request, cmd_request_size);
 
-	rc = ne_wait_for_reply(pdev);
+	rc = ne_wait_for_reply(pdev, ne_cmd_frees_slot(cmd_type) ?
+				     NE_FREE_TIMEOUT_MSECS :
+				     NE_DEFAULT_TIMEOUT_MSECS);
 	if (rc < 0) {
-		dev_err_ratelimited(&pdev->dev, "Error in wait for reply for cmd type=%u [rc=%d]\n",
-				    cmd_type, rc);
+		/*
+		 * A reply does not name the command it answers, so a reply that
+		 * comes after this would be read as the answer to the next one.
+		 */
+		ne_pci_dev->failed = true;
+
+		dev_err(&pdev->dev, "No reply for cmd type=%u, not sending more cmds\n",
+			cmd_type);
 
 		goto unlock_mutex;
 	}
@@ -242,6 +269,18 @@ int ne_do_request(struct pci_dev *pdev, enum ne_pci_dev_cmd_type cmd_type,
 	rc = 0;
 
 unlock_mutex:
+	/*
+	 * A device that does not stop or free a slot leaves the driver unable
+	 * to tell what the slot still uses, so treat it like one that did not
+	 * answer.
+	 */
+	if (rc < 0 && !ne_pci_dev->failed && ne_cmd_frees_slot(cmd_type)) {
+		ne_pci_dev->failed = true;
+
+		dev_err(&pdev->dev, "Slot not freed, cmd type=%u [rc=%d], not sending more cmds\n",
+			cmd_type, rc);
+	}
+
 	mutex_unlock(&ne_pci_dev->pci_dev_mutex);
 
 	return rc;
diff --git a/drivers/virt/nitro_enclaves/ne_pci_dev.h b/drivers/virt/nitro_enclaves/ne_pci_dev.h
index 6e9f28971a4e0..19e129d14a3a0 100644
--- a/drivers/virt/nitro_enclaves/ne_pci_dev.h
+++ b/drivers/virt/nitro_enclaves/ne_pci_dev.h
@@ -279,6 +279,9 @@ struct ne_pci_dev_cmd_reply {
  * struct ne_pci_dev - Nitro Enclaves (NE) PCI device.
  * @cmd_reply_avail:		Variable set if a reply has been sent by the
  *				PCI device.
+ * @failed:			A command got no reply in time, or the device did
+ *				not stop or free a slot. No command is sent to
+ *				the device after that.
  * @cmd_reply_wait_q:		Wait queue for handling command reply from the
  *				PCI device.
  * @enclaves_list:		List of the enclaves managed by the PCI device.
@@ -294,6 +297,7 @@ struct ne_pci_dev_cmd_reply {
  */
 struct ne_pci_dev {
 	atomic_t		cmd_reply_avail;
+	bool			failed;
 	wait_queue_head_t	cmd_reply_wait_q;
 	struct list_head	enclaves_list;
 	struct mutex		enclaves_list_mutex;
-- 
2.47.1


  parent reply	other threads:[~2026-11-11 13:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11 13:49 [PATCH 00/19] nitro_enclaves: Fix enclave release, command retries and the CPU pool Alexander Graf
2026-11-11 13:49 ` [PATCH 01/19] nitro_enclaves: Release the reference on a rejected region page Alexander Graf
2026-11-11 13:49 ` [PATCH 02/19] nitro_enclaves: Split freeing the slot from freeing the enclave Alexander Graf
2026-11-11 13:49 ` [PATCH 03/19] nitro_enclaves: Ignore the reply of a failed slot info command Alexander Graf
2026-11-11 13:49 ` [PATCH 04/19] nitro_enclaves: Retry a command the device asks us to retry Alexander Graf
2026-11-11 13:49 ` Alexander Graf [this message]
2026-11-11 13:49 ` [PATCH 06/19] nitro_enclaves: Set up the device state before requesting its interrupts Alexander Graf
2026-11-11 13:49 ` [PATCH 07/19] nitro_enclaves: Wait for file operations before unbinding the PCI device Alexander Graf

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=712030a1a6fc70eeca39388a193246ff3d53d15d.1794393487.git.graf@amazon.com \
    --to=graf@amazon.com \
    --cc=arnd@arndb.de \
    --cc=aws-nitro-enclaves-devel@amazon.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nh-open-source@amazon.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®