Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220516101635.1082132-1-yoan.picchi@arm.com> <20220516101635.1082132-3-yoan.picchi@arm.com>
 <CAMj1kXGAiA-SkTFD5EgcacYao0RKT7oK0AxvxkR7Ho_KZSGXCw@mail.gmail.com> <20220518165512.3e07c4b3@donnerap.cambridge.arm.com>
In-Reply-To: <20220518165512.3e07c4b3@donnerap.cambridge.arm.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Tue, 24 May 2022 10:04:28 +0200
Message-ID: <CAMj1kXEF6KV=3CXhaRKygBs9hvun7=bKRua5NbWOrksaZBgtCQ@mail.gmail.com>
Subject: Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers
To:     Andre Przywara <andre.przywara@arm.com>
Cc:     yoan.picchi@arm.com, Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S . Miller" <davem@davemloft.net>,
        qat-linux <qat-linux@intel.com>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 18 May 2022 at 17:55, Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Tue, 17 May 2022 10:11:09 +0200
> Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Hi,
>
> > On Mon, 16 May 2022 at 12:16, <yoan.picchi@arm.com> wrote:
> > >
> > > From: Yoan Picchi <yoan.picchi@arm.com>
> > >
> > > This dependency looks outdated. After the previous patch, we have been able
> > > to use this driver to encrypt some data and to create working VF on arm64.
> > >
> > > Signed-off-by: Yoan Picchi <yoan.picchi@arm.com>
> >
> > Are you sure the driver is safe for non-coherent DMA as well?
>
> That depends on your definition of "sure".
> We indeed tested this only on a server with coherent PCIe.
>
> I skimmed through the driver, and it looks like to use the DMA API
> correctly:
> - I see dma_alloc_coherent() calls for DMA ring buffers.
> - There are dma_map_single()/dma_unmap_single() pairs in other parts.
> - Accesses to the BARs are capsuled via macros, using readl/writel.
> - Access the the SRAM BAR is also only done via those macros.
>
> I didn't go through the driver systematically, and of course the
> interesting parts are the ones you don't see easily, so I am eager to hear
> any other opinions on this topic.
>
> Ard, do you have anything special in mind? Is there something to look out
> for, specifically?
>

If it uses the DMA api consistently and correctly, and works as
expected when running under a SMMU, things are probably fine

> The few cards we have access to are in some server in the data centre, so
> I can't easily walk in with, say a RockPro64, and test this there.
>

I suppose this implies that you have tested with SMMUs enabled.

> >
> > > ---
> > >  drivers/crypto/qat/Kconfig | 14 +++++++-------
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/crypto/qat/Kconfig b/drivers/crypto/qat/Kconfig
> > > index 4b90c0f22b03..88df2cf4cac9 100644
> > > --- a/drivers/crypto/qat/Kconfig
> > > +++ b/drivers/crypto/qat/Kconfig
> > > @@ -17,7 +17,7 @@ config CRYPTO_DEV_QAT
> > >
> > >  config CRYPTO_DEV_QAT_DH895xCC
> > >         tristate "Support for Intel(R) DH895xCC"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select CRYPTO_DEV_QAT
> > >         help
> > >           Support for Intel(R) DH895xcc with Intel(R) QuickAssist Technology
> > > @@ -28,7 +28,7 @@ config CRYPTO_DEV_QAT_DH895xCC
> > >
> > >  config CRYPTO_DEV_QAT_C3XXX
> > >         tristate "Support for Intel(R) C3XXX"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select CRYPTO_DEV_QAT
> > >         help
> > >           Support for Intel(R) C3xxx with Intel(R) QuickAssist Technology
> > > @@ -39,7 +39,7 @@ config CRYPTO_DEV_QAT_C3XXX
> > >
> > >  config CRYPTO_DEV_QAT_C62X
> > >         tristate "Support for Intel(R) C62X"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select CRYPTO_DEV_QAT
> > >         help
> > >           Support for Intel(R) C62x with Intel(R) QuickAssist Technology
> > > @@ -50,7 +50,7 @@ config CRYPTO_DEV_QAT_C62X
> > >
> > >  config CRYPTO_DEV_QAT_4XXX
> > >         tristate "Support for Intel(R) QAT_4XXX"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select CRYPTO_DEV_QAT
> > >         help
> > >           Support for Intel(R) QuickAssist Technology QAT_4xxx
> > > @@ -61,7 +61,7 @@ config CRYPTO_DEV_QAT_4XXX
> > >
> > >  config CRYPTO_DEV_QAT_DH895xCCVF
> > >         tristate "Support for Intel(R) DH895xCC Virtual Function"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select PCI_IOV
> > >         select CRYPTO_DEV_QAT
> > >
> > > @@ -74,7 +74,7 @@ config CRYPTO_DEV_QAT_DH895xCCVF
> > >
> > >  config CRYPTO_DEV_QAT_C3XXXVF
> > >         tristate "Support for Intel(R) C3XXX Virtual Function"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select PCI_IOV
> > >         select CRYPTO_DEV_QAT
> > >         help
> > > @@ -86,7 +86,7 @@ config CRYPTO_DEV_QAT_C3XXXVF
> > >
> > >  config CRYPTO_DEV_QAT_C62XVF
> > >         tristate "Support for Intel(R) C62X Virtual Function"
> > > -       depends on X86 && PCI
> > > +       depends on PCI
> > >         select PCI_IOV
> > >         select CRYPTO_DEV_QAT
> > >         help
> > > --
> > > 2.25.1
> > >
>