[v11,20/20] x86/cpu/amd: Do not print FW_BUG for Secure TSC

Message ID	20240731150811.156771-21-nikunj@amd.com (mailing list archive)
State	New, archived
Headers	show Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2076.outbound.protection.outlook.com [40.107.92.76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9AB611CB32F; Wed, 31 Jul 2024 15:09:48 +0000 (UTC) Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C From: Nikunj A Dadhania <nikunj@amd.com> To: <linux-kernel@vger.kernel.org>, <thomas.lendacky@amd.com>, <bp@alien8.de>, <x86@kernel.org>, <kvm@vger.kernel.org> CC: <mingo@redhat.com>, <tglx@linutronix.de>, <dave.hansen@linux.intel.com>, <pgonda@google.com>, <seanjc@google.com>, <pbonzini@redhat.com>, <nikunj@amd.com> Subject: [PATCH v11 20/20] x86/cpu/amd: Do not print FW_BUG for Secure TSC Date: Wed, 31 Jul 2024 20:38:11 +0530 Message-ID: <20240731150811.156771-21-nikunj@amd.com> In-Reply-To: <20240731150811.156771-1-nikunj@amd.com> References: <20240731150811.156771-1-nikunj@amd.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain
Series	Add Secure TSC support for SNP guests \| expand [v11,00/20] Add Secure TSC support for SNP guests [v11,01/20] virt: sev-guest: Replace dev_dbg with pr_debug [v11,02/20] virt: sev-guest: Rename local guest message variables [v11,03/20] virt: sev-guest: Fix user-visible strings [v11,04/20] virt: sev-guest: Ensure the SNP guest messages do not exceed a page [v11,05/20] virt: sev-guest: Use AES GCM crypto library [v11,06/20] x86/sev: Handle failures from snp_init() [v11,07/20] x86/sev: Cache the secrets page address [v11,08/20] virt: sev-guest: Consolidate SNP guest messaging parameters to a struct [v11,09/20] virt: sev-guest: Reduce the scope of SNP command mutex [v11,10/20] virt: sev-guest: Carve out SNP message context structure [v11,11/20] x86/sev: Carve out and export SNP guest messaging init routines [v11,12/20] x86/sev: Relocate SNP guest messaging routines to common code [v11,13/20] x86/cc: Add CC_ATTR_GUEST_SECURE_TSC [v11,14/20] x86/sev: Add Secure TSC support for SNP guests [v11,15/20] x86/sev: Change TSC MSR behavior for Secure TSC enabled guests [v11,16/20] x86/sev: Prevent RDTSC/RDTSCP interception for Secure TSC enabled guests [v11,17/20] x86/sev: Allow Secure TSC feature for SNP guests [v11,18/20] x86/sev: Mark Secure TSC as reliable clocksource [v11,19/20] x86/kvmclock: Skip kvmclock when Secure TSC is available [v11,20/20] x86/cpu/amd: Do not print FW_BUG for Secure TSC

Message ID

20240731150811.156771-21-nikunj@amd.com (mailing list archive)

State

New, archived

Headers

Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
From: Nikunj A Dadhania <nikunj@amd.com>
To: <linux-kernel@vger.kernel.org>, <thomas.lendacky@amd.com>, <bp@alien8.de>,
	<x86@kernel.org>, <kvm@vger.kernel.org>
CC: <mingo@redhat.com>, <tglx@linutronix.de>, <dave.hansen@linux.intel.com>,
	<pgonda@google.com>, <seanjc@google.com>, <pbonzini@redhat.com>,
	<nikunj@amd.com>
Subject: [PATCH v11 20/20] x86/cpu/amd: Do not print FW_BUG for Secure TSC
Date: Wed, 31 Jul 2024 20:38:11 +0530
Message-ID: <20240731150811.156771-21-nikunj@amd.com>
In-Reply-To: <20240731150811.156771-1-nikunj@amd.com>
References: <20240731150811.156771-1-nikunj@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jul 2024 15:09:45.3371
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 2431b0ee-0c4d-4678-e36d-08dcb172d009
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: 
 TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource: 
	DS3PEPF0000C37D.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4137

Series

Add Secure TSC support for SNP guests | expand

Commit Message

Nikunj A. Dadhania July 31, 2024, 3:08 p.m. UTC

When Secure TSC is enabled and TscInvariant (bit 8) in CPUID_8000_0007_edx
is set, the kernel complains with the below firmware bug:

[Firmware Bug]: TSC doesn't count with P0 frequency!

Secure TSC does not need to run at P0 frequency; the TSC frequency is set
by the VMM as part of the SNP_LAUNCH_START command. Skip this check when
Secure TSC is enabled

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Tested-by: Peter Gonda <pgonda@google.com>
---
 arch/x86/kernel/cpu/amd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tom Lendacky Sept. 13, 2024, 5:21 p.m. UTC | #1

On 7/31/24 10:08, Nikunj A Dadhania wrote:
> When Secure TSC is enabled and TscInvariant (bit 8) in CPUID_8000_0007_edx
> is set, the kernel complains with the below firmware bug:
> 
> [Firmware Bug]: TSC doesn't count with P0 frequency!
> 
> Secure TSC does not need to run at P0 frequency; the TSC frequency is set
> by the VMM as part of the SNP_LAUNCH_START command. Skip this check when
> Secure TSC is enabled
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Tested-by: Peter Gonda <pgonda@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/x86/kernel/cpu/amd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index be5889bded49..87b55d2183a0 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -370,7 +370,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
>  
>  static void bsp_init_amd(struct cpuinfo_x86 *c)
>  {
> -	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> +	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
> +	    !cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
>  
>  		if (c->x86 > 0x10 ||
>  		    (c->x86 == 0x10 && c->x86_model >= 0x2)) {

Jim Mattson Sept. 13, 2024, 5:42 p.m. UTC | #2

On Wed, Jul 31, 2024 at 8:16 AM Nikunj A Dadhania <nikunj@amd.com> wrote:
>
> When Secure TSC is enabled and TscInvariant (bit 8) in CPUID_8000_0007_edx
> is set, the kernel complains with the below firmware bug:
>
> [Firmware Bug]: TSC doesn't count with P0 frequency!
>
> Secure TSC does not need to run at P0 frequency; the TSC frequency is set
> by the VMM as part of the SNP_LAUNCH_START command. Skip this check when
> Secure TSC is enabled
>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Tested-by: Peter Gonda <pgonda@google.com>
> ---
>  arch/x86/kernel/cpu/amd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index be5889bded49..87b55d2183a0 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -370,7 +370,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
>
>  static void bsp_init_amd(struct cpuinfo_x86 *c)
>  {
> -       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> +       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
> +           !cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {

Could we extend this to never complain in a virtual machine? i.e.
...
-       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
+       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
+           !cpu_has(c, X86_FEATURE_HYPERVISOR)) {
...

Nikunj A. Dadhania Sept. 16, 2024, 11:40 a.m. UTC | #3

On 9/13/2024 11:12 PM, Jim Mattson wrote:
> On Wed, Jul 31, 2024 at 8:16 AM Nikunj A Dadhania <nikunj@amd.com> wrote:
>>
>> When Secure TSC is enabled and TscInvariant (bit 8) in CPUID_8000_0007_edx
>> is set, the kernel complains with the below firmware bug:
>>
>> [Firmware Bug]: TSC doesn't count with P0 frequency!
>>
>> Secure TSC does not need to run at P0 frequency; the TSC frequency is set
>> by the VMM as part of the SNP_LAUNCH_START command. Skip this check when
>> Secure TSC is enabled
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>> Tested-by: Peter Gonda <pgonda@google.com>
>> ---
>>  arch/x86/kernel/cpu/amd.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index be5889bded49..87b55d2183a0 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -370,7 +370,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
>>
>>  static void bsp_init_amd(struct cpuinfo_x86 *c)
>>  {
>> -       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
>> +       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
>> +           !cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
> 
> Could we extend this to never complain in a virtual machine? i.e.

Let me get more clarity on the below and your commit[1]

> ...
> -       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> +       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
> +           !cpu_has(c, X86_FEATURE_HYPERVISOR)) {
> ...

Or do this for Family 15h and above ?

Regards
Nikunj

1. https://github.com/torvalds/linux/commit/8b0e00fba934

Jim Mattson Sept. 16, 2024, 8:21 p.m. UTC | #4

On Mon, Sep 16, 2024 at 4:41 AM Nikunj A. Dadhania <nikunj@amd.com> wrote:
>
>
>
> On 9/13/2024 11:12 PM, Jim Mattson wrote:
> > On Wed, Jul 31, 2024 at 8:16 AM Nikunj A Dadhania <nikunj@amd.com> wrote:
> >>
> >> When Secure TSC is enabled and TscInvariant (bit 8) in CPUID_8000_0007_edx
> >> is set, the kernel complains with the below firmware bug:
> >>
> >> [Firmware Bug]: TSC doesn't count with P0 frequency!
> >>
> >> Secure TSC does not need to run at P0 frequency; the TSC frequency is set
> >> by the VMM as part of the SNP_LAUNCH_START command. Skip this check when
> >> Secure TSC is enabled
> >>
> >> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> >> Tested-by: Peter Gonda <pgonda@google.com>
> >> ---
> >>  arch/x86/kernel/cpu/amd.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> >> index be5889bded49..87b55d2183a0 100644
> >> --- a/arch/x86/kernel/cpu/amd.c
> >> +++ b/arch/x86/kernel/cpu/amd.c
> >> @@ -370,7 +370,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
> >>
> >>  static void bsp_init_amd(struct cpuinfo_x86 *c)
> >>  {
> >> -       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> >> +       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
> >> +           !cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
> >
> > Could we extend this to never complain in a virtual machine? i.e.
>
> Let me get more clarity on the below and your commit[1]
>
> > ...
> > -       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
> > +       if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
> > +           !cpu_has(c, X86_FEATURE_HYPERVISOR)) {
> > ...
>
> Or do this for Family 15h and above ?

I don't think there exists a virtual firmware that sets this bit on
older CPU families. In fact, before my referenced commit, it wasn't
possible.

> Regards
> Nikunj
>
> 1. https://github.com/torvalds/linux/commit/8b0e00fba934

Something like this is necessary for existing versions of Linux. I
would like to have set HW_CR.TscFreqSel[bit 24] at VCPU creation, but
Sean would not let me. So, now userspace has to do it right after VCPU
creation. I don't have any intention of adding the code to qemu, but
maybe someone will.

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index be5889bded49..87b55d2183a0 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -370,7 +370,8 @@  static void bsp_determine_snp(struct cpuinfo_x86 *c)
 
 static void bsp_init_amd(struct cpuinfo_x86 *c)
 {
-	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
+	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC) &&
+	    !cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
 
 		if (c->x86 > 0x10 ||
 		    (c->x86 == 0x10 && c->x86_model >= 0x2)) {

[v11,20/20] x86/cpu/amd: Do not print FW_BUG for Secure TSC

Commit Message

Comments

Patch