[PATCH] crypto: iaa: Fix some errors in IAA documentation

[PATCH] crypto: iaa: Fix some errors in IAA documentation

[Jerry Snitselaar]

This cleans up the following issues I ran into when trying to use the
scripts and commands in the iaa-crypto.rst document.

- Fix incorrect arguments being passed to accel-config
  config-wq.
    - Replace --device_name with --driver-name.
    - Replace --driver_name with --driver-name.
    - Replace --size with --wq-size.
    - Add missing --priority argument.
- Add missing accel-config config-engine command after the
  config-wq commands.
- Fix wq name passed to accel-config config-wq.
- Add rmmod/modprobe of iaa_crypto to script that disables,
  then enables all devices and workqueues to avoid enable-wq
  failing with -EEXIST when trying to register to compression
  algorithm.
- Fix device name in cases where iaa was used instead of iax.

Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-crypto@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
index de587cf9cbed..330d35df5f16 100644
--- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
+++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
@@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
 
   # configure wq1.0
 
-  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
+  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel --priority=10 --name="iaa_crypto" --driver-name="crypto" iax1/wq1.0
+
+  accel-config config-engine iax1/engine1.0 --group-id=0
 
   # enable IAA device iax1
 
@@ -536,12 +538,20 @@ The below script automatically does that::
 
   echo "End Disable IAA"
 
+  echo "Reload iaa_crypto module"
+
+  rmmod iaa_crypto
+  modprobe iaa_crypto
+
+  echo "End Reload iaa_crypto module"
+
   #
   # configure iaa wqs and devices
   #
   echo "Configure IAA"
   for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
-      accel-config config-wq --group-id=0 --mode=dedicated --size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver_name="crypto" iax${i}/wq${i}
+      accel-config config-wq --group-id=0 --mode=dedicated --wq-size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-name="crypto" iax${i}/wq${i}.0
+      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
   done
 
   echo "End Configure IAA"
@@ -552,10 +562,10 @@ The below script automatically does that::
   echo "Enable IAA"
 
   for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
-      echo enable iaa iaa${i}
-      accel-config enable-device iaa${i}
-      echo enable wq iaa${i}/wq${i}.0
-      accel-config enable-wq iaa${i}/wq${i}.0
+      echo enable iaa iax${i}
+      accel-config enable-device iax${i}
+      echo enable wq iax${i}/wq${i}.0
+      accel-config enable-wq iax${i}/wq${i}.0
   done
 
   echo "End Enable IAA"
-- 
2.41.0

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Jerry Snitselaar]

On Sun, Mar 17, 2024 at 11:44:21PM -0700, Jerry Snitselaar wrote:
> This cleans up the following issues I ran into when trying to use the
> scripts and commands in the iaa-crypto.rst document.
> 
> - Fix incorrect arguments being passed to accel-config
>   config-wq.
>     - Replace --device_name with --driver-name.
>     - Replace --driver_name with --driver-name.
>     - Replace --size with --wq-size.
>     - Add missing --priority argument.
> - Add missing accel-config config-engine command after the
>   config-wq commands.
> - Fix wq name passed to accel-config config-wq.
> - Add rmmod/modprobe of iaa_crypto to script that disables,
>   then enables all devices and workqueues to avoid enable-wq
>   failing with -EEXIST when trying to register to compression
>   algorithm.
> - Fix device name in cases where iaa was used instead of iax.
> 
> Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
>  .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++-----
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> index de587cf9cbed..330d35df5f16 100644
> --- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> +++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> @@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
>  
>    # configure wq1.0
>  
> -  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
> +  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel --priority=10 --name="iaa_crypto" --driver-name="crypto" iax1/wq1.0
> +
> +  accel-config config-engine iax1/engine1.0 --group-id=0
>  
>    # enable IAA device iax1
>  
> @@ -536,12 +538,20 @@ The below script automatically does that::
>  
>    echo "End Disable IAA"
>  
> +  echo "Reload iaa_crypto module"
> +
> +  rmmod iaa_crypto
> +  modprobe iaa_crypto
> +
> +  echo "End Reload iaa_crypto module"
> +
>    #
>    # configure iaa wqs and devices
>    #
>    echo "Configure IAA"
>    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> -      accel-config config-wq --group-id=0 --mode=dedicated --size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver_name="crypto" iax${i}/wq${i}
> +      accel-config config-wq --group-id=0 --mode=dedicated --wq-size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-name="crypto" iax${i}/wq${i}.0
> +      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
>    done
>  
>    echo "End Configure IAA"
> @@ -552,10 +562,10 @@ The below script automatically does that::
>    echo "Enable IAA"
>  
>    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> -      echo enable iaa iaa${i}
> -      accel-config enable-device iaa${i}
> -      echo enable wq iaa${i}/wq${i}.0
> -      accel-config enable-wq iaa${i}/wq${i}.0
> +      echo enable iaa iax${i}
> +      accel-config enable-device iax${i}
> +      echo enable wq iax${i}/wq${i}.0
> +      accel-config enable-wq iax${i}/wq${i}.0
>    done
>  
>    echo "End Enable IAA"
> -- 
> 2.41.0
> 

