From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58180) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WODBF-0001Vv-Pt for qemu-devel@nongnu.org; Thu, 13 Mar 2014 17:28:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WODBB-0006Z0-Dv for qemu-devel@nongnu.org; Thu, 13 Mar 2014 17:28:45 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39512 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WODBB-0006Yv-4n for qemu-devel@nongnu.org; Thu, 13 Mar 2014 17:28:41 -0400 Message-ID: <53222304.3050902@suse.de> Date: Thu, 13 Mar 2014 22:28:36 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1394125778-18746-1-git-send-email-james.hogan@imgtec.com> <1394125778-18746-7-git-send-email-james.hogan@imgtec.com> In-Reply-To: <1394125778-18746-7-git-send-email-james.hogan@imgtec.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 6/9] target-mips: kvm: Add main KVM support for MIPS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: James Hogan , qemu-devel@nongnu.org Cc: Peter Maydell , kvm@vger.kernel.org, Gleb Natapov , Sanjay Lal , Paolo Bonzini , Aurelien Jarno Am 06.03.2014 18:09, schrieb James Hogan: > From: Sanjay Lal >=20 > Implement the main KVM arch API for MIPS. >=20 > Signed-off-by: Sanjay Lal > Signed-off-by: James Hogan > Cc: Aurelien Jarno > Cc: Gleb Natapov > Cc: Paolo Bonzini > Cc: Andreas F=C3=A4rber > Cc: Peter Maydell > --- > Changes in v3: > - s/dprintf/DPRINTF/ (Andreas F=C3=A4rber). > - Use "cs" rather than "cpu" or "env" for CPUState variable names > (Andreas F=C3=A4rber). > - Use CPUMIPSState rather than CPUArchState (Andreas F=C3=A4rber). > - Pass MIPSCPU to cpu_mips_io_interrupts_pending() rather than > CPUMIPSState (Andreas F=C3=A4rber). > - Remove spurious parentheses around cpu_mips_io_interrupts_pending() > call (Andreas F=C3=A4rber). > - Pass MIPSCPU to kvm_mips_set_[ipi_]interrupt (Andreas F=C3=A4rber). > - Make use of error_report (Andreas F=C3=A4rber) and clean up error me= ssages > a little to include __func__. > - Remove inline kvm_mips_{put,get}_one_[ul]reg() declarations from > kvm_mips.h. They're only used in target-mips/kvm.c anyway. > - Make kvm_arch_{put,get}_registers static within target-mips/kvm.c an= d > remove from kvm_mips.h. > - Set sigmask length to 16 from kvm_arch_init() since MIPS Linux has > 128 signals. This is better than cluttering kvm_all.c with TARGET_* > ifdefs (Peter Maydell). >=20 > Changes in v2: > - Expand commit message > - Checkpatch cleanups. > - Some interrupt bug fixes from Yann Le Du > - Add get/set register functionality from Yann Le Du > - Use new 64 bit compatible ABI from Cavium from Sanjay Lal > > - Add dummy kvm_arch_init_irq_routing() > The common KVM code insists on calling kvm_arch_init_irq_routing() a= s > soon as it sees kernel header support for it (regardless of whether > QEMU supports it). Provide a dummy function to satisfy this. > - Remove request_interrupt_window code (Peter Maydell) > --- > target-mips/kvm.c | 472 +++++++++++++++++++++++++++++++++++++++++= ++++++++ > target-mips/kvm_mips.h | 19 ++ > 2 files changed, 491 insertions(+) > create mode 100644 target-mips/kvm.c > create mode 100644 target-mips/kvm_mips.h >=20 > diff --git a/target-mips/kvm.c b/target-mips/kvm.c > new file mode 100644 > index 0000000..0ec343d > --- /dev/null > +++ b/target-mips/kvm.c [...] > +static inline int kvm_mips_put_one_reg(CPUState *cs, int reg_id, int32= *addr) Did you mean int32_t? > +{ > + __u64 val64 =3D (__u64)*addr; > + struct kvm_one_reg cp0reg =3D { > + .id =3D reg_id, > + .addr =3D (__u64)((target_ulong)&val64) > + }; > + > + return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg); > +} > + > +static inline int kvm_mips_put_one_ulreg(CPUState *cs, int reg_id, > + target_ulong *addr) > +{ > + __u64 val64 =3D (__u64)*addr; > + struct kvm_one_reg cp0reg =3D { > + .id =3D reg_id, > + .addr =3D (__u64)((target_ulong)&val64) > + }; > + > + return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg); > +} > + > +static inline int kvm_mips_get_one_reg(CPUState *cs, int reg_id, int32= *addr) int32_t? > +{ > + int ret; > + __u64 val64 =3D 0; > + struct kvm_one_reg cp0reg =3D { > + .id =3D reg_id, > + .addr =3D (__u64)((target_ulong)&val64) > + }; > + > + ret =3D kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg); > + if (ret < 0) { > + return ret; > + } > + > + *addr =3D (int32)val64; int32_t? > + return ret; > +} [snip] int32 is a type used in softfloat that has weird at-least-as-wide semantics and bit us in the past. I'm not sure if we have a policy about __u64 etc. in KVM code. Since it'll be Linux-only I don't see problems currently; for cross-platform parts we prefer uint64_t. Suggest to leave as is unless told otherwise. Otherwise looking good now, thanks for the CPU cleanups! We just had another round of CPU refactorings go in today, but I don't spot a conflict in this patch. Please rebase your local branch to verify. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg