diff mbox series

[v9,2/4] tpm: of: Make of-tree specific function commonly available

Message ID 20230418134409.177485-3-stefanb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series tpm: Preserve TPM measurement log across kexec (ppc64) | expand

Commit Message

Stefan Berger April 18, 2023, 1:44 p.m. UTC 
Simplify tpm_read_log_of() by moving reusable parts of the code into
an inline function that makes it commonly available so it can be
used also for kexec support. Call the new of_tpm_get_sml_parameters()
function from the TPM Open Firmware driver.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Tested-by: Coiby Xu <coxu@redhat.com>
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

---
v9:
 - Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned

v7:
 - Added original comment back into inlined function

v4:
 - converted to inline function
---
 drivers/char/tpm/eventlog/of.c | 30 +++++-----------------------
 include/linux/tpm.h            | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 25 deletions(-)

Comments

Jerry Snitselaar May 24, 2023, 10:56 p.m. UTC | #1 
On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> Simplify tpm_read_log_of() by moving reusable parts of the code into
> an inline function that makes it commonly available so it can be
> used also for kexec support. Call the new of_tpm_get_sml_parameters()
> function from the TPM Open Firmware driver.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Tested-by: Coiby Xu <coxu@redhat.com>
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

> ---
> v9:
>  - Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned
> 
> v7:
>  - Added original comment back into inlined function
> 
> v4:
>  - converted to inline function
> ---
>  drivers/char/tpm/eventlog/of.c | 30 +++++-----------------------
>  include/linux/tpm.h            | 36 ++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
> index 930fe43d5daf..41688d9cbd3b 100644
> --- a/drivers/char/tpm/eventlog/of.c
> +++ b/drivers/char/tpm/eventlog/of.c
> @@ -51,11 +51,10 @@ static int tpm_read_log_memory_region(struct tpm_chip *chip)
>  int tpm_read_log_of(struct tpm_chip *chip)
>  {
>  	struct device_node *np;
> -	const u32 *sizep;
> -	const u64 *basep;
>  	struct tpm_bios_log *log;
>  	u32 size;
>  	u64 base;
> +	int ret;
>  
>  	log = &chip->log;
>  	if (chip->dev.parent && chip->dev.parent->of_node)
> @@ -66,30 +65,11 @@ int tpm_read_log_of(struct tpm_chip *chip)
>  	if (of_property_read_bool(np, "powered-while-suspended"))
>  		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
>  
> -	sizep = of_get_property(np, "linux,sml-size", NULL);
> -	basep = of_get_property(np, "linux,sml-base", NULL);
> -	if (sizep == NULL && basep == NULL)
> +	ret = of_tpm_get_sml_parameters(np, &base, &size);
> +	if (ret == -ENODEV)
>  		return tpm_read_log_memory_region(chip);
> -	if (sizep == NULL || basep == NULL)
> -		return -EIO;
> -
> -	/*
> -	 * For both vtpm/tpm, firmware has log addr and log size in big
> -	 * endian format. But in case of vtpm, there is a method called
> -	 * sml-handover which is run during kernel init even before
> -	 * device tree is setup. This sml-handover function takes care
> -	 * of endianness and writes to sml-base and sml-size in little
> -	 * endian format. For this reason, vtpm doesn't need conversion
> -	 * but physical tpm needs the conversion.
> -	 */
> -	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
> -	    of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
> -		size = be32_to_cpup((__force __be32 *)sizep);
> -		base = be64_to_cpup((__force __be64 *)basep);
> -	} else {
> -		size = *sizep;
> -		base = *basep;
> -	}
> +	if (ret < 0)
> +		return ret;
>  
>  	if (size == 0) {
>  		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 4dc97b9f65fb..dd9a376a1dbb 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -461,4 +461,40 @@ static inline struct tpm_chip *tpm_default_chip(void)
>  	return NULL;
>  }
>  #endif
> +
> +#ifdef CONFIG_OF
> +static inline int of_tpm_get_sml_parameters(struct device_node *np,
> +					    u64 *base, u32 *size)
> +{
> +	const u32 *sizep;
> +	const u64 *basep;
> +
> +	sizep = of_get_property(np, "linux,sml-size", NULL);
> +	basep = of_get_property(np, "linux,sml-base", NULL);
> +	if (sizep == NULL && basep == NULL)
> +		return -ENODEV;
> +	if (sizep == NULL || basep == NULL)
> +		return -EIO;
> +
> +	/*
> +	 * For both vtpm/tpm, firmware has log addr and log size in big
> +	 * endian format. But in case of vtpm, there is a method called
> +	 * sml-handover which is run during kernel init even before
> +	 * device tree is setup. This sml-handover function takes care
> +	 * of endianness and writes to sml-base and sml-size in little
> +	 * endian format. For this reason, vtpm doesn't need conversion
> +	 * but physical tpm needs the conversion.
> +	 */
> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
> +	    of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
> +		*size = be32_to_cpup((__force __be32 *)sizep);
> +		*base = be64_to_cpup((__force __be64 *)basep);
> +	} else {
> +		*size = *sizep;
> +		*base = *basep;
> +	}
> +	return 0;
> +}
> +#endif
> +
>  #endif
> -- 
> 2.38.1
>
Jarkko Sakkinen June 9, 2023, 6:18 p.m. UTC | #2 
On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> > Simplify tpm_read_log_of() by moving reusable parts of the code into
> > an inline function that makes it commonly available so it can be
> > used also for kexec support. Call the new of_tpm_get_sml_parameters()
> > function from the TPM Open Firmware driver.
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Frank Rowand <frowand.list@gmail.com>
> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > Tested-by: Coiby Xu <coxu@redhat.com>
> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
>
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

