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

Message ID 20240916041409.181259-2-probb@iol.unh.edu (mailing list archive)
State New
Headers
Series Adding vfio load params for certain QAT cards |

Commit Message

Patrick Robb Sept. 16, 2024, 4:14 a.m. UTC
DH895XCC, C3XXX, and C62X QuickAssist cards are not designed to run
in an untrusted environment. Consequently, this patch adds commands
to the cryptodev_perf testsuite for loading the vfio driver
with disable_denylist enabled and enabling unsame iommu mode.

Signed-off-by: Patrick Robb <probb@iol.unh.edu>
---
 tests/cryptodev_common.py | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

David Marchand Sept. 16, 2024, 9:30 a.m. UTC | #1
On Mon, Sep 16, 2024 at 6:15 AM Patrick Robb <probb@iol.unh.edu> wrote:
>
> DH895XCC, C3XXX, and C62X QuickAssist cards are not designed to run
> in an untrusted environment. Consequently, this patch adds commands
> to the cryptodev_perf testsuite for loading the vfio driver
> with disable_denylist enabled and enabling unsame iommu mode.

For interested parties, here is the kernel commit:
https://git.kernel.org/linus/50173329c8cc


I am not entirely confident with this patch, for the reasons below.

>
> Signed-off-by: Patrick Robb <probb@iol.unh.edu>
> ---
>  tests/cryptodev_common.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> 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?


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


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


>          test_case.logger.info(
>              "specified the qat hardware device id in cfg: {}".format(dev_id)
>          )
> --
> 2.25.1
>
  
Patrick Robb Sept. 17, 2024, 4:21 a.m. UTC | #2
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.
  
Juraj Linkeš Sept. 18, 2024, 9:57 a.m. UTC | #3
On 17. 9. 2024 6:21, Patrick Robb wrote:
> 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.
> 

The best is to check the device itself. These ID's can be found under:
/sys/bus/pci/devices/<pci>/

The four IDs are:
device
vendor
subsystem_device
subsystem_vendor

and I believe you're looking for device, but it seems you don't actually 
have the hardware. In that case, you can also consult 
https://pci-ids.ucw.cz/v2.2/pci.ids, which says:
     0435  DH895XCC Series QAT

And that it is from:
8086  Intel Corporation

So that seems to match the device. You should definitely add the 0, I'd 
say DTS is looking in /sys/bus/pci/devices/<pci>/device and it's going 
to be there.
  

Patch

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"]:
+            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)
+            test_case.dut.send_expect('echo "1" | tee /sys/module/vfio/parameters/enable_unsafe_noiommu_mode', '# ', 5)
         test_case.logger.info(
             "specified the qat hardware device id in cfg: {}".format(dev_id)
         )