Zhang, Xiantao wrote:
> >From 12457e0fb85ef32f1a1f808be294bebe8d22667c Mon Sep 17 00:00:00 2001
> From: Zhang xiantao <[EMAIL PROTECTED]>
> Date: Fri, 12 Oct 2007 13:29:30 +0800
> Subject: [PATCH] Split kvm_vcpu to support new archs. Define a new sub
> field
> kvm_arch_vcpu to hold arch-specific sections.
>
> I am not sure data fields related to mmu should put under kvm_arch_vcpu
> or not, because 
> IA64 side doesn't need them, and only need kvm module to allocate memory
> for guests.
>
>   


The patch is a good start, but too big... I think there are three
possible approaches to make the split:

1: kvm_vcpu is common, with arch specific fields in a member

struct kvm_arch_vcpu {
... x86 specific fields ...
};

struct kvm_vcpu {
... common fields...
struct kvm_arch_vcpu arch;
};

2. kvm_vcpu is arch specific, with common fields in a member

struct kvm_vcpu_common {
... common fields ...
};

struct kvm_vcpu {
struct kvm_vcpu_common common;
... x86 specific fields ...
};

3. kvm_vcpu contains both common and arch specific fields:

#define KVM_VCPU_COMMON_FIELDS \
struct kvm *kvm; \
struct preempt_notifier *preempt_notifier; \
...

struct kvm_vcpu {
KVM_VCPU_COMMON_FIELDS
... x86 specific fields ....
};

I prefer the second approach to the first one since most fields are arch
specific, and this allows most fields to be accessed directly without
"arch." in front of every field. However I'd like to start with the
third approach since it is least intrusive (the downside is that it is a
little dirty from a C point of view).

I think you left out some of the arch specific fields:

>  struct kvm_vcpu {
>       struct kvm *kvm;
>       struct preempt_notifier preempt_notifier;
>       int vcpu_id;
>       struct mutex mutex;
>       int   cpu;
> -     u64 host_tsc;
>       struct kvm_run *run;
>   

>       int interrupt_window_open;
>   

arch specific, though maybe it's close enough to generic

>       int guest_mode;
>       unsigned long requests;
>   

>       unsigned long irq_summary; /* bit vector: 1 per word in
> irq_pending */
>       DECLARE_BITMAP(irq_pending, KVM_NR_INTERRUPTS);
>   

irq_pending and irq_summary are probably arch specific

> -     unsigned long regs[NR_VCPU_REGS]; /* for rsp:
> vcpu_load_rsp_rip() */
> -     unsigned long rip;      /* needs vcpu_load_rsp_rip() */
> -
> -     unsigned long cr0;
> -     unsigned long cr2;
> -     unsigned long cr3;
> -     unsigned long cr4;
> -     unsigned long cr8;
> -     u64 pdptrs[4]; /* pae */
> -     u64 shadow_efer;
> -     u64 apic_base;
> -     struct kvm_lapic *apic;    /* kernel irqchip context */
>  #define VCPU_MP_STATE_RUNNABLE          0
>  #define VCPU_MP_STATE_UNINITIALIZED     1
>  #define VCPU_MP_STATE_INIT_RECEIVED     2
> @@ -339,7 +329,6 @@ struct kvm_vcpu {
>  #define VCPU_MP_STATE_HALTED            4
>       int mp_state;
>       int sipi_vector;
>   

mp_state (probably) and sipi_vector (certainly)

> -     u64 ia32_misc_enable_msr;
>  
>       struct kvm_mmu mmu;
>   

mmu

>  
> @@ -354,10 +343,6 @@ struct kvm_vcpu {
>  
>       struct kvm_guest_debug guest_debug;
>  
> -     struct i387_fxsave_struct host_fx_image;
> -     struct i387_fxsave_struct guest_fx_image;
> -     int fpu_active;
> -     int guest_fpu_loaded;
>  
>       int mmio_needed;
>       int mmio_read_completed;
> @@ -365,7 +350,6 @@ struct kvm_vcpu {
>       int mmio_size;
>       unsigned char mmio_data[8];
>       gpa_t mmio_phys_addr;
> -     gva_t mmio_fault_cr2;
>       struct kvm_pio_request pio;
>       void *pio_data;
>   

pio is x86 specific

>       wait_queue_head_t wq;
> @@ -375,24 +359,9 @@ struct kvm_vcpu {
>  
>       struct kvm_stat stat;
>  
> -     struct {
> -             int active;
> -             u8 save_iopl;
> -             struct kvm_save_segment {
> -                     u16 selector;
> -                     unsigned long base;
> -                     u32 limit;
> -                     u32 ar;
> -             } tr, es, ds, fs, gs;
> -     } rmode;
>       int halt_request; /* real mode on Intel only */
>   

this too

> +     struct kvm_arch_vcpu arch; /*Arch-specific fields*/
>  
> -     int cpuid_nent;
> -     struct kvm_cpuid_entry cpuid_entries[KVM_MAX_CPUID_ENTRIES];
> -
> -     /* emulate context */
> -
> -     struct x86_emulate_ctxt emulate_ctxt;
>  };
>  


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to