diff mbox

[1/2] pc: fix regression introduced by adding 2.8 machine

Message ID 1473847310-129729-2-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov Sept. 14, 2016, 10:01 a.m. UTC
commit (a4d3c834 pc: Add 2.8 machine) didn't ammend
PC_COMPAT_2_6 to include PC_COMPAT_2_7 which results in

    {\
        .driver   = "virtio-pci",\
        .property = "page-per-vq",\
        .value    = "on",\
    },

and

    {\
        .driver   = TYPE_X86_CPU,\
        .property = "l3-cache",\
        .value    = "off",\
    },

being ignored and as result change PC_COMPAT_2_6 behaviour.

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

Comments

Marcel Apfelbaum Sept. 14, 2016, 11:32 a.m. UTC | #1
On 09/14/2016 01:01 PM, Igor Mammedov wrote:
> commit (a4d3c834 pc: Add 2.8 machine) didn't ammend
> PC_COMPAT_2_6 to include PC_COMPAT_2_7 which results in
>
>     {\
>         .driver   = "virtio-pci",\
>         .property = "page-per-vq",\
>         .value    = "on",\
>     },
>
> and
>
>     {\
>         .driver   = TYPE_X86_CPU,\
>         .property = "l3-cache",\
>         .value    = "off",\
>     },
>
> being ignored and as result change PC_COMPAT_2_6 behaviour.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/i386/pc.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ebba151..d5654ab 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -381,6 +381,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>      HW_COMPAT_2_7
>
>  #define PC_COMPAT_2_6 \
> +    PC_COMPAT_2_7 \
>      HW_COMPAT_2_6 \
>      {\
>          .driver   = "fw_cfg_io",\
>

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

Thanks,
Marcel
Eduardo Habkost Sept. 15, 2016, 2:19 p.m. UTC | #2
On Wed, Sep 14, 2016 at 12:01:49PM +0200, Igor Mammedov wrote:
> commit (a4d3c834 pc: Add 2.8 machine) didn't ammend
> PC_COMPAT_2_6 to include PC_COMPAT_2_7 which results in
> 
>     {\
>         .driver   = "virtio-pci",\
>         .property = "page-per-vq",\
>         .value    = "on",\
>     },
> 
> and
> 
>     {\
>         .driver   = TYPE_X86_CPU,\
>         .property = "l3-cache",\
>         .value    = "off",\
>     },
> 
> being ignored and as result change PC_COMPAT_2_6 behaviour.
> 
[...]
>  #define PC_COMPAT_2_6 \
> +    PC_COMPAT_2_7 \
>      HW_COMPAT_2_6 \
>      {\
>          .driver   = "fw_cfg_io",\

Isn't this supposed to be unnecessary since commit bacc344c
("machine: add properties to compat_props incrementaly")?
Michael S. Tsirkin Sept. 15, 2016, 3:18 p.m. UTC | #3
On Thu, Sep 15, 2016 at 11:19:09AM -0300, Eduardo Habkost wrote:
> On Wed, Sep 14, 2016 at 12:01:49PM +0200, Igor Mammedov wrote:
> > commit (a4d3c834 pc: Add 2.8 machine) didn't ammend
> > PC_COMPAT_2_6 to include PC_COMPAT_2_7 which results in
> > 
> >     {\
> >         .driver   = "virtio-pci",\
> >         .property = "page-per-vq",\
> >         .value    = "on",\
> >     },
> > 
> > and
> > 
> >     {\
> >         .driver   = TYPE_X86_CPU,\
> >         .property = "l3-cache",\
> >         .value    = "off",\
> >     },
> > 
> > being ignored and as result change PC_COMPAT_2_6 behaviour.
> > 
> [...]
> >  #define PC_COMPAT_2_6 \
> > +    PC_COMPAT_2_7 \
> >      HW_COMPAT_2_6 \
> >      {\
> >          .driver   = "fw_cfg_io",\
> 
> Isn't this supposed to be unnecessary since commit bacc344c
> ("machine: add properties to compat_props incrementaly")?
> 
> -- 
> Eduardo


Indeed, I tested page-per-vq and it seems on for 2.6.
Igor, Marcel, could you comment pls?
Igor Mammedov Sept. 16, 2016, 8:03 a.m. UTC | #4
On Thu, 15 Sep 2016 18:18:46 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Sep 15, 2016 at 11:19:09AM -0300, Eduardo Habkost wrote:
> > On Wed, Sep 14, 2016 at 12:01:49PM +0200, Igor Mammedov wrote:  
> > > commit (a4d3c834 pc: Add 2.8 machine) didn't ammend
> > > PC_COMPAT_2_6 to include PC_COMPAT_2_7 which results in
> > > 
> > >     {\
> > >         .driver   = "virtio-pci",\
> > >         .property = "page-per-vq",\
> > >         .value    = "on",\
> > >     },
> > > 
> > > and
> > > 
> > >     {\
> > >         .driver   = TYPE_X86_CPU,\
> > >         .property = "l3-cache",\
> > >         .value    = "off",\
> > >     },
> > > 
> > > being ignored and as result change PC_COMPAT_2_6 behaviour.
> > >   
> > [...]  
> > >  #define PC_COMPAT_2_6 \
> > > +    PC_COMPAT_2_7 \
> > >      HW_COMPAT_2_6 \
> > >      {\
> > >          .driver   = "fw_cfg_io",\  
> > 
> > Isn't this supposed to be unnecessary since commit bacc344c
> > ("machine: add properties to compat_props incrementaly")?
> > 
> > -- 
> > Eduardo  
> 
> 
> Indeed, I tested page-per-vq and it seems on for 2.6.
> Igor, Marcel, could you comment pls?

You are right, I think inconsistency in

#define PC_COMPAT_2_5 \                                                          
    PC_COMPAT_2_6 \                                                              
    HW_COMPAT_2_5  
 
and

#define PC_COMPAT_2_7 \                                                          
    PC_COMPAT_2_8 \                                                              
    HW_COMPAT_2_7

made me think that we still need it.

I'd better clean-up PC_COMPAT_2_5 and PC_COMPAT_2_7
diff mbox

Patch

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ebba151..d5654ab 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -381,6 +381,7 @@  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
     HW_COMPAT_2_7
 
 #define PC_COMPAT_2_6 \
+    PC_COMPAT_2_7 \
     HW_COMPAT_2_6 \
     {\
         .driver   = "fw_cfg_io",\