In addition to the above, the sections related to the modes seem
to be off to me.

Legacy mode in the Intel IOMMU context is when the IOMMU does not have
scalable mode enabled. If you pass intel_iommu=off the Intel IOMMU
will not be initialized, and I think that would correspond to the No IOMMU
mode instead of Legacy mode. The other suggestion for Legacy mode of
disabling VT-d in the BIOS would also be No IOMMU mode, but in
addition to the dma remapping units being disabled it would disable
interrupt remapping since the DMAR table would no longer be presented
to the OS by the BIOS.

I think the modes should be:

Scalable mode: intel_iommu=on,sm_on
Legacy mode: intel_iommu=on
No IOMMU mode: intel_iommu=off (or VT-d disabled in BIOS)


Since Intel IOMMU and scabale mode have config options that allow them
to be enabled by default, there are different parameter variations
that would match the above cases. I don't know if they need to
be detailed here, or if it would just make it more confusing.


Regards,
Jerry

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Tom Zanussi]

Hi Jerry,

On Mon, 2024-03-18 at 00:49 -0700, Jerry Snitselaar wrote:
> On Sun, Mar 17, 2024 at 11:44:21PM -0700, Jerry Snitselaar wrote:
> > This cleans up the following issues I ran into when trying to use
> > the
> > scripts and commands in the iaa-crypto.rst document.
> > 
> > - Fix incorrect arguments being passed to accel-config
> >   config-wq.
> >     - Replace --device_name with --driver-name.
> >     - Replace --driver_name with --driver-name.
> >     - Replace --size with --wq-size.
> >     - Add missing --priority argument.
> > - Add missing accel-config config-engine command after the
> >   config-wq commands.
> > - Fix wq name passed to accel-config config-wq.
> > - Add rmmod/modprobe of iaa_crypto to script that disables,
> >   then enables all devices and workqueues to avoid enable-wq
> >   failing with -EEXIST when trying to register to compression
> >   algorithm.
> > - Fix device name in cases where iaa was used instead of iax.
> > 
> > Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: linux-crypto@vger.kernel.org
> > Cc: linux-doc@vger.kernel.org
> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > ---
> >  .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++-
> > ----
> >  1 file changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > index de587cf9cbed..330d35df5f16 100644
> > --- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > +++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > @@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
> >  
> >    # configure wq1.0
> >  
> > -  accel-config config-wq --group-id=0 --mode=dedicated --
> > type=kernel --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
> > +  accel-config config-wq --group-id=0 --mode=dedicated --
> > type=kernel --priority=10 --name="iaa_crypto" --driver-
> > name="crypto" iax1/wq1.0
> > +
> > +  accel-config config-engine iax1/engine1.0 --group-id=0
> >  
> >    # enable IAA device iax1
> >  
> > @@ -536,12 +538,20 @@ The below script automatically does that::
> >  
> >    echo "End Disable IAA"
> >  
> > +  echo "Reload iaa_crypto module"
> > +
> > +  rmmod iaa_crypto
> > +  modprobe iaa_crypto
> > +
> > +  echo "End Reload iaa_crypto module"
> > +
> >    #
> >    # configure iaa wqs and devices
> >    #
> >    echo "Configure IAA"
> >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > -      accel-config config-wq --group-id=0 --mode=dedicated --
> > size=128 --priority=10 --type=kernel --name="iaa_crypto" --
> > driver_name="crypto" iax${i}/wq${i}
> > +      accel-config config-wq --group-id=0 --mode=dedicated --wq-
> > size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-
> > name="crypto" iax${i}/wq${i}.0
> > +      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
> >    done
> >  
> >    echo "End Configure IAA"
> > @@ -552,10 +562,10 @@ The below script automatically does that::
> >    echo "Enable IAA"
> >  
> >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > -      echo enable iaa iaa${i}
> > -      accel-config enable-device iaa${i}
> > -      echo enable wq iaa${i}/wq${i}.0
> > -      accel-config enable-wq iaa${i}/wq${i}.0
> > +      echo enable iaa iax${i}
> > +      accel-config enable-device iax${i}
> > +      echo enable wq iax${i}/wq${i}.0
> > +      accel-config enable-wq iax${i}/wq${i}.0
> >    done
> >  
> >    echo "End Enable IAA"
> > -- 
> > 2.41.0
> > 
> 
> In addition to the above, the sections related to the modes seem
> to be off to me.
> 
> Legacy mode in the Intel IOMMU context is when the IOMMU does not
> have
> scalable mode enabled. If you pass intel_iommu=off the Intel IOMMU
> will not be initialized, and I think that would correspond to the No
> IOMMU
> mode instead of Legacy mode. The other suggestion for Legacy mode of
> disabling VT-d in the BIOS would also be No IOMMU mode, but in
> addition to the dma remapping units being disabled it would disable
> interrupt remapping since the DMAR table would no longer be presented
> to the OS by the BIOS.
> 
> I think the modes should be:
> 
> Scalable mode: intel_iommu=on,sm_on
> Legacy mode: intel_iommu=on
> No IOMMU mode: intel_iommu=off (or VT-d disabled in BIOS)
> 

Yes, I think you're correct, those make more sense.

> Since Intel IOMMU and scabale mode have config options that allow
> them
> to be enabled by default, there are different parameter variations
> that would match the above cases. I don't know if they need to
> be detailed here, or if it would just make it more confusing.
> 

Personally, I think it would be useful to have them detailed and might
lessen confusion for people setting things up and/or debugging a setup.

Thanks,

Tom

> Regards,
> Jerry
> 

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Jerry Snitselaar]

On Mon, Mar 18, 2024 at 11:26:31AM -0500, Tom Zanussi wrote:
> Hi Jerry,
> 
> On Mon, 2024-03-18 at 00:49 -0700, Jerry Snitselaar wrote:
> > On Sun, Mar 17, 2024 at 11:44:21PM -0700, Jerry Snitselaar wrote:
> > > This cleans up the following issues I ran into when trying to use
> > > the
> > > scripts and commands in the iaa-crypto.rst document.
> > > 
> > > - Fix incorrect arguments being passed to accel-config
> > >   config-wq.
> > >     - Replace --device_name with --driver-name.
> > >     - Replace --driver_name with --driver-name.
> > >     - Replace --size with --wq-size.
> > >     - Add missing --priority argument.
> > > - Add missing accel-config config-engine command after the
> > >   config-wq commands.
> > > - Fix wq name passed to accel-config config-wq.
> > > - Add rmmod/modprobe of iaa_crypto to script that disables,
> > >   then enables all devices and workqueues to avoid enable-wq
> > >   failing with -EEXIST when trying to register to compression
> > >   algorithm.
> > > - Fix device name in cases where iaa was used instead of iax.
> > > 
> > > Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > Cc: linux-crypto@vger.kernel.org
> > > Cc: linux-doc@vger.kernel.org
> > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > ---
> > >  .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++-
> > > ----
> > >  1 file changed, 16 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > index de587cf9cbed..330d35df5f16 100644
> > > --- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > +++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > @@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
> > >  
> > >    # configure wq1.0
> > >  
> > > -  accel-config config-wq --group-id=0 --mode=dedicated --
> > > type=kernel --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
> > > +  accel-config config-wq --group-id=0 --mode=dedicated --
> > > type=kernel --priority=10 --name="iaa_crypto" --driver-
> > > name="crypto" iax1/wq1.0
> > > +
> > > +  accel-config config-engine iax1/engine1.0 --group-id=0
> > >  
> > >    # enable IAA device iax1
> > >  
> > > @@ -536,12 +538,20 @@ The below script automatically does that::
> > >  
> > >    echo "End Disable IAA"
> > >  
> > > +  echo "Reload iaa_crypto module"
> > > +
> > > +  rmmod iaa_crypto
> > > +  modprobe iaa_crypto
> > > +
> > > +  echo "End Reload iaa_crypto module"
> > > +
> > >    #
> > >    # configure iaa wqs and devices
> > >    #
> > >    echo "Configure IAA"
> > >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > > -      accel-config config-wq --group-id=0 --mode=dedicated --
> > > size=128 --priority=10 --type=kernel --name="iaa_crypto" --
> > > driver_name="crypto" iax${i}/wq${i}
> > > +      accel-config config-wq --group-id=0 --mode=dedicated --wq-
> > > size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-
> > > name="crypto" iax${i}/wq${i}.0
> > > +      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
> > >    done
> > >  
> > >    echo "End Configure IAA"
> > > @@ -552,10 +562,10 @@ The below script automatically does that::
> > >    echo "Enable IAA"
> > >  
> > >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > > -      echo enable iaa iaa${i}
> > > -      accel-config enable-device iaa${i}
> > > -      echo enable wq iaa${i}/wq${i}.0
> > > -      accel-config enable-wq iaa${i}/wq${i}.0
> > > +      echo enable iaa iax${i}
> > > +      accel-config enable-device iax${i}
> > > +      echo enable wq iax${i}/wq${i}.0
> > > +      accel-config enable-wq iax${i}/wq${i}.0
> > >    done
> > >  
> > >    echo "End Enable IAA"
> > > -- 
> > > 2.41.0
> > > 
> > 
> > In addition to the above, the sections related to the modes seem
> > to be off to me.
> > 
> > Legacy mode in the Intel IOMMU context is when the IOMMU does not
> > have
> > scalable mode enabled. If you pass intel_iommu=off the Intel IOMMU
> > will not be initialized, and I think that would correspond to the No
> > IOMMU
> > mode instead of Legacy mode. The other suggestion for Legacy mode of
> > disabling VT-d in the BIOS would also be No IOMMU mode, but in
> > addition to the dma remapping units being disabled it would disable
> > interrupt remapping since the DMAR table would no longer be presented
> > to the OS by the BIOS.
> > 
> > I think the modes should be:
> > 
> > Scalable mode: intel_iommu=on,sm_on
> > Legacy mode: intel_iommu=on
> > No IOMMU mode: intel_iommu=off (or VT-d disabled in BIOS)
> > 
> 
> Yes, I think you're correct, those make more sense.
> 
> > Since Intel IOMMU and scabale mode have config options that allow
> > them
> > to be enabled by default, there are different parameter variations
> > that would match the above cases. I don't know if they need to
> > be detailed here, or if it would just make it more confusing.
> > 
> 
> Personally, I think it would be useful to have them detailed and might
> lessen confusion for people setting things up and/or debugging a setup.
> 
> Thanks,
> 
> Tom

