From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq4NI-0003cg-T5 for qemu-devel@nongnu.org; Mon, 09 Dec 2013 12:12:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vq4N3-0004J8-Sw for qemu-devel@nongnu.org; Mon, 09 Dec 2013 12:12:04 -0500 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:33331) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq4N3-0004IM-KU for qemu-devel@nongnu.org; Mon, 09 Dec 2013 12:11:49 -0500 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 9 Dec 2013 17:11:47 -0000 Date: Mon, 9 Dec 2013 18:11:40 +0100 From: Greg Kurz Message-ID: <20131209181140.2858c81a@bahia.local> In-Reply-To: <52603062-FAEC-4D6C-8F56-112488CCBAA9@suse.de> References: <20131125153556.12615.63598.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <52603062-FAEC-4D6C-8F56-112488CCBAA9@suse.de> 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 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. :) Thanks for the review. Cheers. -- Greg