Subject: x86/pv: Improve pv_cpuid()'s API
From: Andrew Cooper <[email-masked]>

pv_cpuid()'s API is awkward to use.  There are already two callers jumping
through hoops to use it, and a third is on its way.

Change the API to take each parameter individually (like its counterpart,
hvm_cpuid(), already does), and introduce a new pv_cpuid_regs() wrapper
implementing the old API.

No functional change.

Signed-off-by: Andrew Cooper <[email-masked]>
Reviewed-by: Jan Beulich <[email-masked]>

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index ab39a45..0053ac0 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3803,7 +3803,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
 
         if ( is_pvh_vcpu(v) )
         {
-            pv_cpuid(regs);
+            pv_cpuid_regs(regs);
             rc = 0;
         }
         else
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 1a22895..77f786c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -972,17 +972,14 @@ static void _domain_cpuid(const struct domain *currd,
         cpuid_count(leaf, subleaf, eax, ebx, ecx, edx);
 }
 
-void pv_cpuid(struct cpu_user_regs *regs)
+void pv_cpuid(uint32_t leaf, uint32_t subleaf,
+              uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
-    uint32_t leaf, subleaf, a, b, c, d;
+    uint32_t a, b, c, d;
+    const struct cpu_user_regs *regs = guest_cpu_user_regs();
     struct vcpu *curr = current;
     struct domain *currd = curr->domain;
 
-    leaf = a = regs->eax;
-    b = regs->ebx;
-    subleaf = c = regs->ecx;
-    d = regs->edx;
-
     if ( cpuid_hypervisor_leaves(leaf, subleaf, &a, &b, &c, &d) )
         goto out;
 
@@ -997,13 +994,7 @@ void pv_cpuid(struct cpu_user_regs *regs)
 
         _domain_cpuid(currd, limit, 0, &limit, &dummy, &dummy, &dummy);
         if ( leaf > limit )
-        {
-            regs->eax = 0;
-            regs->ebx = 0;
-            regs->ecx = 0;
-            regs->edx = 0;
-            return;
-        }
+            goto unsupported;
     }
 
     _domain_cpuid(currd, leaf, subleaf, &a, &b, &c, &d);
@@ -1284,17 +1275,21 @@ void pv_cpuid(struct cpu_user_regs *regs)
     case 0x8000001e: /* Extended topology reporting */
     unsupported:
         a = b = c = d = 0;
-        break;
+        goto out;
     }
 
- out:
     /* VPMU may decide to modify some of the leaves */
     vpmu_do_cpuid(leaf, &a, &b, &c, &d);
 
-    regs->eax = a;
-    regs->ebx = b;
-    regs->ecx = c;
-    regs->edx = d;
+ out:
+    if ( eax )
+        *eax = a;
+    if ( ebx )
+        *ebx = b;
+    if ( ecx )
+        *ecx = c;
+    if ( edx )
+        *edx = d;
 }
 
 static int emulate_invalid_rdtscp(struct cpu_user_regs *regs)
@@ -1353,7 +1348,7 @@ static int emulate_forced_invalid_op(struct cpu_user_regs *regs)
 
     eip += sizeof(instr);
 
-    pv_cpuid(regs);
+    pv_cpuid_regs(regs);
 
     instruction_done(regs, eip, 0);
 
@@ -2828,17 +2823,7 @@ static int priv_op_write_msr(unsigned int reg, uint64_t val,
 int pv_emul_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
                   unsigned int *edx, struct x86_emulate_ctxt *ctxt)
 {
-    struct cpu_user_regs regs = *ctxt->regs;
-
-    regs._eax = *eax;
-    regs._ecx = *ecx;
-
-    pv_cpuid(&regs);
-
-    *eax = regs._eax;
-    *ebx = regs._ebx;
-    *ecx = regs._ecx;
-    *edx = regs._edx;
+    pv_cpuid(*eax, *ecx, eax, ebx, ecx, edx);
 
     return X86EMUL_OKAY;
 }
@@ -3329,7 +3314,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
         if ( v->arch.cpuid_faulting && !guest_kernel_mode(v, regs) )
             goto fail;
 
-        pv_cpuid(regs);
+        pv_cpuid_regs(regs);
         break;
 
     default:
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 5c43ec9..09d25ac 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -654,24 +654,18 @@ static bool_t valid_xcr0(u64 xcr0)
 
 static uint64_t guest_xcr0_max(const struct domain *d)
 {
+    uint32_t eax, edx;
+
     if ( has_hvm_container_domain(d) )
     {
-        uint32_t eax, ecx = 0, edx;
+        uint32_t ecx = 0;
 
         hvm_cpuid(XSTATE_CPUID, &eax, NULL, &ecx, &edx);
-
-        return ((uint64_t)edx << 32) | eax;
     }
     else
-    {
-        struct cpu_user_regs regs = { };
+        pv_cpuid(XSTATE_CPUID, 0, &eax, NULL, NULL, &edx);
 
-        regs._eax = XSTATE_CPUID;
-        regs._ecx = 0;
-        pv_cpuid(&regs);
-
-        return (regs.rdx << 32) | regs._eax;
-    }
+    return ((uint64_t)edx << 32) | eax;
 }
 
 int validate_xstate(const struct domain *d, uint64_t xcr0, uint64_t xcr0_accum,
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 581d7b0..50badab 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -663,7 +663,14 @@ enum get_cpu_vendor {
 };
 
 int get_cpu_vendor(const char vendor_id[], enum get_cpu_vendor);
-void pv_cpuid(struct cpu_user_regs *regs);
+void pv_cpuid(uint32_t leaf, uint32_t subleaf,
+              uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+
+static inline void pv_cpuid_regs(struct cpu_user_regs *regs)
+{
+    pv_cpuid(regs->_eax, regs->_ecx,
+             &regs->_eax, &regs->_ebx, &regs->_ecx, &regs->_edx);
+}
 
 #endif /* !__ASSEMBLY__ */