1. Patchew
  2. linux
  3. [PATCH v9 00/19] x86: Trenchboot secure dynamic launch Linux kernel support
  4. View patch

[PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses

Ross Philipson posted 19 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses
Posted by Ross Philipson 1 year, 6 months ago 
Curently the locality is hard coded to 0 but for DRTM support, access
is needed to localities 1 through 4.

Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
---
 drivers/char/tpm/tpm-chip.c      | 24 +++++++++++++++++++++++-
 drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
 drivers/char/tpm/tpm.h           |  1 +
 include/linux/tpm.h              |  4 ++++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 854546000c92..73eac54d61fb 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
 	if (!chip->ops->request_locality)
 		return 0;
 
-	rc = chip->ops->request_locality(chip, 0);
+	rc = chip->ops->request_locality(chip, chip->pref_locality);
 	if (rc < 0)
 		return rc;
 
@@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm_chip_stop);
 
+/**
+ * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
+ * @chip:	a TPM chip to use
+ * @locality:   the preferred locality
+ *
+ * Return:
+ * * true      - Preferred locality set
+ * * false     - Invalid locality specified
+ */
+bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
+{
+	if (locality < 0 || locality >=TPM_MAX_LOCALITY)
+		return false;
+
+	mutex_lock(&chip->tpm_mutex);
+	chip->pref_locality = locality;
+	mutex_unlock(&chip->tpm_mutex);
+	return true;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
+
 /**
  * tpm_try_get_ops() - Get a ref to the tpm_chip
  * @chip: Chip to ref
@@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	}
 
 	chip->locality = -1;
+	chip->pref_locality = 0;
 	return chip;
 
 out:
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 5da134f12c9a..35f14ccecf0e 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm_is_tpm2);
 
+/**
+ * tpm_preferred_locality() - set the TPM chip preferred locality to open
+ * @chip:	a TPM chip to use
+ * @locality:   the preferred locality
+ *
+ * Return:
+ * * true      - Preferred locality set
+ * * false     - Invalid locality specified
+ */
+bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
+{
+	return tpm_chip_preferred_locality(chip, locality);
+}
+EXPORT_SYMBOL_GPL(tpm_preferred_locality);
+
 /**
  * tpm_pcr_read - read a PCR value from SHA1 bank
  * @chip:	a &struct tpm_chip instance, %NULL for the default chip
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6b8b9956ba69..be465422d3fa 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -267,6 +267,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
 int tpm_chip_bootstrap(struct tpm_chip *chip);
 int tpm_chip_start(struct tpm_chip *chip);
 void tpm_chip_stop(struct tpm_chip *chip);
+bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality);
 struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
 
 struct tpm_chip *tpm_chip_alloc(struct device *dev,
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 363f7078c3a9..935a3457d7c8 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -219,6 +219,9 @@ struct tpm_chip {
 	u8 null_ec_key_y[EC_PT_SZ];
 	struct tpm2_auth *auth;
 #endif
+
+	/* preferred locality - default 0 */
+	int pref_locality;
 };
 
 #define TPM_HEADER_SIZE		10
@@ -461,6 +464,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);
+extern bool tpm_preferred_locality(struct tpm_chip *chip, int locality);
 extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
 extern void tpm_put_ops(struct tpm_chip *chip);
 extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-- 
