On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@xxxxxxxx> wrote:
>
> Using Linux's CPUID #VC processing logic.
>
> Signed-off-by: Varad Gautam <varad.gautam@xxxxxxxx>
> ---
> 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
>
