All of lore.kernel.org
 help / color / mirror / Atom feed
From: Collin Walling <walling@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	pbonzini@redhat.com, borntraeger@de.ibm.com,
	frankja@linux.ibm.com, david@redhat.com, imbrenda@linux.ibm.com,
	heiko.carstens@de.ibm.com, gor@linux.ibm.com
Subject: Re: [PATCH v6 2/2] s390/kvm: diagnose 318 handling
Date: Thu, 14 May 2020 11:24:48 -0400	[thread overview]
Message-ID: <d4cfe6dc-4ce6-b588-88fd-9e0bc6684e8a@linux.ibm.com> (raw)
In-Reply-To: <20200514110544.147a63f8.cohuck@redhat.com>

On 5/14/20 5:05 AM, Cornelia Huck wrote:
> On Wed, 13 May 2020 18:15:57 -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.
>>
>> 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@linux.ibm.com>
>> ---
>>  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.

>> +
>> + 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

  reply	other threads:[~2020-05-14 15:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 22:15 [PATCH v6 0/2] Use DIAG318 to set Control Program Name & Version Codes Collin Walling
2020-05-13 22:15 ` [PATCH v6 1/2] s390/setup: diag318: refactor struct Collin Walling
2020-05-14  7:15   ` Thomas Huth
2020-05-14  8:33   ` Janosch Frank
2020-05-13 22:15 ` [PATCH v6 2/2] s390/kvm: diagnose 318 handling Collin Walling
2020-05-14  2:22   ` kbuild test robot
2020-05-14  2:22     ` kbuild test robot
2020-05-14  7:53   ` Thomas Huth
2020-05-14  8:52     ` Janosch Frank
2020-05-14  9:37       ` David Hildenbrand
2020-05-14  9:49         ` Janosch Frank
2020-05-14  9:53           ` David Hildenbrand
2020-05-14 17:20             ` Collin Walling
2020-05-14 18:37               ` Thomas Huth
2020-05-14 18:53                 ` Collin Walling
2020-05-15  6:16                   ` Cornelia Huck
2020-05-14 18:40       ` Thomas Huth
2020-05-14  9:05   ` Cornelia Huck
2020-05-14 15:24     ` Collin Walling [this message]
2020-05-14 15:49       ` Collin Walling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d4cfe6dc-4ce6-b588-88fd-9e0bc6684e8a@linux.ibm.com \
    --to=walling@linux.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.