Re: [PATCH v3 12/21] PCI/CMA: Add a PCI TSM CMA driver using SPDM

[Jonathan Cameron <jic23@kernel.org>]

On Tue,  1 Sep 2026 11:03:38 +1000
alistair23@gmail.com wrote:

> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Component Measurement and Authentication (CMA, PCIe r6.2 sec 6.31)
> allows for measurement and authentication of PCIe devices.  It is
> based on the Security Protocol and Data Model specification (SPDM,
> https://www.dmtf.org/dsp/DSP0274).
> 
> CMA-SPDM in turn forms the basis for Integrity and Data Encryption
> (IDE, PCIe r6.2 sec 6.33) because the key material used by IDE is
> transmitted over a CMA-SPDM session.
> 
> As a first step, add support for authentication via a CMA TSM driver.
> 
> This was previously discusd here:
> http://lore.kernel.org/69976d7d39c60_2f4a1009@dwillia2-mobl4.notmuch
> 
> By utilising a TSM driver we get a lot of the TSM driver probe policies
> "for free". Currently there is no mechanism to provide evidence to
> userspace, as the TSM system doesn't support that at the moment. That
> can be added later when support by TSM.
> 
> Credits: Jonathan wrote the original proof-of-concept for a CMA implementation.
> Lukas reworked that for upstream. Wilfred contributed fixes for issues
> discovered during testing. Alistair reworked it as a TSM driver.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Co-developed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Co-developed-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

A few things inline.  I have no idea if I'm poking holes in 
my own code or if none of that is left :)

>  
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> new file mode 100644
> index 000000000000..9f2cc0b2ec8a
> --- /dev/null
> +++ b/drivers/pci/cma.c

...



> +static int pci_cma_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_cma_tsm *cma = cma_tsm_from_tsm(pdev->tsm);
> +	int rc;
> +
> +	/*
> +	 * The DOE mailbox lives in the device's config space, so the
> +	 * device must be runtime-resumed for the duration of the SPDM
> +	 * exchange.

Oddly short wrap. Comments go to 80
	/*
	 * The DOE mailbox lives in the device's config space, so the device
	 * must be runtime-resumed for the duration of the SPDM exchange.

> +	 */
> +	rc = pm_runtime_get_sync(&pdev->dev);
	rc = pm_runtime_resume_and_get(&pdev->dev)
	if (rc < 0)
		return rc;

See comments in docs for pm_runtime_get_sync() for info on this.
Basically it doesn't mess up reference counting so you don't need
to fix it in the error path.

Mind you, I haven't looked ahead but we have ACQUIRE macros for
runtime PM that might apply nicely here and remove need to release anything
at all.

	PM_RUNTIME_ACQUIRE(&pdev->dev, pm);
	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
		return -ENXIO;

	

> +	if (rc < 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		return rc;
> +	}
> +
> +	rc = spdm_authenticate(cma->spdm);

If you can use ACQUIRE stuff
	return spdm_authenticate(cma->spdm);

> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	return rc;
> +}

> +
> +static const struct pci_tsm_ops pci_cma_tsm_ops = {
> +	.link_ops = {
> +		.probe		= pci_cma_tsm_probe,
> +		.remove		= pci_cma_tsm_remove,
> +		.connect	= pci_cma_tsm_connect,
> +		.disconnect	= pci_cma_tsm_disconnect,
> +		.bind		= pci_cma_tsm_bind,
> +		.unbind		= pci_cma_tsm_unbind,
> +		.guest_req	= pci_cma_tsm_guest_req,
> +	},
> +};
> +
> +static struct tsm_dev *pci_cma_tsm_dev;
> +
> +static int __init pci_cma_tsm_init(void)
> +{
> +	struct tsm_dev *tsm_dev;
> +
> +	tsm_dev = tsm_register(NULL, (struct pci_tsm_ops *)&pci_cma_tsm_ops);

So this is casting away the const.   Why does tsm_register not take a const?
Can we change that as seems unlikely it will actually modify it?
If there is any chance of a modification in future then we need to drop
the const marking on the structure above.


> +	if (IS_ERR(tsm_dev))
> +		return PTR_ERR(tsm_dev);
> +
> +	pci_cma_tsm_dev = tsm_dev;
> +	return 0;
> +}
> +late_initcall(pci_cma_tsm_init);