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

[PATCH v3] 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.
Test case tdx-quote-generation-sample/test_tdx_attest:
Start tdx_att_get_quote concurrent loop, duration: 1 s.

| msleep_interruptible(time)| 1s       | 5ms      | 2ms      | 1ms    |
| ------------------------- | -------- | -------- | -------- |--------|
| Duration                  | 1.004 s  | 1.005 s  | 1.003 s  |1.036 s |
| Total(Get Quote)          | 2        | 201      | 451      |490     |
| Success:                  | 2        | 201      | 451      |490     |
| Failure:                  | 0        | 0        | 0        |0       |
| Avg total / 1s            | 1.03     | 200.05   | 450.05   |489.89  |
| Avg success / 1s          | 1.03     | 200.05   | 450.05   |489.89  |
| Avg total / 1s / thread   | 1.03     | 200.05   | 450.05   |489.89  |
| Avg success / 1s / thread | 1.03     | 200.05   | 450.05   |489.89  |
| Min elapsed_time          | 938.05 ms| 4.84 ms  | 1.86 ms  |1.80 ms |
| Max elapsed_time          | 938.05 ms| 5.04 ms  | 4.71 ms  |3.82 ms |

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

Since it's a real issue, updating the polling interval to 2ms. Given
that deployed QEs respond fast, we should also reduce the maximum wait
time to 1 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 451 requests per second delivers a
451× performance improvement, which is critical for high-frequency
use cases without vsock.

So, change the 1s (MSEC_PER_SEC) -> 2ms (2 * 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.

v2 -> v3:
 - Reflash the test (1s 5ms 2ms 1ms) results.
 - Using read_poll_timeout() significantly shortens the wait compared with
   the previous while loop. So reduce to 2ms from the test case results.
 - Add the get quote test use case in commit:
   tdx-quote-generation-sample/test_tdx_attest

---
 drivers/virt/coco/tdx-guest/tdx-guest.c | 32 +++++++++++++++++--------
 1 file changed, 22 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..bac7cb9403f9 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 1 seconds
  * is enough time for QE to respond to any Quote requests.
  */
-static u32 getquote_timeout = 30;
+static u32 getquote_timeout = 1;
 
 static long tdx_get_report0(struct tdx_report_req __user *req)
 {
@@ -251,18 +252,29 @@ 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 2 milliseconds to recheck the status is fine for this use
+	 * case.
+	 *
+	 * 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),
+				2 * 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 v3] 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: successful quote dropped on pending signal
--

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

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

This commit reduces the get-quote polling interval from 1 second to 2
milliseconds to improve performance and throughput. It introduces
read_poll_timeout() to handle the polling and timeout logic. The maximum wait
time is also reduced from 30 seconds to 1 second.

> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index d0303e31e8161..bac7cb9403f94 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c

[ ... ]

> @@ -251,18 +252,29 @@ 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 2 milliseconds to recheck the status is fine for this use
> +	 * case.
> +	 *
> +	 * 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),
> +				2 * USEC_PER_MSEC, timeout * USEC_PER_SEC,
> +				false, quote_buf->status);
> +	if (ret)
> +		return ret;
> +
> +	if (signal_pending(current))
> +		return -EINTR;

[Severity: Medium]
Does this code inadvertently discard a successful quote if a signal happens to
be pending at the exact same time the quote completes?

When read_poll_timeout() evaluates its break condition:
status != GET_QUOTE_IN_FLIGHT || signal_pending(current)

It will terminate and return 0 if the quote completes successfully, such as
when status is GET_QUOTE_SUCCESS.

However, if a signal is also concurrently pending, the unconditional check
after the loop will return -EINTR instead.

This causes the caller tdx_report_new_locked() to propagate -EINTR, dropping
the successful quote and potentially causing user-space to retry and overwrite
it.

Could we verify that the quote is actually still in-flight before returning
-EINTR? For example, by checking:
if (quote_buf->status == GET_QUOTE_IN_FLIGHT && signal_pending(current))

> -	return (i == timeout) ? -ETIMEDOUT : 0;
> +	return 0;
>  }

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