lkml.org 
[lkml]   [2021]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC v2 1/4] hisi-acc-vfio-pci: add new vfio_pci driver for HiSilicon ACC devices
    On Mon, Jul 05, 2021 at 10:18:59AM +0000, Shameerali Kolothum Thodi wrote:
    >
    >
    > > -----Original Message-----
    > > From: Max Gurtovoy [mailto:mgurtovoy@nvidia.com]
    > > Sent: 05 July 2021 10:42
    > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
    > > Leon Romanovsky <leon@kernel.org>
    > > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
    > > linux-crypto@vger.kernel.org; alex.williamson@redhat.com; jgg@nvidia.com;
    > > Linuxarm <linuxarm@huawei.com>; liulongfang <liulongfang@huawei.com>;
    > > Zengtao (B) <prime.zeng@hisilicon.com>; yuzenghui
    > > <yuzenghui@huawei.com>; Jonathan Cameron
    > > <jonathan.cameron@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>
    > > Subject: Re: [RFC v2 1/4] hisi-acc-vfio-pci: add new vfio_pci driver for HiSilicon
    > > ACC devices
    > >
    > >
    > > On 7/5/2021 11:47 AM, Shameerali Kolothum Thodi wrote:
    > > >
    > > >> -----Original Message-----
    > > >> From: Leon Romanovsky [mailto:leon@kernel.org]
    > > >> Sent: 04 July 2021 08:04
    > > >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
    > > >> Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
    > > >> linux-crypto@vger.kernel.org; alex.williamson@redhat.com;
    > > jgg@nvidia.com;
    > > >> mgurtovoy@nvidia.com; Linuxarm <linuxarm@huawei.com>; liulongfang
    > > >> <liulongfang@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
    > > >> yuzenghui <yuzenghui@huawei.com>; Jonathan Cameron
    > > >> <jonathan.cameron@huawei.com>; Wangzhou (B)
    > > <wangzhou1@hisilicon.com>
    > > >> 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@huawei.com>
    > > >>> ---
    > > >>> 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.
    >
    > Based on the documentation of the vfio_device_ops callbacks,
    > It looks like we already have a precedence in the case of reflck_attach
    > callback where it uses the vfio core default one, if it is not implemented.

    The reflck_attach pattern is pretty common pattern in the kernel to provide fallback.

    >
    > Also I would imagine that in majority use cases the vendor drivers will be
    > defaulting to core functions.

    Right, this is whole idea of having core functionality in one place, if
    vendor wants/needs, he will overwrite.

    >
    > I think, in any case, it would be good to update the Documentation based on
    > which way we end up doing this.

    The request to update Documentation can be seen as an example of
    choosing not-good API decisions. Expectation to see all drivers to
    use same callbacks with same vfio-core function calls sounds strange
    to me.

    Thanks

    >
    > Thanks,
    > Shameer
    >
    >
    > >
    > >
    > > >
    > > > Thanks,
    > > > Shameer

    \
     
     \ /
      Last update: 2021-07-05 20:27    [W:3.653 / U:0.272 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site