From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MvtNH-0003Dc-TT for qemu-devel@nongnu.org; Thu, 08 Oct 2009 09:49:44 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MvtND-00036U-5O for qemu-devel@nongnu.org; Thu, 08 Oct 2009 09:49:43 -0400 Received: from [199.232.76.173] (port=58856 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MvtND-00036H-1I for qemu-devel@nongnu.org; Thu, 08 Oct 2009 09:49:39 -0400 Received: from e1.ny.us.ibm.com ([32.97.182.141]:35260) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MvtNC-00086T-Hx for qemu-devel@nongnu.org; Thu, 08 Oct 2009 09:49:38 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e1.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id n98DmO8C012991 for ; Thu, 8 Oct 2009 09:48:24 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id n98DnYBh251672 for ; Thu, 8 Oct 2009 09:49:36 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n98DnYNV025654 for ; Thu, 8 Oct 2009 09:49:34 -0400 Message-ID: <4ACDEDEC.60706@us.ibm.com> Date: Thu, 08 Oct 2009 08:49:32 -0500 From: Anthony Liguori MIME-Version: 1.0 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> In-Reply-To: <1254953315-5761-4-git-send-email-glommer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v2 3/9] provide in-kernel ioapic List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: qemu-devel@nongnu.org, kvm-devel , Avi Kivity Glauber Costa wrote: > This patch provides kvm with an in-kernel ioapic. We are currently not enabling it. > The code is heavily based on what's in qemu-kvm.git. > It really ought to be it's own file and own device model. Having the code mixed in with ioapic.c is confusing because it's unclear what code is in use when the in-kernel model is used. > @@ -193,6 +194,79 @@ static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t va > } > } > > +static int kvm_kernel_ioapic_load_from_user(IOAPICState *s) > +{ > + int r = 0; > +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_I386) > No point in checking the KVM_CAP_IRQCHIP. Just require it during build. Otherwise, !KVM_CAP_IRQCHIP is dead code since I'm sure noone is actually testing kernels that old with modern qemu. There's no point in restricting to I386 either. > + struct kvm_irqchip chip; > + struct kvm_ioapic_state *kioapic; > + int i; > + > + if (!(kvm_enabled() && kvm_irqchip_in_kernel())) > + return 0; > + > + chip.chip_id = KVM_IRQCHIP_IOAPIC; > + kioapic = &chip.chip.ioapic; > + > + kioapic->id = s->id; > + kioapic->ioregsel = s->ioregsel; > + kioapic->base_address = IOAPIC_DEFAULT_BASE_ADDRESS; > + kioapic->irr = s->irr; > + for (i = 0; i < IOAPIC_NUM_PINS; i++) { > + kioapic->redirtbl[i].bits = s->ioredtbl[i]; > + } > + > + r = kvm_set_irqchip(&chip); > +#endif > + return r; > +} > + > +static void kvm_kernel_ioapic_save_to_user(IOAPICState *s) > +{ > +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_I386) > + struct kvm_irqchip chip; > + struct kvm_ioapic_state *kioapic; > + int i; > + > + if (!(kvm_enabled() && kvm_irqchip_in_kernel())) > + return; > + chip.chip_id = KVM_IRQCHIP_IOAPIC; > + kvm_get_irqchip(&chip); > + kioapic = &chip.chip.ioapic; > + > + s->id = kioapic->id; > + s->ioregsel = kioapic->ioregsel; > + s->irr = kioapic->irr; > + for (i = 0; i < IOAPIC_NUM_PINS; i++) { > + s->ioredtbl[i] = kioapic->redirtbl[i].bits; > + } > +#endif > +} > + > +static void ioapic_pre_save(void *opaque) > +{ > + IOAPICState *s = (void *)opaque; > + > + kvm_kernel_ioapic_save_to_user(s); > +} > + > +static int ioapic_pre_load(void *opaque) > +{ > + IOAPICState *s = opaque; > + > + /* in case we are doing version 1, we just set these to sane values */ > + s->irr = 0; > + return 0; > +} > + > +static int ioapic_post_load(void *opaque, int version_id) > +{ > + IOAPICState *s = opaque; > + > + return kvm_kernel_ioapic_load_from_user(s); > +} > + > + > static const VMStateDescription vmstate_ioapic = { > .name = "ioapic", > .version_id = 2, > @@ -205,7 +279,10 @@ static const VMStateDescription vmstate_ioapic = { > VMSTATE_UINT32_V(irr, IOAPICState, 2), > VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS), > VMSTATE_END_OF_LIST() > - } > + }, > + .pre_load = ioapic_pre_load, > + .post_load = ioapic_post_load, > + .pre_save = ioapic_pre_save, > }; > The in kernel apic should be a separate device model with a separate savevm section. They are different devices and there's no real advantage to pretending like they're the same device. > static CPUReadMemoryFunc * const ioapic_mem_read[3] = { > diff --git a/kvm-all.c b/kvm-all.c > index 48ae26c..d795285 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -411,6 +411,26 @@ int kvm_check_extension(KVMState *s, unsigned int extension) > return ret; > } > > +#ifdef KVM_CAP_IRQCHIP > +int kvm_set_irqchip(struct kvm_irqchip *chip) > +{ > + if (!kvm_state->irqchip_in_kernel) { > + return 0; > + } > + > + return kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip); > +} > irqchip_in_kernel ought to disappear. -- Regards, Anthony Liguori