diff mbox series

[v2,13/13] vtpm: Correct timeout units and command duration

Message ID 20210506135923.161427-14-jandryuk@gmail.com (mailing list archive)
State New, archived
Headers show
Series vtpmmgr: Some fixes - still incomplete | expand

Commit Message

Jason Andryuk May 6, 2021, 1:59 p.m. UTC
Add two patches:
vtpm-microsecond-duration.patch fixes the units for timeouts and command
durations.
vtpm-command-duration.patch increases the timeout linux uses to allow
commands to succeed.

Linux works around low timeouts, but not low durations.  The second
patch allows commands to complete that often timeout with the lower
command durations.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 stubdom/Makefile                        |  2 +
 stubdom/vtpm-command-duration.patch     | 52 +++++++++++++++++++++++++
 stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
 3 files changed, 106 insertions(+)
 create mode 100644 stubdom/vtpm-command-duration.patch
 create mode 100644 stubdom/vtpm-microsecond-duration.patch

Comments

Samuel Thibault May 6, 2021, 9:52 p.m. UTC | #1
Jason Andryuk, le jeu. 06 mai 2021 09:59:23 -0400, a ecrit:
> Add two patches:
> vtpm-microsecond-duration.patch fixes the units for timeouts and command
> durations.
> vtpm-command-duration.patch increases the timeout linux uses to allow
> commands to succeed.
> 
> Linux works around low timeouts, but not low durations.  The second
> patch allows commands to complete that often timeout with the lower
> command durations.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  stubdom/Makefile                        |  2 +
>  stubdom/vtpm-command-duration.patch     | 52 +++++++++++++++++++++++++
>  stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
>  create mode 100644 stubdom/vtpm-command-duration.patch
>  create mode 100644 stubdom/vtpm-microsecond-duration.patch
> 
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index c6de5f68ae..06aa69d8bc 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -239,6 +239,8 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
>  	patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
>  	patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
>  	patch -d $@ -p1 < vtpm_extern.patch
> +	patch -d $@ -p1 < vtpm-microsecond-duration.patch
> +	patch -d $@ -p1 < vtpm-command-duration.patch
>  	mkdir $@/build
>  	cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
>  	touch $@
> diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
> new file mode 100644
> index 0000000000..6fdf2fc9be
> --- /dev/null
> +++ b/stubdom/vtpm-command-duration.patch
> @@ -0,0 +1,52 @@
> +From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:46:34 -0400
> +Subject: [PATCH 2/2] Increase command durations
> +
> +Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
> +failing commands with -ETIME:
> +tpm tpm0: tpm_try_transmit: send(): error-62
> +
> +The vtpm was returning the data, but it was after the duration timeout
> +in vtpm_send.  Linux may have started being more stringent about timing?
> +
> +The vtpm-stubdom has a little delay since it writes its disk before
> +returning the response.
> +
> +Anyway, the durations are rather low.  When they were 1/10/1000 before
> +converting to microseconds, Linux showed all three durations rounded to
> +10000.  Update them with values from a physical TPM1.2.  These were
> +taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
> +They might be excessive, but I'd rather have a command succeed than
> +return -ETIME.
> +
> +An IFX physical TPM1.2 uses:
> +1000000
> +1500000
> +150000000
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index bebaf10..844afca 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -71,9 +71,9 @@ static void init_timeouts(void)
> +   tpmData.permanent.data.tis_timeouts[1] = 2000000;
> +   tpmData.permanent.data.tis_timeouts[2] = 750000;
> +   tpmData.permanent.data.tis_timeouts[3] = 750000;
> +-  tpmData.permanent.data.cmd_durations[0] = 1000;
> +-  tpmData.permanent.data.cmd_durations[1] = 10000;
> +-  tpmData.permanent.data.cmd_durations[2] = 1000000;
> ++  tpmData.permanent.data.cmd_durations[0] = 3000000;
> ++  tpmData.permanent.data.cmd_durations[1] = 3000000;
> ++  tpmData.permanent.data.cmd_durations[2] = 600000000;
> + }
> + 
> + void tpm_init_data(void)
> +-- 
> +2.30.2
> +
> diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
> new file mode 100644
> index 0000000000..7a906e72c5
> --- /dev/null
> +++ b/stubdom/vtpm-microsecond-duration.patch
> @@ -0,0 +1,52 @@
> +From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:42:10 -0400
> +Subject: [PATCH 1/2] Use microseconds for timeouts and durations
> +
> +The timeout and duration fields should be in microseconds according to
> +the spec.
> +
> +TPM_CAP_PROP_TIS_TIMEOUT:
> +A 4 element array of UINT32 values each denoting the timeout value in
> +microseconds for the following in this order:
> +
> +TPM_CAP_PROP_DURATION:
> +A 3 element array of UINT32 values each denoting the duration value in
> +microseconds of the duration of the three classes of commands:
> +
> +Linux will scale the timeouts up by 1000, but not the durations.  Change
> +the units for both sets as appropriate.
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 14 +++++++-------
> + 1 file changed, 7 insertions(+), 7 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index a3a79ef..bebaf10 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -67,13 +67,13 @@ static void init_nv_storage(void)
> + static void init_timeouts(void)
> + {
> +   /* for the timeouts we use the PC platform defaults */
> +-  tpmData.permanent.data.tis_timeouts[0] = 750;
> +-  tpmData.permanent.data.tis_timeouts[1] = 2000;
> +-  tpmData.permanent.data.tis_timeouts[2] = 750;
> +-  tpmData.permanent.data.tis_timeouts[3] = 750;
> +-  tpmData.permanent.data.cmd_durations[0] = 1;
> +-  tpmData.permanent.data.cmd_durations[1] = 10;
> +-  tpmData.permanent.data.cmd_durations[2] = 1000;
> ++  tpmData.permanent.data.tis_timeouts[0] = 750000;
> ++  tpmData.permanent.data.tis_timeouts[1] = 2000000;
> ++  tpmData.permanent.data.tis_timeouts[2] = 750000;
> ++  tpmData.permanent.data.tis_timeouts[3] = 750000;
> ++  tpmData.permanent.data.cmd_durations[0] = 1000;
> ++  tpmData.permanent.data.cmd_durations[1] = 10000;
> ++  tpmData.permanent.data.cmd_durations[2] = 1000000;
> + }
> + 
> + void tpm_init_data(void)
> +-- 
> +2.30.2
> +
> -- 
> 2.30.2
>
Daniel P. Smith May 10, 2021, 1:40 p.m. UTC | #2
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Add two patches:
> vtpm-microsecond-duration.patch fixes the units for timeouts and command
> durations.
> vtpm-command-duration.patch increases the timeout linux uses to allow
> commands to succeed.
> 
> Linux works around low timeouts, but not low durations.  The second
> patch allows commands to complete that often timeout with the lower
> command durations.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---

Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>

