Message ID | 1445950308-10329-3-git-send-email-caoj.fnst@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
On Tue, 2015-10-27 at 20:51 +0800, Cao jin wrote: > Enable PCIe device multi-function hot-add, just ensure function 0 is added > last, then driver will got the notification to scan the slot. > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > hw/pci/pci.c | 40 +++++++++++++++++++++++++++++++++++++++- > hw/pci/pci_host.c | 13 +++++++++++-- > hw/pci/pcie.c | 18 +++++++++--------- > include/hw/pci/pci.h | 1 + > 4 files changed, 60 insertions(+), 12 deletions(-) > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index 3e26f92..63d7d2f 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,11 @@ 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 || > + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { > return; > } > > @@ -91,7 +96,11 @@ 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 || > + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { > return ~0x0; > } Don't we need to do this in pci_host_config_read_common() and pci_host_config_write_common() instead? Otherwise I don't think we catch config access via mmcfg. Thanks, Alex
Hi Alex On 10/27/2015 10:21 PM, Alex Williamson wrote: > On Tue, 2015-10-27 at 20:51 +0800, Cao jin wrote: >> Enable PCIe device multi-function hot-add, just ensure function 0 is added >> last, then driver will got the notification to scan the slot. >> >> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> >> --- >> hw/pci/pci.c | 40 +++++++++++++++++++++++++++++++++++++++- >> hw/pci/pci_host.c | 13 +++++++++++-- >> hw/pci/pcie.c | 18 +++++++++--------- >> include/hw/pci/pci.h | 1 + >> 4 files changed, 60 insertions(+), 12 deletions(-) > >> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c >> index 3e26f92..63d7d2f 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,11 @@ 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 || >> + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { >> return; >> } >> >> @@ -91,7 +96,11 @@ 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 || >> + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { >> return ~0x0; >> } > > > Don't we need to do this in pci_host_config_read_common() and > pci_host_config_write_common() instead? Otherwise I don't think we > catch config access via mmcfg. Thanks, > Yup...my carelessness, will fix it. Thanks for reminding:) > Alex > > . >
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b0bf540..168b9cc 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -847,6 +847,9 @@ 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); + + pci_dev->bus = bus; if (devfn < 0) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); @@ -864,9 +867,17 @@ 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 && + pci_get_function_0(pci_dev)) { + 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; pci_dev->devfn = devfn; dma_as = pci_device_iommu_address_space(pci_dev); @@ -2454,6 +2465,33 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range) pci_for_each_device_under_bus(bus, pci_dev_get_w64, range); } +static bool pcie_has_upstream_port(PCIDevice *dev) +{ + PCIDevice *parent_dev = pci_bridge_get_device(dev->bus); + + /* Device associated with an upstream port. + * As there are several types of these, it's easier to check the + * parent device: upstream ports are always connected to + * root or downstream ports. + */ + return parent_dev && + pci_is_express(parent_dev) && + parent_dev->exp.exp_cap && + (pcie_cap_get_type(parent_dev) == PCI_EXP_TYPE_ROOT_PORT || + pcie_cap_get_type(parent_dev) == PCI_EXP_TYPE_DOWNSTREAM); +} + +PCIDevice *pci_get_function_0(PCIDevice *pci_dev) +{ + if(pcie_has_upstream_port(pci_dev)) { + /* With an upstream PCIe port, we only support 1 device at slot 0 */ + return pci_dev->bus->devices[0]; + } else { + /* Other bus types might support multiple devices at slots 0-31 */ + return pci_dev->bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]; + } +} + 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..63d7d2f 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,11 @@ 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 || + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { return; } @@ -91,7 +96,11 @@ 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 || + (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev))) { return ~0x0; } diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index a72d516..32c65c2 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. When function 0 is 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_get_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..379b6e1 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); +PCIDevice *pci_get_function_0(PCIDevice *pci_dev); /* Use this wrapper when specific scan order is not required. */ static inline
Enable PCIe device multi-function hot-add, just ensure function 0 is added last, then driver will got the notification to scan the slot. Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- hw/pci/pci.c | 40 +++++++++++++++++++++++++++++++++++++++- hw/pci/pci_host.c | 13 +++++++++++-- hw/pci/pcie.c | 18 +++++++++--------- include/hw/pci/pci.h | 1 + 4 files changed, 60 insertions(+), 12 deletions(-)