From mboxrd@z Thu Jan 1 00:00:00 1970 From: Carsten Otte Subject: Re: [PATCH 0/9] S390x KVM support Date: Thu, 22 Oct 2009 11:18:49 +0200 Message-ID: <4AE02379.4090603@de.ibm.com> References: <1255963059-10298-1-git-send-email-agraf@suse.de> <4ADDE7E3.9090601@de.ibm.com> <4AE0210D.9020409@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Graf , qemu-devel , uli@suse.de, Carsten Otte , hare@suse.de, KVM list To: Avi Kivity Return-path: Received: from mtagate2.de.ibm.com ([195.212.17.162]:43476 "EHLO mtagate2.de.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753837AbZJVJSd (ORCPT ); Thu, 22 Oct 2009 05:18:33 -0400 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate2.de.ibm.com (8.13.1/8.13.1) with ESMTP id n9M9IbSA026625 for ; Thu, 22 Oct 2009 09:18:37 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id n9M9IbHL2826486 for ; Thu, 22 Oct 2009 11:18:37 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n9M9IaO3020493 for ; Thu, 22 Oct 2009 11:18:37 +0200 In-Reply-To: <4AE0210D.9020409@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: >> @@ -116,6 +117,11 @@ >> __u64 cr8; >> __u64 apic_base; >> >> +#ifdef CONFIG_S390 >> + /* the processor status word for s390 */ >> + __u64 psw_mask; /* psw upper half */ >> + __u64 psw_addr; /* psw lower half */ >> +#endif > > Doesn't this break backward compatibility by changing the structure? Yes, but with a zero user base I think it's okay. I'd update our userspace. Once we pull CONFIG_EXPERIMENTAL we keep the API stable. > Additionally, CONFIG_ in public headers are frowned upon as > non-portable. A workaround is to #define __KVM_S390 in and > depend on that. Yea, that's better. >> --- kvm.orig/arch/s390/kvm/kvm-s390.c 2009-10-20 15:01:02.000000000 >> +0200 >> +++ kvm/arch/s390/kvm/kvm-s390.c 2009-10-20 18:13:45.000000000 +0200 >> @@ -421,7 +421,8 @@ >> if (atomic_read(&vcpu->arch.sie_block->cpuflags) & CPUSTAT_RUNNING) >> rc = -EBUSY; >> else >> - vcpu->arch.sie_block->gpsw = psw; >> + vcpu->run->psw_mask = psw.mask; >> + vcpu->run->psw_addr = psw.addr; > > It's traditional to add braces around multi-line else blocks. This is a plain bug, will fix. > I'd also appreciate an explanation of what this is all about. The processor status word does contain various bits about the CPU's state, such as interrupt mask bits, current address space, and the current instruction address. The status is kept in the in-kernel sie control block data structure and has so far only been mirrored into kvm_run during exit_reason == s390_sieic exits because user space needs to work on it. It was never part of get_regs/set_regs and friends as performance optimization: it's needed on almost every exit, having it in kvm_run saves doing syscalls. The gdb stub requires an up-to-date copy at every exit, and therefore the patch moves it out of the union and updates it at all userland exits.