From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Tue, 17 Jun 2014 10:22:32 +0000 Subject: Re: [PATCH 5/6] KVM: PPC: Book3S HV: Access XICS in BE Message-Id: <53A016E8.1060401@suse.de> List-Id: References: <1402482831-8941-1-git-send-email-agraf@suse.de> <1402482831-8941-6-git-send-email-agraf@suse.de> <20140617010204.GB16554@iris.ozlabs.ibm.com> <539FFE39.1000701@suse.de> In-Reply-To: <539FFE39.1000701@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, anton@samba.org On 17.06.14 10:37, Alexander Graf wrote: > > On 17.06.14 03:02, Paul Mackerras wrote: >> On Wed, Jun 11, 2014 at 12:33:50PM +0200, Alexander Graf wrote: >>> On the exit path from the guest we check what type of interrupt we >>> received >>> if we received one. This means we're doing hardware access to the >>> XICS interrupt >>> controller. >>> >>> However, when running on a little endian system, this access is byte >>> reversed. >>> >>> So let's make sure to swizzle the bytes back again and virtuall make >>> XICS >>> accesses big endian. >> ... >> >>> @@ -2241,7 +2253,8 @@ kvmppc_read_intr: >>> 42: /* It's not an IPI and it's for the host, stash it in the PACA >>> * before exit, it will be picked up by the host ICP driver >>> */ >>> - stw r0, HSTATE_SAVED_XIRR(r13) >>> + li r4, HSTATE_SAVED_XIRR >>> + STWX_BE r0, r13, r4 >> This is a paca field, not something mandated by PAPR or shared with >> the guest, so why do we need to keep it BE? If you do make it BE, >> don't you also need to fix kvmppc_get_xics_latch()? > > Yikes. Yes. Thanks a lot for the catch! Eh, no. What we do is we read (good on BE, byte reversed) into r0. Then we swab32() from r0 to r3 on LE, mr from r0 to r3 on BE. r3 gets truncated along the way. The reason we maintain r0 as wrong-endian is that we write it back using the cache inhibited stwcix instruction: > stwcix r0, r6, r7 /* EOI it */ So during the lifetime of r0 as XIRR it's always byte-reversed on LE. That's why we store it using STWX_BE into hstate, because that's the time when we actually swab32() it for further interpretation. Alternatively I could clobber a different register and maintain the byte swapped variant in there if you prefer. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 5/6] KVM: PPC: Book3S HV: Access XICS in BE Date: Tue, 17 Jun 2014 12:22:32 +0200 Message-ID: <53A016E8.1060401@suse.de> References: <1402482831-8941-1-git-send-email-agraf@suse.de> <1402482831-8941-6-git-send-email-agraf@suse.de> <20140617010204.GB16554@iris.ozlabs.ibm.com> <539FFE39.1000701@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, anton@samba.org To: Paul Mackerras Return-path: In-Reply-To: <539FFE39.1000701@suse.de> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 17.06.14 10:37, Alexander Graf wrote: > > On 17.06.14 03:02, Paul Mackerras wrote: >> On Wed, Jun 11, 2014 at 12:33:50PM +0200, Alexander Graf wrote: >>> On the exit path from the guest we check what type of interrupt we >>> received >>> if we received one. This means we're doing hardware access to the >>> XICS interrupt >>> controller. >>> >>> However, when running on a little endian system, this access is byte >>> reversed. >>> >>> So let's make sure to swizzle the bytes back again and virtuall make >>> XICS >>> accesses big endian. >> ... >> >>> @@ -2241,7 +2253,8 @@ kvmppc_read_intr: >>> 42: /* It's not an IPI and it's for the host, stash it in the PACA >>> * before exit, it will be picked up by the host ICP driver >>> */ >>> - stw r0, HSTATE_SAVED_XIRR(r13) >>> + li r4, HSTATE_SAVED_XIRR >>> + STWX_BE r0, r13, r4 >> This is a paca field, not something mandated by PAPR or shared with >> the guest, so why do we need to keep it BE? If you do make it BE, >> don't you also need to fix kvmppc_get_xics_latch()? > > Yikes. Yes. Thanks a lot for the catch! Eh, no. What we do is we read (good on BE, byte reversed) into r0. Then we swab32() from r0 to r3 on LE, mr from r0 to r3 on BE. r3 gets truncated along the way. The reason we maintain r0 as wrong-endian is that we write it back using the cache inhibited stwcix instruction: > stwcix r0, r6, r7 /* EOI it */ So during the lifetime of r0 as XIRR it's always byte-reversed on LE. That's why we store it using STWX_BE into hstate, because that's the time when we actually swab32() it for further interpretation. Alternatively I could clobber a different register and maintain the byte swapped variant in there if you prefer. Alex