[RFC PATCH 0/2] Crypto: Remove x86 dependency on QAT drivers

[RFC PATCH 0/2] Crypto: Remove x86 dependency on QAT drivers

[yoan.picchi]

From: Yoan Picchi <yoan.picchi@arm.com>

The QAT acceleration card can be very helpfull for some tasks like dealing
with IPSEC but it is currently restricted to be used only on x86 machine.
Looking at the code we didn't see any reasons why those drivers might not
work on other architectures. We've successfully built all of them on x86,
arm64, arm32, mips64, powerpc64, riscv64 and sparc64.

We also have tested the driver with an Intel Corporation C62x Chipset
QuickAssist Technology (rev 04) PCIe card on an arm64 server. After the numa
patch, it works with the AF_ALG crypto userland interface, allowing us to
encrypt some data with cbc for instance. We've also successfully created
some VF, bound them to DPDK, and used the card this way, thus showing some
real life usecases of x86 do work on arm64 too.

Please let us know if we missed something that would warrants some further
testing.

Andre Przywara (1):
  crypto: qat: replace get_current_node() with numa_node_id()

Yoan Picchi (1):
  Removes the x86 dependency on the QAT drivers

 drivers/crypto/qat/Kconfig                     | 14 +++++++-------
 drivers/crypto/qat/qat_common/adf_common_drv.h |  5 -----
 drivers/crypto/qat/qat_common/qat_algs.c       |  4 ++--
 drivers/crypto/qat/qat_common/qat_asym_algs.c  |  4 ++--
 4 files changed, 11 insertions(+), 16 deletions(-)

-- 
2.25.1

[RFC PATCH 1/2] crypto: qat: replace get_current_node() with numa_node_id()

[yoan.picchi]

From: Andre Przywara <andre.przywara@arm.com>

Currently the QAT driver code uses a self-defined wrapper function
called get_current_node() when it wants to learn the current NUMA node.
This implementation references the topology_physical_package_id[] array,
which more or less coincidentally contains the NUMA node id, at least
on x86.

Because this is not universal, and Linux offers a direct function to
learn the NUMA node ID, replace that function with a call to
numa_node_id(), which would work everywhere.

This fixes the QAT driver operation on arm64 machines.

Reported-by: Yoan Picchi <Yoan.Picchi@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Yoan Picchi <yoan.picchi@arm.com>
---
 drivers/crypto/qat/qat_common/adf_common_drv.h | 5 -----
 drivers/crypto/qat/qat_common/qat_algs.c       | 4 ++--
 drivers/crypto/qat/qat_common/qat_asym_algs.c  | 4 ++--
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_common_drv.h b/drivers/crypto/qat/qat_common/adf_common_drv.h
index e8c9b77c0d66..b582107db67b 100644
--- a/drivers/crypto/qat/qat_common/adf_common_drv.h
+++ b/drivers/crypto/qat/qat_common/adf_common_drv.h
@@ -49,11 +49,6 @@ struct service_hndl {
 	struct list_head list;
 };
 
