Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <61eb38c9-e40d-456a-86c8-e2d851b947f8@siemens.com>
Date:   Thu, 2 Nov 2023 09:05:27 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/2] tee: optee: Fix supplicant based device
 enumeration
Content-Language: en-US
To:     Sumit Garg <sumit.garg@linaro.org>
Cc:     Jerome Forissier <jerome.forissier@linaro.org>,
        jens.wiklander@linaro.org, op-tee@lists.trustedfirmware.org,
        arnd@linaro.org, ardb@kernel.org, ilias.apalodimas@linaro.org,
        masahisa.kojima@linaro.org, maxim.uvarov@linaro.org,
        jarkko.sakkinen@linux.intel.com, linux-kernel@vger.kernel.org,
        diogo.ivo@siemens.com
References: <20231030155901.80673-1-sumit.garg@linaro.org>
 <20231030155901.80673-2-sumit.garg@linaro.org>
 <a54b6109-5a38-4d72-a197-00fee0162860@linaro.org>
 <1418e74e-175f-4b04-90d7-8bec2987c207@siemens.com>
 <CAFA6WYNe+pt2meP-KDtG2duYqWXHHwTCnHnm97PeNsbi+PJ5ng@mail.gmail.com>
 <cb729887-13ba-4fe4-8824-3062aeb1ef8e@siemens.com>
 <CAFA6WYN-9dYRbBNM8q=H1DE7YMH4_z2fJpyvSG+W=Hy=Vo8NrQ@mail.gmail.com>
