On 12/01/2015 05:09 PM, Eduardo Habkost wrote:
On Tue, Dec 01, 2015 at 04:55:57PM +0200, Marcel Apfelbaum wrote:
On 12/01/2015 04:48 PM, Eduardo Habkost wrote:
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?
Hmm, you can derive the info you need from pci_bus_is_express.
If express-> no acpi_hotplug. This is not 100% true, but since
we don't support acpi hotplug on PCIe machines, it should be OK for now.
What about just checking if AcpiPmInfo.pcihp_io_base is set?
Because this contradicts the "do not probe for piix" requirement.
pcihp_io_base depends on piix query for pm (piix4_pm_find).
So pcihp_io_base is an i440fx only "artifact".
Also, acpi_set_pci_info is called before acpi_build that populates
acpi_get_pm_info. All of that can be taken care of, of course.
At the end of the day, as long as the functionality is preserved,
I personally have no objection in re-factoring.
Thanks,
Marcel