Hi Tom,

This is what I came up with last night for the kernel parameters when thinking about it:


| mode \ default enable | intel_iommu + /sm + | intel_iommu + / sm - | intel_iommu - / sm +  | intel_iommu - / sm - |
|-----------------------+---------------------+----------------------+-----------------------+----------------------|
| Scalable Mode         | nothing             | intel_iommu=sm_on    | intel_iommu=on        | intel_iommu=on,sm_on |
| Legacy Mode           | intel_iommu=sm_off  | nothing              | intel_iommu=on,sm_off | intel_iommu=on       |
| No IOMMU Mode         | intel_iommu=off     | intel_iommu=off      | nothing               | nothing              |


Regards,
Jerry



> 
> > Regards,
> > Jerry
> > 
> 

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Tom Zanussi]

On Mon, 2024-03-18 at 10:58 -0700, Jerry Snitselaar wrote:
> On Mon, Mar 18, 2024 at 11:26:31AM -0500, Tom Zanussi wrote:
> > Hi Jerry,
> > 
> > On Mon, 2024-03-18 at 00:49 -0700, Jerry Snitselaar wrote:
> > > On Sun, Mar 17, 2024 at 11:44:21PM -0700, Jerry Snitselaar wrote:
> > > > This cleans up the following issues I ran into when trying to use
> > > > the
> > > > scripts and commands in the iaa-crypto.rst document.
> > > > 
> > > > - Fix incorrect arguments being passed to accel-config
> > > >   config-wq.
> > > >     - Replace --device_name with --driver-name.
> > > >     - Replace --driver_name with --driver-name.
> > > >     - Replace --size with --wq-size.
> > > >     - Add missing --priority argument.
> > > > - Add missing accel-config config-engine command after the
> > > >   config-wq commands.
> > > > - Fix wq name passed to accel-config config-wq.
> > > > - Add rmmod/modprobe of iaa_crypto to script that disables,
> > > >   then enables all devices and workqueues to avoid enable-wq
> > > >   failing with -EEXIST when trying to register to compression
> > > >   algorithm.
> > > > - Fix device name in cases where iaa was used instead of iax.
> > > > 
> > > > Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> > > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > > Cc: linux-crypto@vger.kernel.org
> > > > Cc: linux-doc@vger.kernel.org
> > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > > ---
> > > >  .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++-
> > > > ----
> > > >  1 file changed, 16 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > > b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > > index de587cf9cbed..330d35df5f16 100644
> > > > --- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > > +++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> > > > @@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
> > > >  
> > > >    # configure wq1.0
> > > >  
> > > > -  accel-config config-wq --group-id=0 --mode=dedicated --
> > > > type=kernel --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
> > > > +  accel-config config-wq --group-id=0 --mode=dedicated --
> > > > type=kernel --priority=10 --name="iaa_crypto" --driver-
> > > > name="crypto" iax1/wq1.0
> > > > +
> > > > +  accel-config config-engine iax1/engine1.0 --group-id=0
> > > >  
> > > >    # enable IAA device iax1
> > > >  
> > > > @@ -536,12 +538,20 @@ The below script automatically does that::
> > > >  
> > > >    echo "End Disable IAA"
> > > >  
> > > > +  echo "Reload iaa_crypto module"
> > > > +
> > > > +  rmmod iaa_crypto
> > > > +  modprobe iaa_crypto
> > > > +
> > > > +  echo "End Reload iaa_crypto module"
> > > > +
> > > >    #
> > > >    # configure iaa wqs and devices
> > > >    #
> > > >    echo "Configure IAA"
> > > >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > > > -      accel-config config-wq --group-id=0 --mode=dedicated --
> > > > size=128 --priority=10 --type=kernel --name="iaa_crypto" --
> > > > driver_name="crypto" iax${i}/wq${i}
> > > > +      accel-config config-wq --group-id=0 --mode=dedicated --wq-
> > > > size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-
> > > > name="crypto" iax${i}/wq${i}.0
> > > > +      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
> > > >    done
> > > >  
> > > >    echo "End Configure IAA"
> > > > @@ -552,10 +562,10 @@ The below script automatically does that::
> > > >    echo "Enable IAA"
> > > >  
> > > >    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> > > > -      echo enable iaa iaa${i}
> > > > -      accel-config enable-device iaa${i}
> > > > -      echo enable wq iaa${i}/wq${i}.0
> > > > -      accel-config enable-wq iaa${i}/wq${i}.0
> > > > +      echo enable iaa iax${i}
> > > > +      accel-config enable-device iax${i}
> > > > +      echo enable wq iax${i}/wq${i}.0
> > > > +      accel-config enable-wq iax${i}/wq${i}.0
> > > >    done
> > > >  
> > > >    echo "End Enable IAA"
> > > > -- 
> > > > 2.41.0
> > > > 
> > > 
> > > In addition to the above, the sections related to the modes seem
> > > to be off to me.
> > > 
> > > Legacy mode in the Intel IOMMU context is when the IOMMU does not
> > > have
> > > scalable mode enabled. If you pass intel_iommu=off the Intel IOMMU
> > > will not be initialized, and I think that would correspond to the No
> > > IOMMU
> > > mode instead of Legacy mode. The other suggestion for Legacy mode of
> > > disabling VT-d in the BIOS would also be No IOMMU mode, but in
> > > addition to the dma remapping units being disabled it would disable
> > > interrupt remapping since the DMAR table would no longer be presented
> > > to the OS by the BIOS.
> > > 
> > > I think the modes should be:
> > > 
> > > Scalable mode: intel_iommu=on,sm_on
> > > Legacy mode: intel_iommu=on
> > > No IOMMU mode: intel_iommu=off (or VT-d disabled in BIOS)
> > > 
> > 
> > Yes, I think you're correct, those make more sense.
> > 
> > > Since Intel IOMMU and scabale mode have config options that allow
> > > them
> > > to be enabled by default, there are different parameter variations
> > > that would match the above cases. I don't know if they need to
> > > be detailed here, or if it would just make it more confusing.
> > > 
> > 
> > Personally, I think it would be useful to have them detailed and might
> > lessen confusion for people setting things up and/or debugging a setup.
> > 
> > Thanks,
> > 
> > Tom
> 
> Hi Tom,
> 
> This is what I came up with last night for the kernel parameters when thinking about it:
> 
> 
> > mode \ default enable | intel_iommu + /sm + | intel_iommu + / sm - | intel_iommu - / sm +  | intel_iommu - / sm - |
> > -----------------------+---------------------+----------------------+-----------------------+----------------------|
> > Scalable Mode         | nothing             | intel_iommu=sm_on    | intel_iommu=on        | intel_iommu=on,sm_on |
> > Legacy Mode           | intel_iommu=sm_off  | nothing              | intel_iommu=on,sm_off | intel_iommu=on       |
> > No IOMMU Mode         | intel_iommu=off     | intel_iommu=off      | nothing               | nothing              |
> 

