Re: [RFC PATCH v2 5/6] PCI/TSM: Authenticate devices via platform TSM

[Xu Yilun <yilun.xu@linux.intel.com>]

On Tue, May 07, 2024 at 11:21:37AM -0700, Dan Williams wrote:
> Xu Yilun wrote:
> > > > If (!ide_cap && tee_cap), we get here but doing the below does not make 
> > > > sense for TEE (which are likely to be VFs).
> > > 
> > > The "!ide_cap && tee_cap" case may also be the "TSM wants to setup IDE
> > > without TDISP flow".
> > 
> > IIUC, should be "TSM wants to setup TDISP without IDE flow"?
> 
> Both are possible. The TSM may need to be involved in IDE key
> establishment even if the PF or its VFs are ever assigned as TDIs. Also,
> as you say, it is possible for a TSM to trust that a device does not
> need IDE established because it is has knowledge that the device is
> integrated into the SOC without physical exposure of its links.
> 
> > But I think aik is talking about VFs (which fit "!ide_cap && tee_cap"),
> > VFs should not be rejected by the following:
> > 
> >       pci_tsm->doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
> >                                              PCI_DOE_PROTO_CMA);
> >       if (!pci_tsm->doe_mb)
> >               return;
> > 
> > VF should check its PF's doe/ide/tee cap and then be added to
> > pci_tsm_devs, is it?
> 
> This path should probably skip VFs because the 'connect' operation is a
> PF-only semantic. I will fix that up.

Agreed. I drafted some simple changes for the idea, that we keep
pci_dev::tsm for every TEE capable device (PF & VF) to execute tsm_ops,
but only adds PFs to pci_tsm_devs for "connect".


diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
index 9c5fb2c46662..31707f0351c6 100644
--- a/drivers/pci/tsm.c
+++ b/drivers/pci/tsm.c
@@ -241,9 +241,14 @@ void pci_tsm_init(struct pci_dev *pdev)
        if (!pci_tsm)
                return;

-       pci_tsm->ide_cap = ide_cap;
        mutex_init(&pci_tsm->exec_lock);

+       if (pdev->is_virtfn) {
+               pdev->tsm = no_free_ptr(pci_tsm);
+               return;
+       }
+
+       pci_tsm->ide_cap = ide_cap;
        pci_tsm->doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
                                               PCI_DOE_PROTO_CMA);
        if (!pci_tsm->doe_mb)
@@ -262,9 +267,14 @@ void pci_tsm_init(struct pci_dev *pdev)

 void pci_tsm_destroy(struct pci_dev *pdev)
 {
+       if (!pdev->tsm)
+               return;
+
        guard(rwsem_write)(&pci_tsm_rwsem);
-       pci_tsm_del(pdev);
-       xa_erase(&pci_tsm_devs, pci_tsm_devid(pdev));
+       if (!pdev->is_virtfn) {
+               pci_tsm_del(pdev);
+               xa_erase(&pci_tsm_devs, pci_tsm_devid(pdev));
+       }
        kfree(pdev->tsm);
        pdev->tsm = NULL;
 }

Thanks,
Yilun

> 
> I still think the PF needs to go through an ->add() callback because I
> do not think we have a cross-vendor unified concept of when IDE without
> TDISP, or TDISP without IDE is supported.