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

[sashiko-bot@kernel.org]

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