Re: [PATCH v6 4/4] x86/sev: register tpm-svsm platform device

[Stefano Garzarella]

On Wed, 9 Apr 2025 at 14:22, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, 2025-04-09 at 13:31 +0200, Borislav Petkov wrote:
> > On Wed, Apr 09, 2025 at 12:43:01PM +0200, Stefano Garzarella wrote:
> > > Sorry, maybe I missed something.
> > >
> > > tpm_svsm.c registers the driver with
> > > module_platform_driver_probe().
> > >
> > > Someone (the platform I guess) has to register the device by
> > > calling platform_device_register(), as we already do for example
> > > for sev_guest.
> >
> > Maybe that platform device thing is the wrong approach. Why does the
> > core code need to register some dummy platform device in the first
> > place? Why can't drivers/char/tpm/tpm_svsm.c probe and init without
> > it?
>
> Because of the way driver and device matching works in Linux.  We have
> to have a struct device because that sits at the he heart of the TPM
> driver binding.  If we have a struct device, it has to sit on a bus
> (because that's the Linux design) and if we don't have a bus then we
> have to use a platform device (or, now, we could use a struct device on
> the faux bus).

I tried to look at faux bus, but IIUC, it doesn't fit. I mean, we
could use it if we had a driver here in sev/core.c, but using a
separate module for the tpm-svsm driver, how do we get the module to
load when we find out that the device is there?

In short, my question: how do we load the module automatically when we
discover the device?

faux seems more useful to me when there's no need to discover the
device, but loading the module itself starts everything. If, on the
other hand, we want to have it load when we discover it, we have to
either have a bus or have core code that registers a platform_device
that will then be recognized by the driver in a separate module.

> Busses can be either physical (PCI, GSC, ...) and
> abstract (virtio, xen, scsi, ...), so it's not impossible, if the SVSM
> has more than one device, that it should have it's own SVSM bus which
> we could then act a bit like the virtio bus and the SVSM vTPM struct
> device could sit on this (the TPM subsystem, like most driver
> subsystems, doesn't care about busses, it only cares that the abstract
> bus device id matching works).

Yes, I'm also looking at introducing a svsm bus as we've already
discussed, but that's going to take some time.

In the end though, platform_device is not that bad IMO. The tpm-svsm
is really a device provided by the (virtual) platform, so doing some
sort of discovery of the bus in sev/core.c and registering the device
if discovered might be a compromise for now until we develop the bus.

If you agree, I'll move all the discovery here in sev/core.c as I
suggested earlier, so that when we get the bus we'll move this code
somehow into the bus.
@Borsilav @James WDYT?

Thanks,
Stefano