Re: [RFC PATCH 07/20] x86/virt/tdx: Expose SEAMLDR information via sysfs

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

Xu Yilun wrote:
[..]
> > > diff --git a/drivers/virt/coco/tdx-tsm/Makefile b/drivers/virt/coco/tdx-tsm/Makefile
> > > new file mode 100644
> > > index 000000000000..09f0ac08988a
> > > --- /dev/null
> > > +++ b/drivers/virt/coco/tdx-tsm/Makefile
> > > @@ -0,0 +1 @@
> > > +obj-$(CONFIG_TDX_TSM_BUS) += tdx-tsm-bus.o
> > 
> > Just name it bus.c.
> 
> I'm about to make the change but I see there is already tdx-guest misc
> virtual device in Guest OS:
> 
>   What:		/sys/devices/virtual/misc/tdx_guest/xxxx
> 
> And if we add another tdx_subsys, we have:
> 
>   What:		/sys/devices/virtual/tdx/xxxx
> 
> Do we really want 2 virtual devices? What's their relationship? I can't
> figure out.
> 
> So I'm considering reuse the misc/tdx_guest device as a tdx root device
> in guest. And that removes the need to have a common tdx tsm bus.
> 
> What do you think?

True, do not need tdx_subsys on the guest side. The tdx_guest driver
is sufficient. This was the approach taken with the RTMR enabling, just
append the sysfs attributes to the existing guest device.

> > > And put the tdx_subsys_init() in tdx-tsm-bus.c. We need to move host
> > > specific initializations out of tdx_subsys_init(), e.g. seamldr_group &
> > > seamldr fw upload.
> > 
> > Just to be clear on the plan here as I think this TD Preserving set
> > should land before we start upstreamming any TDX Connect bits.
> > 
> > - Create drivers/virt/coco/tdx-tsm/bus.c for registering the tdx_subsys.
> >   The tdx_subsys has sysfs attributes like "version" (host and guest
> >   need this, but have different calls to get at the information) and
> >   "firmware" (only host needs that). So the common code will take sysfs
> >   groups passed as a parameter.
> > 
> > - The "tdx_tsm" device which is unused in this patch set can be
> 
> It is used in this patch, Chao creates tdx module 'version' attr on this
> device. But I assume you have different opinion: tdx_subsys represents
> the whole tdx_module and should have the 'version', and tdx_tsm is a
> sub device dedicate for TDX Connect, is it?

The main reason for a tdx_tsm device in addition to the subsys is to
allow for deferred attachment.

Now, that said, the faux_device infrastructure has arrived since this
all started and *could* replace tdx_subsys. The only concern is whether
the tdx_tsm driver ever needs to do probe deferral to wait for IOMMU or
PCI initialization to happen first.

If probe deferral is needed that requires a bus, if probe can always be
synchronous with TDX module init then faux_device could work.