If I just pick tpm only patches they won't apply so maybe TPM changes
should be better separated if that is by any means possible.

Open for counter proposals. Just my thoughts...

I.e. I'm mainly wondering why TPM patches depend on IMA patches?

BR, Jarkko
Stefan Berger June 9, 2023, 6:49 p.m. UTC | #3 
On 6/9/23 14:18, Jarkko Sakkinen wrote:
> On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
>> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
>>> Simplify tpm_read_log_of() by moving reusable parts of the code into
>>> an inline function that makes it commonly available so it can be
>>> used also for kexec support. Call the new of_tpm_get_sml_parameters()
>>> function from the TPM Open Firmware driver.
>>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> Cc: Jarkko Sakkinen <jarkko@kernel.org>
>>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Frank Rowand <frowand.list@gmail.com>
>>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
>>> Tested-by: Coiby Xu <coxu@redhat.com>
>>> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>
>>
>> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> 
> If I just pick tpm only patches they won't apply so maybe TPM changes
> should be better separated if that is by any means possible.

Per the comment here I am putting this series here on hold.
https://lore.kernel.org/linux-integrity/20230418134409.177485-1-stefanb@linux.ibm.com/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7


BR,
    Stefan
Jarkko Sakkinen June 10, 2023, 9:25 a.m. UTC | #4 
On Fri Jun 9, 2023 at 9:49 PM EEST, Stefan Berger wrote:
>
>
> On 6/9/23 14:18, Jarkko Sakkinen wrote:
> > On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> >> On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> >>> Simplify tpm_read_log_of() by moving reusable parts of the code into
> >>> an inline function that makes it commonly available so it can be
> >>> used also for kexec support. Call the new of_tpm_get_sml_parameters()
> >>> function from the TPM Open Firmware driver.
> >>>
> >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> >>> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> >>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> >>> Cc: Rob Herring <robh+dt@kernel.org>
> >>> Cc: Frank Rowand <frowand.list@gmail.com>
> >>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> >>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> >>> Tested-by: Coiby Xu <coxu@redhat.com>
> >>> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>>
> >>
> >> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > 
> > If I just pick tpm only patches they won't apply so maybe TPM changes
> > should be better separated if that is by any means possible.
>
> Per the comment here I am putting this series here on hold.
> https://lore.kernel.org/linux-integrity/20230418134409.177485-1-stefanb@linux.ibm.com/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7

OK, cool.

I've mentioned this in few other emails but say this here too:
I was relocating for last couple of weeks, and thus some latency.
If you choose to repost the series, I'm happy to review it, thanks
:-)

