Re: [RFC PATCH 15/15] nvme-pci: Allow mmaping the CMB in userspace
On 2020-11-09 8:03 a.m., Keith Busch wrote: > On Fri, Nov 06, 2020 at 10:00:36AM -0700, Logan Gunthorpe wrote: >> Allow userspace to obtain CMB memory by mmaping the controller's >> char device. The mmap call allocates and returns a hunk of CMB memory, >> (the offset is ignored) so userspace does not have control over the >> address within the CMB. >> >> A VMA allocated in this way will only be usable by drivers that set >> FOLL_PCI_P2PDMA when calling GUP. And inter-device support will be >> checked the first time the pages are mapped for DMA. >> >> Currently this is only supported by O_DIRECT to an PCI NVMe device >> or through the NVMe passthrough IOCTL. > > Rather than make this be specific to nvme, could pci p2pdma create an > mmap'able file for any resource registered with it? It's certainly possible. However, other people have been arguing that more of this should be specific to NVMe as some use cases do not want to use the genalloc inside p2pdma. Logan
Re: [RFC PATCH 15/15] nvme-pci: Allow mmaping the CMB in userspace
On Fri, Nov 06, 2020 at 10:00:36AM -0700, Logan Gunthorpe wrote: > Allow userspace to obtain CMB memory by mmaping the controller's > char device. The mmap call allocates and returns a hunk of CMB memory, > (the offset is ignored) so userspace does not have control over the > address within the CMB. > > A VMA allocated in this way will only be usable by drivers that set > FOLL_PCI_P2PDMA when calling GUP. And inter-device support will be > checked the first time the pages are mapped for DMA. > > Currently this is only supported by O_DIRECT to an PCI NVMe device > or through the NVMe passthrough IOCTL. Rather than make this be specific to nvme, could pci p2pdma create an mmap'able file for any resource registered with it?
Re: [RFC PATCH 15/15] nvme-pci: Allow mmaping the CMB in userspace
On 2020-11-06 10:39 a.m., Jason Gunthorpe wrote:
> On Fri, Nov 06, 2020 at 10:00:36AM -0700, Logan Gunthorpe wrote:
>> Allow userspace to obtain CMB memory by mmaping the controller's
>> char device. The mmap call allocates and returns a hunk of CMB memory,
>> (the offset is ignored) so userspace does not have control over the
>> address within the CMB.
>>
>> A VMA allocated in this way will only be usable by drivers that set
>> FOLL_PCI_P2PDMA when calling GUP. And inter-device support will be
>> checked the first time the pages are mapped for DMA.
>>
>> Currently this is only supported by O_DIRECT to an PCI NVMe device
>> or through the NVMe passthrough IOCTL.
>>
>> Signed-off-by: Logan Gunthorpe
>> drivers/nvme/host/core.c | 11 +++
>> drivers/nvme/host/nvme.h | 1 +
>> drivers/nvme/host/pci.c | 9 +
>> 3 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index f14316c9b34a..fc642aba671d 100644
>> +++ b/drivers/nvme/host/core.c
>> @@ -3240,12 +3240,23 @@ static long nvme_dev_ioctl(struct file *file,
>> unsigned int cmd,
>> }
>> }
>>
>> +static int nvme_dev_mmap(struct file *file, struct vm_area_struct *vma)
>> +{
>> +struct nvme_ctrl *ctrl = file->private_data;
>> +
>> +if (!ctrl->ops->mmap_cmb)
>> +return -ENODEV;
>> +
>> +return ctrl->ops->mmap_cmb(ctrl, vma);
>> +}
>
> This needs to ensure that the VMA created is destroyed before the
> driver is unprobed - ie the struct pages backing the BAR memory is
> destroyed.
>
> I don't see anything that synchronizes this in the nvme_dev_release()?
Yup, looks like something that needs to be fixed. Though I'd probably do
it in the pci_p2pdma helper code instead.
Logan
Re: [RFC PATCH 15/15] nvme-pci: Allow mmaping the CMB in userspace
On Fri, Nov 06, 2020 at 10:00:36AM -0700, Logan Gunthorpe wrote:
> Allow userspace to obtain CMB memory by mmaping the controller's
> char device. The mmap call allocates and returns a hunk of CMB memory,
> (the offset is ignored) so userspace does not have control over the
> address within the CMB.
>
> A VMA allocated in this way will only be usable by drivers that set
> FOLL_PCI_P2PDMA when calling GUP. And inter-device support will be
> checked the first time the pages are mapped for DMA.
>
> Currently this is only supported by O_DIRECT to an PCI NVMe device
> or through the NVMe passthrough IOCTL.
>
> Signed-off-by: Logan Gunthorpe
> drivers/nvme/host/core.c | 11 +++
> drivers/nvme/host/nvme.h | 1 +
> drivers/nvme/host/pci.c | 9 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f14316c9b34a..fc642aba671d 100644
> +++ b/drivers/nvme/host/core.c
> @@ -3240,12 +3240,23 @@ static long nvme_dev_ioctl(struct file *file,
> unsigned int cmd,
> }
> }
>
> +static int nvme_dev_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct nvme_ctrl *ctrl = file->private_data;
> +
> + if (!ctrl->ops->mmap_cmb)
> + return -ENODEV;
> +
> + return ctrl->ops->mmap_cmb(ctrl, vma);
> +}
This needs to ensure that the VMA created is destroyed before the
driver is unprobed - ie the struct pages backing the BAR memory is
destroyed.
I don't see anything that synchronizes this in the nvme_dev_release()?
Many places do this by putting all the VMAs into an address space and
zaping the address space when unprobing the driver to revoke the
pages, but there is a tricky race here :\
https://lore.kernel.org/dri-devel/20201021125030.gk36...@ziepe.ca/
Jason
