linux-kernel - Re: [PATCH v4 00/17] Add VFIO mediated device support and DEV-MSI support for the idxd driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201030204307.GA683@otc-nc-03>
Date:   Fri, 30 Oct 2020 13:43:07 -0700
From:   "Raj, Ashok" <ashok.raj@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     Dave Jiang <dave.jiang@...el.com>, vkoul@...nel.org,
        megha.dey@...el.com, maz@...nel.org, bhelgaas@...gle.com,
        tglx@...utronix.de, alex.williamson@...hat.com,
        jacob.jun.pan@...el.com, yi.l.liu@...el.com, baolu.lu@...el.com,
        kevin.tian@...el.com, sanjay.k.kumar@...el.com,
        tony.luck@...el.com, jing.lin@...el.com, dan.j.williams@...el.com,
        kwankhede@...dia.com, eric.auger@...hat.com, parav@...lanox.com,
        rafael@...nel.org, netanelg@...lanox.com, shahafs@...lanox.com,
        yan.y.zhao@...ux.intel.com, pbonzini@...hat.com,
        samuel.ortiz@...el.com, mona.hossain@...el.com,
        Megha Dey <megha.dey@...ux.intel.com>,
        dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, kvm@...r.kernel.org,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [PATCH v4 00/17] Add VFIO mediated device support and DEV-MSI
 support for the idxd driver

On Fri, Oct 30, 2020 at 04:30:45PM -0300, Jason Gunthorpe wrote:
> On Fri, Oct 30, 2020 at 12:23:25PM -0700, Raj, Ashok wrote:
> > On Fri, Oct 30, 2020 at 04:17:06PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Oct 30, 2020 at 12:13:48PM -0700, Dave Jiang wrote:
> > > > 
> > > > 
> > > > On 10/30/2020 11:58 AM, Jason Gunthorpe wrote:
> > > > > On Fri, Oct 30, 2020 at 11:50:47AM -0700, Dave Jiang wrote:
> > > > > >   .../ABI/stable/sysfs-driver-dma-idxd          |    6 +
> > > > > >   Documentation/driver-api/vfio/mdev-idxd.rst   |  404 ++++++
> > > > > >   MAINTAINERS                                   |    1 +
> > > > > >   drivers/dma/Kconfig                           |    9 +
> > > > > >   drivers/dma/idxd/Makefile                     |    2 +
> > > > > >   drivers/dma/idxd/cdev.c                       |    6 +-
> > > > > >   drivers/dma/idxd/device.c                     |  294 ++++-
> > > > > >   drivers/dma/idxd/idxd.h                       |   67 +-
> > > > > >   drivers/dma/idxd/init.c                       |   86 ++
> > > > > >   drivers/dma/idxd/irq.c                        |    6 +-
> > > > > >   drivers/dma/idxd/mdev.c                       | 1121 +++++++++++++++++
> > > > > >   drivers/dma/idxd/mdev.h                       |  116 ++
> > > > > 
> > > > > Again, a subsytem driver belongs in the directory hierarchy of the
> > > > > subsystem, not in other random places. All this mdev stuff belongs
> > > > > under drivers/vfio
> > > > 
> > > > Alex seems to have disagreed last time....
> > > > https://lore.kernel.org/dmaengine/20200917113016.425dcde7@x1.home/
> > > 
> > > Nobody else in the kernel is splitting subsystems up anymore
> > >  
> > > > And I do agree with his perspective. The mdev is an extension of the PF
> > > > driver. It's a bit awkward to be a stand alone mdev driver under vfio/mdev/.
> > > 
> > > By this logic we'd have giagantic drivers under drivers/ethernet
> > > touching netdev, rdma, scsi, vdpa, etc just because that is where the
> > > PF driver came from.
> > 
> > What makes you think this is providing services like scsi/rdma/vdpa etc.. ?
> > 
> > for DSA this playes the exact same role, not a different function 
> > as you highlight above. these mdev's are creating DSA for virtualization
> > use. They aren't providing a completely different role or subsystem per-se.
> 
> It is a different subsystem, different maintainer, and different
> reviewers.
> 
> It is a development process problem, it doesn't matter what it is
> doing.

So drawing that parallel, do you expect all drivers that call
pci_register_driver() to be located in drivers/pci? Aren't they scattered
all over the place ata,scsi, platform drivers and such?

As Alex pointed out, i915 and handful of s390 drivers that are mdev users
are not in drivers/vfio. Are you sayint those drivers don't get reviewed? 

This is no different than PF driver offering VF services. Its a logical
extension. 

Reviews happen for mdev users today. What you suggest seems like cutting 
the feet to fit the shoe. Unless the maintainers are asking things 
to be split just because its calling mdev_register_device() that practice 
doesn't exist and would be totally weird if you want to move all callers of
pci_register_driver(). 

Your argument seems interesting even entertaining :-). But honestly i'm not finding it
practical :-). So every caller of mmu_register_notifier() needs to be in
mm? 

What you mention for different functions make absolute sense, not arguing
against that.  but this ain't that. 

And we just follow the asks of the maintainer. 

I know you aren't going to give up, but there is little we can do. I want
the maintainers to make that call and I'm not add more noise to this.

Cheers,
Ashok