On Thu, Oct 22, 2015 at 07:57:52PM +0800, Cao jin wrote:
> Enable pcie 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 <caoj.f...@cn.fujitsu.com>
> ---
> hw/pci/pci.c | 29 +++++++++++++++++++++++++++++
> hw/pci/pci_host.c | 11 +++++++++--
> hw/pci/pcie.c | 18 +++++++++---------
> include/hw/pci/pci.h | 1 +
> 4 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b0bf540..c068248 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -847,6 +847,7 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> PCIConfigWriteFunc *config_write = pc->config_write;
> Error *local_err = NULL;
> AddressSpace *dma_as;
> + DeviceState *dev = DEVICE(pci_dev);
>
> if (devfn < 0) {
> for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> @@ -864,6 +865,16 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> PCI_SLOT(devfn), PCI_FUNC(devfn), name,
> bus->devices[devfn]->name);
> return NULL;
> + } else if (dev->hotplugged &&
> + ((!pc->is_express && bus->devices[PCI_DEVFN(PCI_SLOT(devfn),
> 0)]) ||
> + (pc->is_express && bus->devices[0]))) {
So let's change pci_is_function_0 to pci_get_function_0 and reuse here?
> + error_setg(errp, "PCI: slot %d function 0 already ocuppied by %s,"
> + " new func %s cannot be exposed to guest.",
> + PCI_SLOT(devfn),
> + bus->devices[PCI_DEVFN(PCI_SLOT(devfn), 0)]->name,
> + name);
> +
> + return NULL;
> }
>
> pci_dev->bus = bus;
> @@ -2454,6 +2465,24 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range)
> pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
> }
>
> +/* ARI device function number range is 0-255, means has only 1 function0;
> + * while PCI device has 1 function0 in each slot(up to 32 slot) */
> +bool pci_is_function_0(PCIDevice *pci_dev)
> +{
> + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> + bool is_func0 = false;
> +
> + if (pc->is_express) {
This is not what the spec says. It says:
devices connected to an upstream port.
> + if (!pci_dev->devfn)
> + is_func0 = true;
Just do return !pci_dev->devfn here.
> + } else {
> + if(!PCI_FUNC(pci_dev->devfn))
> + is_func0 = true;
> + }
> +
> + return is_func0;
> +}
> +
> static const TypeInfo pci_device_type_info = {
> .name = TYPE_PCI_DEVICE,
> .parent = TYPE_DEVICE,
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 3e26f92..40087c9 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 */
> @@ -75,7 +76,10 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t
> val, int len)
> PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
>
> - if (!pci_dev) {
> + /* non-zero functions are only exposed when function 0 is present,
> + * allowing direct removal of unexposed functions.
> + */
> + if (!pci_dev || !s->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {
Reuse pci_get_function_0 here too?
> return;
> }
>
> @@ -91,7 +95,10 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
> uint32_t val;
>
> - if (!pci_dev) {
> + /* non-zero functions are only exposed when function 0 is present,
> + * allowing direct removal of unexposed functions.
> + */
> + if (!pci_dev || !s->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {
And here?
> return ~0x0;
> }
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index b1adeaf..d0a8006 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -249,16 +249,16 @@ 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.
Should be "when function 0 is added".
> */
> - 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_is_function_0(pci_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 | PCI_EXP_HP_EV_ABP);
> + }
> }
>
> static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index f5e7fd8..2820cfd 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -397,6 +397,7 @@ void pci_for_each_bus_depth_first(PCIBus *bus,
> void *(*begin)(PCIBus *bus, void
> *parent_state),
> void (*end)(PCIBus *bus, void *state),
> void *parent_state);
> +bool pci_is_function_0(PCIDevice *pci_dev);
>
> /* Use this wrapper when specific scan order is not required. */
> static inline
> --
> 2.1.0
