lkml.org 
[lkml]   [2021]   [Dec]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v8 22/40] x86/sev: move MSR-based VMGEXITs for CPUID to helper
    On Fri, Dec 10, 2021, Brijesh Singh wrote:
    > From: Michael Roth <michael.roth@amd.com>
    >
    > This code will also be used later for SEV-SNP-validated CPUID code in
    > some cases, so move it to a common helper.
    >
    > Signed-off-by: Michael Roth <michael.roth@amd.com>
    > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
    > ---
    > arch/x86/kernel/sev-shared.c | 84 +++++++++++++++++++++++++-----------
    > 1 file changed, 58 insertions(+), 26 deletions(-)
    >
    > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
    > index 3aaef1a18ffe..d89481b31022 100644
    > --- a/arch/x86/kernel/sev-shared.c
    > +++ b/arch/x86/kernel/sev-shared.c
    > @@ -194,6 +194,58 @@ enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, bool set_ghcb_msr,
    > return verify_exception_info(ghcb, ctxt);
    > }
    >
    > +static int sev_cpuid_hv(u32 func, u32 subfunc, u32 *eax, u32 *ebx,

    Having @subfunc, a.k.a. index, in is weird/confusing/fragile because it's not consumed,
    nor is it checked. Peeking ahead, it looks like all future users pass '0'. Taking the
    index but dropping it on the floor is asking for future breakage. Either drop it or
    assert that it's zero.

    > + u32 *ecx, u32 *edx)
    > +{
    > + u64 val;
    > +
    > + if (eax) {
    > + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(func, GHCB_CPUID_REQ_EAX));
    > + VMGEXIT();
    > + val = sev_es_rd_ghcb_msr();
    > +
    > + if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP)
    > + return -EIO;
    > +
    > + *eax = (val >> 32);
    > + }
    > +
    > + if (ebx) {
    > + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(func, GHCB_CPUID_REQ_EBX));
    > + VMGEXIT();
    > + val = sev_es_rd_ghcb_msr();
    > +
    > + if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP)
    > + return -EIO;
    > +
    > + *ebx = (val >> 32);
    > + }
    > +
    > + if (ecx) {
    > + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(func, GHCB_CPUID_REQ_ECX));
    > + VMGEXIT();
    > + val = sev_es_rd_ghcb_msr();
    > +
    > + if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP)
    > + return -EIO;
    > +
    > + *ecx = (val >> 32);
    > + }
    > +
    > + if (edx) {
    > + sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(func, GHCB_CPUID_REQ_EDX));
    > + VMGEXIT();
    > + val = sev_es_rd_ghcb_msr();
    > +
    > + if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP)
    > + return -EIO;
    > +
    > + *edx = (val >> 32);
    > + }

    That's a lot of pasta! If you add

    static int __sev_cpuid_hv(u32 func, int reg_idx, u32 *reg)
    {
    u64 val;

    if (!reg)
    return 0;

    sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(func, reg_idx));
    VMGEXIT();
    val = sev_es_rd_ghcb_msr();
    if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP)
    return -EIO;

    *reg = (val >> 32);
    return 0;
    }

    then this helper can become something like:

    static int sev_cpuid_hv(u32 func, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
    {
    int ret;

    ret = __sev_cpuid_hv(func, GHCB_CPUID_REQ_EAX, eax);
    ret = ret ? : __sev_cpuid_hv(func, GHCB_CPUID_REQ_EBX, ebx);
    ret = ret ? : __sev_cpuid_hv(func, GHCB_CPUID_REQ_ECX, ecx);
    ret = ret ? : __sev_cpuid_hv(func, GHCB_CPUID_REQ_EDX, edx);

    return ret;
    }

    > +
    > + return 0;
    > +}
    > +

    \
     
     \ /
      Last update: 2021-12-30 19:53    [W:2.131 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site