Re: New commands to configure IOV features

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2012-09-20 at 22:50 -0700, Yinghai Lu wrote:
[...]
> Index: linux-2.6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pci.h
> +++ linux-2.6/include/linux/pci.h
> @@ -663,6 +663,7 @@ struct pci_driver {
>         const struct pci_device_id *id_table;   /* must be non-NULL for probe to be called */
>         int  (*probe)  (struct pci_dev *dev, const struct pci_device_id *id);   /* New device inserted */
>         void (*remove) (struct pci_dev *dev);   /* Device removed (NULL if not a hot-plug capable driver) */
> +       void (*set_max_vfs) (struct pci_dev *dev); /* enable sriov */
>         int  (*suspend) (struct pci_dev *dev, pm_message_t state);      /* Device suspended */
>         int  (*suspend_late) (struct pci_dev *dev, pm_message_t state);
>         int  (*resume_early) (struct pci_dev *dev);

Not sure about this; the driver may fail to enable those VFs and it
would be nice to be able to report that failure directly.

That said, this is 'max_vfs' (a limit) and if the driver fails to set up
all the VFs then the *limit* may still be changed.

> Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
> 
> driver later need to check dev->max_vfs in /sys
> 
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
[...]
> +static ssize_t
> +max_vfs_store(struct device *dev, struct device_attribute *attr,
> +                const char *buf, size_t count)
> +{
> +       unsigned long val;
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +
> +       if (strict_strtoul(buf, 0, &val) < 0)
> +               return -EINVAL;
> +
> +       pdev->max_vfs = val;
> +
> +       if (pdev->is_added) {

No locking required here?

> +               int err;
> +               err = device_schedule_callback(dev, max_vfs_callback);

Any particular reason this should be a callback?

[...]
> Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifie
>  };
>  #endif
>  
> -#ifdef CONFIG_PCI_IOV
> -static unsigned int max_vfs;
> -module_param(max_vfs, uint, 0);
> -MODULE_PARM_DESC(max_vfs,
> -                "Maximum number of virtual functions to allocate per
> physical function - default is zero and maximum value is 63");
> -#endif /* CONFIG_PCI_IOV */
> -
>  static unsigned int allow_unsupported_sfp;
>  module_param(allow_unsupported_sfp, uint, 0);
>  MODULE_PARM_DESC(allow_unsupported_sfp,
> @@ -4528,7 +4521,7 @@ static int __devinit ixgbe_sw_init(struc
>  #ifdef CONFIG_PCI_IOV
>         /* assign number of SR-IOV VFs */
>         if (hw->mac.type != ixgbe_mac_82598EB)
> -               adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
> +               adapter->num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;

We are trying to make all SR-IOV capable drivers work the same, so this
weird limiting behaviour should not be retained.

So I think the correct assignment is:
		adapter->num_vfs = min(pdev->max_vfs, 63);

>  #endif
>         /* enable itr by default in dynamic mode */
> @@ -7249,8 +7242,9 @@ static int __devinit ixgbe_probe(struct
>  
>  #ifdef CONFIG_PCI_IOV
>         ixgbe_enable_sriov(adapter, ii);
> -
>  #endif
> +       adapter->ixgbe_info = ii;
> +
>         netdev->features = NETIF_F_SG |
>                            NETIF_F_IP_CSUM |
>                            NETIF_F_IPV6_CSUM |
> @@ -7720,11 +7714,42 @@ static const struct pci_error_handlers i
>         .resume = ixgbe_io_resume,
>  };
>  
> +static void ixgbe_set_max_vfs(struct pci_dev *pdev)
> +{
> +#ifdef CONFIG_PCI_IOV
> +       struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
> +       struct net_device *netdev = adapter->netdev;
> +       struct ixgbe_hw *hw = &adapter->hw;
> +       int num_vfs = 0;
> +
> +       /* assign number of SR-IOV VFs */
> +       if (hw->mac.type != ixgbe_mac_82598EB)
> +               num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;
> +
> +       /* no change */
> +       if (adapter->num_vfs == num_vfs)
> +               return;
> +
> +       if (!num_vfs) {
> +               /* disable sriov */
> +               ixgbe_disable_sriov(adapter);
> +               adapter->num_vfs = 0;
> +       } else if (!adapter->num_vfs && num_vfs) {
> +               /* enable sriov */
> +               adapter->num_vfs = num_vfs;
> +               ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
> +       } else {
> +               /* increase or decrease */

Indeed, increase or decrease is not supported either in our PCI API or
in the SR-IOV spec.  I think I would prefer the PCI core to filter out
unsupported changes (i.e. not call set_max_vfs and maybe report an
error), but I'm not sure about that.

Ben.

> +       }
> +#endif
> +}
> +
[...]

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux