[v6,0/2] Use DIAG318 to set Control Program Name & Version Codes

Message

Collin Walling Jan. 24, 2020, 10:14 p.m. UTC

Changes from v5 -> v6

    Migration and DeviceObject Code:
        - load/save/needed functions now check if kvm_enabled before calling
            kvm_get/set and has_feat (respectively)

    QEMU->KVM Code:
        - added kvm_s390_* stubs for get/set functions for TCG compilation

    VCPU Discussion:
        - calculate the maximum allowed cpu entries by taking the SCCB size,
            subtracting the offset where the CPU entries begin, then dividing
            by the size of a CPU Entry struct
        - if the number of CPU entries exceeds the maximum allowed entries,
            print a warning and break out of the loop
        - no longer imposing a reduced CPU max

Last post: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05535.html

The data associated with DIAGNOSE 0x318 helps to identify the underlying 
hypervisor level (pre-determined by an internal set of codes), as well as the
guest environment (such as Linux, z/VM, etc). These patches, in tandem with
KVM, allow this instruction to be enabled at the guest level, and also to 
enable migration of this data.

The DIAGNOSE 0x318 instruction is a privileged instruction that is executed by
the Linux kernel once and only once during setup (IPL). This requires 
interception by KVM to handle the instruction call safely. The instruction sets
an 8-byte value corresponding to the environment the control program (i.e.
guest) is running with, as well as what hypervisor it is running on.

An update to the analogous KVM patches associated with this patchset are
forthcoming and I will provide a link to the post as a reply to this chain.

Guest support for the diag 318 instruction is accomplished by implementing a 
device class, a cpu model feature, and adjusting the Read Info struct. The Read
Info struct adjustment coincidentally reduces the maximum number of VCPUs we 
can have for a single guest by one.

The instruction is determined by a Read Info byte 134 bit 0. This new byte
expands into the space of the Read Info SCCB, which also contains CPU entries
at the tail-end of this block of data. Due to this expansion, we lose space for
one CPU entry.

A guest can still run safely with the original 248 maximum CPUs, but will lose
access to the 248th CPU entry, meaning that the hypervisor will be unable to
retrieve any information regarding that CPU (weather this means the guest
will actually run with 247 CPUs when 248 are specified is uncertain to me, but
the guest operates just fine on my end).

A device class is used for this instruction in order to streamline the 
migration and reset of the DIAG 318 related data.

A CPU model feature is added for this instruction, appropriately named diag318,
and is available starting with the zEC12 full model, though as long as KVM can
support emulation of this instruction, we can theoretically enable it for _any_
CPU model. It is recommended to explicitly enable the feature via 
-cpu ...,diag318=on (or via libvirt feature XML).

Collin L. Walling (2):
  s390/kvm: header sync for diag318
  s390: diagnose 318 info reset and migration support

 hw/s390x/Makefile.objs              |  1 +
 hw/s390x/diag318.c                  | 85 +++++++++++++++++++++++++++++++++++++
 hw/s390x/diag318.h                  | 40 +++++++++++++++++
 hw/s390x/s390-virtio-ccw.c          | 17 ++++++++
 hw/s390x/sclp.c                     | 13 ++++++
 include/hw/s390x/sclp.h             |  2 +
 linux-headers/asm-s390/kvm.h        |  4 ++
 target/s390x/cpu_features.h         |  1 +
 target/s390x/cpu_features_def.inc.h |  3 ++
 target/s390x/gen-features.c         |  1 +
 target/s390x/kvm-stub.c             | 10 +++++
 target/s390x/kvm.c                  | 29 +++++++++++++
 target/s390x/kvm_s390x.h            |  2 +
 13 files changed, 208 insertions(+)
 create mode 100644 hw/s390x/diag318.c
 create mode 100644 hw/s390x/diag318.h

Comments

[e-mail] Jan. 24, 2020, 10:22 p.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [e-mail]
Subject: [PATCH v6 0/2] Use DIAG318 to set Control Program Name & Version Codes

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
bf39c82 s390: diagnose 318 info reset and migration support
7a5ff34 s390/kvm: header sync for diag318

=== OUTPUT BEGIN ===
1/2 Checking commit 7a5ff34d93c1 (s390/kvm: header sync for diag318)
2/2 Checking commit bf39c8211f83 (s390: diagnose 318 info reset and migration support)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#38: 
new file mode 100644

ERROR: braces {} are necessary for all arms of this statement
#64: FILE: hw/s390x/diag318.c:22:
+    if (kvm_enabled())
[...]

ERROR: braces {} are necessary for all arms of this statement
#74: FILE: hw/s390x/diag318.c:32:
+    if (kvm_enabled())
[...]

ERROR: braces {} are necessary for all arms of this statement
#100: FILE: hw/s390x/diag318.c:58:
+    if (kvm_enabled())
[...]

