linux-kernel - RE: [PATCH v9 04/12] vfio iommu: Add support for mediated devices

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]

Date:   Wed, 26 Oct 2016 07:19:54 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>,
        Kirti Wankhede <kwankhede@...dia.com>
CC:     "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "kraxel@...hat.com" <kraxel@...hat.com>,
        "cjia@...dia.com" <cjia@...dia.com>,
        "qemu-devel@...gnu.org" <qemu-devel@...gnu.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Song, Jike" <jike.song@...el.com>,
        "bjsdjshi@...ux.vnet.ibm.com" <bjsdjshi@...ux.vnet.ibm.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v9 04/12] vfio iommu: Add support for mediated devices

> From: Alex Williamson [mailto:alex.williamson@...hat.com]
> Sent: Monday, October 24, 2016 10:32 AM
> 
> > >> -static long vfio_unpin_pages(unsigned long pfn, long npage,
> > >> -			     int prot, bool do_accounting)
> > >> +static long __vfio_unpin_pages_remote(struct vfio_iommu *iommu,
> > >> +				      unsigned long pfn, long npage, int prot,
> > >> +				      bool do_accounting)
> > >
> > > Have you noticed that it's kind of confusing that
> > > __vfio_{un}pin_pages_remote() uses current, which does a
> > > get_user_pages_fast() while "local" uses a provided task_struct and
> > > uses get_user_pages_*remote*()?  And also what was effectively local
> > > (ie. we're pinning for our own use here) is now "remote" and pinning
> > > for a remote, vendor driver consumer, is now "local".  It's not very
> > > intuitive.
> > >

I questioned this confusing naming in v8 too...

> >
> > 'local' in local_domain was suggested to describe the domain for local
> > page tracking. Earlier suggestions to have 'mdev' or 'noimmu' in this
> > name were discarded. May be we should revisit what the name should be.
> > Any suggestion?
> >
> > For local_domain, to pin pages, flow is:
> >
> > for local_domain
> >     |- vfio_pin_pages()
> >         |- vfio_iommu_type1_pin_pages()
> >             |- __vfio_pin_page_local()
> >                 |-  vaddr_get_pfn(task->mm)
> >                     |- get_user_pages_remote()
> >
> > __vfio_pin_page_local() --> get_user_pages_remote()
> 
> 
> In vfio.c we have the concept of an external user, perhaps that could
> be continued here.  An mdev driver would be an external, or remote
> pinning.
> 

I prefer to use remote here. It's aligned with underlying mm operations

Thanks
Kevin