Patchwork [07/12] configure: fix TPM logic

login
register
mail settings
Submitter Paolo Bonzini
Date April 15, 2013, 1:19 p.m.
Message ID <1366031973-7718-8-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/236607/
State New
Headers show

Comments

Paolo Bonzini - April 15, 2013, 1:19 p.m.
A non-native i386 or x86_64 emulator should not have TPM passthrough
support, since the TPM is only present for those hosts.

Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure                          | 24 ++++++++++++++++++------
 default-configs/i386-softmmu.mak   |  3 +--
 default-configs/x86_64-softmmu.mak |  3 +--
 tpm/Makefile.objs                  |  4 +---
 4 files changed, 21 insertions(+), 13 deletions(-)
Laszlo Ersek - April 15, 2013, 4:02 p.m.
On 04/15/13 15:19, Paolo Bonzini wrote:

> diff --git a/configure b/configure
> index 258c82a..1c1e369 100755
> --- a/configure
> +++ b/configure

> @@ -3436,6 +3445,7 @@ echo "virtio-blk-data-plane $virtio_blk_data_plane"
>  echo "gcov              $gcov_tool"
>  echo "gcov enabled      $gcov"
>  echo "TPM support       $tpm"
> +echo "TPM passthrough   $tpm_passthrough"
>  
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"

(tiny merge conflict with 0a12ec87)

diff --cc configure
index a97bf31,1c1e369..0000000
--- a/configure
+++ b/configure
@@@ -3503,7 -3445,7 +3513,8 @@@ echo "virtio-blk-data-plane $virtio_blk
  echo "gcov              $gcov_tool"
  echo "gcov enabled      $gcov"
  echo "TPM support       $tpm"
+ echo "TPM passthrough   $tpm_passthrough"
 +echo "libssh2 support   $libssh2"

  if test "$sdl_too_old" = "yes"; then
  echo "-> Your SDL version is too old - please upgrade to have SDL support"
Peter Maydell - April 15, 2013, 4:09 p.m.
On 15 April 2013 14:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> +if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then

test -o is deprecated by POSIX; better to use
  if test ... && ( test ... || test ... ); then

in new code.

thanks
-- PMM
Paolo Bonzini - April 15, 2013, 4:14 p.m.
Il 15/04/2013 18:09, Peter Maydell ha scritto:
>> > +if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then
> test -o is deprecated by POSIX; better to use
>   if test ... && ( test ... || test ... ); then
> 
> in new code.

True, on the other hand "(.*test" has no match at all in configure; and
since we know that $cpu does not begin with a dash, it is portable in
practice.

I would agree with you, but it looks like convention trumps the
suggested practice.

Paolo
Peter Maydell - April 15, 2013, 4:21 p.m.
On 15 April 2013 17:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 15/04/2013 18:09, Peter Maydell ha scritto:
>>> > +if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then
>> test -o is deprecated by POSIX; better to use
>>   if test ... && ( test ... || test ... ); then
>>
>> in new code.
>
> True, on the other hand "(.*test" has no match at all in configure; and
> since we know that $cpu does not begin with a dash, it is portable in
> practice.
>
> I would agree with you, but it looks like convention trumps the
> suggested practice.

I've been consistently saying this in code review for
all new shell code for some time now. You just happen
to have the first example of X && (Y || Z); most
of configure's conditionals are not that complex.

-- PMM
Markus Armbruster - April 16, 2013, 8:28 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> A non-native i386 or x86_64 emulator should not have TPM passthrough
> support, since the TPM is only present for those hosts.
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Breaks the build for me:

make: Entering directory `/work/armbru/qemu/bld-x86'
make[1]: *** No rule to make target `../tpm/tpm_tis.o', needed by `qemu-system-x86_64'.
make[1]: Target `all' not remade because of errors.
make: *** [subdir-x86_64-softmmu] Error 2
make: Target `all' not remade because of errors.
make: Leaving directory `/work/armbru/qemu/bld-x86'

Workaround: drop --enable-tpm.
Paolo Bonzini - April 16, 2013, 8:42 a.m.
Il 16/04/2013 10:28, Markus Armbruster ha scritto:
> Breaks the build for me:
> 
> make: Entering directory `/work/armbru/qemu/bld-x86'
> make[1]: *** No rule to make target `../tpm/tpm_tis.o', needed by `qemu-system-x86_64'.
> make[1]: Target `all' not remade because of errors.
> make: *** [subdir-x86_64-softmmu] Error 2
> make: Target `all' not remade because of errors.
> make: Leaving directory `/work/armbru/qemu/bld-x86'
> 
> Workaround: drop --enable-tpm.

I had a stale .o in my build directory.  Sending patch, BTW why isn't
TPM enabled by default?

Paolo

Patch

diff --git a/configure b/configure
index 258c82a..1c1e369 100755
--- a/configure
+++ b/configure
@@ -2337,6 +2337,15 @@  EOF
 fi
 
 ##########################################
+# TPM passthrough is only on x86 Linux
+
+if test "$targetos" = Linux && test "$cpu" = i386 -o "$cpu" = x86_64; then
+  tpm_passthrough=$tpm
+else
+  tpm_passthrough=no
+fi
+
+##########################################
 # adjust virtio-blk-data-plane based on linux-aio
 
 if test "$virtio_blk_data_plane" = "yes" -a \
@@ -3436,6 +3445,7 @@  echo "virtio-blk-data-plane $virtio_blk_data_plane"
 echo "gcov              $gcov_tool"
 echo "gcov enabled      $gcov"
 echo "TPM support       $tpm"
+echo "TPM passthrough   $tpm_passthrough"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3811,6 +3821,14 @@  bsd)
 ;;
 esac
 
+# TPM passthrough support?
+if test "$tpm" = "yes"; then
+  echo 'CONFIG_TPM=$(CONFIG_SOFTMMU)' >> $config_host_mak
+  if test "$tpm_passthrough" = "yes"; then
+    echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
+  fi
+fi
+
 # use default implementation for tracing backend-specific routines
 trace_default=yes
 echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
@@ -4338,12 +4356,6 @@  if test "$gprof" = "yes" ; then
   fi
 fi
 
-if test "$tpm" = "yes"; then
-  if test "$target_softmmu" = "yes" ; then
-    echo "CONFIG_TPM=y" >> $config_host_mak
-  fi
-fi
-
 if test "$ARCH" = "tci"; then
   linker_script=""
 else
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 6d9d364..368a776 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -27,8 +27,7 @@  CONFIG_HPET=y
 CONFIG_APPLESMC=y
 CONFIG_I8259=y
 CONFIG_PFLASH_CFI01=y
-CONFIG_TPM_TIS=y
-CONFIG_TPM_PASSTHROUGH=y
+CONFIG_TPM_TIS=$(CONFIG_TPM)
 CONFIG_PCI_HOTPLUG=y
 CONFIG_MC146818RTC=y
 CONFIG_PAM=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 3b06310..2711b83 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -27,8 +27,7 @@  CONFIG_HPET=y
 CONFIG_APPLESMC=y
 CONFIG_I8259=y
 CONFIG_PFLASH_CFI01=y
-CONFIG_TPM_TIS=y
-CONFIG_TPM_PASSTHROUGH=y
+CONFIG_TPM_TIS=$(CONFIG_TPM)
 CONFIG_PCI_HOTPLUG=y
 CONFIG_MC146818RTC=y
 CONFIG_PAM=y
diff --git a/tpm/Makefile.objs b/tpm/Makefile.objs
index 8676824..366e4a7 100644
--- a/tpm/Makefile.objs
+++ b/tpm/Makefile.objs
@@ -1,6 +1,4 @@ 
 common-obj-y = tpm.o
-ifeq ($(CONFIG_TPM),y)
-common-obj-y += tpm_backend.o
+common-obj-$(CONFIG_TPM) += tpm_backend.o
 common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
 common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
-endif