ACK: [SRU][F][H][I][PATCH 0/x] KVM hardware diagnose data improvements for guest kernel (LP: 1953334)

[Philip Cox]

On 2021-12-07 3:38 p.m., frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/1953334
>
> SRU Justification:
>
> [Impact]
>
> * Hardware diagnose data (diag 318) of KVM guest kernel cannot be handled.
>
> * A fix is needed to enhance problem determination of guest kernel under KVM
>    using DIAG 0x318 instruction execution.
>
> * The s390x diagnose 318 instruction sets the control program name code (CPNC)
>    and control program version code (CPVC) to provide useful information
>    regarding the OS during debugging.
>
> * The CPNC is explicitly set to 4 to indicate a Linux/KVM environment.
>
> [Fix]
>
> * In general the following 4 commits are needed:
>
> * 3fd8417f2c728d810a3b26d7e2008012ffb7fd01 3fd8417f2c72 "KVM: s390: add debug statement for diag 318 CPNC data"
>    https://launchpadlibrarian.net/572621875/0004-KVM-s390-add-debug-statement-for-diag-318-CPNC-data.patch
>
> * 6cbf1e960fa52e4c63a6dfa4cda8736375b34ccc 6cbf1e960fa5 "KVM: s390: remove diag318 reset code"
>    https://launchpadlibrarian.net/572621873/0003-KVM-s390-remove-diag318-reset-code.patch
>
> * 23a60f834406c8e3805328b630d09d5546b460c1 23a60f834406 "s390/kvm: diagnose 0x318 sync and reset"
>    https://launchpadlibrarian.net/572621869/0002-s390-kvm-diagnose-0x318-sync-and-reset.patch
>
> * a23816f3cdcbffe5dc6e8c331914b3f51b87c2f3 a23816f3cdcb "s390/setup: diag 318: refactor struct"
>    https://launchpadlibrarian.net/572620815/0001-s390-setup-diag-318-refactor-struct.patch
>
> * For jammy, hirsute and impish only the first commit is needed, the others are already in.
>
> * For focal all 4 commits are needed, but since they do not apply cleanly on focal, the attached backports need to be used instead.
>
> [Test Case]
>
> * Setup an IBM Z or LinuxONE LPAR with Ubuntu Server as KVM host.
>
> * And setup an Ubuntu KVM virtual on top.
>
> * It can then be observed if the CPNC (diag318 data) has been successfully set
>    by looking at the s390dbf messages for the KVM guest.
>
> * The CPNC will always be 4 (denotes Linux environment).
>
> * Another way to test this is by running the sync_regs_test under
>    tools/testing/selftests/kvm/s390x/sync_regs_test.
>    Just running the kernel self test suite can trigger this.
>
> [Where problems could occur]
>
> * The approach here is to provide additional debug and diagnose information on top.
>
> * Hence even if the diag318 changes are broken,
>    the existing functionality shouldn't be harmed.
>
> * The changes themselves are relatively discernible and mostly introduce new structures.
>
> * However, with the functional changes broken code could be introduced
>    (e.g. due to erroneous pointer arithmetic for example)
>    that does not compile or causes crashes.
>    But this is what the test builds are for
>    (https://launchpad.net/~fheimes/+archive/ubuntu/lp1953334).
>
> * On top the diag318 diagnose data might not properly provided - maybe empty or wrong.
>    Again that is what the test builds and the verification later is targeted at.
>
> * Since diag318 is s390x specific, all the modifications touch s390x code only.
>    (in arch/s390/kvm/ kvm-s390.c and vsie.c, arch/s390/kernel/setup.c,
>     arch/s390/include/asm/ kvm_host.h, kvm.h and diag.h)
>    At least no other other architecture will be affected.
>
> * Well, there is one tiny bit of a common code change,
>    but it's just a new define statement in include/uapi/linux/kvm.h
>    ('#define KVM_CAP_S390_DIAG318 186')
>
> [Other]
>
> * Request was to add the patches to focal / 20.04,
>    but to avoid potential regressions on upgrades,
>    the patches need to be added to jammy, impish and hirsute, too.
>
> * As mentioned above, Jammy, Hirsute and Impish includes almost everything needed,
>    except 3fd8417f2c72 "KVM: s390: add debug statement for diag 318 CPNC data".
>
> * Hence the SRU is for Focal, Jammy, Hirsute and Impish,
>    but less invasive for Jammy, Hirsute and Impish,
>    also because commit 3fd8417f2c72 can be cleanly cherry-picked form there.
>
> * LP#1953338 is related to this bug and covers the qemu/KVM bits.
>
> Collin Walling (4):
>    s390/setup: diag 318: refactor struct
>    s390/kvm: diagnose 0x318 sync and reset
>    KVM: s390: remove diag318 reset code
>    KVM: s390: add debug statement for diag 318 CPNC data
>
>   arch/s390/include/asm/diag.h     |  6 ++----
>   arch/s390/include/asm/kvm_host.h |  4 +++-
>   arch/s390/include/uapi/asm/kvm.h |  7 +++++--
>   arch/s390/kernel/setup.c         |  3 +--
>   arch/s390/kvm/kvm-s390.c         | 10 +++++++++-
>   arch/s390/kvm/vsie.c             |  1 +
>   include/uapi/linux/kvm.h         |  1 +
>   7 files changed, 22 insertions(+), 10 deletions(-)
>

Acked-by: Philip Cox <philip.cox at canonical.com>