Re: [Qemu-devel] [PATCH] PCIE: fix regression with coldplugged multifunction device

[Michael S. Tsirkin]

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