From:   Jan Kiszka <jan.kiszka@siemens.com>
In-Reply-To: <CAFA6WYN-9dYRbBNM8q=H1DE7YMH4_z2fJpyvSG+W=Hy=Vo8NrQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MHR2V21zcW1qRTBRaW9jL2NIeGRMdWx3MDdvWjNXeTF1TzNoaWJ2UHpST2Za?=
 =?utf-8?B?RmVCT0g2aDB0WWhhd005TDFUZnQ3OFdJWUxlbVVJQkptVGROY1ZOWmdBTTFy?=
 =?utf-8?B?eS80NHhiSVJPdWkwc2Q0SHhMMWdxNnlpUS9MckVzbjRVVGpuaWZXeEhlNTdH?=
 =?utf-8?B?OXJoT0hJZ2hWNThiaEtrUFJDU2RLNUZ0ZTB6UUUyV3cvL3M5MDVFWXEyU0Vo?=
 =?utf-8?B?RlpaR29DT1Y0bVhpTmo4a01aaFBjS1dkMXo3OHJvaFh3RlV2bWhEMDgrRUZi?=
 =?utf-8?B?R1BPa3UrcUhhbWZYSG1vdklleVFYeHZGZklnNXYrSnNLcHVPbnZwM0J1QmNE?=
 =?utf-8?B?aitSd2tSNEQzbldBVGVrZ3UycngvaWhLVEQySzZkM3VRSHo0WGw2Uy84OGZW?=
 =?utf-8?B?MUs0OTcydFZSNi91b3Z6M1E3VkRuN1VHakMzaWdvRHhPU05TQjYvOGJwYTBO?=
 =?utf-8?B?NWZKUVFwVnVZcE11NGR6Qjd5ekk2WDAzVUhLNzh3WjFlZlMrMXpWV3k1OUNh?=
 =?utf-8?B?R3Ntc1h2OGZGOU5XVHlCWSt5OEZjVW9vejFwTHZ3eU5aL3gzK3A4UTZLUlV3?=
 =?utf-8?B?eXVodWdPdHhqZlZjcEdNWWJQamZ1cnBJaFNqTDVwbUZ2QjJkL1QveXdWRnFh?=
 =?utf-8?B?dlBkTFJZSGdtWTJxcG9keHNOTXlMWEtUd3F4RUlVM05teG1GVUtaS2xiWFBM?=
 =?utf-8?B?bGVwaVBxTS9VMXRvMG9FY2g5Qm42ZWZ0L09KOFlVZ0FueGNyOTlwZW5sb3ox?=
 =?utf-8?B?ZjZBTFFVblZmOEJoamZBRytWS3RaOVM2YU43akp5ZnR0eW1jQi9FTUpQMXN5?=
 =?utf-8?B?c3hhNGwrdVd1Z0Zpd1gvbGorVit1VjZ1ekJsY0psMlU2UkVaUUlpeDVsemlV?=
 =?utf-8?B?dHUwVEFlcThZaUlzaXZKb3U1bW90YUVDMFVYZGUxaTNvUXpwQWJGNXU1MGZ2?=
 =?utf-8?B?OGI5QU9iWWYrNS9KS1I0SUlxQmVYWWxaUzZnSE9XRGlOVFVYTXZXTU1vUFl5?=
 =?utf-8?B?WlpTaWMrT0doc2ROMkhLUGxKci9pQlM2cXVtd0FVM3pYb3NSazkrMHphb1NJ?=
 =?utf-8?B?Qi9QYWhoWVRSc1Qrc1M1MjlBT013Y0NKMDlJZTBuWmNYQVdQNDJPSmRXbzRj?=
 =?utf-8?B?d2Nna3N1d3RJRnNMNE5NYjAraHN2Vld0bzBOQkgzK2NobEZxOC95eGFscFNK?=
 =?utf-8?B?aExuVTdhZWpVR01DZlhPVm5CSHQ1Mkd2OG1uTnk3WDVoWFFXOGRTNkZSWVhj?=
 =?utf-8?B?Vm1mT3FDL2hmU05qTTA2UVAxQ01LNS9mSlhLQjJsc2QyUVVWcE1YTnFDdElX?=
 =?utf-8?B?WDhtdlkrK3hOTjBPOEoxeDZqNFRmS213UzhwcUV5L1EwOUhyRGRZUXFTQktE?=
 =?utf-8?B?U25WakRoaHQ0RG5Yd2xNcE04Z2NCNlVqZDAzeXdsam1MNS9oR0M1UDRta050?=
 =?utf-8?B?V0VBaTV5bmxsbmROanA2N1czSnNQaWhSK2pEcXIyMlhYaGdXS0RTVjREZW9W?=
 =?utf-8?B?amwyYy9DTytwekxFQWErYXZIU21ER3FWN3M2d0FxYjNlK0pRVUcrM3VYRk5F?=
 =?utf-8?B?UkUxMHFGbGdRVldqdmV0bGlRSVNjb3F2bXpOaG90anBOWFlzbVRjRlhCYWxj?=
 =?utf-8?B?K2E1VVphS2tCSEFJem1PVlhjVDFpYUlEcFM3bXU1OWZrb2p0cTkzTmZvK2ox?=
 =?utf-8?B?c2dFbDlYVERJU3h5ZnlRRXR5SVlhZVN0STV4TDVaUWswMVVJMitRNmIxWXdF?=
 =?utf-8?B?TnRieXVBSXIyVzNaUkFWOVJwL1YzYUY5QitMeXlIUXdSVUw2cmwydFpLTEUr?=
 =?utf-8?B?VzJTMm5KMkRXakJxOXRXd1ZHYi85YjR4VmxnbmlWZWltMVVZcS9vT2d6QzZ5?=
 =?utf-8?B?dUlkanlwRE9KMytqRkVLM2piZnJhS0o2VHBUOC9od1BNU2wzdWQ2MHpVYkJ4?=
 =?utf-8?B?OVEzc1JiMzlhcnBMUDJaYklWcGhMOE5FWm92QkVDSVZSSlVvbVR5VmVhNS9J?=
 =?utf-8?B?cGVwbVJRTUFSMkFpWTRUSTdiTmZyOGVTb1RvZ0hZRVkvclZ5N0wrektockdR?=
 =?utf-8?B?d1RSWmlqRWlVMnRpcSsrQkpzRmRLdFR2V3ppN1ppMjdvdFloZ2Q5ZHFJako0?=
 =?utf-8?B?MC9ucG1SM0srN01oVjNiaVMxKzFmM3JsL3VTSkxJdWVZSUp6b2E0M2RWREtn?=
 =?utf-8?B?aVE9PQ==?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 17742225-ae78-4b13-0589-08dbdb7a7b17
