Re: [PATCH 5/7] PCI/TSM: Add Device Security (TVM Guest) operations support

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Dan Williams <dan.j.williams@intel.com> writes:


....

> +
> +static int pci_tsm_lock(struct pci_dev *pdev, struct tsm_dev *tsm_dev)
> +{
> +	const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
> +	struct pci_tsm *tsm;
> +	int rc;
> +
> +	ACQUIRE(device_intr, lock)(&pdev->dev);
> +	if ((rc = ACQUIRE_ERR(device_intr, &lock)))
> +		return rc;
> +
> +	if (pdev->dev.driver)
> +		return -EBUSY;
> +
> +	tsm = ops->lock(pdev);
> +	if (IS_ERR(tsm))
> +		return PTR_ERR(tsm);
> +
> +	pdev->tsm = tsm;
> +	return 0;
> +}
>

This is slightly different from connect() callback in that we don't have
pdev->tsm initialized when calling ->lock() callback. Should we do
something like below? (I also included the arch changes to show how
destructor is being used.)

modified   drivers/pci/tsm.c
@@ -917,11 +917,19 @@ int pci_tsm_devsec_constructor(struct pci_dev *pdev, struct pci_tsm_devsec *tsm,
 	pci_tsm->tdi = NULL;
 	pci_tsm->pdev = pdev;
 	pci_tsm->ops = ops;
+	pdev->tsm = pci_tsm;
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_tsm_devsec_constructor);
 
+int pci_tsm_devsec_destructor(struct pci_dev *pdev)
+{
+	pdev->tsm = NULL;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_tsm_devsec_destructor);
+
 /**
  * pci_tsm_pf0_constructor() - common 'struct pci_tsm_pf0' (DSM) initialization
  * @pdev: Physical Function 0 PCI device (as indicated by is_pci_tsm_pf0())
modified   drivers/virt/coco/arm-cca-guest/arm-cca.c
@@ -217,13 +217,17 @@ static struct pci_tsm *cca_tsm_lock(struct pci_dev *pdev)
 		return ERR_PTR(ret);
 
 	ret = rhi_da_vdev_set_tdi_state(vdev_id, RHI_DA_TDI_CONFIG_LOCKED);
-	if (ret)
+	if (ret) {
+		pci_tsm_devsec_destructor(pdev);
 		return ERR_PTR(-EIO);
+	}
 
 	/* This will be done by above rhi in later spec */
 	ret = rsi_device_lock(pdev);
-	if (ret)
+	if (ret) {
+		pci_tsm_devsec_destructor(pdev);
 		return ERR_PTR(-EIO);
+	}
 
 	return &no_free_ptr(cca_dsc)->pci.base;
 }
@@ -245,6 +249,7 @@ static void cca_tsm_unlock(struct pci_dev *pdev)
 		return;
 	}
 
+	pci_tsm_devsec_destructor(pdev);
 	kfree(cca_dsc);
 }
 
modified   include/linux/pci-tsm.h
@@ -220,6 +220,7 @@ int pci_tsm_pf0_constructor(struct pci_dev *pdev, struct pci_tsm_pf0 *tsm,
 			    const struct pci_tsm_ops *ops);
 int pci_tsm_devsec_constructor(struct pci_dev *pdev, struct pci_tsm_devsec *tsm,
 			       const struct pci_tsm_ops *ops);
+int pci_tsm_devsec_destructor(struct pci_dev *pdev);
 void pci_tsm_pf0_destructor(struct pci_tsm_pf0 *tsm);
 int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u32 tdi_id);
 void pci_tsm_unbind(struct pci_dev *pdev);



-aneesh