| Message ID | 20220209164420.8894-8-varad.gautam@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add #VC exception handling for AMD SEV-ES | expand |
On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@suse.com> wrote: > > Using Linux's CPUID #VC processing logic. > > Signed-off-by: Varad Gautam <varad.gautam@suse.com> > --- > lib/x86/amd_sev_vc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 98 insertions(+) > > diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c > index 142f2cd..9ee67c0 100644 > --- a/lib/x86/amd_sev_vc.c > +++ b/lib/x86/amd_sev_vc.c > @@ -2,6 +2,7 @@ > > #include "amd_sev.h" > #include "svm.h" > +#include "x86/xsave.h" > > extern phys_addr_t ghcb_addr; > > @@ -52,6 +53,100 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt) > ctxt->regs->rip += ctxt->insn.length; > } > > +static inline u64 lower_bits(u64 val, unsigned int bits) > +{ > + u64 mask = (1ULL << bits) - 1; > + > + return (val & mask); > +} This isn't used in this patch. I guess it ends up being used later, in path 9: "x86: AMD SEV-ES: Handle IOIO #VC". Let's introduce it there if we're going to put it in this file. Though, again, maybe it's worth creating a platform agnostic bit library, and put this and `_test_bit()` (introduced in a previous patch) there. > + > +static inline void sev_es_wr_ghcb_msr(u64 val) > +{ > + wrmsr(MSR_AMD64_SEV_ES_GHCB, val); > +} > + > +static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, > + struct es_em_ctxt *ctxt, > + u64 exit_code, u64 exit_info_1, > + u64 exit_info_2) > +{ > + enum es_result ret; > + > + /* Fill in protocol and format specifiers */ > + ghcb->protocol_version = GHCB_PROTOCOL_MAX; > + ghcb->ghcb_usage = GHCB_DEFAULT_USAGE; > + > + ghcb_set_sw_exit_code(ghcb, exit_code); > + ghcb_set_sw_exit_info_1(ghcb, exit_info_1); > + ghcb_set_sw_exit_info_2(ghcb, exit_info_2); > + > + sev_es_wr_ghcb_msr(__pa(ghcb)); > + VMGEXIT(); > + > + if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) { > + u64 info = ghcb->save.sw_exit_info_2; > + unsigned long v; > + > + info = ghcb->save.sw_exit_info_2; This line seems redundant, since `info` is already initialized to this value when it's declared, two lines above. That being said, I see this is how the code is in Linux as well. I wonder if it was done like this on accident. > + v = info & SVM_EVTINJ_VEC_MASK; > + > + /* Check if exception information from hypervisor is sane. */ > + if ((info & SVM_EVTINJ_VALID) && > + ((v == GP_VECTOR) || (v == UD_VECTOR)) && > + ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { > + ctxt->fi.vector = v; > + if (info & SVM_EVTINJ_VALID_ERR) > + ctxt->fi.error_code = info >> 32; > + ret = ES_EXCEPTION; > + } else { > + ret = ES_VMM_ERROR; > + } > + } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) { > + ret = ES_VMM_ERROR; > + } else { > + ret = ES_OK; > + } > + > + return ret; > +} > + > +static enum es_result vc_handle_cpuid(struct ghcb *ghcb, > + struct es_em_ctxt *ctxt) > +{ > + struct ex_regs *regs = ctxt->regs; > + u32 cr4 = read_cr4(); > + enum es_result ret; > + > + ghcb_set_rax(ghcb, regs->rax); > + ghcb_set_rcx(ghcb, regs->rcx); > + > + if (cr4 & X86_CR4_OSXSAVE) { > + /* Safe to read xcr0 */ > + u64 xcr0; > + xgetbv_checking(XCR_XFEATURE_ENABLED_MASK, &xcr0); > + ghcb_set_xcr0(ghcb, xcr0); > + } else > + /* xgetbv will cause #GP - use reset value for xcr0 */ > + ghcb_set_xcr0(ghcb, 1); nit: Consider adding curly braces to the else branch, so that it matches the if branch. > + > + ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0); > + if (ret != ES_OK) > + return ret; > + > + if (!(ghcb_rax_is_valid(ghcb) && > + ghcb_rbx_is_valid(ghcb) && > + ghcb_rcx_is_valid(ghcb) && > + ghcb_rdx_is_valid(ghcb))) > + return ES_VMM_ERROR; > + > + regs->rax = ghcb->save.rax; > + regs->rbx = ghcb->save.rbx; > + regs->rcx = ghcb->save.rcx; > + regs->rdx = ghcb->save.rdx; > + > + return ES_OK; > +} > + > static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, > struct ghcb *ghcb, > unsigned long exit_code) > @@ -59,6 +154,9 @@ static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, > enum es_result result; > > switch (exit_code) { > + case SVM_EXIT_CPUID: > + result = vc_handle_cpuid(ghcb, ctxt); > + break; > default: > /* > * Unexpected #VC exception > -- > 2.32.0 >
On 2/12/22 10:32 PM, Marc Orr wrote: > On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@suse.com> wrote: >> >> Using Linux's CPUID #VC processing logic. >> >> Signed-off-by: Varad Gautam <varad.gautam@suse.com> >> --- >> lib/x86/amd_sev_vc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 98 insertions(+) >> >> diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c >> index 142f2cd..9ee67c0 100644 >> --- a/lib/x86/amd_sev_vc.c >> +++ b/lib/x86/amd_sev_vc.c >> @@ -2,6 +2,7 @@ >> >> #include "amd_sev.h" >> #include "svm.h" >> +#include "x86/xsave.h" >> >> extern phys_addr_t ghcb_addr; >> >> @@ -52,6 +53,100 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt) >> ctxt->regs->rip += ctxt->insn.length; >> } >> >> +static inline u64 lower_bits(u64 val, unsigned int bits) >> +{ >> + u64 mask = (1ULL << bits) - 1; >> + >> + return (val & mask); >> +} > > This isn't used in this patch. I guess it ends up being used later, in > path 9: "x86: AMD SEV-ES: Handle IOIO #VC". Let's introduce it there > if we're going to put it in this file. Though, again, maybe it's worth > creating a platform agnostic bit library, and put this and > `_test_bit()` (introduced in a previous patch) there. > Ack, it makes sense to introduce it later (and at a different place). >> + >> +static inline void sev_es_wr_ghcb_msr(u64 val) >> +{ >> + wrmsr(MSR_AMD64_SEV_ES_GHCB, val); >> +} >> + >> +static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, >> + struct es_em_ctxt *ctxt, >> + u64 exit_code, u64 exit_info_1, >> + u64 exit_info_2) >> +{ >> + enum es_result ret; >> + >> + /* Fill in protocol and format specifiers */ >> + ghcb->protocol_version = GHCB_PROTOCOL_MAX; >> + ghcb->ghcb_usage = GHCB_DEFAULT_USAGE; >> + >> + ghcb_set_sw_exit_code(ghcb, exit_code); >> + ghcb_set_sw_exit_info_1(ghcb, exit_info_1); >> + ghcb_set_sw_exit_info_2(ghcb, exit_info_2); >> + >> + sev_es_wr_ghcb_msr(__pa(ghcb)); >> + VMGEXIT(); >> + >> + if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) { >> + u64 info = ghcb->save.sw_exit_info_2; >> + unsigned long v; >> + >> + info = ghcb->save.sw_exit_info_2; > > This line seems redundant, since `info` is already initialized to this > value when it's declared, two lines above. That being said, I see this > is how the code is in Linux as well. I wonder if it was done like this > on accident. > Nice catch, it seems so. It's harmless, but I will drop it in v3. >> + v = info & SVM_EVTINJ_VEC_MASK; >> + >> + /* Check if exception information from hypervisor is sane. */ >> + if ((info & SVM_EVTINJ_VALID) && >> + ((v == GP_VECTOR) || (v == UD_VECTOR)) && >> + ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { >> + ctxt->fi.vector = v; >> + if (info & SVM_EVTINJ_VALID_ERR) >> + ctxt->fi.error_code = info >> 32; >> + ret = ES_EXCEPTION; >> + } else { >> + ret = ES_VMM_ERROR; >> + } >> + } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) { >> + ret = ES_VMM_ERROR; >> + } else { >> + ret = ES_OK; >> + } >> + >> + return ret; >> +} >> + >> +static enum es_result vc_handle_cpuid(struct ghcb *ghcb, >> + struct es_em_ctxt *ctxt) >> +{ >> + struct ex_regs *regs = ctxt->regs; >> + u32 cr4 = read_cr4(); >> + enum es_result ret; >> + >> + ghcb_set_rax(ghcb, regs->rax); >> + ghcb_set_rcx(ghcb, regs->rcx); >> + >> + if (cr4 & X86_CR4_OSXSAVE) { >> + /* Safe to read xcr0 */ >> + u64 xcr0; >> + xgetbv_checking(XCR_XFEATURE_ENABLED_MASK, &xcr0); >> + ghcb_set_xcr0(ghcb, xcr0); >> + } else >> + /* xgetbv will cause #GP - use reset value for xcr0 */ >> + ghcb_set_xcr0(ghcb, 1); > > nit: Consider adding curly braces to the else branch, so that it > matches the if branch. > Will do. >> + >> + ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0); >> + if (ret != ES_OK) >> + return ret; >> + >> + if (!(ghcb_rax_is_valid(ghcb) && >> + ghcb_rbx_is_valid(ghcb) && >> + ghcb_rcx_is_valid(ghcb) && >> + ghcb_rdx_is_valid(ghcb))) >> + return ES_VMM_ERROR; >> + >> + regs->rax = ghcb->save.rax; >> + regs->rbx = ghcb->save.rbx; >> + regs->rcx = ghcb->save.rcx; >> + regs->rdx = ghcb->save.rdx; >> + >> + return ES_OK; >> +} >> + >> static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, >> struct ghcb *ghcb, >> unsigned long exit_code) >> @@ -59,6 +154,9 @@ static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, >> enum es_result result; >> >> switch (exit_code) { >> + case SVM_EXIT_CPUID: >> + result = vc_handle_cpuid(ghcb, ctxt); >> + break; >> default: >> /* >> * Unexpected #VC exception >> -- >> 2.32.0 >> >
diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c index 142f2cd..9ee67c0 100644 --- a/lib/x86/amd_sev_vc.c +++ b/lib/x86/amd_sev_vc.c @@ -2,6 +2,7 @@ #include "amd_sev.h" #include "svm.h" +#include "x86/xsave.h" extern phys_addr_t ghcb_addr; @@ -52,6 +53,100 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt) ctxt->regs->rip += ctxt->insn.length; } +static inline u64 lower_bits(u64 val, unsigned int bits) +{ + u64 mask = (1ULL << bits) - 1; + + return (val & mask); +} + +static inline void sev_es_wr_ghcb_msr(u64 val) +{ + wrmsr(MSR_AMD64_SEV_ES_GHCB, val); +} + +static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, + struct es_em_ctxt *ctxt, + u64 exit_code, u64 exit_info_1, + u64 exit_info_2) +{ + enum es_result ret; + + /* Fill in protocol and format specifiers */ + ghcb->protocol_version = GHCB_PROTOCOL_MAX; + ghcb->ghcb_usage = GHCB_DEFAULT_USAGE; + + ghcb_set_sw_exit_code(ghcb, exit_code); + ghcb_set_sw_exit_info_1(ghcb, exit_info_1); + ghcb_set_sw_exit_info_2(ghcb, exit_info_2); + + sev_es_wr_ghcb_msr(__pa(ghcb)); + VMGEXIT(); + + if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) { + u64 info = ghcb->save.sw_exit_info_2; + unsigned long v; + + info = ghcb->save.sw_exit_info_2; + v = info & SVM_EVTINJ_VEC_MASK; + + /* Check if exception information from hypervisor is sane. */ + if ((info & SVM_EVTINJ_VALID) && + ((v == GP_VECTOR) || (v == UD_VECTOR)) && + ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { + ctxt->fi.vector = v; + if (info & SVM_EVTINJ_VALID_ERR) + ctxt->fi.error_code = info >> 32; + ret = ES_EXCEPTION; + } else { + ret = ES_VMM_ERROR; + } + } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) { + ret = ES_VMM_ERROR; + } else { + ret = ES_OK; + } + + return ret; +} + +static enum es_result vc_handle_cpuid(struct ghcb *ghcb, + struct es_em_ctxt *ctxt) +{ + struct ex_regs *regs = ctxt->regs; + u32 cr4 = read_cr4(); + enum es_result ret; + + ghcb_set_rax(ghcb, regs->rax); + ghcb_set_rcx(ghcb, regs->rcx); + + if (cr4 & X86_CR4_OSXSAVE) { + /* Safe to read xcr0 */ + u64 xcr0; + xgetbv_checking(XCR_XFEATURE_ENABLED_MASK, &xcr0); + ghcb_set_xcr0(ghcb, xcr0); + } else + /* xgetbv will cause #GP - use reset value for xcr0 */ + ghcb_set_xcr0(ghcb, 1); + + ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0); + if (ret != ES_OK) + return ret; + + if (!(ghcb_rax_is_valid(ghcb) && + ghcb_rbx_is_valid(ghcb) && + ghcb_rcx_is_valid(ghcb) && + ghcb_rdx_is_valid(ghcb))) + return ES_VMM_ERROR; + + regs->rax = ghcb->save.rax; + regs->rbx = ghcb->save.rbx; + regs->rcx = ghcb->save.rcx; + regs->rdx = ghcb->save.rdx; + + return ES_OK; +} + static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, struct ghcb *ghcb, unsigned long exit_code) @@ -59,6 +154,9 @@ static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, enum es_result result; switch (exit_code) { + case SVM_EXIT_CPUID: + result = vc_handle_cpuid(ghcb, ctxt); + break; default: /* * Unexpected #VC exception
Using Linux's CPUID #VC processing logic. Signed-off-by: Varad Gautam <varad.gautam@suse.com> --- lib/x86/amd_sev_vc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+)