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

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

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

> Aneesh Kumar K.V wrote:
>> 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.)
>
> Do you need to walk pdev->tsm when you are creating the tsm context?
>
> For example, pass @pdev and the lock context structure to
> rsi_device_lock()?

Sure I can pass struct cca_guest_dsc *dsm as an agrument to
rsi_device_lock().

I was comparing this to connect() callback which when getting called
will already have pdev->tsm set.

static int pci_tsm_connect(struct pci_dev *pdev, struct tsm_dev *tsm_dev)
{
	int rc;
	struct pci_tsm_pf0 *tsm_pf0;
	const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
	struct pci_tsm *pci_tsm __free(tsm_remove) = ops->probe(pdev);

.....

	pdev->tsm = pci_tsm;
	tsm_pf0 = to_pci_tsm_pf0(pdev->tsm);

...
	rc = ops->connect(pdev);
	if (rc)
		return rc;

	pdev->tsm = no_free_ptr(pci_tsm);

}


-aneesh