2.39.3
Re: [PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses
Posted by Jarkko Sakkinen 1 year, 6 months ago 
On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
> Curently the locality is hard coded to 0 but for DRTM support, access
> is needed to localities 1 through 4.
>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> ---
>  drivers/char/tpm/tpm-chip.c      | 24 +++++++++++++++++++++++-
>  drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
>  drivers/char/tpm/tpm.h           |  1 +
>  include/linux/tpm.h              |  4 ++++
>  4 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 854546000c92..73eac54d61fb 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
>  	if (!chip->ops->request_locality)
>  		return 0;
>  
> -	rc = chip->ops->request_locality(chip, 0);
> +	rc = chip->ops->request_locality(chip, chip->pref_locality);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm_chip_stop);
>  
> +/**
> + * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
> + * @chip:	a TPM chip to use
> + * @locality:   the preferred locality
> + *
> + * Return:
> + * * true      - Preferred locality set
> + * * false     - Invalid locality specified
> + */
> +bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
> +{
> +	if (locality < 0 || locality >=TPM_MAX_LOCALITY)
> +		return false;
> +
> +	mutex_lock(&chip->tpm_mutex);
> +	chip->pref_locality = locality;
> +	mutex_unlock(&chip->tpm_mutex);
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
> +
>  /**
>   * tpm_try_get_ops() - Get a ref to the tpm_chip
>   * @chip: Chip to ref
> @@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	}
>  
>  	chip->locality = -1;
> +	chip->pref_locality = 0;
>  	return chip;
>  
>  out:
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 5da134f12c9a..35f14ccecf0e 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>  
> +/**
> + * tpm_preferred_locality() - set the TPM chip preferred locality to open
> + * @chip:	a TPM chip to use
> + * @locality:   the preferred locality
> + *
> + * Return:
> + * * true      - Preferred locality set
> + * * false     - Invalid locality specified
> + */
> +bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
> +{
> +	return tpm_chip_preferred_locality(chip, locality);
> +}
> +EXPORT_SYMBOL_GPL(tpm_preferred_locality);

 What good does this extra wrapping do?

 tpm_set_default_locality() and default_locality would make so much more
 sense in any case.

 BR, Jarkko
Re: [PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses
Posted by ross.philipson@oracle.com 1 year, 6 months ago 
On 6/4/24 1:27 PM, Jarkko Sakkinen wrote:
> On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
>> Curently the locality is hard coded to 0 but for DRTM support, access
>> is needed to localities 1 through 4.
>>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>> ---
>>   drivers/char/tpm/tpm-chip.c      | 24 +++++++++++++++++++++++-
>>   drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
>>   drivers/char/tpm/tpm.h           |  1 +
>>   include/linux/tpm.h              |  4 ++++
>>   4 files changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 854546000c92..73eac54d61fb 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
>>   	if (!chip->ops->request_locality)
>>   		return 0;
>>   
>> -	rc = chip->ops->request_locality(chip, 0);
>> +	rc = chip->ops->request_locality(chip, chip->pref_locality);
>>   	if (rc < 0)
>>   		return rc;
>>   
>> @@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
>>   }
>>   EXPORT_SYMBOL_GPL(tpm_chip_stop);
>>   
>> +/**
>> + * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
>> + * @chip:	a TPM chip to use
>> + * @locality:   the preferred locality
>> + *
>> + * Return:
>> + * * true      - Preferred locality set
>> + * * false     - Invalid locality specified
>> + */
>> +bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
>> +{
>> +	if (locality < 0 || locality >=TPM_MAX_LOCALITY)
>> +		return false;
>> +
>> +	mutex_lock(&chip->tpm_mutex);
>> +	chip->pref_locality = locality;
>> +	mutex_unlock(&chip->tpm_mutex);
>> +	return true;
>> +}
>> +EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
>> +
>>   /**
>>    * tpm_try_get_ops() - Get a ref to the tpm_chip
>>    * @chip: Chip to ref
>> @@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>>   	}
>>   
>>   	chip->locality = -1;
>> +	chip->pref_locality = 0;
>>   	return chip;
>>   
>>   out:
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 5da134f12c9a..35f14ccecf0e 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
>>   }
>>   EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>>   
>> +/**
>> + * tpm_preferred_locality() - set the TPM chip preferred locality to open
>> + * @chip:	a TPM chip to use
>> + * @locality:   the preferred locality
>> + *
>> + * Return:
>> + * * true      - Preferred locality set
>> + * * false     - Invalid locality specified
>> + */
>> +bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
>> +{
>> +	return tpm_chip_preferred_locality(chip, locality);
>> +}
>> +EXPORT_SYMBOL_GPL(tpm_preferred_locality);
> 
>   What good does this extra wrapping do?
> 
>   tpm_set_default_locality() and default_locality would make so much more
>   sense in any case.

Are you mainly just talking about my naming choices here and in the 
follow-on response? Can you clarify what you are requesting?

Thanks
Ross

> 
>   BR, Jarkko
Re: [PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses
Posted by Jarkko Sakkinen 1 year, 6 months ago 
On Wed Jun 5, 2024 at 1:14 AM EEST,  wrote:
> On 6/4/24 1:27 PM, Jarkko Sakkinen wrote:
> > On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
> >> Curently the locality is hard coded to 0 but for DRTM support, access
> >> is needed to localities 1 through 4.
> >>
> >> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> >> ---
> >>   drivers/char/tpm/tpm-chip.c      | 24 +++++++++++++++++++++++-
> >>   drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
> >>   drivers/char/tpm/tpm.h           |  1 +
> >>   include/linux/tpm.h              |  4 ++++
> >>   4 files changed, 43 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >> index 854546000c92..73eac54d61fb 100644
> >> --- a/drivers/char/tpm/tpm-chip.c
> >> +++ b/drivers/char/tpm/tpm-chip.c
> >> @@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
> >>   	if (!chip->ops->request_locality)
> >>   		return 0;
> >>   
> >> -	rc = chip->ops->request_locality(chip, 0);
> >> +	rc = chip->ops->request_locality(chip, chip->pref_locality);
> >>   	if (rc < 0)
> >>   		return rc;
> >>   
> >> @@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
> >>   }
> >>   EXPORT_SYMBOL_GPL(tpm_chip_stop);
> >>   
> >> +/**
> >> + * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
> >> + * @chip:	a TPM chip to use
> >> + * @locality:   the preferred locality
> >> + *
> >> + * Return:
> >> + * * true      - Preferred locality set
> >> + * * false     - Invalid locality specified
> >> + */
> >> +bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
> >> +{
> >> +	if (locality < 0 || locality >=TPM_MAX_LOCALITY)
> >> +		return false;
> >> +
> >> +	mutex_lock(&chip->tpm_mutex);
> >> +	chip->pref_locality = locality;
> >> +	mutex_unlock(&chip->tpm_mutex);
> >> +	return true;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
> >> +
> >>   /**
> >>    * tpm_try_get_ops() - Get a ref to the tpm_chip
> >>    * @chip: Chip to ref
> >> @@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
> >>   	}
> >>   
> >>   	chip->locality = -1;
> >> +	chip->pref_locality = 0;
> >>   	return chip;
> >>   
> >>   out:
> >> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> >> index 5da134f12c9a..35f14ccecf0e 100644
> >> --- a/drivers/char/tpm/tpm-interface.c
> >> +++ b/drivers/char/tpm/tpm-interface.c
> >> @@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
> >>   }
> >>   EXPORT_SYMBOL_GPL(tpm_is_tpm2);
> >>   
> >> +/**
> >> + * tpm_preferred_locality() - set the TPM chip preferred locality to open
> >> + * @chip:	a TPM chip to use
> >> + * @locality:   the preferred locality
> >> + *
> >> + * Return:
> >> + * * true      - Preferred locality set
> >> + * * false     - Invalid locality specified
> >> + */
> >> +bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
> >> +{
> >> +	return tpm_chip_preferred_locality(chip, locality);
> >> +}
> >> +EXPORT_SYMBOL_GPL(tpm_preferred_locality);
> > 
> >   What good does this extra wrapping do?
> > 
> >   tpm_set_default_locality() and default_locality would make so much more
> >   sense in any case.
>
> Are you mainly just talking about my naming choices here and in the 
> follow-on response? Can you clarify what you are requesting?

I'd prefer:

1. Name the variable as default_locality.
2. Only create a single expored to function to tpm-chip.c:
   tpm_chip_set_default_locality().
3. Call this function in all call sites.

"tpm_preferred_locality" should be just removed, as tpm_chip_*
is exported anyway.

BR, Jarkko
Re: [PATCH v9 16/19] tpm: Add ability to set the preferred locality the TPM chip uses
Posted by ross.philipson@oracle.com 1 year, 6 months ago 
On 6/4/24 3:50 PM, Jarkko Sakkinen wrote:
> On Wed Jun 5, 2024 at 1:14 AM EEST,  wrote:
>> On 6/4/24 1:27 PM, Jarkko Sakkinen wrote:
>>> On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
>>>> Curently the locality is hard coded to 0 but for DRTM support, access
>>>> is needed to localities 1 through 4.
>>>>
>>>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>>>> ---
>>>>    drivers/char/tpm/tpm-chip.c      | 24 +++++++++++++++++++++++-
>>>>    drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++
>>>>    drivers/char/tpm/tpm.h           |  1 +
>>>>    include/linux/tpm.h              |  4 ++++
>>>>    4 files changed, 43 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>>>> index 854546000c92..73eac54d61fb 100644
>>>> --- a/drivers/char/tpm/tpm-chip.c
>>>> +++ b/drivers/char/tpm/tpm-chip.c
>>>> @@ -44,7 +44,7 @@ static int tpm_request_locality(struct tpm_chip *chip)
>>>>    	if (!chip->ops->request_locality)
>>>>    		return 0;
>>>>    
>>>> -	rc = chip->ops->request_locality(chip, 0);
>>>> +	rc = chip->ops->request_locality(chip, chip->pref_locality);
>>>>    	if (rc < 0)
>>>>    		return rc;
>>>>    
>>>> @@ -143,6 +143,27 @@ void tpm_chip_stop(struct tpm_chip *chip)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(tpm_chip_stop);
>>>>    
>>>> +/**
>>>> + * tpm_chip_preferred_locality() - set the TPM chip preferred locality to open
>>>> + * @chip:	a TPM chip to use
>>>> + * @locality:   the preferred locality
>>>> + *
>>>> + * Return:
>>>> + * * true      - Preferred locality set
>>>> + * * false     - Invalid locality specified
>>>> + */
>>>> +bool tpm_chip_preferred_locality(struct tpm_chip *chip, int locality)
>>>> +{
>>>> +	if (locality < 0 || locality >=TPM_MAX_LOCALITY)
>>>> +		return false;
>>>> +
>>>> +	mutex_lock(&chip->tpm_mutex);
>>>> +	chip->pref_locality = locality;
>>>> +	mutex_unlock(&chip->tpm_mutex);
>>>> +	return true;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(tpm_chip_preferred_locality);
>>>> +
>>>>    /**
>>>>     * tpm_try_get_ops() - Get a ref to the tpm_chip
>>>>     * @chip: Chip to ref
>>>> @@ -374,6 +395,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>>>>    	}
>>>>    
>>>>    	chip->locality = -1;
>>>> +	chip->pref_locality = 0;
>>>>    	return chip;
>>>>    
>>>>    out:
>>>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>>> index 5da134f12c9a..35f14ccecf0e 100644
>>>> --- a/drivers/char/tpm/tpm-interface.c
>>>> +++ b/drivers/char/tpm/tpm-interface.c
>>>> @@ -274,6 +274,21 @@ int tpm_is_tpm2(struct tpm_chip *chip)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>>>>    
>>>> +/**
>>>> + * tpm_preferred_locality() - set the TPM chip preferred locality to open
>>>> + * @chip:	a TPM chip to use
>>>> + * @locality:   the preferred locality
>>>> + *
>>>> + * Return:
>>>> + * * true      - Preferred locality set
>>>> + * * false     - Invalid locality specified
>>>> + */
>>>> +bool tpm_preferred_locality(struct tpm_chip *chip, int locality)
>>>> +{
>>>> +	return tpm_chip_preferred_locality(chip, locality);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(tpm_preferred_locality);
>>>
>>>    What good does this extra wrapping do?
>>>
>>>    tpm_set_default_locality() and default_locality would make so much more
>>>    sense in any case.
>>
>> Are you mainly just talking about my naming choices here and in the
>> follow-on response? Can you clarify what you are requesting?
> 
> I'd prefer:
> 
> 1. Name the variable as default_locality.
> 2. Only create a single expored to function to tpm-chip.c:
>     tpm_chip_set_default_locality().
> 3. Call this function in all call sites.
> 
> "tpm_preferred_locality" should be just removed, as tpm_chip_*
> is exported anyway.

Ok got it, thanks.

> 
> BR, Jarkko
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.