[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 19 Apr 2023 15:22:07 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=QkzvTkT6qCBo0nLBvino6SeRdGrgn2Mu2/5R9W2P/tw=; b=CmkU2Df/3KqSWZ28TbC93C7GPrmjZvgODKGKrEbb5zGa1SvOUPMywTsJK8oR2U9hIEz75XGO6Mnv+IEHcQQ0deDsPcbd7NR+nGlMd0kF8TPPfYkOjeioKRsS6zDuOVZ3XL4RFgSh3AHj3P1uMzXd+uu/+p/0XGI3+66jXkjWgGihHYHFXyqTJiPGl2lhdrvab1S/RDJPwiIhLyDWhB4cGh/Nk9gPXnRQgVOyo2Lhwry/tP3jCLQ5WWRfRIEht75/EFli8UirV15+WbJemUodS+M1J8ZLBQ4cQlc+b3RJ/l7sB9C5Zp0sr+6M3UHzuEqSrqLkuW9r8Ks8TP1+0Bl1WA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FXyfTx4Au/z9FUgZcbipaKHvzr3dVolHb6jL9L+9SKoEAiVzmBZvsNOv+pzpkM0eRP4+jHo3spB/ZVORyQ+4kDIv+UkcIBNPMcyZf7F4Pu11FsY9x2DmEo/8KkdmmDcwm/Rn+t/BKsHD6SKaBPnCW672jS82U+6U9P1u0t3JKgzLdeOobxnqf3WCc01Ezds6YBWpgU+bTZVRrlTFpBGKdZS1DWHLalByd476BHNG8bzLsE7ZlRY4rVuYAwXeSVWHvqKtHVBLjrrauwr+/qHabxVV3XI8dhJ/yiojCKt8IoNCiwiwvHbPduI67ZeUgvw0iiqwjfTyNOCawv/IqoxP/Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Henry Wang <Henry.Wang@xxxxxxx>, Community Manager <community.manager@xxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 19 Apr 2023 13:22:37 +0000
  • Ironport-data: A9a23:Ls/fc6x48xKake/Kc+R6t+ezxyrEfRIJ4+MujC+fZmUNrF6WrkUAn WIYUGDTPKqIYWSkeopwaYS+9x9S68DcmIRlSQI6/CAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTrafYEidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw//F+UIHUMja4mtC5QRiPKET5TcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KVhsq dAfDQ4iVxash8CW/YPld/Fy2P12eaEHPKtH0p1h5RfwKK9+BLrlHODN79Ie2yosjMdTG/qYf 9AedTdkcBXHZVtIJ0sTD5U92uyvgxETcRUB8A7T+fVxvDCVlVQvuFTuGIO9ltiiX8Jak1zev mvb12/4HgsbJJqUzj/tHneE37eSwX+nCd1CfFG+3sIxjXGv4TAvMkw9el+fp96poHDicPsKf iT4/QJr98De7neDTNPwQhm5q36spQMHVpxbFOhSwCGAzLDFpTmQAGcsRyRELtchsaceTjsv0 0KPns/4QzlmtrSaRGi15rqStSm1OyUeMSkFfyBsZQkY59jupqkjgxSJScxseIaulcH8Ezz0x zGMrQA9iq8VgMpN0L+0lXjYhxq8q56PSRQ6ji3HU2Tg4g5naYqNY42z9UOd/ftGNJyeTFSKo D4Dgcf20QwVJZSElSjISuNSGrisvq6BKGeF2QApGIQ9/TOw/XLlZZpX/Dx1OEZuNIADZCPtZ 0jQ/whW4fe/IUeXUEO+WKrpY+xC8EQqPY6Nuiz8BjaWXqVMSQ==
  • Ironport-hdrordr: A9a23:92FipKtRlJ0eLaIsy8bvG2o17skDZ9V00zEX/kB9WHVpm62j+v xG+c5xvyMc5wxhO03I5urwWpVoLUmzyXcX2+Us1NWZPDUO0VHARL2KhrGM/9SPIUzDH+dmpM JdT5Q=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.