crypto: move common bits for all emulators to libqemuutil

[Qemu-devel] [PATCH] crypto: move common bits for all emulators to libqemuutil

Posted by Paolo Bonzini 4 years, 8 months ago

qcrypto_random_*, AES and qcrypto_init do not need to be linked as a whole
and are the only parts that are used by user-mode emulation.  Place them
in libqemuutil, so that whatever needs them will pick them up automatically.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS                         | 3 +++
 Makefile                            | 4 +---
 Makefile.objs                       | 1 -
 Makefile.target                     | 2 --
 crypto/Makefile.objs                | 7 -------
 util/Makefile.objs                  | 5 +++++
 {crypto => util}/aes.c              | 0
 crypto/init.c => util/crypto-init.c | 0
 {crypto => util}/random-gcrypt.c    | 0
 {crypto => util}/random-gnutls.c    | 0
 {crypto => util}/random-platform.c  | 0
 11 files changed, 9 insertions(+), 13 deletions(-)
 rename {crypto => util}/aes.c (100%)
 rename crypto/init.c => util/crypto-init.c (100%)
 rename {crypto => util}/random-gcrypt.c (100%)
 rename {crypto => util}/random-gnutls.c (100%)
 rename {crypto => util}/random-platform.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index cad58b9..c8feaeb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2141,6 +2141,9 @@ F: tests/test-crypto-*
 F: tests/benchmark-crypto-*
 F: tests/crypto-tls-*
 F: tests/pkix_asn1_tab.c
+F: util/aes.c
+F: util/crypto-init.c
+F: util/random-*.c
 F: qemu.sasl
 
 Coroutines
diff --git a/Makefile b/Makefile
index dfd158c..73fbba0 100644
--- a/Makefile
+++ b/Makefile
@@ -425,7 +425,6 @@ dummy := $(call unnest-vars,, \
                 block-obj-y \
                 block-obj-m \
                 crypto-obj-y \
-                crypto-user-obj-y \
                 qom-obj-y \
                 io-obj-y \
                 common-obj-y \
@@ -498,8 +497,7 @@ subdir-capstone: .git-submodule-status
 subdir-slirp: .git-submodule-status
 	$(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)")
 
-$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) \
-	$(qom-obj-y) $(crypto-user-obj-$(CONFIG_USER_ONLY))
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(qom-obj-y)
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 # Only keep -O and -g cflags
diff --git a/Makefile.objs b/Makefile.objs
index 658cfc9..4d7e49e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -25,7 +25,6 @@ block-obj-m = block/
 # crypto-obj-y is code used by both qemu system emulation and qemu-img
 
 crypto-obj-y = crypto/
-crypto-user-obj-y = crypto/
 
 #######################################################################
 # qom-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/Makefile.target b/Makefile.target
