When using Secure TSC, the GUEST_TSC_FREQ MSR reports a frequency based on the nominal P0 frequency, which deviates slightly (typically ~0.2%) from the actual mean TSC frequency due to clocking parameters. Over extended VM uptime, this discrepancy accumulates, causing clock skew between the hypervisor and SEV-SNP VM, leading to early timer interrupts as perceived by the guest.
The guest kernel relies on the reported nominal frequency for TSC-based timekeeping, while the actual frequency set during SNP_LAUNCH_START may differ. This mismatch results in inaccurate time calculations, causing the guest to perceive hrtimers as firing earlier than expected.
Utilize the TSC_FACTOR from the SEV firmware's secrets page (see "Secrets Page Format" in the SNP Firmware ABI Specification) to calculate the mean TSC frequency, ensuring accurate timekeeping and mitigating clock skew in SEV-SNP VMs.
Use early_ioremap_encrypted() to map the secrets page as ioremap_encrypted() uses kmalloc() which is not available during early TSC initialization and causes a panic.
Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests") Reviewed-by: Tom Lendacky thomas.lendacky@amd.com Cc: stable@vger.kernel.org Signed-off-by: Nikunj A Dadhania nikunj@amd.com --- arch/x86/include/asm/sev.h | 5 ++++- arch/x86/coco/sev/core.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 58e028d42e41..655d7e37bbcc 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -282,8 +282,11 @@ struct snp_secrets_page { u8 svsm_guest_vmpl; u8 rsvd3[3];
+ /* The percentage decrease from nominal to mean TSC frequency. */ + u32 tsc_factor; + /* Remainder of page */ - u8 rsvd4[3744]; + u8 rsvd4[3740]; } __packed;
struct snp_msg_desc { diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index b6db4e0b936b..ffd44712cec0 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void)
void __init snp_secure_tsc_init(void) { + struct snp_secrets_page *secrets; unsigned long long tsc_freq_mhz; + void *mem;
if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) return;
+ mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE); + if (!mem) { + pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n"); + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC); + } + + secrets = (__force struct snp_secrets_page *)mem; + setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);
+ /* + * Obtain the mean TSC frequency by decreasing the nominal TSC frequency with + * TSC_FACTOR as documented in the SNP Firmware ABI specification: + * + * GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001)) + * + * which is equivalent to: + * + * GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000; + */ + snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000; + x86_platform.calibrate_cpu = securetsc_get_tsc_khz; x86_platform.calibrate_tsc = securetsc_get_tsc_khz; + + early_memunmap(mem, PAGE_SIZE); }
base-commit: 337964c8abfbef645cbbe25245e25c11d9d1fc4c
Nikunj A Dadhania nikunj@amd.com writes:
When using Secure TSC, the GUEST_TSC_FREQ MSR reports a frequency based on the nominal P0 frequency, which deviates slightly (typically ~0.2%) from the actual mean TSC frequency due to clocking parameters. Over extended VM uptime, this discrepancy accumulates, causing clock skew between the hypervisor and SEV-SNP VM, leading to early timer interrupts as perceived by the guest.
The guest kernel relies on the reported nominal frequency for TSC-based timekeeping, while the actual frequency set during SNP_LAUNCH_START may differ. This mismatch results in inaccurate time calculations, causing the guest to perceive hrtimers as firing earlier than expected.
Utilize the TSC_FACTOR from the SEV firmware's secrets page (see "Secrets Page Format" in the SNP Firmware ABI Specification) to calculate the mean TSC frequency, ensuring accurate timekeeping and mitigating clock skew in SEV-SNP VMs.
Use early_ioremap_encrypted() to map the secrets page as ioremap_encrypted() uses kmalloc() which is not available during early TSC initialization and causes a panic.
Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests") Reviewed-by: Tom Lendacky thomas.lendacky@amd.com Cc: stable@vger.kernel.org Signed-off-by: Nikunj A Dadhania nikunj@amd.com
A gentle reminder for review.
Thanks Nikunj
arch/x86/include/asm/sev.h | 5 ++++- arch/x86/coco/sev/core.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 58e028d42e41..655d7e37bbcc 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -282,8 +282,11 @@ struct snp_secrets_page { u8 svsm_guest_vmpl; u8 rsvd3[3];
- /* The percentage decrease from nominal to mean TSC frequency. */
- u32 tsc_factor;
- /* Remainder of page */
- u8 rsvd4[3744];
- u8 rsvd4[3740];
} __packed; struct snp_msg_desc { diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index b6db4e0b936b..ffd44712cec0 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void) void __init snp_secure_tsc_init(void) {
- struct snp_secrets_page *secrets; unsigned long long tsc_freq_mhz;
- void *mem;
if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) return;
- mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);
- if (!mem) {
pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n");sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC);- }
- secrets = (__force struct snp_secrets_page *)mem;
- setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);
- /*
* Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/- snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;
- x86_platform.calibrate_cpu = securetsc_get_tsc_khz; x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
- early_memunmap(mem, PAGE_SIZE);
}
base-commit: 337964c8abfbef645cbbe25245e25c11d9d1fc4c
2.43.0
On Mon, Jun 23, 2025 at 9:17 PM Nikunj A Dadhania nikunj@amd.com wrote:
Nikunj A Dadhania nikunj@amd.com writes:
When using Secure TSC, the GUEST_TSC_FREQ MSR reports a frequency based on the nominal P0 frequency, which deviates slightly (typically ~0.2%) from the actual mean TSC frequency due to clocking parameters. Over extended VM uptime, this discrepancy accumulates, causing clock skew between the hypervisor and SEV-SNP VM, leading to early timer interrupts as perceived by the guest.
The guest kernel relies on the reported nominal frequency for TSC-based timekeeping, while the actual frequency set during SNP_LAUNCH_START may differ. This mismatch results in inaccurate time calculations, causing the guest to perceive hrtimers as firing earlier than expected.
Utilize the TSC_FACTOR from the SEV firmware's secrets page (see "Secrets Page Format" in the SNP Firmware ABI Specification) to calculate the mean TSC frequency, ensuring accurate timekeeping and mitigating clock skew in SEV-SNP VMs.
Use early_ioremap_encrypted() to map the secrets page as ioremap_encrypted() uses kmalloc() which is not available during early TSC initialization and causes a panic.
Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests") Reviewed-by: Tom Lendacky thomas.lendacky@amd.com Cc: stable@vger.kernel.org Signed-off-by: Nikunj A Dadhania nikunj@amd.com
A gentle reminder for review.
Thanks Nikunj
arch/x86/include/asm/sev.h | 5 ++++- arch/x86/coco/sev/core.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 58e028d42e41..655d7e37bbcc 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -282,8 +282,11 @@ struct snp_secrets_page { u8 svsm_guest_vmpl; u8 rsvd3[3];
/* The percentage decrease from nominal to mean TSC frequency. */u32 tsc_factor;/* Remainder of page */
u8 rsvd4[3744];
u8 rsvd4[3740];} __packed;
struct snp_msg_desc { diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index b6db4e0b936b..ffd44712cec0 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void)
void __init snp_secure_tsc_init(void) {
struct snp_secrets_page *secrets; unsigned long long tsc_freq_mhz;void *mem; if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) return;mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);if (!mem) {pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n");sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC);}secrets = (__force struct snp_secrets_page *)mem;setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);/** Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;
To better match the documentation and to not store an intermediate result in a global, it'd be good to add local variables. I'm also not a big fan of ABI constants in the implementation.
guest_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000); snp_tsc_freq_khz = guest_tsc_freq_khz - SNP_SCALE_TSC_FACTOR(guest_tsc_freq_khz * secrets->tsc_factor);
...in a header somewhere... /** * The SEV-SNP secrets page contains an encoding of the percentage decrease * from nominal TSC frequency to mean TSC frequency due to clocking parameters. * The encoding is in parts per 100,000, so the following is an integer-based scaling macro. */ #define SNP_SCALE_TSC_FACTOR(x) ((x) / 100000)
x86_platform.calibrate_cpu = securetsc_get_tsc_khz; x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
early_memunmap(mem, PAGE_SIZE);}
base-commit: 337964c8abfbef645cbbe25245e25c11d9d1fc4c
2.43.0
Thanks for the review.
On 6/25/2025 12:34 AM, Dionna Amalie Glaze wrote:
On Mon, Jun 23, 2025 at 9:17 PM Nikunj A Dadhania nikunj@amd.com wrote:
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index b6db4e0b936b..ffd44712cec0 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void)
void __init snp_secure_tsc_init(void) {
struct snp_secrets_page *secrets; unsigned long long tsc_freq_mhz;void *mem; if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) return;mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);if (!mem) {pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n");sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC);}secrets = (__force struct snp_secrets_page *)mem;setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);/** Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;To better match the documentation and to not store an intermediate result in a global, it'd be good to add local variables.
As there is no branches, IMHO having intermediate result should be fine and not sure if that will improve the readability as there will be three variables now in the function (tsc_freq_mhz, guest_tsc_freq_khz and snp_tsc_freq_khz) adding to confusion.
I'm also not a big fan of ABI constants in the implementation.
Sure, and we will need to move the comment above to the header as well.
guest_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000); snp_tsc_freq_khz = guest_tsc_freq_khz - SNP_SCALE_TSC_FACTOR(guest_tsc_freq_khz * secrets->tsc_factor);
...in a header somewhere... /**
- The SEV-SNP secrets page contains an encoding of the percentage decrease
- from nominal TSC frequency to mean TSC frequency due to clocking parameters.
- The encoding is in parts per 100,000, so the following is an
integer-based scaling macro. */ #define SNP_SCALE_TSC_FACTOR(x) ((x) / 100000)
How about something below:
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 655d7e37bbcc..d4151f0aa03c 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -223,6 +223,18 @@ struct snp_tsc_info_resp { u8 rsvd2[100]; } __packed;
+ +/* Obtain the mean TSC frequency by decreasing the nominal TSC frequency with + * TSC_FACTOR as documented in the SNP Firmware ABI specification: + * + * GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001)) + * + * which is equivalent to: + * + * GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000; + */ +#define SNP_SCALE_TSC_FREQ(freq, factor) ((freq) - ((freq) * (factor)) / 100000) + struct snp_guest_req { void *req_buf; size_t req_sz; diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index ffd44712cec0..9e1e8affb5a8 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2184,19 +2184,8 @@ void __init snp_secure_tsc_init(void)
setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); - snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000); - - /* - * Obtain the mean TSC frequency by decreasing the nominal TSC frequency with - * TSC_FACTOR as documented in the SNP Firmware ABI specification: - * - * GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001)) - * - * which is equivalent to: - * - * GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000; - */ - snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000; + snp_tsc_freq_khz = (unsigned long) SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, + secrets->tsc_factor);
x86_platform.calibrate_cpu = securetsc_get_tsc_khz; x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
Regards, Nikunj
On 6/24/25 23:55, Nikunj A. Dadhania wrote:
Thanks for the review.
On 6/25/2025 12:34 AM, Dionna Amalie Glaze wrote:
On Mon, Jun 23, 2025 at 9:17 PM Nikunj A Dadhania nikunj@amd.com wrote:
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index b6db4e0b936b..ffd44712cec0 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2167,15 +2167,39 @@ static unsigned long securetsc_get_tsc_khz(void)
void __init snp_secure_tsc_init(void) {
struct snp_secrets_page *secrets; unsigned long long tsc_freq_mhz;void *mem; if (!cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) return;mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);if (!mem) {pr_err("Unable to get TSC_FACTOR: failed to map the SNP secrets page.\n");sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECURE_TSC);}secrets = (__force struct snp_secrets_page *)mem;setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz); snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);/** Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;To better match the documentation and to not store an intermediate result in a global, it'd be good to add local variables.
As there is no branches, IMHO having intermediate result should be fine and not sure if that will improve the readability as there will be three variables now in the function (tsc_freq_mhz, guest_tsc_freq_khz and snp_tsc_freq_khz) adding to confusion.
I'm also not a big fan of ABI constants in the implementation.
Sure, and we will need to move the comment above to the header as well.
guest_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000); snp_tsc_freq_khz = guest_tsc_freq_khz - SNP_SCALE_TSC_FACTOR(guest_tsc_freq_khz * secrets->tsc_factor);
...in a header somewhere... /**
- The SEV-SNP secrets page contains an encoding of the percentage decrease
- from nominal TSC frequency to mean TSC frequency due to clocking parameters.
- The encoding is in parts per 100,000, so the following is an
integer-based scaling macro. */ #define SNP_SCALE_TSC_FACTOR(x) ((x) / 100000)
How about something below:
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 655d7e37bbcc..d4151f0aa03c 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -223,6 +223,18 @@ struct snp_tsc_info_resp { u8 rsvd2[100]; } __packed;
+/* Obtain the mean TSC frequency by decreasing the nominal TSC frequency with
- TSC_FACTOR as documented in the SNP Firmware ABI specification:
- GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))
- which is equivalent to:
- GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;
- */
+#define SNP_SCALE_TSC_FREQ(freq, factor) ((freq) - ((freq) * (factor)) / 100000)
struct snp_guest_req { void *req_buf; size_t req_sz; diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index ffd44712cec0..9e1e8affb5a8 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2184,19 +2184,8 @@ void __init snp_secure_tsc_init(void) setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz);
- snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);
- /*
* Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/- snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;
- snp_tsc_freq_khz = (unsigned long) SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000,
secrets->tsc_factor);
I would make any casts live in the macro. Although snp_tsc_freq_khz is a u64, right, but is always returned/used as an unsigned long? I'm wondering why it isn't defined as an unsigned long? Not sure how everything would look.
Thanks, Tom
x86_platform.calibrate_cpu = securetsc_get_tsc_khz; x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
Regards, Nikunj
On 6/25/2025 7:01 PM, Tom Lendacky wrote:
On 6/24/25 23:55, Nikunj A. Dadhania wrote:
Thanks for the review.
On 6/25/2025 12:34 AM, Dionna Amalie Glaze wrote:
On Mon, Jun 23, 2025 at 9:17 PM Nikunj A Dadhania nikunj@amd.com wrote:
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index ffd44712cec0..9e1e8affb5a8 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2184,19 +2184,8 @@ void __init snp_secure_tsc_init(void) setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ); rdmsrq(MSR_AMD64_GUEST_TSC_FREQ, tsc_freq_mhz);
- snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000);
- /*
* Obtain the mean TSC frequency by decreasing the nominal TSC frequency with* TSC_FACTOR as documented in the SNP Firmware ABI specification:** GUEST_TSC_FREQ * (1 - (TSC_FACTOR * 0.00001))** which is equivalent to:** GUEST_TSC_FREQ -= (GUEST_TSC_FREQ * TSC_FACTOR) / 100000;*/- snp_tsc_freq_khz -= (snp_tsc_freq_khz * secrets->tsc_factor) / 100000;
- snp_tsc_freq_khz = (unsigned long) SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000,
secrets->tsc_factor);I would make any casts live in the macro. Although snp_tsc_freq_khz is a u64, right, but is always returned/used as an unsigned long? I'm wondering why it isn't defined as an unsigned long? Not sure how everything would look.
The unsigned long requirement came from the calibrate callbacks:
arch/x86/include/asm/x86_init.h:312: unsigned long (*calibrate_cpu)(void); arch/x86/include/asm/x86_init.h:313: unsigned long (*calibrate_tsc)(void);
But as you suggested we can drop the cast here and securetsc_get_tsc_khz() should cast the return to unsigned long. I am trying to recall why didn't we do this in the first place.
@@ -2162,20 +2162,32 @@ void __init snp_secure_tsc_prepare(void)
static unsigned long securetsc_get_tsc_khz(void) { - return snp_tsc_freq_khz; + return (unsigned long)snp_tsc_freq_khz; }
And:
- snp_tsc_freq_khz = (unsigned long)(tsc_freq_mhz * 1000); + snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
I will send an updated patch with the above changes.
Regards Nikunj
linux-stable-mirror@lists.linaro.org
-
Dionna Amalie Glaze -
Nikunj A Dadhania -
Nikunj A. Dadhania
-
Tom Lendacky