[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aJIo4riWyW7fRtal@yilunxu-OptiPlex-7050>
Date: Tue, 5 Aug 2025 23:53:06 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: linux-coco@...ts.linux.dev, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, bhelgaas@...gle.com, aik@....com,
lukas@...ner.de, Samuel Ortiz <sameo@...osinc.com>
Subject: Re: [PATCH v4 04/10] PCI/TSM: Authenticate devices via platform TSM
> +static int pci_tsm_connect(struct pci_dev *pdev, struct tsm_dev *tsm_dev)
> +{
> + int rc;
> + struct pci_tsm_pf0 *tsm_pf0;
> + const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
> + struct pci_tsm *pci_tsm __free(tsm_remove) = ops->probe(pdev);
> +
> + if (!pci_tsm)
> + return -ENXIO;
> +
> + pdev->tsm = pci_tsm;
> + tsm_pf0 = to_pci_tsm_pf0(pdev->tsm);
> +
> + ACQUIRE(mutex_intr, lock)(&tsm_pf0->lock);
> + if ((rc = ACQUIRE_ERR(mutex_intr, &lock)))
> + return rc;
> +
> + rc = ops->connect(pdev);
> + if (rc)
> + return rc;
> +
> + pdev->tsm = no_free_ptr(pci_tsm);
> +
> + /*
> + * Now that the DSM is established, probe() all the potential
> + * dependent functions. Failure to probe a function is not fatal
> + * to connect(), it just disables subsequent security operations
> + * for that function.
> + */
> + pci_tsm_probe_fns(pdev);
> + return 0;
> +}
> +}
> +
[...]
> +static void pf0_sysfs_enable(struct pci_dev *pdev)
> +{
> + pci_dbg(pdev, "Device Security Manager detected (%s%s )\n",
> + pdev->ide_cap ? " ide" : "",
> + pdev->devcap & PCI_EXP_DEVCAP_TEE ? " tee" : "");
> +
> + sysfs_update_group(&pdev->dev.kobj, &pci_tsm_auth_attr_group);
> + sysfs_update_group(&pdev->dev.kobj, &pci_tsm_pf0_attr_group);
> +}
> +
> +int pci_tsm_register(struct tsm_dev *tsm_dev)
> +{
[...]
> + for_each_pci_dev(pdev)
> + if (is_pci_tsm_pf0(pdev))
> + pf0_sysfs_enable(pdev);
Now the tsm attributes are exposed to user before ops->probe(), from
user's POV, tsm link operation for this device is already ready ...
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_register);
[...]
> +struct pci_tsm_ops {
> + /*
> + * struct pci_tsm_link_ops - Manage physical link and the TSM/DSM session
> + * @probe: probe device for tsm link operation readiness, setup
So I think the probe callback is losing the meaning of readiness check.
Users see the 'connect/disconnect', they write 'connect' and found
errors no matter ->probe() fails or ->connect() fails.
Maybe just remove the responsibility of readiness check from ->probe(),
I found it simplifies code when implementing tdx-tsm driver.
Thanks,
Yilun
> + * DSM context
> + * @remove: destroy DSM context
> + * @connect: establish / validate a secure connection (e.g. IDE)
> + * with the device
> + * @disconnect: teardown the secure link
> + *
> + * @probe and @remove run in pci_tsm_rwsem held for write context. All
> + * other ops run under the @pdev->tsm->lock mutex and pci_tsm_rwsem held
> + * for read.
> + */
> + struct_group_tagged(pci_tsm_link_ops, link_ops,
> + struct pci_tsm *(*probe)(struct pci_dev *pdev);
> + void (*remove)(struct pci_tsm *tsm);
> + int (*connect)(struct pci_dev *pdev);
> + void (*disconnect)(struct pci_dev *pdev);
> + );
Powered by blists - more mailing lists