1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v7 0/9] i386: Enable TOPOEXT to support hyperthreading on AMD CPU
  4. View patch

[Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info

Babu Moger posted 9 patches 7 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Babu Moger 7 years, 7 months ago 
This will be used to control the cache information.
By default new information will be displayed. If user
passes "-cpu legacy-cache" then older information will
be displayed even if the hardware supports new information.
It will be "on" for machine type "pc-q35-2.10" for compatibility.

Signed-off-by: Babu Moger <babu.moger@amd.com>
Tested-by: Geoffrey McRae <geoff@hostfission.com>
---
 include/hw/i386/pc.h | 4 ++++
 target/i386/cpu.c    | 1 +
 target/i386/cpu.h    | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ffee841..d904a3c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "q35-pcihost",\
         .property = "x-pci-hole64-fix",\
         .value    = "off",\
+    },{\
+        .driver   = TYPE_X86_CPU,\
+        .property = "legacy-cache",\
+        .value    = "on",\
     },
 
 #define PC_COMPAT_2_9 \
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5d88363..a27b658 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5138,6 +5138,7 @@ static Property x86_cpu_properties[] = {
                      false),
     DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
     DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
+    DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),
 
     /*
      * From "Requirements for Implementing the Microsoft
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1213bb7..852586a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1395,6 +1395,11 @@ struct X86CPU {
      */
     bool enable_l3_cache;
 
+    /* Compatibility bits for old machine types.
+     * If true present the old cache topology information
+     */
+    bool legacy_cache;
+
     /* Compatibility bits for old machine types: */
     bool enable_cpuid_0xb;
 
-- 
2.7.4
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Eduardo Habkost 7 years, 7 months ago 
On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> This will be used to control the cache information.
> By default new information will be displayed. If user
> passes "-cpu legacy-cache" then older information will
> be displayed even if the hardware supports new information.
> It will be "on" for machine type "pc-q35-2.10" for compatibility.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> ---
>  include/hw/i386/pc.h | 4 ++++
>  target/i386/cpu.c    | 1 +
>  target/i386/cpu.h    | 5 +++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ffee841..d904a3c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>          .driver   = "q35-pcihost",\
>          .property = "x-pci-hole64-fix",\
>          .value    = "off",\
> +    },{\
> +        .driver   = TYPE_X86_CPU,\
> +        .property = "legacy-cache",\
> +        .value    = "on",\
>      },
>  

Looks good, except that we now need pc-*-2.13 machines, and this
should be moved to PC_COMPAT_2_12.

Also, I suggest squashing this with patch 5/9, and applying it
before patch 3/9.

-- 
Eduardo
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Moger, Babu 7 years, 7 months ago 

> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Monday, May 7, 2018 2:14 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel@redhat.com; pbonzini@redhat.com;
> rth@twiddle.net; mtosatti@redhat.com; geoff@hostfission.com;
> kash@tripleback.net; qemu-devel@nongnu.org; kvm@vger.kernel.org
> Subject: Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control
> cache info
> 
> On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> > This will be used to control the cache information.
> > By default new information will be displayed. If user
> > passes "-cpu legacy-cache" then older information will
> > be displayed even if the hardware supports new information.
> > It will be "on" for machine type "pc-q35-2.10" for compatibility.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > ---
> >  include/hw/i386/pc.h | 4 ++++
> >  target/i386/cpu.c    | 1 +
> >  target/i386/cpu.h    | 5 +++++
> >  3 files changed, 10 insertions(+)
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index ffee841..d904a3c 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> >          .driver   = "q35-pcihost",\
> >          .property = "x-pci-hole64-fix",\
> >          .value    = "off",\
> > +    },{\
> > +        .driver   = TYPE_X86_CPU,\
> > +        .property = "legacy-cache",\
> > +        .value    = "on",\
> >      },
> >
> 
> Looks good, except that we now need pc-*-2.13 machines, and this
> should be moved to PC_COMPAT_2_12.

