linux-kernel - Re: [RFT][PATCH v1 6/6] vfio: Replace phys_pfn with phys_page for vfio_pin

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:   Fri, 17 Jun 2022 15:06:25 -0700
From:   Nicolin Chen <nicolinc@...dia.com>
To:     Christoph Hellwig <hch@...radead.org>
CC:     <kwankhede@...dia.com>, <corbet@....net>, <hca@...ux.ibm.com>,
        <gor@...ux.ibm.com>, <agordeev@...ux.ibm.com>,
        <borntraeger@...ux.ibm.com>, <svens@...ux.ibm.com>,
        <zhenyuw@...ux.intel.com>, <zhi.a.wang@...el.com>,
        <jani.nikula@...ux.intel.com>, <joonas.lahtinen@...ux.intel.com>,
        <rodrigo.vivi@...el.com>, <tvrtko.ursulin@...ux.intel.com>,
        <airlied@...ux.ie>, <daniel@...ll.ch>, <farman@...ux.ibm.com>,
        <mjrosato@...ux.ibm.com>, <pasic@...ux.ibm.com>,
        <vneethv@...ux.ibm.com>, <oberpar@...ux.ibm.com>,
        <freude@...ux.ibm.com>, <akrowiak@...ux.ibm.com>,
        <jjherne@...ux.ibm.com>, <alex.williamson@...hat.com>,
        <cohuck@...hat.com>, <jgg@...dia.com>, <kevin.tian@...el.com>,
        <jchrist@...ux.ibm.com>, <kvm@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-s390@...r.kernel.org>,
        <intel-gvt-dev@...ts.freedesktop.org>,
        <intel-gfx@...ts.freedesktop.org>,
        <dri-devel@...ts.freedesktop.org>
Subject: Re: [RFT][PATCH v1 6/6] vfio: Replace phys_pfn with phys_page for
 vfio_pin_pages()

On Fri, Jun 17, 2022 at 01:54:05AM -0700, Christoph Hellwig wrote:
> There is a bunch of code an comments in the iommu type1 code that
> suggest we can pin memory that is not page backed.  

Would you mind explaining the use case for pinning memory that
isn't page backed? And do we have such use case so far?

> >  int vfio_pin_pages(struct vfio_device *device, dma_addr_t iova,
> > +		   int npage, int prot, struct page **phys_page)
> 
> I don't think phys_page makes much sense as an argument name.
> I'd just call this pages.

OK.

> > +			phys_page[i] = pfn_to_page(vpfn->pfn);
> 
> Please store the actual page pointer in the vfio_pfn structure.

OK.

> >  		remote_vaddr = dma->vaddr + (iova - dma->iova);
> > -		ret = vfio_pin_page_external(dma, remote_vaddr, &phys_pfn[i],
> > +		ret = vfio_pin_page_external(dma, remote_vaddr, &phys_pfn,
> >  					     do_accounting);
> 
> Please just return the actual page from vaddr_get_pfns through
> vfio_pin_pages_remote and vfio_pin_page_external, maybe even as a prep
> patch as that is a fair amount of churn.

I can do that. I tried once, but there were just too much changes
inside type1 code that felt like a chain reaction. If we plan to
eventually replace with IOMMUFD implementations, these changes in
type1 might not be necessary, I thought.