Re: [Qemu-devel] [PATCH v5 7/7] pc: add PC_I440FX_COMPAT to disable aercap for vifo device

[Alex Williamson]

On Wed, 2015-03-18 at 15:05 +0100, Michael S. Tsirkin wrote:
> On Wed, Mar 18, 2015 at 08:02:26AM -0600, Alex Williamson wrote:
> > On Wed, 2015-03-18 at 14:23 +0100, Michael S. Tsirkin wrote:
> > > typo in subject: vfio, not vifo.
> > > 
> > > On Thu, Mar 12, 2015 at 06:23:59PM +0800, Chen Fan wrote:
> > > > for piix4 chipset, we don't need to expose aer, so introduce
> > > > PC_I440FX_COMPAT for all piix4 machines to disable aercap,
> > > > and add HW_COMPAT_2_2 to disable aercap for all lower
> > > > than 2.3.
> > > > 
> > > > Signed-off-by: Chen Fan <address@hidden>
> > > 
> > > Well vfio is never migrated ATM.
> > > So why is compat code needed at all?
> > 
> > It's not for migration, it's to maintain current behavior on existing
> > platforms.  If someone gets an uncorrected AER error on q35 machine type
> > today, the VM stops.  With this change, AER would be exposed to the
> > guest and the guest could handle it.  The compat change therefore
> > maintains the stop VM behavior on existing q35 machine types.
> 
> If stop VM behaviour is useful, expose it to users.
> If not, then don't.
> I don't see why does it have to be tied to machine types.

Because q35-2.2 machine type will currently do a stop VM on uncorrected
AER error.  If we don't tie that to a machine option then q35-2.2 would
suddenly start exposing the error to the guest.  That's a fairly
significant change in behavior for a static machine type.

> >  As I
> > commented here, the 440fx part of this patch is unnecessary since AER
> > cannot be exposed to the guest on a conventional PCI chipset.  Thanks,
> > 
> > Alex
> > 
> > > > ---
> > > >  hw/i386/pc_piix.c   |  9 +++++++++
> > > >  hw/i386/pc_q35.c    |  4 ++++
> > > >  include/hw/compat.h | 10 ++++++++++
> > > >  3 files changed, 23 insertions(+)
> > > > 
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index 8eab4ba..ff9d312 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -307,6 +307,11 @@ static void pc_init1(MachineState *machine,
> > > >  
> > > >  static void pc_init_pci(MachineState *machine)
> > > >  {
> > > > +    static GlobalProperty pc_compat_props[] = {
> > > > +        PC_I440FX_COMPAT,
> > > > +        { /* end of list */ }
> > > > +    };
> > > > +    qdev_prop_register_global_list(pc_compat_props);
> > > >      pc_init1(machine, 1, 1);
> > > >  }
> > > >  
> > > > @@ -534,6 +539,10 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
> > > >      PC_I440FX_2_2_MACHINE_OPTIONS,
> > > >      .name = "pc-i440fx-2.2",
> > > >      .init = pc_init_pci_2_2,
> > > > +    .compat_props = (GlobalProperty[]) {
> > > > +        HW_COMPAT_2_2,
> > > > +        { /* end of list */ }
> > > > +    },
> > > >  };
> > > >  
> > > >  #define PC_I440FX_2_1_MACHINE_OPTIONS                           \
> > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > > index c0f21fe..97afb7d 100644
> > > > --- a/hw/i386/pc_q35.c
> > > > +++ b/hw/i386/pc_q35.c
> > > > @@ -431,6 +431,10 @@ static QEMUMachine pc_q35_machine_v2_2 = {
> > > >      PC_Q35_2_2_MACHINE_OPTIONS,
> > > >      .name = "pc-q35-2.2",
> > > >      .init = pc_q35_init_2_2,
> > > > +    .compat_props = (GlobalProperty[]) {
> > > > +        HW_COMPAT_2_2,
> > > > +        { /* end of list */ }
> > > > +    },
> > > >  };
> > > >  
> > > >  #define PC_Q35_2_1_MACHINE_OPTIONS                      \
> > > > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > > > index 313682a..40c974a 100644
> > > > --- a/include/hw/compat.h
> > > > +++ b/include/hw/compat.h
> > > > @@ -1,7 +1,17 @@
> > > >  #ifndef HW_COMPAT_H
> > > >  #define HW_COMPAT_H
> > > >  
> > > > +#define HW_COMPAT_2_2 PC_I440FX_COMPAT
> > > > +
> > > > +#define PC_I440FX_COMPAT \
> > > > +        {\
> > > > +            .driver   = "vfio-pci",\
> > > > +            .property = "x-aer",\
> > > > +            .value    = "off",\
> > > > +        }
> > > > +
> > > >  #define HW_COMPAT_2_1 \
> > > > +        HW_COMPAT_2_2, \
> > > >          {\
> > > >              .driver   = "intel-hda",\
> > > >              .property = "old_msi_addr",\
> > > > -- 
> > > > 1.9.3
> > > > 
> > 
> >