| Message ID | 20221214194056.161492-48-michael.roth@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand |
On Wed, Dec 14, 2022 at 01:40:39PM -0600, Michael Roth wrote: > From: Brijesh Singh <brijesh.singh@amd.com> > > SEV-SNP VMs can ask the hypervisor to change the page state in the RMP > table to be private or shared using the Page State Change MSR protocol > as defined in the GHCB specification. > > Forward these requests to userspace via KVM_EXIT_VMGEXIT so the VMM can > issue the KVM ioctls to update the page state accordingly. > > Co-developed-by: Michael Roth <michael.roth@amd.com> > Signed-off-by: Michael Roth <michael.roth@amd.com> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > --- > arch/x86/include/asm/sev-common.h | 9 ++++++++ > arch/x86/kvm/svm/sev.c | 25 +++++++++++++++++++++++ > arch/x86/kvm/trace.h | 34 +++++++++++++++++++++++++++++++ > arch/x86/kvm/x86.c | 1 + > 4 files changed, 69 insertions(+) > > diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h > index 0a9055cdfae2..ee38f7408470 100644 > --- a/arch/x86/include/asm/sev-common.h > +++ b/arch/x86/include/asm/sev-common.h > @@ -93,6 +93,10 @@ enum psc_op { > }; > > #define GHCB_MSR_PSC_REQ 0x014 > +#define GHCB_MSR_PSC_GFN_POS 12 > +#define GHCB_MSR_PSC_GFN_MASK GENMASK_ULL(39, 0) > +#define GHCB_MSR_PSC_OP_POS 52 > +#define GHCB_MSR_PSC_OP_MASK 0xf > #define GHCB_MSR_PSC_REQ_GFN(gfn, op) \ > /* GHCBData[55:52] */ \ > (((u64)((op) & 0xf) << 52) | \ > @@ -102,6 +106,11 @@ enum psc_op { > GHCB_MSR_PSC_REQ) > > #define GHCB_MSR_PSC_RESP 0x015 > +#define GHCB_MSR_PSC_ERROR_POS 32 > +#define GHCB_MSR_PSC_ERROR_MASK GENMASK_ULL(31, 0) > +#define GHCB_MSR_PSC_ERROR GENMASK_ULL(31, 0) > +#define GHCB_MSR_PSC_RSVD_POS 12 > +#define GHCB_MSR_PSC_RSVD_MASK GENMASK_ULL(19, 0) > #define GHCB_MSR_PSC_RESP_VAL(val) \ > /* GHCBData[63:32] */ \ > (((u64)(val) & GENMASK_ULL(63, 32)) >> 32) > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index d7b467b620aa..d7988629073b 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -29,6 +29,7 @@ > #include "svm_ops.h" > #include "cpuid.h" > #include "trace.h" > +#include "mmu.h" > > #ifndef CONFIG_KVM_AMD_SEV > /* > @@ -3350,6 +3351,23 @@ static void set_ghcb_msr(struct vcpu_svm *svm, u64 value) > svm->vmcb->control.ghcb_gpa = value; > } > > +/* > + * TODO: need to get the value set by userspace in vcpu->run->vmgexit.ghcb_msr > + * and process that here accordingly. > + */ > +static int snp_complete_psc_msr_protocol(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + > + set_ghcb_msr_bits(svm, 0, > + GHCB_MSR_PSC_ERROR_MASK, GHCB_MSR_PSC_ERROR_POS); > + > + set_ghcb_msr_bits(svm, 0, GHCB_MSR_PSC_RSVD_MASK, GHCB_MSR_PSC_RSVD_POS); > + set_ghcb_msr_bits(svm, GHCB_MSR_PSC_RESP, GHCB_MSR_INFO_MASK, GHCB_MSR_INFO_POS); > + > + return 1; /* resume */ > +} > + > static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm) > { > struct vmcb_control_area *control = &svm->vmcb->control; > @@ -3450,6 +3468,13 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm) > GHCB_MSR_INFO_POS); > break; > } > + case GHCB_MSR_PSC_REQ: > + vcpu->run->exit_reason = KVM_EXIT_VMGEXIT; > + vcpu->run->vmgexit.ghcb_msr = control->ghcb_gpa; > + vcpu->arch.complete_userspace_io = snp_complete_psc_msr_protocol; > + > + ret = -1; > + break; What's the reasoning behind returning an error (-1) here? This error bubbles all the way up to the `KVM_RUN` ioctl. Would it be more appropriate to return 0? Returning 0 would cause a VM exit without indicating an error to userspace. > case GHCB_MSR_TERM_REQ: { > u64 reason_set, reason_code; > > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index 83843379813e..65861d2d086c 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -7,6 +7,7 @@ > #include <asm/svm.h> > #include <asm/clocksource.h> > #include <asm/pvclock-abi.h> > +#include <asm/sev-common.h> > > #undef TRACE_SYSTEM > #define TRACE_SYSTEM kvm > @@ -1831,6 +1832,39 @@ TRACE_EVENT(kvm_vmgexit_msr_protocol_exit, > __entry->vcpu_id, __entry->ghcb_gpa, __entry->result) > ); > > +/* > + * Tracepoint for the SEV-SNP page state change processing > + */ > +#define psc_operation \ > + {SNP_PAGE_STATE_PRIVATE, "private"}, \ > + {SNP_PAGE_STATE_SHARED, "shared"} \ > + > +TRACE_EVENT(kvm_snp_psc, > + TP_PROTO(unsigned int vcpu_id, u64 pfn, u64 gpa, u8 op, int level), > + TP_ARGS(vcpu_id, pfn, gpa, op, level), > + > + TP_STRUCT__entry( > + __field(int, vcpu_id) > + __field(u64, pfn) > + __field(u64, gpa) > + __field(u8, op) > + __field(int, level) > + ), > + > + TP_fast_assign( > + __entry->vcpu_id = vcpu_id; > + __entry->pfn = pfn; > + __entry->gpa = gpa; > + __entry->op = op; > + __entry->level = level; > + ), > + > + TP_printk("vcpu %u, pfn %llx, gpa %llx, op %s, level %d", > + __entry->vcpu_id, __entry->pfn, __entry->gpa, > + __print_symbolic(__entry->op, psc_operation), > + __entry->level) > +); > + > #endif /* _TRACE_KVM_H */ > > #undef TRACE_INCLUDE_PATH > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 732f9cbbadb5..08dd1ef7e136 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -13481,6 +13481,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_enter); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_exit); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_enter); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_exit); > +EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_snp_psc); > > static int __init kvm_x86_init(void) > { > -- > 2.25.1 > Regards, Tom
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h index 0a9055cdfae2..ee38f7408470 100644 --- a/arch/x86/include/asm/sev-common.h +++ b/arch/x86/include/asm/sev-common.h @@ -93,6 +93,10 @@ enum psc_op { }; #define GHCB_MSR_PSC_REQ 0x014 +#define GHCB_MSR_PSC_GFN_POS 12 +#define GHCB_MSR_PSC_GFN_MASK GENMASK_ULL(39, 0) +#define GHCB_MSR_PSC_OP_POS 52 +#define GHCB_MSR_PSC_OP_MASK 0xf #define GHCB_MSR_PSC_REQ_GFN(gfn, op) \ /* GHCBData[55:52] */ \ (((u64)((op) & 0xf) << 52) | \ @@ -102,6 +106,11 @@ enum psc_op { GHCB_MSR_PSC_REQ) #define GHCB_MSR_PSC_RESP 0x015 +#define GHCB_MSR_PSC_ERROR_POS 32 +#define GHCB_MSR_PSC_ERROR_MASK GENMASK_ULL(31, 0) +#define GHCB_MSR_PSC_ERROR GENMASK_ULL(31, 0) +#define GHCB_MSR_PSC_RSVD_POS 12 +#define GHCB_MSR_PSC_RSVD_MASK GENMASK_ULL(19, 0) #define GHCB_MSR_PSC_RESP_VAL(val) \ /* GHCBData[63:32] */ \ (((u64)(val) & GENMASK_ULL(63, 32)) >> 32) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index d7b467b620aa..d7988629073b 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -29,6 +29,7 @@ #include "svm_ops.h" #include "cpuid.h" #include "trace.h" +#include "mmu.h" #ifndef CONFIG_KVM_AMD_SEV /* @@ -3350,6 +3351,23 @@ static void set_ghcb_msr(struct vcpu_svm *svm, u64 value) svm->vmcb->control.ghcb_gpa = value; } +/* + * TODO: need to get the value set by userspace in vcpu->run->vmgexit.ghcb_msr + * and process that here accordingly. + */ +static int snp_complete_psc_msr_protocol(struct kvm_vcpu *vcpu) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + set_ghcb_msr_bits(svm, 0, + GHCB_MSR_PSC_ERROR_MASK, GHCB_MSR_PSC_ERROR_POS); + + set_ghcb_msr_bits(svm, 0, GHCB_MSR_PSC_RSVD_MASK, GHCB_MSR_PSC_RSVD_POS); + set_ghcb_msr_bits(svm, GHCB_MSR_PSC_RESP, GHCB_MSR_INFO_MASK, GHCB_MSR_INFO_POS); + + return 1; /* resume */ +} + static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm) { struct vmcb_control_area *control = &svm->vmcb->control; @@ -3450,6 +3468,13 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm) GHCB_MSR_INFO_POS); break; } + case GHCB_MSR_PSC_REQ: + vcpu->run->exit_reason = KVM_EXIT_VMGEXIT; + vcpu->run->vmgexit.ghcb_msr = control->ghcb_gpa; + vcpu->arch.complete_userspace_io = snp_complete_psc_msr_protocol; + + ret = -1; + break; case GHCB_MSR_TERM_REQ: { u64 reason_set, reason_code; diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h index 83843379813e..65861d2d086c 100644 --- a/arch/x86/kvm/trace.h +++ b/arch/x86/kvm/trace.h @@ -7,6 +7,7 @@ #include <asm/svm.h> #include <asm/clocksource.h> #include <asm/pvclock-abi.h> +#include <asm/sev-common.h> #undef TRACE_SYSTEM #define TRACE_SYSTEM kvm @@ -1831,6 +1832,39 @@ TRACE_EVENT(kvm_vmgexit_msr_protocol_exit, __entry->vcpu_id, __entry->ghcb_gpa, __entry->result) ); +/* + * Tracepoint for the SEV-SNP page state change processing + */ +#define psc_operation \ + {SNP_PAGE_STATE_PRIVATE, "private"}, \ + {SNP_PAGE_STATE_SHARED, "shared"} \ + +TRACE_EVENT(kvm_snp_psc, + TP_PROTO(unsigned int vcpu_id, u64 pfn, u64 gpa, u8 op, int level), + TP_ARGS(vcpu_id, pfn, gpa, op, level), + + TP_STRUCT__entry( + __field(int, vcpu_id) + __field(u64, pfn) + __field(u64, gpa) + __field(u8, op) + __field(int, level) + ), + + TP_fast_assign( + __entry->vcpu_id = vcpu_id; + __entry->pfn = pfn; + __entry->gpa = gpa; + __entry->op = op; + __entry->level = level; + ), + + TP_printk("vcpu %u, pfn %llx, gpa %llx, op %s, level %d", + __entry->vcpu_id, __entry->pfn, __entry->gpa, + __print_symbolic(__entry->op, psc_operation), + __entry->level) +); + #endif /* _TRACE_KVM_H */ #undef TRACE_INCLUDE_PATH diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 732f9cbbadb5..08dd1ef7e136 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -13481,6 +13481,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_enter); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_exit); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_enter); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_exit); +EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_snp_psc); static int __init kvm_x86_init(void) {