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

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

Xu Yilun wrote:
> > +static const struct attribute_group *tdx_subsys_groups[] = {
> > +	SEAMLDR_GROUP,
> > +	NULL,
> > +};
> > +
> >  static void tdx_subsys_init(void)
> >  {
> >  	struct tdx_tsm *tdx_tsm;
> >  	int err;
> >  
> > +	err = get_seamldr_info();
> > +	if (err) {
> > +		pr_err("failed to get seamldr info %d\n", err);
> > +		return;
> > +	}
> > +
> >  	/* Establish subsystem for global TDX module attributes */
> > -	err = subsys_virtual_register(&tdx_subsys, NULL);
> > +	err = subsys_virtual_register(&tdx_subsys, tdx_subsys_groups);
> >  	if (err) {
> >  		pr_err("failed to register tdx_subsys %d\n", err);
> >  		return;
> 
> As mentioned, TDX Connect also uses this virtual TSM device. And I tend
> to extend it to TDX guest, also make the guest TSM management run on
> the virtual device which represents the TDG calls and TDG_VP_VM calls.
> 
> So I'm considering extract the common part of tdx_subsys_init() out of
> TDX host and into a separate file, e.g.
> 
> ---
> 
> +source "drivers/virt/coco/tdx-tsm/Kconfig"
> +
>  config TSM
>         bool
> diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> index c0c3733be165..a54d3cb5b4e9 100644
> --- a/drivers/virt/coco/Makefile
> +++ b/drivers/virt/coco/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
>  obj-$(CONFIG_ARM_CCA_GUEST)    += arm-cca-guest/
>  obj-$(CONFIG_TSM)              += tsm-core.o
>  obj-$(CONFIG_TSM_GUEST)                += guest/
> +obj-y                          += tdx-tsm/
> diff --git a/drivers/virt/coco/tdx-tsm/Kconfig b/drivers/virt/coco/tdx-tsm/Kconfig
> new file mode 100644
> index 000000000000..768175f8bb2c
> --- /dev/null
> +++ b/drivers/virt/coco/tdx-tsm/Kconfig
> @@ -0,0 +1,2 @@
> +config TDX_TSM_BUS
> +       bool
> 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.

> ---
> 
> 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
  registered on the "tdx" bus to move feature support like TDX Connect
  into a typical driver model.

So the change for this set is create a bus.c that is host/guest
agnostic, drop the tdx_tsm device and leave that to the TDX Connect
patches to add back. 

The TDX Connect pathes will register the tdx_tsm device near where the
bus is registered for the host and guest cases.

Concerns?

In the meantime, until this set lands in tip we can work out the
organization in tsm.git#staging.