From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> To: Tomas Winkler <tomas.winkler@intel.com> Cc: Jason Gunthorpe <jgg@ziepe.ca>, Alexander Usyskin <alexander.usyskin@intel.com>, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands. Date: Mon, 5 Mar 2018 14:56:42 +0200 [thread overview] Message-ID: <20180305125642.GA3425@linux.intel.com> (raw) In-Reply-To: <20180304121205.16934-1-tomas.winkler@intel.com> On Sun, Mar 04, 2018 at 02:12:03PM +0200, Tomas Winkler wrote: > TPM2_CC_Create(0x153) and TPM2_CC_CreatePrimary (0x131) involve generation > of crypto keys which can be a computationally intensive task. > The timeout is set to 3min. > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> Where is the cover letter? Please send separate patches if they are unrelated *or* add a cover letter that describes what they do as a whole. I will not review the next version if it does not have cover letter describing the high level change and containing the change log. > --- > drivers/char/tpm/tpm-interface.c | 4 ++++ > drivers/char/tpm/tpm.h | 27 ++++++++++++++++----------- > drivers/char/tpm/tpm2-cmd.c | 8 +++++--- > 3 files changed, 25 insertions(+), 14 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 85bdfa8c3348..c0aa9d11ec7a 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -699,6 +699,10 @@ int tpm_get_timeouts(struct tpm_chip *chip) > msecs_to_jiffies(TPM2_DURATION_MEDIUM); > chip->duration[TPM_LONG] = > msecs_to_jiffies(TPM2_DURATION_LONG); > + chip->duration[TPM_LONG_LONG] = > + msecs_to_jiffies(TPM2_DURATION_LONG_LONG); > + chip->duration[TPM_UNDEFINED] = > + msecs_to_jiffies(TPM2_DURATION_DEFAULT); > > chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS; > return 0; > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index f895fba4e20d..192ba68b39c2 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -67,7 +67,9 @@ enum tpm_duration { > TPM_SHORT = 0, > TPM_MEDIUM = 1, > TPM_LONG = 2, > - TPM_UNDEFINED, > + TPM_LONG_LONG = 3, > + TPM_UNDEFINED = 4, > + TPM_DURATION_MAX, This is starting to rotten to become unmaintainable. Here is what I suggest to move forward: * Have essentially two duration types: 1. Default 2. Long 'default' is the old long duration i.e. two seconds. 'long' is a We should probably have two durations: enum tpm_duration { TPM_DURATION_DEFAULT = 2000, TPM_DURATION_LONG = 300000, }; These would be both for TPM 1.2 and TPM 2.0. Instead of having table for every ordinal there should be a small tables describing commands that require long timeout. > - duration = 2 * 60 * HZ; > + duration = msecs_to_jiffies(TPM2_DURATION_DEFAULT); NAK for this change. /Jarkko
WARNING: multiple messages have this Message-ID (diff)
From: jarkko.sakkinen@linux.intel.com (Jarkko Sakkinen) To: linux-security-module@vger.kernel.org Subject: [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands. Date: Mon, 5 Mar 2018 14:56:42 +0200 [thread overview] Message-ID: <20180305125642.GA3425@linux.intel.com> (raw) In-Reply-To: <20180304121205.16934-1-tomas.winkler@intel.com> On Sun, Mar 04, 2018 at 02:12:03PM +0200, Tomas Winkler wrote: > TPM2_CC_Create(0x153) and TPM2_CC_CreatePrimary (0x131) involve generation > of crypto keys which can be a computationally intensive task. > The timeout is set to 3min. > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> Where is the cover letter? Please send separate patches if they are unrelated *or* add a cover letter that describes what they do as a whole. I will not review the next version if it does not have cover letter describing the high level change and containing the change log. > --- > drivers/char/tpm/tpm-interface.c | 4 ++++ > drivers/char/tpm/tpm.h | 27 ++++++++++++++++----------- > drivers/char/tpm/tpm2-cmd.c | 8 +++++--- > 3 files changed, 25 insertions(+), 14 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 85bdfa8c3348..c0aa9d11ec7a 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -699,6 +699,10 @@ int tpm_get_timeouts(struct tpm_chip *chip) > msecs_to_jiffies(TPM2_DURATION_MEDIUM); > chip->duration[TPM_LONG] = > msecs_to_jiffies(TPM2_DURATION_LONG); > + chip->duration[TPM_LONG_LONG] = > + msecs_to_jiffies(TPM2_DURATION_LONG_LONG); > + chip->duration[TPM_UNDEFINED] = > + msecs_to_jiffies(TPM2_DURATION_DEFAULT); > > chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS; > return 0; > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index f895fba4e20d..192ba68b39c2 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -67,7 +67,9 @@ enum tpm_duration { > TPM_SHORT = 0, > TPM_MEDIUM = 1, > TPM_LONG = 2, > - TPM_UNDEFINED, > + TPM_LONG_LONG = 3, > + TPM_UNDEFINED = 4, > + TPM_DURATION_MAX, This is starting to rotten to become unmaintainable. Here is what I suggest to move forward: * Have essentially two duration types: 1. Default 2. Long 'default' is the old long duration i.e. two seconds. 'long' is a We should probably have two durations: enum tpm_duration { TPM_DURATION_DEFAULT = 2000, TPM_DURATION_LONG = 300000, }; These would be both for TPM 1.2 and TPM 2.0. Instead of having table for every ordinal there should be a small tables describing commands that require long timeout. > - duration = 2 * 60 * HZ; > + duration = msecs_to_jiffies(TPM2_DURATION_DEFAULT); NAK for this change. /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-03-05 12:56 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-04 12:12 [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands Tomas Winkler 2018-03-04 12:12 ` Tomas Winkler 2018-03-04 12:12 ` [PATCH 2/3] tpm: add new tpm2 commands according to TCG 1.36 Tomas Winkler 2018-03-04 12:12 ` Tomas Winkler 2018-03-05 13:02 ` Jarkko Sakkinen 2018-03-05 13:02 ` Jarkko Sakkinen 2018-03-04 12:12 ` [PATCH 3/3] tpm_crb: use __le64 annotated variable for response buffer address Tomas Winkler 2018-03-04 12:12 ` Tomas Winkler 2018-03-05 13:03 ` Jarkko Sakkinen 2018-03-05 13:03 ` Jarkko Sakkinen 2018-03-06 8:28 ` Jarkko Sakkinen 2018-03-06 8:28 ` Jarkko Sakkinen 2018-03-06 8:34 ` Winkler, Tomas 2018-03-06 8:34 ` Winkler, Tomas 2018-03-06 15:39 ` Jason Gunthorpe 2018-03-06 15:39 ` Jason Gunthorpe 2018-03-05 12:56 ` Jarkko Sakkinen [this message] 2018-03-05 12:56 ` [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands Jarkko Sakkinen 2018-03-05 13:09 ` Winkler, Tomas 2018-03-05 13:09 ` Winkler, Tomas 2018-03-05 17:59 ` Jarkko Sakkinen 2018-03-05 17:59 ` Jarkko Sakkinen 2018-03-05 18:04 ` Winkler, Tomas 2018-03-05 18:04 ` Winkler, Tomas 2018-03-06 8:02 ` Jarkko Sakkinen 2018-03-06 8:02 ` Jarkko Sakkinen 2018-03-06 8:09 ` Winkler, Tomas 2018-03-06 8:09 ` Winkler, Tomas 2018-03-06 7:49 ` Jarkko Sakkinen 2018-03-06 7:49 ` Jarkko Sakkinen 2018-03-06 8:06 ` Winkler, Tomas 2018-03-06 8:06 ` Winkler, Tomas 2018-03-06 16:32 ` James Bottomley 2018-03-06 16:32 ` James Bottomley 2018-03-06 16:32 ` James Bottomley 2018-03-06 16:45 ` Winkler, Tomas 2018-03-06 16:45 ` Winkler, Tomas 2018-03-06 16:45 ` Winkler, Tomas 2018-03-06 18:36 ` Mimi Zohar 2018-03-06 18:36 ` Mimi Zohar 2018-03-06 18:36 ` Mimi Zohar 2018-03-06 21:59 ` Jason Gunthorpe 2018-03-06 21:59 ` Jason Gunthorpe 2018-03-06 21:59 ` Jason Gunthorpe 2018-03-07 15:22 ` Mimi Zohar 2018-03-07 15:22 ` Mimi Zohar 2018-03-07 15:22 ` Mimi Zohar 2018-03-07 15:41 ` Winkler, Tomas 2018-03-07 15:41 ` Winkler, Tomas 2018-03-07 15:41 ` Winkler, Tomas 2018-03-07 15:54 ` Jonathan Corbet 2018-03-07 15:54 ` Jonathan Corbet 2018-03-07 16:04 ` Winkler, Tomas 2018-03-07 16:35 ` Mimi Zohar 2018-03-07 16:35 ` Mimi Zohar 2018-03-07 18:24 ` Jonathan Corbet 2018-03-07 18:24 ` Jonathan Corbet 2018-03-10 12:46 ` Jarkko Sakkinen 2018-03-10 12:46 ` Jarkko Sakkinen 2018-03-10 12:44 ` Jarkko Sakkinen 2018-03-10 12:44 ` Jarkko Sakkinen 2018-03-10 12:37 ` Jarkko Sakkinen 2018-03-10 12:37 ` Jarkko Sakkinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180305125642.GA3425@linux.intel.com \ --to=jarkko.sakkinen@linux.intel.com \ --cc=alexander.usyskin@intel.com \ --cc=jgg@ziepe.ca \ --cc=linux-integrity@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=tomas.winkler@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.