-static inline int get_current_node(void)
-{
-	return topology_physical_package_id(raw_smp_processor_id());
-}
-
 int adf_service_register(struct service_hndl *service);
 int adf_service_unregister(struct service_hndl *service);
 
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index f998ed58457c..c0ffaebcc8b8 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -618,7 +618,7 @@ static int qat_alg_aead_newkey(struct crypto_aead *tfm, const u8 *key,
 {
 	struct qat_alg_aead_ctx *ctx = crypto_aead_ctx(tfm);
 	struct qat_crypto_instance *inst = NULL;
-	int node = get_current_node();
+	int node = numa_node_id();
 	struct device *dev;
 	int ret;
 
@@ -1042,7 +1042,7 @@ static int qat_alg_skcipher_newkey(struct qat_alg_skcipher_ctx *ctx,
 {
 	struct qat_crypto_instance *inst = NULL;
 	struct device *dev;
-	int node = get_current_node();
+	int node = numa_node_id();
 	int ret;
 
 	inst = qat_crypto_get_instance_node(node);
diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index b0b78445418b..3701eac10bce 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -480,7 +480,7 @@ static int qat_dh_init_tfm(struct crypto_kpp *tfm)
 {
 	struct qat_dh_ctx *ctx = kpp_tfm_ctx(tfm);
 	struct qat_crypto_instance *inst =
-			qat_crypto_get_instance_node(get_current_node());
+			qat_crypto_get_instance_node(numa_node_id());
 
 	if (!inst)
 		return -EINVAL;
@@ -1218,7 +1218,7 @@ static int qat_rsa_init_tfm(struct crypto_akcipher *tfm)
 {
 	struct qat_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct qat_crypto_instance *inst =
-			qat_crypto_get_instance_node(get_current_node());
+			qat_crypto_get_instance_node(numa_node_id());
 
 	if (!inst)
 		return -EINVAL;
-- 
2.25.1

[RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[yoan.picchi]

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>
---
 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

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Ard Biesheuvel]

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?

> ---
>  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
>

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Yoan Picchi]

 >> From: Yoan Picchi <yoan.picchi@arm.com>
 >>
 >> The QAT acceleration card can be very helpfull for some tasks like
 >> dealing with IPSEC but it is currently restricted to be used only on 
x86 machine.
 >> Looking at the code we didn't see any reasons why those drivers might
 >> not work on other architectures. We've successfully built all of them
 >> on x86, arm64, arm32, mips64, powerpc64, riscv64 and sparc64.
 >>
 >> We also have tested the driver with an Intel Corporation C62x Chipset
 >> QuickAssist Technology (rev 04) PCIe card on an arm64 server. After
 >> the numa patch, it works with the AF_ALG crypto userland interface,
 >> allowing us to encrypt some data with cbc for instance. We've also
 >> successfully created some VF, bound them to DPDK, and used the card
 >> this way, thus showing some real life usecases of x86 do work on 
arm64 too.
 >>
 >> Please let us know if we missed something that would warrants some
 >> further testing.
 >Thanks Yoan.
 >
 >Can you please confirm that you tested the driver on the platform you 
reported using a kernel with CONFIG_CRYPTO_MANAGER_DISABLE_TESTS not set 
and CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y and the self test >is passing?
 >You can check it by running
 >    $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | 
sort This should report:
 >    driver       : qat_aes_cbc
 >    driver       : qat_aes_cbc_hmac_sha1
 >    driver       : qat_aes_cbc_hmac_sha256
 >    driver       : qat_aes_cbc_hmac_sha512
 >    driver       : qat_aes_ctr
 >    driver       : qat_aes_xts
 >    driver       : qat-dh
 >    driver       : qat-rsa
 >
 >Note that if you are using the HEAD of cryptodev-2.6 you will have to 
either revert 8893d27ffcaf6ec6267038a177cb87bcde4dd3de or apply
 >https://patchwork.kernel.org/project/linux-crypto/list/?series=639755 
as the algorithms have been temporarily disabled.
 >
 >Regards,
 >
 >--
 >Giovanni

Hi Giovanni.

Thanks for the instructions, I did not know of this test.
I rebuilt my kernel on arm64 with those parameter and I confirm I get 
the same output with
$ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort

Kindly,
Yoan

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Giovanni Cabiddu]

On Wed, May 18, 2022 at 02:00:40PM +0100, Yoan Picchi wrote:
> >> From: Yoan Picchi <yoan.picchi@arm.com>
> >>
> >> The QAT acceleration card can be very helpfull for some tasks like
> >> dealing with IPSEC but it is currently restricted to be used only on x86
> machine.
> >> Looking at the code we didn't see any reasons why those drivers might
> >> not work on other architectures. We've successfully built all of them
> >> on x86, arm64, arm32, mips64, powerpc64, riscv64 and sparc64.
> >>
> >> We also have tested the driver with an Intel Corporation C62x Chipset
> >> QuickAssist Technology (rev 04) PCIe card on an arm64 server. After
> >> the numa patch, it works with the AF_ALG crypto userland interface,
> >> allowing us to encrypt some data with cbc for instance. We've also
> >> successfully created some VF, bound them to DPDK, and used the card
> >> this way, thus showing some real life usecases of x86 do work on arm64
> too.
> >>
> >> Please let us know if we missed something that would warrants some
> >> further testing.
> >Thanks Yoan.
> >
> >Can you please confirm that you tested the driver on the platform you
> reported using a kernel with CONFIG_CRYPTO_MANAGER_DISABLE_TESTS not set and
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y and the self test >is passing?
> >You can check it by running
> >    $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort
> This should report:
> >    driver       : qat_aes_cbc
> >    driver       : qat_aes_cbc_hmac_sha1
> >    driver       : qat_aes_cbc_hmac_sha256
> >    driver       : qat_aes_cbc_hmac_sha512
> >    driver       : qat_aes_ctr
> >    driver       : qat_aes_xts
> >    driver       : qat-dh
> >    driver       : qat-rsa
> >
> >Note that if you are using the HEAD of cryptodev-2.6 you will have to
> either revert 8893d27ffcaf6ec6267038a177cb87bcde4dd3de or apply
> >https://patchwork.kernel.org/project/linux-crypto/list/?series=639755 as
> the algorithms have been temporarily disabled.
> >
> >Regards,
> >
> >--
> >Giovanni
> 
> Hi Giovanni.
> 
> Thanks for the instructions, I did not know of this test.
> I rebuilt my kernel on arm64 with those parameter and I confirm I get the
> same output with
> $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort
Thats great. Thanks.

Is the platform where you ran the tests little or big endian?
If little endian, can you re-test on a big endian system?

Thanks,

-- 
Giovanni

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Andre Przywara]

On Tue, 24 May 2022 08:51:31 +0100
Giovanni Cabiddu <giovanni.cabiddu@intel.com> wrote:

Hi,

> On Wed, May 18, 2022 at 02:00:40PM +0100, Yoan Picchi wrote:
> > >> From: Yoan Picchi <yoan.picchi@arm.com>
> > >>
> > >> The QAT acceleration card can be very helpfull for some tasks like
> > >> dealing with IPSEC but it is currently restricted to be used only on x86  
> > machine.  
> > >> Looking at the code we didn't see any reasons why those drivers might
> > >> not work on other architectures. We've successfully built all of them
> > >> on x86, arm64, arm32, mips64, powerpc64, riscv64 and sparc64.
> > >>
> > >> We also have tested the driver with an Intel Corporation C62x Chipset
> > >> QuickAssist Technology (rev 04) PCIe card on an arm64 server. After
> > >> the numa patch, it works with the AF_ALG crypto userland interface,
> > >> allowing us to encrypt some data with cbc for instance. We've also
> > >> successfully created some VF, bound them to DPDK, and used the card
> > >> this way, thus showing some real life usecases of x86 do work on arm64  
> > too.  
> > >>
> > >> Please let us know if we missed something that would warrants some
> > >> further testing.  
> > >Thanks Yoan.
> > >
> > >Can you please confirm that you tested the driver on the platform you  
> > reported using a kernel with CONFIG_CRYPTO_MANAGER_DISABLE_TESTS not set and
> > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y and the self test >is passing?  
> > >You can check it by running
> > >��� $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort  
> > This should report:  
> > >��� driver������ : qat_aes_cbc
> > >��� driver������ : qat_aes_cbc_hmac_sha1
> > >��� driver������ : qat_aes_cbc_hmac_sha256
> > >��� driver������ : qat_aes_cbc_hmac_sha512
> > >��� driver������ : qat_aes_ctr
> > >��� driver������ : qat_aes_xts
> > >��� driver������ : qat-dh
> > >��� driver������ : qat-rsa
> > >
> > >Note that if you are using the HEAD of cryptodev-2.6 you will have to  
> > either revert 8893d27ffcaf6ec6267038a177cb87bcde4dd3de or apply  
> > >https://patchwork.kernel.org/project/linux-crypto/list/?series=639755 as  
> > the algorithms have been temporarily disabled.  
> > >
> > >Regards,
> > >
> > >--
> > >Giovanni  
> > 
> > Hi Giovanni.
> > 
> > Thanks for the instructions, I did not know of this test.
> > I rebuilt my kernel on arm64 with those parameter and I confirm I get the
> > same output with
> > $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort  
> Thats great. Thanks.
> 
> Is the platform where you ran the tests little or big endian?

It's definitely little endian.
The cores in there can be switched between LE and BE, but I think
realistically no one ever runs a BE configuration. Compiling the kernel
for BE is rather straight-forward, but I wouldn't know of any serious
userland to run with it (short of a self-compiled busybox or buildroot).

> If little endian, can you re-test on a big endian system?

So you can just compile a kernel with CONFIG_CPU_BIG_ENDIAN=y, but you
cannot boot it easily, since CONFIG_EFI depends on !CPU_BIG_ENDIAN,
and UEFI is the only way to boot that (server) machine.
So kexec and a KVM guest are the other options. Kexec has the disadvantage of
requiring a DT (because ACPI is also incompatible with BE), and for KVM we
would need to check whether this actually works, since BE guests are much
less tested, plus the device pass-through imposing more challenges.

So testing this in BE is a bit more involved, and the practicality of such
a setup is very questionable. If you are concerned, should we just say:
	depends on PCI && !CPU_BIG_ENDIAN
At least until we have reports that confirm proper BE operation?

Cheers,
Andre

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Giovanni Cabiddu]

On Wed, May 25, 2022 at 02:12:39PM +0100, Andre Przywara wrote:
> So testing this in BE is a bit more involved, and the practicality of such
> a setup is very questionable. If you are concerned, should we just say:
> 	depends on PCI && !CPU_BIG_ENDIAN
> At least until we have reports that confirm proper BE operation?
Sounds reasonable.

Thanks,

-- 
Giovanni

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Andre Przywara]

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?

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.

Cheers,
Andre

> 
> > ---
> >  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
> >  

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Ard Biesheuvel]

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
> > >
>

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Yoan Picchi]

 > 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.

