Message-ID: <BYAPR18MB2791983145098BD7F1E22D4CA0100@BYAPR18MB2791.namprd18.prod.outlook.com>
Date:   Mon, 2 Nov 2020 09:34:32 +0000
From:   Srujana Challa <schalla@...vell.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     Herbert Xu <herbert@...dor.apana.org.au>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Geethasowjanya Akula <gakula@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Suheil Chandran <schandran@...vell.com>,
        "Narayana Prasad Raju Athreya" <pathreya@...vell.com>
Subject: RE: [EXT] Re: [PATCH v8,net-next,00/12] Add Support for Marvell
 OcteonTX2

> On Wed, Oct 28, 2020 at 6:50 PM Srujana Challa <schalla@...vell.com> wrote:
> >
> > This series introduces crypto(CPT) drivers(PF & VF) for Marvell OcteonTX2
> > CN96XX Soc.
> >
> > OcteonTX2 SOC's resource virtualization unit (RVU) supports multiple
> > physical and virtual functions. Each of the PF/VF's functionality is
> > determined by what kind of resources are attached to it. When the CPT
> > block is attached to a VF, it can function as a security device.
> > The following document provides an overview of the hardware and
> > different drivers for the OcteonTX2 SOC:
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.kernel.org_doc_Documentation_networking_device-
> 5Fdrivers_marvell_octeontx2.rst&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=Fj
> 4OoD5hcKFpANhTWdwQzjT1Jpf7veC5263T47JVpnc&m=NJ9fKbMEy2o3y4ca5r-
> kkUR9gadv3CZqF3CJSDvXpnM&s=ScxKfEYpF9bGu2bsz15tQSKhM58NJpMe2-
> 7sxjT_yiI&e=
> >
> > The CPT PF driver is responsible for:
> > - Forwarding messages to/from VFs from/to admin function(AF),
> > - Enabling/disabling VFs,
> > - Loading/unloading microcode (creation/deletion of engine groups).
> >
> > The CPT VF driver works as a crypto offload device.
> >
> > This patch series includes:
> > - Patch to update existing Marvell sources to support the CPT driver.
> > - Patch that adds mailbox messages to the admin function (AF) driver,
> > to configure CPT HW registers.
> > - CPT PF driver patches that include AF<=>PF<=>VF mailbox communication,
> > sriov_configure, and firmware load to the acceleration engines.
> > - CPT VF driver patches that include VF<=>PF mailbox communication and
> > crypto offload support through the kernel cryptographic API.
> >
> > This series is tested with CRYPTO_EXTRA_TESTS enabled and
> > CRYPTO_DISABLE_TESTS disabled.
> >
> > Changes since v7:
> >  * Removed writable entries in debugfs.
> >  * Dropped IPsec support.
> > Changes since v6:
> >  * Removed driver version.
> > Changes since v4:
> >  * Rebased the patches onto net-next tree with base
> >    'commit bc081a693a56 ("Merge branch 'Offload-tc-vlan-mangle-to-
> mscc_ocelot-switch'")'
> > Changes since v3:
> >  * Splitup the patches into smaller patches with more informartion.
> > Changes since v2:
> >  * Fixed C=1 warnings.
> >  * Added code to exit CPT VF driver gracefully.
> >  * Moved OcteonTx2 asm code to a header file under include/linux/soc/
> > Changes since v1:
> >  * Moved Makefile changes from patch4 to patch2 and patch3.
> >
> > Srujana Challa (12):
> >   octeontx2-pf: move lmt flush to include/linux/soc
> >   octeontx2-af: add mailbox interface for CPT
> >   octeontx2-af: add debugfs entries for CPT block
> >   drivers: crypto: add Marvell OcteonTX2 CPT PF driver
> >   crypto: octeontx2: add mailbox communication with AF
> >   crypto: octeontx2: enable SR-IOV and mailbox communication with VF
> >   crypto: octeontx2: load microcode and create engine groups
> >   crypto: octeontx2: add LF framework
> >   crypto: octeontx2: add support to get engine capabilities
> >   crypto: octeontx2: add virtual function driver support
> >   crypto: octeontx2: add support to process the crypto request
> >   crypto: octeontx2: register with linux crypto framework
> >
> >  MAINTAINERS                                   |    2 +
> >  drivers/crypto/marvell/Kconfig                |   14 +
> >  drivers/crypto/marvell/Makefile               |    1 +
> >  drivers/crypto/marvell/octeontx2/Makefile     |   10 +
> >  .../marvell/octeontx2/otx2_cpt_common.h       |  123 ++
> >  .../marvell/octeontx2/otx2_cpt_hw_types.h     |  464 +++++
> >  .../marvell/octeontx2/otx2_cpt_mbox_common.c  |  202 ++
> >  .../marvell/octeontx2/otx2_cpt_reqmgr.h       |  197 ++
> >  drivers/crypto/marvell/octeontx2/otx2_cptlf.c |  426 +++++
> >  drivers/crypto/marvell/octeontx2/otx2_cptlf.h |  351 ++++
> >  drivers/crypto/marvell/octeontx2/otx2_cptpf.h |   52 +
> >  .../marvell/octeontx2/otx2_cptpf_main.c       |  570 ++++++
> >  .../marvell/octeontx2/otx2_cptpf_mbox.c       |  331 ++++
> >  .../marvell/octeontx2/otx2_cptpf_ucode.c      | 1533 +++++++++++++++
> >  .../marvell/octeontx2/otx2_cptpf_ucode.h      |  162 ++
> >  drivers/crypto/marvell/octeontx2/otx2_cptvf.h |   28 +
> >  .../marvell/octeontx2/otx2_cptvf_algs.c       | 1665 +++++++++++++++++
> >  .../marvell/octeontx2/otx2_cptvf_algs.h       |  170 ++
> >  .../marvell/octeontx2/otx2_cptvf_main.c       |  401 ++++
> >  .../marvell/octeontx2/otx2_cptvf_mbox.c       |  139 ++
> >  .../marvell/octeontx2/otx2_cptvf_reqmgr.c     |  539 ++++++
> >  .../ethernet/marvell/octeontx2/af/Makefile    |    3 +-
> >  .../net/ethernet/marvell/octeontx2/af/mbox.h  |   33 +
> >  .../net/ethernet/marvell/octeontx2/af/rvu.c   |    2 +-
> >  .../net/ethernet/marvell/octeontx2/af/rvu.h   |    2 +
> >  .../ethernet/marvell/octeontx2/af/rvu_cpt.c   |  229 +++
> >  .../marvell/octeontx2/af/rvu_debugfs.c        |  304 +++
> >  .../ethernet/marvell/octeontx2/af/rvu_reg.h   |   63 +-
> >  .../marvell/octeontx2/nic/otx2_common.h       |   13 +-
> >  include/linux/soc/marvell/octeontx2/asm.h     |   29 +
> >  30 files changed, 8038 insertions(+), 20 deletions(-)
> >  create mode 100644 drivers/crypto/marvell/octeontx2/Makefile
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cpt_common.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cpt_hw_types.h
> >  create mode 100644
> drivers/crypto/marvell/octeontx2/otx2_cpt_mbox_common.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cpt_reqmgr.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptlf.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptlf.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptpf.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptpf_mbox.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf_algs.h
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf_mbox.c
> >  create mode 100644 drivers/crypto/marvell/octeontx2/otx2_cptvf_reqmgr.c
> >  create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> >  create mode 100644 include/linux/soc/marvell/octeontx2/asm.h
> 
> For netdrv review. I don't have a lot of detailed feedback. Overall it
> looks sensible to me.
> 
> It is obviously a big beast of a patchset. Looking at previous review
> comments, the documentation is now a lot better, which helps follow
> the thread.
> 
> The point about parsing tar files remains open. In general error-prone
> parsing is better left to userspace. In practice this is not a lot of
> code, nor terribly complex.
> 
> The other point is that some files are very close to their oncteontx
> counterparts. With minor cleanups, such as using in-place cpu_to_be64s
> swap. Those cleanups probably make sense to the older device, too. One
> option would be to apply them there first, then copy the files and use
> coccinelle for mass renaming. But frankly that is a lot of extra work
> to arrive at the same state. I just used --color-words=. --no-index to
> perform an iterative review, which works well enough.

Thank you for the suggestion. The 2 devices have different capabilities and
our intent is to keep both the drivers separate without having a mutual code
dependency.
And our future hardware version will deviate even more, which reduces the
amount of common code.

Powered by blists - more mailing lists