[v14,1/6] i386: Set TOPOEXT unconditionally for comapatibility

Message ID 1528939107-17193-2-git-send-email-babu.moger@amd.com
State New
Headers show
Series
  • i386: Enable TOPOEXT to support hyperthreading on AMD CPU
Related show

Commit Message

Babu Moger June 14, 2018, 1:18 a.m.
Enabling TOPOEXT feature might cause compatibility issues if
older kernels does not set this feature. Lets set this feature
unconditionally.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 target/i386/kvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eduardo Habkost June 14, 2018, 2:21 a.m. | #1
On Wed, Jun 13, 2018 at 09:18:22PM -0400, Babu Moger wrote:
> Enabling TOPOEXT feature might cause compatibility issues if
> older kernels does not set this feature. Lets set this feature
> unconditionally.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  target/i386/kvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 445e0e0..6f2cca7 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -372,6 +372,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>          if (host_tsx_blacklisted()) {
>              ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
>          }
> +    } else if (function == 0x80000001 && reg == R_ECX) {
> +        /* Enabling topoext feature might cause compatibility issues if
> +         * older kernel does not set this feature. Lets set this feature
> +         * unconditionally.
> +         */

Thanks.  I will apply and rewrite the comment as:

        /*
         * It's safe to enable TOPOEXT even if it's not returned
         * by GET_SUPPORTED_CPUID.  Unconditionally enabling
         * TOPOEXT here let us keep CPU models runnable on
         * older kernels even when TOPOEXT is enabled.
         */

> +        ret |= CPUID_EXT3_TOPOEXT;
>      } else if (function == 0x80000001 && reg == R_EDX) {
>          /* On Intel, kvm returns cpuid according to the Intel spec,
>           * so add missing bits according to the AMD spec:
> -- 
> 1.8.3.1
>
Babu Moger June 14, 2018, 1:24 p.m. | #2
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Wednesday, June 13, 2018 9:22 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com;
> rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com
> Subject: Re: [PATCH v14 1/6] i386: Set TOPOEXT unconditionally for
> comapatibility
> 
> On Wed, Jun 13, 2018 at 09:18:22PM -0400, Babu Moger wrote:
> > Enabling TOPOEXT feature might cause compatibility issues if
> > older kernels does not set this feature. Lets set this feature
> > unconditionally.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  target/i386/kvm.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 445e0e0..6f2cca7 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -372,6 +372,12 @@ uint32_t
> kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
> >          if (host_tsx_blacklisted()) {
> >              ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
> >          }
> > +    } else if (function == 0x80000001 && reg == R_ECX) {
> > +        /* Enabling topoext feature might cause compatibility issues if
> > +         * older kernel does not set this feature. Lets set this feature
> > +         * unconditionally.
> > +         */
> 
> Thanks.  I will apply and rewrite the comment as:
Sure. Thanks
> 
>         /*
>          * It's safe to enable TOPOEXT even if it's not returned
>          * by GET_SUPPORTED_CPUID.  Unconditionally enabling
>          * TOPOEXT here let us keep CPU models runnable on
>          * older kernels even when TOPOEXT is enabled.
>          */
> 
> > +        ret |= CPUID_EXT3_TOPOEXT;
> >      } else if (function == 0x80000001 && reg == R_EDX) {
> >          /* On Intel, kvm returns cpuid according to the Intel spec,
> >           * so add missing bits according to the AMD spec:
> > --
> > 1.8.3.1
> >
> 
> --
> Eduardo

Patch

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 445e0e0..6f2cca7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -372,6 +372,12 @@  uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
         if (host_tsx_blacklisted()) {
             ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
         }
+    } else if (function == 0x80000001 && reg == R_ECX) {
+        /* Enabling topoext feature might cause compatibility issues if
+         * older kernel does not set this feature. Lets set this feature
+         * unconditionally.
+         */
+        ret |= CPUID_EXT3_TOPOEXT;
     } else if (function == 0x80000001 && reg == R_EDX) {
         /* On Intel, kvm returns cpuid according to the Intel spec,
          * so add missing bits according to the AMD spec: