From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlwhk-0003Zh-Ce for qemu-devel@nongnu.org; Tue, 13 Oct 2015 06:21:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zlwhh-0008I0-OS for qemu-devel@nongnu.org; Tue, 13 Oct 2015 06:21:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35484) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlwhh-0008Hr-HG for qemu-devel@nongnu.org; Tue, 13 Oct 2015 06:21:09 -0400 Date: Tue, 13 Oct 2015 13:21:03 +0300 From: "Michael S. Tsirkin" Message-ID: <20151013132042-mutt-send-email-mst@redhat.com> References: <1444725695-27517-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1444725695-27517-3-git-send-email-caoj.fnst@cn.fujitsu.com> <20151013115045-mutt-send-email-mst@redhat.com> <561CD4DA.20001@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <561CD4DA.20001@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: pbonzini@redhat.com, alex.williamson@redhat.com, qemu-devel@nongnu.org, izumi.taku@jp.fujitsu.com On Tue, Oct 13, 2015 at 05:54:34PM +0800, Cao jin wrote: > Hi Michael > > On 10/13/2015 04:51 PM, Michael S. Tsirkin wrote: > >On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote: > >>In case user regret when hot-adding multi-function, should roll back, > >>device_del the function added but not exposed to the guest. > >> > >>Signed-off-by: Cao jin > > > >I think this patch should come first, before we enable the > >functionality that depends on it. > > > Do you mean, the function should be removed individually in any condition? I just mean the patches in the series should be reordered. > Because as you know, device_del pci_dev will remove all the functions in the > slot that are fulled exposed to the guest, Alex also mentioned this > limitation before. > > > >>--- > >> hw/pci/pci_host.c | 6 +++++- > >> hw/pci/pcie.c | 22 +++++++++++++++++----- > >> 2 files changed, 22 insertions(+), 6 deletions(-) > >> > >>diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > >>index 3e26f92..35e5cf3 100644 > >>--- a/hw/pci/pci_host.c > >>+++ b/hw/pci/pci_host.c > >>@@ -20,6 +20,7 @@ > >> > >> #include "hw/pci/pci.h" > >> #include "hw/pci/pci_host.h" > >>+#include "hw/pci/pci_bus.h" > >> #include "trace.h" > >> > >> /* debug PCI */ > >>@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len) > >> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len) > >> { > >> PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr); > >>+ PCIDevice *f0 = NULL; > >> uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1); > >> uint32_t val; > >>+ uint8_t slot = (addr >> 11) & 0x1F; > >> > >>- if (!pci_dev) { > >>+ f0 = s->devices[PCI_DEVFN(slot, 0)]; > >>+ if (!pci_dev || (!f0 && pci_dev)) { > >> return ~0x0; > >> } > >> > >>diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > >>index 89bf61b..58d2153 100644 > >>--- a/hw/pci/pcie.c > >>+++ b/hw/pci/pcie.c > >>@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > >> } > >> } > >> > >>+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque) > >>+{ > >>+ object_unparent(OBJECT(dev)); > >>+} > >>+ > >> void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev, > >> DeviceState *dev, Error **errp) > >> { > >> uint8_t *exp_cap; > >>+ PCIDevice *pci_dev = PCI_DEVICE(dev); > >>+ PCIBus *bus = pci_dev->bus; > >> > >> pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp); > >> > >>+ /* In case user regret when hot-adding multi function, remove the function > >>+ * that is unexposed to guest individually, without interaction with guest. > >>+ */ > >>+ if (PCI_FUNC(pci_dev->devfn) > 0 && > >>+ bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) { > >>+ pcie_unplug_device(bus, pci_dev, NULL); > >>+ > >>+ return; > >>+ } > >>+ > >> pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev)); > >> } > >> > >>@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev) > >> hotplug_event_update_event_status(dev); > >> } > >> > >>-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque) > >>-{ > >>- object_unparent(OBJECT(dev)); > >>-} > >>- > >> void pcie_cap_slot_write_config(PCIDevice *dev, > >> uint32_t addr, uint32_t val, int len) > >> { > >>-- > >>2.1.0 > >. > > > > -- > Yours Sincerely, > > Cao Jin