Hi Suneel,
On Tue, 29 Oct 2019 at 14:08, Suneel Garapati <suneelgli...@gmail.com> wrote:
>
> From: Suneel Garapati <sgarap...@marvell.com>
>
> If SR-IOV capability is present, use it to initialize Virtual function
> (VF) PCI device instances. pci_sriov_init function will read SR-IOV
> registers to create VF devices under the PF PCI device and also bind
> driver if available. This function needs to be invoked from Physical
> function device driver which expects VF device support, creating
> minimal impact on existing framework.
Please expand your abbreviations .
This seems like a nice implementation overall, but it needs a sandbox
test and some documentation.
>
> Signed-off-by: Suneel Garapati <sgarap...@marvell.com>
> ---
> drivers/pci/pci-uclass.c | 113 +++++++++++++++++++++++++++++++++++++++
> include/pci.h | 18 +++++++
> 2 files changed, 131 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 51f7135723..3be49c7115 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1535,6 +1535,119 @@ int dm_pci_flr(struct udevice *dev)
> return 0;
> }
>
is this supported to be exported? It needs a comment.
> +int pci_sriov_init(struct udevice *pdev, int vf_en)
> +{
> + u16 vendor, device;
> + struct udevice *bus;
> + struct udevice *dev;
> + pci_dev_t bdf;
> + u16 ctrl;
> + u16 num_vfs;
> + u16 total_vf;
> + u16 vf_offset;
> + u16 vf_stride;
> + int vf, ret;
> + int pos;
> +
> + pos = dm_pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
> + if (!pos) {
> + printf("Error: SRIOV capability not found\n");
debug()
> + return -ENOENT;
> + }
> +
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_CTRL, &ctrl);
> +
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_TOTAL_VF, &total_vf);
> + if (vf_en > total_vf)
> + vf_en = total_vf;
> + dm_pci_write_config16(pdev, pos + PCI_SRIOV_NUM_VF, vf_en);
> +
> + ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
> + dm_pci_write_config16(pdev, pos + PCI_SRIOV_CTRL, ctrl);
> +
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_NUM_VF, &num_vfs);
> + if (num_vfs > vf_en)
> + num_vfs = vf_en;
> +
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_VF_OFFSET, &vf_offset);
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_VF_STRIDE, &vf_stride);
> +
> + dm_pci_read_config16(pdev, PCI_VENDOR_ID, &vendor);
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_VF_DID, &device);
> +
> + bdf = dm_pci_get_bdf(pdev);
> +
> + pci_get_bus(PCI_BUS(bdf), &bus);
> +
> + if (!bus)
> + return -ENODEV;
> +
> + bdf += PCI_BDF(0, 0, vf_offset);
> +
> + for (vf = 0; vf < num_vfs; vf++) {
This function is way too long - can you please split?
> + struct pci_child_platdata *pplat;
> + ulong class;
> +
> + pci_bus_read_config(bus, bdf, PCI_CLASS_DEVICE,
> + &class, PCI_SIZE_16);
> +
> + debug("%s: bus %d/%s: found VF %x:%x\n", __func__,
> + bus->seq, bus->name, PCI_DEV(bdf), PCI_FUNC(bdf));
> +
> + /* Find this device in the device tree */
> + ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);
> +
> + if (ret == -ENODEV) {
> + struct pci_device_id find_id;
> +
> + memset(&find_id, 0, sizeof(find_id));
'\0'
> +
> + find_id.vendor = vendor;
> + find_id.device = device;
> + find_id.class = class;
> +
> + ret = pci_find_and_bind_driver(bus, &find_id,
> + bdf, &dev);
Can you share any code with pci_bind_bus_devices(), perhaps but
pulling part of it into a function?
> +
> + if (ret)
> + return ret;
> + }
> +
> + /* Update the platform data */
> + pplat = dev_get_parent_platdata(dev);
> + pplat->devfn = PCI_MASK_BUS(bdf);
> + pplat->vendor = vendor;
> + pplat->device = device;
> + pplat->class = class;
> + pplat->is_virtfn = true;
> + pplat->pfdev = pdev;
> + pplat->virtid = vf * vf_stride + vf_offset;
> +
> + debug("%s: bus %d/%s: found VF %x:%x %x:%x class %lx id %x\n",
> + __func__, dev->seq, dev->name, PCI_DEV(bdf),
> + PCI_FUNC(bdf), vendor, device, class, pplat->virtid);
> + bdf += PCI_BDF(0, 0, vf_stride);
> + }
> +
> + return 0;
> +}
> +
> +int pci_sriov_get_totalvfs(struct udevice *pdev)
> +{
> + u16 total_vf;
> + int pos;
> +
> + pos = dm_pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
> + if (!pos) {
> + printf("Error: SRIOV capability not found\n");
debug()
> + return -ENOENT;
> + }
> +
> + dm_pci_read_config16(pdev, pos + PCI_SRIOV_TOTAL_VF, &total_vf);
> +
> + return total_vf;
> +}
> +
> UCLASS_DRIVER(pci) = {
> .id = UCLASS_PCI,
> .name = "pci",
> diff --git a/include/pci.h b/include/pci.h
> index 0b14842285..1343d0e9fb 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -475,6 +475,17 @@
> #define PCI_EXP_DEVCAP_FLR 0x10000000 /* Function Level Reset */
> #define PCI_EXP_DEVCTL 8 /* Device Control */
> #define PCI_EXP_DEVCTL_BCR_FLR 0x8000 /* Bridge Configuration Retry / FLR
> */
> +/* Single Root I/O Virtualization Registers */
> +#define PCI_SRIOV_CAP 0x04 /* SR-IOV Capabilities */
> +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */
> +#define PCI_SRIOV_CTRL_VFE 0x01 /* VF Enable */
> +#define PCI_SRIOV_CTRL_MSE 0x08 /* VF Memory Space Enable */
> +#define PCI_SRIOV_INITIAL_VF 0x0c /* Initial VFs */
> +#define PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */
> +#define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs */
> +#define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */
> +#define PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */
> +#define PCI_SRIOV_VF_DID 0x1a /* VF Device ID */
>
> /* Include the ID list */
>
> @@ -868,6 +879,10 @@ struct pci_child_platdata {
> unsigned short vendor;
> unsigned short device;
> unsigned int class;
> +
> + bool is_virtfn;
> + struct udevice *pfdev;
> + int virtid;
Add comments. You can see them above!
> };
>
> /* PCI bus operations */
> @@ -1178,6 +1193,9 @@ int pci_generic_mmap_read_config(
> ulong *valuep,
> enum pci_size_t size);
>
> +int pci_sriov_init(struct udevice *pdev, int vf_en);
> +int pci_sriov_get_totalvfs(struct udevice *pdev);
Comments!
> +
> #ifdef CONFIG_DM_PCI_COMPAT
> /* Compatibility with old naming */
> static inline int pci_write_config_dword(pci_dev_t pcidev, int offset,
> --
> 2.23.0
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
