* Re: [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in tpm_tis [not found] <1510090328-153106-1-git-send-email-azhar.shaikh@intel.com> @ 2017-11-14 21:38 ` Jarkko Sakkinen 2017-11-14 21:59 ` Shaikh, Azhar [not found] ` <1510090328-153106-3-git-send-email-azhar.shaikh@intel.com> 1 sibling, 1 reply; 9+ messages in thread From: Jarkko Sakkinen @ 2017-11-14 21:38 UTC (permalink / raw) To: Azhar Shaikh; +Cc: linux-integrity, jarkko.sakkinen On Tue, Nov 07, 2017 at 01:32:06PM -0800, Azhar Shaikh wrote: > Changes from v1: > - Patch 1: "tpm: Keep CLKRUN enabled throughout the duration of transmit_cmd()" > - Add NULL checks before calling clk_toggle callback > - Use IS_ENABLED instead of ifdef in tpm_tis_clkrun_toggle() > - Do not call tpm_platform_begin_xfer() and tpm_platform_end_xfer() > from tpm_tis_clkrun_toggle(). Make them static again. > > - Patch 2: "tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy" > - This is a new patch in this series as per suggestion from Jason. > - Is the current implementation ok or I should move the code in tpm_tis_pnp_remove() > and tpm_tis_plat_remove() inside tpm_tis_remove(). That way all the unmapping > can be done in one place, instead of 3 different places now. Also the unmapping > in tpm_tis_init() can be moved to tpm_tis_remove(), since in case of error > tpm_tis_core_init() calls tpm_tis_remove(). Kindly suggest. > > > Azhar Shaikh (2): > tpm: Keep CLKRUN enabled throughout the duration of transmit_cmd() > tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy > > drivers/char/tpm/tpm-interface.c | 6 +++ > drivers/char/tpm/tpm_tis.c | 102 ++++++++++++++++++++++++--------------- > drivers/char/tpm/tpm_tis_core.c | 21 ++++++++ > drivers/char/tpm/tpm_tis_core.h | 1 + > include/linux/tpm.h | 1 + > 5 files changed, 93 insertions(+), 38 deletions(-) > > -- > 1.9.1 > Please include my email (@linux.intel.com) to the TO-field for all TPM patches (and the cover letter). I will eventually catch these as I go through the ML but sometimes there is more latency to do that when I have a busy period. In a less busy period there is of course less latency. As this is a shared list with IMA and EVM I have to check every message whether it is a TPM patch. Thus, in a shared list including maintainers is even more important. You can find maintainers for every subsystem from MAINTAINERS file in the root of the Linux GIT tree. Right now I have a very busy period as I'm upstream the SGX driver. That is why it took a week to even spot this (just did). I'll try to find time this week to properly review your changes but cannot promise it will be tomorrow because, well, I just saw the patch set. With a quick oversight I do not see anything that would shock me but I still have to look into it with time and care. /Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in tpm_tis 2017-11-14 21:38 ` [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in tpm_tis Jarkko Sakkinen @ 2017-11-14 21:59 ` Shaikh, Azhar 2017-11-15 8:13 ` Jarkko Sakkinen 0 siblings, 1 reply; 9+ messages in thread From: Shaikh, Azhar @ 2017-11-14 21:59 UTC (permalink / raw) To: Jarkko Sakkinen; +Cc: linux-integrity, Sakkinen, Jarkko >-----Original Message----- >From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity- >owner@vger.kernel.org] On Behalf Of Jarkko Sakkinen >Sent: Tuesday, November 14, 2017 1:39 PM >To: Shaikh, Azhar <azhar.shaikh@intel.com> >Cc: linux-integrity@vger.kernel.org; Sakkinen, Jarkko ><jarkko.sakkinen@intel.com> >Subject: Re: [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in >tpm_tis > >On Tue, Nov 07, 2017 at 01:32:06PM -0800, Azhar Shaikh wrote: >> Changes from v1: >> - Patch 1: "tpm: Keep CLKRUN enabled throughout the duration of >transmit_cmd()" >> - Add NULL checks before calling clk_toggle callback >> - Use IS_ENABLED instead of ifdef in tpm_tis_clkrun_toggle() >> - Do not call tpm_platform_begin_xfer() and tpm_platform_end_xfer() >> from tpm_tis_clkrun_toggle(). Make them static again. >> >> - Patch 2: "tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy" >> - This is a new patch in this series as per suggestion from Jason. >> - Is the current implementation ok or I should move the code in >tpm_tis_pnp_remove() >> and tpm_tis_plat_remove() inside tpm_tis_remove(). That way all the >unmapping >> can be done in one place, instead of 3 different places now. Also the >unmapping >> in tpm_tis_init() can be moved to tpm_tis_remove(), since in case of error >> tpm_tis_core_init() calls tpm_tis_remove(). Kindly suggest. >> >> >> Azhar Shaikh (2): >> tpm: Keep CLKRUN enabled throughout the duration of transmit_cmd() >> tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy >> >> drivers/char/tpm/tpm-interface.c | 6 +++ >> drivers/char/tpm/tpm_tis.c | 102 ++++++++++++++++++++++++--------- >------ >> drivers/char/tpm/tpm_tis_core.c | 21 ++++++++ >> drivers/char/tpm/tpm_tis_core.h | 1 + >> include/linux/tpm.h | 1 + >> 5 files changed, 93 insertions(+), 38 deletions(-) >> >> -- >> 1.9.1 >> > >Please include my email (@linux.intel.com) to the TO-field for all TPM >patches (and the cover letter). > Sorry about this. Going forward will include @linux.intel.com and other TPM maintainers to the TO-field. >I will eventually catch these as I go through the ML but sometimes there >is more latency to do that when I have a busy period. In a less busy >period there is of course less latency. > >As this is a shared list with IMA and EVM I have to check every message >whether it is a TPM patch. Thus, in a shared list including maintainers >is even more important. You can find maintainers for every subsystem >from MAINTAINERS file in the root of the Linux GIT tree. > >Right now I have a very busy period as I'm upstream the SGX driver. That >is why it took a week to even spot this (just did). > >I'll try to find time this week to properly review your changes but >cannot promise it will be tomorrow because, well, I just saw the patch >set. > Sure, I understand. Thank you! >With a quick oversight I do not see anything that would shock me but I >still have to look into it with time and care. > >/Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in tpm_tis 2017-11-14 21:59 ` Shaikh, Azhar @ 2017-11-15 8:13 ` Jarkko Sakkinen 0 siblings, 0 replies; 9+ messages in thread From: Jarkko Sakkinen @ 2017-11-15 8:13 UTC (permalink / raw) To: Shaikh, Azhar; +Cc: Jarkko Sakkinen, linux-integrity On Tue, Nov 14, 2017 at 11:59:45PM +0200, Shaikh, Azhar wrote: > Sorry about this. Going forward will include @linux.intel.com and > other TPM maintainers to the TO-field. NP, didn't really affect me :-) But if you want less latency to your review, you should do this. /Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <1510090328-153106-3-git-send-email-azhar.shaikh@intel.com>]
* Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy [not found] ` <1510090328-153106-3-git-send-email-azhar.shaikh@intel.com> @ 2017-11-14 23:28 ` Jason Gunthorpe 2017-11-15 8:14 ` Jarkko Sakkinen 0 siblings, 1 reply; 9+ messages in thread From: Jason Gunthorpe @ 2017-11-14 23:28 UTC (permalink / raw) To: Azhar Shaikh; +Cc: linux-integrity, jarkko.sakkinen On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: > +#ifdef CONFIG_X86 > + if (is_bsw()) > + iounmap(phy->ilb_base_addr); > +#endif This whole thing would be much better if is_bsw was just bool is_bsw(void) { if (!IS_ENABLED(CONFIG_X86)) return false; [..] } Then drop every single one of these #ifdef CONFIG_X86 Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy 2017-11-14 23:28 ` [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy Jason Gunthorpe @ 2017-11-15 8:14 ` Jarkko Sakkinen 2017-11-15 17:58 ` Shaikh, Azhar 2017-11-15 19:36 ` Shaikh, Azhar 0 siblings, 2 replies; 9+ messages in thread From: Jarkko Sakkinen @ 2017-11-15 8:14 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: Azhar Shaikh, linux-integrity On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: > On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: > > > +#ifdef CONFIG_X86 > > + if (is_bsw()) > > + iounmap(phy->ilb_base_addr); > > +#endif > > This whole thing would be much better if is_bsw was just > > bool is_bsw(void) > { > if (!IS_ENABLED(CONFIG_X86)) > return false; > [..] > } > > Then drop every single one of these #ifdef CONFIG_X86 > > Jason +1 /Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy 2017-11-15 8:14 ` Jarkko Sakkinen @ 2017-11-15 17:58 ` Shaikh, Azhar 2017-11-15 19:36 ` Shaikh, Azhar 1 sibling, 0 replies; 9+ messages in thread From: Shaikh, Azhar @ 2017-11-15 17:58 UTC (permalink / raw) To: Sakkinen, Jarkko, Jason Gunthorpe; +Cc: linux-integrity >-----Original Message----- >From: Sakkinen, Jarkko >Sent: Wednesday, November 15, 2017 12:14 AM >To: Jason Gunthorpe <jgg@ziepe.ca> >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; linux-integrity@vger.kernel.org >Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > >On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: >> On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: >> >> > +#ifdef CONFIG_X86 >> > + if (is_bsw()) >> > + iounmap(phy->ilb_base_addr); >> > +#endif >> >> This whole thing would be much better if is_bsw was just >> >> bool is_bsw(void) >> { >> if (!IS_ENABLED(CONFIG_X86)) >> return false; >> [..] >> } >> >> Then drop every single one of these #ifdef CONFIG_X86 >> >> Jason > >+1 > Thank you for the suggestion. Will incorporate this in next patch series. >/Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy 2017-11-15 8:14 ` Jarkko Sakkinen 2017-11-15 17:58 ` Shaikh, Azhar @ 2017-11-15 19:36 ` Shaikh, Azhar 2017-11-15 19:39 ` Jason Gunthorpe 1 sibling, 1 reply; 9+ messages in thread From: Shaikh, Azhar @ 2017-11-15 19:36 UTC (permalink / raw) To: Jarkko Sakkinen, Jason Gunthorpe; +Cc: linux-integrity >-----Original Message----- >From: Shaikh, Azhar >Sent: Wednesday, November 15, 2017 9:59 AM >To: Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; Jason Gunthorpe ><jgg@ziepe.ca> >Cc: linux-integrity@vger.kernel.org >Subject: RE: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > > > >>-----Original Message----- >>From: Sakkinen, Jarkko >>Sent: Wednesday, November 15, 2017 12:14 AM >>To: Jason Gunthorpe <jgg@ziepe.ca> >>Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; >>linux-integrity@vger.kernel.org >>Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >>tpm_tis_tcg_phy >> >>On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: >>> On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: >>> >>> > +#ifdef CONFIG_X86 >>> > + if (is_bsw()) >>> > + iounmap(phy->ilb_base_addr); >>> > +#endif >>> >>> This whole thing would be much better if is_bsw was just >>> >>> bool is_bsw(void) >>> { >>> if (!IS_ENABLED(CONFIG_X86)) >>> return false; >>> [..] >>> } >>> >>> Then drop every single one of these #ifdef CONFIG_X86 >>> >>> Jason >> >>+1 >> > >Thank you for the suggestion. >Will incorporate this in next patch series. > If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef CONFIG_X86, I will still get compilation errors for non-x86 platforms, since INTEL_FAM6_ATOM_AIRMONT will be undefined. bool is_bsw(void) { if (!IS_ENABLED(CONFIG_X86)) return false; return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); } I think I will have to keep is_bsw() implementation unchanged? >>/Jarkko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy 2017-11-15 19:36 ` Shaikh, Azhar @ 2017-11-15 19:39 ` Jason Gunthorpe 2017-11-15 19:57 ` Shaikh, Azhar 0 siblings, 1 reply; 9+ messages in thread From: Jason Gunthorpe @ 2017-11-15 19:39 UTC (permalink / raw) To: Shaikh, Azhar; +Cc: Jarkko Sakkinen, linux-integrity On Wed, Nov 15, 2017 at 07:36:47PM +0000, Shaikh, Azhar wrote: > If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef CONFIG_X86, I will still get compilation errors for non-x86 platforms, since INTEL_FAM6_ATOM_AIRMONT will be undefined. > > bool is_bsw(void) > { > if (!IS_ENABLED(CONFIG_X86)) > return false; > return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); > } > > I think I will have to keep is_bsw() implementation unchanged? No, still move the #ifdef to is_bsw, just like this instead: bool is_bsw(void) { #ifdef CONFIG_X86 return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); #else return false; #endif } Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy 2017-11-15 19:39 ` Jason Gunthorpe @ 2017-11-15 19:57 ` Shaikh, Azhar 0 siblings, 0 replies; 9+ messages in thread From: Shaikh, Azhar @ 2017-11-15 19:57 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: Jarkko Sakkinen, linux-integrity >-----Original Message----- >From: Jason Gunthorpe [mailto:jgg@ziepe.ca] >Sent: Wednesday, November 15, 2017 11:40 AM >To: Shaikh, Azhar <azhar.shaikh@intel.com> >Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>; linux- >integrity@vger.kernel.org >Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > >On Wed, Nov 15, 2017 at 07:36:47PM +0000, Shaikh, Azhar wrote: > >> If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef >CONFIG_X86, I will still get compilation errors for non-x86 platforms, since >INTEL_FAM6_ATOM_AIRMONT will be undefined. >> >> bool is_bsw(void) >> { >> if (!IS_ENABLED(CONFIG_X86)) >> return false; >> return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 >: 0); >> } >> >> I think I will have to keep is_bsw() implementation unchanged? > >No, still move the #ifdef to is_bsw, just like this instead: > >bool is_bsw(void) >{ >#ifdef CONFIG_X86 > return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 > : 0); >#else > return false; >#endif >} > Ok, sure. Should have thought myself about this... :( >Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-15 19:57 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1510090328-153106-1-git-send-email-azhar.shaikh@intel.com> 2017-11-14 21:38 ` [PATCH RFC v2 0/2] Fix corner cases with disabling CLKRUN in tpm_tis Jarkko Sakkinen 2017-11-14 21:59 ` Shaikh, Azhar 2017-11-15 8:13 ` Jarkko Sakkinen [not found] ` <1510090328-153106-3-git-send-email-azhar.shaikh@intel.com> 2017-11-14 23:28 ` [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to tpm_tis_tcg_phy Jason Gunthorpe 2017-11-15 8:14 ` Jarkko Sakkinen 2017-11-15 17:58 ` Shaikh, Azhar 2017-11-15 19:36 ` Shaikh, Azhar 2017-11-15 19:39 ` Jason Gunthorpe 2017-11-15 19:57 ` Shaikh, Azhar
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.