Re: [PATCH v3 15/15] s390/vfio-ap: Add live guest migration chapter to vfio-ap.rst

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Malformed reStructuredText syntax leading to potential build warnings (grid table and nested lists).
- [Low] Multiple typographical, grammatical errors, and inconsistent variable names in the newly added text.
- [Low] Logical contradiction in section heading.
--

commit 5afeaef82632a184153d66843b7e2a58e98d0cdd
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: Add live guest migration chapter to vfio-ap.rst
    
This commit adds a 'Live Guest Migration' chapter to the vfio-ap.rst
documentation. It describes the details for initiating live guest migration
for a guest to which AP adapters, domains and control domains have been
passed through.

> diff --git a/Documentation/arch/s390/vfio-ap.rst b/Documentation/arch/s390/vfio-ap.rst
> index ac0c07f76ddd7..fc0707c0dffd6 100644
> --- a/Documentation/arch/s390/vfio-ap.rst
> +++ b/Documentation/arch/s390/vfio-ap.rst

[ ... ]

> +To be considered compatible, the AP configuration for the destination host must
> +meet these requirements:
>  
> -      (QEMU) device-del id=<device-id>
> +* Each AP adapter, domain and control domain number assigned to the source
> +  guest's AP configuration must be in the destination host's AP
> +  configuration. This can be verified as follows:
>  
> -      For example, to hot unplug the vfio_ap mediated device that was specified
> -      on the qemu command line with 'id=hostdev0' when the guest was started:
> +  * On the source host, display the AP configuration of the source guest:
>  
> -         (QEMU) device-del id=hostdev0
> +    .. code-block:: bash
>  
> -After live migration of the KVM guest completes, an AP configuration can be
> -restored to the KVM guest by hot plugging a vfio_ap mediated device on the target
> -system into the guest in one of two ways:
> +       cat /sys/devices/vfio_ap/matrix/$UUID/guest_matrix
> +       cat /sys/devices/vfio_ap/matrix/$UUID/control_domains
>  
> -1. If the KVM guest was started with libvirt, you can hot plug a matrix mediated
> -   device into the guest via the following virsh commands:
> +    .. note::
> +       Replace ``$UUID`` with the actual UUID of your mediated device.
>  
> -   virsh attach-device <guestname> <path-to-device-xml>
> +  * Display the AP configuration of the destination host:
>  
> -      For example, to hot plug mdev 62177883-f1bb-47f0-914d-32a22e3a8804 into
> -      the guest named 'my-guest':
> +    .. code-block:: bash
>  
> -         virsh attach-device my-guest ~/config/my-guest-hostdev.xml
> +       lszcrypt -V
> +       cat /sys/bus/ap/ap_control_domain_mask
>  
> -            The contents of my-guest-hostdev.xml:
> +    Note that each bit in the ap_control_domain_mask corresponds to
> +    correspond to domain numbers 0-255. If a bit is set to one, then

[Severity: Low]
This isn't a bug, but the phrase "corresponds to correspond to" appears
to be duplicated.

> +    the control domain is available on the destination host. For
> +    example:

[ ... ]

> ++-----------------------------------------------------------------------------+
> +| Table Keys                                                                  |
> ++=============================================================================+
> +|                                                                             |
> +| [1]        The hardware capability must be the same for both the source     |
> +|            and destination queue device.                                    |
> +|                                                                             |
> +| [2]        The source and destination queues must either both have          |
> +|            full native card function or both have stateless functions.      |
> +|            If the functional capabilities don't match, then the source      |
> +|            queue can have stateless capabilities since full native card     |
> +|            function can handle stateless functions; otherwise, migration    |
> +|            will fail.                                                       |
> +|                                                                             |
> +| [3]        The AP type on the source and destination guests can             |
> +|            differ if the queue passed through to the target guest is a      |
> +|            newer model (backwards compatible)                               |
> ++--------------+--------------------------------------------------------------+

[Severity: Low]
Will this malformed table bottom border cause sphinx build warnings?

The table body defines a single column, but the bottom border is split
into two columns by the + character, which violates reStructuredText grid 
table syntax.

> +
> +* To verify the hardware capabilities are compatible:
> +
> +  * On both the source and destination hosts, display the hardware capabilities
> +    for each AP adapter in the AP configuration for that host::
> +
> +       cat /sys/bus/ap/devices/card$APID/ap_functions
> +
> +    .. note::
> +       The ``$APID`` is the two-character adapter number in hexidecimal format;

[Severity: Low]
This isn't a bug, but there is a typo in the word "hexidecimal".

