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

[Xu Yilun]

> >>>> +	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.