Re: [RFC PATCH 20/27] coco/tdx-host: Add connect()/disconnect() handlers prototype

[<dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> On Fri, 19 Sep 2025 07:22:29 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > 
> > Add basic skeleton for connect()/disconnect() handlers. The major steps
> > are SPDM setup first and then IDE selective stream setup.
> > 
> > No detailed TDX Connect implementation.
> > 
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Feels like use of __free() in here is inappropriate to me.
> 
> > ---
> >  drivers/virt/coco/tdx-host/tdx-host.c | 49 ++++++++++++++++++++++++++-
> >  1 file changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> > index f5a869443b15..0d052a1acf62 100644
> > --- a/drivers/virt/coco/tdx-host/tdx-host.c
> > +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> > @@ -104,13 +104,60 @@ static int __maybe_unused tdx_spdm_msg_exchange(struct tdx_link *tlink,
> >  	return ret;
> 
> > +
> > +static void __tdx_link_disconnect(struct tdx_link *tlink)
> > +{
> > +	tdx_ide_stream_teardown(tlink);
> > +	tdx_spdm_session_teardown(tlink);
> > +}
> > +
> > +DEFINE_FREE(__tdx_link_disconnect, struct tdx_link *, if (_T) __tdx_link_disconnect(_T))
> > +
> >  static int tdx_link_connect(struct pci_dev *pdev)
> >  {
> > -	return -ENXIO;
> > +	struct tdx_link *tlink = to_tdx_link(pdev->tsm);
> > +	int ret;
> > +
> > +	struct tdx_link *__tlink __free(__tdx_link_disconnect) = tlink;
> I'm not a fan on an ownership pass like this just for purposes of cleaning up.

Yeah this needs a rethink. The session and the stream are independent
resources. It can be a composite object that encapsulates both
resources, but not tlink directly.

...chalk this up to RFC expediency.

> I'd be a bit happier if you could make it
> 	struct tdx_link *tlink __free(__tdx_link_disconnect) = to_tdx_link(pdev->dsm);
> 
> but I still don't really like it.  I think I'd just not use __free and stick
> to traditional cleanup in via a goto. 

I would not go that far, but certainly I can see that being preferable
than reusing the existing base 'struct tdx_link *' as the cleanup
variable.