From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MvtgW-0000mL-04 for qemu-devel@nongnu.org; Thu, 08 Oct 2009 10:09:36 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MvtgR-0000iP-FQ for qemu-devel@nongnu.org; Thu, 08 Oct 2009 10:09:35 -0400 Received: from [199.232.76.173] (port=49319 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MvtgR-0000iE-4q for qemu-devel@nongnu.org; Thu, 08 Oct 2009 10:09:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28278) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MvtgQ-00045o-Jp for qemu-devel@nongnu.org; Thu, 08 Oct 2009 10:09:30 -0400 Message-ID: <4ACDF297.9010303@redhat.com> Date: Thu, 08 Oct 2009 16:09:27 +0200 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH v2 4/9] provide in-kernel apic References: <1254953315-5761-1-git-send-email-glommer@redhat.com> <1254953315-5761-2-git-send-email-glommer@redhat.com> <1254953315-5761-3-git-send-email-glommer@redhat.com> <1254953315-5761-4-git-send-email-glommer@redhat.com> <1254953315-5761-5-git-send-email-glommer@redhat.com> <4ACDEF42.6020706@us.ibm.com> In-Reply-To: <4ACDEF42.6020706@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Glauber Costa , qemu-devel@nongnu.org, kvm-devel On 10/08/2009 03:55 PM, Anthony Liguori wrote: > Glauber Costa wrote: >> This patch provides kvm with an in-kernel apic. We are currently not >> enabling it. >> The code is heavily based on what's in qemu-kvm.git. >> >> Signed-off-by: Glauber Costa >> --- >> hw/apic.c | 135 >> +++++++++++++++++++++++++++++++++++++++++++++++++++-- >> kvm.h | 3 + >> target-i386/kvm.c | 18 +++++++ >> 3 files changed, 152 insertions(+), 4 deletions(-) >> >> diff --git a/hw/apic.c b/hw/apic.c >> index c89008e..5635607 100644 >> --- a/hw/apic.c >> +++ b/hw/apic.c >> @@ -299,7 +299,11 @@ void cpu_set_apic_base(CPUState *env, uint64_t val) >> #endif >> if (!s) >> return; >> - s->apicbase = (val & 0xfffff000) | >> + >> + if (kvm_enabled() && kvm_irqchip_in_kernel()) >> + s->apicbase = val; >> + else >> + s->apicbase = (val & 0xfffff000) | >> (s->apicbase & (MSR_IA32_APICBASE_BSP | >> MSR_IA32_APICBASE_ENABLE)); >> /* if disabled, cannot be enabled again */ >> if (!(val & MSR_IA32_APICBASE_ENABLE)) { >> @@ -497,6 +501,13 @@ void apic_init_reset(CPUState *env) >> s->wait_for_sipi = 1; >> >> env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP); >> + >> +#ifdef KVM_CAP_MP_STATE >> + if (kvm_enabled() && kvm_irqchip_in_kernel()) >> + env->mp_state >> + = env->halted ? KVM_MP_STATE_UNINITIALIZED : >> KVM_MP_STATE_RUNNABLE; >> +#endif > > I don't think CAP_MP_STATE should be treated as an optional feature. > >> +static int kvm_kernel_lapic_load_from_user(APICState *s) >> +{ >> + int r = 0; >> +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_I386) >> + struct kvm_lapic_state apic; >> + struct kvm_lapic_state *klapic = &apic; >> + int i; >> + >> + if (!(kvm_enabled() && kvm_irqchip_in_kernel())) >> + return 0; >> + >> + memset(klapic, 0, sizeof apic); >> + kapic_set_reg(klapic, 0x2, s->id << 24); >> + kapic_set_reg(klapic, 0x8, s->tpr); >> + kapic_set_reg(klapic, 0xd, s->log_dest << 24); >> + kapic_set_reg(klapic, 0xe, s->dest_mode << 28 | 0x0fffffff); >> + kapic_set_reg(klapic, 0xf, s->spurious_vec); >> + for (i = 0; i < 8; i++) { >> + kapic_set_reg(klapic, 0x10 + i, s->isr[i]); >> + kapic_set_reg(klapic, 0x18 + i, s->tmr[i]); >> + kapic_set_reg(klapic, 0x20 + i, s->irr[i]); >> + } >> + kapic_set_reg(klapic, 0x28, s->esr); >> + kapic_set_reg(klapic, 0x30, s->icr[0]); >> + kapic_set_reg(klapic, 0x31, s->icr[1]); >> + for (i = 0; i < APIC_LVT_NB; i++) >> + kapic_set_reg(klapic, 0x32 + i, s->lvt[i]); >> + kapic_set_reg(klapic, 0x38, s->initial_count); >> + kapic_set_reg(klapic, 0x3e, s->divide_conf); >> + >> + r = kvm_set_lapic(s->cpu_env, klapic); >> +#endif >> + return r; >> +} > > You should probably just setup VMState such that it directly saves > kvm_lapic_state and then have the pre/post functions call the kernel > ioctls to sync it. There's not a whole lot of point switching the > state between two different structures. It ensures the two models are compatible. Since they're the same device from the point of view of the guest, there's no reason for them to have different representations or to be incompatible. -- error compiling committee.c: too many arguments to function