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

2014-02-17 Thread Igor Mammedov 
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/qemu-devel@nongnu.org/msg216226.html

Reported-By: Nigel Kukard nkukard+q...@lbsd.net
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
 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

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

2014-02-17 Thread 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/qemu-devel@nongnu.org/msg216226.html
 
 Reported-By: Nigel Kukard nkukard+q...@lbsd.net
 Signed-off-by: Igor Mammedov imamm...@redhat.com

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