Re: [PATCH/RFC 5/9] KVM: s390: introduce post handlers for STSI

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 16.03.2015 um 12:55 schrieb Paolo Bonzini:
> 
> 
> On 16/03/2015 09:51, Christian Borntraeger wrote:
>> From: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>
>> The Store System Information (STSI) instruction currently collects all
>> information it relays to the caller in the kernel. Some information,
>> however, is only available in user space. An example of this is the
>> guest name: The kernel always sets "KVMGuest", but user space knows the
>> actual guest name.
>>
>> This patch introduces a new exit, KVM_EXIT_S390_STSI, guarded by a
>> capability that can be enabled by user space if it wants to be able to
>> insert such data. User space will be provided with the target buffer
>> and the requested STSI function code.
>>
>> Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  Documentation/virtual/kvm/api.txt | 28 ++++++++++++++++++++++++++++
>>  arch/s390/include/asm/kvm_host.h  |  1 +
>>  arch/s390/kvm/kvm-s390.c          |  5 +++++
>>  arch/s390/kvm/priv.c              | 16 +++++++++++++++-
>>  include/uapi/linux/kvm.h          | 11 +++++++++++
>>  5 files changed, 60 insertions(+), 1 deletion(-)
> 
> This is okay, but it made me curious about s390 specifics because it's
> different from the x86 equivalent.  The x86 info is always 16-byte (4
> times 32 bit) and the selectors are few (roughly 30), and it is passed
> to the kernel at init time via KVM_SET_CPUID2.  How many selectors are
> there on s390, and how big is the info that can be written?

Its up to a page and the content depends on the function code, selector1
and selector2.
See arch/s390/kernel/sysinfo.c for the details or zarchitecture principles of
opration the STSI instruction.


Some time ago we had patches that passed the guest name
and UUID into the kernel - then Alex Graf asked, why we cant do that in 
userspace.
https://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg00094.html

Now, stsi also gives the bare information as provides by upper
layers: e.g. 2.2.2 gives basic LPAR information. When doing that under
KVM or z/VM, KVM or z/VM will pass that information along to the underlying 
LPAR hypervisor and give the result back to the guest.

So I came up with the idea to let KVM do what it needs but also allow
QEMU to override (3.2.2 - this 2nd level hypervisor)

https://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg00286.html

Just passing in uuid and guest name (e.g. via VM attribute) would suffice
for what we need today, but it certainly goes back to where we came from
(minus that we dont have the config device, instead we now have VM attributes)
The overall code would certainly by smaller, but then its kernel only.

Christian



