Patchwork [V13,6/7] Introduce --enable-tpm-passthrough configure option

login
register
mail settings
Submitter Stefan Berger
Date Dec. 12, 2011, 7:12 p.m.
Message ID <1323717136-21661-7-git-send-email-stefanb@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/130944/
State New
Headers show

Comments

Stefan Berger - Dec. 12, 2011, 7:12 p.m.
Introduce --enable-tpm-passthrough configure option.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 configure |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)
Anthony Liguori - Dec. 12, 2011, 11:27 p.m.
On 12/12/2011 01:12 PM, Stefan Berger wrote:
> Introduce --enable-tpm-passthrough configure option.
>
> Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
> ---
>   configure |   16 +++++++++++++++-
>   1 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 25995bc..ffb599e 100755
> --- a/configure
> +++ b/configure
> @@ -186,6 +186,7 @@ zlib="yes"
>   guest_agent="yes"
>   libiscsi=""
>   tpm="no"
> +tpm_passthrough="no"

Same as before, please probe for existence.

Regards,

Anthony Liguori

>
>   # parse CC options first
>   for opt do
> @@ -787,11 +788,20 @@ for opt do
>     ;;
>     --enable-tpm) tpm="yes"
>     ;;
> +  --enable-tpm-passthrough) tpm_passthrough="yes"
> +  ;;
>     *) echo "ERROR: unknown option $opt"; show_help="yes"
>     ;;
>     esac
>   done
>
> +if test "$tpm" = "no" ; then
> +    if test "$tpm_passthrough" = "yes"; then
> +        echo "ERROR: --enable-tpm-passthrough requires --enable-tpm"
> +        exit 1
> +    fi
> +fi
> +
>   #
>   # If cpu ~= sparc and  sparc_cpu hasn't been defined, plug in the right
>   # QEMU_CFLAGS/LDFLAGS (assume sparc_v8plus for 32-bit and sparc_v9 for 64-bit)
> @@ -1074,6 +1084,7 @@ echo "  --enable-usb-redir       enable usb network redirection support"
>   echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
>   echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
>   echo "  --enable-tpm             enable TPM support"
> +echo "  --enable-tpm-passthrough enable TPM passthrough driver"
>   echo ""
>   echo "NOTE: The object files are built at the place where configure is launched"
>   exit 1
> @@ -2850,6 +2861,7 @@ echo "OpenGL support    $opengl"
>   echo "libiscsi support  $libiscsi"
>   echo "build guest agent $guest_agent"
>   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"
> @@ -3722,7 +3734,9 @@ fi
>   if test "$tpm" = "yes"; then
>     if test "$target_softmmu" = "yes" ; then
>       if test "$linux" = "yes" ; then
> -      echo "CONFIG_TPM_PASSTHROUGH=y">>  $config_target_mak
> +      if test "$tpm_passthrough" = "yes" ; then
> +        echo "CONFIG_TPM_PASSTHROUGH=y">>  $config_target_mak
> +      fi
>       fi
>       echo "CONFIG_TPM=y">>  $config_host_mak
>     fi
Stefan Berger - Dec. 13, 2011, 12:12 a.m.
On 12/12/2011 06:27 PM, Anthony Liguori wrote:
> On 12/12/2011 01:12 PM, Stefan Berger wrote:
>> Introduce --enable-tpm-passthrough configure option.
>>
>> Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
>> ---
>>   configure |   16 +++++++++++++++-
>>   1 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 25995bc..ffb599e 100755
>> --- a/configure
>> +++ b/configure
>> @@ -186,6 +186,7 @@ zlib="yes"
>>   guest_agent="yes"
>>   libiscsi=""
>>   tpm="no"
>> +tpm_passthrough="no"
>
> Same as before, please probe for existence.


We would be probing for /dev/tpm0. Is that really what we want that this 
driver only gets compiled if /dev/tpm0 is (currently) available?

  Regards,
      Stefan
Paul Brook - Dec. 13, 2011, 4:51 a.m.
> >> +tpm_passthrough="no"
> > 
> > Same as before, please probe for existence.
> 
> We would be probing for /dev/tpm0. Is that really what we want that this
> driver only gets compiled if /dev/tpm0 is (currently) available?

If what you say is true then this code should always be enabled.

Paul
Stefan Berger - Dec. 13, 2011, 12:51 p.m.
On 12/12/2011 11:51 PM, Paul Brook wrote:
>>>> +tpm_passthrough="no"
>>> Same as before, please probe for existence.
>> We would be probing for /dev/tpm0. Is that really what we want that this
>> driver only gets compiled if /dev/tpm0 is (currently) available?
> If what you say is true then this code should always be enabled.
>
Michael Tsirkin previously requested that there be an option for the TPM 
passthrough driver to be selectively enabled since at least using 
/dev/tpm0 may not be what everybody wants. The passthrough driver at 
some point will also be able to use sockets to communicate with a TPM 
when a file descriptor is passed to Qemu, so maybe that changes then?


    Stefan
