ACK/Cmnt: [SRU][J][F][PATCH 0/8] Problems with HVCS and hotplugging (LP: 2056373)

[Stefan Bader]

On 08.03.24 13:28, frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/2056373
> 
> SRU Justification:
> 
> [Impact]
> 
>   * HVCS (Hypervisor Virtual Console Server) is broken because the
>     virtual terminal mkvterm fails, caused by pvmutil failing.
> 
>   * When mkvterm is ran, it ultimately fails because it calls pvmutil
>     which fails.
>     pvmutil calls drmgr, and drmgr is adding a slot correctly.
>     However, when drmgr writes the slot information to ?/add_slot,
>     the return is -ENODEV.
> 
>   * This leads to HVCS never having probe() called.
> 
>   * In addition, HVCS is missing patches/fixes, and is broken without them.
> 
> [Fix]
> 
>   * Fix one and two is required for focal only, all other for focal and jammy:
> 
>   * 57409d4fb12c 57409d4fb12c185b2c0689e0496878c8f6bb5b58
>     "powerpc/pseries: Fix bad drc_index_start value parsing of drc-info entry"
> 
>   * c5e76fa05b2d c5e76fa05b2df519b9f08571cc57e623c1569faa
>     "powerpc/pseries: Fix of_read_drc_info_cell() to point at next record"
> 
>   * 6a9a733edd46 6a9a733edd46732e906d976dc21a42dd361e53cc
>     "hvcs: Fix hvcs port reference counting"
> 
>   * 760aa5e81f33 760aa5e81f33e0da82512c4288489739a6d1c556
>     "hvcs: Use dev_groups to manage hvcs device attributes"
> 
>   * 503a90dd619d 503a90dd619d52dcac2cc68bd742aa914c7cd47a
>     "hvcs: Use driver groups to manage driver attributes"
> 
>   * 3a8d3b366ce4 3a8d3b366ce47024bf274eac783f8af5df2780f5
>     "hvcs: Get reference to tty in remove"
> 
>   * d432228bc7b1 d432228bc7b1b3f0ed06510278ff5a77b3749fe6
>     "hvcs: Use vhangup in hotplug remove"
> 
>   * 28d49f8cbe9c 28d49f8cbe9c7966f91ee1b5ec2f997f6e55bf9f
>     "hvcs: Synchronize hotplug remove with port free"
> 
> [Test Plan]
> 
>   * The high level test plan is to run mkvterm with an id.
>   
>   * mkvterm will fail because /dev/hvcs* device nodes are missing.
> 
>   * Details see https://bugs.launchpad.net/bugs/2023243 for more information.
>     Especially the script provided by IBM
>     (see original bug description: `---Steps to Reproduce---`).
> 
>   * IBM will (stress) test the updated kernel(s) provided in -proposed.
> 
> [Where problems could occur]
> 
>   * The first two commits affect arch/powerpc/platforms/pseries/of_helpers.c
>     and are needed to fix the hotplugging issue seen when drmgr goes to write
>     the slot information to /sys/bus/pci/slots/control/add_slot.
>     In case of issues here hotplugging with drmgr might break.
> 
>   * The issue lies in rpadlpar_io and rpaphp calling an of helper function
>     of_read_drc_info_cell(). Without these commits, the value stored
>     drc_index_start is incorrect.
>     This ultimately results in the entire SLOT string being incorrect,
>     and rpaphp never finding the newly added slot by drmgr.
>     rpadlpar then returns -ENODEV.
>     Therefore, HVCS is never probed, and the device nodes are never created.
> 
>   * HVCS, rpadlpar_io, and rpaphp should ideally not even need to be loaded
>     prior to drmgr adding a vio slot.
>     If rpadlpar_io and rpaphp are not loaded, drmgr will load them.
>     In addition, if rpadlpar_io and rpaphp register the new slot correctly,
>     rpadlpar_io will call dlpar_add_vio_slot(),
>     which calls vio_register_device_node() with the device node.
>     This is what tells the driver core to init and probe HVCS
>     (which is needed to create the device nodes).
> 
>   * The remaning 6 commits are needed for HVCS, that is essentially
>     broken without them.
>     Overall, issues they fix are race conditions, hotplug remove issues,
>     as well as memory leaks.
> 
>   * Please notice that this is entirely ppc64el architecture-specifc.
> 
> [Other Info]
> 
>   * All the commits listed above are included in mantic and noble.
>     Hence these are set to Invalid.
> 
>   * Meanwhile these requested commits have been added to other
>     kernels and distros.
> 
> Brian King (6):
>    hvcs: Fix hvcs port reference counting
>    hvcs: Use dev_groups to manage hvcs device attributes
>    hvcs: Use driver groups to manage driver attributes
>    hvcs: Get reference to tty in remove
>    hvcs: Use vhangup in hotplug remove
>    hvcs: Synchronize hotplug remove with port free
> 
> Tyrel Datwyler (2):
>    powerpc/pseries: Fix bad drc_index_start value parsing of drc-info
>      entry
>    powerpc/pseries: Fix of_read_drc_info_cell() to point at next record
> 
>   arch/powerpc/platforms/pseries/of_helpers.c | 10 +--
>   drivers/tty/hvc/hvcs.c                      | 91 ++++++++-------------
>   2 files changed, 38 insertions(+), 63 deletions(-)
> 
Limited to architectural code and all cherry picks. Test plan by vendor 
in place.

Acked-by: Stefan Bader <stefan.bader at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 48643 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240315/b197420f/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240315/b197420f/attachment-0001.sig>