On Tue, Dec 01, 2015 at 04:07:33PM +0200, Marcel Apfelbaum wrote: > On 11/30/2015 05:07 PM, Eduardo Habkost wrote: > >On Sun, Nov 29, 2015 at 10:46:03AM +0200, Marcel Apfelbaum wrote: > >>On 11/27/2015 07:28 PM, Eduardo Habkost wrote: > >>>On Thu, Nov 26, 2015 at 06:00:28PM +0200, Marcel Apfelbaum wrote: > >>>>Add bus property to PC machines and use it when looking > >>>>for primary PCI root bus (bus 0). > >>>> > >>>>Signed-off-by: Marcel Apfelbaum <mar...@redhat.com> > >>> > >>>I can't pretend I have reviewed the q35 part, but the changes are > >>>an improvement to the existing code that depended on > >>>find_i440fx(). > >>> > >>>Acked-by: Eduardo Habkost <ehabk...@redhat.com> > >> > >>Thanks! > >> > >>> > >>>BTW, what's missing to allow us to change acpi_set_pci_info() to > >>>use PCMachine::bus instead of find_i440fx(), too? How much of the > >>>PCI hotplug stuff is different in q35? > >> > >>It is pretty different. > >>i440fx has acpi based hotplug while q35 has PCIe native hotplug. Since is > >>"native", no acpi info is necessary. > >> > >>Having said that, if we have an PCIe-PCI bridge, the pci devices behind it > >>cannot be hotplugged/unplugged right now. > >> > >>Once we decide to add hotplug support for this scenario, maybe we can get > >>rid of > >>find_i440fx(). > > > >Thanks for the explanation. I wonder if there's a better way to > >check if ACPI-based hotplug is needed by looking at > >PCMachineState or PCIBus, so we don't couple the ACPI code to > >piix.c. > > > > I suppose we can do something about it, like adding a property to > PCMachineState, > lets say bool acpi_hotplug and set it false for Q35. > > Then we have: > pcm = PC_MACHINE(current_machine); > if(pcm->acpi_hotplug) { > bus = pcm->bus; > ... > } > > Sounds acceptable? If yes, I'll send a patch on top since is not directly > related.S
There's no existing field or method in PCIBus that can be already used for that? If not, that sounds good to me. But I would move the field to PCMachineClass instead of PCMachineState. Would you like me to do it? I am planning to submit other changes to remove q35/piix dependencies from acpi-build.c. -- Eduardo