Michael S. Tsirkin - Dec. 13, 2011, 1:51 p.m.
On Tue, Dec 13, 2011 at 07:51:17AM -0500, Stefan Berger wrote:
> On 12/12/2011 11:51 PM, Paul Brook wrote:
> >>>>+tpm_passthrough="no"
> >>>Same as before, please probe for existence.
> >>We would be probing for /dev/tpm0. Is that really what we want that this
> >>driver only gets compiled if /dev/tpm0 is (currently) available?
> >If what you say is true then this code should always be enabled.
> >
> Michael Tsirkin previously requested that there be an option for the
> TPM passthrough driver to be selectively enabled since at least
> using /dev/tpm0 may not be what everybody wants. The passthrough
> driver at some point will also be able to use sockets to communicate
> with a TPM when a file descriptor is passed to Qemu, so maybe that
> changes then?
> 
> 
>    Stefan

The passthrough as it is, is pretty easy to misuse.
This is a hardware problem, not software, and
I don't think it's fixable.

So I do not think all downstreams will want to support this
mode, making it easy to disable this is IMO important.
Paul Brook - Dec. 13, 2011, 5:25 p.m.
> On 12/12/2011 11:51 PM, Paul Brook wrote:
> >>>> +tpm_passthrough="no"
> >>> 
> >>> Same as before, please probe for existence.
> >> 
> >> We would be probing for /dev/tpm0. Is that really what we want that this
> >> driver only gets compiled if /dev/tpm0 is (currently) available?
> > 
> > If what you say is true then this code should always be enabled.
> 
> Michael Tsirkin previously requested that there be an option for the TPM
> passthrough driver to be selectively enabled since at least using
> /dev/tpm0 may not be what everybody wants. The passthrough driver at
> some point will also be able to use sockets to communicate with a TPM
> when a file descriptor is passed to Qemu, so maybe that changes then?

Surely that's a runtime decision made by the qemu user, not a compile time 
decision made by the distribution vendor.  Testing functionality of the build 
machine is fundamentally wrong.  At best it completely breaks cross compiling, 
at worst it subtly breaks building on a dev machine and running on a 
production server.

Configure time probing only makes sense for code that will fail to build if 
external compile time dependencies are not present.

If you really think building qemu without this functionality is useful, then 
maybe add a configure option to disable it.  With the possible exception of 
developer debugging aids, code that is not enabled by default is clearly not 
worth having and should be removed altogether.

Paul
Anthony Liguori - Dec. 13, 2011, 5:41 p.m.
On 12/13/2011 07:51 AM, Michael S. Tsirkin wrote:
> On Tue, Dec 13, 2011 at 07:51:17AM -0500, Stefan Berger wrote:
>> On 12/12/2011 11:51 PM, Paul Brook wrote:
>>>>>> +tpm_passthrough="no"
>>>>> Same as before, please probe for existence.
>>>> We would be probing for /dev/tpm0. Is that really what we want that this
>>>> driver only gets compiled if /dev/tpm0 is (currently) available?
>>> If what you say is true then this code should always be enabled.
>>>
>> Michael Tsirkin previously requested that there be an option for the
>> TPM passthrough driver to be selectively enabled since at least
>> using /dev/tpm0 may not be what everybody wants. The passthrough
>> driver at some point will also be able to use sockets to communicate
>> with a TPM when a file descriptor is passed to Qemu, so maybe that
>> changes then?
>>
>>
>>     Stefan
>
> The passthrough as it is, is pretty easy to misuse.
> This is a hardware problem, not software, and
> I don't think it's fixable.

Can you elaborate?  And can this be documented such that users are aware of this.

Regards,

Anthony Liguori

