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.

Thanks,
Marcel




Reply via email to