Re: S390 testing for IOMMUFD

[Eric Farman <farman@linux.ibm.com>]

On Tue, 2022-11-08 at 10:37 -0400, Jason Gunthorpe wrote:
> On Tue, Nov 08, 2022 at 09:19:17AM -0500, Eric Farman wrote:
> > On Tue, 2022-11-08 at 09:54 -0400, Jason Gunthorpe wrote:
> > > On Tue, Nov 08, 2022 at 08:50:53AM -0500, Matthew Rosato wrote:
> > > 
> > > > FWIW, vfio-pci via s390 is working fine so far, though I'll put
> > > > it
> > > > through more paces over the next few weeks and report if I find
> > > > anything.
> > > 
> > > OK great
> > > 
> > > > As far as mdev drivers...  
> > > > 
> > > > -ccw: Sounds like Eric is already aware there is an issue and
> > > > is
> > > > investigating (I see errors as well).
> > 
> > I -think- the problem for -ccw is that the new vfio_pin_pages
> > requires
> > the input addresses to be page-aligned, and while most of ours are,
> > the
> > first one in any given transaction may not be. We never bothered to
> > mask off the addresses since it was handled for us, and we needed
> > to
> > keep the offsets anyway.
> > 
> > By happenstance, I had some code that would do the masking
> > ourselves
> > (for an unrelated reason); I'll see if I can get that fit on top
> > and if
> > it helps matters. After coffee.
> 
> Oh, yes, that makes alot of sense.
> 
> Ah, if that is how VFIO worked we could match it like below:

That's a start. The pin appears to have worked, but the unpin fails at
the bottom of iommufd_access_unpin_pages:

WARN_ON(!iopt_area_contig_done(&iter));

> 
>  EXPORT_SYMBOL_NS_GPL(iommufd_access_unpin_pages, IOMMUFD);
>  
>  static bool iopt_area_contig_is_aligned(struct iopt_area_contig_iter
> *iter,
> -                                       bool first)
> +                                       bool first, unsigned long
> first_iova)
>  {
> -       if (iopt_area_start_byte(iter->area, iter->cur_iova) %
> PAGE_SIZE)
> +       unsigned long start_offset = first ? (first_iova % PAGE_SIZE)
> : 0;
> +
> +       if ((iopt_area_start_byte(iter->area, iter->cur_iova) %
> PAGE_SIZE) !=
> +           start_offset)
>                 return false;
>  
>         if (!iopt_area_contig_done(iter) &&
> @@ -607,7 +610,7 @@ int iommufd_access_pin_pages(struct
> iommufd_access *access, unsigned long iova,
>                         iopt_area_iova_to_index(area, iter.cur_iova);
>  
>                 if (area->prevent_access ||
> -                   !iopt_area_contig_is_aligned(&iter, first)) {
> +                   !iopt_area_contig_is_aligned(&iter, first, iova))
> {
>                         rc = -EINVAL;
>                         goto err_remove;
>                 }
> 
> Jason