On 20 January 2017 at 14:20, Ard Biesheuvel <[email protected]> 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 <[email protected]>
> ---
>
> 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
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
