S390 testing for IOMMUFD

S390 testing for IOMMUFD

[Jason Gunthorpe]

On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
> [
> This has been in linux-next for a little while now, and we've completed
> the syzkaller run. 1300 hours of CPU time have been invested since the
> last report with no improvement in coverage or new detections. syzkaller
> coverage reached 69%(75%), and review of the misses show substantial
> amounts are WARN_ON's and other debugging which are not expected to be
> covered.
> ]
> 
> iommufd is the user API to control the IOMMU subsystem as it relates to
> managing IO page tables that point at user space memory.

[chop cc list]

s390 mdev maintainers,

Can I ask your help to test this with the two S390 mdev drivers? Now
that gvt is passing and we've covered alot of the QA ground it is a
good time to run it.

Take the branch from here:

https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next

And build the kernel with 

CONFIG_VFIO_CONTAINER=n
CONFIG_IOMMUFD=y
CONFIG_IOMMUFD_VFIO_CONTAINER=y

And your existing stuff should work with iommufd providing the iommu
support to vfio. There will be a dmesg confirming this.

Let me know if there are any problems!

If I recall there was some desire from the S390 platform team to start
building on iommufd to create some vIOMMU acceleration for S390
guests, this is a necessary first step.

Thanks,
Jason

Re: S390 testing for IOMMUFD

[Christian Borntraeger]

Am 08.11.22 um 02:09 schrieb Jason Gunthorpe:
> On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
>> [
>> This has been in linux-next for a little while now, and we've completed
>> the syzkaller run. 1300 hours of CPU time have been invested since the
>> last report with no improvement in coverage or new detections. syzkaller
>> coverage reached 69%(75%), and review of the misses show substantial
>> amounts are WARN_ON's and other debugging which are not expected to be
>> covered.
>> ]
>>
>> iommufd is the user API to control the IOMMU subsystem as it relates to
>> managing IO page tables that point at user space memory.
> 
> [chop cc list]
> 
> s390 mdev maintainers,
> 
> Can I ask your help to test this with the two S390 mdev drivers? Now
> that gvt is passing and we've covered alot of the QA ground it is a
> good time to run it.
> 
> Take the branch from here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next

> 
> And build the kernel with
> 
> CONFIG_VFIO_CONTAINER=n
> CONFIG_IOMMUFD=y
> CONFIG_IOMMUFD_VFIO_CONTAINER=y
> 
> And your existing stuff should work with iommufd providing the iommu
> support to vfio. There will be a dmesg confirming this.

Gave it a quick spin with vfio_ap:
[  401.679199] vfio_ap_mdev b01a7c33-9696-48b2-9a98-050e8e17c69a: Adding to iommu group 1
[  402.085386] iommufd: IOMMUFD is providing /dev/vfio/vfio, not VFIO.