Ok. Sure. There is no PC_COMPAT_2_12 definition now.  I will add it and include this new property.

> 
> Also, I suggest squashing this with patch 5/9, and applying it
> before patch 3/9.

Ok. Sure.

> 
> --
> Eduardo
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Eduardo Habkost 7 years, 7 months ago 
On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> This will be used to control the cache information.
> By default new information will be displayed. If user
> passes "-cpu legacy-cache" then older information will
> be displayed even if the hardware supports new information.
> It will be "on" for machine type "pc-q35-2.10" for compatibility.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>
> ---
>  include/hw/i386/pc.h | 4 ++++
>  target/i386/cpu.c    | 1 +
>  target/i386/cpu.h    | 5 +++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ffee841..d904a3c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>          .driver   = "q35-pcihost",\
>          .property = "x-pci-hole64-fix",\
>          .value    = "off",\
> +    },{\
> +        .driver   = TYPE_X86_CPU,\
> +        .property = "legacy-cache",\
> +        .value    = "on",\
>      },

>  
>  #define PC_COMPAT_2_9 \
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 5d88363..a27b658 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5138,6 +5138,7 @@ static Property x86_cpu_properties[] = {
>                       false),
>      DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
>      DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> +    DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),


Hmm, this can get messy if we start adding cache info to other
CPU models in future QEMU versions.  e.g.: what if we add cache
info to Opteron_G3 on QEMU 2.14?

I suggest adding this to x86_cpu_load_def():

  cpu->legacy_cache = !cpu->cache_info.valid;

(Or equivalent code, in case cache_info is changed to be a
pointer)

This way, only EPYC will have legacy-cache=false by now, making
it easier to write compatibility code for other CPU models in the
future.


>  
>      /*
>       * From "Requirements for Implementing the Microsoft
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 1213bb7..852586a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1395,6 +1395,11 @@ struct X86CPU {
>       */
>      bool enable_l3_cache;
>  
> +    /* Compatibility bits for old machine types.
> +     * If true present the old cache topology information
> +     */
> +    bool legacy_cache;
> +
>      /* Compatibility bits for old machine types: */
>      bool enable_cpuid_0xb;
>  
> -- 
> 2.7.4
> 

-- 
Eduardo
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Moger, Babu 7 years, 7 months ago 

> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Tuesday, May 8, 2018 9:26 AM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel@redhat.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 v7 4/9] i386: Add new property to control cache info
> 
> On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> > This will be used to control the cache information.
> > By default new information will be displayed. If user
> > passes "-cpu legacy-cache" then older information will
> > be displayed even if the hardware supports new information.
> > It will be "on" for machine type "pc-q35-2.10" for compatibility.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > ---
> >  include/hw/i386/pc.h | 4 ++++
> >  target/i386/cpu.c    | 1 +
> >  target/i386/cpu.h    | 5 +++++
> >  3 files changed, 10 insertions(+)
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index ffee841..d904a3c 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> >          .driver   = "q35-pcihost",\
> >          .property = "x-pci-hole64-fix",\
> >          .value    = "off",\
> > +    },{\
> > +        .driver   = TYPE_X86_CPU,\
> > +        .property = "legacy-cache",\
> > +        .value    = "on",\
> >      },
> 
> >
> >  #define PC_COMPAT_2_9 \
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 5d88363..a27b658 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5138,6 +5138,7 @@ static Property x86_cpu_properties[] = {
> >                       false),
> >      DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU,
> vmware_cpuid_freq, true),
> >      DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> > +    DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),

Ok. I will remove this about line and add below code in x86_cpu_load_def.

