[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] PCI-e device multi-function hot-add support
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] PCI-e device multi-function hot-add support |
Date: |
Thu, 10 Sep 2015 09:29:27 -0600 |
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 <address@hidden>
> ---
> 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,