| Message ID | 20211210154332.11526-23-brijesh.singh@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
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; > +} > +
On Thu, Dec 30, 2021 at 06:52:52PM +0000, Sean Christopherson wrote: > 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; Looks good, will make these changes. -Mike
On Thu, Jan 06, 2022 at 12:38:37PM -0600, Venu Busireddy wrote: > On 2021-12-10 09:43:14 -0600, 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, > > + 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); > > + } > > + > > + return 0; > > +} > > + > > /* > > * Boot VC Handler - This is the first VC handler during boot, there is no GHCB > > * page yet, so it only supports the MSR based communication with the > > @@ -202,39 +254,19 @@ enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, bool set_ghcb_msr, > > void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code) > > { > > unsigned int fn = lower_bits(regs->ax, 32); > > - unsigned long val; > > + u32 eax, ebx, ecx, edx; > > > > /* Only CPUID is supported via MSR protocol */ > > if (exit_code != SVM_EXIT_CPUID) > > goto fail; > > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EAX)); > > - VMGEXIT(); > > - val = sev_es_rd_ghcb_msr(); > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > + if (sev_cpuid_hv(fn, 0, &eax, &ebx, &ecx, &edx)) > > goto fail; > > - regs->ax = val >> 32; > > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EBX)); > > - VMGEXIT(); > > - val = sev_es_rd_ghcb_msr(); > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > - goto fail; > > - regs->bx = val >> 32; > > - > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_ECX)); > > - VMGEXIT(); > > - val = sev_es_rd_ghcb_msr(); > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > - goto fail; > > - regs->cx = val >> 32; > > - > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EDX)); > > - VMGEXIT(); > > - val = sev_es_rd_ghcb_msr(); > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > - goto fail; > > - regs->dx = val >> 32; > > + regs->ax = eax; > > + regs->bx = ebx; > > + regs->cx = ecx; > > + regs->dx = edx; > > What is the intent behind declaring e?x as local variables, instead > of passing the addresses of regs->?x to sev_cpuid_hv()? Is it to > prevent touching any of the regs->?x unless there is no error from > sev_cpuid_hv()? If so, wouldn't it be better to hide this logic from > the callers by declaring the local variables in sev_cpuid_hv() itself, > and moving the four "*e?x = (val >> 32);" statements there to the end > of the function (just before last the return)? With that change, the > callers can safely pass the addresses of regs->?x to do_vc_no_ghcb(), > knowing that the values will only be touched if there is no error? For me it was more about readability. E?X are well-defined as 32-bit values, whereas regs->?x are longs. It seemed more readable to me to have sev_cpuid_hv()/snp_cpuid() expect/return the actual native types, and leave it up to the caller to cast/shift if necessary. It also seems more robust for future re-use, since, for instance, if we ever introduced another callsite that happened to already use u32 locally, it seems like it would be a mess trying to setup up temp long* args or do casts to pass them into these functions and then shift/cast them back just so we could save a few lines at this particular callsite. > > Venu
On 2022-01-06 14:21:35 -0600, Michael Roth wrote: > On Thu, Jan 06, 2022 at 12:38:37PM -0600, Venu Busireddy wrote: > > On 2021-12-10 09:43:14 -0600, 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> Reviewed-by: Venu Busireddy <venu.busireddy@oracle.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, > > > + 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); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > /* > > > * Boot VC Handler - This is the first VC handler during boot, there is no GHCB > > > * page yet, so it only supports the MSR based communication with the > > > @@ -202,39 +254,19 @@ enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, bool set_ghcb_msr, > > > void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code) > > > { > > > unsigned int fn = lower_bits(regs->ax, 32); > > > - unsigned long val; > > > + u32 eax, ebx, ecx, edx; > > > > > > /* Only CPUID is supported via MSR protocol */ > > > if (exit_code != SVM_EXIT_CPUID) > > > goto fail; > > > > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EAX)); > > > - VMGEXIT(); > > > - val = sev_es_rd_ghcb_msr(); > > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > > + if (sev_cpuid_hv(fn, 0, &eax, &ebx, &ecx, &edx)) > > > goto fail; > > > - regs->ax = val >> 32; > > > > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EBX)); > > > - VMGEXIT(); > > > - val = sev_es_rd_ghcb_msr(); > > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > > - goto fail; > > > - regs->bx = val >> 32; > > > - > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_ECX)); > > > - VMGEXIT(); > > > - val = sev_es_rd_ghcb_msr(); > > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > > - goto fail; > > > - regs->cx = val >> 32; > > > - > > > - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EDX)); > > > - VMGEXIT(); > > > - val = sev_es_rd_ghcb_msr(); > > > - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) > > > - goto fail; > > > - regs->dx = val >> 32; > > > + regs->ax = eax; > > > + regs->bx = ebx; > > > + regs->cx = ecx; > > > + regs->dx = edx; > > > > What is the intent behind declaring e?x as local variables, instead > > of passing the addresses of regs->?x to sev_cpuid_hv()? Is it to > > prevent touching any of the regs->?x unless there is no error from > > sev_cpuid_hv()? If so, wouldn't it be better to hide this logic from > > the callers by declaring the local variables in sev_cpuid_hv() itself, > > and moving the four "*e?x = (val >> 32);" statements there to the end > > of the function (just before last the return)? With that change, the > > callers can safely pass the addresses of regs->?x to do_vc_no_ghcb(), > > knowing that the values will only be touched if there is no error? > > For me it was more about readability. E?X are well-defined as 32-bit > values, whereas regs->?x are longs. It seemed more readable to me to > have sev_cpuid_hv()/snp_cpuid() expect/return the actual native types, > and leave it up to the caller to cast/shift if necessary. > > It also seems more robust for future re-use, since, for instance, if we > ever introduced another callsite that happened to already use u32 locally, > it seems like it would be a mess trying to setup up temp long* args or do > casts to pass them into these functions and then shift/cast them back just > so we could save a few lines at this particular callsite. Got it. Venu
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, + 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); + } + + return 0; +} + /* * Boot VC Handler - This is the first VC handler during boot, there is no GHCB * page yet, so it only supports the MSR based communication with the @@ -202,39 +254,19 @@ enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, bool set_ghcb_msr, void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code) { unsigned int fn = lower_bits(regs->ax, 32); - unsigned long val; + u32 eax, ebx, ecx, edx; /* Only CPUID is supported via MSR protocol */ if (exit_code != SVM_EXIT_CPUID) goto fail; - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EAX)); - VMGEXIT(); - val = sev_es_rd_ghcb_msr(); - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) + if (sev_cpuid_hv(fn, 0, &eax, &ebx, &ecx, &edx)) goto fail; - regs->ax = val >> 32; - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EBX)); - VMGEXIT(); - val = sev_es_rd_ghcb_msr(); - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) - goto fail; - regs->bx = val >> 32; - - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_ECX)); - VMGEXIT(); - val = sev_es_rd_ghcb_msr(); - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) - goto fail; - regs->cx = val >> 32; - - sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EDX)); - VMGEXIT(); - val = sev_es_rd_ghcb_msr(); - if (GHCB_RESP_CODE(val) != GHCB_MSR_CPUID_RESP) - goto fail; - regs->dx = val >> 32; + regs->ax = eax; + regs->bx = ebx; + regs->cx = ecx; + regs->dx = edx; /* * This is a VC handler and the #VC is only raised when SEV-ES is