BR, Jarkko
Jarkko Sakkinen June 28, 2023, 11:07 p.m. UTC | #5 
On Fri, 2023-06-09 at 14:49 -0400, Stefan Berger wrote:
> 
> On 6/9/23 14:18, Jarkko Sakkinen wrote:
> > On Thu May 25, 2023 at 1:56 AM EEST, Jerry Snitselaar wrote:
> > > On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> > > > Simplify tpm_read_log_of() by moving reusable parts of the code into
> > > > an inline function that makes it commonly available so it can be
> > > > used also for kexec support. Call the new of_tpm_get_sml_parameters()
> > > > function from the TPM Open Firmware driver.
> > > > 
> > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > > > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > > > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Frank Rowand <frowand.list@gmail.com>
> > > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > > > Tested-by: Coiby Xu <coxu@redhat.com>
> > > > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > 
> > > 
> > > Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > 
> > If I just pick tpm only patches they won't apply so maybe TPM changes
> > should be better separated if that is by any means possible.
> 
> Per the comment here I am putting this series here on hold.
> https://lore.kernel.org/linux-integrity/20230418134409.177485-1-stefanb@linux.ibm.com/T/#m03745c2af2c46f19f329522fcb6ccb2bf2eaedc7

Hi, sorry for late response. The Midsummer weekend really
messed my schedules (it is a big thing Finland). This year
the timing with the kernel cycle has been conflicting.

OK cool.

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
index 930fe43d5daf..41688d9cbd3b 100644
--- a/drivers/char/tpm/eventlog/of.c
+++ b/drivers/char/tpm/eventlog/of.c
@@ -51,11 +51,10 @@  static int tpm_read_log_memory_region(struct tpm_chip *chip)
 int tpm_read_log_of(struct tpm_chip *chip)
 {
 	struct device_node *np;
-	const u32 *sizep;
-	const u64 *basep;
 	struct tpm_bios_log *log;
 	u32 size;
 	u64 base;
+	int ret;
 
 	log = &chip->log;
 	if (chip->dev.parent && chip->dev.parent->of_node)
@@ -66,30 +65,11 @@  int tpm_read_log_of(struct tpm_chip *chip)
 	if (of_property_read_bool(np, "powered-while-suspended"))
 		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
 
-	sizep = of_get_property(np, "linux,sml-size", NULL);
-	basep = of_get_property(np, "linux,sml-base", NULL);
-	if (sizep == NULL && basep == NULL)
+	ret = of_tpm_get_sml_parameters(np, &base, &size);
+	if (ret == -ENODEV)
 		return tpm_read_log_memory_region(chip);
-	if (sizep == NULL || basep == NULL)
-		return -EIO;
-
-	/*
-	 * For both vtpm/tpm, firmware has log addr and log size in big
-	 * endian format. But in case of vtpm, there is a method called
-	 * sml-handover which is run during kernel init even before
-	 * device tree is setup. This sml-handover function takes care
-	 * of endianness and writes to sml-base and sml-size in little
-	 * endian format. For this reason, vtpm doesn't need conversion
-	 * but physical tpm needs the conversion.
-	 */
-	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
-	    of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
-		size = be32_to_cpup((__force __be32 *)sizep);
-		base = be64_to_cpup((__force __be64 *)basep);
-	} else {
-		size = *sizep;
-		base = *basep;
-	}
+	if (ret < 0)
+		return ret;
 
 	if (size == 0) {
 		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 4dc97b9f65fb..dd9a376a1dbb 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -461,4 +461,40 @@  static inline struct tpm_chip *tpm_default_chip(void)
 	return NULL;
 }
 #endif
+
+#ifdef CONFIG_OF
+static inline int of_tpm_get_sml_parameters(struct device_node *np,
+					    u64 *base, u32 *size)
+{
+	const u32 *sizep;
+	const u64 *basep;
+
+	sizep = of_get_property(np, "linux,sml-size", NULL);
+	basep = of_get_property(np, "linux,sml-base", NULL);
+	if (sizep == NULL && basep == NULL)
+		return -ENODEV;
+	if (sizep == NULL || basep == NULL)
+		return -EIO;
+
+	/*
+	 * For both vtpm/tpm, firmware has log addr and log size in big
+	 * endian format. But in case of vtpm, there is a method called
+	 * sml-handover which is run during kernel init even before
+	 * device tree is setup. This sml-handover function takes care
+	 * of endianness and writes to sml-base and sml-size in little
+	 * endian format. For this reason, vtpm doesn't need conversion
+	 * but physical tpm needs the conversion.
+	 */
+	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 &&
+	    of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) {
+		*size = be32_to_cpup((__force __be32 *)sizep);
+		*base = be64_to_cpup((__force __be64 *)basep);
+	} else {
+		*size = *sizep;
+		*base = *basep;
+	}
+	return 0;
+}
+#endif
+
 #endif