Very nice. I think it would need a little explanation on how to read
the table and mention of how the defaults correspond to actual config
options e.g. sm+ = CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON=y,
etc...

Thanks,

Tom

> 
> Regards,
> Jerry
> 
> 
> 
> > 
> > > Regards,
> > > Jerry
> > > 
> > 
> 

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Jerry Snitselaar]

On Mon, Mar 18, 2024 at 02:01:18PM -0500, Tom Zanussi wrote:
...
> > 
> > This is what I came up with last night for the kernel parameters when thinking about it:
> > 
> > 
> > > mode \ default enable | intel_iommu + /sm + | intel_iommu + / sm - | intel_iommu - / sm +  | intel_iommu - / sm - |
> > > -----------------------+---------------------+----------------------+-----------------------+----------------------|
> > > Scalable Mode         | nothing             | intel_iommu=sm_on    | intel_iommu=on        | intel_iommu=on,sm_on |
> > > Legacy Mode           | intel_iommu=sm_off  | nothing              | intel_iommu=on,sm_off | intel_iommu=on       |
> > > No IOMMU Mode         | intel_iommu=off     | intel_iommu=off      | nothing               | nothing              |
> > 
> 
> Very nice. I think it would need a little explanation on how to read
> the table and mention of how the defaults correspond to actual config
> options e.g. sm+ = CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON=y,
> etc...
> 
> Thanks,
> 
> Tom
>

