| Message ID | 20160220081705.GA12981@intel.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: > Hi James, > > I'm sorry for the late pull request for 4.5. The reason for this was > the latency in my previous one. I picked with care the absolutely > critical fixes so that we can make a sound tpmdd release. > > I really hope you can still pick these as one of them is absolutely > critical to get authorization policy sealing API right (kernel keeps > it finger out of user space created objects). Pushed to next for more testing and review. This really is getting too late in the development cycle for so many fixes. It means the code was not ready to be merged in the first place. Also, any idea why I'm seeing this: drivers/char/tpm/tpm_tis.c:838: warning: ‘tpm_tis_resume’ defined but not used
On Mon, Feb 22, 2016 at 12:56:53PM +1100, James Morris wrote: > On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: > > > Hi James, > > > > I'm sorry for the late pull request for 4.5. The reason for this was > > the latency in my previous one. I picked with care the absolutely > > critical fixes so that we can make a sound tpmdd release. > > > > I really hope you can still pick these as one of them is absolutely > > critical to get authorization policy sealing API right (kernel keeps > > it finger out of user space created objects). > > Pushed to next for more testing and review. > > This really is getting too late in the development cycle for so many > fixes. It means the code was not ready to be merged in the first place. I fully agree what you're saying. I'll learn the lesson here and take factors more conservative attitude from now on. No excuses. I'm sorry about this. Partly the reason for recent increase in regressions has been increased real-world use of TPM2 and thus issues have started to pop up that's a lame excuse anyway. > Also, any idea why I'm seeing this: > > drivers/char/tpm/tpm_tis.c:838: warning: ‘tpm_tis_resume’ defined but not > used Bisected the patch: 00194826e6be Do you want me to send a pull request containing a fix for the build warning or reverting the whole commit? My call would be to apply the fix because this commit has been tested both TPM 1.2 by Martin and with TPM 2.0 by me and things have worked well. I can live with either option. I already pushed a fix to my master for this issue: https://github.com/jsakkine/linux-tpmdd/commit/6386544ad7bceb3d0248b85da29d4d99eebe9161 > -- > James Morris > <jmorris@namei.org> I've been recently working on a custom BR environment that bundles my latest master with initramfs user space [1]. At minimum I'll start using this environment to create builds of this env with and without PM for release testing and run the images both 1.2 and 2.0 HW. This should prevent the warning you experienced never happening again. [1] http://git.infradead.org/users/jjs/buildroot-tpmdd.git /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 04:50:23PM +0200, Jarkko Sakkinen wrote: > I already pushed a fix to my master for this issue: > > https://github.com/jsakkine/linux-tpmdd/commit/6386544ad7bceb3d0248b85da29d4d99eebe9161 The goal is to reduce the number of #ifdef'd code segments so we have fewer problems in future with a large .config test matrix. I'd rather see a __maybe_unused annotation instead. Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 10:52:45AM -0700, Jason Gunthorpe wrote: > On Mon, Feb 22, 2016 at 04:50:23PM +0200, Jarkko Sakkinen wrote: > > > I already pushed a fix to my master for this issue: > > > > https://github.com/jsakkine/linux-tpmdd/commit/6386544ad7bceb3d0248b85da29d4d99eebe9161 > > The goal is to reduce the number of #ifdef'd code segments so we have > fewer problems in future with a large .config test matrix. > > I'd rather see a __maybe_unused annotation instead. Agreed that it's a better form but at this point it's probably revert the breaking change and move to that later on. Otherwise, I don't see reason not to include the patch that you authored to the release. I've used it in my test kernels for quite some time now and it has worked without issues. I sent my fix for review now. > Jason /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 09:08:28PM +0200, Jarkko Sakkinen wrote: > On Mon, Feb 22, 2016 at 10:52:45AM -0700, Jason Gunthorpe wrote: > > On Mon, Feb 22, 2016 at 04:50:23PM +0200, Jarkko Sakkinen wrote: > > > > > I already pushed a fix to my master for this issue: > > > > > > https://github.com/jsakkine/linux-tpmdd/commit/6386544ad7bceb3d0248b85da29d4d99eebe9161 > > > > The goal is to reduce the number of #ifdef'd code segments so we have > > fewer problems in future with a large .config test matrix. > > > > I'd rather see a __maybe_unused annotation instead. > > Agreed that it's a better form but at this point it's probably revert > the breaking change and move to that later on. Otherwise, I don't see > reason not to include the patch that you authored to the release. I've > used it in my test kernels for quite some time now and it has worked > without issues. > > I sent my fix for review now. A warning with some kconfigs is very minor, we can take the time to fix it properly for 4.6. I am surprised the 0day -next builds/etc didn't notice this - Jarkko is your tree included in that process somehow? (sorry, I don't remember the details) Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 12:11:48PM -0700, Jason Gunthorpe wrote: > On Mon, Feb 22, 2016 at 09:08:28PM +0200, Jarkko Sakkinen wrote: > > On Mon, Feb 22, 2016 at 10:52:45AM -0700, Jason Gunthorpe wrote: > > > On Mon, Feb 22, 2016 at 04:50:23PM +0200, Jarkko Sakkinen wrote: > > > > > > > I already pushed a fix to my master for this issue: > > > > > > > > https://github.com/jsakkine/linux-tpmdd/commit/6386544ad7bceb3d0248b85da29d4d99eebe9161 > > > > > > The goal is to reduce the number of #ifdef'd code segments so we have > > > fewer problems in future with a large .config test matrix. > > > > > > I'd rather see a __maybe_unused annotation instead. > > > > Agreed that it's a better form but at this point it's probably revert > > the breaking change and move to that later on. Otherwise, I don't see > > reason not to include the patch that you authored to the release. I've > > used it in my test kernels for quite some time now and it has worked > > without issues. > > > > I sent my fix for review now. > > A warning with some kconfigs is very minor, we can take the time to > fix it properly for 4.6. I don't see any problem adding those too ifdefs back for 4.5 release. > I am surprised the 0day -next builds/etc didn't notice this - Jarkko is > your tree included in that process somehow? (sorry, I don't remember > the details) It has been and I've received notifications from there from time to time about my master branch and fixed issues accordingly. > Jason /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon Feb 22 16, Jarkko Sakkinen wrote: >On Mon, Feb 22, 2016 at 12:56:53PM +1100, James Morris wrote: >> On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: >> >> > Hi James, >> > >> > I'm sorry for the late pull request for 4.5. The reason for this was >> > the latency in my previous one. I picked with care the absolutely >> > critical fixes so that we can make a sound tpmdd release. >> > >> > I really hope you can still pick these as one of them is absolutely >> > critical to get authorization policy sealing API right (kernel keeps >> > it finger out of user space created objects). >> >> Pushed to next for more testing and review. >> >> This really is getting too late in the development cycle for so many >> fixes. It means the code was not ready to be merged in the first place. > >I fully agree what you're saying. I'll learn the lesson here and take >factors more conservative attitude from now on. No excuses. I'm sorry >about this. > >Partly the reason for recent increase in regressions has been >increased real-world use of TPM2 and thus issues have started to pop >up that's a lame excuse anyway. > Would it be worthwhile to have a tpm branch that gets pulled by -next directly so changes will have already been going through the paces in -next prior to the pull reuqest to James? snits ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Thu, 25 Feb 2016, Jerry Snitselaar wrote: > On Mon Feb 22 16, Jarkko Sakkinen wrote: > >On Mon, Feb 22, 2016 at 12:56:53PM +1100, James Morris wrote: > > > On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: > > > > > > > Hi James, > > > > > > > > I'm sorry for the late pull request for 4.5. The reason for this was > > > > the latency in my previous one. I picked with care the absolutely > > > > critical fixes so that we can make a sound tpmdd release. > > > > > > > > I really hope you can still pick these as one of them is absolutely > > > > critical to get authorization policy sealing API right (kernel keeps > > > > it finger out of user space created objects). > > > > > > Pushed to next for more testing and review. > > > > > > This really is getting too late in the development cycle for so many > > > fixes. It means the code was not ready to be merged in the first place. > > > >I fully agree what you're saying. I'll learn the lesson here and take > >factors more conservative attitude from now on. No excuses. I'm sorry > >about this. > > > >Partly the reason for recent increase in regressions has been > >increased real-world use of TPM2 and thus issues have started to pop > >up that's a lame excuse anyway. > > > > Would it be worthwhile to have a tpm branch that gets pulled by -next > directly so changes will have already been going through the paces in > -next prior to the pull reuqest to James? That would be useful, some other subsystems do that.
On Mon, 22 Feb 2016, Jarkko Sakkinen wrote: > Do you want me to send a pull request containing a fix for the build > warning or reverting the whole commit? My call would be to apply the > fix because this commit has been tested both TPM 1.2 by Martin and > with TPM 2.0 by me and things have worked well. > Send me a pull request just for the fix. I won't be pushing these changes to Linus for 4.5, they'll have to wait until the 4.6.
On Fri, Feb 26, 2016 at 06:57:49PM +1100, James Morris wrote: > On Mon, 22 Feb 2016, Jarkko Sakkinen wrote: > > > Do you want me to send a pull request containing a fix for the build > > warning or reverting the whole commit? My call would be to apply the > > fix because this commit has been tested both TPM 1.2 by Martin and > > with TPM 2.0 by me and things have worked well. > > > > Send me a pull request just for the fix. > > I won't be pushing these changes to Linus for 4.5, they'll have to wait > until the 4.6. OK > -- > James Morris > <jmorris@namei.org> /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Fri, Feb 26, 2016 at 02:38:52PM +1100, James Morris wrote: > On Thu, 25 Feb 2016, Jerry Snitselaar wrote: > > > On Mon Feb 22 16, Jarkko Sakkinen wrote: > > >On Mon, Feb 22, 2016 at 12:56:53PM +1100, James Morris wrote: > > > > On Sat, 20 Feb 2016, Jarkko Sakkinen wrote: > > > > > > > > > Hi James, > > > > > > > > > > I'm sorry for the late pull request for 4.5. The reason for this was > > > > > the latency in my previous one. I picked with care the absolutely > > > > > critical fixes so that we can make a sound tpmdd release. > > > > > > > > > > I really hope you can still pick these as one of them is absolutely > > > > > critical to get authorization policy sealing API right (kernel keeps > > > > > it finger out of user space created objects). > > > > > > > > Pushed to next for more testing and review. > > > > > > > > This really is getting too late in the development cycle for so many > > > > fixes. It means the code was not ready to be merged in the first place. > > > > > >I fully agree what you're saying. I'll learn the lesson here and take > > >factors more conservative attitude from now on. No excuses. I'm sorry > > >about this. > > > > > >Partly the reason for recent increase in regressions has been > > >increased real-world use of TPM2 and thus issues have started to pop > > >up that's a lame excuse anyway. > > > > > > > Would it be worthwhile to have a tpm branch that gets pulled by -next > > directly so changes will have already been going through the paces in > > -next prior to the pull reuqest to James? > > That would be useful, some other subsystems do that. I'll start using it. > -- > James Morris > <jmorris@namei.org> /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Hi James, I'm sorry for the late pull request for 4.5. The reason for this was the latency in my previous one. I picked with care the absolutely critical fixes so that we can make a sound tpmdd release. I really hope you can still pick these as one of them is absolutely critical to get authorization policy sealing API right (kernel keeps it finger out of user space created objects). BR, /Jarkko The following changes since commit 8e0ee3c9faed7ca68807ea45141775856c438ac0: tpm: fix the cleanup of struct tpm_chip (2016-02-10 04:12:08 +0200) are available in the git repository at: https://github.com/jsakkine/linux-tpmdd.git tags/tpmdd-next-20160220 for you to fetch changes up to 99cda8cb4639de81cde785b5bab9bc52e916e594: tpm_crb: tpm2_shutdown() must be called before tpm_chip_unregister() (2016-02-20 09:59:33 +0200) ---------------------------------------------------------------- remaining tpmdd fixes for Linux 4.5 ---------------------------------------------------------------- Harald Hoyer (1): tpm_eventlog.c: fix binary_bios_measurements Jarkko Sakkinen (4): tpm: fix: set continueSession attribute for the unseal operation tpm: fix: return rc when devm_add_action() fails tpm_crb/tis: fix: use dev_name() for /proc/iomem tpm_crb: tpm2_shutdown() must be called before tpm_chip_unregister() drivers/char/tpm/tpm-chip.c | 7 ++++++- drivers/char/tpm/tpm2-cmd.c | 10 +++++++--- drivers/char/tpm/tpm_crb.c | 8 +++++--- drivers/char/tpm/tpm_eventlog.c | 14 ++++++++++---- drivers/char/tpm/tpm_tis.c | 4 +++- 5 files changed, 31 insertions(+), 12 deletions(-) ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140