From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxdYy-0001YD-V2 for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:43:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxdYr-0003dO-57 for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:43:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34498) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxdYq-0003cB-Rp for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:43:33 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5JEhV6v017189 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 19 Jun 2014 10:43:32 -0400 Date: Thu, 19 Jun 2014 17:43:58 +0300 From: "Michael S. Tsirkin" Message-ID: <20140619144358.GB10359@redhat.com> References: <1403185941-19561-1-git-send-email-marcel.a@redhat.com> <1403185941-19561-4-git-send-email-marcel.a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1403185941-19561-4-git-send-email-marcel.a@redhat.com> Subject: Re: [Qemu-devel] [PATCH 3/3] hw/pcie: better hotplug/hotunplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: qemu-devel@nongnu.org On Thu, Jun 19, 2014 at 04:52:21PM +0300, Marcel Apfelbaum wrote: > Hotplug triggers both 'present detect change' and > 'attention button pressed'. > > Hotunplug starts by triggering 'attention button pressed', > then waits for the OS to power off the device and only > then detaches it. > Pls not here that current code is broken: it does surprise removal which crashes guests. > Signed-off-by: Marcel Apfelbaum > --- > hw/pci/pcie.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index f8bf515..9cfd93d 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -258,7 +258,8 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > 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); > + pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), > + PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP); > } > > void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > @@ -268,10 +269,7 @@ void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp); > > - object_unparent(OBJECT(dev)); > - pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA, > - PCI_EXP_SLTSTA_PDS); > - pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC); > + pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev)); > } > > /* pci express slot for pci express root/downstream port > @@ -389,6 +387,18 @@ void pcie_cap_slot_write_config(PCIDevice *dev, > sltsta); > } > pls add code comments explaining the logic here. > + if ((sltsta & PCI_EXP_SLTSTA_PDS) && (val & PCI_EXP_SLTCTL_PCC) && > + ((val & PCI_EXP_SLTCTL_PIC_OFF) == PCI_EXP_SLTCTL_PIC_OFF)) { > + PCIDevice *slot_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(dev))->devices[0]; > + if (slot_dev) { Here you want to remove all devices behind the bridge? You need to do this for all functions, not just function 0. > + object_unparent(OBJECT(slot_dev)); > + pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA, > + PCI_EXP_SLTSTA_PDS); > + pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, > + PCI_EXP_SLTSTA_PDC); These bits need to be cleared in any case? > + } > + } > + > hotplug_event_notify(dev); > > /* > -- > 1.8.3.1