Re: [PATCH 5/5] KVM: Add support for in-kernel LAPIC model

["Gregory Haskins" <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>]

>>> On Sun, Apr 22, 2007 at  5:04 AM, in message <462B250E.6050603-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote: 
> Gregory Haskins wrote:
>> Signed- off- by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>> ---
>>
>>  drivers/kvm/Makefile   |    2 
>>  drivers/kvm/kernint.c  |  168 +++++
>>  drivers/kvm/kvm.h      |   14 
>>  drivers/kvm/kvm_main.c |  142 +++++
>>  drivers/kvm/lapic.c    | 1472 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/kvm.h    |   16 -
>>  6 files changed, 1808 insertions(+), 6 deletions(- )
>>
>> diff -- git a/drivers/kvm/Makefile b/drivers/kvm/Makefile
>> index 540afbc..1aad737 100644
>> ---  a/drivers/kvm/Makefile
>> +++ b/drivers/kvm/Makefile
>> @@ - 2,7 +2,7 @@
>>  # Makefile for Kernel- based Virtual Machine module
>>  #
>>  
>> - kvm- objs := kvm_main.o mmu.o x86_emulate.o userint.o
>> +kvm- objs := kvm_main.o mmu.o x86_emulate.o userint.o lapic.o kernint.o
>>  obj- $(CONFIG_KVM) += kvm.o
>>  kvm- intel- objs = vmx.o
>>  obj- $(CONFIG_KVM_INTEL) += kvm- intel.o
>> diff -- git a/drivers/kvm/kernint.c b/drivers/kvm/kernint.c
>> new file mode 100644
>> index 0000000..979a4aa
>> ---  /dev/null
>> +++ b/drivers/kvm/kernint.c
>> @@ - 0,0 +1,168 @@
>> +/*
>> + * Kernel Interrupt IRQ device
>> + *
>> + * Provides a model for connecting in- kernel interrupt resources to a VCPU.
>> + * 
>> + * A typical modern x86 processor has the concept of an internal Local- APIC 
>> + * and some external signal pins.  The way in which interrupts are injected 
> is
>> + * dependent on whether software enables the LAPIC or not.  When enabled,
>> + * interrupts are acknowledged through the LAPIC.  Otherwise they are 
> through
>> + * an externally connected PIC (typically an i8259 on the BSP)  
>> + *
>> + * Copyright (C) 2007 Novell
>> + *
>> + * Authors:
>> + *   Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top- level directory.
>> + *
>> + */
>> +
>> +#include "kvm.h"
>> +
>> +extern int kvm_kern_lapic_init(struct kvm_vcpu *vcpu, 
>> +			       struct kvm_irqdevice *irq_dev);
>> +struct kvm_kernint {
>> +	spinlock_t                    lock;
>> +	atomic_t                      ref_count;
>> +	struct kvm_vcpu              *vcpu;
>> +	struct kvm_irqdevice         *self_irq;
>> +	struct kvm_irqdevice         *ext_irq;
>> +	struct kvm_irqdevice          apic_irq;
>> +	struct kvm_lapic             *apic_dev;
>> +
>> +};
>>   
> 
> This is nice.  I don't think a ref count is really necessary though, as 
> the configuration is fairly static.

Ack.

> 
>>  struct kvm_stat {
>> @@ - 570,6 +574,9 @@ extern struct kvm_arch_ops *kvm_arch_ops;
>>  int kvm_init_arch(struct kvm_arch_ops *ops, struct module *module);
>>  void kvm_exit_arch(void);
>>  
>> +int kvm_apicbus_send(struct kvm *kvm, int dest, int trig_mode, int level, 
>> +		     int dest_mode, int delivery_mode, int vector);
>> +
>>   
> 
> Pack'em into a struct?

I see no reason why not.  I'll make this change.

> 
> 
> [... actual lapic code ...]
> 
>>  struct kvm_cpuid_entry {
>>  	__u32 function;
>>  	__u32 eax;
>> @@ - 284,6 +295,9 @@ struct kvm_signal_mask {
>>  #define KVM_CREATE_VCPU           _IO(KVMIO,  0x41)
>>  #define KVM_GET_DIRTY_LOG         _IOW(KVMIO, 0x42, struct kvm_dirty_log)
>>  #define KVM_SET_MEMORY_ALIAS      _IOW(KVMIO, 0x43, struct 
> kvm_memory_alias)
>> +#define KVM_ENABLE_KERNEL_PIC     _IOW(KVMIO, 0x44, __u32)
>> +#define KVM_ISA_INTERRUPT         _IOW(KVMIO, 0x45, struct kvm_interrupt)
>> +#define KVM_APIC_MSG		  _IOW(KVMIO, 0x46, struct kvm_apic_msg)
>>  
>>  /*
>>   * ioctls for vcpu fds
>> @@ - 302,5 +316,5 @@ struct kvm_signal_mask {
>>  #define KVM_SET_SIGNAL_MASK       _IOW(KVMIO,  0x8b, struct 
> kvm_signal_mask)
>>  #define KVM_GET_FPU               _IOR(KVMIO,  0x8c, struct kvm_fpu)
>>  #define KVM_SET_FPU               _IOW(KVMIO,  0x8d, struct kvm_fpu)
>> -
>> +#define KVM_APIC_RESET		  _IO(KVMIO,   0x8e)
>>  #endif
>>
>>   
> 
> You need to advertise the lapic ioctls via KVM_CHECK_EXTENSION.

Ack.

> 
> Overall looks good.

Thanks!
-Greg




-------------------------------------------------------------------------
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/