Re: [dpdk-dev] [PATCH v2 1/8] crypto/bcmfs: add BCMFS driver

[Vikas Gupta <vikas.gupta@broadcom.com>]

Hi Akhil,

On Tue, Sep 29, 2020 at 12:19 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:
>
> Hi Vikas,
>
> > +BCMFS crypto PMD depend upon the devices present in the path
> > +/sys/bus/platform/devices/fs<version>/<dev_name> on the platform.
> > +Each cryptodev PMD instance can be attached to the nodes present
> > +in the mentioned path.
>
> It would be good, if you can mention the details about the SDKs which need
> To be installed, any kernel dependencies if any.
> The device path mentioned is from which rootfs? This looks incomplete documentation.
Ok sure I`ll add missing items  in next patch set.
>
> > diff --git a/doc/guides/cryptodevs/index.rst b/doc/guides/cryptodevs/index.rst
> > index a67ed5a28..5d7e028bd 100644
> > --- a/doc/guides/cryptodevs/index.rst
> > +++ b/doc/guides/cryptodevs/index.rst
> > @@ -29,3 +29,4 @@ Crypto Device Drivers
> >      qat
> >      virtio
> >      zuc
> > +    bcmfs
>
> It is better to maintain an alphabetical order.
Sure.
>
> > diff --git a/drivers/crypto/bcmfs/bcmfs_device.c
> > b/drivers/crypto/bcmfs/bcmfs_device.c
> > new file mode 100644
> > index 000000000..47c776de6
> > --- /dev/null
> > +++ b/drivers/crypto/bcmfs/bcmfs_device.c
> > @@ -0,0 +1,256 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(C) 2020 Broadcom.
> > + * All rights reserved.
> > + */
> > +
> > +#include <dirent.h>
> > +#include <stdbool.h>
> > +#include <sys/queue.h>
> > +
> > +#include <rte_string_fns.h>
> > +
> > +#include "bcmfs_device.h"
> > +#include "bcmfs_logs.h"
> > +
> > +struct bcmfs_device_attr {
> > +     const char name[BCMFS_MAX_PATH_LEN];
> > +     const char suffix[BCMFS_DEV_NAME_LEN];
> > +     const enum bcmfs_device_type type;
> > +     const uint32_t offset;
> > +     const uint32_t version;
> > +};
> > +
> > +/* BCMFS supported devices */
> > +static struct bcmfs_device_attr dev_table[] = {
> > +     {
> > +             .name = "fs4",
> > +             .suffix = "crypto_mbox",
> > +             .type = BCMFS_SYM_FS4,
> > +             .offset = 0,
> > +             .version = 0x76303031
> > +     },
> > +     {
> > +             .name = "fs5",
> > +             .suffix = "mbox",
> > +             .type = BCMFS_SYM_FS5,
> > +             .offset = 0,
> > +             .version = 0x76303032
> > +     },
> > +     {
> > +             /* sentinel */
> > +     }
> > +};
> > +
> > +TAILQ_HEAD(fsdev_list, bcmfs_device);
> > +static struct fsdev_list fsdev_list = TAILQ_HEAD_INITIALIZER(fsdev_list);
> > +
> > +static struct bcmfs_device *
> > +fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
> > +                    char *dirpath,
> > +                    char *devname,
> > +                    enum bcmfs_device_type dev_type __rte_unused)
> > +{
> > +     struct bcmfs_device *fsdev;
> > +
> > +     fsdev = calloc(1, sizeof(*fsdev));
>
> Can we use rte_calloc
will fix it in next patch set.
>
> > +     if (!fsdev)
> > +             return NULL;
> > +
> > +     if (strlen(dirpath) > sizeof(fsdev->dirname)) {
> > +             BCMFS_LOG(ERR, "dir path name is too long");
> > +             goto cleanup;
> > +     }
> > +
> > +     if (strlen(devname) > sizeof(fsdev->name)) {
> > +             BCMFS_LOG(ERR, "devname is too long");
> > +             goto cleanup;
> > +     }
> > +
> > +     strcpy(fsdev->dirname, dirpath);
> > +     strcpy(fsdev->name, devname);
> > +
> > +     fsdev->vdev = vdev;
> > +
> > +     TAILQ_INSERT_TAIL(&fsdev_list, fsdev, next);
> > +
> > +     return fsdev;
> > +
> > +cleanup:
> > +     free(fsdev);
> > +
> > +     return NULL;
> > +}
> > +
>
> <snip>
>
> > diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build
> > index a2423507a..8e06d0533 100644
> > --- a/drivers/crypto/meson.build
> > +++ b/drivers/crypto/meson.build
> > @@ -23,7 +23,8 @@ drivers = ['aesni_gcm',
> >          'scheduler',
> >          'snow3g',
> >          'virtio',
> > -        'zuc']
> > +        'zuc',
> > +        'bcmfs']
>
> Please maintain an alphabetical order.
Sure.