> 
> Paolo
> 
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index f03178d..74d18b6 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -3304,3 +3304,31 @@ Returns: 0 on success, negative value on error
>>  Allows use of the vector registers introduced with z13 processor, and
>>  provides for the synchronization between host and user space.  Will
>>  return -EINVAL if the machine does not support vectors.
>> +
>> +7.4 KVM_CAP_S390_USER_STSI
>> +
>> +Architectures: s390
>> +Parameters: none
>> +
>> +This capability allows post-handlers for the STSI instruction. After
>> +initial handling in the kernel, KVM exits to user space with
>> +KVM_EXIT_S390_STSI to allow user space to insert further data.
>> +
>> +Before exiting to userspace, kvm handlers should fill in s390_stsi field of
>> +vcpu->run:
>> +struct {
>> +	__u64 addr;
>> +	__u8 ar;
>> +	__u8 reserved;
>> +	__u8 fc;
>> +	__u8 sel1;
>> +	__u16 sel2;
>> +} s390_stsi;
>> +
>> +@addr - guest address of STSI SYSIB
>> +@fc   - function code
>> +@sel1 - selector 1
>> +@sel2 - selector 2
>> +@ar   - access register number
>> +
>> +KVM handlers should exit to userspace with rc = -EREMOTE.
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 347a333..2356a8c 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -565,6 +565,7 @@ struct kvm_arch{
>>  	int use_vectors;
>>  	int user_cpu_state_ctrl;
>>  	int user_sigp;
>> +	int user_stsi;
>>  	struct s390_io_adapter *adapters[MAX_S390_IO_ADAPTERS];
>>  	wait_queue_head_t ipte_wq;
>>  	int ipte_lock_count;
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 92855e2..1f5baf4 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -178,6 +178,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_VM_ATTRIBUTES:
>>  	case KVM_CAP_MP_STATE:
>>  	case KVM_CAP_S390_USER_SIGP:
>> +	case KVM_CAP_S390_USER_STSI:
>>  		r = 1;
>>  		break;
>>  	case KVM_CAP_S390_MEM_OP:
>> @@ -280,6 +281,10 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>>  		kvm->arch.use_vectors = MACHINE_HAS_VX;
>>  		r = MACHINE_HAS_VX ? 0 : -EINVAL;
>>  		break;
>> +	case KVM_CAP_S390_USER_STSI:
>> +		kvm->arch.user_stsi = 1;
>> +		r = 0;
>> +		break;
>>  	default:
>>  		r = -EINVAL;
>>  		break;
>> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
>> index 5f26425..211102c 100644
>> --- a/arch/s390/kvm/priv.c
>> +++ b/arch/s390/kvm/priv.c
>> @@ -488,6 +488,16 @@ static void handle_stsi_3_2_2(struct kvm_vcpu *vcpu, struct sysinfo_3_2_2 *mem)
>>  	ASCEBC(mem->vm[0].cpi, 16);
>>  }
>>  
>> +static void insert_stsi_usr_data(struct kvm_vcpu *vcpu, u64 addr, u8 fc,
>> +				 u8 sel1, u16 sel2)
>> +{
>> +	vcpu->run->exit_reason = KVM_EXIT_S390_STSI;
>> +	vcpu->run->s390_stsi.addr = addr;
>> +	vcpu->run->s390_stsi.fc = fc;
>> +	vcpu->run->s390_stsi.sel1 = sel1;
>> +	vcpu->run->s390_stsi.sel2 = sel2;
>> +}
>> +
>>  static int handle_stsi(struct kvm_vcpu *vcpu)
>>  {
>>  	int fc = (vcpu->run->s.regs.gprs[0] & 0xf0000000) >> 28;
>> @@ -547,11 +557,15 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>>  		rc = kvm_s390_inject_prog_cond(vcpu, rc);
>>  		goto out;
>>  	}
>> +	if (vcpu->kvm->arch.user_stsi) {
>> +		insert_stsi_usr_data(vcpu, operand2, fc, sel1, sel2);
>> +		rc = -EREMOTE;
>> +	}
>>  	trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2);
>>  	free_page(mem);
>>  	kvm_s390_set_psw_cc(vcpu, 0);
>>  	vcpu->run->s.regs.gprs[0] = 0;
>> -	return 0;
>> +	return rc;
>>  out_no_data:
>>  	kvm_s390_set_psw_cc(vcpu, 3);
>>  out:
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 74bbd9a..279ffd2 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -172,6 +172,7 @@ struct kvm_pit_config {
>>  #define KVM_EXIT_S390_TSCH        22
>>  #define KVM_EXIT_EPR              23
>>  #define KVM_EXIT_SYSTEM_EVENT     24
>> +#define KVM_EXIT_S390_STSI        25
>>  
>>  /* For KVM_EXIT_INTERNAL_ERROR */
>>  /* Emulate instruction failed. */
>> @@ -309,6 +310,15 @@ struct kvm_run {
>>  			__u32 type;
>>  			__u64 flags;
>>  		} system_event;
>> +		/* KVM_EXIT_S390_STSI */
>> +		struct {
>> +			__u64 addr;
>> +			__u8 ar;
>> +			__u8 reserved;
>> +			__u8 fc;
>> +			__u8 sel1;
>> +			__u16 sel2;
>> +		} s390_stsi;
>>  		/* Fix the size of the union. */
>>  		char padding[256];
>>  	};
>> @@ -780,6 +790,7 @@ struct kvm_ppc_smmu_info {
>>  #define KVM_CAP_S390_USER_SIGP 106
>>  #define KVM_CAP_S390_VECTOR_REGISTERS 107
>>  #define KVM_CAP_S390_MEM_OP 108
>> +#define KVM_CAP_S390_USER_STSI 109
>>  
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>  
>>
>