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

[Eduardo Habkost]

On Fri, Sep 16, 2016 at 10:08:30AM +0200, Igor Mammedov wrote:
> On Thu, 15 Sep 2016 11:23:47 -0300
> Eduardo Habkost  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 
> > > ---
> > >  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).

-- 
Eduardo

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

[Igor Mammedov]

On Thu, 15 Sep 2016 11:23:47 -0300
Eduardo Habkost  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 
> > ---
> >  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
> >   
> 

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

[Eduardo Habkost]

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 
> ---
>  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
> 

-- 
Eduardo

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

[Marcel Apfelbaum]

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 

Thanks,
Marcel



Signed-off-by: Igor Mammedov 
---
 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 \