* S390 testing for IOMMUFD
[not found] <0-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com>
@ 2022-11-08 1:09 ` Jason Gunthorpe
2022-11-08 10:12 ` Christian Borntraeger
2022-11-08 13:50 ` Matthew Rosato
0 siblings, 2 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 1:09 UTC (permalink / raw)
To: Cornelia Huck, Eric Farman, Matthew Rosato, Niklas Schnelle,
Tony Krowiak, Halil Pasic, Jason Herne, linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 1:09 ` S390 testing for IOMMUFD Jason Gunthorpe
@ 2022-11-08 10:12 ` Christian Borntraeger
2022-11-08 14:04 ` Anthony Krowiak
2022-11-08 13:50 ` Matthew Rosato
1 sibling, 1 reply; 19+ messages in thread
From: Christian Borntraeger @ 2022-11-08 10:12 UTC (permalink / raw)
To: Jason Gunthorpe, Cornelia Huck, Eric Farman, Matthew Rosato,
Niklas Schnelle, Tony Krowiak, Halil Pasic, Jason Herne,
linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 1:09 ` S390 testing for IOMMUFD Jason Gunthorpe
2022-11-08 10:12 ` Christian Borntraeger
@ 2022-11-08 13:50 ` Matthew Rosato
2022-11-08 13:54 ` Jason Gunthorpe
1 sibling, 1 reply; 19+ messages in thread
From: Matthew Rosato @ 2022-11-08 13:50 UTC (permalink / raw)
To: Jason Gunthorpe, Cornelia Huck, Eric Farman, Niklas Schnelle,
Tony Krowiak, Halil Pasic, Jason Herne, linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 13:50 ` Matthew Rosato
@ 2022-11-08 13:54 ` Jason Gunthorpe
2022-11-08 14:19 ` Eric Farman
0 siblings, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 13:54 UTC (permalink / raw)
To: Matthew Rosato
Cc: Cornelia Huck, Eric Farman, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 10:12 ` Christian Borntraeger
@ 2022-11-08 14:04 ` Anthony Krowiak
2022-11-09 14:49 ` Anthony Krowiak
0 siblings, 1 reply; 19+ messages in thread
From: Anthony Krowiak @ 2022-11-08 14:04 UTC (permalink / raw)
To: Christian Borntraeger, Jason Gunthorpe, Cornelia Huck,
Eric Farman, Matthew Rosato, Niklas Schnelle, Halil Pasic,
Jason Herne, linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 13:54 ` Jason Gunthorpe
@ 2022-11-08 14:19 ` Eric Farman
2022-11-08 14:37 ` Jason Gunthorpe
0 siblings, 1 reply; 19+ messages in thread
From: Eric Farman @ 2022-11-08 14:19 UTC (permalink / raw)
To: Jason Gunthorpe, Matthew Rosato
Cc: Cornelia Huck, Niklas Schnelle, Tony Krowiak, Halil Pasic,
Jason Herne, linux-s390, iommu, Kevin Tian, Alex Williamson, kvm,
Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 14:19 ` Eric Farman
@ 2022-11-08 14:37 ` Jason Gunthorpe
2022-11-08 15:29 ` Eric Farman
0 siblings, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 14:37 UTC (permalink / raw)
To: Eric Farman
Cc: Matthew Rosato, Cornelia Huck, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 14:37 ` Jason Gunthorpe
@ 2022-11-08 15:29 ` Eric Farman
2022-11-08 19:18 ` Matthew Rosato
2022-11-08 19:34 ` Jason Gunthorpe
0 siblings, 2 replies; 19+ messages in thread
From: Eric Farman @ 2022-11-08 15:29 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Matthew Rosato, Cornelia Huck, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 15:29 ` Eric Farman
@ 2022-11-08 19:18 ` Matthew Rosato
2022-11-08 20:04 ` Jason Gunthorpe
2022-11-08 19:34 ` Jason Gunthorpe
1 sibling, 1 reply; 19+ messages in thread
From: Matthew Rosato @ 2022-11-08 19:18 UTC (permalink / raw)
To: Eric Farman, Jason Gunthorpe, Tony Krowiak, Jason Herne,
Christian Borntraeger
Cc: Cornelia Huck, Niklas Schnelle, Halil Pasic, linux-s390, iommu,
Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 15:29 ` Eric Farman
2022-11-08 19:18 ` Matthew Rosato
@ 2022-11-08 19:34 ` Jason Gunthorpe
2022-11-08 20:07 ` Eric Farman
1 sibling, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 19:34 UTC (permalink / raw)
To: Eric Farman
Cc: Matthew Rosato, Cornelia Huck, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
[-- 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 19:18 ` Matthew Rosato
@ 2022-11-08 20:04 ` Jason Gunthorpe
2022-11-08 20:17 ` Eric Farman
0 siblings, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 20:04 UTC (permalink / raw)
To: Matthew Rosato
Cc: Eric Farman, Tony Krowiak, Jason Herne, Christian Borntraeger,
Cornelia Huck, Niklas Schnelle, Halil Pasic, linux-s390, iommu,
Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 19:34 ` Jason Gunthorpe
@ 2022-11-08 20:07 ` Eric Farman
2022-11-08 20:10 ` Jason Gunthorpe
0 siblings, 1 reply; 19+ messages in thread
From: Eric Farman @ 2022-11-08 20:07 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Matthew Rosato, Cornelia Huck, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 20:07 ` Eric Farman
@ 2022-11-08 20:10 ` Jason Gunthorpe
0 siblings, 0 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-08 20:10 UTC (permalink / raw)
To: Eric Farman
Cc: Matthew Rosato, Cornelia Huck, Niklas Schnelle, Tony Krowiak,
Halil Pasic, Jason Herne, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 20:04 ` Jason Gunthorpe
@ 2022-11-08 20:17 ` Eric Farman
0 siblings, 0 replies; 19+ messages in thread
From: Eric Farman @ 2022-11-08 20:17 UTC (permalink / raw)
To: Jason Gunthorpe, Matthew Rosato
Cc: Tony Krowiak, Jason Herne, Christian Borntraeger, Cornelia Huck,
Niklas Schnelle, Halil Pasic, linux-s390, iommu, Kevin Tian,
Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-08 14:04 ` Anthony Krowiak
@ 2022-11-09 14:49 ` Anthony Krowiak
2022-11-09 16:12 ` Jason Gunthorpe
2022-11-09 19:09 ` Anthony Krowiak
0 siblings, 2 replies; 19+ messages in thread
From: Anthony Krowiak @ 2022-11-09 14:49 UTC (permalink / raw)
To: Christian Borntraeger, Jason Gunthorpe, Cornelia Huck,
Eric Farman, Matthew Rosato, Niklas Schnelle, Halil Pasic,
Jason Herne, linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-09 14:49 ` Anthony Krowiak
@ 2022-11-09 16:12 ` Jason Gunthorpe
2022-11-09 19:13 ` Anthony Krowiak
2022-11-09 19:09 ` Anthony Krowiak
1 sibling, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-09 16:12 UTC (permalink / raw)
To: Anthony Krowiak
Cc: Christian Borntraeger, Cornelia Huck, Eric Farman, Matthew Rosato,
Niklas Schnelle, Halil Pasic, Jason Herne, linux-s390, iommu,
Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-09 14:49 ` Anthony Krowiak
2022-11-09 16:12 ` Jason Gunthorpe
@ 2022-11-09 19:09 ` Anthony Krowiak
1 sibling, 0 replies; 19+ messages in thread
From: Anthony Krowiak @ 2022-11-09 19:09 UTC (permalink / raw)
To: Christian Borntraeger, Jason Gunthorpe, Cornelia Huck,
Eric Farman, Matthew Rosato, Niklas Schnelle, Halil Pasic,
Jason Herne, linux-s390
Cc: iommu, Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-09 16:12 ` Jason Gunthorpe
@ 2022-11-09 19:13 ` Anthony Krowiak
2022-11-09 20:43 ` Jason Gunthorpe
0 siblings, 1 reply; 19+ messages in thread
From: Anthony Krowiak @ 2022-11-09 19:13 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christian Borntraeger, Cornelia Huck, Eric Farman, Matthew Rosato,
Niklas Schnelle, Halil Pasic, Jason Herne, linux-s390, iommu,
Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: S390 testing for IOMMUFD
2022-11-09 19:13 ` Anthony Krowiak
@ 2022-11-09 20:43 ` Jason Gunthorpe
0 siblings, 0 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2022-11-09 20:43 UTC (permalink / raw)
To: Anthony Krowiak
Cc: Christian Borntraeger, Cornelia Huck, Eric Farman, Matthew Rosato,
Niklas Schnelle, Halil Pasic, Jason Herne, linux-s390, iommu,
Kevin Tian, Alex Williamson, kvm, Lu Baolu, Nicolin Chen
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
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-11-09 20:43 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com>
2022-11-08 1:09 ` S390 testing for IOMMUFD Jason Gunthorpe
2022-11-08 10:12 ` Christian Borntraeger
2022-11-08 14:04 ` Anthony Krowiak
2022-11-09 14:49 ` Anthony Krowiak
2022-11-09 16:12 ` Jason Gunthorpe
2022-11-09 19:13 ` Anthony Krowiak
2022-11-09 20:43 ` Jason Gunthorpe
2022-11-09 19:09 ` Anthony Krowiak
2022-11-08 13:50 ` Matthew Rosato
2022-11-08 13:54 ` Jason Gunthorpe
2022-11-08 14:19 ` Eric Farman
2022-11-08 14:37 ` Jason Gunthorpe
2022-11-08 15:29 ` Eric Farman
2022-11-08 19:18 ` Matthew Rosato
2022-11-08 20:04 ` Jason Gunthorpe
2022-11-08 20:17 ` Eric Farman
2022-11-08 19:34 ` Jason Gunthorpe
2022-11-08 20:07 ` Eric Farman
2022-11-08 20:10 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).