Received-SPF: pass (google.com: domain of linux-crypto+bounces-4743-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <f9cd1025-700d-423f-a5c2-182d625b3440@oracle.com>
Date: Tue, 4 Jun 2024 16:04:15 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v9 16/19] tpm: Add ability to set the preferred locality
 the TPM chip uses
To: Jarkko Sakkinen <jarkko@kernel.org>, linux-kernel@vger.kernel.org,
        x86@kernel.org, linux-integrity@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-crypto@vger.kernel.org,
        kexec@lists.infradead.org, linux-efi@vger.kernel.org,
        iommu@lists.linux-foundation.org
Cc: dpsmith@apertussolutions.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, hpa@zytor.com, dave.hansen@linux.intel.com,
        ardb@kernel.org, mjg59@srcf.ucam.org,
        James.Bottomley@hansenpartnership.com, peterhuewe@gmx.de, jgg@ziepe.ca,
        luto@amacapital.net, nivedita@alum.mit.edu,
        herbert@gondor.apana.org.au, davem@davemloft.net, corbet@lwn.net,
        ebiederm@xmission.com, dwmw2@infradead.org, baolu.lu@linux.intel.com,
        kanth.ghatraju@oracle.com, andrew.cooper3@citrix.com,
        trenchboot-devel@googlegroups.com, ross.philipson@oracle.com
References: <20240531010331.134441-1-ross.philipson@oracle.com>
 <20240531010331.134441-17-ross.philipson@oracle.com>
 <D1RIKP548WG9.Q5MM7K3FRH4P@kernel.org>
 <282fddde-d156-4bd3-906b-0318d0a3746d@oracle.com>
 <D1RLMF7VZ0BQ.1Y0SVHN39UQNN@kernel.org>
