| Message ID | 20210430121616.2295-6-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Fri, Apr 30, 2021 at 07:16:01AM -0500, Brijesh Singh wrote: > An SNP-active guest will use the page state change NAE VMGEXIT defined in > the GHCB specification to ask the hypervisor to make the guest page > private or shared in the RMP table. In addition to the private/shared, > the guest can also ask the hypervisor to split or combine multiple 4K > validated pages as a single 2M page or vice versa. > > See GHCB specification section Page State Change for additional > information. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/include/asm/sev-common.h | 46 +++++++++++++++++++++++++++++++ > arch/x86/include/uapi/asm/svm.h | 2 ++ > 2 files changed, 48 insertions(+) > > diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h > index 8142e247d8da..07b8612bf182 100644 > --- a/arch/x86/include/asm/sev-common.h > +++ b/arch/x86/include/asm/sev-common.h > @@ -67,6 +67,52 @@ > #define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER \ > (BIT_ULL(3) | GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION) > > +/* SNP Page State Change */ > +#define GHCB_MSR_PSC_REQ 0x014 > +#define SNP_PAGE_STATE_PRIVATE 1 > +#define SNP_PAGE_STATE_SHARED 2 > +#define SNP_PAGE_STATE_PSMASH 3 > +#define SNP_PAGE_STATE_UNSMASH 4 > +#define GHCB_MSR_PSC_GFN_POS 12 > +#define GHCB_MSR_PSC_GFN_MASK 0xffffffffffULL Why don't you use GENMASK_ULL() as I suggested? All those "f"s are harder to count than seeing the start and end of a mask. > +#define GHCB_MSR_PSC_OP_POS 52 Also, having defines for 12 and 52 doesn't make it more readable, frankly. When I see GENMASK_ULL(51, 12) it is kinda clear what it is. With the defines, I have to go chase every define and replace it with the number mentally. > +#define GHCB_MSR_PSC_OP_MASK 0xf > +#define GHCB_MSR_PSC_REQ_GFN(gfn, op) \ > + (((unsigned long)((op) & GHCB_MSR_PSC_OP_MASK) << GHCB_MSR_PSC_OP_POS) | \ > + (((gfn) << GHCB_MSR_PSC_GFN_POS) & GHCB_MSR_PSC_GFN_MASK) | GHCB_MSR_PSC_REQ) > + > +#define GHCB_MSR_PSC_RESP 0x015 > +#define GHCB_MSR_PSC_ERROR_POS 32 > +#define GHCB_MSR_PSC_ERROR_MASK 0xffffffffULL > +#define GHCB_MSR_PSC_RSVD_POS 12 > +#define GHCB_MSR_PSC_RSVD_MASK 0xfffffULL > +#define GHCB_MSR_PSC_RESP_VAL(val) ((val) >> GHCB_MSR_PSC_ERROR_POS) > + > +/* SNP Page State Change NAE event */ > +#define VMGEXIT_PSC_MAX_ENTRY 253 > +#define VMGEXIT_PSC_INVALID_HEADER 0x100000001 > +#define VMGEXIT_PSC_INVALID_ENTRY 0x100000002 > +#define VMGEXIT_PSC_FIRMWARE_ERROR(x) ((x & 0xffffffffULL) | 0x200000000) > + > +struct __packed snp_page_state_header { > + u16 cur_entry; > + u16 end_entry; > + u32 reserved; > +}; > + > +struct __packed snp_page_state_entry { > + u64 cur_page:12; > + u64 gfn:40; > + u64 operation:4; > + u64 pagesize:1; > + u64 reserved:7; Please write it like this: u64 cur_page : 12, gfn : 40, ... to show that all those fields are part of the same u64. struct perf_event_attr in include/uapi/linux/perf_event.h is a good example. > +}; > + > +struct __packed snp_page_state_change { > + struct snp_page_state_header header; > + struct snp_page_state_entry entry[VMGEXIT_PSC_MAX_ENTRY]; > +}; > + > #define GHCB_MSR_TERM_REQ 0x100 > #define GHCB_MSR_TERM_REASON_SET_POS 12 > #define GHCB_MSR_TERM_REASON_SET_MASK 0xf > diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h > index 7fbc311e2de1..f7bf12cad58c 100644 > --- a/arch/x86/include/uapi/asm/svm.h > +++ b/arch/x86/include/uapi/asm/svm.h > @@ -108,6 +108,7 @@ > #define SVM_VMGEXIT_AP_JUMP_TABLE 0x80000005 > #define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0 > #define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1 > +#define SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE 0x80000010 SVM_VMGEXIT_SNP_PSC > #define SVM_VMGEXIT_HYPERVISOR_FEATURES 0x8000fffd > #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff > > @@ -216,6 +217,7 @@ > { SVM_VMGEXIT_NMI_COMPLETE, "vmgexit_nmi_complete" }, \ > { SVM_VMGEXIT_AP_HLT_LOOP, "vmgexit_ap_hlt_loop" }, \ > { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \ > + { SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE, "vmgexit_page_state_change" }, \ Ditto. Thx.
On 5/18/21 5:41 AM, Borislav Petkov wrote: > On Fri, Apr 30, 2021 at 07:16:01AM -0500, Brijesh Singh wrote: >> An SNP-active guest will use the page state change NAE VMGEXIT defined in >> the GHCB specification to ask the hypervisor to make the guest page >> private or shared in the RMP table. In addition to the private/shared, >> the guest can also ask the hypervisor to split or combine multiple 4K >> validated pages as a single 2M page or vice versa. >> >> See GHCB specification section Page State Change for additional >> information. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> arch/x86/include/asm/sev-common.h | 46 +++++++++++++++++++++++++++++++ >> arch/x86/include/uapi/asm/svm.h | 2 ++ >> 2 files changed, 48 insertions(+) >> >> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h >> index 8142e247d8da..07b8612bf182 100644 >> --- a/arch/x86/include/asm/sev-common.h >> +++ b/arch/x86/include/asm/sev-common.h >> @@ -67,6 +67,52 @@ >> #define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER \ >> (BIT_ULL(3) | GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION) >> >> +/* SNP Page State Change */ >> +#define GHCB_MSR_PSC_REQ 0x014 >> +#define SNP_PAGE_STATE_PRIVATE 1 >> +#define SNP_PAGE_STATE_SHARED 2 >> +#define SNP_PAGE_STATE_PSMASH 3 >> +#define SNP_PAGE_STATE_UNSMASH 4 >> +#define GHCB_MSR_PSC_GFN_POS 12 >> +#define GHCB_MSR_PSC_GFN_MASK 0xffffffffffULL > Why don't you use GENMASK_ULL() as I suggested? All those "f"s are > harder to count than seeing the start and end of a mask. > >> +#define GHCB_MSR_PSC_OP_POS 52 > Also, having defines for 12 and 52 doesn't make it more readable, > frankly. > > When I see GENMASK_ULL(51, 12) it is kinda clear what it is. With the > defines, I have to go chase every define and replace it with the number > mentally. I was still adhering to the previous convention in the file to define the values. I will submit prepatch to change them to also use GENMASK so that its consistent with SNP updates I add in this series. >> +#define GHCB_MSR_PSC_OP_MASK 0xf >> +#define GHCB_MSR_PSC_REQ_GFN(gfn, op) \ >> + (((unsigned long)((op) & GHCB_MSR_PSC_OP_MASK) << GHCB_MSR_PSC_OP_POS) | \ >> + (((gfn) << GHCB_MSR_PSC_GFN_POS) & GHCB_MSR_PSC_GFN_MASK) | GHCB_MSR_PSC_REQ) >> + >> +#define GHCB_MSR_PSC_RESP 0x015 >> +#define GHCB_MSR_PSC_ERROR_POS 32 >> +#define GHCB_MSR_PSC_ERROR_MASK 0xffffffffULL >> +#define GHCB_MSR_PSC_RSVD_POS 12 >> +#define GHCB_MSR_PSC_RSVD_MASK 0xfffffULL >> +#define GHCB_MSR_PSC_RESP_VAL(val) ((val) >> GHCB_MSR_PSC_ERROR_POS) >> + >> +/* SNP Page State Change NAE event */ >> +#define VMGEXIT_PSC_MAX_ENTRY 253 >> +#define VMGEXIT_PSC_INVALID_HEADER 0x100000001 >> +#define VMGEXIT_PSC_INVALID_ENTRY 0x100000002 >> +#define VMGEXIT_PSC_FIRMWARE_ERROR(x) ((x & 0xffffffffULL) | 0x200000000) >> + >> +struct __packed snp_page_state_header { >> + u16 cur_entry; >> + u16 end_entry; >> + u32 reserved; >> +}; >> + >> +struct __packed snp_page_state_entry { >> + u64 cur_page:12; >> + u64 gfn:40; >> + u64 operation:4; >> + u64 pagesize:1; >> + u64 reserved:7; > Please write it like this: > > u64 cur_page : 12, > gfn : 40, > ... > > to show that all those fields are part of the same u64. > > struct perf_event_attr in include/uapi/linux/perf_event.h is a good > example. > Ah, thanks for pointing. I will switch to using it. >> +}; >> + >> +struct __packed snp_page_state_change { >> + struct snp_page_state_header header; >> + struct snp_page_state_entry entry[VMGEXIT_PSC_MAX_ENTRY]; >> +}; >> + >> #define GHCB_MSR_TERM_REQ 0x100 >> #define GHCB_MSR_TERM_REASON_SET_POS 12 >> #define GHCB_MSR_TERM_REASON_SET_MASK 0xf >> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h >> index 7fbc311e2de1..f7bf12cad58c 100644 >> --- a/arch/x86/include/uapi/asm/svm.h >> +++ b/arch/x86/include/uapi/asm/svm.h >> @@ -108,6 +108,7 @@ >> #define SVM_VMGEXIT_AP_JUMP_TABLE 0x80000005 >> #define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0 >> #define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1 >> +#define SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE 0x80000010 > SVM_VMGEXIT_SNP_PSC Noted. > >> #define SVM_VMGEXIT_HYPERVISOR_FEATURES 0x8000fffd >> #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff >> >> @@ -216,6 +217,7 @@ >> { SVM_VMGEXIT_NMI_COMPLETE, "vmgexit_nmi_complete" }, \ >> { SVM_VMGEXIT_AP_HLT_LOOP, "vmgexit_ap_hlt_loop" }, \ >> { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \ >> + { SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE, "vmgexit_page_state_change" }, \ > Ditto. These strings are printed in the trace so I thought giving them a full name makes sense > > Thx. >
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h index 8142e247d8da..07b8612bf182 100644 --- a/arch/x86/include/asm/sev-common.h +++ b/arch/x86/include/asm/sev-common.h @@ -67,6 +67,52 @@ #define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER \ (BIT_ULL(3) | GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION) +/* SNP Page State Change */ +#define GHCB_MSR_PSC_REQ 0x014 +#define SNP_PAGE_STATE_PRIVATE 1 +#define SNP_PAGE_STATE_SHARED 2 +#define SNP_PAGE_STATE_PSMASH 3 +#define SNP_PAGE_STATE_UNSMASH 4 +#define GHCB_MSR_PSC_GFN_POS 12 +#define GHCB_MSR_PSC_GFN_MASK 0xffffffffffULL +#define GHCB_MSR_PSC_OP_POS 52 +#define GHCB_MSR_PSC_OP_MASK 0xf +#define GHCB_MSR_PSC_REQ_GFN(gfn, op) \ + (((unsigned long)((op) & GHCB_MSR_PSC_OP_MASK) << GHCB_MSR_PSC_OP_POS) | \ + (((gfn) << GHCB_MSR_PSC_GFN_POS) & GHCB_MSR_PSC_GFN_MASK) | GHCB_MSR_PSC_REQ) + +#define GHCB_MSR_PSC_RESP 0x015 +#define GHCB_MSR_PSC_ERROR_POS 32 +#define GHCB_MSR_PSC_ERROR_MASK 0xffffffffULL +#define GHCB_MSR_PSC_RSVD_POS 12 +#define GHCB_MSR_PSC_RSVD_MASK 0xfffffULL +#define GHCB_MSR_PSC_RESP_VAL(val) ((val) >> GHCB_MSR_PSC_ERROR_POS) + +/* SNP Page State Change NAE event */ +#define VMGEXIT_PSC_MAX_ENTRY 253 +#define VMGEXIT_PSC_INVALID_HEADER 0x100000001 +#define VMGEXIT_PSC_INVALID_ENTRY 0x100000002 +#define VMGEXIT_PSC_FIRMWARE_ERROR(x) ((x & 0xffffffffULL) | 0x200000000) + +struct __packed snp_page_state_header { + u16 cur_entry; + u16 end_entry; + u32 reserved; +}; + +struct __packed snp_page_state_entry { + u64 cur_page:12; + u64 gfn:40; + u64 operation:4; + u64 pagesize:1; + u64 reserved:7; +}; + +struct __packed snp_page_state_change { + struct snp_page_state_header header; + struct snp_page_state_entry entry[VMGEXIT_PSC_MAX_ENTRY]; +}; + #define GHCB_MSR_TERM_REQ 0x100 #define GHCB_MSR_TERM_REASON_SET_POS 12 #define GHCB_MSR_TERM_REASON_SET_MASK 0xf diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index 7fbc311e2de1..f7bf12cad58c 100644 --- a/arch/x86/include/uapi/asm/svm.h +++ b/arch/x86/include/uapi/asm/svm.h @@ -108,6 +108,7 @@ #define SVM_VMGEXIT_AP_JUMP_TABLE 0x80000005 #define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0 #define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1 +#define SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE 0x80000010 #define SVM_VMGEXIT_HYPERVISOR_FEATURES 0x8000fffd #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff @@ -216,6 +217,7 @@ { SVM_VMGEXIT_NMI_COMPLETE, "vmgexit_nmi_complete" }, \ { SVM_VMGEXIT_AP_HLT_LOOP, "vmgexit_ap_hlt_loop" }, \ { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \ + { SVM_VMGEXIT_SNP_PAGE_STATE_CHANGE, "vmgexit_page_state_change" }, \ { SVM_VMGEXIT_HYPERVISOR_FEATURES, "vmgexit_hypervisor_feature" }, \ { SVM_EXIT_ERR, "invalid_guest_state" }
An SNP-active guest will use the page state change NAE VMGEXIT defined in the GHCB specification to ask the hypervisor to make the guest page private or shared in the RMP table. In addition to the private/shared, the guest can also ask the hypervisor to split or combine multiple 4K validated pages as a single 2M page or vice versa. See GHCB specification section Page State Change for additional information. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/include/asm/sev-common.h | 46 +++++++++++++++++++++++++++++++ arch/x86/include/uapi/asm/svm.h | 2 ++ 2 files changed, 48 insertions(+)