[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] PCIE: fix regression with coldplugged multifunc
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] PCIE: fix regression with coldplugged multifunction device |
Date: |
Mon, 17 Feb 2014 16:20:44 +0200 |
On Mon, Feb 17, 2014 at 03:00:06PM +0100, Igor Mammedov wrote:
> QEMU abort is caused by misplaced assertion, which should
> be checked only when device is hotplugged.
>
> Refernce to regression report:
> http://www.mail-archive.com/address@hidden/msg216226.html
>
> Reported-By: Nigel Kukard <address@hidden>
> Signed-off-by: Igor Mammedov <address@hidden>
Applied, thanks!
BTW we really should make hotplug return an error not assert.
> ---
> hw/pci/pcie.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 8ecd11e..02cde6f 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -221,29 +221,23 @@ static void pcie_cap_slot_hotplug_common(PCIDevice
> *hotplug_dev,
> DeviceState *dev,
> uint8_t **exp_cap, Error **errp)
> {
> - PCIDevice *pci_dev = PCI_DEVICE(dev);
> *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
> uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
>
> - PCIE_DEV_PRINTF(pci_dev, "hotplug state: %d\n", state);
> + PCIE_DEV_PRINTF(PCI_DEVICE(dev), "hotplug state: %d\n", state);
> if (sltsta & PCI_EXP_SLTSTA_EIS) {
> /* the slot is electromechanically locked.
> * This error is propagated up to qdev and then to HMP/QMP.
> */
> error_setg_errno(errp, -EBUSY, "slot is electromechanically locked");
> }
> -
> - /* TODO: multifunction hot-plug.
> - * Right now, only a device of function = 0 is allowed to be
> - * hot plugged/unplugged.
> - */
> - assert(PCI_FUNC(pci_dev->devfn) == 0);
> }
>
> void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> uint8_t *exp_cap;
> + PCIDevice *pci_dev = PCI_DEVICE(dev);
>
> pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap,
> errp);
>
> @@ -256,6 +250,12 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> return;
> }
>
> + /* TODO: multifunction hot-plug.
> + * Right now, only a device of function = 0 is allowed to be
> + * hot plugged/unplugged.
> + */
> + assert(PCI_FUNC(pci_dev->devfn) == 0);
> +
> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
> PCI_EXP_SLTSTA_PDS);
> pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
> --
> 1.7.1