Sorry for the delay, I was away for a few days.
Actually, our previous attempts were with the iommu set to passthrough, 
but I
just tested without the passthrough and it works the same way.
 >
 > > >
 > > > > ---
 > > > >  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
 > > > >
 > >

Re: [RFC PATCH 2/2] Removes the x86 dependency on the QAT drivers

[Ard Biesheuvel]

On Mon, 30 May 2022 at 11:58, Yoan Picchi <yoan.picchi@foss.arm.com> wrote:
>
>  > 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.
>
> Sorry for the delay, I was away for a few days.
> Actually, our previous attempts were with the iommu set to passthrough,
> but I
> just tested without the passthrough and it works the same way.

Thanks for confirming.

So this looks fine to me as far as un-x86-like DMA topologies are
concerned. I do agree that big-endian should be forbidden or tested
thoroughly as well.

Re: [RFC PATCH 0/2] Crypto: Remove x86 dependency on QAT drivers

[Giovanni Cabiddu]

On Mon, May 16, 2022 at 10:16:33AM +0000, yoan.picchi@arm.com wrote:
> From: Yoan Picchi <yoan.picchi@arm.com>
> 
> The QAT acceleration card can be very helpfull for some tasks like dealing
> with IPSEC but it is currently restricted to be used only on x86 machine.
> Looking at the code we didn't see any reasons why those drivers might not
> work on other architectures. We've successfully built all of them on x86,
> arm64, arm32, mips64, powerpc64, riscv64 and sparc64.
> 
> We also have tested the driver with an Intel Corporation C62x Chipset
> QuickAssist Technology (rev 04) PCIe card on an arm64 server. After the numa
> patch, it works with the AF_ALG crypto userland interface, allowing us to
> encrypt some data with cbc for instance. We've also successfully created
> some VF, bound them to DPDK, and used the card this way, thus showing some
> real life usecases of x86 do work on arm64 too.
> 
> Please let us know if we missed something that would warrants some further
> testing.
Thanks Yoan.

Can you please confirm that you tested the driver on the platform you
reported using a kernel with CONFIG_CRYPTO_MANAGER_DISABLE_TESTS not set
and CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y and the self test is passing?
You can check it by running
    $ cat /proc/crypto | grep -B 4 passed | grep -e "qat_\|qat-" | sort
This should report:
    driver       : qat_aes_cbc
    driver       : qat_aes_cbc_hmac_sha1
    driver       : qat_aes_cbc_hmac_sha256
    driver       : qat_aes_cbc_hmac_sha512
    driver       : qat_aes_ctr
    driver       : qat_aes_xts
    driver       : qat-dh
    driver       : qat-rsa

Note that if you are using the HEAD of cryptodev-2.6 you will have to
either revert 8893d27ffcaf6ec6267038a177cb87bcde4dd3de or apply
https://patchwork.kernel.org/project/linux-crypto/list/?series=639755 as
the algorithms have been temporarily disabled.

Regards,

-- 
Giovanni