From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35820) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqRFe-0006i2-BD for qemu-devel@nongnu.org; Tue, 10 Dec 2013 12:37:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VqRFU-0003eq-AW for qemu-devel@nongnu.org; Tue, 10 Dec 2013 12:37:42 -0500 Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:46491) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VqRFU-0003eX-1y for qemu-devel@nongnu.org; Tue, 10 Dec 2013 12:37:32 -0500 Received: from /spool/local by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 10 Dec 2013 17:37:29 -0000 Date: Tue, 10 Dec 2013 18:37:23 +0100 From: Greg Kurz Message-ID: <20131210183723.5db9b4cf@bahia.local> In-Reply-To: References: <20131125153556.12615.63598.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <52603062-FAEC-4D6C-8F56-112488CCBAA9@suse.de> <20131209181140.2858c81a@bahia.local> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-ppc: ppc64 target's virtio can be either endian. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Rusty Russell , "qemu-ppc@nongnu.org list:PowerPC" , QEMU Developers On Mon, 9 Dec 2013 18:18:26 +0100 Alexander Graf wrote: > > On 09.12.2013, at 18:11, Greg Kurz wrote: > > > On Mon, 9 Dec 2013 16:33:59 +0100 > > Alexander Graf wrote: > >> > >> On 25.11.2013, at 16:35, Greg Kurz wrote: > >> > >>> We base it on the OS endian, as reflected by the endianness of the > >>> interrupt vectors (handled through the ILE bit in the LPCR register). > >>> > >>> This patch does two things: > >>> - make LPCR a KVM register > >>> - implement virtio_get_byteswap() over LPCR > >>> > >>> This patch requires to have the following defined in the linux > >>> headers: > >>> > >>> $ grep LPCR linux-headers/asm-powerpc/kvm.h > >>> #define KVM_REG_PPC_LPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | > >>> 0xb5) > >>> > >>> Suggested-by: Benjamin Herrenschmidt > >>> Signed-off-by: Rusty Russell > >>> Signed-off-by: Greg Kurz > >>> --- > >>> target-ppc/kvm.c | 4 ++++ > >>> target-ppc/misc_helper.c | 14 ++++++++++++++ > >>> 2 files changed, 18 insertions(+) > >>> > >>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > >>> index 10d0cd9..b450a22 100644 > >>> --- a/target-ppc/kvm.c > >>> +++ b/target-ppc/kvm.c > >>> @@ -869,6 +869,8 @@ int kvm_arch_put_registers(CPUState *cs, int > >>> level) DPRINTF("Warning: Unable to set VPA information to > >>> KVM\n"); } > >>> } > >>> + > >>> + kvm_put_one_spr(cs, KVM_REG_PPC_LPCR, SPR_LPCR); > >>> #endif /* TARGET_PPC64 */ > >>> } > >>> > >>> @@ -1091,6 +1093,8 @@ int kvm_arch_get_registers(CPUState *cs) > >>> DPRINTF("Warning: Unable to get VPA information from > >>> KVM\n"); } > >>> } > >>> + > >>> + kvm_get_one_spr(cs, KVM_REG_PPC_LPCR, SPR_LPCR); > >>> #endif > >>> } > >>> > >>> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c > >>> index 616aab6..0e0743a 100644 > >>> --- a/target-ppc/misc_helper.c > >>> +++ b/target-ppc/misc_helper.c > >>> @@ -20,6 +20,8 @@ > >>> #include "helper.h" > >>> > >>> #include "helper_regs.h" > >>> +#include "hw/virtio/virtio.h" > >>> +#include "sysemu/kvm.h" > >>> > >>> /*****************************************************************************/ > >>> /* SPR accesses */ > >>> @@ -116,3 +118,15 @@ void ppc_store_msr(CPUPPCState *env, target_ulong > >>> value) { > >>> hreg_store_msr(env, value, 0); > >>> } > >>> + > >>> +bool virtio_get_byteswap(void) > >>> +{ > >>> + PowerPCCPU *cp = POWERPC_CPU(first_cpu); > >>> + CPUPPCState *env = &cp->env; > >>> + > >>> + if (kvm_enabled()) { > >>> + kvm_arch_get_registers(first_cpu); > >> > >> This function is not defined when CONFIG_KVM is disabled. > >> > >> Please do > >> > >> cpu_synchronize_state(first_cpu); > >> > >> instead. > >> > > > > Oups... I'll fix that. > > > >> Also can't virtio_get_byteswap pass in the CPU pointer of the CPU > >> that's calling in this moment? I'm not sure how racy it is to > >> synchronize the first cpu while we're not in the first cpu's execution > >> thread. > >> > > > > I kept the choices made by Rusty in the original serie. According to > > these, I am not sure if we can use current_cpu: > > > > https://lists.gnu.org/archive/html/qemu-devel/2013-08/msg01156.html > > https://lists.gnu.org/archive/html/qemu-devel/2013-08/msg01504.html > > > >> Either way, if we do this "right" we don't even have to jump through > >> these hoops, as little endian setting simply happens steered from QEMU, > >> so QEMU will have all knowledge about the guest's little endian mode > >> without the need to synchronize any state. > >> > > > > Sure. This is just primary work to get virtio working for cross-endian > > cases. :) > > Yeah, but so far LE switching code is not upstream in KVM - and for TCG > we don't need the cpu_synchronize_state() call at all. > > > Alex > Sure, but for TCG, cpu_synchronize_state() does nothing since KVM is not enabled... what would be wrong with that ? Cheers. -- Greg