From 9b176fbdc0799411f77bfe5508a822b63a32f375 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Feb 2022 12:06:54 +0100 Subject: [PATCH 1/7] Revert "boot-timestamps: Discard firmware init time when running in a VM" This reverts commit f699bd81e8e18da2d2fc11e7fb7dce95f8bb3f9e (#22063) We should suppress the TSC data when we generate it if we assume its invalid, not when we consume it, because at that point we don't even know if the data stems from TSC or something else. --- src/shared/boot-timestamps.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/shared/boot-timestamps.c b/src/shared/boot-timestamps.c index e00b37aa32..8786e89c0e 100644 --- a/src/shared/boot-timestamps.c +++ b/src/shared/boot-timestamps.c @@ -5,13 +5,11 @@ #include "efi-loader.h" #include "macro.h" #include "time-util.h" -#include "virt.h" int boot_timestamps(const dual_timestamp *n, dual_timestamp *firmware, dual_timestamp *loader) { usec_t x = 0, y = 0, a; int r; dual_timestamp _n; - bool use_firmware = true; assert(firmware); assert(loader); @@ -26,10 +24,6 @@ int boot_timestamps(const dual_timestamp *n, dual_timestamp *firmware, dual_time r = efi_loader_get_boot_usec(&x, &y); if (r < 0) return r; - - /* If we are running in a VM, the init timestamp would - * be equivalent to the host uptime. */ - use_firmware = detect_vm() <= 0; } /* Let's convert this to timestamps where the firmware @@ -39,14 +33,12 @@ int boot_timestamps(const dual_timestamp *n, dual_timestamp *firmware, dual_time * the monotonic timestamps here as negative of the actual * value. */ - if (use_firmware) { - firmware->monotonic = y; - a = n->monotonic + firmware->monotonic; - firmware->realtime = n->realtime > a ? n->realtime - a : 0; - } else - firmware->monotonic = firmware->realtime = 0; - + firmware->monotonic = y; loader->monotonic = y - x; + + a = n->monotonic + firmware->monotonic; + firmware->realtime = n->realtime > a ? n->realtime - a : 0; + a = n->monotonic + loader->monotonic; loader->realtime = n->realtime > a ? n->realtime - a : 0; From 1e66a233734a34a9eedf4b1c02cb7246f86bbd48 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:18:44 +0100 Subject: [PATCH 2/7] sd-boot: split out TSC/time API These are a whole family of functions, let's give them their own .c/.h file. No code changes, just splitting things out. --- src/boot/efi/boot.c | 1 + src/boot/efi/meson.build | 2 ++ src/boot/efi/ticks.c | 67 ++++++++++++++++++++++++++++++++++++++++ src/boot/efi/ticks.h | 9 ++++++ src/boot/efi/util.c | 62 +------------------------------------ src/boot/efi/util.h | 4 --- 6 files changed, 80 insertions(+), 65 deletions(-) create mode 100644 src/boot/efi/ticks.c create mode 100644 src/boot/efi/ticks.h diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index c431c38e9f..b2f4f873c3 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -18,6 +18,7 @@ #include "random-seed.h" #include "secure-boot.h" #include "shim.h" +#include "ticks.h" #include "util.h" #include "xbootldr.h" diff --git a/src/boot/efi/meson.build b/src/boot/efi/meson.build index d23352d48e..f6004e47ec 100644 --- a/src/boot/efi/meson.build +++ b/src/boot/efi/meson.build @@ -322,6 +322,7 @@ efi_headers = files( 'secure-boot.h', 'shim.h', 'splash.h', + 'ticks.h', 'util.h', 'xbootldr.h', ) @@ -334,6 +335,7 @@ common_sources = files( 'measure.c', 'pe.c', 'secure-boot.c', + 'ticks.c', 'util.c', ) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c new file mode 100644 index 0000000000..68b2d5806c --- /dev/null +++ b/src/boot/efi/ticks.c @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include + +#include "ticks.h" + +#ifdef __x86_64__ +UINT64 ticks_read(void) { + UINT64 a, d; + __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d)); + return (d << 32) | a; +} +#elif defined(__i386__) +UINT64 ticks_read(void) { + UINT64 val; + __asm__ volatile ("rdtsc" : "=A" (val)); + return val; +} +#elif defined(__aarch64__) +UINT64 ticks_read(void) { + UINT64 val; + __asm__ volatile ("mrs %0, cntpct_el0" : "=r" (val)); + return val; +} +#else +UINT64 ticks_read(void) { + UINT64 val = 1; + return val; +} +#endif + +#if defined(__aarch64__) +UINT64 ticks_freq(void) { + UINT64 freq; + __asm__ volatile ("mrs %0, cntfrq_el0": "=r" (freq)); + return freq; +} +#else +/* count TSC ticks during a millisecond delay */ +UINT64 ticks_freq(void) { + UINT64 ticks_start, ticks_end; + + ticks_start = ticks_read(); + BS->Stall(1000); + ticks_end = ticks_read(); + + return (ticks_end - ticks_start) * 1000UL; +} +#endif + +UINT64 time_usec(void) { + UINT64 ticks; + static UINT64 freq; + + ticks = ticks_read(); + if (ticks == 0) + return 0; + + if (freq == 0) { + freq = ticks_freq(); + if (freq == 0) + return 0; + } + + return 1000UL * 1000UL * ticks / freq; +} diff --git a/src/boot/efi/ticks.h b/src/boot/efi/ticks.h new file mode 100644 index 0000000000..389c7051b1 --- /dev/null +++ b/src/boot/efi/ticks.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include +#include + +UINT64 ticks_read(void); +UINT64 ticks_freq(void); +UINT64 time_usec(void); diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 257646551c..e6250207f6 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -3,69 +3,9 @@ #include #include +#include "ticks.h" #include "util.h" -#ifdef __x86_64__ -UINT64 ticks_read(void) { - UINT64 a, d; - __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d)); - return (d << 32) | a; -} -#elif defined(__i386__) -UINT64 ticks_read(void) { - UINT64 val; - __asm__ volatile ("rdtsc" : "=A" (val)); - return val; -} -#elif defined(__aarch64__) -UINT64 ticks_read(void) { - UINT64 val; - __asm__ volatile ("mrs %0, cntpct_el0" : "=r" (val)); - return val; -} -#else -UINT64 ticks_read(void) { - UINT64 val = 1; - return val; -} -#endif - -#if defined(__aarch64__) -UINT64 ticks_freq(void) { - UINT64 freq; - __asm__ volatile ("mrs %0, cntfrq_el0": "=r" (freq)); - return freq; -} -#else -/* count TSC ticks during a millisecond delay */ -UINT64 ticks_freq(void) { - UINT64 ticks_start, ticks_end; - - ticks_start = ticks_read(); - BS->Stall(1000); - ticks_end = ticks_read(); - - return (ticks_end - ticks_start) * 1000UL; -} -#endif - -UINT64 time_usec(void) { - UINT64 ticks; - static UINT64 freq; - - ticks = ticks_read(); - if (ticks == 0) - return 0; - - if (freq == 0) { - freq = ticks_freq(); - if (freq == 0) - return 0; - } - - return 1000UL * 1000UL * ticks / freq; -} - EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b) { assert(b); diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 3d659f7043..33b17d04fb 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -41,10 +41,6 @@ EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b); -UINT64 ticks_read(void); -UINT64 ticks_freq(void); -UINT64 time_usec(void); - EFI_STATUS efivar_set(const EFI_GUID *vendor, const CHAR16 *name, const CHAR16 *value, UINT32 flags); EFI_STATUS efivar_set_raw(const EFI_GUID *vendor, const CHAR16 *name, const void *buf, UINTN size, UINT32 flags); EFI_STATUS efivar_set_uint_string(const EFI_GUID *vendor, const CHAR16 *name, UINTN i, UINT32 flags); From 476c0e969a704387c24bd4f125e4c7c53013c876 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:23:19 +0100 Subject: [PATCH 3/7] sd-boot: add overflow check to TSC reads On some archs the counter is 32bit, and in case of virtualization it might actually overflow, who knows. --- src/boot/efi/ticks.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index 68b2d5806c..fd0e2b2348 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -45,6 +45,10 @@ UINT64 ticks_freq(void) { BS->Stall(1000); ticks_end = ticks_read(); + if (ticks_end < ticks_start) /* Check for an overflow (which is not that unlikely, given on some + * archs the value is 32bit) */ + return 0; + return (ticks_end - ticks_start) * 1000UL; } #endif From efbb86797df10ac72fe5ae29afc856d94700e2da Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:25:07 +0100 Subject: [PATCH 4/7] sd-boot: don't export ticks_read() and ticks_freq() They only have a single user in time_usec(), hence don't expose them. --- src/boot/efi/ticks.c | 12 ++++++------ src/boot/efi/ticks.h | 2 -- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index fd0e2b2348..ec6a6fd13e 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -6,39 +6,39 @@ #include "ticks.h" #ifdef __x86_64__ -UINT64 ticks_read(void) { +static UINT64 ticks_read(void) { UINT64 a, d; __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d)); return (d << 32) | a; } #elif defined(__i386__) -UINT64 ticks_read(void) { +static UINT64 ticks_read(void) { UINT64 val; __asm__ volatile ("rdtsc" : "=A" (val)); return val; } #elif defined(__aarch64__) -UINT64 ticks_read(void) { +static UINT64 ticks_read(void) { UINT64 val; __asm__ volatile ("mrs %0, cntpct_el0" : "=r" (val)); return val; } #else -UINT64 ticks_read(void) { +static UINT64 ticks_read(void) { UINT64 val = 1; return val; } #endif #if defined(__aarch64__) -UINT64 ticks_freq(void) { +static UINT64 ticks_freq(void) { UINT64 freq; __asm__ volatile ("mrs %0, cntfrq_el0": "=r" (freq)); return freq; } #else /* count TSC ticks during a millisecond delay */ -UINT64 ticks_freq(void) { +static UINT64 ticks_freq(void) { UINT64 ticks_start, ticks_end; ticks_start = ticks_read(); diff --git a/src/boot/efi/ticks.h b/src/boot/efi/ticks.h index 389c7051b1..ba259a6cc9 100644 --- a/src/boot/efi/ticks.h +++ b/src/boot/efi/ticks.h @@ -4,6 +4,4 @@ #include #include -UINT64 ticks_read(void); -UINT64 ticks_freq(void); UINT64 time_usec(void); From fb63526f726772f8fea64830ec05d69c1a48a74e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:26:49 +0100 Subject: [PATCH 5/7] sd-boot: encapsulate freq cache in ticks_freq() It's usually nicer to abstract a cache away in the provider of a value, hence do so. --- src/boot/efi/ticks.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index ec6a6fd13e..3e7ff68823 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -40,6 +40,10 @@ static UINT64 ticks_freq(void) { /* count TSC ticks during a millisecond delay */ static UINT64 ticks_freq(void) { UINT64 ticks_start, ticks_end; + static UINT64 cache = 0; + + if (cache != 0) + return cache; ticks_start = ticks_read(); BS->Stall(1000); @@ -49,23 +53,21 @@ static UINT64 ticks_freq(void) { * archs the value is 32bit) */ return 0; - return (ticks_end - ticks_start) * 1000UL; + cache = (ticks_end - ticks_start) * 1000UL; + return cache; } #endif UINT64 time_usec(void) { - UINT64 ticks; - static UINT64 freq; + UINT64 ticks, freq; ticks = ticks_read(); if (ticks == 0) return 0; - if (freq == 0) { - freq = ticks_freq(); - if (freq == 0) - return 0; - } + freq = ticks_freq(); + if (freq == 0) + return 0; return 1000UL * 1000UL * ticks / freq; } From 16cec133c63d1836e5f3ceeb452b8f65d9d210d5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:27:48 +0100 Subject: [PATCH 6/7] sd-boot: return 0 (not 1) from ticks_read() in fallback implementation The single consumer of ticks_read() (i.e. time_usec()) checks for == 0 to detect the "not supported/invalid" case, hence actually return the right value for that. --- src/boot/efi/ticks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index 3e7ff68823..4ed43d0970 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -25,8 +25,7 @@ static UINT64 ticks_read(void) { } #else static UINT64 ticks_read(void) { - UINT64 val = 1; - return val; + return 0; } #endif From 3bcc999fa555198b5155ae5bb1989bee141267bc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Feb 2022 11:35:10 +0100 Subject: [PATCH 7/7] sd-boot: don't use TSC in virtualized environments Replaces: f699bd81e8e18da2d2fc11e7fb7dce95f8bb3f9e Fixes: #22060 --- src/boot/efi/ticks.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index 4ed43d0970..45980bafe8 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -2,18 +2,47 @@ #include #include +#if defined(__i386__) || defined(__x86_64__) +#include +#endif #include "ticks.h" +#if defined(__i386__) || defined(__x86_64__) +static BOOLEAN in_hypervisor(void) { + uint32_t eax, ebx, ecx, edx; + + /* The TSC might or might not be virtualized in VMs (and thus might not be accurate or start at zero + * at boot), depending on hypervisor and CPU functionality. If it's not virtualized it's not useful + * for keeping time, hence don't attempt to use it. + * + * This is a dumbed down version of src/basic/virt.c's detect_vm() that safely works in the UEFI + * environment. */ + + if (__get_cpuid(1, &eax, &ebx, &ecx, &edx) == 0) + return FALSE; + + return !!(ecx & 0x80000000U); +} +#endif + #ifdef __x86_64__ static UINT64 ticks_read(void) { UINT64 a, d; + + if (in_hypervisor()) + return 0; + __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d)); return (d << 32) | a; } #elif defined(__i386__) static UINT64 ticks_read(void) { UINT64 val; + + if (in_hypervisor()) + return 0; + __asm__ volatile ("rdtsc" : "=A" (val)); return val; }