From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 559E381EA0 for ; Fri, 20 Jan 2017 06:22:20 -0800 (PST) Received: by mail-io0-x233.google.com with SMTP id v96so62222706ioi.0 for ; Fri, 20 Jan 2017 06:22:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=DrCsXOapxif3ZJ8NOkQVcnmsHzPHVfOnD9TfnlYE8TI=; b=fk9B+bZf2j20fFRD+iGPlUFNvYayMP2pGKsNJcApHy1Xo9zBheZz+Ol5aNQkCQB4ZS X2mFgQl3p80tfamiM/8XW80gp4hDJw4wx3yGt5H1rdGMkSqN7nvBjETuOqaKrSxsBcuz /i5CudI00dzbmksLd2YL7Wbiy9JGl1LsuF4o8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=DrCsXOapxif3ZJ8NOkQVcnmsHzPHVfOnD9TfnlYE8TI=; b=UKhciCT5Pxv9dbFn1WiIBsmz5kMUmNbjAKImEmUGi1nEFUyuDwRNufQjHIZQ4KgNsM GoKrHvGKnhN26J5gUjCx1q6I/44nnc/x9WU8oVO24xqME24YKa6OxxUXgqiHeg1DoHz8 4DyddQ4+h+lhygRcyfgJ28m05qS8LDi8X4nI16m3s+CTTEI4Xe/jB2/3AXOb9xaPZFuF e17/kuXEzmvb6JDhAswX3GIDwai+CqhoBUZwikChKpYCymUip8b8WaRZLXidZlDTtkTM zkLE7O08lduC2yYTPa1texNoOjoMcSr5iS+UGF9wSXx3kqpbQbLgm+bsRtURXS+Rv4eS 5GTQ== X-Gm-Message-State: AIkVDXIgI+WSQqGX5kW0EE5d0It8QOe85M+1ulnWS1U5gNvaNUbcZ9IIZl6ohbgmF37UAdXQFna9C9TR9V1/xmYD X-Received: by 10.107.32.207 with SMTP id g198mr12458372iog.83.1484922139550; Fri, 20 Jan 2017 06:22:19 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.135 with HTTP; Fri, 20 Jan 2017 06:22:19 -0800 (PST) In-Reply-To: <1484922043-21762-1-git-send-email-ard.biesheuvel@linaro.org> References: <1484922043-21762-1-git-send-email-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Fri, 20 Jan 2017 14:22:19 +0000 Message-ID: To: "edk2-devel@lists.01.org" Cc: Leif Lindholm , Ryan Harkin , Ard Biesheuvel Subject: Re: [PATCH] ArmPkg/ArmGenericTimerVirtCounterLib: deal with broken generic timers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jan 2017 14:22:20 -0000 Content-Type: text/plain; charset=UTF-8 On 20 January 2017 at 14:20, Ard Biesheuvel wrote: > Users of ArmGenericTimerVirtCounterLib may execute under virtualization, > which implies that they may be affected by core errata of the host. > > Some implementations of the ARM Generic Timer are affected by errata where > reads of the counter and reads or writes to the timer value may execute > incorrectly when issued around the time the counter is incremented by > the hardware. > > Since we can easily work around this without affecting performance too > much, implement an unconditional workaround that compares two subsequent > reads of the counter to ensure the value is correct. Note that the number > for attempts should be limited to avoid breaking platforms such as QEMU > with TCG emulation, since that has been observed never to return the same > value from back to back reads of the counter register. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > > Note that this patch applies on top of the patch 'ArmPkg/ArmLib: remove > indirection layer from timer register accessors' that I send out earlier > today. > > ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c | 51 ++++++++++++++++++-- > 1 file changed, 48 insertions(+), 3 deletions(-) > > diff --git a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c > index 69a4ceb62db6..9fe673e8222c 100644 > --- a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c > +++ b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c > @@ -70,13 +70,36 @@ ArmGenericTimerGetTimerFreq ( > return ArmReadCntFrq (); > } > > +// > +// The virtual counter may be used under virtualization on a host that > +// is affected by one of the various errata where reads to the counter > +// register may return incorrect values when the access occurs at the exact > +// time that the counter is incremented by the hardware. This affects the > +// timer as well as the counter. > +// So repeat the read until we get the same value twice. Unfortunately, > +// platforms such as QEMU with TCG emulation (i.e., non-virtualized) appear > +// never to return the same value twice, so we need to set a retry limit. > +// > +#define MAX_RETRIES 200 > + > UINTN > EFIAPI > ArmGenericTimerGetTimerVal ( > VOID > ) > { > - return ArmReadCntvTval (); > + UINTN Result; > + UINTN Tries; > + > + Tries = 0; > + do { > + // > + // Keep reading until we see the same value twice in a row. See above. > + // > + Result = ArmReadCntvTval (); > + } while (Result != ArmReadCntvTval () && ++Tries < MAX_RETRIES); > + > + return Result; > } > > > @@ -86,7 +109,18 @@ ArmGenericTimerSetTimerVal ( > IN UINTN Value > ) > { > - ArmWriteCntvTval (Value); > + UINTN CounterVal; > + UINTN Tries; > + > + Tries = 0; > + do { > + // > + // Read the counter before and after the write to TVAL, to ensure that > + // the write to TVAL did not involve a corrupted sample of the counter. > + // > + CounterVal = ArmReadCntvCt (); > + ArmWriteCntvTval (Value); I wonder if we need an isb here > + } while (CounterVal != ArmReadCntvCt () && ++Tries < MAX_RETRIES); > } > > UINT64 > @@ -95,7 +129,18 @@ ArmGenericTimerGetSystemCount ( > VOID > ) > { > - return ArmReadCntvCt (); > + UINT64 Result; > + UINTN Tries; > + > + Tries = 0; > + do { > + // > + // Keep reading until we see the same value twice in a row. See above. > + // > + Result = ArmReadCntvCt (); > + } while (Result != ArmReadCntvCt () && ++Tries < MAX_RETRIES); > + > + return Result; > } > > UINTN > -- > 2.7.4 >