Re: [PATCH v1 1/3] powerpc/kernel: Split up pci_bus_add_device
On 9/21/17 3:43 PM, Bjorn Helgaas wrote: On Mon, Sep 18, 2017 at 02:26:49PM -0500, Bryant G. Ly wrote: When enabling SR-IOV one might want to have their own version of starting device drivers for the VFs. This patch allows for SR-IOV callers to use pci_bus_add_virtfn_device instead of generic pci_bus_add_device. When enabling SR-IOV in PSeries architecture the dynamic VFs created within the sriov_configure sysfs call will not load the device driver as firmware will load the device node when the VF device is assigned to the logical partition. So we needed a way to overwrite the way device driver matching is done for virtual functions on powervm. Signed-off-by: Bryant G. LySigned-off-by: Juan J. Alvarez --- drivers/pci/bus.c | 51 ++- drivers/pci/iov.c | 2 +- include/linux/pci.h | 3 +++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index bc56cf19afd3..86daf62c4048 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -302,16 +302,9 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { } void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } -/** - * pci_bus_add_device - start driver for a single device - * @dev: device to add - * - * This adds add sysfs entries and start device drivers - */ -void pci_bus_add_device(struct pci_dev *dev) -{ - int retval; +void pci_bus_add_sysfs_entries(struct pci_dev *dev) +{ /* * Can not put in pci_device_add yet because resources * are not assigned yet for some devices. @@ -321,6 +314,11 @@ void pci_bus_add_device(struct pci_dev *dev) pci_create_sysfs_dev_files(dev); pci_proc_attach_device(dev); pci_bridge_d3_update(dev); +} + +void pci_bus_match_device_driver(struct pci_dev *dev) +{ + int retval; dev->match_driver = true; retval = device_attach(>dev); @@ -333,6 +331,41 @@ void pci_bus_add_device(struct pci_dev *dev) dev->is_added = 1; } + +#ifdef CONFIG_PCI_IOV +void __weak pci_bus_match_virtfn_driver(struct pci_dev *dev) +{ + pci_bus_match_device_driver(dev); +} + +/** + * pci_bus_add_virtfn_device - start driver for a virtual function device + * @dev: device to add + * + * This adds add sysfs entries and start device drivers for + * virtual function devices + * + */ +void pci_bus_add_virtfn_device(struct pci_dev *pdev) +{ + pci_bus_add_sysfs_entries(pdev); + pci_bus_match_virtfn_driver(pdev); +} +EXPORT_SYMBOL_GPL(pci_bus_add_virtfn_device); Is there any way we can avoid adding this new interface? I don't really want a new global exported symbol just to support this highly platform-specific functionality. If we could figure out a way to set dev->match_driver to false instead of true in pci_bus_add_device(), it *looks* like that would be enough to accomplish what you need. Maybe that could be done via a pcibios hook or a new function pointer in struct pci_host_bridge, similar to the swizzle_irq pointer? Yes! I will remove the first two patches in the series and only utilize the third patch but addingpdev->match_driver = -1; under pseries_pcibios_bus_add_device, which is a new function I added for machine dependent bus add of a device. The patch will then require https://patchwork.kernel.org/patch/9882915/ which basically allows for match_driver state to not get changed later under the pci_bus_match due to allowing for the match_driver to be a int vs a bool. I will push up another patch removing the first two patches and adding that extra line. -Bryant +#endif + +/** + * pci_bus_add_device - start driver for a single device + * @dev: device to add + * + * This adds add sysfs entries and start device drivers + */ +void pci_bus_add_device(struct pci_dev *dev) +{ + pci_bus_add_sysfs_entries(dev); + pci_bus_match_device_driver(dev); +} + EXPORT_SYMBOL_GPL(pci_bus_add_device); /** diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index ac41c8be9200..16cc72545847 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -162,7 +162,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset) pci_device_add(virtfn, virtfn->bus); - pci_bus_add_device(virtfn); + pci_bus_add_virtfn_device(virtfn); sprintf(buf, "virtfn%u", id); rc = sysfs_create_link(>dev.kobj, >dev.kobj, buf); if (rc) diff --git a/include/linux/pci.h b/include/linux/pci.h index f68c58a93dd0..39f5c0b4bf23 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -911,6 +911,9 @@ struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn); void pci_device_add(struct pci_dev *dev, struct pci_bus *bus); unsigned int pci_scan_child_bus(struct pci_bus *bus); void pci_bus_add_device(struct pci_dev *dev); +#ifdef CONFIG_PCI_IOV +void pci_bus_add_virtfn_device(struct pci_dev *dev); +#endif void
Re: [PATCH v1 1/3] powerpc/kernel: Split up pci_bus_add_device
On Mon, Sep 18, 2017 at 02:26:49PM -0500, Bryant G. Ly wrote: > When enabling SR-IOV one might want to have their > own version of starting device drivers for the VFs. > This patch allows for SR-IOV callers to use > pci_bus_add_virtfn_device instead of generic > pci_bus_add_device. > > When enabling SR-IOV in PSeries architecture the > dynamic VFs created within the sriov_configure sysfs call > will not load the device driver as firmware will load > the device node when the VF device is assigned to the > logical partition. So we needed a way to overwrite the > way device driver matching is done for virtual functions > on powervm. > > Signed-off-by: Bryant G. Ly> Signed-off-by: Juan J. Alvarez > --- > drivers/pci/bus.c | 51 ++- > drivers/pci/iov.c | 2 +- > include/linux/pci.h | 3 +++ > 3 files changed, 46 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index bc56cf19afd3..86daf62c4048 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -302,16 +302,9 @@ void __weak pcibios_resource_survey_bus(struct pci_bus > *bus) { } > > void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } > > -/** > - * pci_bus_add_device - start driver for a single device > - * @dev: device to add > - * > - * This adds add sysfs entries and start device drivers > - */ > -void pci_bus_add_device(struct pci_dev *dev) > -{ > - int retval; > > +void pci_bus_add_sysfs_entries(struct pci_dev *dev) > +{ > /* >* Can not put in pci_device_add yet because resources >* are not assigned yet for some devices. > @@ -321,6 +314,11 @@ void pci_bus_add_device(struct pci_dev *dev) > pci_create_sysfs_dev_files(dev); > pci_proc_attach_device(dev); > pci_bridge_d3_update(dev); > +} > + > +void pci_bus_match_device_driver(struct pci_dev *dev) > +{ > + int retval; > > dev->match_driver = true; > retval = device_attach(>dev); > @@ -333,6 +331,41 @@ void pci_bus_add_device(struct pci_dev *dev) > > dev->is_added = 1; > } > + > +#ifdef CONFIG_PCI_IOV > +void __weak pci_bus_match_virtfn_driver(struct pci_dev *dev) > +{ > + pci_bus_match_device_driver(dev); > +} > + > +/** > + * pci_bus_add_virtfn_device - start driver for a virtual function device > + * @dev: device to add > + * > + * This adds add sysfs entries and start device drivers for > + * virtual function devices > + * > + */ > +void pci_bus_add_virtfn_device(struct pci_dev *pdev) > +{ > + pci_bus_add_sysfs_entries(pdev); > + pci_bus_match_virtfn_driver(pdev); > +} > +EXPORT_SYMBOL_GPL(pci_bus_add_virtfn_device); Is there any way we can avoid adding this new interface? I don't really want a new global exported symbol just to support this highly platform-specific functionality. If we could figure out a way to set dev->match_driver to false instead of true in pci_bus_add_device(), it *looks* like that would be enough to accomplish what you need. Maybe that could be done via a pcibios hook or a new function pointer in struct pci_host_bridge, similar to the swizzle_irq pointer? > +#endif > + > +/** > + * pci_bus_add_device - start driver for a single device > + * @dev: device to add > + * > + * This adds add sysfs entries and start device drivers > + */ > +void pci_bus_add_device(struct pci_dev *dev) > +{ > + pci_bus_add_sysfs_entries(dev); > + pci_bus_match_device_driver(dev); > +} > + > EXPORT_SYMBOL_GPL(pci_bus_add_device); > > /** > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index ac41c8be9200..16cc72545847 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -162,7 +162,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int > reset) > > pci_device_add(virtfn, virtfn->bus); > > - pci_bus_add_device(virtfn); > + pci_bus_add_virtfn_device(virtfn); > sprintf(buf, "virtfn%u", id); > rc = sysfs_create_link(>dev.kobj, >dev.kobj, buf); > if (rc) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index f68c58a93dd0..39f5c0b4bf23 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -911,6 +911,9 @@ struct pci_dev *pci_scan_single_device(struct pci_bus > *bus, int devfn); > void pci_device_add(struct pci_dev *dev, struct pci_bus *bus); > unsigned int pci_scan_child_bus(struct pci_bus *bus); > void pci_bus_add_device(struct pci_dev *dev); > +#ifdef CONFIG_PCI_IOV > +void pci_bus_add_virtfn_device(struct pci_dev *dev); > +#endif > void pci_read_bridge_bases(struct pci_bus *child); > struct resource *pci_find_parent_resource(const struct pci_dev *dev, > struct resource *res); > -- > 2.11.0 (Apple Git-81) >
Re: [PATCH v1 1/3] powerpc/kernel: Split up pci_bus_add_device
Hi Bryant, [auto build test WARNING on powerpc/next] [also build test WARNING on v4.14-rc1 next-20170919] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Bryant-G-Ly/Prepartion-for-SR-IOV-PowerVM-Enablement/20170920-114754 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/init.h:1: warning: no structured comments found include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id' include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id' kernel/sched/core.c:2080: warning: No description found for parameter 'rf' kernel/sched/core.c:2080: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local' include/linux/wait.h:555: warning: No description found for parameter 'wq' include/linux/wait.h:555: warning: Excess function parameter 'wq_head' description in 'wait_event_interruptible_hrtimeout' include/linux/wait.h:759: warning: No description found for parameter 'wq_head' include/linux/wait.h:759: warning: Excess function parameter 'wq' description in 'wait_event_killable' include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create' kernel/sys.c:1: warning: no structured comments found include/linux/device.h:968: warning: No description found for parameter 'dma_ops' drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found include/linux/iio/iio.h:603: warning: No description found for parameter 'trig_readonly' include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev' include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig' include/linux/device.h:969: warning: No description found for parameter 'dma_ops' drivers/ata/libata-eh.c:1449: warning: No description found for parameter 'link' drivers/ata/libata-eh.c:1449: warning: Excess function parameter 'ap' description in 'ata_eh_done' drivers/ata/libata-eh.c:1590: warning: No description found for parameter 'qc' drivers/ata/libata-eh.c:1590: warning: Excess function parameter 'dev' description in 'ata_eh_request_sense' drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 'cached' description in 'nand_write_page' drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 'cached' description in 'nand_write_page' >> drivers/pci/bus.c:351: warning: No description found for parameter 'pdev' >> drivers/pci/bus.c:351: warning: Excess function parameter 'dev' description >> in 'pci_bus_add_virtfn_device' arch/s390/include/asm/cmb.h:1: warning: no structured comments found drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 'rq' drivers/scsi/constants.c:1: warning: no structured comments found include/linux/usb/gadget.h:230: warning: No description found for parameter 'claimed' include/linux/usb/gadget.h:230: warning: No description found for parameter 'enabled' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_altset_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_stall_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_zlp_not_supp' fs/inode.c:1666: warning: No description found for parameter 'rcu' include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_list' include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode' include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags' include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle' include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved' include/linux/jbd2.h:497: warning: No description found for parameter 'h_type' include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no' include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies' include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits' include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs' include/linux/jbd2.h:1050: warning: No description found
[PATCH v1 1/3] powerpc/kernel: Split up pci_bus_add_device
When enabling SR-IOV one might want to have their own version of starting device drivers for the VFs. This patch allows for SR-IOV callers to use pci_bus_add_virtfn_device instead of generic pci_bus_add_device. When enabling SR-IOV in PSeries architecture the dynamic VFs created within the sriov_configure sysfs call will not load the device driver as firmware will load the device node when the VF device is assigned to the logical partition. So we needed a way to overwrite the way device driver matching is done for virtual functions on powervm. Signed-off-by: Bryant G. LySigned-off-by: Juan J. Alvarez --- drivers/pci/bus.c | 51 ++- drivers/pci/iov.c | 2 +- include/linux/pci.h | 3 +++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index bc56cf19afd3..86daf62c4048 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -302,16 +302,9 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { } void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } -/** - * pci_bus_add_device - start driver for a single device - * @dev: device to add - * - * This adds add sysfs entries and start device drivers - */ -void pci_bus_add_device(struct pci_dev *dev) -{ - int retval; +void pci_bus_add_sysfs_entries(struct pci_dev *dev) +{ /* * Can not put in pci_device_add yet because resources * are not assigned yet for some devices. @@ -321,6 +314,11 @@ void pci_bus_add_device(struct pci_dev *dev) pci_create_sysfs_dev_files(dev); pci_proc_attach_device(dev); pci_bridge_d3_update(dev); +} + +void pci_bus_match_device_driver(struct pci_dev *dev) +{ + int retval; dev->match_driver = true; retval = device_attach(>dev); @@ -333,6 +331,41 @@ void pci_bus_add_device(struct pci_dev *dev) dev->is_added = 1; } + +#ifdef CONFIG_PCI_IOV +void __weak pci_bus_match_virtfn_driver(struct pci_dev *dev) +{ + pci_bus_match_device_driver(dev); +} + +/** + * pci_bus_add_virtfn_device - start driver for a virtual function device + * @dev: device to add + * + * This adds add sysfs entries and start device drivers for + * virtual function devices + * + */ +void pci_bus_add_virtfn_device(struct pci_dev *pdev) +{ + pci_bus_add_sysfs_entries(pdev); + pci_bus_match_virtfn_driver(pdev); +} +EXPORT_SYMBOL_GPL(pci_bus_add_virtfn_device); +#endif + +/** + * pci_bus_add_device - start driver for a single device + * @dev: device to add + * + * This adds add sysfs entries and start device drivers + */ +void pci_bus_add_device(struct pci_dev *dev) +{ + pci_bus_add_sysfs_entries(dev); + pci_bus_match_device_driver(dev); +} + EXPORT_SYMBOL_GPL(pci_bus_add_device); /** diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index ac41c8be9200..16cc72545847 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -162,7 +162,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset) pci_device_add(virtfn, virtfn->bus); - pci_bus_add_device(virtfn); + pci_bus_add_virtfn_device(virtfn); sprintf(buf, "virtfn%u", id); rc = sysfs_create_link(>dev.kobj, >dev.kobj, buf); if (rc) diff --git a/include/linux/pci.h b/include/linux/pci.h index f68c58a93dd0..39f5c0b4bf23 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -911,6 +911,9 @@ struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn); void pci_device_add(struct pci_dev *dev, struct pci_bus *bus); unsigned int pci_scan_child_bus(struct pci_bus *bus); void pci_bus_add_device(struct pci_dev *dev); +#ifdef CONFIG_PCI_IOV +void pci_bus_add_virtfn_device(struct pci_dev *dev); +#endif void pci_read_bridge_bases(struct pci_bus *child); struct resource *pci_find_parent_resource(const struct pci_dev *dev, struct resource *res); -- 2.11.0 (Apple Git-81)