diff mbox series

[Part1,RFC,v4,16/36] KVM: SVM: define new SEV_FEATURES field in the VMCB Save State Area

Message ID 20210707181506.30489-17-brijesh.singh@amd.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand

Commit Message

Brijesh Singh July 7, 2021, 6:14 p.m. UTC
The hypervisor uses the SEV_FEATURES field (offset 3B0h) in the Save State
Area to control the SEV-SNP guest features such as SNPActive, vTOM,
ReflectVC etc. An SEV-SNP guest can read the SEV_FEATURES fields through
the SEV_STATUS MSR.

While at it, update the dump_vmcb() to log the VMPL level.

See APM2 Table 15-34 and B-4 for more details.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/include/asm/svm.h | 15 +++++++++++++--
 arch/x86/kvm/svm/svm.c     |  4 ++--
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Borislav Petkov Aug. 17, 2021, 5:54 p.m. UTC | #1
On Wed, Jul 07, 2021 at 01:14:46PM -0500, Brijesh Singh wrote:
> The hypervisor uses the SEV_FEATURES field (offset 3B0h) in the Save State
> Area to control the SEV-SNP guest features such as SNPActive, vTOM,
> ReflectVC etc. An SEV-SNP guest can read the SEV_FEATURES fields through
> the SEV_STATUS MSR.
> 
> While at it, update the dump_vmcb() to log the VMPL level.
> 
> See APM2 Table 15-34 and B-4 for more details.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  arch/x86/include/asm/svm.h | 15 +++++++++++++--
>  arch/x86/kvm/svm/svm.c     |  4 ++--
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 772e60efe243..ff614cdcf628 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -212,6 +212,15 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>  #define SVM_NESTED_CTL_SEV_ENABLE	BIT(1)
>  #define SVM_NESTED_CTL_SEV_ES_ENABLE	BIT(2)
>  
> +#define SVM_SEV_FEATURES_SNP_ACTIVE		BIT(0)
> +#define SVM_SEV_FEATURES_VTOM			BIT(1)
> +#define SVM_SEV_FEATURES_REFLECT_VC		BIT(2)
> +#define SVM_SEV_FEATURES_RESTRICTED_INJECTION	BIT(3)
> +#define SVM_SEV_FEATURES_ALTERNATE_INJECTION	BIT(4)
> +#define SVM_SEV_FEATURES_DEBUG_SWAP		BIT(5)
> +#define SVM_SEV_FEATURES_PREVENT_HOST_IBS	BIT(6)
> +#define SVM_SEV_FEATURES_BTB_ISOLATION		BIT(7)

Only some of those get used and only later. Please introduce only those
with the patch that adds usage.

Also,

s/SVM_SEV_FEATURES_/SVM_SEV_FEAT_/g

at least.

And by the way, why is this patch and the next 3 part of the guest set?
They look like they belong into the hypervisor set.
Borislav Petkov Aug. 17, 2021, 5:59 p.m. UTC | #2
On Tue, Aug 17, 2021 at 07:54:15PM +0200, Borislav Petkov wrote:
> And by the way, why is this patch and the next 3 part of the guest set?
> They look like they belong into the hypervisor set.

