[PAL/Linux-SGX] Add "parse frequency from /proc/cpuinfo" fallback

[dimakuv]

Description of the changes

Some hypervisors (like Microsoft Hyper-V) currently do not expose CPUID leaves 0x15 and 0x16 (Core Crystal Clock/Process Frequency), and also do not expose hypervisor-specific synthetic CPUID leaf 0x40000010 (Timing Information). In this case, if "Invariant TSC" feature is found in CPUID leaves, we know that TSC is stable and we need to find its frequency: we try to parse the "Processor Brand String" CPUID leaves and extract the CPU frequency from that string (assuming that TSC frequency is equal to this base CPU frequency).

For context see:

• Gramine always use ocall_gettime() instead of rdtsc in QEMU VMs #1009
• [PAL/Linux-SGX] Add Invariant TSC query fallback for hypervisors #1012 (prerequisite for this PR)
• [PAL/Linux-SGX] Use clock_gettime() from untrusted vDSO #1154 (failed attempt)

How to test this PR?

Manually, e.g. on MS Azure cloud.

---

This change is 

[kailun-qin]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

---

pal/src/host/linux-sgx/pal_misc.c line 139 at r1 (raw file):

    model_name[sizeof(model_name) - 1] = '\0';

    const char* s = strchr(model_name, '@');

Is this method (using @ pattern) always correct?

Any special reason that we don't follow the algo suggested in the SDM (specifically "Algorithm for Extracting Processor Frequency" in Section 3.2 CPUID, Volume. 2A, https://www.felixcloutier.com/x86/cpuid#extracting-the-processor-frequency-from-brand-strings)?

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin)

---

pal/src/host/linux-sgx/pal_misc.c line 139 at r1 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Is this method (using @ pattern) always correct?

Any special reason that we don't follow the algo suggested in the SDM (specifically "Algorithm for Extracting Processor Frequency" in Section 3.2 CPUID, Volume. 2A, https://www.felixcloutier.com/x86/cpuid#extracting-the-processor-frequency-from-brand-strings)?

Done. Thanks, Kailun, I wasn't aware of that text. Now I closely follow that algorithm.

---

pal/src/host/linux-sgx/pal_misc.c line 145 at r2 (raw file):

        hz_str--;
    if (hz_str - model_name < 3 || *hz_str-- != 'z' || *hz_str-- != 'H')
        return 0;

FYI: I first tried to use strstr() but it searches from left to right and ignores the \0 in the "needle" arg. So strings like 1.0THz 3.0GHz were incorrectly parsed (the first number was taken).

So I ended up just doing iterations over chars.

[kailun-qin]

Reviewable status: 0 of 1 files reviewed, 6 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

---

pal/src/host/linux-sgx/pal_misc.c line 123 at r2 (raw file):

}

/* return TSC frequency or 0 if cannot parse CPU model name */

brand string (just a bit more precise), same for the below.

Code quote:

model name

---

pal/src/host/linux-sgx/pal_misc.c line 127 at r2 (raw file):

    uint32_t words[CPUID_WORD_NUM];

    char model_name[48 + 1] = {0};

The returned brand string should always be NULL-terminated. Why we need an extra byte for \0?

---

pal/src/host/linux-sgx/pal_misc.c line 145 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

FYI: I first tried to use strstr() but it searches from left to right and ignores the \0 in the "needle" arg. So strings like 1.0THz 3.0GHz were incorrectly parsed (the first number was taken).

So I ended up just doing iterations over chars.

An alternative is to reverse the string first and then strstr().

---

pal/src/host/linux-sgx/pal_misc.c line 149 at r2 (raw file):

    uint64_t multiplier = 0;
    if (*hz_str == 'T')
        multiplier = 1000UL * 1000 * 1000 * 1000;

Suggest to make macros (e.g., KHZ, MHZ, GHZ, THZ) for theses and have MHZ, GHZ, THZ built on top of KHZ like #define MHZ(x) (KHZ(x) * 1000).

Code quote:

1000UL * 1000 * 1000 * 1000

---

pal/src/host/linux-sgx/pal_misc.c line 159 at r2 (raw file):

    /* scan digits in reverse order until we hit space/tab or beginning of string */
    const char* s = hz_str;
    while (s > model_name && *s != ' ' && *s != '\t')

Not blocking but do we need to check \t?

Code quote:

*s != '\t'

---

pal/src/host/linux-sgx/pal_misc.c line 170 at r2 (raw file):

        return 0;
    }
    if (base <= 0 || base >= 1000) {

Though w/ very low possibility, why can't base == 0?

Code quote:

base <= 0

---

pal/src/host/linux-sgx/pal_misc.c line 179 at r2 (raw file):

        s++;
        fractional = strtol(s, &end, 10);
        if (fractional <= 0 || end - s > 3) {

What about fractional == 0? It's normal to have sth like 2.0GHz or maybe 2.00GHz I guess? Seems the current version will return 0 frequency directly.

Code quote:

fractional <= 0

[dimakuv]

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin)

---

pal/src/host/linux-sgx/pal_misc.c line 123 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

brand string (just a bit more precise), same for the below.

Done

---

pal/src/host/linux-sgx/pal_misc.c line 127 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

The returned brand string should always be NULL-terminated. Why we need an extra byte for \0?

1. To be on the safe side always (Linux also does the same, though I'm lazy to find the link again).
2. Malicious VM can intercept CPUID and provide some garbage non-NULL-terminated value. Gramine should protect against such a case, otherwise we may have buffer overrun vulnerabilities.

---

pal/src/host/linux-sgx/pal_misc.c line 145 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

An alternative is to reverse the string first and then strstr().

Meh, I feel like reversing the string makes the parsing logic only more unreadable.

---

pal/src/host/linux-sgx/pal_misc.c line 149 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Suggest to make macros (e.g., KHZ, MHZ, GHZ, THZ) for theses and have MHZ, GHZ, THZ built on top of KHZ like #define MHZ(x) (KHZ(x) * 1000).

Done, with slightly more readable names

---

pal/src/host/linux-sgx/pal_misc.c line 159 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Not blocking but do we need to check \t?

Well, the Intel SDM says scan digits until blank, and I'm not sure what blank means here. So I'm defaulting to the classic "space or tab".

---

pal/src/host/linux-sgx/pal_misc.c line 170 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Though w/ very low possibility, why can't base == 0?

Done. You're right, probably smth like 0.8GHz is possible.

---

pal/src/host/linux-sgx/pal_misc.c line 179 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

What about fractional == 0? It's normal to have sth like 2.0GHz or maybe 2.00GHz I guess? Seems the current version will return 0 frequency directly.

Done. Good catch.

[kailun-qin]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners

[dimakuv]

To the best of my knowledge, the missing CPUID leaves were added in newer MS Hyper-V versions. Also, the PR #1834 supersedes this one anyway. Closing.