From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Subject: Re: [Qemu-devel] [PATCH v3 6/9] target-mips: kvm: Add main KVM support for MIPS Date: Thu, 13 Mar 2014 22:28:36 +0100 Message-ID: <53222304.3050902@suse.de> References: <1394125778-18746-1-git-send-email-james.hogan@imgtec.com> <1394125778-18746-7-git-send-email-james.hogan@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Peter Maydell , kvm@vger.kernel.org, Gleb Natapov , Sanjay Lal , Paolo Bonzini , Aurelien Jarno To: James Hogan , qemu-devel@nongnu.org Return-path: Received: from cantor2.suse.de ([195.135.220.15]:48662 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754167AbaCMV2k (ORCPT ); Thu, 13 Mar 2014 17:28:40 -0400 In-Reply-To: <1394125778-18746-7-git-send-email-james.hogan@imgtec.com> Sender: kvm-owner@vger.kernel.org List-ID: 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)= =2E > - Make use of error_report (Andreas F=C3=A4rber) and clean up error = messages > 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 = and > 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()= as > soon as it sees kernel header support for it (regardless of whethe= r > 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, int= 32 *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, int= 32 *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= =BCrnberg