diff mbox

[2/2] target-i386: turn off CPU.l3-cache only for 2.7 and older machine types

Message ID 1473847310-129729-3-git-send-email-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Igor Mammedov Sept. 14, 2016, 10:01 a.m. UTC
commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
misplaced compat property putting it in new 2.8 machine type
which would effectively to disable feature until 2.9 is released.
Intent of commit probably should be to disable feature for 2.7
and older while allowing not yet released 2.8 to have feature
enabled by default.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/i386/pc.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Marcel Apfelbaum Sept. 14, 2016, 11:35 a.m. UTC | #1
On 09/14/2016 01:01 PM, Igor Mammedov wrote:
> commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
> misplaced compat property putting it in new 2.8 machine type
> which would effectively to disable feature until 2.9 is released.
> Intent of commit probably should be to disable feature for 2.7
> and older while allowing not yet released 2.8 to have feature
> enabled by default.

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/i386/pc.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index d5654ab..1c5fd08 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -369,17 +369,16 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
>  #define PC_COMPAT_2_8 \
> +
> +#define PC_COMPAT_2_7 \
> +    PC_COMPAT_2_8 \
> +    HW_COMPAT_2_7 \
>      {\
>          .driver   = TYPE_X86_CPU,\
>          .property = "l3-cache",\
>          .value    = "off",\
>      },
>
> -
> -#define PC_COMPAT_2_7 \
> -    PC_COMPAT_2_8 \
> -    HW_COMPAT_2_7
> -
>  #define PC_COMPAT_2_6 \
>      PC_COMPAT_2_7 \
>      HW_COMPAT_2_6 \
>
Eduardo Habkost Sept. 15, 2016, 2:23 p.m. UTC | #2
On Wed, Sep 14, 2016 at 12:01:50PM +0200, Igor Mammedov wrote:
> commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
> misplaced compat property putting it in new 2.8 machine type
> which would effectively to disable feature until 2.9 is released.
> Intent of commit probably should be to disable feature for 2.7
> and older while allowing not yet released 2.8 to have feature
> enabled by default.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/i386/pc.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index d5654ab..1c5fd08 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -369,17 +369,16 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_8 \
> +
> +#define PC_COMPAT_2_7 \
> +    PC_COMPAT_2_8 \

Same as patch 1/2: this doesn't seem to be necessary since commit
bacc344c548ce165a0001276ece56ee4b0bddae3.

> +    HW_COMPAT_2_7 \
>      {\
>          .driver   = TYPE_X86_CPU,\
>          .property = "l3-cache",\
>          .value    = "off",\
>      },
>  
> -
> -#define PC_COMPAT_2_7 \
> -    PC_COMPAT_2_8 \
> -    HW_COMPAT_2_7
> -
>  #define PC_COMPAT_2_6 \
>      PC_COMPAT_2_7 \
>      HW_COMPAT_2_6 \
> -- 
> 2.7.4
>
Igor Mammedov Sept. 16, 2016, 8:08 a.m. UTC | #3
On Thu, 15 Sep 2016 11:23:47 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Wed, Sep 14, 2016 at 12:01:50PM +0200, Igor Mammedov wrote:
> > commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
> > misplaced compat property putting it in new 2.8 machine type
> > which would effectively to disable feature until 2.9 is released.
> > Intent of commit probably should be to disable feature for 2.7
> > and older while allowing not yet released 2.8 to have feature
> > enabled by default.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/i386/pc.h | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index d5654ab..1c5fd08 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -369,17 +369,16 @@ int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >  
> >  #define PC_COMPAT_2_8 \
> > +
> > +#define PC_COMPAT_2_7 \
> > +    PC_COMPAT_2_8 \  
> 
> Same as patch 1/2: this doesn't seem to be necessary since commit
> bacc344c548ce165a0001276ece56ee4b0bddae3.

"l3-cache" is off for 2.8 regardless of bacc344c while it should be off
only for 2.7 and older.

Anyway, I'll respin because of 1/2 should be rewritten.

> 
> > +    HW_COMPAT_2_7 \
> >      {\
> >          .driver   = TYPE_X86_CPU,\
> >          .property = "l3-cache",\
> >          .value    = "off",\
> >      },
> >  
> > -
> > -#define PC_COMPAT_2_7 \
> > -    PC_COMPAT_2_8 \
> > -    HW_COMPAT_2_7
> > -
> >  #define PC_COMPAT_2_6 \
> >      PC_COMPAT_2_7 \
> >      HW_COMPAT_2_6 \
> > -- 
> > 2.7.4
> >   
>
Eduardo Habkost Sept. 16, 2016, 11:31 a.m. UTC | #4
On Fri, Sep 16, 2016 at 10:08:30AM +0200, Igor Mammedov wrote:
> On Thu, 15 Sep 2016 11:23:47 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Wed, Sep 14, 2016 at 12:01:50PM +0200, Igor Mammedov wrote:
> > > commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
> > > misplaced compat property putting it in new 2.8 machine type
> > > which would effectively to disable feature until 2.9 is released.
> > > Intent of commit probably should be to disable feature for 2.7
> > > and older while allowing not yet released 2.8 to have feature
> > > enabled by default.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  include/hw/i386/pc.h | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index d5654ab..1c5fd08 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -369,17 +369,16 @@ int e820_get_num_entries(void);
> > >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > >  
> > >  #define PC_COMPAT_2_8 \
> > > +
> > > +#define PC_COMPAT_2_7 \
> > > +    PC_COMPAT_2_8 \  
> > 
> > Same as patch 1/2: this doesn't seem to be necessary since commit
> > bacc344c548ce165a0001276ece56ee4b0bddae3.
> 
> "l3-cache" is off for 2.8 regardless of bacc344c while it should be off
> only for 2.7 and older.
> 
> Anyway, I'll respin because of 1/2 should be rewritten.

Yes, l3-cache on PC_COMPAT_2_7 is necessary. I was only talking
about the PC_COMPAT_2_8 line above, because it looked like a
newly added line (but now I see it was just the line from patch
1/2 being moved, sorry for the confusion).
diff mbox

Patch

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index d5654ab..1c5fd08 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -369,17 +369,16 @@  int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_COMPAT_2_8 \
+
+#define PC_COMPAT_2_7 \
+    PC_COMPAT_2_8 \
+    HW_COMPAT_2_7 \
     {\
         .driver   = TYPE_X86_CPU,\
         .property = "l3-cache",\
         .value    = "off",\
     },
 
-
-#define PC_COMPAT_2_7 \
-    PC_COMPAT_2_8 \
-    HW_COMPAT_2_7
-
 #define PC_COMPAT_2_6 \
     PC_COMPAT_2_7 \
     HW_COMPAT_2_6 \