On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device
> > *pdev,
> >     chip->cdev.owner = THIS_MODULE;
> >     chip->cdev.kobj.parent = &chip->dev.kobj;
> > 
> > +   chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > GFP_KERNEL);
> > +   if (!chip->work_space.context_buf) {
> > +           rc = -ENOMEM;
> > +           goto out;
> > +   }
> > +
> 
> I think the work_buf handling can be greatly simplified by making it a
> pointer to the space: it's only usable between tpm2_prepare_space() and
> tpm2_commit_space() which are protected by the chip mutex, so there's
> no need for it to exist outside of these calls (i.e. it can be NULL).
> 
> Doing it this way also saves the allocation and copying overhead of
> work_space.
> 
> The patch below can be folded to effect this.
> 
> James
> 
> ---
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 13cac09..770a8c0 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -131,7 +131,6 @@ static void tpm_dev_release(struct device *dev)
>       mutex_unlock(&idr_lock);
>  
>       kfree(chip->log.bios_event_log);
> -     kfree(chip->work_space.context_buf);
>       kfree(chip);
>  }
>  
> @@ -206,12 +205,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>       chip->cdev.kobj.parent = &chip->dev.kobj;
>       chip->cdevrm.kobj.parent = &chip->devrm.kobj;
>  
> -     chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> -     if (!chip->work_space.context_buf) {
> -             rc = -ENOMEM;
> -             goto out;
> -     }
> -
>       return chip;
>  
>  out:
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8009ed4..adf7810 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -211,7 +211,7 @@ struct tpm_chip {
>       char ppi_version[TPM_PPI_VERSION_LEN + 1];
>  #endif /* CONFIG_ACPI */
>  
> -     struct tpm_space work_space;
> +     struct tpm_space *work_space;
>       u32 nr_commands;
>       u32 *cc_attrs_tbl;
>  };
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 44e5501..285361e 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -27,7 +27,7 @@ enum tpm2_handle_types {
>  
>  static void tpm2_flush_space(struct tpm_chip *chip)
>  {
> -     struct tpm_space *space = &chip->work_space;
> +     struct tpm_space *space = chip->work_space;
>       int i;
>  
>       for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
> @@ -45,7 +45,7 @@ struct tpm2_context {
>  
>  static int tpm2_load_space(struct tpm_chip *chip)
>  {
> -     struct tpm_space *space = &chip->work_space;
> +     struct tpm_space *space = chip->work_space;
>       struct tpm2_context *ctx;
>       struct tpm_buf buf;
>       int i;
> @@ -99,7 +99,7 @@ static int tpm2_load_space(struct tpm_chip *chip)
>  
>  static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t 
> len)
>  {
> -     struct tpm_space *space = &chip->work_space;
> +     struct tpm_space *space = chip->work_space;
>       unsigned int nr_handles;
>       u32 vhandle;
>       u32 phandle;
> @@ -147,9 +147,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct 
> tpm_space *space,
>       if (!space)
>               return 0;
>  
> -     memcpy(&chip->work_space.context_tbl, &space->context_tbl,
> -            sizeof(space->context_tbl));
> -     memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
> +     chip->work_space = space;
>  
>       rc = tpm2_load_space(chip);
>       if (rc)
> @@ -164,7 +162,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct 
> tpm_space *space,
>  
>  static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t 
> len)
>  {
> -     struct tpm_space *space = &chip->work_space;
> +     struct tpm_space *space = chip->work_space;
>       u32 phandle;
>       u32 vhandle;
>       u32 attrs;
> @@ -222,7 +220,7 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 
> cc, u8 *rsp, size_t len)
>  
>  static int tpm2_save_space(struct tpm_chip *chip)
>  {
> -     struct tpm_space *space = &chip->work_space;
> +     struct tpm_space *space = chip->work_space;
>       struct tpm_buf buf;
>       int i;
>       int j;
> @@ -295,9 +293,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct 
> tpm_space *space,
>       if (rc)
>               return rc;
>  
> -     memcpy(&space->context_tbl, &chip->work_space.context_tbl,
> -            sizeof(space->context_tbl));
> -     memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
> +     chip->work_space = NULL;
>  
>       return 0;
>  }

[x]

/Jarkko

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Reply via email to