Some tests seem to work, but others dont (running into timeouts). I need to look
into that (or ideally Tony will have a look, FWIW tests.test_vfio_ap.VfioAPAssignMdevToGuestTest
fails for me.


The same kernel tree with defconfig (instead of CONFIG_IOMMUFD_VFIO_CONTAINER=y) works fine.
> 
> Let me know if there are any problems!
> 
> If I recall there was some desire from the S390 platform team to start
> building on iommufd to create some vIOMMU acceleration for S390
> guests, this is a necessary first step.
> 
> Thanks,
> Jason

Re: S390 testing for IOMMUFD

[Anthony Krowiak]

On 11/8/22 5:12 AM, Christian Borntraeger wrote:
>
>
> Am 08.11.22 um 02:09 schrieb Jason Gunthorpe:
>> On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
>>> [
>>> This has been in linux-next for a little while now, and we've completed
>>> the syzkaller run. 1300 hours of CPU time have been invested since the
>>> last report with no improvement in coverage or new detections. 
>>> syzkaller
>>> coverage reached 69%(75%), and review of the misses show substantial
>>> amounts are WARN_ON's and other debugging which are not expected to be
>>> covered.
>>> ]
>>>
>>> iommufd is the user API to control the IOMMU subsystem as it relates to
>>> managing IO page tables that point at user space memory.
>>
>> [chop cc list]
>>
>> s390 mdev maintainers,
>>
>> Can I ask your help to test this with the two S390 mdev drivers? Now
>> that gvt is passing and we've covered alot of the QA ground it is a
>> good time to run it.
>>
>> Take the branch from here:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next 
>>
>
>>
>> And build the kernel with
>>
>> CONFIG_VFIO_CONTAINER=n
>> CONFIG_IOMMUFD=y
>> CONFIG_IOMMUFD_VFIO_CONTAINER=y
>>
>> And your existing stuff should work with iommufd providing the iommu
>> support to vfio. There will be a dmesg confirming this.
>
> Gave it a quick spin with vfio_ap:
> [  401.679199] vfio_ap_mdev b01a7c33-9696-48b2-9a98-050e8e17c69a: 
> Adding to iommu group 1
> [  402.085386] iommufd: IOMMUFD is providing /dev/vfio/vfio, not VFIO.
>
> Some tests seem to work, but others dont (running into timeouts). I 
> need to look
> into that (or ideally Tony will have a look, FWIW 
> tests.test_vfio_ap.VfioAPAssignMdevToGuestTest
> fails for me.


I'm looking into it.


>
>
> The same kernel tree with defconfig (instead of 
> CONFIG_IOMMUFD_VFIO_CONTAINER=y) works fine.
>>
>> Let me know if there are any problems!
>>
>> If I recall there was some desire from the S390 platform team to start
>> building on iommufd to create some vIOMMU acceleration for S390
>> guests, this is a necessary first step.
>>
>> Thanks,
>> Jason

Re: S390 testing for IOMMUFD

[Anthony Krowiak]

On 11/8/22 9:04 AM, Anthony Krowiak wrote:
>
> On 11/8/22 5:12 AM, Christian Borntraeger wrote:
>>
>>
>> Am 08.11.22 um 02:09 schrieb Jason Gunthorpe:
>>> On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
>>>> [
>>>> This has been in linux-next for a little while now, and we've 
>>>> completed
>>>> the syzkaller run. 1300 hours of CPU time have been invested since the
>>>> last report with no improvement in coverage or new detections. 
>>>> syzkaller
>>>> coverage reached 69%(75%), and review of the misses show substantial
>>>> amounts are WARN_ON's and other debugging which are not expected to be
>>>> covered.
>>>> ]
>>>>
>>>> iommufd is the user API to control the IOMMU subsystem as it 
>>>> relates to
>>>> managing IO page tables that point at user space memory.
>>>
>>> [chop cc list]
>>>
>>> s390 mdev maintainers,
>>>
>>> Can I ask your help to test this with the two S390 mdev drivers? Now
>>> that gvt is passing and we've covered alot of the QA ground it is a
>>> good time to run it.
>>>
>>> Take the branch from here:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next 
>>>
>>
>>>
>>> And build the kernel with
>>>
>>> CONFIG_VFIO_CONTAINER=n
>>> CONFIG_IOMMUFD=y
>>> CONFIG_IOMMUFD_VFIO_CONTAINER=y
>>>
>>> And your existing stuff should work with iommufd providing the iommu
>>> support to vfio. There will be a dmesg confirming this.
>>
>> Gave it a quick spin with vfio_ap:
>> [  401.679199] vfio_ap_mdev b01a7c33-9696-48b2-9a98-050e8e17c69a: 
>> Adding to iommu group 1
>> [  402.085386] iommufd: IOMMUFD is providing /dev/vfio/vfio, not VFIO.
>>
>> Some tests seem to work, but others dont (running into timeouts). I 
>> need to look
>> into that (or ideally Tony will have a look, FWIW 
>> tests.test_vfio_ap.VfioAPAssignMdevToGuestTest
>> fails for me.
>
>
> I'm looking into it.


I cloned the 
https://lore.kernel.org/kvm/Y2q3nFXwOk9jul5u@nvidia.com/T/#m76a9c609c5ccd1494c05c6f598f9c8e75b7c9888 
repo and ran the vfio_ap test cases. The tests ran without encountering 
the errors related to the vfio_pin_pages() function, but I did see two 
tests fail attempting to run crypto tests on the guest. I also saw a 
WARN_ON stack trace in the dmesg output indicating a timeout occurred 
trying to verify the completion of a queue reset. The reset problem has 
reared its ugly head in our CI, so this may be a good thing as it will 
allow me to debug why its happening.


>
>
>>
>>
>> The same kernel tree with defconfig (instead of 
>> CONFIG_IOMMUFD_VFIO_CONTAINER=y) works fine.
>>>
>>> Let me know if there are any problems!
>>>
>>> If I recall there was some desire from the S390 platform team to start
>>> building on iommufd to create some vIOMMU acceleration for S390
>>> guests, this is a necessary first step.
>>>
>>> Thanks,
>>> Jason

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

On Wed, Nov 09, 2022 at 09:49:01AM -0500, Anthony Krowiak wrote:
> I cloned the https://lore.kernel.org/kvm/Y2q3nFXwOk9jul5u@nvidia.com/T/#m76a9c609c5ccd1494c05c6f598f9c8e75b7c9888
> repo and ran the vfio_ap test cases. The tests ran without encountering the
> errors related to the vfio_pin_pages() function

I updated the git repos with this change now

> but I did see two tests fail attempting to run crypto tests on the
> guest. I also saw a WARN_ON stack trace in the dmesg output
> indicating a timeout occurred trying to verify the completion of a
> queue reset. The reset problem has reared its ugly head in our CI,
> so this may be a good thing as it will allow me to debug why its
> happening.

Please let me know if you think this is iommufd related, from your
description it sounds like an existing latent bug?

Thanks,
Jason

Re: S390 testing for IOMMUFD

[Anthony Krowiak]

On 11/9/22 11:12 AM, Jason Gunthorpe wrote:
> On Wed, Nov 09, 2022 at 09:49:01AM -0500, Anthony Krowiak wrote:
>> I cloned the https://lore.kernel.org/kvm/Y2q3nFXwOk9jul5u@nvidia.com/T/#m76a9c609c5ccd1494c05c6f598f9c8e75b7c9888
>> repo and ran the vfio_ap test cases. The tests ran without encountering the
>> errors related to the vfio_pin_pages() function
> I updated the git repos with this change now
>
>> but I did see two tests fail attempting to run crypto tests on the
>> guest. I also saw a WARN_ON stack trace in the dmesg output
>> indicating a timeout occurred trying to verify the completion of a
>> queue reset. The reset problem has reared its ugly head in our CI,
>> so this may be a good thing as it will allow me to debug why its
>> happening.
> Please let me know if you think this is iommufd related, from your
> description it sounds like an existing latent bug?


Just in case you missed my response to my previous email, the problems I 
was seeing were due to using a set regression tests that I patched to 
try to improve the tests performance. When I ran the vanilla tests they 
ran successfully without any problems with your patch. I will continue 
testing but as of now, it looks good to me.


>
> Thanks,
> Jason

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

On Wed, Nov 09, 2022 at 02:13:22PM -0500, Anthony Krowiak wrote:
> 
> On 11/9/22 11:12 AM, Jason Gunthorpe wrote:
> > On Wed, Nov 09, 2022 at 09:49:01AM -0500, Anthony Krowiak wrote:
> > > I cloned the https://lore.kernel.org/kvm/Y2q3nFXwOk9jul5u@nvidia.com/T/#m76a9c609c5ccd1494c05c6f598f9c8e75b7c9888
> > > repo and ran the vfio_ap test cases. The tests ran without encountering the
> > > errors related to the vfio_pin_pages() function
> > I updated the git repos with this change now
> > 
> > > but I did see two tests fail attempting to run crypto tests on the
> > > guest. I also saw a WARN_ON stack trace in the dmesg output
> > > indicating a timeout occurred trying to verify the completion of a
> > > queue reset. The reset problem has reared its ugly head in our CI,
> > > so this may be a good thing as it will allow me to debug why its
> > > happening.
> > Please let me know if you think this is iommufd related, from your
> > description it sounds like an existing latent bug?
> 
> 
> Just in case you missed my response to my previous email, the problems I was
> seeing were due to using a set regression tests that I patched to try to
> improve the tests performance. When I ran the vanilla tests they ran
> successfully without any problems with your patch. I will continue testing
> but as of now, it looks good to me.

Great, can all of you provide some Tested-by's for the various things
you've run?

Thanks,
Jason

Re: S390 testing for IOMMUFD

[Anthony Krowiak]

On 11/9/22 9:49 AM, Anthony Krowiak wrote:
>
> On 11/8/22 9:04 AM, Anthony Krowiak wrote:
>>
>> On 11/8/22 5:12 AM, Christian Borntraeger wrote:
>>>
>>>
>>> Am 08.11.22 um 02:09 schrieb Jason Gunthorpe:
>>>> On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
>>>>> [
>>>>> This has been in linux-next for a little while now, and we've 
>>>>> completed
>>>>> the syzkaller run. 1300 hours of CPU time have been invested since 
>>>>> the
>>>>> last report with no improvement in coverage or new detections. 
>>>>> syzkaller
>>>>> coverage reached 69%(75%), and review of the misses show substantial
>>>>> amounts are WARN_ON's and other debugging which are not expected 
>>>>> to be
>>>>> covered.
>>>>> ]
>>>>>
>>>>> iommufd is the user API to control the IOMMU subsystem as it 
>>>>> relates to
>>>>> managing IO page tables that point at user space memory.
>>>>
>>>> [chop cc list]
>>>>
>>>> s390 mdev maintainers,
>>>>
>>>> Can I ask your help to test this with the two S390 mdev drivers? Now
>>>> that gvt is passing and we've covered alot of the QA ground it is a
>>>> good time to run it.
>>>>
>>>> Take the branch from here:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next 
>>>>
>>>
>>>>
>>>> And build the kernel with
>>>>
>>>> CONFIG_VFIO_CONTAINER=n
>>>> CONFIG_IOMMUFD=y
>>>> CONFIG_IOMMUFD_VFIO_CONTAINER=y
>>>>
>>>> And your existing stuff should work with iommufd providing the iommu
>>>> support to vfio. There will be a dmesg confirming this.
>>>
>>> Gave it a quick spin with vfio_ap:
>>> [  401.679199] vfio_ap_mdev b01a7c33-9696-48b2-9a98-050e8e17c69a: 
>>> Adding to iommu group 1
>>> [  402.085386] iommufd: IOMMUFD is providing /dev/vfio/vfio, not VFIO.
>>>
>>> Some tests seem to work, but others dont (running into timeouts). I 
>>> need to look
>>> into that (or ideally Tony will have a look, FWIW 
>>> tests.test_vfio_ap.VfioAPAssignMdevToGuestTest
>>> fails for me.
>>
>>
>> I'm looking into it.
>
>
> I cloned the 
> https://lore.kernel.org/kvm/Y2q3nFXwOk9jul5u@nvidia.com/T/#m76a9c609c5ccd1494c05c6f598f9c8e75b7c9888 
> repo and ran the vfio_ap test cases. The tests ran without 
> encountering the errors related to the vfio_pin_pages() function, but 
> I did see two tests fail attempting to run crypto tests on the guest. 
> I also saw a WARN_ON stack trace in the dmesg output indicating a 
> timeout occurred trying to verify the completion of a queue reset. The 
> reset problem has reared its ugly head in our CI, so this may be a 
> good thing as it will allow me to debug why its happening.


The problems I encountered were due to using a set of regression tests 
that were not vanilla. They contained some changes I made to try to 
improve performance of the tests. After restoring the vanilla regression 
tests, I was able to successfully execute the tests without any problems 
or errors with Jason's vfio_ap_(un)pin_pages patch installed, so that 
looks good to me.


>
>
>>
>>
>>>
>>>
>>> The same kernel tree with defconfig (instead of 
>>> CONFIG_IOMMUFD_VFIO_CONTAINER=y) works fine.
>>>>
>>>> Let me know if there are any problems!
>>>>
>>>> If I recall there was some desire from the S390 platform team to start
>>>> building on iommufd to create some vIOMMU acceleration for S390
>>>> guests, this is a necessary first step.
>>>>
>>>> Thanks,
>>>> Jason

Re: S390 testing for IOMMUFD

[Matthew Rosato]

On 11/7/22 8:09 PM, Jason Gunthorpe wrote:
> On Mon, Nov 07, 2022 at 08:48:53PM -0400, Jason Gunthorpe wrote:
>> [
>> This has been in linux-next for a little while now, and we've completed
>> the syzkaller run. 1300 hours of CPU time have been invested since the
>> last report with no improvement in coverage or new detections. syzkaller
>> coverage reached 69%(75%), and review of the misses show substantial
>> amounts are WARN_ON's and other debugging which are not expected to be
>> covered.
>> ]
>>
>> iommufd is the user API to control the IOMMU subsystem as it relates to
>> managing IO page tables that point at user space memory.
> 
> [chop cc list]
> 
> s390 mdev maintainers,
> 
> Can I ask your help to test this with the two S390 mdev drivers? Now
> that gvt is passing and we've covered alot of the QA ground it is a
> good time to run it.
> 
> Take the branch from here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/log/?h=for-next
> 
> And build the kernel with 
> 
> CONFIG_VFIO_CONTAINER=n
> CONFIG_IOMMUFD=y
> CONFIG_IOMMUFD_VFIO_CONTAINER=y
> 
> And your existing stuff should work with iommufd providing the iommu
> support to vfio. There will be a dmesg confirming this.
> 
> Let me know if there are any problems!

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.

As far as mdev drivers...  

-ccw: Sounds like Eric is already aware there is an issue and is investigating (I see errors as well).

-ap: I see the exact same issue that Christian mentioned...  I'll talk to Tony & Jason about it.

> 
> If I recall there was some desire from the S390 platform team to start
> building on iommufd to create some vIOMMU acceleration for S390
> guests, this is a necessary first step.

There's probably something here for -ccw in the future, but you might be thinking of s390 vfio-pci e.g. to implement the in-kernel handling of nested mappings on s390 -- yep, work in in progress here, not ready for sharing yet but I have been most recently basing my work on top of the nesting series https://github.com/yiliu1765/iommufd/tree/iommufd-v6.0-rc3-nesting

Matt

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

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).
> 
> -ap: I see the exact same issue that Christian mentioned...  I'll talk to Tony & Jason about it.

A clue what is going wrong might get a quick realization on the
problem?

I'm guessing something in the vfio side more than the access 'pinning'
part?

> > If I recall there was some desire from the S390 platform team to start
> > building on iommufd to create some vIOMMU acceleration for S390
> > guests, this is a necessary first step.
> 
> There's probably something here for -ccw in the future, but you
> might be thinking of s390 vfio-pci e.g. to implement the in-kernel
> handling of nested mappings on s390 -- yep, work in in progress
> here, not ready for sharing yet but I have been most recently basing
> my work on top of the nesting series
> https://github.com/yiliu1765/iommufd/tree/iommufd-v6.0-rc3-nesting

The relation is that if vfio-pci wants to do the above then
vfio-ccw/ap will have to run on iommufd when used in the same VM as
vfio-pci.

So we need it to work :)

Jason

Re: S390 testing for IOMMUFD

[Eric Farman]

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.

Eric

> > 
> > -ap: I see the exact same issue that Christian mentioned...  I'll
> > talk to Tony & Jason about it.
> 
> A clue what is going wrong might get a quick realization on the
> problem?
> 
> I'm guessing something in the vfio side more than the access
> 'pinning'
> part?
> 
> > > If I recall there was some desire from the S390 platform team to
> > > start
> > > building on iommufd to create some vIOMMU acceleration for S390
> > > guests, this is a necessary first step.
> > 
> > There's probably something here for -ccw in the future, but you
> > might be thinking of s390 vfio-pci e.g. to implement the in-kernel
> > handling of nested mappings on s390 -- yep, work in in progress
> > here, not ready for sharing yet but I have been most recently
> > basing
> > my work on top of the nesting series
> > https://github.com/yiliu1765/iommufd/tree/iommufd-v6.0-rc3-nesting
> 
> The relation is that if vfio-pci wants to do the above then
> vfio-ccw/ap will have to run on iommufd when used in the same VM as
> vfio-pci.
> 
> So we need it to work :)
> 
> Jason

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

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:

 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

Re: S390 testing for IOMMUFD

[Eric Farman]

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

Re: S390 testing for IOMMUFD

[Matthew Rosato]

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
> 

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

On Tue, Nov 08, 2022 at 02:18:12PM -0500, Matthew Rosato wrote:

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

Right, so my first fix isn't the right thing. The APIs are mismatched
too much. The length gets all messed up in the process.

So how about this? (drop the prior attempt)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index d835a77aaf26d9..b590ca3c186396 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -1906,8 +1906,13 @@ int vfio_pin_pages(struct vfio_device *device, dma_addr_t iova,
 
 		if (iova > ULONG_MAX)
 			return -EINVAL;
+		/*
+		 * VFIO ignores the sub page offset, npages is from the start of
+		 * a PAGE_SIZE chunk of IOVA.
+		 */
 		ret = iommufd_access_pin_pages(
-			device->iommufd_access, iova, npage * PAGE_SIZE, pages,
+			device->iommufd_access, ALIGN_DOWN(iova, PAGE_SIZE),
+			npage * PAGE_SIZE, pages,
 			(prot & IOMMU_WRITE) ? IOMMUFD_ACCESS_RW_WRITE : 0);
 		if (ret)
 			return ret;
@@ -1937,7 +1942,8 @@ void vfio_unpin_pages(struct vfio_device *device, dma_addr_t iova, int npage)
 	if (device->iommufd_access) {
 		if (WARN_ON(iova > ULONG_MAX))
 			return;
-		iommufd_access_unpin_pages(device->iommufd_access, iova,
+		iommufd_access_unpin_pages(device->iommufd_access,
+					   ALIGN_DOWN(iova, PAGE_SIZE),
 					   npage * PAGE_SIZE);
 		return;
 	}

Thanks,
Jason

Re: S390 testing for IOMMUFD

[Eric Farman]

On Tue, 2022-11-08 at 16:04 -0400, Jason Gunthorpe wrote:
> On Tue, Nov 08, 2022 at 02:18:12PM -0500, Matthew Rosato wrote:
> 
> > 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)
> 
> Right, so my first fix isn't the right thing. The APIs are mismatched
> too much. The length gets all messed up in the process.
> 
> So how about this? (drop the prior attempt)

That seems to get the sniff tests for both -ccw and -ap working. I'll
keep playing with it for -ccw; Tony and Jason can do more validation on
the -ap side.

> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index d835a77aaf26d9..b590ca3c186396 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -1906,8 +1906,13 @@ int vfio_pin_pages(struct vfio_device *device,
> dma_addr_t iova,
>  
>                 if (iova > ULONG_MAX)
>                         return -EINVAL;
> +               /*
> +                * VFIO ignores the sub page offset, npages is from
> the start of
> +                * a PAGE_SIZE chunk of IOVA.
> +                */
>                 ret = iommufd_access_pin_pages(
> -                       device->iommufd_access, iova, npage *
> PAGE_SIZE, pages,
> +                       device->iommufd_access, ALIGN_DOWN(iova,
> PAGE_SIZE),
> +                       npage * PAGE_SIZE, pages,
>                         (prot & IOMMU_WRITE) ?
> IOMMUFD_ACCESS_RW_WRITE : 0);
>                 if (ret)
>                         return ret;
> @@ -1937,7 +1942,8 @@ void vfio_unpin_pages(struct vfio_device
> *device, dma_addr_t iova, int npage)
>         if (device->iommufd_access) {
>                 if (WARN_ON(iova > ULONG_MAX))
>                         return;
> -               iommufd_access_unpin_pages(device->iommufd_access,
> iova,
> +               iommufd_access_unpin_pages(device->iommufd_access,
> +                                          ALIGN_DOWN(iova,
> PAGE_SIZE),
>                                            npage * PAGE_SIZE);
>                 return;
>         }
> 
> Thanks,
> Jason

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

On Tue, Nov 08, 2022 at 10:29:33AM -0500, 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));

This seems like a different bug, probably a ccw driver bug. The
WARN_ON is designed to detect cases where the driver is unpinning an
IOVA range that is not exactly what it pinned. The pin side already
does this validation, so if it fails it means pin/unpin did not have
identical iova ranges. Some debugging prints should confirm this.

I looked at CCW and came up with the following two things, can you
look at them and finish them off? It will probably help.

Thanks,
Jason

[-- Attachment #2: 0001-vfio-ccw-Convert-to-use-vfio_dma_rw.patch --]
[-- Type: text/x-diff, Size: 2878 bytes --]

From b6884847ece19733065fa246c3bbea63cec474c3 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgg@nvidia.com>
Date: Tue, 8 Nov 2022 15:21:04 -0400
Subject: [PATCH 1/2] vfio/ccw: Convert to use vfio_dma_rw()

Do not open code a slow version of vfio_dma_rw() as copy_from_iova().
The core code provides this function now, just call it directly.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_cp.c | 57 ++++------------------------------
 1 file changed, 6 insertions(+), 51 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index 7b02e97f4b2914..f5f6eff005b99f 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -228,51 +228,6 @@ static void convert_ccw0_to_ccw1(struct ccw1 *source, unsigned long len)
 	}
 }
 
-/*
- * Within the domain (@mdev), copy @n bytes from a guest physical
- * address (@iova) to a host physical address (@to).
- */
-static long copy_from_iova(struct vfio_device *vdev, void *to, u64 iova,
-			   unsigned long n)
-{
-	struct page_array pa = {0};
-	int i, ret;
-	unsigned long l, m;
-
-	ret = page_array_alloc(&pa, iova, n);
-	if (ret < 0)
-		return ret;
-
-	ret = page_array_pin(&pa, vdev);
-	if (ret < 0) {
-		page_array_unpin_free(&pa, vdev);
-		return ret;
-	}
-
-	l = n;
-	for (i = 0; i < pa.pa_nr; i++) {
-		void *from = kmap_local_page(pa.pa_page[i]);
-
-		m = PAGE_SIZE;
-		if (i == 0) {
-			from += iova & (PAGE_SIZE - 1);
-			m -= iova & (PAGE_SIZE - 1);
-		}
-
-		m = min(l, m);
-		memcpy(to + (n - l), from, m);
-		kunmap_local(from);
-
-		l -= m;
-		if (l == 0)
-			break;
-	}
-
-	page_array_unpin_free(&pa, vdev);
-
-	return l;
-}
-
 /*
  * Helpers to operate ccwchain.
  */
@@ -471,10 +426,10 @@ static int ccwchain_handle_ccw(u32 cda, struct channel_program *cp)
 	int len, ret;
 
 	/* Copy 2K (the most we support today) of possible CCWs */
-	len = copy_from_iova(vdev, cp->guest_cp, cda,
-			     CCWCHAIN_LEN_MAX * sizeof(struct ccw1));
-	if (len)
-		return len;
+	ret = vfio_dma_rw(vdev, cda, cp->guest_cp,
+			  CWCHAIN_LEN_MAX * sizeof(struct ccw1));
+	if (ret)
+		return ret;
 
 	/* Convert any Format-0 CCWs to Format-1 */
 	if (!cp->orb.cmd.fmt)
@@ -572,7 +527,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 	if (ccw_is_idal(ccw)) {
 		/* Read first IDAW to see if it's 4K-aligned or not. */
 		/* All subsequent IDAws will be 4K-aligned. */
-		ret = copy_from_iova(vdev, &iova, ccw->cda, sizeof(iova));
+		ret = vfio_dma_rw(vdev, ccw->cda, &iova, sizeof(iova));
 		if (ret)
 			return ret;
 	} else {
@@ -601,7 +556,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 
 	if (ccw_is_idal(ccw)) {
 		/* Copy guest IDAL into host IDAL */
-		ret = copy_from_iova(vdev, idaws, ccw->cda, idal_len);
+		ret = vfio_dma_rw(vdev, ccw->cda, idaws, idal_len);
 		if (ret)
 			goto out_unpin;
 
-- 
2.38.1


[-- Attachment #3: 0002-vfio-ccw-Fix-error-unwinding-around-page_array_unpin.patch --]
[-- Type: text/x-diff, Size: 2653 bytes --]

From 7cd2cccf37db91d18da9d041826f0460a56fc95c Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgg@nvidia.com>
Date: Tue, 8 Nov 2022 15:31:08 -0400
Subject: [PATCH 2/2] vfio/ccw: Fix error unwinding around
 page_array_unpin_free()

We should only call page_array_unpin() if page_array_pin() has succeeded.
If page_array_pin() fails then it undoes all its changes internally.

Split free and unpin into two functions and only call unpin in the one case
where everything has succeeded.

Add missing pa_nr = idaw_nr assignment

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/s390/cio/vfio_ccw_cp.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
index f5f6eff005b99f..4eab1b2fb32dd2 100644
--- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -164,9 +164,8 @@ static int page_array_pin(struct page_array *pa, struct vfio_device *vdev)
 }
 
 /* Unpin the pages before releasing the memory. */
-static void page_array_unpin_free(struct page_array *pa, struct vfio_device *vdev)
+static void page_array_free(struct page_array *pa)
 {
-	page_array_unpin(pa, vdev, pa->pa_nr);
 	kfree(pa->pa_iova);
 }
 
@@ -558,7 +557,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 		/* Copy guest IDAL into host IDAL */
 		ret = vfio_dma_rw(vdev, ccw->cda, idaws, idal_len);
 		if (ret)
-			goto out_unpin;
+			goto out_free_pa;
 
 		/*
 		 * Copy guest IDAWs into page_array, in case the memory they
@@ -566,6 +565,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 		 */
 		for (i = 0; i < idaw_nr; i++)
 			pa->pa_iova[i] = idaws[i];
+		pa->pa_nr = idaw_nr;
 	} else {
 		/*
 		 * No action is required here; the iova addresses in page_array
@@ -577,7 +577,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 	if (ccw_does_data_transfer(ccw)) {
 		ret = page_array_pin(pa, vdev);
 		if (ret < 0)
-			goto out_unpin;
+			goto out_free_pa;
 	} else {
 		pa->pa_nr = 0;
 	}
@@ -590,8 +590,8 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,
 
 	return 0;
 
-out_unpin:
-	page_array_unpin_free(pa, vdev);
+out_free_pa:
+	page_array_free(pa);
 out_free_idaws:
 	kfree(idaws);
 out_init:
@@ -697,7 +697,8 @@ void cp_free(struct channel_program *cp)
 	cp->initialized = false;
 	list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) {
 		for (i = 0; i < chain->ch_len; i++) {
-			page_array_unpin_free(chain->ch_pa + i, vdev);
+			page_array_unpin(pa, vdev, pa->pa_nr);
+			page_array_free(chain->ch_pa + i);
 			ccwchain_cda_free(chain, i);
 		}
 		ccwchain_free(chain);
-- 
2.38.1

Re: S390 testing for IOMMUFD

[Eric Farman]

On Tue, 2022-11-08 at 15:34 -0400, Jason Gunthorpe wrote:
> On Tue, Nov 08, 2022 at 10:29:33AM -0500, 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));
> 
> This seems like a different bug, probably a ccw driver bug. The
> WARN_ON is designed to detect cases where the driver is unpinning an
> IOVA range that is not exactly what it pinned. The pin side already
> does this validation, so if it fails it means pin/unpin did not have
> identical iova ranges. Some debugging prints should confirm this.
> 
> I looked at CCW and came up with the following two things, can you
> look at them and finish them off? It will probably help.

I happen to already have patch 1 in a series I've been working on in
parallel with the private/parent split. I haven't forgotten it. :)

Patch 2 doesn't address the above symptoms, but a lot of that code is
getting reworked by the aforementioned series so I didn't spend a lot
of time studying your suggestion. And as I type this I see you just
sent a new patch, let me go try that...

Re: S390 testing for IOMMUFD

[Jason Gunthorpe]

On Tue, Nov 08, 2022 at 03:07:19PM -0500, Eric Farman wrote:
> Patch 2 doesn't address the above symptoms, but a lot of that code is
> getting reworked by the aforementioned series so I didn't spend a lot
> of time studying your suggestion. And as I type this I see you just
> sent a new patch, let me go try that...

Patch2 is following the assumption the WARN_ON is triggered by a
failure path that isn't work right. Eg trying to unpin a 0 IOVA
because the failure flow is wrong. Removing all the calls to unpin on
failure paths make that impossible.

Jason