> +       for example, card02 or card1f. The ``/sys/bus/ap/devices`` directory
> +       also lists the APQNs of the queue devices installed in the respective
> +       host system with the first two characters being the APID.
> +
> +  * Verify the hardware capabilities for each AP adapter device on the source
> +    and destination host are compatible. The ``ap_functions`` attribute is a
> +    bitmask. The bits in the mask read from left to right starting with bit 0.
> +    Each bit that is set to one indicates the corresponding hardware capability
> +    is installed:
> +
> +    * Bits 0-3 are the facilities bits. Each bit value must match for the AP
> +      devices on the source and destination systems. The values indicate the
> +      following:
> +      * bit-0: APSC is installed
> +      * bit-1: AP4KM is installed
> +      * bit-2: AP4KC is installed

[Severity: Low]
Is a blank line needed before the nested list items (starting with bit-0)
to avoid reStructuredText parsing warnings?

> +
> +    * Bits 3-5 specify the adapter Modes. Only one of these bits will be set to
> +      one and must be the same for both the source and destination adapter
> +      devices:
> +      * bit-3: CCA-mode
> +      * bit-4: Accelerator-mode
> +      * bit-5: XCP-mode (EP11)
> +
> +    * Bit 6 specifies whether APXA is installed and must be the same for both
> +      the source and destination adapter devices.
> +
> +    * Bit 7 specifies command whether command filtering is installed

[Severity: Low]
This isn't a bug, but the phrase "command whether command filtering" has
duplicated words.

> +      and must be the same for both the source and destination adapter devices.

[ ... ]

> +One or more queues are not bound to the vfio_ap driver on the destination host
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The queue devices passed through to the source guest are not installed in the
> +destination host or are not bound to its vfio_ap device driver.
> +
> +**Source host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Log          | Message                                                        |
> ++==============+================================================================+
> +| Console log: | error: operation failed: migration failed. Message from the    |
> +|              | source host: operation failed: job 'migration out' failed:     |
> +|              | Sibling indicated error 1. Message from the destination host:  |
> +|              | operation failed: job 'migration in' failed: load of migration |
> +|              | failed: No such device: Failed to load vmstate version_id: 1,  |
> +|              | ret: -19                                                       |
> ++--------------+----------------------------------------------------------------+
> +
> +**Destinaton host**

[Severity: Low]
This isn't a bug, but there is a typo in the word "Destinaton".

> +
> ++--------------+----------------------------------------------------------------+
> +| Kernel log:  | vfio_ap_mdev $UUID: Queue $APID.$APQI not available to         |
> +|              | vfio_ap driver on target host                                  |
> ++--------------+----------------------------------------------------------------+
> +| QEMU log:    | shutting down, reason=failed                                   |
> +|              | qemu-system-s390x: terminating on signal $SIGNAL_NUM from      |
> +|              | pid $PID $DAEMON                                               |
> ++--------------+----------------------------------------------------------------+

[ ... ]

> +**Destination host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Log          | Message                                                        |
> ++==============+================================================================+
> +| Console log: | vfio_ap_mdev $UUID: migration failed: Failed to get hwinfo for |
> +|              | queue $APQN on target host: rc=$RESPONSE-CODE"                 |

[Severity: Low]
This isn't a bug, but there appears to be an unmatched trailing quote 
character at the end of the rc=$RESPONSE-CODE string.

> ++--------------+----------------------------------------------------------------+
> +| QEMU log:    | shutting down, reason=failed                                   |
> +|              | qemu-system-s390x: terminating on signal $SIGNAL-ID from pid   |
> +|              | $PID $DAEMON                                                   |
> ++--------------+----------------------------------------------------------------+
> +
> +.. note::
> +   * $UUID is the UUID of the mediated device being migrated from the source host.
> +   * $APQN (for example, 00.0033) of the queue device that is not compatible.
> +   * $RESPONSE_CODE is the response code from the AP instruction.
> +   * $SIGNAL_NUM is the signal number of the signal that caused the QEMU process
> +     to terminate.
> +   * $PID is the process ID of the QEMU process on the destination host.
> +   * $DAEMON is the name of the QEMU daemon process.
> +   * This message will be logged for each queue for which this error is detected.
> +
> +The AP configurations of the source guest and destination host not incompatible
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Severity: Low]
Does this heading have a double negative? 

The text below it enumerates failures when the configurations are not 
compatible, but the heading says "not incompatible" which logically means
they are compatible.

> +The functional capabilities or facilities available on one or more of the queues
> +passed through to the source guest are not compatible with the queue device with
> +the same APQN on the destination system (see the Hardware Capabilities table above)
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630104132.1598851-1-akrowiak@linux.ibm.com?part=15