Re: [PATCH v5 2/2] s390/kvm: diagnose 318 handling

[Collin Walling <walling@linux.ibm.com>]

On 6/26/19 8:11 AM, Cornelia Huck wrote:
> On Tue, 25 Jun 2019 11:03:42 -0400
> Collin Walling <walling@linux.ibm.com> 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.
>>
>> The get/set functions are introduced primarily for VM migration and
>> reset, though no harm could be done to the system if a userspace
>> program decides to alter this data (this is highly discouraged).
>>
>> The Control Program Name Code (CPNC) is stored in the SIE block (if
>> host hardware supports it) 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.
>>
>> At this time, the CPVC is not reported during a VM_EVENT as its
>> format is yet to be properly defined.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>   Documentation/virtual/kvm/devices/vm.txt | 14 ++++++
>>   arch/s390/include/asm/kvm_host.h         |  5 +-
>>   arch/s390/include/uapi/asm/kvm.h         |  4 ++
>>   arch/s390/kvm/diag.c                     | 17 +++++++
>>   arch/s390/kvm/kvm-s390.c                 | 81 ++++++++++++++++++++++++++++++++
>>   arch/s390/kvm/kvm-s390.h                 |  1 +
>>   arch/s390/kvm/vsie.c                     |  2 +
>>   7 files changed, 123 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
>> index 4ffb82b..56f7d9c 100644
>> --- a/Documentation/virtual/kvm/devices/vm.txt
>> +++ b/Documentation/virtual/kvm/devices/vm.txt
>> @@ -268,3 +268,17 @@ Parameters: address of a buffer in user space to store the data (u64) to;
>>   	    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
>> +Architectures: s390
>> +
>> +6.1. KVM_S390_VM_MISC_DIAG318 (r/w)
>> +
>> +Allows userspace to access the DIAGNOSE 0x318 information which consists of a
>> +1-byte "Control Program Name Code" and a 7-byte "Control Program Version Code".
>> +This information is initialized during IPL and must be preserved during
>> +migration.
>> +
>> +Parameters: address of a buffer in user space to store the data (u64) to
>> +Returns:    -EFAULT if the given address is not accessible from kernel space
>> +	     0 in case of success.
> 
> Hm, this looks a bit incomplete to me. IIUC, the guest will set this
> via diag 318, and this interface is intended to be used by user space
> for retrieving/setting this during migration. What about the following:
> 
> 
> 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.
> 
> 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
> 	     0 in case of success
> 
> Otherwise, no further comments from me.
> 

This is much more concise, thank you!