index 72c267f..6737fea 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -181,7 +181,6 @@ dummy := $(call unnest-vars,.., \
                block-obj-m \
                chardev-obj-y \
                crypto-obj-y \
-               crypto-user-obj-y \
                qom-obj-y \
                io-obj-y \
                common-obj-y \
@@ -190,7 +189,6 @@ all-obj-y += $(common-obj-y)
 all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
-all-obj-$(CONFIG_USER_ONLY) += $(crypto-user-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 7fe2fa9..e17f3fb 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -19,10 +19,6 @@ crypto-obj-y += tlscredspsk.o
 crypto-obj-y += tlscredsx509.o
 crypto-obj-y += tlssession.o
 crypto-obj-y += secret.o
-crypto-rng-obj-$(CONFIG_GCRYPT) += random-gcrypt.o
-crypto-rng-obj-$(if $(CONFIG_GCRYPT),n,$(CONFIG_GNUTLS)) += random-gnutls.o
-crypto-rng-obj-$(if $(CONFIG_GCRYPT),n,$(if $(CONFIG_GNUTLS),n,y)) += random-platform.o
-crypto-obj-y += $(crypto-rng-obj-y)
 crypto-obj-y += pbkdf.o
 crypto-obj-$(CONFIG_NETTLE) += pbkdf-nettle.o
 crypto-obj-$(if $(CONFIG_NETTLE),n,$(CONFIG_GCRYPT)) += pbkdf-gcrypt.o
@@ -36,7 +32,4 @@ crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 
-# Let the userspace emulators avoid linking stuff they won't use.
-crypto-user-obj-y = aes.o $(crypto-rng-obj-y) init.o
-
 stub-obj-y += pbkdf-stub.o
diff --git a/util/Makefile.objs b/util/Makefile.objs
index a817ced..5e80fd9 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -57,3 +57,8 @@ util-obj-$(CONFIG_POSIX) += drm.o
 util-obj-y += guest-random.o
 
 stub-obj-y += filemonitor-stub.o
+
+util-obj-$(CONFIG_GCRYPT) += random-gcrypt.o
+util-obj-$(if $(CONFIG_GCRYPT),n,$(CONFIG_GNUTLS)) += random-gnutls.o
+util-obj-$(if $(CONFIG_GCRYPT),n,$(if $(CONFIG_GNUTLS),n,y)) += random-platform.o
+util-obj-y += aes.o crypto-init.o
diff --git a/crypto/aes.c b/util/aes.c
similarity index 100%
rename from crypto/aes.c
rename to util/aes.c
diff --git a/crypto/init.c b/util/crypto-init.c
similarity index 100%
rename from crypto/init.c
rename to util/crypto-init.c
diff --git a/crypto/random-gcrypt.c b/util/random-gcrypt.c
similarity index 100%
rename from crypto/random-gcrypt.c
rename to util/random-gcrypt.c
diff --git a/crypto/random-gnutls.c b/util/random-gnutls.c
similarity index 100%
rename from crypto/random-gnutls.c
rename to util/random-gnutls.c
diff --git a/crypto/random-platform.c b/util/random-platform.c
similarity index 100%
rename from crypto/random-platform.c
rename to util/random-platform.c
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] crypto: move common bits for all emulators to libqemuutil

Posted by Daniel P. Berrangé 4 years, 8 months ago

On Tue, Aug 06, 2019 at 09:11:15AM +0200, Paolo Bonzini wrote:
> qcrypto_random_*, AES and qcrypto_init do not need to be linked as a whole
> and are the only parts that are used by user-mode emulation.  Place them
> in libqemuutil, so that whatever needs them will pick them up automatically.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  MAINTAINERS                         | 3 +++
>  Makefile                            | 4 +---
>  Makefile.objs                       | 1 -
>  Makefile.target                     | 2 --
>  crypto/Makefile.objs                | 7 -------
>  util/Makefile.objs                  | 5 +++++
>  {crypto => util}/aes.c              | 0
>  crypto/init.c => util/crypto-init.c | 0
>  {crypto => util}/random-gcrypt.c    | 0
>  {crypto => util}/random-gnutls.c    | 0
>  {crypto => util}/random-platform.c  | 0

Ewww, definitely do not want to see these files moved as it spreads the
crypto related code over multiple locations again, which is exactly what
I spent time fixing when introducing the crypto/ directory.

Placing them to libqemuutil.a shouldn't mean we need to move the code too.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] crypto: move common bits for all emulators to libqemuutil

Posted by Paolo Bonzini 4 years, 8 months ago

On 06/08/19 11:41, Daniel P. Berrangé wrote:
> On Tue, Aug 06, 2019 at 09:11:15AM +0200, Paolo Bonzini wrote:
>> qcrypto_random_*, AES and qcrypto_init do not need to be linked as a whole
>> and are the only parts that are used by user-mode emulation.  Place them
>> in libqemuutil, so that whatever needs them will pick them up automatically.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  MAINTAINERS                         | 3 +++
>>  Makefile                            | 4 +---
>>  Makefile.objs                       | 1 -
>>  Makefile.target                     | 2 --
>>  crypto/Makefile.objs                | 7 -------
>>  util/Makefile.objs                  | 5 +++++
>>  {crypto => util}/aes.c              | 0
>>  crypto/init.c => util/crypto-init.c | 0
>>  {crypto => util}/random-gcrypt.c    | 0
>>  {crypto => util}/random-gnutls.c    | 0
>>  {crypto => util}/random-platform.c  | 0
> 
> Ewww, definitely do not want to see these files moved as it spreads the
> crypto related code over multiple locations again, which is exactly what
> I spent time fixing when introducing the crypto/ directory.
> 
> Placing them to libqemuutil.a shouldn't mean we need to move the code too.

Fair enough, will do in v2.

Paolo

Re: [Qemu-devel] [PATCH] crypto: move common bits for all emulators to libqemuutil

Posted by Daniel P. Berrangé 4 years, 8 months ago

On Tue, Aug 06, 2019 at 03:07:40PM +0200, Paolo Bonzini wrote:
> On 06/08/19 11:41, Daniel P. Berrangé wrote:
> > On Tue, Aug 06, 2019 at 09:11:15AM +0200, Paolo Bonzini wrote:
> >> qcrypto_random_*, AES and qcrypto_init do not need to be linked as a whole
> >> and are the only parts that are used by user-mode emulation.  Place them
> >> in libqemuutil, so that whatever needs them will pick them up automatically.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>  MAINTAINERS                         | 3 +++
> >>  Makefile                            | 4 +---
> >>  Makefile.objs                       | 1 -
> >>  Makefile.target                     | 2 --
> >>  crypto/Makefile.objs                | 7 -------
> >>  util/Makefile.objs                  | 5 +++++
> >>  {crypto => util}/aes.c              | 0
> >>  crypto/init.c => util/crypto-init.c | 0
> >>  {crypto => util}/random-gcrypt.c    | 0
> >>  {crypto => util}/random-gnutls.c    | 0
> >>  {crypto => util}/random-platform.c  | 0
> > 
> > Ewww, definitely do not want to see these files moved as it spreads the
> > crypto related code over multiple locations again, which is exactly what
> > I spent time fixing when introducing the crypto/ directory.
> > 
> > Placing them to libqemuutil.a shouldn't mean we need to move the code too.
> 
> Fair enough, will do in v2.

I would love to be able to have a libqemucrypto.a library for everything
in crypto/ really, but the way we do QOM registration via library
constructors breaks this. The ld linker isn't clever enough to see the .o
is used via a constructor so unhelpfully throws away all the .o files
containing QOM objects :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|