> 
> 
> Hmm, this can get messy if we start adding cache info to other
> CPU models in future QEMU versions.  e.g.: what if we add cache
> info to Opteron_G3 on QEMU 2.14?
> 
> I suggest adding this to x86_cpu_load_def():
> 
>   cpu->legacy_cache = !cpu->cache_info.valid;
> 
> (Or equivalent code, in case cache_info is changed to be a
> pointer)
> 
> This way, only EPYC will have legacy-cache=false by now, making
> it easier to write compatibility code for other CPU models in the
> future.
> 
> 
> >
> >      /*
> >       * From "Requirements for Implementing the Microsoft
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 1213bb7..852586a 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1395,6 +1395,11 @@ struct X86CPU {
> >       */
> >      bool enable_l3_cache;
> >
> > +    /* Compatibility bits for old machine types.
> > +     * If true present the old cache topology information
> > +     */
> > +    bool legacy_cache;
> > +
> >      /* Compatibility bits for old machine types: */
> >      bool enable_cpuid_0xb;
> >
> > --
> > 2.7.4
> >
> 
> --
> Eduardo
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Eduardo Habkost 7 years, 7 months ago 
On Tue, May 08, 2018 at 05:26:16PM +0000, Moger, Babu wrote:
> 
> 
> > -----Original Message-----
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Tuesday, May 8, 2018 9:26 AM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: mst@redhat.com; marcel@redhat.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 v7 4/9] i386: Add new property to control cache info
> > 
> > On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> > > This will be used to control the cache information.
> > > By default new information will be displayed. If user
> > > passes "-cpu legacy-cache" then older information will
> > > be displayed even if the hardware supports new information.
> > > It will be "on" for machine type "pc-q35-2.10" for compatibility.
> > >
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > > ---
> > >  include/hw/i386/pc.h | 4 ++++
> > >  target/i386/cpu.c    | 1 +
> > >  target/i386/cpu.h    | 5 +++++
> > >  3 files changed, 10 insertions(+)
> > >
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index ffee841..d904a3c 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> > uint64_t *);
> > >          .driver   = "q35-pcihost",\
> > >          .property = "x-pci-hole64-fix",\
> > >          .value    = "off",\
> > > +    },{\
> > > +        .driver   = TYPE_X86_CPU,\
> > > +        .property = "legacy-cache",\
> > > +        .value    = "on",\
> > >      },
> > 
> > >
> > >  #define PC_COMPAT_2_9 \
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index 5d88363..a27b658 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -5138,6 +5138,7 @@ static Property x86_cpu_properties[] = {
> > >                       false),
> > >      DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU,
> > vmware_cpuid_freq, true),
> > >      DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> > > +    DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),
> 
> Ok. I will remove this about line and add below code in x86_cpu_load_def.

You need this line forthe PC_COMPAT_* entry to work.

But in this case, the default here won't matter because it will
be overwritten by x86_cpu_load_def().  So it's a good idea to add
a comment noting that the default will depend on the CPU model
being chosen, and point people to x86_cpu_load_def()).

> 
> > 
> > 
> > Hmm, this can get messy if we start adding cache info to other
> > CPU models in future QEMU versions.  e.g.: what if we add cache
> > info to Opteron_G3 on QEMU 2.14?
> > 
> > I suggest adding this to x86_cpu_load_def():
> > 
> >   cpu->legacy_cache = !cpu->cache_info.valid;
> > 
> > (Or equivalent code, in case cache_info is changed to be a
> > pointer)
> > 
> > This way, only EPYC will have legacy-cache=false by now, making
> > it easier to write compatibility code for other CPU models in the
> > future.
> > 
> > 
> > >
> > >      /*
> > >       * From "Requirements for Implementing the Microsoft
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index 1213bb7..852586a 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > @@ -1395,6 +1395,11 @@ struct X86CPU {
> > >       */
> > >      bool enable_l3_cache;
> > >
> > > +    /* Compatibility bits for old machine types.
> > > +     * If true present the old cache topology information
> > > +     */
> > > +    bool legacy_cache;
> > > +
> > >      /* Compatibility bits for old machine types: */
> > >      bool enable_cpuid_0xb;
> > >
> > > --
> > > 2.7.4
> > >
> > 
> > --
> > Eduardo

-- 
Eduardo
Re: [Qemu-devel] [PATCH v7 4/9] i386: Add new property to control cache info
Posted by Moger, Babu 7 years, 7 months ago 

> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Tuesday, May 8, 2018 1:34 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel@redhat.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 v7 4/9] i386: Add new property to control cache info
> 
> On Tue, May 08, 2018 at 05:26:16PM +0000, Moger, Babu wrote:
> >
> >
> > > -----Original Message-----
> > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > Sent: Tuesday, May 8, 2018 9:26 AM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: mst@redhat.com; marcel@redhat.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 v7 4/9] i386: Add new property to control cache info
> > >
> > > On Thu, Apr 26, 2018 at 11:26:44AM -0500, Babu Moger wrote:
> > > > This will be used to control the cache information.
> > > > By default new information will be displayed. If user
> > > > passes "-cpu legacy-cache" then older information will
> > > > be displayed even if the hardware supports new information.
> > > > It will be "on" for machine type "pc-q35-2.10" for compatibility.
> > > >
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > Tested-by: Geoffrey McRae <geoff@hostfission.com>
> > > > ---
> > > >  include/hw/i386/pc.h | 4 ++++
> > > >  target/i386/cpu.c    | 1 +
> > > >  target/i386/cpu.h    | 5 +++++
> > > >  3 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > > index ffee841..d904a3c 100644
> > > > --- a/include/hw/i386/pc.h
> > > > +++ b/include/hw/i386/pc.h
> > > > @@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> > > uint64_t *);
> > > >          .driver   = "q35-pcihost",\
> > > >          .property = "x-pci-hole64-fix",\
> > > >          .value    = "off",\
> > > > +    },{\
> > > > +        .driver   = TYPE_X86_CPU,\
> > > > +        .property = "legacy-cache",\
> > > > +        .value    = "on",\
> > > >      },
> > >
> > > >
> > > >  #define PC_COMPAT_2_9 \
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > index 5d88363..a27b658 100644
> > > > --- a/target/i386/cpu.c
> > > > +++ b/target/i386/cpu.c
> > > > @@ -5138,6 +5138,7 @@ static Property x86_cpu_properties[] = {
> > > >                       false),
> > > >      DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU,
> > > vmware_cpuid_freq, true),
> > > >      DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> > > > +    DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),
> >
> > Ok. I will remove this about line and add below code in x86_cpu_load_def.
> 
> You need this line forthe PC_COMPAT_* entry to work.
> 
> But in this case, the default here won't matter because it will
> be overwritten by x86_cpu_load_def().  So it's a good idea to add
> a comment noting that the default will depend on the CPU model
> being chosen, and point people to x86_cpu_load_def()).

Ok.  Got it. Thanks

> 
> >
> > >
> > >
> > > Hmm, this can get messy if we start adding cache info to other
> > > CPU models in future QEMU versions.  e.g.: what if we add cache
> > > info to Opteron_G3 on QEMU 2.14?
> > >
> > > I suggest adding this to x86_cpu_load_def():
> > >
> > >   cpu->legacy_cache = !cpu->cache_info.valid;
> > >
> > > (Or equivalent code, in case cache_info is changed to be a
> > > pointer)
> > >
> > > This way, only EPYC will have legacy-cache=false by now, making
> > > it easier to write compatibility code for other CPU models in the
> > > future.
> > >
> > >
> > > >
> > > >      /*
> > > >       * From "Requirements for Implementing the Microsoft
> > > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > > index 1213bb7..852586a 100644
> > > > --- a/target/i386/cpu.h
> > > > +++ b/target/i386/cpu.h
> > > > @@ -1395,6 +1395,11 @@ struct X86CPU {
> > > >       */
> > > >      bool enable_l3_cache;
> > > >
> > > > +    /* Compatibility bits for old machine types.
> > > > +     * If true present the old cache topology information
> > > > +     */
> > > > +    bool legacy_cache;
> > > > +
> > > >      /* Compatibility bits for old machine types: */
> > > >      bool enable_cpuid_0xb;
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > >
> > > --
> > > Eduardo
> 
> --
> Eduardo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.