On Wed, 14 Aug 2024 10:25:08 -0300
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> On Fri, Aug 09, 2024 at 12:09:09PM -0400, Peter Xu wrote:
> > @@ -1672,30 +1679,49 @@ static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf)
> > if (vdev->pm_runtime_engaged || !__vfio_pci_memory_enabled(vdev))
> > goto out_unlock;
> >
> > - ret = vmf_insert_pfn(vma, vmf->address, pfn + pgoff);
> > - if (ret & VM_FAULT_ERROR)
> > - goto out_unlock;
> > -
> > - /*
> > - * Pre-fault the remainder of the vma, abort further insertions and
> > - * supress error if fault is encountered during pre-fault.
> > - */
> > - for (; addr < vma->vm_end; addr += PAGE_SIZE, pfn++) {
> > - if (addr == vmf->address)
> > - continue;
> > -
> > - if (vmf_insert_pfn(vma, addr, pfn) & VM_FAULT_ERROR)
> > - break;
> > + switch (order) {
> > + case 0:
> > + ret = vmf_insert_pfn(vma, vmf->address, pfn + pgoff);
> > + break;
> > +#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> > + case PMD_ORDER:
> > + ret = vmf_insert_pfn_pmd(vmf, __pfn_to_pfn_t(pfn + pgoff,
> > + PFN_DEV), false);
> > + break;
> > +#endif
> > +#ifdef CONFIG_ARCH_SUPPORTS_PUD_PFNMAP
> > + case PUD_ORDER:
> > + ret = vmf_insert_pfn_pud(vmf, __pfn_to_pfn_t(pfn + pgoff,
> > + PFN_DEV), false);
> > + break;
> > +#endif
>
> I feel like this switch should be in some general function?
>
> vmf_insert_pfn_order(vmf, order, __pfn_to_pfn_t(pfn + pgoff, PFN_DEV), false);
>
> No reason to expose every driver to this when you've already got a
> nice contract to have the driver work on the passed in order.
>
> What happens if the driver can't get a PFN that matches the requested
> order?
There was some alignment and size checking chopped from the previous
reply that triggered a fallback, but in general PCI BARs are a power of
two and naturally aligned, so there should always be an order aligned
pfn. Thanks,
Alex
