Re: [PATCH] hw/crypto: add Allwinner sun4i-ss crypto device
Le Mon, Apr 25, 2022 at 03:29:32PM +0100, Daniel P. Berrangé a écrit : > On Mon, Apr 25, 2022 at 03:03:11PM +0200, LABBE Corentin wrote: > > diff --git a/crypto/hash-nettle.c b/crypto/hash-nettle.c > > index 1ca1a41062..b9342b4fe1 100644 > > --- a/crypto/hash-nettle.c > > +++ b/crypto/hash-nettle.c > > @@ -26,10 +26,22 @@ > > #include > > #include > > > > +#ifndef nettle_sha256_compress > > +#define nettle_sha256_compress _nettle_sha256_compress > > +void _nettle_sha256_compress(uint32_t *state, const uint8_t *input, const > > uint32_t *k); > > +#endif > > + > > +#ifndef nettle_sha512_compress > > +#define nettle_sha512_compress _nettle_sha512_compress > > +void _nettle_sha512_compress(uint32_t *state, const uint8_t *input, const > > uint32_t *k); > > +#endif > > The 'nettle_sha256_compress' function doesn't exist in any header file > from nettle that I've looked at. > > The '_nettle_sha256_compress' function exists as a symbol in the .so > library, but it is clearly not intended for public usage: > > $ nm -a -D /usr/lib64/libnettle.so | grep sha256_compress > 00027730 T _nettle_sha256_compress@@NETTLE_INTERNAL_8_4 > > So this #define magic is definitely not something we can do. > > IOW, unless I'm missing something, I don't think we can even > use nettle for your desired goal, which leaves us no impl at > all. sha256 is only necessary for the upcomming sun8i-ce, so it do not block sun4i-ss. I try to contact nettle to add necessary headers. So for sun4i-ss, what do you think about my qcrypto_nettle_compress implementation ?
Re: [PATCH] hw/crypto: add Allwinner sun4i-ss crypto device
Le Mon, Apr 25, 2022 at 11:19:16AM +0100, Daniel P. Berrangé a écrit : > On Sun, Apr 24, 2022 at 09:10:23PM +0200, LABBE Corentin wrote: > > Le Thu, Apr 21, 2022 at 01:38:00PM +0100, Peter Maydell a écrit : > > > On Sun, 10 Apr 2022 at 20:12, Corentin Labbe wrote: > > > > > > > > From: Corentin LABBE > > > > > > > > The Allwinner A10 has a cryptographic offloader device which > > > > could be easily emulated. > > > > The emulated device is tested with Linux only as any of BSD does not > > > > support it. > > > > > > > > Signed-off-by: Corentin LABBE > > > > > > Hi; thanks for this patch, and sorry it's taken me a while to get > > > to reviewing it. > > > > > > (Daniel, I cc'd you since this device model is making use of crypto > > > related APIs.) > > > > > > Firstly, a note on patch structure. This is quite a large patch, > > > and I think it would be useful to split it at least into two parts: > > > (1) add the new device model > > > (2) change the allwinner SoC to create that new device > > > > Hello > > > > I will do it for next iteration > > > > > > > > > diff --git a/docs/system/arm/cubieboard.rst > > > > b/docs/system/arm/cubieboard.rst > > > > index 344ff8cef9..7836643ba4 100644 > > > > --- a/docs/system/arm/cubieboard.rst > > > > +++ b/docs/system/arm/cubieboard.rst > > > > @@ -14,3 +14,4 @@ Emulated devices: > > > > - SDHCI > > > > - USB controller > > > > - SATA controller > > > > +- crypto > > > > diff --git a/docs/system/devices/allwinner-sun4i-ss.rst > > > > b/docs/system/devices/allwinner-sun4i-ss.rst > > > > new file mode 100644 > > > > index 00..6e7d2142b5 > > > > --- /dev/null > > > > +++ b/docs/system/devices/allwinner-sun4i-ss.rst > > > > @@ -0,0 +1,31 @@ > > > > +Allwinner sun4i-ss > > > > +== > > > > > > If you create a new rst file in docs, you need to put it into the > > > manual by adding it to some table of contents. Otherwise sphinx > > > will complain when you build the documentation, and users won't be > > > able to find it. (If you pass 'configure' the --enable-docs option > > > that will check that you have everything installed to be able to > > > build the docs.) > > > > > > There are two options here: you can have this document, and > > > add it to the toctree in docs/system/device-emulation.rst, and > > > make the "crypto" bullet point in cubieboard.rst be a hyperlink to > > > the device-emulation.rst file. Or you can compress the information > > > down and put it into orangepi.rst. > > > > > > > +The ``sun4i-ss`` emulates the Allwinner cryptographic offloader > > > > +present on early Allwinner SoCs (A10, A10s, A13, A20, A33) > > > > +In qemu only A10 via the cubieboard machine is supported. > > > > + > > > > +The emulated hardware is capable of handling the following algorithms: > > > > +- SHA1 and MD5 hash algorithms > > > > +- AES/DES/DES3 in CBC/ECB > > > > +- PRNG > > > > + > > > > +The emulated hardware does not handle yet: > > > > +- CTS for AES > > > > +- CTR for AES/DES/DES3 > > > > +- IRQ and DMA mode > > > > +Anyway the Linux driver also does not handle them yet. > > > > + > > > > +The emulation needs a real crypto backend, for the moment only > > > > gnutls/nettle is supported. > > > > +So the device emulation needs qemu to be compiled with optionnal > > > > gnutls. > > > > > > > diff --git a/hw/Kconfig b/hw/Kconfig > > > > index ad20cce0a9..43bd7fc14d 100644 > > > > --- a/hw/Kconfig > > > > +++ b/hw/Kconfig > > > > @@ -6,6 +6,7 @@ source audio/Kconfig > > > > source block/Kconfig > > > > source char/Kconfig > > > > source core/Kconfig > > > > +source crypto/Kconfig > > > > source display/Kconfig > > > > source dma/Kconfig > > > > source gpio/Kconfig > > > > > > I don't think we really need a new subdirectory of hw/ > > > for a single device. If you can find two other devices that > > > already exist in QEMU that would also belong in hw/crypto/ > > > then we can creat
Re: [PATCH] hw/crypto: add Allwinner sun4i-ss crypto device
Le Thu, Apr 21, 2022 at 01:38:00PM +0100, Peter Maydell a écrit : > On Sun, 10 Apr 2022 at 20:12, Corentin Labbe wrote: > > > > From: Corentin LABBE > > > > The Allwinner A10 has a cryptographic offloader device which > > could be easily emulated. > > The emulated device is tested with Linux only as any of BSD does not > > support it. > > > > Signed-off-by: Corentin LABBE > > Hi; thanks for this patch, and sorry it's taken me a while to get > to reviewing it. > > (Daniel, I cc'd you since this device model is making use of crypto > related APIs.) > > Firstly, a note on patch structure. This is quite a large patch, > and I think it would be useful to split it at least into two parts: > (1) add the new device model > (2) change the allwinner SoC to create that new device Hello I will do it for next iteration > > > diff --git a/docs/system/arm/cubieboard.rst b/docs/system/arm/cubieboard.rst > > index 344ff8cef9..7836643ba4 100644 > > --- a/docs/system/arm/cubieboard.rst > > +++ b/docs/system/arm/cubieboard.rst > > @@ -14,3 +14,4 @@ Emulated devices: > > - SDHCI > > - USB controller > > - SATA controller > > +- crypto > > diff --git a/docs/system/devices/allwinner-sun4i-ss.rst > > b/docs/system/devices/allwinner-sun4i-ss.rst > > new file mode 100644 > > index 00..6e7d2142b5 > > --- /dev/null > > +++ b/docs/system/devices/allwinner-sun4i-ss.rst > > @@ -0,0 +1,31 @@ > > +Allwinner sun4i-ss > > +== > > If you create a new rst file in docs, you need to put it into the > manual by adding it to some table of contents. Otherwise sphinx > will complain when you build the documentation, and users won't be > able to find it. (If you pass 'configure' the --enable-docs option > that will check that you have everything installed to be able to > build the docs.) > > There are two options here: you can have this document, and > add it to the toctree in docs/system/device-emulation.rst, and > make the "crypto" bullet point in cubieboard.rst be a hyperlink to > the device-emulation.rst file. Or you can compress the information > down and put it into orangepi.rst. > > > +The ``sun4i-ss`` emulates the Allwinner cryptographic offloader > > +present on early Allwinner SoCs (A10, A10s, A13, A20, A33) > > +In qemu only A10 via the cubieboard machine is supported. > > + > > +The emulated hardware is capable of handling the following algorithms: > > +- SHA1 and MD5 hash algorithms > > +- AES/DES/DES3 in CBC/ECB > > +- PRNG > > + > > +The emulated hardware does not handle yet: > > +- CTS for AES > > +- CTR for AES/DES/DES3 > > +- IRQ and DMA mode > > +Anyway the Linux driver also does not handle them yet. > > + > > +The emulation needs a real crypto backend, for the moment only > > gnutls/nettle is supported. > > +So the device emulation needs qemu to be compiled with optionnal gnutls. > > > diff --git a/hw/Kconfig b/hw/Kconfig > > index ad20cce0a9..43bd7fc14d 100644 > > --- a/hw/Kconfig > > +++ b/hw/Kconfig > > @@ -6,6 +6,7 @@ source audio/Kconfig > > source block/Kconfig > > source char/Kconfig > > source core/Kconfig > > +source crypto/Kconfig > > source display/Kconfig > > source dma/Kconfig > > source gpio/Kconfig > > I don't think we really need a new subdirectory of hw/ > for a single device. If you can find two other devices that > already exist in QEMU that would also belong in hw/crypto/ > then we can create it. Otherwise just put this device in > hw/misc. I plan to add at least one other hw/crypto device (allwinner H3 sun8i-ce). I have another one already ready (rockchip rk3288) but I delay it since there are no related SoC in qemu yet. > > > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > > index 97f3b38019..fd8232b1d4 100644 > > --- a/hw/arm/Kconfig > > +++ b/hw/arm/Kconfig > > @@ -317,6 +317,7 @@ config ALLWINNER_A10 > > select AHCI > > select ALLWINNER_A10_PIT > > select ALLWINNER_A10_PIC > > +select ALLWINNER_CRYPTO_SUN4I_SS > > select ALLWINNER_EMAC > > select SERIAL > > select UNIMP > > diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c > > index 05e84728cb..e9104ee028 100644 > > --- a/hw/arm/allwinner-a10.c > > +++ b/hw/arm/allwinner-a10.c > > @@ -23,6 +23,7 @@ > > #include "hw/misc/unimp.h" > > #include "sysemu/sysemu.h" > > #include "hw/boards.h" > > +#include "hw/crypto/allwinner-sun4i-ss.h" > > #include "hw/usb/hcd-ohci.h" > > > > #define AW_A10_MMC0_BASE0x01c0f000 > > @@ -32,6 +33,7 @@ > > #define AW_A10_EMAC_BASE0x01c0b000 > > #define AW_A10_EHCI_BASE0x01c14000 > > #define AW_A10_OHCI_BASE0x01c14400 > > +#define AW_A10_CRYPTO_BASE 0x01c15000 > > #define AW_A10_SATA_BASE0x01c18000 > > #define AW_A10_RTC_BASE 0x01c20d00 > > > > @@ -48,6 +50,10 @@ static void aw_a10_init(Object *obj) > > > > object_initialize_child(obj, "emac", >emac, TYPE_AW_EMAC); > > > > +#if defined CONFIG_NETTLE > > +object_initialize_child(obj, "crypto", >crypto,
