Re: [PATCH 4/6] PCI/TSM: Add pci_tsm_bind() helper for instantiating TDIs

dan.j.williams
2025-11-05T21:50:02Z

Aneesh Kumar K.V wrote:
[..]
> Can we set tdi in the constructor? I use it later as part of a bind()
> callback. I’d prefer not to pass tdi as an argument to those functions,
> since functions like do_dev_communicate() are reused in other contexts
> where tdi isn't needed.

I think if you need this it would be ok to set tsm->tdi in your driver.
It is not the greatest fit for pci_tsm_tdi_constructor() because that
function really does not know if the tdi is fully formed and ready to
publish via tsm->tdi. It only knows that the common fields have been
initialized. See below...

[..]
> modified drivers/pci/tsm.c
> @@ -391,8 +391,6 @@ int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u32 tdi_id)
> if (IS_ERR(tdi))
> return PTR_ERR(tdi);
>
> - pdev->tsm->tdi = tdi;
> -

I think it is ok to keep this even if the TSM driver already did it.

> return 0;
> }
> EXPORT_SYMBOL_GPL(pci_tsm_bind);
> @@ -998,6 +996,7 @@ static struct pci_dev *find_dsm_dev(struct pci_dev *pdev)
> void pci_tsm_tdi_constructor(struct pci_dev *pdev, struct pci_tdi *tdi,
> struct kvm *kvm, u32 tdi_id)
> {
> + pdev->tsm->tdi = tdi;
> tdi->pdev = pdev;
> tdi->kvm = kvm;
> tdi->tdi_id = tdi_id;

This ordering potentially causes something that walks tdi->pdev to fail,
and who knows if the TSM driver calls pci_tsm_tdi_constructor() before
or after TSM driver specific init. It all around feels like the TSM
driver is responsible for making the "early publish" decision, not this
constructor.