From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: "Winkler, Tomas" <tomas.winkler@intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
"Usyskin, Alexander" <alexander.usyskin@intel.com>,
"linux-integrity@vger.kernel.org"
<linux-integrity@vger.kernel.org>,
"linux-security-module@vger.kernel.org"
<linux-security-module@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2 v3] tpm: cmd_ready command can be issued only after granting locality
Date: Tue, 20 Feb 2018 16:57:29 +0200 [thread overview]
Message-ID: <1519138649.26293.1.camel@linux.intel.com> (raw)
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B942240B1@hasmsx108.ger.corp.intel.com>
On Mon, 2018-02-19 at 11:43 +0000, Winkler, Tomas wrote:
> > All local variable declarations must be in the beginning of the
> > function.
>
> Who says?
It is coherent how we have everything else.
It is much easier to see the stack allocation this way when the
allocation is only done in the beginning of each function. If
you really need to do such pattern, then it would be a better
idea to consider an additional helper function.
> > Your comment about not overriding error code is incorrect.
>
> Please explain?
'l_rc' overrides 'rc' in the case when both are non-zero.
> > The value of 'rc' should be never overridden, which kind of
> > supports to "just
> > print" behavior that we had for a locality error.
>
> You are not consistent, you've agreed with propagating it to user
> space. The error will be propagated in case of an error in
> locality relinquish the device is pretty much in non functional
> state and provious errors do not matter much, but rc value won't
> be modified if locality_reliquish succeeds.
Well, sometimes you fail to notice things and I failed to notice the
collision above. The commit message does not describe why 'l_rc'
overrides 'rc' in the case when both are non-zero. What was the
reasoning, which made you end up with this priority order? Why is
'l_rc' more important than 'rc'?
My take is that does it really make sense have this change as part
of a high priority bug fix that should be as localized as possible?
Seems like a non-trivial problem by itself.
/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/2 v3] tpm: cmd_ready command can be issued only after granting locality
Date: Tue, 20 Feb 2018 16:57:29 +0200 [thread overview]
Message-ID: <1519138649.26293.1.camel@linux.intel.com> (raw)
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B942240B1@hasmsx108.ger.corp.intel.com>
On Mon, 2018-02-19 at 11:43 +0000, Winkler, Tomas wrote:
> > All local variable declarations must be in the beginning of the
> > function.
>
> Who says?
It is coherent how we have everything else.
It is much easier to see the stack allocation this way when the
allocation is only done in the beginning of each function. If
you really need to do such pattern, then it would be a better
idea to consider an additional helper function.
> > Your comment about not overriding error code is incorrect.
>
> Please explain?
'l_rc' overrides 'rc' in the case when both are non-zero.
> > The value of 'rc' should be never overridden, which kind of
> > supports to "just
> > print" behavior that we had for a locality error.
>
> You are not consistent, you've agreed with propagating it to user
> space. The error will be propagated in case of an error in
> locality relinquish the device is pretty much in non functional
> state and provious errors do not matter much, but rc value won't
> be modified if locality_reliquish succeeds.
Well, sometimes you fail to notice things and I failed to notice the
collision above. The commit message does not describe why 'l_rc'
overrides 'rc' in the case when both are non-zero. What was the
reasoning, which made you end up with this priority order? Why is
'l_rc' more important than 'rc'?
My take is that does it really make sense have this change as part
of a high priority bug fix that should be as localized as possible?
Seems like a non-trivial problem by itself.
/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-02-20 14:57 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-14 13:43 [PATCH 0/2 v3] tpm: fix locality and power saving handling Tomas Winkler
2018-02-14 13:43 ` Tomas Winkler
2018-02-14 13:43 ` [PATCH 1/2 v3] tpm: cmd_ready command can be issued only after granting locality Tomas Winkler
2018-02-14 13:43 ` Tomas Winkler
2018-02-19 11:27 ` Jarkko Sakkinen
2018-02-19 11:27 ` Jarkko Sakkinen
2018-02-19 11:43 ` Winkler, Tomas
2018-02-19 11:43 ` Winkler, Tomas
2018-02-20 14:57 ` Jarkko Sakkinen [this message]
2018-02-20 14:57 ` Jarkko Sakkinen
2018-02-20 20:26 ` Winkler, Tomas
2018-02-20 20:26 ` Winkler, Tomas
2018-02-20 23:03 ` Jarkko Sakkinen
2018-02-20 23:03 ` Jarkko Sakkinen
2018-02-20 14:12 ` Jarkko Sakkinen
2018-02-20 14:12 ` Jarkko Sakkinen
2018-02-14 13:43 ` [PATCH 2/2 v3] tpm: separate cmd_ready/go_idle from runtime_pm Tomas Winkler
2018-02-14 13:43 ` Tomas Winkler
2018-02-19 11:52 ` Jarkko Sakkinen
2018-02-19 11:52 ` Jarkko Sakkinen
2018-02-20 14:13 ` Jarkko Sakkinen
2018-02-20 14:13 ` Jarkko Sakkinen
2018-03-05 18:08 ` Jarkko Sakkinen
2018-03-05 18:08 ` Jarkko Sakkinen
2018-03-23 8:12 ` Jarkko Sakkinen
2018-03-23 8:12 ` 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=1519138649.26293.1.camel@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: link
Be 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.