>  stubdom/Makefile                        |  2 +
>  stubdom/vtpm-command-duration.patch     | 52 +++++++++++++++++++++++++
>  stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
>  create mode 100644 stubdom/vtpm-command-duration.patch
>  create mode 100644 stubdom/vtpm-microsecond-duration.patch
> 
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index c6de5f68ae..06aa69d8bc 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -239,6 +239,8 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
>  	patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
>  	patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
>  	patch -d $@ -p1 < vtpm_extern.patch
> +	patch -d $@ -p1 < vtpm-microsecond-duration.patch
> +	patch -d $@ -p1 < vtpm-command-duration.patch
>  	mkdir $@/build
>  	cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
>  	touch $@
> diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
> new file mode 100644
> index 0000000000..6fdf2fc9be
> --- /dev/null
> +++ b/stubdom/vtpm-command-duration.patch
> @@ -0,0 +1,52 @@
> +From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:46:34 -0400
> +Subject: [PATCH 2/2] Increase command durations
> +
> +Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
> +failing commands with -ETIME:
> +tpm tpm0: tpm_try_transmit: send(): error-62
> +
> +The vtpm was returning the data, but it was after the duration timeout
> +in vtpm_send.  Linux may have started being more stringent about timing?
> +
> +The vtpm-stubdom has a little delay since it writes its disk before
> +returning the response.
> +
> +Anyway, the durations are rather low.  When they were 1/10/1000 before
> +converting to microseconds, Linux showed all three durations rounded to
> +10000.  Update them with values from a physical TPM1.2.  These were
> +taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
> +They might be excessive, but I'd rather have a command succeed than
> +return -ETIME.
> +
> +An IFX physical TPM1.2 uses:
> +1000000
> +1500000
> +150000000
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index bebaf10..844afca 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -71,9 +71,9 @@ static void init_timeouts(void)
> +   tpmData.permanent.data.tis_timeouts[1] = 2000000;
> +   tpmData.permanent.data.tis_timeouts[2] = 750000;
> +   tpmData.permanent.data.tis_timeouts[3] = 750000;
> +-  tpmData.permanent.data.cmd_durations[0] = 1000;
> +-  tpmData.permanent.data.cmd_durations[1] = 10000;
> +-  tpmData.permanent.data.cmd_durations[2] = 1000000;
> ++  tpmData.permanent.data.cmd_durations[0] = 3000000;
> ++  tpmData.permanent.data.cmd_durations[1] = 3000000;
> ++  tpmData.permanent.data.cmd_durations[2] = 600000000;
> + }
> + 
> + void tpm_init_data(void)
> +-- 
> +2.30.2
> +
> diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
> new file mode 100644
> index 0000000000..7a906e72c5
> --- /dev/null
> +++ b/stubdom/vtpm-microsecond-duration.patch
> @@ -0,0 +1,52 @@
> +From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:42:10 -0400
> +Subject: [PATCH 1/2] Use microseconds for timeouts and durations
> +
> +The timeout and duration fields should be in microseconds according to
> +the spec.
> +
> +TPM_CAP_PROP_TIS_TIMEOUT:
> +A 4 element array of UINT32 values each denoting the timeout value in
> +microseconds for the following in this order:
> +
> +TPM_CAP_PROP_DURATION:
> +A 3 element array of UINT32 values each denoting the duration value in
> +microseconds of the duration of the three classes of commands:
> +
> +Linux will scale the timeouts up by 1000, but not the durations.  Change
> +the units for both sets as appropriate.
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 14 +++++++-------
> + 1 file changed, 7 insertions(+), 7 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index a3a79ef..bebaf10 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -67,13 +67,13 @@ static void init_nv_storage(void)
> + static void init_timeouts(void)
> + {
> +   /* for the timeouts we use the PC platform defaults */
> +-  tpmData.permanent.data.tis_timeouts[0] = 750;
> +-  tpmData.permanent.data.tis_timeouts[1] = 2000;
> +-  tpmData.permanent.data.tis_timeouts[2] = 750;
> +-  tpmData.permanent.data.tis_timeouts[3] = 750;
> +-  tpmData.permanent.data.cmd_durations[0] = 1;
> +-  tpmData.permanent.data.cmd_durations[1] = 10;
> +-  tpmData.permanent.data.cmd_durations[2] = 1000;
> ++  tpmData.permanent.data.tis_timeouts[0] = 750000;
> ++  tpmData.permanent.data.tis_timeouts[1] = 2000000;
> ++  tpmData.permanent.data.tis_timeouts[2] = 750000;
> ++  tpmData.permanent.data.tis_timeouts[3] = 750000;
> ++  tpmData.permanent.data.cmd_durations[0] = 1000;
> ++  tpmData.permanent.data.cmd_durations[1] = 10000;
> ++  tpmData.permanent.data.cmd_durations[2] = 1000000;
> + }
> + 
> + void tpm_init_data(void)
> +-- 
> +2.30.2
> +
>
diff mbox series