X-MS-Exchange-CrossTenant-AuthSource: AS4PR10MB6181.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2023 08:05:30.1602
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NBqkIQwpFwqbI+LIGr2loLQ87KbalLWgA8YfDZqOzOEqxMxwmLMlTdFsuAvhtmFmPTdjHrhpmzg39CCF+n1BxQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS1PR10MB5650
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 02 Nov 2023 01:05:45 -0700 (PDT)

On 02.11.23 09:02, Sumit Garg wrote:
> On Thu, 2 Nov 2023 at 13:28, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 02.11.23 08:37, Sumit Garg wrote:
>>> On Tue, 31 Oct 2023 at 17:14, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>
>>>> On 31.10.23 12:04, Jerome Forissier wrote:
>>>>>
>>>>>
>>>>> On 10/30/23 16:59, Sumit Garg wrote:
>>>>>> Currently supplicant dependent optee device enumeration only registers
>>>>>> devices whenever tee-supplicant is invoked for the first time. But it
>>>>>> forgets to remove devices when tee-supplicant daemon stops running and
>>>>>> closes its context gracefully. This leads to following error for fTPM
>>>>>> driver during reboot/shutdown:
>>>>>>
>>>>>> [   73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
>>>>>>
>>>>>> Fix this by separating supplicant dependent devices so that the
>>>>>> user-space service can detach supplicant devices before closing the
>>>>>> supplicant.
>>>>>>
>>>>>> Reported-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> Link: https://github.com/OP-TEE/optee_os/issues/6094
>>>>>> Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration")
>>>>>> Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> Tested-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>>>>>> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
>>>>>> ---
>>>>>>  drivers/tee/optee/device.c | 13 ++++++++++---
>>>>>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c
>>>>>> index 64f0e047c23d..78fc0a15c463 100644
>>>>>> --- a/drivers/tee/optee/device.c
>>>>>> +++ b/drivers/tee/optee/device.c
>>>>>> @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev)
>>>>>>      kfree(optee_device);
>>>>>>  }
>>>>>>
>>>>>> -static int optee_register_device(const uuid_t *device_uuid)
>>>>>> +static int optee_register_device(const uuid_t *device_uuid, u32 func)
>>>>>>  {
>>>>>>      struct tee_client_device *optee_device = NULL;
>>>>>> +    const char *dev_name_fmt = NULL;
>>>>>>      int rc;
>>>>>>
>>>>>>      optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
>>>>>> @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
>>>>>>
>>>>>>      optee_device->dev.bus = &tee_bus_type;
>>>>>>      optee_device->dev.release = optee_release_device;
>>>>>> -    if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
>>>>>> +
>>>>>> +    if (func == PTA_CMD_GET_DEVICES_SUPP)
>>>>>> +            dev_name_fmt = "optee-ta-supp-%pUb";
>>>>>> +    else
>>>>>> +            dev_name_fmt = "optee-ta-%pUb";
>>>>>
>>>>> That's an ABI change, isn't it?
>>>>
>>>
>>> Indeed it is an ABI break although we would like this to be backported
>>> but don't want to break existing users. So I brainstormed on it and
>>> came up with an alternative fix via device attribute in v4. Please
>>> have a look.
>>>
>>>> Oh, here did this come from! Yes, I recently had to adjust some systemd
>>>> service due to carrying this patch but looking for the change only in
>>>> upstream:
>>>>
>>>> https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043a5
>>>>
>>>
>>> You don't need to unbind all of the optee devices. v4 would help you
>>> to maintain backwards compatibility, can you retest it?
>>
>> How do I know from tee-supplicant perspective which devices I need to
>> unbind? There could be one in the future that will also use storage and
>> will therefore also fail once the supplicant is gone.
>>
> 
> With v4, the devices where the below attribute is present need to
> unbind before closing tee-supplicant.
> 
> /sys/bus/tee/devices/optee-ta-<uuid>/need_supplicant

OK - but that will only help in future kernels, nothing we have today.
Thus, the shutdown script cannot assume to alone kill those devices
unless it find a certain upcoming kernel release.

Jan

-- 
Siemens AG, Technology
Linux Expert Center