Content-Language: en-US
From: ross.philipson@oracle.com
In-Reply-To: <D1RLMF7VZ0BQ.1Y0SVHN39UQNN@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?VnFKYWI5bldQeG5SM0h4b0wxZVBtbnVaTlFHbzZpb2NOcDUzd0cvYlpKYzh4?=
 =?utf-8?B?QmlHdlZLdTJmSXhBSkwzOC9adE5Wa3lGS0w3OEkyY0laaEtCVmJaM2NwRXZn?=
 =?utf-8?B?UUVxMGI2RTNDTUdnTWpBSmZvV25xV1FMWE5VY2R6YnNQSWI4L1lHVlowWjdR?=
 =?utf-8?B?WDljVm80bVkvMHhrazRMUGNkOS81aEdGT1lWNG1FdTl5dExCanUzd201WGhn?=
 =?utf-8?B?bklWcCtWdm9HOXRIQ3QzcXpCYm5rczhpam9Vekt0clNlc1dMWGNybk55aWxs?=
 =?utf-8?B?Mis2dkxrOUsrcFVybjR6bkxUSkdpYUNkbW9nNVlkanRXa3BEU0NHUnZBUmV4?=
 =?utf-8?B?VFU0UThZZlFKaXl4T2xGWFBVQW1qM0ZWSFN6QW10c0tRK2s0YXVIeXhyQ1JD?=
 =?utf-8?B?L1BoaS8rZU9QZTc1OG9SRUxoUnVDd3RNRzRublVGY1V2MnJTOXNNQklobmZu?=
 =?utf-8?B?UW5ySnhZa3BHRkc2OUR2SUFNbzhSQlh3alUwQmhwZFhDaEd3TU0vbUZtOTRm?=
 =?utf-8?B?MUJ3SEtEVVhpUEc5djFuYlZHdzA2R1VyanNKZnVEODJDTTBkNGF5dWhQWER1?=
 =?utf-8?B?VkVRTzhFVURoRFJEUkFlaUtmdGg0WXZVTUIwNVU3YU0vUU14UjNnTGZtNzFv?=
 =?utf-8?B?MmRzWGJ2SVVxQU9CblBsVXY5U05FNTcvNnZ6aFd4T1NwT01mbXBPZ2R2ZVVp?=
 =?utf-8?B?aDhyYUVkazJidU9EakdFWVJEUVRVSmNIWjNNekl6UXNtMkdrYmdGSWUyR1pC?=
 =?utf-8?B?NFpReG1xcGdIZXJLTXdvNHZmSXM2T1dhMjRWNXZRbFR6cjRNOE9tTnpkQlRM?=
 =?utf-8?B?eUNKN2NaOTZTMlZCaU9obmVCZ0ZweXZXUWVZWSs0TVRSTjdqTnNyWkNEZnA3?=
 =?utf-8?B?UnIrTENQNCtPc3czL2ZJRUxHTE05aDlFeE1XRXBaOVFHQkNmSVYwdDNHRUwv?=
 =?utf-8?B?WEM2SjFmVzZFK0VBY1IzT2pqWldqSDNNaC80c2VveEVMdFFvMXFNYm1CZGo5?=
 =?utf-8?B?R1FHWGxVQ3ZWNldPNkxxVDBIczV4bHhGYUtUbTd5U2F6YWxCMG14bTMyQW5H?=
 =?utf-8?B?SnpabFEyeVAyVnNtNk5aNzZ2QkYwYnRSbHlWa2VYWXExajBKZkxuTStmL2dJ?=
 =?utf-8?B?d3JYQ1QxU0MrZnVmaUc5TnZMS01YM0gvQnhtK2gxN1VLN1M0TVVwYjk2YzhU?=
 =?utf-8?B?blh1dk9wbklodTEvNjBDcnNkVGNPb1FBNDJDTGg2dnFOUk91Wm1XUXNHd05j?=
 =?utf-8?B?djM0Vno2NEg3cjNvWEh1Y254NEJxQ1pYc1VHVTMzalJDdWczcWxpQmZYQWF2?=
 =?utf-8?B?TGFSWjMzVE1LTmhOejNIUHlLdHAxc0xpWklZYVNPYktkMk91SnpDQ1lCTity?=
 =?utf-8?B?Ymh4TUdFcm9WN2taOFF5S0ZQTzFCOHZlV08vYVdtNmJES014MWRJR2lQT1p5?=
 =?utf-8?B?R280ZGhSV0tua3FDVDc4S2E0dUVtREhxK3hNcGg3OEtzd1cvWTc2RDZTckw0?=
 =?utf-8?B?VitSVjdlUVBBMjdsbW1TRGVMUjlwV3hnTkxvd0FEek92VkNpQjR0TmcrNDVQ?=
 =?utf-8?B?ZkZhRGJDNkVhNnV4djlIN0MwU1RwOG9hbzUyZWcvZ3hVYmhPM0FXTVlFMUJu?=
 =?utf-8?B?ZVk0RUpPQWpMNFRFUGJlcVc5TFhyelBYaVdybVE2dXJRWDZmdzZFVFBTQzNZ?=
 =?utf-8?B?dEQ1ZmlucHBocUtRblQ5dWQ2OG4rK0VXcmxIRHJHZno2Wi9vZmw3elV0MGIx?=
 =?utf-8?B?dWRsbkpNWDl0ZnROcUhLY09id2RTaDBhTitJZlhxMW5KMHFhKzJTZUw5bzBW?=
 =?utf-8?B?MHAxcE4vcTlna3NXM1Y0RHNsbjAvbVJHKytPdUZyUlpsZUltN25EZlhBam9u?=
 =?utf-8?B?SDlzclEwMFRoS3MzWHVEV3pjY2RIcEtuM1ArbHpzaFIvWVlpNXpoR0dYSnpq?=
 =?utf-8?B?ZzVuK1NDSmowZ3JuMFJiei9PbWVCcU9UTU0wSklscE0vQll3aDI0b3M5bktl?=
 =?utf-8?B?V3VjcSswQ3ZVNlJhVCtVM1U4cmFwWjYvQ0swWDJxOEdZT0RRUWI5bWtpbFBZ?=
 =?utf-8?B?aXpLcWtTWVU5cDNxeFphQ1ZzL0kxYjhxT0tWN2xiR0hrK2pBWERpYjlnNTFN?=
 =?utf-8?B?S0gyaWN5TnZnemkya0JlNHd4Qi9ndk9yTENPajZCSXVkRnFITHpQT2JOb1R3?=
 =?utf-8?B?WGc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	wWQSVSL7BKw+SlGl1VmwRbTnVK8Jr6k0CyVV43TttOgALxbwc9psK2QHRRMc12bfKSAoDRSIZgYd1nW+ecGAdLZeOWEM7kqGdljGlQWJXb7feMP0GY0ua72w7coCoR5bte/nLhlkymuIh0vjLTT0Al5eZSZOqLlzajHrVria+yiA8T8dQgD/NCD1O49HjUL+sGZQ9fLxS8zHtIgZSEbeZg2TEMc7oBzQGMVH20QYlpHDoprbmncXwtVJb8u4ig7/ANHh8yGHNRzkiSrNd+fRUZsEUAc+Lg4j+4Va+sLTaRmkc0hksBftkn/F6pWwmjoR2G2a/A/+qz8z0VIwKAkElmdN/aUbintcDhg+I+LybjTtZhfQRpG1MmzQ5Dr5zxab8nAnw6dkt5TopXkkitje2HXoEJmSRueMO95aXXwgCT1HX8iatN63NKhNTsdJpXvWr7EdXD5A+NX6W0NwXIK1kzOH9B8/J90EyCxXs8T3leR2XKcr4M96HTsgNX3FM707gv1s5lkgFbbukoXPJ59JrHepxLG/IssM58pxYp2suszy5nbX3Qpv15Eemhwp5Dms6TNnobi1oSsxNfy3Y3GscU2pJ+UKB+la4cv7Dps4ZU8=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a329e3ac-33b4-4d66-c77d-08dc84eab077
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7224.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 23:04:30.0096
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6QeydzdyZhPj70W+084gVK9Tbwuq+nns1jDigk0uhc/fHJn03DLxuwyxI7qBUztdm4i7hufmX3Pca+J9KF4PIJQk8Q9sUZmby6aK123SI4A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB4833
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-04_11,2024-06-04_02,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 spamscore=0
 malwarescore=0 suspectscore=0 phishscore=0 mlxlogscore=999 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2406040187
X-Proofpoint-GUID: u1uqOaelVb-YNDSrb5rcZ3GvnwFjgSpi
X-Proofpoint-ORIG-GUID: u1uqOaelVb-YNDSrb5rcZ3GvnwFjgSpi

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
>