Yes, if something like this were to go into the doc it would need more
explanation. This was me just trying to map out the different
combinations last night in an org-mode table in emacs.

Re: [PATCH] crypto: iaa: Fix some errors in IAA documentation

[Tom Zanussi]

Hi Jerry,

On Sun, 2024-03-17 at 23:44 -0700, Jerry Snitselaar wrote:
> This cleans up the following issues I ran into when trying to use the
> scripts and commands in the iaa-crypto.rst document.
> 
> - Fix incorrect arguments being passed to accel-config
>   config-wq.
>     - Replace --device_name with --driver-name.
>     - Replace --driver_name with --driver-name.
>     - Replace --size with --wq-size.
>     - Add missing --priority argument.
> - Add missing accel-config config-engine command after the
>   config-wq commands.
> - Fix wq name passed to accel-config config-wq.
> - Add rmmod/modprobe of iaa_crypto to script that disables,
>   then enables all devices and workqueues to avoid enable-wq
>   failing with -EEXIST when trying to register to compression
>   algorithm.
> - Fix device name in cases where iaa was used instead of iax.
> 
 
Yeah, I have these fixes in my own test scripts as well - I thought I
had updated the docs with them but obviously not :-/

Thanks for noticing/testing and fixing them.

Reviewed-by: Tom Zanussi <tom.zanussi@linux.intel.com>

> Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
>  .../driver-api/crypto/iaa/iaa-crypto.rst      | 22 ++++++++++++++---
> --
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> index de587cf9cbed..330d35df5f16 100644
> --- a/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> +++ b/Documentation/driver-api/crypto/iaa/iaa-crypto.rst
> @@ -179,7 +179,9 @@ has the old 'iax' device naming in place) ::
>  
>    # configure wq1.0
>  
> -  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel
> --name="iaa_crypto" --device_name="crypto" iax1/wq1.0
> +  accel-config config-wq --group-id=0 --mode=dedicated --type=kernel
> --priority=10 --name="iaa_crypto" --driver-name="crypto" iax1/wq1.0
> +
> +  accel-config config-engine iax1/engine1.0 --group-id=0
>  
>    # enable IAA device iax1
>  
> @@ -536,12 +538,20 @@ The below script automatically does that::
>  
>    echo "End Disable IAA"
>  
> +  echo "Reload iaa_crypto module"
> +
> +  rmmod iaa_crypto
> +  modprobe iaa_crypto
> +
> +  echo "End Reload iaa_crypto module"
> +
>    #
>    # configure iaa wqs and devices
>    #
>    echo "Configure IAA"
>    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> -      accel-config config-wq --group-id=0 --mode=dedicated --
> size=128 --priority=10 --type=kernel --name="iaa_crypto" --
> driver_name="crypto" iax${i}/wq${i}
> +      accel-config config-wq --group-id=0 --mode=dedicated --wq-
> size=128 --priority=10 --type=kernel --name="iaa_crypto" --driver-
> name="crypto" iax${i}/wq${i}.0
> +      accel-config config-engine iax${i}/engine${i}.0 --group-id=0
>    done
>  
>    echo "End Configure IAA"
> @@ -552,10 +562,10 @@ The below script automatically does that::
>    echo "Enable IAA"
>  
>    for ((i = 1; i < ${num_iaa} * 2; i += 2)); do
> -      echo enable iaa iaa${i}
> -      accel-config enable-device iaa${i}
> -      echo enable wq iaa${i}/wq${i}.0
> -      accel-config enable-wq iaa${i}/wq${i}.0
> +      echo enable iaa iax${i}
> +      accel-config enable-device iax${i}
> +      echo enable wq iax${i}/wq${i}.0
> +      accel-config enable-wq iax${i}/wq${i}.0
>    done
>  
>    echo "End Enable IAA"