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

RE: [PATCH] xen/arm: avoid vtimer flip-flop transition in context switch


  • To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Wei Chen <Wei.Chen@xxxxxxx>
  • Date: Wed, 15 Jun 2022 10:36:51 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; 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=DF6nkyADRZPAd/nl+T3h6ASr7u7DpDEePS/gSzT2PIg=; b=hLrr86ynyNpn8XfzezdyRMX2DfagmWMCsgirxG3pqXWwsMy9DQ46ek4k+LUAiLbeGC22Mvo4KTJYGjGCdACl7nIbR/+wSXL8QUuh2JzwH98OGIb8hKpMvHADqhecFis5udMHQ1FNhGm3sH7Jj1OoJAA9/zn5/tqhfwyxEl4nV1TQwzt4vGujbrjo6mgL9MbvpMAVaYBOMVWcO+c7aTbXlDakJ5L/q55bJ6/RdA109V1VrGvFv/ZspI3oF3TcFlWDGRAutrh8JqeeWUh1I4LcCEV8O4H68HDVjle5QJoD/yz1qYlDIvdcw8JF2qCuUP5m3c5NQsJaEHr8nYsQH/HGng==
  • 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=DF6nkyADRZPAd/nl+T3h6ASr7u7DpDEePS/gSzT2PIg=; b=Cppwx+SBZUDy6dLUkZWybsaQ7cVa6kW02U59YgZBdtKhHKEgV29ffJdIkxYvYAzxbxIG1iarx03iy8x/68M+M22Pt/3nBn0zSf4wwOLRD24i3iq/qPRH9OLd/zOkJd4orptU0y2euqx/XdeS4bqoQraD3QxwxSRqWoMyT7IsuUCXo5cW98PfokxVpr1bR1q/v0p38PFlXeDqV1jtYcdqDC5me1Ld8MhLUNQhLEmw/+SNuKSMdzqLBT5ZLi9J/VQ2ry9dxxH5Mh8gZLdVMGBIespfaEvG5yx3zlbUpU2hI05w+AYjbYLqGClZVrgm4FnGta3BY/oAgGpLKK09atrTHg==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=V3RrgsWW7rs/tt4uUU7AwvomHZWaD0D089fNBI/UXx3AGwq6DPCo4YQC/UaUaa1Z5+HO4cJ+zE4Ejz9th1U57Q0gcZFT+1p6eShM/Aryo+lFZebsj+5CvcfsIwh5wEQ8X5yMyoJhjJiy98yoJMEAUM6GrSvvZln4u37zF07G+OGf3pVT7i14hC7w/wkRW9jKe3GDlAYZV9rZko5d/br0/Kfn6lktmeRbq2lUW1Vo0tleNIPLBqBSs2KAo8EolX2hLcSHvw4DT9vkGTHKm/+lVtge3uohe8W1a67WhXQxcBtTpNAwcIA/6BMfyIV8C8dnWxa4xVj5FaDpZ3LFaApl8Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VYAnj0bhnkmaGFjzB7oAA9xu4R8+od+EeOkjDp1FEo+t6eHOrptjvpTyndffpwX2bJKIQUlJCWx6kNX/QiGoh2xswfD06Ou6+xL3utuiGLWcPbPbNJteeA6EC/tbhZqpxLmhTSfp/xNtWh6l2ep3QIVAgtKLMfezlOKYx0kUxwx5AGYjWVYahrkDLPXOaIWvhSXPLBzxsFOivlbgYxQkBe6RYeTloTKk3GJVXwJAJ/m4k3xqIFtIXQO5FAfOB3HaBiFYQCX3//2zDPJBsGtZ/K4WvX0RNtIYMj1UXq8UtJd89luoEiiunArQP0YF2IyErev9i2uJhODrSIH7aH0cfg==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: nd <nd@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 15 Jun 2022 10:37:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYgFjGGQRmuhVkyUGP4eiVYzlBr61QOR4AgAABKRA=
  • Thread-topic: [PATCH] xen/arm: avoid vtimer flip-flop transition in context switch

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2022年6月15日 17:47
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH] xen/arm: avoid vtimer flip-flop transition in context
> switch
> 
> Hi Wei,
> 
> Title: I don't quite understand what you mean by "flip-flop transition".
> 

Sorry for the no accurate words. I mean the time reaches to the MAX uint64_t
and continue from 0. Maybe an "overflow" be better for this description. 

> On 15/06/2022 02:39, Wei Chen wrote:
> > virt_vtimer_save is calculating the new time for the vtimer and
> > has a potential risk of timer flip in:
> > "v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
> > - boot_count".
> > In this formula, "cval + offset" could make uint64_t overflow.
> > Generally speaking, this is difficult to trigger.
> > But unfortunately
> > the problem was encountered with a platform where the timer started
> > with a very huge initial value, like 0xF333899122223333. On this
> > platform cval + offset is overflowing after running for a while.
> 
> I am not sure how this is a problem. Yes, uint64_t will overflow with
> "cval + offset", but AFAIK the overall result will still be correct and
> not undefined.
> 

Yes, just as you said, even overflow, but the result is still correct.
I had just noticed the overflow, but thought in a wrong way.

We have encountered this overflow in one RTOS guest:
PCNT:      0xf33ad45367e4a4ff
EL2_OFF:   0xf333333359e29a7f
BOOT_TICK: 0xf333333359e00000
VCNT:      0x0007a1200e020a80
If there is no timer in list, RTOS will calculate a huge number for
"infinite wait", for example:
VCAL:    0x0ff7a1200e020a80
EL2_OFF + VCAL - BOOT_TICK = 0x032ad45367e4a4ff - 0xf333333359e00000 = 
0xFF7A1200E04A4FF
EL2_OFF - BOOT_TICK + VCAL = 0x29a7f + 0x0ff7a1200e020a80 = 0xFF7A1200E04A4FF


> >
> > So in this patch, we adjust the formula to use "offset - boot_count"
> > first, and then use the result to plus cval. This will avoid the
> > uint64_t overflow.
> 
> Technically, the overflow is still present because the (offset -
> boot_count) is a non-zero value *and* cval is a 64-bit value.
> 

Yes, GuestOS can issue any valid 64-bit value for their usage.

> So I think the equation below should be reworked to...
> 
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> >   xen/arch/arm/vtimer.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> > index 5bb5970f58..86e63303c8 100644
> > --- a/xen/arch/arm/vtimer.c
> > +++ b/xen/arch/arm/vtimer.c
> > @@ -144,8 +144,9 @@ void virt_timer_save(struct vcpu *v)
> >       if ( (v->arch.virt_timer.ctl & CNTx_CTL_ENABLE) &&
> >            !(v->arch.virt_timer.ctl & CNTx_CTL_MASK))
> >       {
> > -        set_timer(&v->arch.virt_timer.timer, ticks_to_ns(v-
> >arch.virt_timer.cval +
> > -                  v->domain->arch.virt_timer_base.offset - boot_count));
> > +        set_timer(&v->arch.virt_timer.timer,
> > +                  ticks_to_ns(v->domain->arch.virt_timer_base.offset -
> > +                              boot_count + v->arch.virt_timer.cval));
> 
> ... something like:
> 
> ticks_to_ns(offset - boot_count) + ticks_to_ns(cval);
> 
> The first part of the equation should always be the same. So it could be
> stored in struct domain.
> 

If you think there is still some values to continue this patch, I will
address this comment : )


Thanks,
Wei Chen

> Cheers,
> 
> --
> Julien Grall

 


Rackspace

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