From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: [PATCH]: kvm-s390: add KVM_S390_GET/SET_SREGS2 call for additional
 hw regs
Date: Tue, 20 Dec 2011 10:59:57 +0100
Message-ID: <4EF05C9D.6040306@de.ibm.com>
References: <4EF0577D.6010902@de.ibm.com> <4EF05AD0.8050808@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	KVM list <kvm@vger.kernel.org>, Alexander Graf <agraf@suse.de>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Jens Freimann <jfrei@linux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Carsten Otte <cotte@de.ibm.com>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:46567 "EHLO
	e06smtp11.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752222Ab1LTKAN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 20 Dec 2011 05:00:13 -0500
Received: from /spool/local
	by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted
	for <kvm@vger.kernel.org> from <borntraeger@de.ibm.com>;
	Tue, 20 Dec 2011 10:00:05 -0000
Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213])
	by d06nrmr1307.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pBK9xxv82326560
	for <kvm@vger.kernel.org>; Tue, 20 Dec 2011 09:59:59 GMT
Received: from d06av03.portsmouth.uk.ibm.com (localhost.localdomain [127.0.0.1])
	by d06av03.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pBK9xwa7017782
	for <kvm@vger.kernel.org>; Tue, 20 Dec 2011 02:59:59 -0700
In-Reply-To: <4EF05AD0.8050808@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

>>  arch/s390/include/asm/kvm.h |    9 +++++++++
>>  arch/s390/kvm/kvm-s390.c    |   24 ++++++++++++++++++++++++
>>  include/linux/kvm.h         |    4 ++++
>>  3 files changed, 37 insertions(+)
> 
> The lack of documentation is not.

Ok, will do.
> 
> 
>> @@ -673,6 +674,29 @@ long kvm_arch_vcpu_ioctl(struct file *fi
>>  	case KVM_S390_INITIAL_RESET:
>>  		r = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
>>  		break;
>> +	case KVM_S390_GET_SREGS2: {
>> +		struct kvm_s390_sregs2 sregs2;
>> +
>> +		sregs2.prefix = vcpu->arch.sie_block->prefix;
>> +		sregs2.gbea = vcpu->arch.sie_block->gbea;
>> +		sregs2.cputm = vcpu->arch.sie_block->cputm;
>> +		sregs2.ckc = vcpu->arch.sie_block->ckc;
>> +		sregs2.todpr = vcpu->arch.sie_block->todpr;
>> +		r = copy_to_user(argp, &sregs2, sizeof(sregs2));
> 
> Need to return -EFAULT, not the number of remaining bytes to copy.

Will fix.

>> +	case KVM_S390_SET_SREGS2: {
>> +		struct kvm_s390_sregs2 sregs2;
>> +
>> +		vcpu->arch.sie_block->prefix = sregs2.prefix;
>> +		vcpu->arch.sie_block->gbea = sregs2.gbea;
>> +		vcpu->arch.sie_block->cputm = sregs2.cputm;
>> +		vcpu->arch.sie_block->ckc = sregs2.ckc;
>> +		vcpu->arch.sie_block->todpr = sregs2.todpr;
> 
> Copying uninitialized data.
> 
>> +		r = copy_from_user(&sregs2, argp, sizeof(sregs2));
> 
> Then initializing it.

Hmm, a brown paper bag bug. Since life migration does not yet work
I only tested the get case (via dump). Sorry about that.

> 
>> +	        vcpu->arch.sie_block->ihcpu     = 0xffff;
> 
> What's this?

tlb flush. Necessary after setting the prefix register.

Thanks