Re: S390 testing for IOMMUFD

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 11/8/22 10:29 AM, Eric Farman wrote:
> 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));
> 

Update on why -ap is failing -- I see vfio_pin_pages requests from vfio_ap_irq_enable that are failing on -EINVAL -- input is not page-aligned, just like what vfio-ccw was hitting.

I just tried a quick hack to force these to page-aligned requests and with that the vfio-ap tests I'm running start passing again.  So I think a proper fix in the iommufd code for this will also fix vfio-ap (we will test of course)

>>
>>  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
>