On Wed, Apr 19, 2023 at 02:14:44PM +0200, Jan Beulich wrote: > 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
Oh, thanks. > > > 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. Sure. > > --- 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. Hm, at least for XenServer we will backport this to 4.13, other vendors might backport to different versions, at which point I'm not sure the comment is very helpful. It can be misleading because it might seem to infer that checking the Xen version will tell you whether the flag is ignored or not. What about: /* * Request the timeout to be in the future (return -ETIME if it's passed) * but can be ignored by the hypervisor. */ > All this said - while I'm likely to ack the final patch, I would still feel > a little uneasy doing so. Thanks, Roger.
