Re: [PATCH v2 2/3] vfio/ism: Implement vfio_pci driver for ISM devices

[Alexandra Winter <wintera@linux.ibm.com>]

On 02.03.26 13:18, Julian Ruess wrote:
>>> +struct ism_vfio_pci_core_device {
>>> +	struct vfio_pci_core_device core_device;
>>> +};
>>> +
>>> +static int ism_pci_open_device(struct vfio_device *core_vdev)
>>> +{
>>> +	struct ism_vfio_pci_core_device *ivdev;
>>> +	struct vfio_pci_core_device *vdev;
>>
>> Why do you need 'struct ism_vfio_pci_core_device'?
>> Unlike other vfio_pci variant drivers your struct only has a single member.
>> I see it is used below as well, but couldn't you directly use 'struct vfio_pci_core_device'
>> in all places?
> Theoretically yes, but functions like vfio_alloc_device() expect this parent
> struct.


Couldn't you do something like:

static int ism_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
	struct vfio_pci_core_device *vdev;
	int ret;

	vdev = vfio_alloc_device(vfio_pci_core_device, vdev, &pdev->dev,
				 &ism_pci_ops);
	...


But I'm just thinking out loud. No strong feelings.
If you think it's worth to define an additional struct, so your driver looks more like the others - that's fine with me.



>> Where do you free 'data' ?
> void *data __free(kfree) = NULL;

Thank you; I learned something.