[PATCH v2] virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

[PATCH v2] virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

[Jun Miao]

The TD guest sends TDREPORT to the TD Quoting Enclave via a vsock or
a tdvmcall. In general, vsock is indeed much faster than tdvmcall,
and Quote requests usually take a few millisecond to complete rather
than seconds based on actual measurements.

The following get quote time via tdvmcall were obtained on the GNR:

| msleep_interruptible(time)     | 1s       | 5ms      | 1ms        |
| ------------------------------ | -------- | -------- | ---------- |
| Duration                       | 1.004 s  | 1.005 s  | 1.036 s    |
| Total(Get Quote)               | 167      | 142      | 167        |
| Success:                       | 167      | 142      | 167        |
| Failure:                       | 0        | 0        | 0          |
| Avg total / 1s                 | 0.97     | 141.31   | 166.35     |
| Avg success / 1s               | 0.97     | 141.31   | 166.35     |
| Avg total / 1s / thread        | 0.97     | 141.31   | 166.35     |
| Avg success / 1s / thread      | 0.97     | 141.31   | 166.35     |
| Min elapsed_time               | 1025.95ms| 6.85 ms  | 2.99 ms    |
| Max elapsed_time               | 1025.95ms| 10.93 ms | 10.76 ms   |

According to trace analysis, the typical execution tdvmcall get the
quote time is 4 ms. Therefore, 5 ms is a reasonable balance between
performance efficiency and CPU overhead.

Since it's a real issue, updating the polling interval to 5ms. Given
that deployed QEs respond fast, we should also reduce the maximum wait
time to 2 seconds (from 30 seconds) to fail faster on errors

And compared to the previous throughput of one request per second,
the current 5ms can get 142 requests per second delivers a
142× performance improvement, which is critical for high-frequency
use cases without vsock.

So, change the 1s (MSEC_PER_SEC) -> 5ms (5 * USEC_PER_MSEC)

Suggested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Jun Miao <jun.miao@intel.com>
---

v1 -> v2:
 - Add "Suggested-by: Sathyanarayanan Kuppuswamy"
 - Use read_poll_timeout() from <linux/iopoll.h> to simplify the
   timeout handling instead of manual loop counters.
 - Reduce the maximum wait time to 2 seconds (from 30 seconds) to fail
   faster on errors.

---
 drivers/virt/coco/tdx-guest/tdx-guest.c | 34 +++++++++++++++++--------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index d0303e31e816..a73d1f0335be 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -16,6 +16,7 @@
 #include <linux/set_memory.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/iopoll.h>
 #include <linux/sockptr.h>
 #include <linux/tsm.h>
 #include <linux/tsm-mr.h>
@@ -198,10 +199,10 @@ static void *quote_data;
 static DEFINE_MUTEX(quote_lock);
 
 /*
- * GetQuote request timeout in seconds. Expect that 30 seconds
+ * GetQuote request timeout in seconds. Expect that 2 seconds
  * is enough time for QE to respond to any Quote requests.
  */
-static u32 getquote_timeout = 30;
+static u32 getquote_timeout = 2;
 
 static long tdx_get_report0(struct tdx_report_req __user *req)
 {
@@ -251,18 +252,31 @@ static void *alloc_quote_buf(void)
  */
 static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
 {
-	int i = 0;
+	u64 status;
+	int ret;
 
 	/*
-	 * Quote requests usually take a few seconds to complete, so waking up
-	 * once per second to recheck the status is fine for this use case.
+	 * Quote requests usually take a few milliseconds to complete, so waking
+	 * up once per 5 milliseconds to recheck the status is fine for this use
+	 * case. Fixed-interval polling doesn't scale well since QE response
+	 * times vary by implementation; an interrupt-based approach would be
+	 * the proper long-term solution.
+	 *
+	 * Also break out early if a signal is pending so the caller can be
+	 * interrupted while waiting.
 	 */
-	while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
-		if (msleep_interruptible(MSEC_PER_SEC))
-			return -EINTR;
-	}
+	ret = read_poll_timeout(READ_ONCE, status,
+				status != GET_QUOTE_IN_FLIGHT ||
+					signal_pending(current),
+				5 * USEC_PER_MSEC, timeout * USEC_PER_SEC,
+				false, quote_buf->status);
+	if (ret)
+		return ret;
+
+	if (signal_pending(current))
+		return -EINTR;
 
-	return (i == timeout) ? -ETIMEDOUT : 0;
+	return 0;
 }
 
 static int tdx_report_new_locked(struct tsm_report *report, void *data)
-- 
2.43.0

Re: [PATCH v2] virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] virt: tdx-guest: wait_for_quote_completion() discards successful quote on pending signal
--

commit c4b6f45a215285a9c5c32f569a08072ce93e37a2
Author: Jun Miao <jun.miao@intel.com>

    virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

This commit replaces the manual msleep_interruptible() loop with
read_poll_timeout() to wait for TDX quote completion. It reduces the check
interval from 1s to 5ms and overall timeout from 30s to 2s to improve
performance.

> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index d0303e31e8161..a73d1f0335bee 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
[ ... ]
> @@ -251,18 +252,31 @@ static void *alloc_quote_buf(void)
>   */
>  static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
>  {
> -	int i = 0;
> +	u64 status;
> +	int ret;
>  
>  	/*
> -	 * Quote requests usually take a few seconds to complete, so waking up
> -	 * once per second to recheck the status is fine for this use case.
> +	 * Quote requests usually take a few milliseconds to complete, so waking
> +	 * up once per 5 milliseconds to recheck the status is fine for this use
> +	 * case. Fixed-interval polling doesn't scale well since QE response
> +	 * times vary by implementation; an interrupt-based approach would be
> +	 * the proper long-term solution.
> +	 *
> +	 * Also break out early if a signal is pending so the caller can be
> +	 * interrupted while waiting.
>  	 */
> -	while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
> -		if (msleep_interruptible(MSEC_PER_SEC))
> -			return -EINTR;
> -	}
> +	ret = read_poll_timeout(READ_ONCE, status,
> +				status != GET_QUOTE_IN_FLIGHT ||
> +					signal_pending(current),
> +				5 * USEC_PER_MSEC, timeout * USEC_PER_SEC,
> +				false, quote_buf->status);
> +	if (ret)
> +		return ret;
> +
> +	if (signal_pending(current))
> +		return -EINTR;

[Severity: Medium]
Will this discard a successfully completed quote if a signal happens to be
pending?

If read_poll_timeout() completes successfully because the break condition
status != GET_QUOTE_IN_FLIGHT is met, it returns 0.

However, the subsequent unconditional check for signal_pending(current) will
evaluate to true if a signal was received, causing the function to return
-EINTR instead of success.

Since the caller logs any non-zero return value from this function as a
timeout, does this force userspace to needlessly retry an expensive
hypercall while incorrectly logging a timeout error?

>  
> -	return (i == timeout) ? -ETIMEDOUT : 0;
> +	return 0;
>  }
>  
>  static int tdx_report_new_locked(struct tsm_report *report, void *data)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917085533.750434-1-jun.miao@intel.com?part=1