diff mbox series

[07/11] x86/sev: Provide SVSM discovery support

Message ID 2518c82f24f3e5d7533eea72512cea9ce985704b.1706307364.git.thomas.lendacky@amd.com
State New
Headers
Series Provide SEV-SNP support for running under an SVSM |

Commit Message

Tom Lendacky Jan. 26, 2024, 10:16 p.m. UTC 
  The SVSM specification documents an alternative method of discovery for
the SVSM using a reserved CPUID bit and a reserved MSR.

For the CPUID support, the #VC handler of an SEV-SNP guest should modify
the returned value in the EAX register for the 0x8000001f CPUID function
by setting bit 28 when an SVSM is present.

For the MSR support, new reserved MSR 0xc001f000 has been defined. A #VC
should be generated when accessing this MSR. The #VC handler is expected
to ignore writes to this MSR and return the physical calling area address
(CAA) on reads of this MSR.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/msr-index.h |  2 ++
 arch/x86/kernel/sev-shared.c     |  4 ++++
 arch/x86/kernel/sev.c            | 17 +++++++++++++++++
 3 files changed, 23 insertions(+)

Comments

Jeremi Piotrowski Jan. 29, 2024, 10:41 a.m. UTC | #1 
On Fri, Jan 26, 2024 at 04:16:00PM -0600, Tom Lendacky wrote:
> The SVSM specification documents an alternative method of discovery for
> the SVSM using a reserved CPUID bit and a reserved MSR.
> 
> For the CPUID support, the #VC handler of an SEV-SNP guest should modify
> the returned value in the EAX register for the 0x8000001f CPUID function
> by setting bit 28 when an SVSM is present.
> 

It seems awkward that the guest would have to set the bit in the CPUID response
itself. Is there no way for the SVSM to fixup the CPUID page contents, or the
CPUID instruction return?

Could you also add a definition for the feature to arch/x86/include/asm/cpufeatures.h.

> For the MSR support, new reserved MSR 0xc001f000 has been defined. A #VC
> should be generated when accessing this MSR. The #VC handler is expected
> to ignore writes to this MSR and return the physical calling area address
> (CAA) on reads of this MSR.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/include/asm/msr-index.h |  2 ++
>  arch/x86/kernel/sev-shared.c     |  4 ++++
>  arch/x86/kernel/sev.c            | 17 +++++++++++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index f1bd7b91b3c6..4746135cbe21 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -622,6 +622,8 @@
>  
>  #define MSR_AMD64_VIRT_SPEC_CTRL	0xc001011f
>  
> +#define MSR_SVSM_CAA			0xc001f000
> +
>  /* AMD Collaborative Processor Performance Control MSRs */
>  #define MSR_AMD_CPPC_CAP1		0xc00102b0
>  #define MSR_AMD_CPPC_ENABLE		0xc00102b1
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index f26e872bc5d0..9bd7d7e75b31 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -628,6 +628,10 @@ static int snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
>  		/* node ID */
>  		leaf->ecx = (leaf->ecx & GENMASK(31, 8)) | (leaf_hv.ecx & GENMASK(7, 0));
>  		break;
> +	case 0x8000001f:
> +		if (vmpl)
> +			leaf->eax |= BIT(28);
> +		break;
>  	default:
>  		/* No fix-ups needed, use values as-is. */
>  		break;
Tom Lendacky Jan. 29, 2024, 3:18 p.m. UTC | #2 
On 1/29/24 04:41, Jeremi Piotrowski wrote:
> On Fri, Jan 26, 2024 at 04:16:00PM -0600, Tom Lendacky wrote:
>> The SVSM specification documents an alternative method of discovery for
>> the SVSM using a reserved CPUID bit and a reserved MSR.
>>
>> For the CPUID support, the #VC handler of an SEV-SNP guest should modify
>> the returned value in the EAX register for the 0x8000001f CPUID function
>> by setting bit 28 when an SVSM is present.
>>
> 
> It seems awkward that the guest would have to set the bit in the CPUID response
> itself. Is there no way for the SVSM to fixup the CPUID page contents, or the
> CPUID instruction return?

