Re: [Xen-devel] [PATCH v2 06/25] x86/hvm: Dispatch cpuid_viridian_leaves() from guest_cpuid()

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 09 January 2017 11:03
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: [PATCH v2 06/25] x86/hvm: Dispatch cpuid_viridian_leaves() from
> guest_cpuid()
> 
> ... rather than from the legacy path.  Update the API to match guest_cpuid(),
> and remove its dependence on current.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> v2:
>  * Introduce a break statement into guest_cpuid().
>  * Retain the explicit zeroing of SP and Service branch.
>  * Drop the EFER/SVM adjustment and leave the code as it was.
>  * Consistently use uint32_t.
> ---
>  xen/arch/x86/cpuid.c               | 12 +++++++
>  xen/arch/x86/hvm/hvm.c             |  3 --
>  xen/arch/x86/hvm/viridian.c        | 65 +++++++++++++++++++-----------------
> --
>  xen/include/asm-x86/hvm/viridian.h |  9 ++----
>  4 files changed, 46 insertions(+), 43 deletions(-)
> 
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index 0d54465..69a4f1b 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -337,6 +337,18 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
> 
>      *res = EMPTY_LEAF;
> 
> +    /*
> +     * First pass:
> +     * - Dispatch the virtualised leaves to their respective handlers.
> +     */
> +    switch ( leaf )
> +    {
> +    case 0x40000000 ... 0x400000ff:
> +        if ( is_viridian_domain(d) )
> +            return cpuid_viridian_leaves(v, leaf, subleaf, res);
> +        break;
> +    }
> +
>      /* {hvm,pv}_cpuid() have this expectation. */
>      ASSERT(v == curr);
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 4c0f561..9ff5f92 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3353,9 +3353,6 @@ void hvm_cpuid(unsigned int input, unsigned int
> *eax, unsigned int *ebx,
>      if ( !edx )
>          edx = &dummy;
> 
> -    if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )
> -        return;
> -
>      if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
>          return;
> 
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index f6abdd2..37c8f68 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -66,77 +66,76 @@
>  #define CPUID6A_MSR_BITMAPS     (1 << 1)
>  #define CPUID6A_NESTED_PAGING   (1 << 3)
> 
> -int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
> -                          unsigned int *ebx, unsigned int *ecx,
> -                          unsigned int *edx)
> +void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
> +                           uint32_t subleaf, struct cpuid_leaf *res)
>  {
> -    struct domain *d = current->domain;
> +    const struct domain *d = v->domain;
> 
> -    if ( !is_viridian_domain(d) )
> -        return 0;
> +    ASSERT(is_viridian_domain(d));
> +    ASSERT(leaf >= 0x40000000 && leaf < 0x40000100);
> 
>      leaf -= 0x40000000;
> -    if ( leaf > 6 )
> -        return 0;
> 
> -    *eax = *ebx = *ecx = *edx = 0;
>      switch ( leaf )
>      {
>      case 0:
> -        *eax = 0x40000006; /* Maximum leaf */
> -        *ebx = 0x7263694d; /* Magic numbers  */
> -        *ecx = 0x666F736F;
> -        *edx = 0x76482074;
> +        res->a = 0x40000006; /* Maximum leaf */
> +        res->b = 0x7263694d; /* Magic numbers  */
> +        res->c = 0x666F736F;
> +        res->d = 0x76482074;
>          break;
> +
>      case 1:
> -        *eax = 0x31237648; /* Version number */
> +        res->a = 0x31237648; /* Version number */
>          break;
> +
>      case 2:
>          /* Hypervisor information, but only if the guest has set its
>             own version number. */
>          if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
>              break;
> -        *eax = 1; /* Build number */
> -        *ebx = (xen_major_version() << 16) | xen_minor_version();
> -        *ecx = 0; /* SP */
> -        *edx = 0; /* Service branch and number */
> +        res->a = 1; /* Build number */
> +        res->b = (xen_major_version() << 16) | xen_minor_version();
> +        res->c = 0; /* SP */
> +        res->d = 0; /* Service branch and number */
>          break;
> +
>      case 3:
>          /* Which hypervisor MSRs are available to the guest */
> -        *eax = (CPUID3A_MSR_APIC_ACCESS |
> -                CPUID3A_MSR_HYPERCALL   |
> -                CPUID3A_MSR_VP_INDEX);
> +        res->a = (CPUID3A_MSR_APIC_ACCESS |
> +                  CPUID3A_MSR_HYPERCALL   |
> +                  CPUID3A_MSR_VP_INDEX);
>          if ( !(viridian_feature_mask(d) & HVMPV_no_freq) )
> -            *eax |= CPUID3A_MSR_FREQ;
> +            res->a |= CPUID3A_MSR_FREQ;
>          if ( viridian_feature_mask(d) & HVMPV_time_ref_count )
> -            *eax |= CPUID3A_MSR_TIME_REF_COUNT;
> +            res->a |= CPUID3A_MSR_TIME_REF_COUNT;
>          if ( viridian_feature_mask(d) & HVMPV_reference_tsc )
> -            *eax |= CPUID3A_MSR_REFERENCE_TSC;
> +            res->a |= CPUID3A_MSR_REFERENCE_TSC;
>          break;
> +
>      case 4:
>          /* Recommended hypercall usage. */
>          if ( (d->arch.hvm_domain.viridian.guest_os_id.raw == 0) ||
>               (d->arch.hvm_domain.viridian.guest_os_id.fields.os < 4) )
>              break;
> -        *eax = CPUID4A_RELAX_TIMER_INT;
> +        res->a = CPUID4A_RELAX_TIMER_INT;
>          if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush )
> -            *eax |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
> +            res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
>          if ( !cpu_has_vmx_apic_reg_virt )
> -            *eax |= CPUID4A_MSR_BASED_APIC;
> -        *ebx = 2047; /* long spin count */
> +            res->a |= CPUID4A_MSR_BASED_APIC;
> +        res->b = 2047; /* long spin count */
>          break;
> +
>      case 6:
>          /* Detected and in use hardware features. */
>          if ( cpu_has_vmx_virtualize_apic_accesses )
> -            *eax |= CPUID6A_APIC_OVERLAY;
> +            res->a |= CPUID6A_APIC_OVERLAY;
>          if ( cpu_has_vmx_msr_bitmap || (read_efer() & EFER_SVME) )
> -            *eax |= CPUID6A_MSR_BITMAPS;
> +            res->a |= CPUID6A_MSR_BITMAPS;
>          if ( hap_enabled(d) )
> -            *eax |= CPUID6A_NESTED_PAGING;
> +            res->a |= CPUID6A_NESTED_PAGING;
>          break;
>      }
> -
> -    return 1;
>  }
> 
>  static void dump_guest_os_id(const struct domain *d)
> diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-
> x86/hvm/viridian.h
> index bdbccd5..8c45d0f 100644
> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -97,13 +97,8 @@ struct viridian_domain
>      union viridian_reference_tsc reference_tsc;
>  };
> 
> -int
> -cpuid_viridian_leaves(
> -    unsigned int leaf,
> -    unsigned int *eax,
> -    unsigned int *ebx,
> -    unsigned int *ecx,
> -    unsigned int *edx);
> +void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
> +                           uint32_t subleaf, struct cpuid_leaf *res);
> 
>  int
>  wrmsr_viridian_regs(
> --
> 2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel