From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47806) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Za3n0-0007hq-88 for qemu-devel@nongnu.org; Thu, 10 Sep 2015 11:29:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Za3mz-0000SY-6V for qemu-devel@nongnu.org; Thu, 10 Sep 2015 11:29:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Za3mz-0000SS-1d for qemu-devel@nongnu.org; Thu, 10 Sep 2015 11:29:29 -0400 Message-ID: <1441898967.20355.578.camel@redhat.com> From: Alex Williamson Date: Thu, 10 Sep 2015 09:29:27 -0600 In-Reply-To: <1441887143-26756-2-git-send-email-caoj.fnst@cn.fujitsu.com> References: <1441887143-26756-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1441887143-26756-2-git-send-email-caoj.fnst@cn.fujitsu.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] PCI-e device multi-function hot-add support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: pbonzini@redhat.com, qemu-devel@nongnu.org On Thu, 2015-09-10 at 20:12 +0800, Cao jin wrote: > Enable PCI-e device multifunction hot, just ensure the function 0 > added last, then driver will got the notification to scan all the > function in the slot. > > Signed-off-by: Cao jin > --- > hw/pci/pcie.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 6e28985..61ebefd 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -249,16 +249,20 @@ 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. > + /* To enable multifunction hot-plug, we just ensure the function > + * 0 added last. Until function 0 added, we set the sltsta and > + * inform OS via event notification. > */ > - 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 | PCI_EXP_HP_EV_ABP); > + if (!(pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) && > + PCI_FUNC(pci_dev->devfn) > 0) { The PCI spec is actually not entirely clear about whether each function in a multifunction device needs to have the multifunction bit set or if only function 0 needs to set it. From a discovery perspective, a kernel should only scan for function 0 and only scan non-zero funcions if function 0 reports the multifunction bit set. Therefore, it doesn't particularly matter what non-zero functions report for the multifunction bit. QEMU allows either interpretation (see comment in pci_init_multifunction), so this appears to be a new requirement that breaks assumptions elsewhere. Thanks, Alex > + error_setg(errp, "single function device, function number must be 0," > + "but got %d", PCI_FUNC(pci_dev->devfn)); > + } else if (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 | PCI_EXP_HP_EV_ABP); > + } > } > > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,