[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/cpuid: Fix handling of the CPUID.7[0].eax levelling MSR


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 10 Sep 2019 11:29:12 +0200
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=roger.pau@xxxxxxxxxx; spf=Pass smtp.mailfrom=roger.pau@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>
  • Delivery-date: Tue, 10 Sep 2019 09:29:43 +0000
  • Ironport-sdr: emy+h7wpiPHzDEKNsnFGivM84z4eka/3xW3RKcF2EL+SABH1EqczLGoJK05oA9zL3ilzJyhWiv i1MuapvknCyDnlhPRgnOB6+mSqec9D+d71Jc56/6DoN9m3srnCfyZ9M6nXo/ErGBErVexiutux XVruPf7WFXLM6LHDsmifcWiFy6lWPVap5/4ZX5PCObitB3lax8yPC/IVVkZMnOYkUyWY9lltTT qCD+aYQZEL5EwXiIi1A4ILaXnwRUcsDgpLjc3fM65GMFGe0ZiXFhb/gqMLVgFYsGseGWYpaB9S OSM=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Sep 09, 2019 at 04:51:24PM +0100, Andrew Cooper wrote:
> 7a0 is an integer field, not a mask - taking the logical and of the hardware
> and policy values results in nonsense.  Instead, take the policy value
> directly.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Even Rome hardware has 7[0].eax still as 0, and there is no sensible reason to
> set max_subleaf higher at this point, so this is only a latent bug for now.
> ---
>  xen/arch/x86/domctl.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 1e98fc8009..35ad8cb51c 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -218,11 +218,16 @@ static int update_domain_cpuid_info(struct domain *d,
>          if ( is_pv_domain(d) && ((levelling_caps & LCAP_7ab0) == LCAP_7ab0) )
>          {
>              uint64_t mask = cpuidmask_defaults._7ab0;
> -            uint32_t eax = ctl->eax;
> -            uint32_t ebx = p->feat._7b0;
>  
> -            if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | 
> X86_VENDOR_HYGON) )
> -                mask &= ((uint64_t)eax << 32) | ebx;
> +            /*
> +             * Leaf 7[0].eax is max_subleaf, not a feature mask.  Take it
> +             * wholesale from the policy, but clamp the features in 7[0].ebx
> +             * per usual.
> +             */
> +            if ( boot_cpu_data.x86_vendor &
> +                 (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
> +                mask = (((uint64_t)p->feat.max_subleaf << 32) |
> +                        ((uint32_t)mask | p->feat._7b0));

Why do you set the high bits of the mask (63:30) with the max subleaf?
According to the document I have bits 63:30 are reserved, and that
seems to match the expected CPUID return value, that lists CPUID
Fn0000_0007_EAX_x0 content as reserved.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.