On Mon, Jun 02, 2025 at 06:36:37PM +0530, Aneesh Kumar K.V wrote:
> Xu Yilun <yilun...@linux.intel.com> writes:
>
> > Add optional PCI driver callbacks to notify TSM events. For now, these
> > handlers may be called during pci_tsm_unbind(). By calling these
> > handlers, TSM driver askes for external collaboration to finish entire
> > TSM unbind flow.
> >
> > If platform TSM driver could finish TSM bind/unbind all by itself, don't
> > call these handlers.
> >
> > Host may need to configure various system components according to
> > platform trusted firmware's requirements. E.g. for Intel TDX Connect,
> > host should do private MMIO mapping in S-EPT, trusted DMA setup, device
> > ownership claiming and device TDISP state transition. Some operations are
> > out of control of PCI TSM, so need collaboration by external components
> > like IOMMU driver, KVM.
> >
> > Further more, trusted firmware may enforce executing these operations
> > in a fixed sequence. E.g. Intel TDX Connect enforces the following
> > sequences for TSM unbind:
> >
> > 1. STOP TDI via TDISP message STOP_INTERFACE
> > 2. Private MMIO unmap from Secure EPT
> > 3. Trusted Device Context Table cleanup for the TDI
> > 4. TDI ownership reclaim and metadata free
> >
> > PCI TSM could do Step 1 and 4, but need KVM for Step 2 and IOMMU driver
> > for Step 3. While it is possible TSM provides finer grained APIs like
> > tdi_stop() & tdi_free(), and the caller ensures the sequence, it is
> > better these specific enforcement could be managed in platform TSM
> > driver. By introducing TSM handlers, platform TSM driver controls the
> > operation sequence and notify other components to do the real work.
> >
> > Currently add 3 callbacks for TDX Connect. disable_mmio() is for
> > VFIO to invalidate MMIO so that KVM could unmap them from S-EPT.
> > recover_mmio() is to re-validate MMIO so that KVM could map them
> > again for shared assigned device. disable_trusted_dma() is to cleanup
> > trusted IOMMU setup.
> >
> > Signed-off-by: Xu Yilun <yilun...@linux.intel.com>
> > ---
> > include/linux/pci-tsm.h | 7 +++++++
> > include/linux/pci.h | 3 +++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> > index 737767f8a9c5..ed549724eb5b 100644
> > --- a/include/linux/pci-tsm.h
> > +++ b/include/linux/pci-tsm.h
> > @@ -157,6 +157,13 @@ struct pci_tsm_ops {
> > int (*accept)(struct pci_dev *pdev);
> > };
> >
> > +/* pci drivers callbacks for TSM */
> > +struct pci_tsm_handlers {
> > + void (*disable_mmio)(struct pci_dev *dev);
> > + void (*recover_mmio)(struct pci_dev *dev);
> > + void (*disable_trusted_dma)(struct pci_dev *dev);
> > +};
> > +
> > enum pci_doe_proto {
> > PCI_DOE_PROTO_CMA = 1,
> > PCI_DOE_PROTO_SSESSION = 2,
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 5f37957da18f..4f768b4658e8 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -545,6 +545,7 @@ struct pci_dev {
> > #endif
> > #ifdef CONFIG_PCI_TSM
> > struct pci_tsm *tsm; /* TSM operation state */
> > + void *trusted_dma_owner;
> > #endif
> > u16 acs_cap; /* ACS Capability offset */
> > u8 supported_speeds; /* Supported Link Speeds Vector */
> > @@ -957,6 +958,7 @@ struct module;
> > * @sriov_get_vf_total_msix: PF driver callback to get the total number of
> > * MSI-X vectors available for distribution to the VFs.
> > * @err_handler: See Documentation/PCI/pci-error-recovery.rst
> > + * @tsm_handler: Optional driver callbacks to handle TSM requirements.
> > * @groups: Sysfs attribute groups.
> > * @dev_groups: Attributes attached to the device that will be
> > * created once it is bound to the driver.
> > @@ -982,6 +984,7 @@ struct pci_driver {
> > int (*sriov_set_msix_vec_count)(struct pci_dev *vf, int
> > msix_vec_count); /* On PF */
> > u32 (*sriov_get_vf_total_msix)(struct pci_dev *pf);
> > const struct pci_error_handlers *err_handler;
> > + struct pci_tsm_handlers *tsm_handler;
> > const struct attribute_group **groups;
> > const struct attribute_group **dev_groups;
> > struct device_driver driver;
> > --
> > 2.25.1
>
> It looks like the TSM feature is currently interacting with several
> components: struct pci_driver, VFIO, iommufd, and pci_tsm_ops.
>
> Should we consider limiting this scattering? Would it make sense to
> encapsulate this logic within pci_tsm_ops?
I'm keeping on trying which is a better solution. Encapsulating all in
pci_tsm_ops is the most attactive one from SW POV, but only if the TSM
operations has no impact/dependency to other components. Unfortunately
it is not true, e.g. the private MMIO mapping/unmapping is actually
a writting to leaf S-EPT entry, but it requires non-leaf page-table-page
management in KVM.
Thanks,
Yilun
>
> -aneesh
