Re: [RFC PATCH 06/15] virt: tdx-guest: Support TDI bind and unbind operations

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 10/19/2026 10:16 AM, Duan, Zhenzhong wrote:
>>> +
>>>   static struct pci_tsm *tdx_devsec_lock(struct tsm_dev *tsm_dev, struct
>> pci_dev *pdev)
>>>   {
>>> +	u8 state;
>>>   	int ret;
>>>
>>>   	struct tdx_devsec *tdevsec __free(kfree) = kzalloc(sizeof(*tdevsec),
>> GFP_KERNEL);
>>> @@ -174,6 +233,19 @@ static struct pci_tsm *tdx_devsec_lock(struct
>> tsm_dev *tsm_dev, struct pci_dev *
>>>   	if (!tdx_is_dev_teeio_support(tdevsec))
>>>   		return ERR_PTR(-EOPNOTSUPP);
>>>
>>> +	struct tdx_devsec *tdevsec_bind __free(tdx_tdi_unbind_dev) =
>> tdx_tdi_bind_dev(tdevsec);
>>> +	if (IS_ERR(tdevsec_bind))
>>> +		return ERR_CAST(tdevsec_bind);
>>> +
>>> +	ret = tdx_tdi_read_state(tdevsec, &state);
>>> +	if (ret)
>>> +		return ERR_PTR(ret);
>>> +
>>> +	if (state != TDI_STATE_CONFIG_LOCKED)
>>> +		return ERR_PTR(-EIO);
>>> +
>>> +	retain_and_null_ptr(tdevsec_bind);
>>> +
>>>   	return &no_free_ptr(tdevsec)->pci.base_tsm;
>>>   }
>>>
>>> @@ -181,6 +253,9 @@ static void tdx_devsec_unlock(struct pci_tsm *tsm)
>>>   {
>>>   	struct tdx_devsec *tdevsec = to_tdx_devsec(tsm);
>>>
>>> +	if (WARN_ON(tdx_tdi_unbind_dev(tdevsec)))
>>> +		return;
>> <Thinking out loud>
>>
>> So unlock involves calling TDCM_UNBIND which in turn might fail for any
>> number of reasons and yet we have no way of communicating that. So after
>> an unlock a TD might want to rebind and issue bind on a device which is
>> already bound because prior unbind failed. Is this something we should
>> be worried about ?
> I completely agree with your point here.
> We need pci_tsm_ops::unlock() in TSM framework to return an error status
> (e.g., int) so that failures in the underlying hardware-level unbind can
> be bubbled up to the caller rather than silently ignored.

please remember raising this when folks are discussing generic guest TSM
framework series.

Btw, for TDX specific tdx_devsec_unlock(), the WARN_ON() is not proper IMHO.
the failure can happen for various reasons, and the WARN can be treated as fatal
when panic_on_warn is true. I think pr_warn_ratelimit() would suffice?

And the direct return after failure happens seems also problematic. E.g., it
leaks tdevsec. After later patches, it seems to leak more.