[PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[Paul Turner]

From: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

This just separates vmx/svm specific fields off kvm_vcpu into kvm_vmx_data 
and kvm_svm_data fields respectively.  Ideally these should be compiled 
out depending on target architecture, at least the waste is organized now. 
I didn't notice any svm specific fields, however I've included the empty 
struct for completeness, let me know if I've missed anything and I'll 
refresh.

p.s. let me know if pine mangles this email/the patch at all, I did a test 
mail to myself and it seemed to come out ok :)

- Paul

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 0a42608..6c15395 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -267,6 +267,41 @@ struct kvm_stat {
  	u32 efer_reload;
  };

+struct kvm_vmx_data {
+      	int msr_offset_efer;
+
+       #ifdef CONFIG_X86_64
+	        int msr_offset_kernel_gs_base;
+       #endif
+
+       struct vmx_host_state {
+		int loaded;
+		u16 fs_sel, gs_sel, ldt_sel;
+		int fs_gs_ldt_reload_needed;
+	} host_state;
+
+      	struct vmx_msr_entry *guest_msrs;
+	struct vmx_msr_entry *host_msrs;
+
+
+       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 */
+
+};
+
+struct kvm_svm_data {
+
+};
+
  struct kvm_vcpu {
  	struct kvm *kvm;
  	union {
@@ -301,12 +336,6 @@ #define NR_IRQ_WORDS KVM_IRQ_BITMAP_SIZE
  	u64 ia32_misc_enable_msr;
  	int nmsrs;
  	int save_nmsrs;
-	int msr_offset_efer;
-#ifdef CONFIG_X86_64
-	int msr_offset_kernel_gs_base;
-#endif
-	struct vmx_msr_entry *guest_msrs;
-	struct vmx_msr_entry *host_msrs;

  	struct kvm_mmu mmu;

@@ -325,11 +354,6 @@ #endif
  	char *guest_fx_image;
  	int fpu_active;
  	int guest_fpu_loaded;
-	struct vmx_host_state {
-		int loaded;
-		u16 fs_sel, gs_sel, ldt_sel;
-		int fs_gs_ldt_reload_needed;
-	} vmx_host_state;

  	int mmio_needed;
  	int mmio_read_completed;
@@ -345,23 +369,18 @@ #endif
  	sigset_t sigset;

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

  	int cpuid_nent;
  	struct kvm_cpuid_entry cpuid_entries[KVM_MAX_CPUID_ENTRIES];
+
+       union {
+       	struct kvm_vmx_data vmx;
+        	struct kvm_svm_data svm;
+       };
  };

struct kvm_mem_alias {
 	gfn_t base_gfn;
  	unsigned long npages;

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

Re: [PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[Avi Kivity]

Paul Turner wrote:
> From: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> This just separates vmx/svm specific fields off kvm_vcpu into 
> kvm_vmx_data and kvm_svm_data fields respectively.  Ideally these 
> should be compiled out depending on target architecture, at least the 
> waste is organized now. I didn't notice any svm specific fields, 
> however I've included the empty struct for completeness, let me know 
> if I've missed anything and I'll refresh.
>

This is much needed; thanks.

> p.s. let me know if pine mangles this email/the patch at all, I did a 
> test mail to myself and it seemed to come out ok :)
>

There are many spaces-instead-of-tabs violations.

>
> +
>  struct kvm_vcpu {
>      struct kvm *kvm;
>      union {

This union holds vmx/svm specific members that can be moved into the new 
structures.

> +
> +       union {
> +           struct kvm_vmx_data vmx;
> +            struct kvm_svm_data svm;
> +       };
>  };
>

If you make this

+       union {
+           struct kvm_vmx_data vmx[1];
+            struct kvm_svm_data svm[1];
+       };

then we can later change it to a zero-sized array with variable-size 
allocation, with no additional code changes.


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

Re: [PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[Jun Koi]

On 6/26/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> Paul Turner wrote:
> > From: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> >
> > This just separates vmx/svm specific fields off kvm_vcpu into
> > kvm_vmx_data and kvm_svm_data fields respectively.  Ideally these
> > should be compiled out depending on target architecture, at least the
> > waste is organized now. I didn't notice any svm specific fields,
> > however I've included the empty struct for completeness, let me know
> > if I've missed anything and I'll refresh.
> >
>
> This is much needed; thanks.
>
> > p.s. let me know if pine mangles this email/the patch at all, I did a
> > test mail to myself and it seemed to come out ok :)
> >
>
> There are many spaces-instead-of-tabs violations.
>
> >
> > +
> >  struct kvm_vcpu {
> >      struct kvm *kvm;
> >      union {
>
> This union holds vmx/svm specific members that can be moved into the new
> structures.
>
> > +
> > +       union {
> > +           struct kvm_vmx_data vmx;
> > +            struct kvm_svm_data svm;
> > +       };
> >  };
> >
>
> If you make this
>
> +       union {
> +           struct kvm_vmx_data vmx[1];
> +            struct kvm_svm_data svm[1];
> +       };
>
> then we can later change it to a zero-sized array with variable-size
> allocation, with no additional code changes.
>

Why do we want to make it array? I suppose that we only have 1 struct
(either vmx or svm) for each vcpu, no?


Thanks,
Jun

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

Re: [PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[Paul Turner]

Because zero sizing then 'over allocating' the struct on intel lets
you save space on svm, same extends obviously for other architectures
in future.

- Paul

On 6/25/07, Jun Koi <junkoi2004-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 6/26/07, Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> > Paul Turner wrote:
> > > From: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > >
> > > This just separates vmx/svm specific fields off kvm_vcpu into
> > > kvm_vmx_data and kvm_svm_data fields respectively.  Ideally these
> > > should be compiled out depending on target architecture, at least the
> > > waste is organized now. I didn't notice any svm specific fields,
> > > however I've included the empty struct for completeness, let me know
> > > if I've missed anything and I'll refresh.
> > >
> >
> > This is much needed; thanks.
> >
> > > p.s. let me know if pine mangles this email/the patch at all, I did a
> > > test mail to myself and it seemed to come out ok :)
> > >
> >
> > There are many spaces-instead-of-tabs violations.
> >
> > >
> > > +
> > >  struct kvm_vcpu {
> > >      struct kvm *kvm;
> > >      union {
> >
> > This union holds vmx/svm specific members that can be moved into the new
> > structures.
> >
> > > +
> > > +       union {
> > > +           struct kvm_vmx_data vmx;
> > > +            struct kvm_svm_data svm;
> > > +       };
> > >  };
> > >
> >
> > If you make this
> >
> > +       union {
> > +           struct kvm_vmx_data vmx[1];
> > +            struct kvm_svm_data svm[1];
> > +       };
> >
> > then we can later change it to a zero-sized array with variable-size
> > allocation, with no additional code changes.
> >
>
> Why do we want to make it array? I suppose that we only have 1 struct
> (either vmx or svm) for each vcpu, no?
>
>
> Thanks,
> Jun
>

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

Re: [PATCH 1/2] Separate vmx/svm fields from kvm_vcpu

[Avi Kivity]

>>
>> If you make this
>>
>> +       union {
>> +           struct kvm_vmx_data vmx[1];
>> +            struct kvm_svm_data svm[1];
>> +       };
>>
>> then we can later change it to a zero-sized array with variable-size
>> allocation, with no additional code changes.
>>
>
> Why do we want to make it array? I suppose that we only have 1 struct
> (either vmx or svm) for each vcpu, no?
>

If it's an array of size one, you can refer to the only element with 
vcpu->vmx->member.  If it's later changed to an array of size zero, you 
can keep the references the same, and change the allocations to 
kmalloc(sizeof(struct kvm_vcpu) +sizeof(struct kvm_vmx_data)) so that 
you don't allocate any unnecessary memory.


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