test suite reviews and discussions
 help / color / mirror / Atom feed
* [PATCH 0/1] Adding vfio load params for certain QAT cards
@ 2024-09-16  4:14 Patrick Robb
  2024-09-16  4:14 ` [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT Patrick Robb
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick Robb @ 2024-09-16  4:14 UTC (permalink / raw)
  To: david.marchand; +Cc: dts, Patrick Robb

In adding a QAT 8970 card to testing at the UNH Community Lab, we
learned that certain QAT cards are not meant to run in an untrusted
environment, and require a particular linux configuration. Namely, that
disable denylist be enabled for vfio, and that unsame iommu mode be
enabled.

You can read more at this commit: 
https://github.com/torvalds/linux/commit/50173329c8cc0c892eaa7a9d0f0692ac39cd7b04

Patrick Robb (1):
  tests/cryptodev_common.py Supporting vfio denylist for QAT

 tests/cryptodev_common.py | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT
  2024-09-16  4:14 [PATCH 0/1] Adding vfio load params for certain QAT cards Patrick Robb
@ 2024-09-16  4:14 ` Patrick Robb
  2024-09-16  9:30   ` David Marchand
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick Robb @ 2024-09-16  4:14 UTC (permalink / raw)
  To: david.marchand; +Cc: dts, Patrick Robb

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

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT
  2024-09-16  4:14 ` [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT Patrick Robb
@ 2024-09-16  9:30   ` David Marchand
  2024-09-17  4:21     ` Patrick Robb
  0 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2024-09-16  9:30 UTC (permalink / raw)
  To: Patrick Robb; +Cc: dts

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
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT
  2024-09-16  9:30   ` David Marchand
@ 2024-09-17  4:21     ` Patrick Robb
  2024-09-18  9:57       ` Juraj Linkeš
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick Robb @ 2024-09-17  4:21 UTC (permalink / raw)
  To: David Marchand; +Cc: dts, Dharmik Jayesh Thakkar

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT
  2024-09-17  4:21     ` Patrick Robb
@ 2024-09-18  9:57       ` Juraj Linkeš
  0 siblings, 0 replies; 5+ messages in thread
From: Juraj Linkeš @ 2024-09-18  9:57 UTC (permalink / raw)
  To: Patrick Robb, David Marchand; +Cc: dts, Dharmik Jayesh Thakkar



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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-09-18  9:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-16  4:14 [PATCH 0/1] Adding vfio load params for certain QAT cards Patrick Robb
2024-09-16  4:14 ` [PATCH 1/1] tests/cryptodev_common.py Supporting vfio denylist for QAT Patrick Robb
2024-09-16  9:30   ` David Marchand
2024-09-17  4:21     ` Patrick Robb
2024-09-18  9:57       ` Juraj Linkeš

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