Re: [PATCH v8 4/9] PCI/TSM: Establish Secure Sessions and Link Encryption

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

Xu Yilun wrote:
> > +#ifdef CONFIG_PCI_TSM
> > +int pci_tsm_register(struct tsm_dev *tsm_dev);
> > +void pci_tsm_unregister(struct tsm_dev *tsm_dev);
> > +int pci_tsm_link_constructor(struct pci_dev *pdev, struct pci_tsm *tsm,
> > +			     struct tsm_dev *tsm_dev);
> > +int pci_tsm_pf0_constructor(struct pci_dev *pdev, struct pci_tsm_pf0 *tsm,
> > +			    struct tsm_dev *tsm_dev);
> > +void pci_tsm_pf0_destructor(struct pci_tsm_pf0 *tsm);
> > +int pci_tsm_doe_transfer(struct pci_dev *pdev, u8 type, const void *req,
> > +			 size_t req_sz, void *resp, size_t resp_sz);
> > +#else
> > +static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> > +{
> > +	return 0;
> > +}
> > +static inline void pci_tsm_unregister(struct tsm_dev *tsm_dev)
> > +{
> > +}
> > +static inline int pci_tsm_doe_transfer(struct pci_dev *pdev, u8 type,
> > +				       const void *req, size_t req_sz,
> > +				       void *resp, size_t resp_sz)
> 
> Any concern to keep the stub without PCI_TSM?
> pci_tsm_pf0_constructor/destructor() don't do this.

True. There should be no callers of this when CONFIG_PCI_TSM=n. Will
append a cleanup for this.

> 
> > +{
> > +	return -ENXIO;
> > +}
> > +#endif
> 
> [...]
> 
> > diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> > new file mode 100644
> > index 000000000000..6a2849f77adc
> > --- /dev/null
> > +++ b/drivers/pci/tsm.c
> > @@ -0,0 +1,643 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Interface with platform TEE Security Manager (TSM) objects as defined by
> > + * PCIe r7.0 section 11 TEE Device Interface Security Protocol (TDISP)
> > + *
> > + * Copyright(c) 2024-2025 Intel Corporation. All rights reserved.
> > + */
> > +
> > +#define dev_fmt(fmt) "PCI/TSM: " fmt
> > +
> > +#include <linux/bitfield.h>
> 
> No need the bitfield.h
> 
> > +#include <linux/pci.h>
> > +#include <linux/pci-doe.h>
> > +#include <linux/pci-tsm.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/tsm.h>
> > +#include <linux/xarray.h>
> 
> No need the xarray.h
> 
> Anyway, they are all minor and cause no impact, I don't expect a new
> version.

Ah yes, holdovers from early versions, will append your reviewed-by in
the pull request.

> 
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>