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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
>Subject: Re: [RFC PATCH 06/15] virt: tdx-guest: Support TDI bind and unbind
>operations
>
>> >>>> +	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);
>> >>>
>> >>> While this does seem correct, it's not really following the spec or
>I'm being too
>> >>> rigid, namely:
>> >>>
>> >>> Since TDI can change the state any time, TD can't fully rely on the
>Known RUN
>> >>> state in any scenario. However, TD can rely on the Known
>> >>> RUN state after TDG.TDI.START as a confirmation of the Interface
>Start.
>> >>
>> >> About TDI changing state any time, do you mean host changes TDI state,
>> >> e.g., through TDH.TDI.STOP? Or you mean TDI device goes into TDISP
>error state
>> >> due to hw error? Can't they happen even after TDG.TDI.START?
>> >
>> >I've taken this verbatim from the latest TDX connect spec, I'd assume
>> >what it refers to is the TDI/device getting into error state because the
>> >DSM deemed so because of whatever reason. I believe there is an inherent
>> >TOCTOU race here, because you can at best get a momentary snapshot of
>> >the state but the DSM, which is inherently an independent entry could
>> >for whatever reason decide to enter in error state (highly unlikely, but
>> >still possible!)
>>
>> Agreed, it's a best effort check, error state can happen at any time.
>
>I think we don't need this best effort check. The guest would eventually
>verify the states before RUN with the help of TDX module. So to me,
>TDI state read is just a debug interface. For simplicity, drop it.

OK, will do.