>
> So I do not think all downstreams will want to support this
> mode, making it easy to disable this is IMO important.
>
Stefan Berger - Dec. 13, 2011, 5:48 p.m.
On 12/13/2011 12:41 PM, Anthony Liguori wrote:
> On 12/13/2011 07:51 AM, Michael S. Tsirkin wrote:
>> On Tue, Dec 13, 2011 at 07:51:17AM -0500, Stefan Berger wrote:
>>> On 12/12/2011 11:51 PM, Paul Brook wrote:
>>>>>>> +tpm_passthrough="no"
>>>>>> Same as before, please probe for existence.
>>>>> We would be probing for /dev/tpm0. Is that really what we want 
>>>>> that this
>>>>> driver only gets compiled if /dev/tpm0 is (currently) available?
>>>> If what you say is true then this code should always be enabled.
>>>>
>>> Michael Tsirkin previously requested that there be an option for the
>>> TPM passthrough driver to be selectively enabled since at least
>>> using /dev/tpm0 may not be what everybody wants. The passthrough
>>> driver at some point will also be able to use sockets to communicate
>>> with a TPM when a file descriptor is passed to Qemu, so maybe that
>>> changes then?
>>>
>>>
>>>     Stefan
>>
>> The passthrough as it is, is pretty easy to misuse.
>> This is a hardware problem, not software, and
>> I don't think it's fixable.
>
> Can you elaborate?  And can this be documented such that users are 
> aware of this.
>

 From qemu-doc.html:

Some notes about using the host's TPM with the passthrough driver:

The TPM device accessed by the passthrough driver must not be used by 
any other application on the host.

Since the host's firmware (BIOS/UEFI) has already initialized the TPM, 
the VM's firmware (BIOS/UEFI) will not be able to initialize the TPM 
again and may therefore not show a TPM-specific menu that would 
otherwise allow the user to configure the TPM, e.g., allow the user to 
enable/disable or activate/deactivate the TPM. Further, if TPM ownership 
is released from within a VM then the host's TPM will get disabled and 
deactivated. To enable and activate the TPM again afterwards, the host 
has to be rebooted and the user is required to enter the firmware's menu 
to enable and activate the TPM. If the TPM is left disabled and/or 
deactivated most TPM commands will fail.
> Regards,
>
> Anthony Liguori
>

Stefan
Paul Brook - Dec. 13, 2011, 8:33 p.m.
> The TPM device accessed by the passthrough driver must not be used by
> any other application on the host.
> 
> Since the host's firmware (BIOS/UEFI) has already initialized the TPM,
> the VM's firmware (BIOS/UEFI) will not be able to initialize the TPM
> again and may therefore not show a TPM-specific menu that would
> otherwise allow the user to configure the TPM, e.g., allow the user to
> enable/disable or activate/deactivate the TPM. Further, if TPM ownership
> is released from within a VM then the host's TPM will get disabled and
> deactivated. To enable and activate the TPM again afterwards, the host
> has to be rebooted and the user is required to enter the firmware's menu
> to enable and activate the TPM. If the TPM is left disabled and/or
> deactivated most TPM commands will fail.

Presumably the same is true of any other application that has access to 
/dev/tpm0?

This doesn't sound any different to any other kind of device passthrough (e.g. 
USB).  If the host is also talking to the device then it's likely to get 
confused.  If the guest tells the device to do something suicidal then it'll 
most likely die.

Complicated of cource by the fact that the whole point of the TPM is you don't 
really trust yourself, so the usual failure mode is a complete nervous 
breakdown.  But that comes with the territory.

Paul

Patch

diff --git a/configure b/configure
index 25995bc..ffb599e 100755
--- a/configure
+++ b/configure
@@ -186,6 +186,7 @@  zlib="yes"
 guest_agent="yes"
 libiscsi=""
 tpm="no"
+tpm_passthrough="no"
 
 # parse CC options first
 for opt do
@@ -787,11 +788,20 @@  for opt do
   ;;
   --enable-tpm) tpm="yes"
   ;;
+  --enable-tpm-passthrough) tpm_passthrough="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
 done
 
+if test "$tpm" = "no" ; then
+    if test "$tpm_passthrough" = "yes"; then
+        echo "ERROR: --enable-tpm-passthrough requires --enable-tpm"
+        exit 1
+    fi
+fi
+
 #
 # If cpu ~= sparc and  sparc_cpu hasn't been defined, plug in the right
 # QEMU_CFLAGS/LDFLAGS (assume sparc_v8plus for 32-bit and sparc_v9 for 64-bit)
@@ -1074,6 +1084,7 @@  echo "  --enable-usb-redir       enable usb network redirection support"
 echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
 echo "  --enable-tpm             enable TPM support"
+echo "  --enable-tpm-passthrough enable TPM passthrough driver"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2850,6 +2861,7 @@  echo "OpenGL support    $opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 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"
@@ -3722,7 +3734,9 @@  fi
 if test "$tpm" = "yes"; then
   if test "$target_softmmu" = "yes" ; then
     if test "$linux" = "yes" ; then
-      echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_target_mak
+      if test "$tpm_passthrough" = "yes" ; then
+        echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_target_mak
+      fi
     fi
     echo "CONFIG_TPM=y" >> $config_host_mak
   fi