Re: [PATCH 1/2] x86/platform/amd: move final timeout check to after final sleep

[Suma Hegde <Suma.Hegde@amd.com>]

Hi,


On 5/30/2025 9:45 PM, Jake Hillion via B4 Relay wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> From: Jake Hillion <jake@hillion.co.uk>
>
> __hsmp_send_message sleeps between result read attempts and has a
> timeout of 100ms. Under extreme load it's possible for these sleeps to
> take a long time, exceeding the 100ms. In this case the current code
> does not check the register and fails with ETIMEDOUT.
>
> Refactor the loop to ensure there is at least one read of the register
> after a sleep of any duration. This removes instances of ETIMEDOUT with
> a single caller, even with a misbehaving scheduler. Tested on AMD
> Bergamo machines.
>
> Suggested-by: Blaise Sanouillet <linux@blaise.sanouillet.com>
> Signed-off-by: Jake Hillion <jake@hillion.co.uk>

Reviewed-by: Suma Hegde <suma.hegde@amd.com>
Tested-by: Suma Hegde <suma.hegde@amd.com>


> ---
>   drivers/platform/x86/amd/hsmp/hsmp.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index e262e8a97b4542a389e09a82dad71f7d2e8b2449..f35c639457ac425e79dead2515c0eddea0759323 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -99,7 +99,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
>          short_sleep = jiffies + msecs_to_jiffies(HSMP_SHORT_SLEEP);
>          timeout = jiffies + msecs_to_jiffies(HSMP_MSG_TIMEOUT);
>
> -       while (time_before(jiffies, timeout)) {
> +       while (true) {
>                  ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_RD);
>                  if (ret) {
>                          dev_err(sock->dev, "Error %d reading mailbox status\n", ret);
> @@ -108,6 +108,10 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
>
>                  if (mbox_status != HSMP_STATUS_NOT_READY)
>                          break;
> +
> +               if (!time_before(jiffies, timeout))
> +                       break;
> +
>                  if (time_before(jiffies, short_sleep))
>                          usleep_range(50, 100);
>                  else
>
> --
> 2.47.2
>
>
Thanks and Regards,

Suma