Re: [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT

[Patrick Robb <probb@iol.unh.edu>]

On Mon, Sep 16, 2024 at 5:30 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, Sep 16, 2024 at 6:15 AM Patrick Robb <probb@iol.unh.edu> wrote:
> >
> >
> > diff --git a/tests/cryptodev_common.py b/tests/cryptodev_common.py
> > index b550b46869df..37483c51e3e7 100644
> > --- a/tests/cryptodev_common.py
> > +++ b/tests/cryptodev_common.py
> > @@ -15,6 +15,10 @@ def bind_qat_device(test_case, driver="igb_uio"):
> >
> >      if "crypto_dev_id" in conf.suite_cfg:
> >          dev_id = conf.suite_cfg["crypto_dev_id"]
> > +        if dev_id in ["37c8", "435", "19e2"]:
>
> Usually, PCI ids are represented on 4 chars, leading 0 included, so I
> would expect 0435.
> Do you have such hw and did you test with it?

I agree that 3 char id is strange.

This was some months ago, but I believe I grabbed the dev ids from
https://doc.dpdk.org/guides/cryptodevs/qat.html#available-kernel-drivers
based on the kernel commit you posted above.

This website, which I usually use when adding a new PCI device to
legacy dts, https://devicehunt.com/view/type/pci/vendor/8086/device/0435
suggests that the correct id is 0435. If this sounds right to you, I
can submit a new version of this series with the 0 added, and a
dpdk/doc patch for the page I included above (presuming that the id in
the table is actually wrong).

In terms of testing, yes we do have a c62x / 37c8 card (qat 8970), and
this commit did "fix" the testsuite for that card. I included the
other two cards because even though I didn't have the hardware to test
the change, I felt that my reading of the linux kernel commit above
indicated it was appropriate to sumit this change, even without having
run it on the specific hardware. Please let me know if this is
inappropriate.

>
>
> > +            test_case.dut.send_expect('modprobe -r vfio_iommu_type1; modprobe -r vfio_pci; modprobe -r vfio_virqfd; modprobe -r vfio', '# ', 5)
> > +            test_case.dut.send_expect('modprobe vfio-pci disable_denylist=1 enable_sriov=1', '# ', 5)
>
> While I do understand the disable_denylist option, the enable_sriov=
> part seems a different topic...
> Why is sriov needed in this test?
>

I believe the enable_sriov requirement comes from the fact that this
testsuite is running from virtual functons on the QAT card. You can
find an example similar to how we set those up here:
https://doc.dpdk.org/guides/cryptodevs/qat.html#binding-the-available-vfs-to-the-vfio-pci-driver

However, these specific options for the QAT cards in question came
from Dharmik Thakkar (ARM) so I'm CCing him here as I think he is more
of an authority on the subject than I am.
https://inbox.dpdk.org/ci/AS4PR08MB755371CDAF5C105050D64850F7CDA@AS4PR08MB7553.eurprd08.prod.outlook.com/

>
> > +            test_case.dut.send_expect('echo "1" | tee /sys/module/vfio/parameters/enable_unsafe_noiommu_mode', '# ', 5)
>
> This helps in systems that do not have a IOMMU (or an emulated one for
> virtual machines).
> I suspect forcing noiommu will break setups with a hw iommu as DPDK
> will force PA when noiommu is detected.

This is an ARM Neoverse N-1 Ampere Mt. Jade system, which appears to have IOMMU:

```
probb@arm-ampere-dut:~$ sudo dmesg | grep -i IOMMU
[31804.426192] c6xxvf 0000:03:01.0: Adding to iommu group 59
[31804.473157] c6xxvf 0000:03:01.1: Adding to iommu group 60
[31804.533436] c6xxvf 0000:03:01.2: Adding to iommu group 61
[31804.577145] c6xxvf 0000:03:01.3: Adding to iommu group 62
[31804.621197] c6xxvf 0000:03:01.4: Adding to iommu group 63
[31804.665133] c6xxvf 0000:03:01.5: Adding to iommu group 64
```

But, I guess this is another question for Dharmik I think, who
suggested the enable_unsafe_no_iommu_mode setting and again is more
knowledgeable on the subject than I am. However, I am happy to test
this with enable_unsafe_noiommu_mode=0. I'll give it a whirl now and
report back.