From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 4/4] KVM: Add support for in-kernel LAPIC model Date: Mon, 07 May 2007 13:17:41 +0300 Message-ID: <463EFCC5.8050408@qumranet.com> References: <20070502212713.16738.8133.stgit@novell1.haskins.net> <20070502214330.16738.51436.stgit@novell1.haskins.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Gregory Haskins Return-path: In-Reply-To: <20070502214330.16738.51436.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Gregory Haskins wrote: > + > +struct kvm_kernint { > + struct kvm_vcpu *vcpu; > + struct kvm_irqdevice *self_irq; > + struct kvm_irqdevice *ext_irq; > + struct kvm_irqdevice apic_irq; > + > +}; > Can you elaborate on what these are? I sort of understand ext_irq and apic_irq, but what is self_irq? And why have the vcpu member? > + > +static struct kvm_irqdevice *get_irq_dev(struct kvm_kernint *s) > +{ > + struct kvm_irqdevice *dev; > + > + if (kvm_lapic_enabled(s->vcpu)) > + dev = &s->apic_irq; > + else > + dev = s->ext_irq; > + > + if (!dev) > + kvm_crash_guest(s->vcpu->kvm); > Can this happen? Doesn't there always have to be an external interrupt controller when the lapic is disabled? > + > +static void kvm_ext_intr(struct kvm_irqsink *this, > + struct kvm_irqdevice *dev, > + kvm_irqpin_t pin) > +{ > + struct kvm_kernint *s = (struct kvm_kernint*)this->private; > + > + /* > + * If the EXTINT device sent us an interrupt, forward it to the LINT0 > + * pin of the LAPIC > + */ > + if (pin != kvm_irqpin_localint) > + return; > Whitespace. > + > + /* > + * "irq 0" = LINT0, 1 = LINT1 > + */ > + kvm_irqdevice_set_pin(&s->apic_irq, 0, 1); > +} > + > +int kvm_kernint_init(struct kvm_vcpu *vcpu) > +{ > + struct kvm_irqdevice *irqdev = &vcpu->irq.dev; > + struct kvm_kernint *s; > + > + s = kzalloc(sizeof(*s), GFP_KERNEL); > + if (!s) > + return -ENOMEM; > Whitespace. > + > + s->vcpu = vcpu; > + > + /* > + * Configure the irqdevice interface > + */ > + irqdev->ack = kernint_irqdev_ack; > + irqdev->set_pin = kernint_irqdev_set_pin; > + irqdev->destructor = kernint_irqdev_destructor; > + > + irqdev->private = s; > + s->self_irq = irqdev; > + > + /* > + * Configure the EXTINT device if this is the BSP processor > + */ > + if (!vcpu_slot(vcpu)) { > + struct kvm_irqsink sink = { > + .set_intr = kvm_ext_intr, > + .private = s > + }; > + > + s->ext_irq = &vcpu->kvm->isa_irq; > + kvm_irqdevice_register_sink(s->ext_irq, &sink); > + } > + > + /* > + * Configure the LAPIC device > + */ > + kvm_irqdevice_init(&s->apic_irq); > + > + { > + struct kvm_irqsink sink = { > + .set_intr = kvm_apic_intr, > + .private = s > + }; > + > + kvm_irqdevice_register_sink(&s->apic_irq, &sink); > + } > Just move the variable definition to the top to avoid the wierd indentation. > + > +/* > + *----------------------------------------------------------------------- > + * KERNEL-TIMERS > + * > + * Unfortunately we really need HRTIMERs to do this right, but they might > + * not exist on olders kernels (sigh). So we roughly abstract this interface > + * to support nanosecond resolution, and then emulate it as best we can if > + * the real HRTIMERs arent available > + *----------------------------------------------------------------------- > + */ > The correct way to use non-hrtimer kernels is to write the hrtimer api in terms of the old api, in external-module-compat.h. > + > +struct kvm_apic_timer { > + int (*function)(void *private); > + void *private; > +#ifdef KVM_NO_HRTIMER > + struct timer_list dev; > +#else > + struct hrtimer dev; > +#endif > +}; > ... for example: #define hrtimer timer_list I'd just drop it for now; it makes the code hard to read. We can re-add it later. [I didn't audit the lapic code] Where's vcpu->cr8 gone? -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/