It's possible for the SVSM to update the CPUID information provided to the 
guest, but it hasn't been specifically called out as a requirement in the 
SVSM spec. That is why snp_cpuid_postprocess() is used to ensure that the 
bit is set. But, yes, we could update the CPUID information in the CPUID 
page directly instead of the snp_cpuid_postprocess() change.

I'll look into that and see what it takes.

> 
> Could you also add a definition for the feature to arch/x86/include/asm/cpufeatures.h.

Yes, I missed that, will add it.

Thanks,
Tom

> 
>> For the MSR support, new reserved MSR 0xc001f000 has been defined. A #VC
>> should be generated when accessing this MSR. The #VC handler is expected
>> to ignore writes to this MSR and return the physical calling area address
>> (CAA) on reads of this MSR.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>   arch/x86/include/asm/msr-index.h |  2 ++
>>   arch/x86/kernel/sev-shared.c     |  4 ++++
>>   arch/x86/kernel/sev.c            | 17 +++++++++++++++++
>>   3 files changed, 23 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index f1bd7b91b3c6..4746135cbe21 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -622,6 +622,8 @@
>>   
>>   #define MSR_AMD64_VIRT_SPEC_CTRL	0xc001011f
>>   
>> +#define MSR_SVSM_CAA			0xc001f000
>> +
>>   /* AMD Collaborative Processor Performance Control MSRs */
>>   #define MSR_AMD_CPPC_CAP1		0xc00102b0
>>   #define MSR_AMD_CPPC_ENABLE		0xc00102b1
>> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
>> index f26e872bc5d0..9bd7d7e75b31 100644
>> --- a/arch/x86/kernel/sev-shared.c
>> +++ b/arch/x86/kernel/sev-shared.c
>> @@ -628,6 +628,10 @@ static int snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
>>   		/* node ID */
>>   		leaf->ecx = (leaf->ecx & GENMASK(31, 8)) | (leaf_hv.ecx & GENMASK(7, 0));
>>   		break;
>> +	case 0x8000001f:
>> +		if (vmpl)
>> +			leaf->eax |= BIT(28);
>> +		break;
>>   	default:
>>   		/* No fix-ups needed, use values as-is. */
>>   		break;
diff mbox series

Patch

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index f1bd7b91b3c6..4746135cbe21 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -622,6 +622,8 @@ 
 
 #define MSR_AMD64_VIRT_SPEC_CTRL	0xc001011f
 
+#define MSR_SVSM_CAA			0xc001f000
+
 /* AMD Collaborative Processor Performance Control MSRs */
 #define MSR_AMD_CPPC_CAP1		0xc00102b0
 #define MSR_AMD_CPPC_ENABLE		0xc00102b1
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index f26e872bc5d0..9bd7d7e75b31 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -628,6 +628,10 @@  static int snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
 		/* node ID */
 		leaf->ecx = (leaf->ecx & GENMASK(31, 8)) | (leaf_hv.ecx & GENMASK(7, 0));
 		break;
+	case 0x8000001f:
+		if (vmpl)
+			leaf->eax |= BIT(28);
+		break;
 	default:
 		/* No fix-ups needed, use values as-is. */
 		break;
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index ddb9141f0959..121a9bad86c9 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -1326,12 +1326,29 @@  int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
 	return 0;
 }
 
+static enum es_result vc_handle_svsm_caa_msr(struct es_em_ctxt *ctxt)
+{
+	struct pt_regs *regs = ctxt->regs;
+
+	/* Writes to the SVSM CAA msr are ignored */
+	if (ctxt->insn.opcode.bytes[1] == 0x30)
+		return ES_OK;
+
+	regs->ax = lower_32_bits(this_cpu_read(svsm_caa_pa));
+	regs->dx = upper_32_bits(this_cpu_read(svsm_caa_pa));
+
+	return ES_OK;
+}
+
 static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 {
 	struct pt_regs *regs = ctxt->regs;
 	enum es_result ret;
 	u64 exit_info_1;
 
+	if (regs->cx == MSR_SVSM_CAA)
+		return vc_handle_svsm_caa_msr(ctxt);
+
 	/* Is it a WRMSR? */
 	exit_info_1 = (ctxt->insn.opcode.bytes[1] == 0x30) ? 1 : 0;