On 5/14/20 11:24 AM, Collin Walling wrote: > On 5/14/20 5:05 AM, Cornelia Huck wrote: >> On Wed, 13 May 2020 18:15:57 -0400 >> Collin Walling <walling@xxxxxxxxxxxxx> wrote: >> >>> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must >>> be intercepted by SIE and handled via KVM. Let's introduce some >>> functions to communicate between userspace and KVM via ioctls. These >>> will be used to get/set the diag318 related information, as well as >>> check the system if KVM supports handling this instruction. >>> >>> This information can help with diagnosing the environment the VM is >>> running in (Linux, z/VM, etc) if the OS calls this instruction. >>> >>> By default, this feature is disabled and can only be enabled if a >>> user space program (such as QEMU) explicitly requests it. >>> >>> The Control Program Name Code (CPNC) is stored in the SIE block >>> and a copy is retained in each VCPU. The Control Program Version >>> Code (CPVC) is not designed to be stored in the SIE block, so we >>> retain a copy in each VCPU next to the CPNC. >>> >>> Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx> >>> --- >>> Documentation/virt/kvm/devices/vm.rst | 29 +++++++++ >>> arch/s390/include/asm/kvm_host.h | 6 +- >>> arch/s390/include/uapi/asm/kvm.h | 5 ++ >>> arch/s390/kvm/diag.c | 20 ++++++ >>> arch/s390/kvm/kvm-s390.c | 89 +++++++++++++++++++++++++++ >>> arch/s390/kvm/kvm-s390.h | 1 + >>> arch/s390/kvm/vsie.c | 2 + >>> 7 files changed, 151 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/virt/kvm/devices/vm.rst b/Documentation/virt/kvm/devices/vm.rst >>> index 0aa5b1cfd700..9344d45ace6d 100644 >>> --- a/Documentation/virt/kvm/devices/vm.rst >>> +++ b/Documentation/virt/kvm/devices/vm.rst >>> @@ -314,3 +314,32 @@ Allows userspace to query the status of migration mode. >>> if it is enabled >>> :Returns: -EFAULT if the given address is not accessible from kernel space; >>> 0 in case of success. >>> + >>> +6. GROUP: KVM_S390_VM_MISC >> >> This needs to be rstyfied, matching the remainder of the file. >> >>> +Architectures: s390 >>> + >>> + 6.1. KVM_S390_VM_MISC_ENABLE_DIAG318 >>> + >>> + Allows userspace to enable the DIAGNOSE 0x318 instruction call for a >>> + guest OS. By default, KVM will not allow this instruction to be executed >>> + by a guest, even if support is in place. Userspace must explicitly enable >>> + the instruction handling for DIAGNOSE 0x318 via this call. >>> + >>> + Parameters: none >>> + Returns: 0 after setting a flag telling KVM to enable this feature >>> + >>> + 6.2. KVM_S390_VM_MISC_DIAG318 (r/w) >>> + >>> + Allows userspace to retrieve and set the DIAGNOSE 0x318 information, >>> + which consists of a 1-byte "Control Program Name Code" and a 7-byte >>> + "Control Program Version Code" (a 64 bit value all in all). This >>> + information is set by the guest (usually during IPL). This interface is >>> + intended to allow retrieving and setting it during migration; while no >>> + real harm is done if the information is changed outside of migration, >>> + it is strongly discouraged. >> >> (Sorry if we discussed that already, but that was some time ago and the >> info has dropped out of my cache...) >> >> Had we considered doing this in userspace only? If QEMU wanted to >> emulate diag 318 in tcg, it would basically need to mirror what KVM >> does; diag 318 does not seem like something where we want to optimize >> for performance, so dropping to userspace seems feasible? We'd just >> need an interface for userspace to forward anything set by the guest. >> > > My reservation with respect to handling this in userspace only is that > the data set by the instruction call is relevant to both host-level and > guest-level kernels. That data is set during kernel setup. > > Right now, the instruction call is used to set a hard-coded "name code" > value, but later we want to use this instruction to also set some sort > of unique version code. The format of the version code is not yet > determined. > > If guest support is handled in userspace only, then we'll have to update > the version codes in both the Linux kernel /and/ QEMU, which might be a > bit messy if things go out of sync. > In an attempt to clear up some fogginess with respect to "what" the version code may entail, we're thinking of some sort of 7-byte combination that denotes both the kernel version and a value that denotes the distro + release. We're not 100% solid on exactly what that format will look like just yet, but all of the discussions have revolved around that theme. >>> + >>> + Parameters: address of a buffer in user space (u64), where the >>> + information is read from or stored into >>> + Returns: -EFAULT if the given address is not accessible from kernel space; >>> + -EOPNOTSUPP if feature has not been requested to be enabled first; >>> + 0 in case of success >> > > -- -- Regards, Collin Stay safe and stay healthy
