mbox series

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

Message ID 20240308122820.316586-1-frank.heimes@canonical.com
Headers show
Series Problems with HVCS and hotplugging (LP: 2056373) | expand

Message

Frank Heimes March 8, 2024, 12:28 p.m. UTC
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(-)

Comments

Stefan Bader March 15, 2024, 9:21 a.m. UTC | #1
On 08.03.24 13:28, frank.heimes@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@canonical.com>
Cengiz Can March 15, 2024, 10:52 a.m. UTC | #2
On 3/8/24 15:28, frank.heimes@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

Acked-by: Cengiz Can <cengiz.can@canonical.com>

> 
>   arch/powerpc/platforms/pseries/of_helpers.c | 10 +--
>   drivers/tty/hvc/hvcs.c                      | 91 ++++++++-------------
>   2 files changed, 38 insertions(+), 63 deletions(-)
>
Stefan Bader March 15, 2024, 2:02 p.m. UTC | #3
On 08.03.24 13:28, frank.heimes@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(-)
> 

Applied to jammy,focal:linux/master-next. Thanks.

-Stefan