Re: [RFC v2 1/4] hisi-acc-vfio-pci: add new vfio_pci driver for HiSilicon ACC devices

From: Max Gurtovoy
Date: Mon Jul 05 2021 - 05:41:43 EST



On 7/5/2021 11:47 AM, Shameerali Kolothum Thodi wrote:

-----Original Message-----
From: Leon Romanovsky [mailto:leon@xxxxxxxxxx]
Sent: 04 July 2021 08:04
To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>
Cc: kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
linux-crypto@xxxxxxxxxxxxxxx; alex.williamson@xxxxxxxxxx; jgg@xxxxxxxxxx;
mgurtovoy@xxxxxxxxxx; Linuxarm <linuxarm@xxxxxxxxxx>; liulongfang
<liulongfang@xxxxxxxxxx>; Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>;
yuzenghui <yuzenghui@xxxxxxxxxx>; Jonathan Cameron
<jonathan.cameron@xxxxxxxxxx>; Wangzhou (B) <wangzhou1@xxxxxxxxxxxxx>
Subject: Re: [RFC v2 1/4] hisi-acc-vfio-pci: add new vfio_pci driver for HiSilicon
ACC devices

On Fri, Jul 02, 2021 at 10:58:46AM +0100, Shameer Kolothum wrote:
Add a vendor-specific vfio_pci driver for HiSilicon ACC devices.
This will be extended in follow-up patches to add support for
vfio live migration feature.

Signed-off-by: Shameer Kolothum
<shameerali.kolothum.thodi@xxxxxxxxxx>
---
drivers/vfio/pci/Kconfig | 9 +++
drivers/vfio/pci/Makefile | 2 +
drivers/vfio/pci/hisi_acc_vfio_pci.c | 100 +++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
create mode 100644 drivers/vfio/pci/hisi_acc_vfio_pci.c
<...>

+static const struct vfio_device_ops hisi_acc_vfio_pci_ops = {
+ .name = "hisi-acc-vfio-pci",
+ .open = hisi_acc_vfio_pci_open,
+ .release = vfio_pci_core_release,
+ .ioctl = vfio_pci_core_ioctl,
+ .read = vfio_pci_core_read,
+ .write = vfio_pci_core_write,
+ .mmap = vfio_pci_core_mmap,
+ .request = vfio_pci_core_request,
+ .match = vfio_pci_core_match,
+ .reflck_attach = vfio_pci_core_reflck_attach,
I don't remember what was proposed in vfio-pci-core conversion patches,
but would expect that default behaviour is to fallback to vfio_pci_core_* API
if ".release/.ioctl/e.t.c" are not redefined.
Yes, that would be nice, but don't think it does that in latest(v4).

Hi Max,
Could we please consider fall back to the core defaults, may be check and assign defaults
in vfio_pci_core_register_device() ?

I don't see why we should do this.

vfio_pci_core.ko is just a library driver. It shouldn't decide for the vendor driver ops.

If a vendor driver would like to use its helper functions - great.

If it wants to override it - great.

If it wants to leave some op as NULL - it can do it also.



Thanks,
Shameer