Patch

diff --git a/stubdom/Makefile b/stubdom/Makefile
index c6de5f68ae..06aa69d8bc 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -239,6 +239,8 @@  tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
 	patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
 	patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
 	patch -d $@ -p1 < vtpm_extern.patch
+	patch -d $@ -p1 < vtpm-microsecond-duration.patch
+	patch -d $@ -p1 < vtpm-command-duration.patch
 	mkdir $@/build
 	cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
 	touch $@
diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
new file mode 100644
index 0000000000..6fdf2fc9be
--- /dev/null
+++ b/stubdom/vtpm-command-duration.patch
@@ -0,0 +1,52 @@ 
+From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
+From: Jason Andryuk <jandryuk@gmail.com>
+Date: Sun, 14 Mar 2021 12:46:34 -0400
+Subject: [PATCH 2/2] Increase command durations
+
+Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
+failing commands with -ETIME:
+tpm tpm0: tpm_try_transmit: send(): error-62
+
+The vtpm was returning the data, but it was after the duration timeout
+in vtpm_send.  Linux may have started being more stringent about timing?
+
+The vtpm-stubdom has a little delay since it writes its disk before
+returning the response.
+
+Anyway, the durations are rather low.  When they were 1/10/1000 before
+converting to microseconds, Linux showed all three durations rounded to
+10000.  Update them with values from a physical TPM1.2.  These were
+taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
+They might be excessive, but I'd rather have a command succeed than
+return -ETIME.
+
+An IFX physical TPM1.2 uses:
+1000000
+1500000
+150000000
+
+Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
+---
+ tpm/tpm_data.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
+index bebaf10..844afca 100644
+--- a/tpm/tpm_data.c
++++ b/tpm/tpm_data.c
+@@ -71,9 +71,9 @@ static void init_timeouts(void)
+   tpmData.permanent.data.tis_timeouts[1] = 2000000;
+   tpmData.permanent.data.tis_timeouts[2] = 750000;
+   tpmData.permanent.data.tis_timeouts[3] = 750000;
+-  tpmData.permanent.data.cmd_durations[0] = 1000;
+-  tpmData.permanent.data.cmd_durations[1] = 10000;
+-  tpmData.permanent.data.cmd_durations[2] = 1000000;
++  tpmData.permanent.data.cmd_durations[0] = 3000000;
++  tpmData.permanent.data.cmd_durations[1] = 3000000;
++  tpmData.permanent.data.cmd_durations[2] = 600000000;
+ }
+ 
+ void tpm_init_data(void)
+-- 
+2.30.2
+
diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
new file mode 100644
index 0000000000..7a906e72c5
--- /dev/null
+++ b/stubdom/vtpm-microsecond-duration.patch
@@ -0,0 +1,52 @@ 
+From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
+From: Jason Andryuk <jandryuk@gmail.com>
+Date: Sun, 14 Mar 2021 12:42:10 -0400
+Subject: [PATCH 1/2] Use microseconds for timeouts and durations
+
+The timeout and duration fields should be in microseconds according to
+the spec.
+
+TPM_CAP_PROP_TIS_TIMEOUT:
+A 4 element array of UINT32 values each denoting the timeout value in
+microseconds for the following in this order:
+
+TPM_CAP_PROP_DURATION:
+A 3 element array of UINT32 values each denoting the duration value in
+microseconds of the duration of the three classes of commands:
+
+Linux will scale the timeouts up by 1000, but not the durations.  Change
+the units for both sets as appropriate.
+
+Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
+---
+ tpm/tpm_data.c | 14 +++++++-------
+ 1 file changed, 7 insertions(+), 7 deletions(-)
+
+diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
+index a3a79ef..bebaf10 100644
+--- a/tpm/tpm_data.c
++++ b/tpm/tpm_data.c
+@@ -67,13 +67,13 @@ static void init_nv_storage(void)
+ static void init_timeouts(void)
+ {
+   /* for the timeouts we use the PC platform defaults */
+-  tpmData.permanent.data.tis_timeouts[0] = 750;
+-  tpmData.permanent.data.tis_timeouts[1] = 2000;
+-  tpmData.permanent.data.tis_timeouts[2] = 750;
+-  tpmData.permanent.data.tis_timeouts[3] = 750;
+-  tpmData.permanent.data.cmd_durations[0] = 1;
+-  tpmData.permanent.data.cmd_durations[1] = 10;
+-  tpmData.permanent.data.cmd_durations[2] = 1000;
++  tpmData.permanent.data.tis_timeouts[0] = 750000;
++  tpmData.permanent.data.tis_timeouts[1] = 2000000;
++  tpmData.permanent.data.tis_timeouts[2] = 750000;
++  tpmData.permanent.data.tis_timeouts[3] = 750000;
++  tpmData.permanent.data.cmd_durations[0] = 1000;
++  tpmData.permanent.data.cmd_durations[1] = 10000;
++  tpmData.permanent.data.cmd_durations[2] = 1000000;
+ }
+ 
+ void tpm_init_data(void)
+-- 
+2.30.2
+