Re: [PATCH v2] xen/vcpu: ignore VCPU_SSHOTTMR_future

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.04.2023 13:45, Roger Pau Monne wrote:
> The usage of VCPU_SSHOTTMR_future in Linux prior to 4.7 is bogus.
> When the hypervisor returns -ENOTIME (timeout in the past) Linux keeps

Nit: -ETIME

> retrying to setup the timer with a higher timeout instead of
> self-injecting a timer interrupt.
>[...]
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -9,6 +9,8 @@ The format is based on [Keep a 
> Changelog](https://keepachangelog.com/en/1.0.0/)
>  ### Changed
>   - Repurpose command line gnttab_max_{maptrack_,}frames options so they don't
>     cap toolstack provided values.
> + - Ignore VCPU_SSHOTTMR_future VCPUOP_set_singleshot_timer flag. The only
> +   known user doesn't use it properly, leading to in-guest breakage.

Might this read a little better as

 - Ignore VCPUOP_set_singleshot_timer's VCPU_SSHOTTMR_future flag. The only
   known user doesn't use it properly, leading to in-guest breakage.

> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -150,7 +150,7 @@ typedef struct vcpu_set_singleshot_timer 
> vcpu_set_singleshot_timer_t;
>  DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
>  
>  /* Flags to VCPUOP_set_singleshot_timer. */
> - /* Require the timeout to be in the future (return -ETIME if it's passed). 
> */
> + /* Ignored. */

I think this could do with something like "as of Xen 4.18", as the public
header shouldn't be tied to a specific version (and then perhaps also
retaining the original text). Arguably mentioning a specific version may be
a little odd in case we'd consider backporting this, but something would
imo better be said.

All this said - while I'm likely to ack the final patch, I would still feel
a little uneasy doing so.

Jan