[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/4] Fix broken PCIe device after migration
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH 0/4] Fix broken PCIe device after migration |
Date: |
Sun, 27 Feb 2022 05:22:33 -0500 |
On Fri, Feb 25, 2022 at 04:50:54PM +0100, Igor Mammedov wrote:
> On Fri, 25 Feb 2022 08:50:43 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Fri, Feb 25, 2022 at 02:18:23PM +0100, Igor Mammedov wrote:
> > > On Fri, 25 Feb 2022 04:58:46 -0500
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > > > On Thu, Feb 24, 2022 at 12:44:07PM -0500, Igor Mammedov wrote:
> > > > > Currently ACPI PCI hotplug is enabled by default for Q35 machine
> > > > > type and overrides native PCIe hotplug. It works as expected when
> > > > > a PCIe device is hotplugged into slot, however the device becomes
> > > > > in-operational after migration. Which is caused by BARs being
> > > > > disabled on target due to powered off status of the slot.
> > > > >
> > > > > Proposed fix disables power control on PCIe slot when ACPI pcihp
> > > > > takes over hotplug control and makes PCIe slot check if power
> > > > > control is enabled before trying to change slot's power. Which
> > > > > leaves slot always powered on and that makes PCI core keep BARs
> > > > > enabled.
> > > >
> > > >
> > > > I thought some more about this. One of the reasons we
> > > > did not remove the hotplug capability is really so
> > > > it's easier to layer acpi on top of pcihp if we
> > > > want to do it down the road. And it would be quite annoying
> > > > if we had to add more hack to go back to having capability.
> > > >
> > > > How about instead of patch 3 we call pci_set_power(dev, true) for all
> > > > devices where ACPI is managing hotplug immediately on plug? This will
> > > > fix the bug avoiding headache with migration.
> > >
> > > true it would be more migration friendly (v6.2 still broken
> > > but that can't be helped), since we won't alter pci_config at all.
> > > Although it's still more hackish compared to disabling SLTCAP_PCP
> > > (though it seems guest OSes have no issues with SLTCAP_PCP being
> > > present but not really operational, at least for ~6months the thing
> > > was released (6.1-6.2-now)).
> > >
> > > Let me play with this idea and see if it works and at what cost,
> > > though I still prefer cleaner SLTCAP_PCP disabling to make sure
> > > guest OS won't get wrong idea about power control being present
> > > when it's not actually not.
> >
> > Well the control is present, isn't it? Can be used to e.g. reset the
> > device behind the bridge.
>
> can you point to how reset is supposed to work?
Well, I am alluding to this code in linux
static const struct pci_reset_fn_method pci_reset_fn_methods[] = {
{ },
{ pci_dev_specific_reset, .name = "device_specific" },
{ pci_dev_acpi_reset, .name = "acpi" },
{ pcie_reset_flr, .name = "flr" },
{ pci_af_flr, .name = "af_flr" },
{ pci_pm_reset, .name = "pm" },
{ pci_reset_bus_function, .name = "bus" },
};
Thinkably down the road linux could add a method powering the secondary bus
off then back on as a way to reset devices behind it.
There are plenty of other ways so it's not that I can say why that
specific way of doing it is useful.
> >
> > >
> > > > Patch 2 does seem like a good idea.
> > > >
> > > > > PS:
> > > > > it's still hacky approach as all ACPI PCI hotplug is, but it's
> > > > > the least intrusive one. Alternative, I've considered, could be
> > > > > chaining hotplug callbacks and making pcihp ones call overriden
> > > > > native callbacks while inhibiting hotplug event in native callbacks
> > > > > somehow. But that were a bit more intrusive and spills over to SHPC
> > > > > if implemented systematically, so I ditched that for now. It could
> > > > > be resurrected later on if current approach turns out to be
> > > > > insufficient.
> > > > >
> > > > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2053584
> > > > > CC: mst@redhat.com
> > > > > CC: kraxel@redhat.com
> > > > >
> > > > > Igor Mammedov (4):
> > > > > pci: expose TYPE_XIO3130_DOWNSTREAM name
> > > > > pcie: update slot power status only is power control is enabled
> > > > > acpi: pcihp: disable power control on PCIe slot
> > > > > q35: compat: keep hotplugged PCIe device broken after migration for
> > > > > 6.2-older machine types
> > > > >
> > > > > include/hw/acpi/pcihp.h | 4 +++-
> > > > > include/hw/pci-bridge/xio3130_downstream.h | 15 +++++++++++++++
> > > > > hw/acpi/acpi-pci-hotplug-stub.c | 3 ++-
> > > > > hw/acpi/ich9.c | 21 ++++++++++++++++++++-
> > > > > hw/acpi/pcihp.c | 16 +++++++++++++++-
> > > > > hw/acpi/piix4.c | 3 ++-
> > > > > hw/core/machine.c | 4 +++-
> > > > > hw/pci-bridge/xio3130_downstream.c | 3 ++-
> > > > > hw/pci/pcie.c | 5 ++---
> > > > > 9 files changed, 64 insertions(+), 10 deletions(-)
> > > > > create mode 100644 include/hw/pci-bridge/xio3130_downstream.h
> > > > >
> > > > > --
> > > > > 2.31.1
> > > >
> >
- [PATCH 3/4] acpi: pcihp: disable power control on PCIe slot, (continued)
- [PATCH 3/4] acpi: pcihp: disable power control on PCIe slot, Igor Mammedov, 2022/02/24
- [PATCH 4/4] q35: compat: keep hotplugged PCIe device broken after migration for 6.2-older machine types, Igor Mammedov, 2022/02/24
- Re: [PATCH 0/4] Fix broken PCIe device after migration, Michael S. Tsirkin, 2022/02/24
- Re: [PATCH 0/4] Fix broken PCIe device after migration, Michael S. Tsirkin, 2022/02/25
- Re: [PATCH 0/4] Fix broken PCIe device after migration, Igor Mammedov, 2022/02/25