Aha, patch 20 and further need the definitions.
Brijesh Singh Aug. 17, 2021, 6:11 p.m. UTC | #3
On 8/17/21 12:54 PM, Borislav Petkov wrote:
> On Wed, Jul 07, 2021 at 01:14:46PM -0500, Brijesh Singh wrote:
>> The hypervisor uses the SEV_FEATURES field (offset 3B0h) in the Save State
>> Area to control the SEV-SNP guest features such as SNPActive, vTOM,
>> ReflectVC etc. An SEV-SNP guest can read the SEV_FEATURES fields through
>> the SEV_STATUS MSR.
>>
>> While at it, update the dump_vmcb() to log the VMPL level.
>>
>> See APM2 Table 15-34 and B-4 for more details.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   arch/x86/include/asm/svm.h | 15 +++++++++++++--
>>   arch/x86/kvm/svm/svm.c     |  4 ++--
>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 772e60efe243..ff614cdcf628 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -212,6 +212,15 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>>   #define SVM_NESTED_CTL_SEV_ENABLE	BIT(1)
>>   #define SVM_NESTED_CTL_SEV_ES_ENABLE	BIT(2)
>>   
>> +#define SVM_SEV_FEATURES_SNP_ACTIVE		BIT(0)
>> +#define SVM_SEV_FEATURES_VTOM			BIT(1)
>> +#define SVM_SEV_FEATURES_REFLECT_VC		BIT(2)
>> +#define SVM_SEV_FEATURES_RESTRICTED_INJECTION	BIT(3)
>> +#define SVM_SEV_FEATURES_ALTERNATE_INJECTION	BIT(4)
>> +#define SVM_SEV_FEATURES_DEBUG_SWAP		BIT(5)
>> +#define SVM_SEV_FEATURES_PREVENT_HOST_IBS	BIT(6)
>> +#define SVM_SEV_FEATURES_BTB_ISOLATION		BIT(7)
> 
> Only some of those get used and only later. Please introduce only those
> with the patch that adds usage.
> 

Okay.

> Also,
> 
> s/SVM_SEV_FEATURES_/SVM_SEV_FEAT_/g
> 

I can do that.

> at least.
> 
> And by the way, why is this patch and the next 3 part of the guest set?
> They look like they belong into the hypervisor set.
> 

This is needed by the AP creation, in SNP the AP creation need to 
populate the VMSA page and thus need to use some of macros and fields  etc.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 772e60efe243..ff614cdcf628 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -212,6 +212,15 @@  struct __attribute__ ((__packed__)) vmcb_control_area {
 #define SVM_NESTED_CTL_SEV_ENABLE	BIT(1)
 #define SVM_NESTED_CTL_SEV_ES_ENABLE	BIT(2)
 
+#define SVM_SEV_FEATURES_SNP_ACTIVE		BIT(0)
+#define SVM_SEV_FEATURES_VTOM			BIT(1)
+#define SVM_SEV_FEATURES_REFLECT_VC		BIT(2)
+#define SVM_SEV_FEATURES_RESTRICTED_INJECTION	BIT(3)
+#define SVM_SEV_FEATURES_ALTERNATE_INJECTION	BIT(4)
+#define SVM_SEV_FEATURES_DEBUG_SWAP		BIT(5)
+#define SVM_SEV_FEATURES_PREVENT_HOST_IBS	BIT(6)
+#define SVM_SEV_FEATURES_BTB_ISOLATION		BIT(7)
+
 struct vmcb_seg {
 	u16 selector;
 	u16 attrib;
@@ -230,7 +239,8 @@  struct vmcb_save_area {
 	struct vmcb_seg ldtr;
 	struct vmcb_seg idtr;
 	struct vmcb_seg tr;
-	u8 reserved_1[43];
+	u8 reserved_1[42];
+	u8 vmpl;
 	u8 cpl;
 	u8 reserved_2[4];
 	u64 efer;
@@ -295,7 +305,8 @@  struct vmcb_save_area {
 	u64 sw_exit_info_1;
 	u64 sw_exit_info_2;
 	u64 sw_scratch;
-	u8 reserved_11[56];
+	u64 sev_features;
+	u8 reserved_11[48];
 	u64 xcr0;
 	u8 valid_bitmap[16];
 	u64 x87_state_gpa;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e088086f3de6..293c9e03da5a 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3184,8 +3184,8 @@  static void dump_vmcb(struct kvm_vcpu *vcpu)
 	       "tr:",
 	       save01->tr.selector, save01->tr.attrib,
 	       save01->tr.limit, save01->tr.base);
-	pr_err("cpl:            %d                efer:         %016llx\n",
-		save->cpl, save->efer);
+	pr_err("vmpl: %d   cpl:  %d               efer:          %016llx\n",
+		save->vmpl, save->cpl, save->efer);
 	pr_err("%-15s %016llx %-13s %016llx\n",
 	       "cr0:", save->cr0, "cr2:", save->cr2);
 	pr_err("%-15s %016llx %-13s %016llx\n",