ERROR: adding a line without newline at end of file
#173: FILE: hw/s390x/diag318.h:40:
+#endif /* HW_DIAG318_H */

WARNING: line over 80 characters
#314: FILE: target/s390x/cpu_features_def.inc.h:124:
+DEF_FEAT(DIAG318, "diag318", SCLP_BYTE_134, 0, "Control program name and version codes")

total: 4 errors, 2 warnings, 310 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Collin Walling March 17, 2020, 9:34 p.m. UTC | #2

Please note: a new patch is in the works to extend the SCCB for the SCLP
response for RSCPI. This will help alleviate the issue of losing space 
for CPU
entries. The appropriate patch will be introduced at the beginning of this
series once it is ready for upstream.


Thanks for your patience and understanding,
- Collin

On 1/24/20 5:14 PM, Collin Walling wrote:
> Changes from v5 -> v6
>
>      Migration and DeviceObject Code:
>          - load/save/needed functions now check if kvm_enabled before calling
>              kvm_get/set and has_feat (respectively)
>
>      QEMU->KVM Code:
>          - added kvm_s390_* stubs for get/set functions for TCG compilation
>
>      VCPU Discussion:
>          - calculate the maximum allowed cpu entries by taking the SCCB size,
>              subtracting the offset where the CPU entries begin, then dividing
>              by the size of a CPU Entry struct
>          - if the number of CPU entries exceeds the maximum allowed entries,
>              print a warning and break out of the loop
>          - no longer imposing a reduced CPU max
>
> Last post: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05535.html
>
> The data associated with DIAGNOSE 0x318 helps to identify the underlying
> hypervisor level (pre-determined by an internal set of codes), as well as the
> guest environment (such as Linux, z/VM, etc). These patches, in tandem with
> KVM, allow this instruction to be enabled at the guest level, and also to
> enable migration of this data.
>
> The DIAGNOSE 0x318 instruction is a privileged instruction that is executed by
> the Linux kernel once and only once during setup (IPL). This requires
> interception by KVM to handle the instruction call safely. The instruction sets
> an 8-byte value corresponding to the environment the control program (i.e.
> guest) is running with, as well as what hypervisor it is running on.
>
> An update to the analogous KVM patches associated with this patchset are
> forthcoming and I will provide a link to the post as a reply to this chain.
>
> Guest support for the diag 318 instruction is accomplished by implementing a
> device class, a cpu model feature, and adjusting the Read Info struct. The Read
> Info struct adjustment coincidentally reduces the maximum number of VCPUs we
> can have for a single guest by one.
>
> The instruction is determined by a Read Info byte 134 bit 0. This new byte
> expands into the space of the Read Info SCCB, which also contains CPU entries
> at the tail-end of this block of data. Due to this expansion, we lose space for
> one CPU entry.
>
> A guest can still run safely with the original 248 maximum CPUs, but will lose
> access to the 248th CPU entry, meaning that the hypervisor will be unable to
> retrieve any information regarding that CPU (weather this means the guest
> will actually run with 247 CPUs when 248 are specified is uncertain to me, but
> the guest operates just fine on my end).
>
> A device class is used for this instruction in order to streamline the
> migration and reset of the DIAG 318 related data.
>
> A CPU model feature is added for this instruction, appropriately named diag318,
> and is available starting with the zEC12 full model, though as long as KVM can
> support emulation of this instruction, we can theoretically enable it for _any_
> CPU model. It is recommended to explicitly enable the feature via
> -cpu ...,diag318=on (or via libvirt feature XML).
>
> Collin L. Walling (2):
>    s390/kvm: header sync for diag318
>    s390: diagnose 318 info reset and migration support
>
>   hw/s390x/Makefile.objs              |  1 +
>   hw/s390x/diag318.c                  | 85 +++++++++++++++++++++++++++++++++++++
>   hw/s390x/diag318.h                  | 40 +++++++++++++++++
>   hw/s390x/s390-virtio-ccw.c          | 17 ++++++++
>   hw/s390x/sclp.c                     | 13 ++++++
>   include/hw/s390x/sclp.h             |  2 +
>   linux-headers/asm-s390/kvm.h        |  4 ++
>   target/s390x/cpu_features.h         |  1 +
>   target/s390x/cpu_features_def.inc.h |  3 ++
>   target/s390x/gen-features.c         |  1 +
>   target/s390x/kvm-stub.c             | 10 +++++
>   target/s390x/kvm.c                  | 29 +++++++++++++
>   target/s390x/kvm_s390x.h            |  2 +
>   13 files changed, 208 insertions(+)
>   create mode 100644 hw/s390x/diag318.c
>   create mode 100644 hw/s390x/diag318.h
>