Re: [Qemu-devel] [PATCH 1/7] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen

[Stefano Stabellini]

On Wed, 9 Dec 2015, Eduardo Habkost wrote:
> On Tue, Dec 08, 2015 at 03:07:22PM +0100, Gerd Hoffmann wrote:
> > rename pc_xen_hvm_init_pci to pc_i440fx_init_pci,
> > use it for both xen and non-xen init.
> > 
> > Signed-off-by: Gerd Hoffmann <address@hidden>
> > ---
> >  hw/i386/pc_piix.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 2e41efe..ce6c3c5 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine)
> >      pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
> >  }
> >  
> > -#ifdef CONFIG_XEN
> > -static void pc_xen_hvm_init_pci(MachineState *machine)
> > +static void pc_i440fx_init_pci(MachineState *machine)
> >  {
> > -    const char *pci_type = has_igd_gfx_passthru ?
> > +    const char *pci_type = machine->igd_gfx_passthru ?
> >                  TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : 
> > TYPE_I440FX_PCI_DEVICE;
> >  
> 
> Have you considered removing the has_igd_gfx_passthru global
> completely?

Indeed. It doesn't make much sense anymore.


> >      pc_init1(machine,
> > @@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
> >               pci_type);
> >  }
> >  
> > +#ifdef CONFIG_XEN
> >  static void pc_xen_hvm_init(MachineState *machine)
> >  {
> >      PCIBus *bus;
> > @@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine)
> >          exit(1);
> >      }
> >  
> > -    pc_xen_hvm_init_pci(machine);
> +    pc_i440fx_init_pci(machine);
> >  
> >      bus = pci_find_primary_bus();
> >      if (bus != NULL) {
> > @@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine)
> >          if (compat) { \
> >              compat(machine); \
> >          } \
> > -        pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> > -                 TYPE_I440FX_PCI_DEVICE); \
> > +        pc_i440fx_init_pci(machine); \
> 
> machine->igd_gfx_passthru defaults to false, meaning that in the
> default case the pc_init1() arguments in pc_i440fx_init_pci()
> will be the same as the call being replaced above, keeping
> exactly the same behavior.
> 
> This change breaks compatibility in the unlikely case somebody is
> already using igd-passthru=on in non-xenfv machines. I don't
> think it would make sense to keep a broken igd-passthru option in
> pc-2.5 and older for compatibility if nobody ever used that
> option, but it would be nice to mention that in the commit
> message.
> 
> Reviewed-by: Eduardo Habkost <address@hidden>

I agree

Reviewed-by: Stefano Stabellini <address@hidden>