Re: [PATCH v2 2/3] doc: restructure QAT PMD guide

["Mcnamara, John" <john.mcnamara@intel.com>]

Hi Fiona,

Thanks for the improvements to the doc. Some minor comments below.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Thursday, March 9, 2017 7:21 PM
> To: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Cc: Jain, Deepak K <deepak.k.jain@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Griffin, John <john.griffin@intel.com>
> Subject: [dpdk-dev] [PATCH v2 2/3] doc: restructure QAT PMD guide
> 
> Restructure QAT PMD instructions and add a device table to minimise
> duplication for each device and make it easier to add devices.
> Fix some device name typos.
> 

> Technology C3xxx** -device, kernel 4.5 or greater is needed.
> -See instructions for `Installation using kernel.org driver`_ below.
> +.. csv-table::
> +   :header: "device", "driver", "kernel module", "pci driver", "PF DID",
> "num PFs", "VF DID", "num VFs per PF"
> +   :stub-columns: 1
> 
> +   DH895xCC,01.org,icp_qa_al, n/a,435,1,443,32
> +   DH895xCC,4.4+,qat_dh895xcc,dh895xcc,435,1,443,32
> +   C62x,4.5+,qat_c62x,c6xx,37c8,3,37c9,16
> +   C3xxx,4.5+,qat_c3xxx,c3xxx,19e2,1,19e3,16

This table format is hard to read, not recommended in the Documentation
Guidelines and we are trying to remove it in other sections so use the
standard table format instead:


.. _table_qat_pmds_drivers:

.. table:: QAT devices and drivers

   +----------+--------+---------------+------------+--------+---------+--------+------------+
   | Device   | Driver | Kernel Module | Pci Driver | PF Did | Num PFs | Vf Did | VFs per PF |
   +==========+========+===============+============+========+=========+========+============+
   | DH895xCC | 01.org | icp_qa_al     | n/a        | 435    | 1       | 443    | 32         |
   +----------+--------+---------------+------------+--------+---------+--------+------------+
   | DH895xCC | 4.4+   | qat_dh895xcc  | dh895xcc   | 435    | 1       | 443    | 32         |
   +----------+--------+---------------+------------+--------+---------+--------+------------+
   | C62x     | 4.5+   | qat_c62x      | c6xx       | 37c8   | 3       | 37c9   | 16         |
   +----------+--------+---------------+------------+--------+---------+--------+------------+
   | C3xxx    | 4.5+   | qat_c3xxx     | c3xxx      | 19e2   | 1       | 19e3   | 16         |
   +----------+--------+---------------+------------+--------+---------+--------+------------+


> 
> -Installation using 01.org QAT driver
> +
> +The ``driver`` column indicates either the linux kernel version in

Use ``Driver`` here to match new table heading.

Also s/linux/Linux/g.


> 
> -NOTE: There is no driver available for **Intel QuickAssist Technology
> C62x** and -**Intel QuickAssist Technology C3xxx** devices on 01.org.
> +The examples below are based on the C62x device, if you have a
> +different device use the corresponding values in the above table.
> +
> +In BIOS ensure that SRIOV is enabled and either
> +a) disable VT-d or
> +b) enable VT-d and set ``"intel_iommu=on iommu=pt"`` in the grub file.

This doesn't render as a list. Probably a bullet list would be better here:

    In BIOS ensure that SRIOV is enabled and either:

    * Disable VT-d or
    * Enable VT-d and set ``"intel_iommu=on iommu=pt"`` in the grub file.



> +     echo 16 > /sys/bus/pci/drivers/c6xx/0000\:1a\:00.0/sriov_numvfs
> +     echo 16 > /sys/bus/pci/drivers/c6xx/0000\:3d\:00.0/sriov_numvfs
> +     echo 16 > /sys/bus/pci/drivers/c6xx/0000\:3f\:00.0/sriov_numvfs

Backslashes are not required here.

> +
> +**Notes**:
> +
> +If the QAT kernel modules are not loaded and you see an error like
> +``Failed to load MMP firmware qat_895xcc_mmp.bin`` in kernel logs, this
> +may be as a result of not using a distribution, but just updating the
> kernel directly.

You could use the RST Note directive here:

.. Note::

   If the QAT kernel modules are not loaded and you see an error like ``Failed
   to load MMP firmware qat_895xcc_mmp.bin`` in kernel logs, this may be as a
   result of not using a distribution, but just updating the kernel directly.


> +
> +Download firmware from the kernel firmware repo at:
> +http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git
> +/tree/


Would be better as an embedded link:

    Download firmware from the `kernel firmware repo
    <http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/>`_.


> +
> +Copy qat binaries to /lib/firmware:
> +
> +*    ``cp qat_895xcc.bin /lib/firmware``
> +*    ``cp qat_895xcc_mmp.bin /lib/firmware``

Here and elsewhere, instead of using a bullet list of commands it would be
clearer and simpler to use a verbatim section:


    Copy qat binaries to ``/lib/firmware``::

       cp qat_895xcc.bin /lib/firmware
       cp qat_895xcc_mmp.bin /lib/firmware


> +
> +cd to your linux source root directory and start the qat kernel modules:
> +
> +*    ``insmod ./drivers/crypto/qat/qat_common/intel_qat.ko``
> +*    ``insmod ./drivers/crypto/qat/qat_dh895xcc/qat_dh895xcc.ko``
> +
> +
> +If you see the following warning in /var/log/messages it can be ignored:
> +``IOMMU should be enabled for SR-IOV to work correctly``

Backquote ``/var/log/messages``.


> 
> -**Note**: If using a later kernel and the build fails with an error
> relating to ``strict_stroul`` not being available apply the following
> patch:
> +**Notes**:
> +
> +If using a later kernel and the build fails with an error relating to
> ``strict_stroul`` not being available apply the following patch:

Also make this an RST note:


.. Note::

   If using a later kernel and the build fails with an error relating to
   ``strict_stroul`` not being available apply the following patch:





>  Binding the available VFs to the DPDK UIO driver
>  ------------------------------------------------
> 
> -For **Intel(R) QuickAssist Technology DH895xcc** device:
> -The unbind command below assumes ``bdfs`` of ``03:01.00-03:04.07``, if
> yours are different adjust the unbind command below::
> 
> -   cd $RTE_SDK
> -   modprobe uio
> -   insmod ./build/kmod/igb_uio.ko
> +Unbind the VFs from the stock driver so they can be bound to the uio
> driver.
> 
> -   for device in $(seq 1 4); do \
> -       for fn in $(seq 0 7); do \
> -           echo -n 0000:03:0${device}.${fn} > \
> -           /sys/bus/pci/devices/0000\:03\:0${device}.${fn}/driver/unbind;
> \
> -       done; \
> -   done
> +For **Intel(R) QuickAssist Technology DH895xCC** device:


It would be better to make this and follow sub-sections into real sub-sections
at a lower level:


For an Intel(R) QuickAssist Technology DH895xCC device
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


For an Intel(R) QuickAssist Technology C62x device
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


For Intel(R) QuickAssist Technology C3xxx device
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Bind to the DPDK uio driver
~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
John