[PATCH 0/3] Reduce the size of the vmcs_field_to_offset_table

[PATCH 0/3] Reduce the size of the vmcs_field_to_offset_table

[Jim Mattson]

The vmcs_field_to_offset_table is quite sparse, with its size determined
by the highest VMCS field encoding (0x6c16 for HOST_RIP). To support multiple
VMCS12 formats, we will need multiple vmcs_field_to_offset_tables, and
it would be a shame to replicate this sparse table.

Jim Mattson (3):
  kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX
  kvm: vmx: Change vmcs_field_type to vmcs_field_width
  kvm: vmx: Reduce size of vmcs_field_to_offset_table

 arch/x86/kvm/vmx.c | 197 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 117 insertions(+), 80 deletions(-)

-- 
2.15.1.620.gb9897f4670-goog

[PATCH 1/3] kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX

[Jim Mattson]

This is the highest index value used in any supported VMCS12 field
encoding. It is used to populate the IA32_VMX_VMCS_ENUM MSR.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 669f5f74857d..dfce28498636 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -418,6 +418,12 @@ struct __packed vmcs12 {
  */
 #define VMCS12_SIZE 0x1000
 
+/*
+ * VMCS12_MAX_FIELD_INDEX is the highest index value used in any
+ * supported VMCS12 field encoding.
+ */
+#define VMCS12_MAX_FIELD_INDEX 0x17
+
 /*
  * The nested_vmx structure is part of vcpu_vmx, and holds information we need
  * for correct emulation of VMX (i.e., nested VMX) on this vcpu.
@@ -3005,7 +3011,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 	rdmsrl(MSR_IA32_VMX_CR4_FIXED1, vmx->nested.nested_vmx_cr4_fixed1);
 
 	/* highest index: VMX_PREEMPTION_TIMER_VALUE */
-	vmx->nested.nested_vmx_vmcs_enum = 0x2e;
+	vmx->nested.nested_vmx_vmcs_enum = VMCS12_MAX_FIELD_INDEX << 1;
 }
 
 /*
-- 
2.15.1.620.gb9897f4670-goog

[PATCH 2/3] kvm: vmx: Change vmcs_field_type to vmcs_field_width

[Jim Mattson]

Per the SDM, "[VMCS] Fields are grouped by width (16-bit, 32-bit,
etc.) and type (guest-state, host-state, etc.)." Previously, the width
was indicated by vmcs_field_type. To avoid confusion when we start
dealing with both field width and field type, change vmcs_field_type
to vmcs_field_width.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 58 +++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index dfce28498636..1847000fbb0c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4050,17 +4050,17 @@ static void free_kvm_area(void)
 	}
 }
 
-enum vmcs_field_type {
-	VMCS_FIELD_TYPE_U16 = 0,
-	VMCS_FIELD_TYPE_U64 = 1,
-	VMCS_FIELD_TYPE_U32 = 2,
-	VMCS_FIELD_TYPE_NATURAL_WIDTH = 3
+enum vmcs_field_width {
+	VMCS_FIELD_WIDTH_U16 = 0,
+	VMCS_FIELD_WIDTH_U64 = 1,
+	VMCS_FIELD_WIDTH_U32 = 2,
+	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
 };
 
-static inline int vmcs_field_type(unsigned long field)
+static inline int vmcs_field_width(unsigned long field)
 {
 	if (0x1 & field)	/* the *_HIGH fields are all 32 bit */
-		return VMCS_FIELD_TYPE_U32;
+		return VMCS_FIELD_WIDTH_U32;
 	return (field >> 13) & 0x3 ;
 }
 
@@ -4098,7 +4098,7 @@ static void init_vmcs_shadow_fields(void)
 
 		clear_bit(field, vmx_vmwrite_bitmap);
 		clear_bit(field, vmx_vmread_bitmap);
-		if (vmcs_field_type(field) == VMCS_FIELD_TYPE_U64) {
+		if (vmcs_field_width(field) == VMCS_FIELD_WIDTH_U64) {
 			clear_bit(field + 1, vmx_vmwrite_bitmap);
 			clear_bit(field + 1, vmx_vmread_bitmap);
 		}
@@ -4107,7 +4107,7 @@ static void init_vmcs_shadow_fields(void)
 		unsigned long field = shadow_read_only_fields[i];
 
 		clear_bit(field, vmx_vmread_bitmap);
-		if (vmcs_field_type(field) == VMCS_FIELD_TYPE_U64)
+		if (vmcs_field_width(field) == VMCS_FIELD_WIDTH_U64)
 			clear_bit(field + 1, vmx_vmread_bitmap);
 	}
 }
@@ -7751,17 +7751,17 @@ static inline int vmcs12_read_any(struct kvm_vcpu *vcpu,
 
 	p = ((char *)(get_vmcs12(vcpu))) + offset;
 
-	switch (vmcs_field_type(field)) {
-	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+	switch (vmcs_field_width(field)) {
+	case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 		*ret = *((natural_width *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U16:
+	case VMCS_FIELD_WIDTH_U16:
 		*ret = *((u16 *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U32:
+	case VMCS_FIELD_WIDTH_U32:
 		*ret = *((u32 *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U64:
+	case VMCS_FIELD_WIDTH_U64:
 		*ret = *((u64 *)p);
 		return 0;
 	default:
@@ -7778,17 +7778,17 @@ static inline int vmcs12_write_any(struct kvm_vcpu *vcpu,
 	if (offset < 0)
 		return offset;
 
-	switch (vmcs_field_type(field)) {
-	case VMCS_FIELD_TYPE_U16:
+	switch (vmcs_field_width(field)) {
+	case VMCS_FIELD_WIDTH_U16:
 		*(u16 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_U32:
+	case VMCS_FIELD_WIDTH_U32:
 		*(u32 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_U64:
+	case VMCS_FIELD_WIDTH_U64:
 		*(u64 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+	case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 		*(natural_width *)p = field_value;
 		return 0;
 	default:
@@ -7813,17 +7813,17 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
 
 	for (i = 0; i < num_fields; i++) {
 		field = fields[i];
-		switch (vmcs_field_type(field)) {
-		case VMCS_FIELD_TYPE_U16:
+		switch (vmcs_field_width(field)) {
+		case VMCS_FIELD_WIDTH_U16:
 			field_value = vmcs_read16(field);
 			break;
-		case VMCS_FIELD_TYPE_U32:
+		case VMCS_FIELD_WIDTH_U32:
 			field_value = vmcs_read32(field);
 			break;
-		case VMCS_FIELD_TYPE_U64:
+		case VMCS_FIELD_WIDTH_U64:
 			field_value = vmcs_read64(field);
 			break;
-		case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+		case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 			field_value = vmcs_readl(field);
 			break;
 		default:
@@ -7861,17 +7861,17 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
 			field = fields[q][i];
 			vmcs12_read_any(&vmx->vcpu, field, &field_value);
 
-			switch (vmcs_field_type(field)) {
-			case VMCS_FIELD_TYPE_U16:
+			switch (vmcs_field_width(field)) {
+			case VMCS_FIELD_WIDTH_U16:
 				vmcs_write16(field, (u16)field_value);
 				break;
-			case VMCS_FIELD_TYPE_U32:
+			case VMCS_FIELD_WIDTH_U32:
 				vmcs_write32(field, (u32)field_value);
 				break;
-			case VMCS_FIELD_TYPE_U64:
+			case VMCS_FIELD_WIDTH_U64:
 				vmcs_write64(field, (u64)field_value);
 				break;
-			case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+			case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 				vmcs_writel(field, (long)field_value);
 				break;
 			default:
-- 
2.15.1.620.gb9897f4670-goog

[PATCH 3/3] kvm: vmx: Reduce size of vmcs_field_to_offset_table

[Jim Mattson]

The vmcs_field_to_offset_table was a rather sparse table of short
integers with a maximum index of 0x6c16, amounting to 55342 bytes. Now
that we are considering support for multiple VMCS12 formats, it would
be unfortunate to replicate that large, sparse table. Using a
three-dimensional table indexed by VMCS field index, VMCS field type,
and VMCS field width, it's relatively easy to reduce that table to 768
bytes.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 145 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 88 insertions(+), 57 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1847000fbb0c..bd601b0984d1 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -714,11 +714,15 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
 	return &(to_vmx(vcpu)->pi_desc);
 }
 
-#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
-#define FIELD(number, name)	[number] = VMCS12_OFFSET(name)
-#define FIELD64(number, name)	[number] = VMCS12_OFFSET(name), \
-				[number##_HIGH] = VMCS12_OFFSET(name)+4
+#define VMCS_FIELD_HIGH(field)  ((field) & 1)
+#define VMCS_FIELD_INDEX(field) (((field) >> 1) & 0x1ff)
+#define VMCS_FIELD_TYPE(field)  (((field) >> 10) & 3)
+#define VMCS_FIELD_WIDTH(field) (((field) >> 13) & 3)
 
+#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
+#define FIELD(n, name) \
+	[VMCS_FIELD_INDEX(n)][VMCS_FIELD_TYPE(n)][VMCS_FIELD_WIDTH(n)] = \
+		VMCS12_OFFSET(name)
 
 static unsigned long shadow_read_only_fields[] = {
 	/*
@@ -779,7 +783,7 @@ static unsigned long shadow_read_write_fields[] = {
 static int max_shadow_read_write_fields =
 	ARRAY_SIZE(shadow_read_write_fields);
 
-static const unsigned short vmcs_field_to_offset_table[] = {
+static const unsigned short vmcs_field_to_offset_table[][4][4] = {
 	FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
 	FIELD(POSTED_INTR_NV, posted_intr_nv),
 	FIELD(GUEST_ES_SELECTOR, guest_es_selector),
@@ -799,39 +803,39 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD(HOST_FS_SELECTOR, host_fs_selector),
 	FIELD(HOST_GS_SELECTOR, host_gs_selector),
 	FIELD(HOST_TR_SELECTOR, host_tr_selector),
-	FIELD64(IO_BITMAP_A, io_bitmap_a),
-	FIELD64(IO_BITMAP_B, io_bitmap_b),
-	FIELD64(MSR_BITMAP, msr_bitmap),
-	FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
-	FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
-	FIELD64(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
-	FIELD64(TSC_OFFSET, tsc_offset),
-	FIELD64(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
-	FIELD64(APIC_ACCESS_ADDR, apic_access_addr),
-	FIELD64(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
-	FIELD64(VM_FUNCTION_CONTROL, vm_function_control),
-	FIELD64(EPT_POINTER, ept_pointer),
-	FIELD64(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
-	FIELD64(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
-	FIELD64(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
-	FIELD64(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
-	FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
-	FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
-	FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
-	FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
-	FIELD64(PML_ADDRESS, pml_address),
-	FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
-	FIELD64(GUEST_IA32_PAT, guest_ia32_pat),
-	FIELD64(GUEST_IA32_EFER, guest_ia32_efer),
-	FIELD64(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
-	FIELD64(GUEST_PDPTR0, guest_pdptr0),
-	FIELD64(GUEST_PDPTR1, guest_pdptr1),
-	FIELD64(GUEST_PDPTR2, guest_pdptr2),
-	FIELD64(GUEST_PDPTR3, guest_pdptr3),
-	FIELD64(GUEST_BNDCFGS, guest_bndcfgs),
-	FIELD64(HOST_IA32_PAT, host_ia32_pat),
-	FIELD64(HOST_IA32_EFER, host_ia32_efer),
-	FIELD64(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
+	FIELD(IO_BITMAP_A, io_bitmap_a),
+	FIELD(IO_BITMAP_B, io_bitmap_b),
+	FIELD(MSR_BITMAP, msr_bitmap),
+	FIELD(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
+	FIELD(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
+	FIELD(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
+	FIELD(TSC_OFFSET, tsc_offset),
+	FIELD(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
+	FIELD(APIC_ACCESS_ADDR, apic_access_addr),
+	FIELD(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
+	FIELD(VM_FUNCTION_CONTROL, vm_function_control),
+	FIELD(EPT_POINTER, ept_pointer),
+	FIELD(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
+	FIELD(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
+	FIELD(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
+	FIELD(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
+	FIELD(EPTP_LIST_ADDRESS, eptp_list_address),
+	FIELD(XSS_EXIT_BITMAP, xss_exit_bitmap),
+	FIELD(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
+	FIELD(VMCS_LINK_POINTER, vmcs_link_pointer),
+	FIELD(PML_ADDRESS, pml_address),
+	FIELD(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
+	FIELD(GUEST_IA32_PAT, guest_ia32_pat),
+	FIELD(GUEST_IA32_EFER, guest_ia32_efer),
+	FIELD(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
+	FIELD(GUEST_PDPTR0, guest_pdptr0),
+	FIELD(GUEST_PDPTR1, guest_pdptr1),
+	FIELD(GUEST_PDPTR2, guest_pdptr2),
+	FIELD(GUEST_PDPTR3, guest_pdptr3),
+	FIELD(GUEST_BNDCFGS, guest_bndcfgs),
+	FIELD(HOST_IA32_PAT, host_ia32_pat),
+	FIELD(HOST_IA32_EFER, host_ia32_efer),
+	FIELD(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
 	FIELD(PIN_BASED_VM_EXEC_CONTROL, pin_based_vm_exec_control),
 	FIELD(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control),
 	FIELD(EXCEPTION_BITMAP, exception_bitmap),
@@ -923,15 +927,56 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD(HOST_RIP, host_rip),
 };
 
+enum vmcs_field_width {
+	VMCS_FIELD_WIDTH_U16 = 0,
+	VMCS_FIELD_WIDTH_U64 = 1,
+	VMCS_FIELD_WIDTH_U32 = 2,
+	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
+};
+
+static inline int vmcs_field_width(unsigned long field)
+{
+	if (VMCS_FIELD_HIGH(field))	/* the *_HIGH fields are all 32 bit */
+		return VMCS_FIELD_WIDTH_U32;
+	return VMCS_FIELD_WIDTH(field);
+}
+
+enum vmcs_field_type {
+	VMCS_FIELD_TYPE_CONTROL = 0,
+	VMCS_FIELD_TYPE_INFO = 1,
+	VMCS_FIELD_TYPE_GUEST = 2,
+	VMCS_FIELD_TYPE_HOST = 3
+};
+
+static inline int vmcs_field_type(unsigned long field)
+{
+	return VMCS_FIELD_TYPE(field);
+}
+
 static inline short vmcs_field_to_offset(unsigned long field)
 {
-	BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) > SHRT_MAX);
+	unsigned index = VMCS_FIELD_INDEX(field);
+	unsigned type = VMCS_FIELD_TYPE(field);
+	unsigned width = VMCS_FIELD_WIDTH(field);
+	short offset;
 
-	if (field >= ARRAY_SIZE(vmcs_field_to_offset_table) ||
-	    vmcs_field_to_offset_table[field] == 0)
+	BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) >
+		     VMCS12_MAX_FIELD_INDEX + 1);
+	if (VMCS_FIELD_HIGH(field) && width != VMCS_FIELD_WIDTH_U64)
 		return -ENOENT;
 
-	return vmcs_field_to_offset_table[field];
+	if (index >= ARRAY_SIZE(vmcs_field_to_offset_table))
+		return -ENOENT;
+
+	offset = vmcs_field_to_offset_table[index][type][width];
+
+	if (offset == 0)
+		return -ENOENT;
+
+	if (VMCS_FIELD_HIGH(field))
+		offset += sizeof(u32);
+
+	return offset;
 }
 
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
@@ -4050,23 +4095,9 @@ static void free_kvm_area(void)
 	}
 }
 
-enum vmcs_field_width {
-	VMCS_FIELD_WIDTH_U16 = 0,
-	VMCS_FIELD_WIDTH_U64 = 1,
-	VMCS_FIELD_WIDTH_U32 = 2,
-	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
-};
-
-static inline int vmcs_field_width(unsigned long field)
-{
-	if (0x1 & field)	/* the *_HIGH fields are all 32 bit */
-		return VMCS_FIELD_WIDTH_U32;
-	return (field >> 13) & 0x3 ;
-}
-
 static inline int vmcs_field_readonly(unsigned long field)
 {
-	return (((field >> 10) & 0x3) == 1);
+	return VMCS_FIELD_TYPE(field) == VMCS_FIELD_TYPE_INFO;
 }
 
 static void init_vmcs_shadow_fields(void)
-- 
2.15.1.620.gb9897f4670-goog

Re: [PATCH 3/3] kvm: vmx: Reduce size of vmcs_field_to_offset_table

[Paolo Bonzini]

On 21/12/2017 21:46, Jim Mattson wrote:
> The vmcs_field_to_offset_table was a rather sparse table of short
> integers with a maximum index of 0x6c16, amounting to 55342 bytes. Now
> that we are considering support for multiple VMCS12 formats, it would
> be unfortunate to replicate that large, sparse table. Using a
> three-dimensional table indexed by VMCS field index, VMCS field type,
> and VMCS field width, it's relatively easy to reduce that table to 768
> bytes.

Would it be faster if, instead of the three-dimensional table, we
indexed the array by

   ((field >> 10) | ((field << 5) & 0x3ff))

which would waste one bit, but still reduce the table a lot.

Alternatively, it could use rotate-right-16(field, 10):

   (u16)((field >> 10) | ((field << 6))

That would waste two bits, but it would faster still to build the index.

Paolo

> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 145 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 88 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 1847000fbb0c..bd601b0984d1 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -714,11 +714,15 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
>  	return &(to_vmx(vcpu)->pi_desc);
>  }
>  
> -#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
> -#define FIELD(number, name)	[number] = VMCS12_OFFSET(name)
> -#define FIELD64(number, name)	[number] = VMCS12_OFFSET(name), \
> -				[number##_HIGH] = VMCS12_OFFSET(name)+4
> +#define VMCS_FIELD_HIGH(field)  ((field) & 1)
> +#define VMCS_FIELD_INDEX(field) (((field) >> 1) & 0x1ff)
> +#define VMCS_FIELD_TYPE(field)  (((field) >> 10) & 3)
> +#define VMCS_FIELD_WIDTH(field) (((field) >> 13) & 3)
>  
> +#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
> +#define FIELD(n, name) \
> +	[VMCS_FIELD_INDEX(n)][VMCS_FIELD_TYPE(n)][VMCS_FIELD_WIDTH(n)] = \
> +		VMCS12_OFFSET(name)
>  
>  static unsigned long shadow_read_only_fields[] = {
>  	/*
> @@ -779,7 +783,7 @@ static unsigned long shadow_read_write_fields[] = {
>  static int max_shadow_read_write_fields =
>  	ARRAY_SIZE(shadow_read_write_fields);
>  
> -static const unsigned short vmcs_field_to_offset_table[] = {
> +static const unsigned short vmcs_field_to_offset_table[][4][4] = {
>  	FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
>  	FIELD(POSTED_INTR_NV, posted_intr_nv),
>  	FIELD(GUEST_ES_SELECTOR, guest_es_selector),
> @@ -799,39 +803,39 @@ static const unsigned short vmcs_field_to_offset_table[] = {
>  	FIELD(HOST_FS_SELECTOR, host_fs_selector),
>  	FIELD(HOST_GS_SELECTOR, host_gs_selector),
>  	FIELD(HOST_TR_SELECTOR, host_tr_selector),
> -	FIELD64(IO_BITMAP_A, io_bitmap_a),
> -	FIELD64(IO_BITMAP_B, io_bitmap_b),
> -	FIELD64(MSR_BITMAP, msr_bitmap),
> -	FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
> -	FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
> -	FIELD64(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
> -	FIELD64(TSC_OFFSET, tsc_offset),
> -	FIELD64(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
> -	FIELD64(APIC_ACCESS_ADDR, apic_access_addr),
> -	FIELD64(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
> -	FIELD64(VM_FUNCTION_CONTROL, vm_function_control),
> -	FIELD64(EPT_POINTER, ept_pointer),
> -	FIELD64(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
> -	FIELD64(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
> -	FIELD64(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
> -	FIELD64(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
> -	FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
> -	FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
> -	FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
> -	FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
> -	FIELD64(PML_ADDRESS, pml_address),
> -	FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
> -	FIELD64(GUEST_IA32_PAT, guest_ia32_pat),
> -	FIELD64(GUEST_IA32_EFER, guest_ia32_efer),
> -	FIELD64(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
> -	FIELD64(GUEST_PDPTR0, guest_pdptr0),
> -	FIELD64(GUEST_PDPTR1, guest_pdptr1),
> -	FIELD64(GUEST_PDPTR2, guest_pdptr2),
> -	FIELD64(GUEST_PDPTR3, guest_pdptr3),
> -	FIELD64(GUEST_BNDCFGS, guest_bndcfgs),
> -	FIELD64(HOST_IA32_PAT, host_ia32_pat),
> -	FIELD64(HOST_IA32_EFER, host_ia32_efer),
> -	FIELD64(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
> +	FIELD(IO_BITMAP_A, io_bitmap_a),
> +	FIELD(IO_BITMAP_B, io_bitmap_b),
> +	FIELD(MSR_BITMAP, msr_bitmap),
> +	FIELD(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
> +	FIELD(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
> +	FIELD(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
> +	FIELD(TSC_OFFSET, tsc_offset),
> +	FIELD(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
> +	FIELD(APIC_ACCESS_ADDR, apic_access_addr),
> +	FIELD(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
> +	FIELD(VM_FUNCTION_CONTROL, vm_function_control),
> +	FIELD(EPT_POINTER, ept_pointer),
> +	FIELD(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
> +	FIELD(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
> +	FIELD(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
> +	FIELD(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
> +	FIELD(EPTP_LIST_ADDRESS, eptp_list_address),
> +	FIELD(XSS_EXIT_BITMAP, xss_exit_bitmap),
> +	FIELD(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
> +	FIELD(VMCS_LINK_POINTER, vmcs_link_pointer),
> +	FIELD(PML_ADDRESS, pml_address),
> +	FIELD(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
> +	FIELD(GUEST_IA32_PAT, guest_ia32_pat),
> +	FIELD(GUEST_IA32_EFER, guest_ia32_efer),
> +	FIELD(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
> +	FIELD(GUEST_PDPTR0, guest_pdptr0),
> +	FIELD(GUEST_PDPTR1, guest_pdptr1),
> +	FIELD(GUEST_PDPTR2, guest_pdptr2),
> +	FIELD(GUEST_PDPTR3, guest_pdptr3),
> +	FIELD(GUEST_BNDCFGS, guest_bndcfgs),
> +	FIELD(HOST_IA32_PAT, host_ia32_pat),
> +	FIELD(HOST_IA32_EFER, host_ia32_efer),
> +	FIELD(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
>  	FIELD(PIN_BASED_VM_EXEC_CONTROL, pin_based_vm_exec_control),
>  	FIELD(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control),
>  	FIELD(EXCEPTION_BITMAP, exception_bitmap),
> @@ -923,15 +927,56 @@ static const unsigned short vmcs_field_to_offset_table[] = {
>  	FIELD(HOST_RIP, host_rip),
>  };
>  
> +enum vmcs_field_width {
> +	VMCS_FIELD_WIDTH_U16 = 0,
> +	VMCS_FIELD_WIDTH_U64 = 1,
> +	VMCS_FIELD_WIDTH_U32 = 2,
> +	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
> +};
> +
> +static inline int vmcs_field_width(unsigned long field)
> +{
> +	if (VMCS_FIELD_HIGH(field))	/* the *_HIGH fields are all 32 bit */
> +		return VMCS_FIELD_WIDTH_U32;
> +	return VMCS_FIELD_WIDTH(field);
> +}
> +
> +enum vmcs_field_type {
> +	VMCS_FIELD_TYPE_CONTROL = 0,
> +	VMCS_FIELD_TYPE_INFO = 1,
> +	VMCS_FIELD_TYPE_GUEST = 2,
> +	VMCS_FIELD_TYPE_HOST = 3
> +};
> +
> +static inline int vmcs_field_type(unsigned long field)
> +{
> +	return VMCS_FIELD_TYPE(field);
> +}
> +
>  static inline short vmcs_field_to_offset(unsigned long field)
>  {
> -	BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) > SHRT_MAX);
> +	unsigned index = VMCS_FIELD_INDEX(field);
> +	unsigned type = VMCS_FIELD_TYPE(field);
> +	unsigned width = VMCS_FIELD_WIDTH(field);
> +	short offset;
>  
> -	if (field >= ARRAY_SIZE(vmcs_field_to_offset_table) ||
> -	    vmcs_field_to_offset_table[field] == 0)
> +	BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) >
> +		     VMCS12_MAX_FIELD_INDEX + 1);
> +	if (VMCS_FIELD_HIGH(field) && width != VMCS_FIELD_WIDTH_U64)
>  		return -ENOENT;
>  
> -	return vmcs_field_to_offset_table[field];
> +	if (index >= ARRAY_SIZE(vmcs_field_to_offset_table))
> +		return -ENOENT;
> +
> +	offset = vmcs_field_to_offset_table[index][type][width];
> +
> +	if (offset == 0)
> +		return -ENOENT;
> +
> +	if (VMCS_FIELD_HIGH(field))
> +		offset += sizeof(u32);
> +
> +	return offset;
>  }
>  
>  static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
> @@ -4050,23 +4095,9 @@ static void free_kvm_area(void)
>  	}
>  }
>  
> -enum vmcs_field_width {
> -	VMCS_FIELD_WIDTH_U16 = 0,
> -	VMCS_FIELD_WIDTH_U64 = 1,
> -	VMCS_FIELD_WIDTH_U32 = 2,
> -	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
> -};
> -
> -static inline int vmcs_field_width(unsigned long field)
> -{
> -	if (0x1 & field)	/* the *_HIGH fields are all 32 bit */
> -		return VMCS_FIELD_WIDTH_U32;
> -	return (field >> 13) & 0x3 ;
> -}
> -
>  static inline int vmcs_field_readonly(unsigned long field)
>  {
> -	return (((field >> 10) & 0x3) == 1);
> +	return VMCS_FIELD_TYPE(field) == VMCS_FIELD_TYPE_INFO;
>  }
>  
>  static void init_vmcs_shadow_fields(void)
> 

Re: [PATCH 3/3] kvm: vmx: Reduce size of vmcs_field_to_offset_table

[Jim Mattson]

On Fri, Dec 22, 2017 at 8:26 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/12/2017 21:46, Jim Mattson wrote:
>> The vmcs_field_to_offset_table was a rather sparse table of short
>> integers with a maximum index of 0x6c16, amounting to 55342 bytes. Now
>> that we are considering support for multiple VMCS12 formats, it would
>> be unfortunate to replicate that large, sparse table. Using a
>> three-dimensional table indexed by VMCS field index, VMCS field type,
>> and VMCS field width, it's relatively easy to reduce that table to 768
>> bytes.
>
> Would it be faster if, instead of the three-dimensional table, we
> indexed the array by
>
>    ((field >> 10) | ((field << 5) & 0x3ff))

That doesn't look quite right to me. Do you mean:
        ((field >> 10) & 0x1f) | ((field & 0x3ff) << 5)

> which would waste one bit, but still reduce the table a lot.
>
> Alternatively, it could use rotate-right-16(field, 10):
>
>    (u16)((field >> 10) | ((field << 6))
>
> That would waste two bits, but it would faster still to build the index.

Amusingly, gcc prefers a rol to a ror.

On my E5-1650 v4 workstation, I get:

3D: 16 cycles @ 768 bytes
Alternative 1: 8 cycles @ 2982 bytes
Alternative 2: 4 cycles @ 5926 bytes

Let me send out v2 using the rol.

> Paolo
>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>> ---
>>  arch/x86/kvm/vmx.c | 145 ++++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 88 insertions(+), 57 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 1847000fbb0c..bd601b0984d1 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -714,11 +714,15 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
>>       return &(to_vmx(vcpu)->pi_desc);
>>  }
>>
>> -#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
>> -#define FIELD(number, name)  [number] = VMCS12_OFFSET(name)
>> -#define FIELD64(number, name)        [number] = VMCS12_OFFSET(name), \
>> -                             [number##_HIGH] = VMCS12_OFFSET(name)+4
>> +#define VMCS_FIELD_HIGH(field)  ((field) & 1)
>> +#define VMCS_FIELD_INDEX(field) (((field) >> 1) & 0x1ff)
>> +#define VMCS_FIELD_TYPE(field)  (((field) >> 10) & 3)
>> +#define VMCS_FIELD_WIDTH(field) (((field) >> 13) & 3)
>>
>> +#define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
>> +#define FIELD(n, name) \
>> +     [VMCS_FIELD_INDEX(n)][VMCS_FIELD_TYPE(n)][VMCS_FIELD_WIDTH(n)] = \
>> +             VMCS12_OFFSET(name)
>>
>>  static unsigned long shadow_read_only_fields[] = {
>>       /*
>> @@ -779,7 +783,7 @@ static unsigned long shadow_read_write_fields[] = {
>>  static int max_shadow_read_write_fields =
>>       ARRAY_SIZE(shadow_read_write_fields);
>>
>> -static const unsigned short vmcs_field_to_offset_table[] = {
>> +static const unsigned short vmcs_field_to_offset_table[][4][4] = {
>>       FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
>>       FIELD(POSTED_INTR_NV, posted_intr_nv),
>>       FIELD(GUEST_ES_SELECTOR, guest_es_selector),
>> @@ -799,39 +803,39 @@ static const unsigned short vmcs_field_to_offset_table[] = {
>>       FIELD(HOST_FS_SELECTOR, host_fs_selector),
>>       FIELD(HOST_GS_SELECTOR, host_gs_selector),
>>       FIELD(HOST_TR_SELECTOR, host_tr_selector),
>> -     FIELD64(IO_BITMAP_A, io_bitmap_a),
>> -     FIELD64(IO_BITMAP_B, io_bitmap_b),
>> -     FIELD64(MSR_BITMAP, msr_bitmap),
>> -     FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
>> -     FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
>> -     FIELD64(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
>> -     FIELD64(TSC_OFFSET, tsc_offset),
>> -     FIELD64(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
>> -     FIELD64(APIC_ACCESS_ADDR, apic_access_addr),
>> -     FIELD64(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
>> -     FIELD64(VM_FUNCTION_CONTROL, vm_function_control),
>> -     FIELD64(EPT_POINTER, ept_pointer),
>> -     FIELD64(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
>> -     FIELD64(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
>> -     FIELD64(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
>> -     FIELD64(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
>> -     FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
>> -     FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
>> -     FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
>> -     FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
>> -     FIELD64(PML_ADDRESS, pml_address),
>> -     FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
>> -     FIELD64(GUEST_IA32_PAT, guest_ia32_pat),
>> -     FIELD64(GUEST_IA32_EFER, guest_ia32_efer),
>> -     FIELD64(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
>> -     FIELD64(GUEST_PDPTR0, guest_pdptr0),
>> -     FIELD64(GUEST_PDPTR1, guest_pdptr1),
>> -     FIELD64(GUEST_PDPTR2, guest_pdptr2),
>> -     FIELD64(GUEST_PDPTR3, guest_pdptr3),
>> -     FIELD64(GUEST_BNDCFGS, guest_bndcfgs),
>> -     FIELD64(HOST_IA32_PAT, host_ia32_pat),
>> -     FIELD64(HOST_IA32_EFER, host_ia32_efer),
>> -     FIELD64(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
>> +     FIELD(IO_BITMAP_A, io_bitmap_a),
>> +     FIELD(IO_BITMAP_B, io_bitmap_b),
>> +     FIELD(MSR_BITMAP, msr_bitmap),
>> +     FIELD(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
>> +     FIELD(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
>> +     FIELD(VM_ENTRY_MSR_LOAD_ADDR, vm_entry_msr_load_addr),
>> +     FIELD(TSC_OFFSET, tsc_offset),
>> +     FIELD(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
>> +     FIELD(APIC_ACCESS_ADDR, apic_access_addr),
>> +     FIELD(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
>> +     FIELD(VM_FUNCTION_CONTROL, vm_function_control),
>> +     FIELD(EPT_POINTER, ept_pointer),
>> +     FIELD(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
>> +     FIELD(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
>> +     FIELD(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
>> +     FIELD(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
>> +     FIELD(EPTP_LIST_ADDRESS, eptp_list_address),
>> +     FIELD(XSS_EXIT_BITMAP, xss_exit_bitmap),
>> +     FIELD(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
>> +     FIELD(VMCS_LINK_POINTER, vmcs_link_pointer),
>> +     FIELD(PML_ADDRESS, pml_address),
>> +     FIELD(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
>> +     FIELD(GUEST_IA32_PAT, guest_ia32_pat),
>> +     FIELD(GUEST_IA32_EFER, guest_ia32_efer),
>> +     FIELD(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl),
>> +     FIELD(GUEST_PDPTR0, guest_pdptr0),
>> +     FIELD(GUEST_PDPTR1, guest_pdptr1),
>> +     FIELD(GUEST_PDPTR2, guest_pdptr2),
>> +     FIELD(GUEST_PDPTR3, guest_pdptr3),
>> +     FIELD(GUEST_BNDCFGS, guest_bndcfgs),
>> +     FIELD(HOST_IA32_PAT, host_ia32_pat),
>> +     FIELD(HOST_IA32_EFER, host_ia32_efer),
>> +     FIELD(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
>>       FIELD(PIN_BASED_VM_EXEC_CONTROL, pin_based_vm_exec_control),
>>       FIELD(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control),
>>       FIELD(EXCEPTION_BITMAP, exception_bitmap),
>> @@ -923,15 +927,56 @@ static const unsigned short vmcs_field_to_offset_table[] = {
>>       FIELD(HOST_RIP, host_rip),
>>  };
>>
>> +enum vmcs_field_width {
>> +     VMCS_FIELD_WIDTH_U16 = 0,
>> +     VMCS_FIELD_WIDTH_U64 = 1,
>> +     VMCS_FIELD_WIDTH_U32 = 2,
>> +     VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
>> +};
>> +
>> +static inline int vmcs_field_width(unsigned long field)
>> +{
>> +     if (VMCS_FIELD_HIGH(field))     /* the *_HIGH fields are all 32 bit */
>> +             return VMCS_FIELD_WIDTH_U32;
>> +     return VMCS_FIELD_WIDTH(field);
>> +}
>> +
>> +enum vmcs_field_type {
>> +     VMCS_FIELD_TYPE_CONTROL = 0,
>> +     VMCS_FIELD_TYPE_INFO = 1,
>> +     VMCS_FIELD_TYPE_GUEST = 2,
>> +     VMCS_FIELD_TYPE_HOST = 3
>> +};
>> +
>> +static inline int vmcs_field_type(unsigned long field)
>> +{
>> +     return VMCS_FIELD_TYPE(field);
>> +}
>> +
>>  static inline short vmcs_field_to_offset(unsigned long field)
>>  {
>> -     BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) > SHRT_MAX);
>> +     unsigned index = VMCS_FIELD_INDEX(field);
>> +     unsigned type = VMCS_FIELD_TYPE(field);
>> +     unsigned width = VMCS_FIELD_WIDTH(field);
>> +     short offset;
>>
>> -     if (field >= ARRAY_SIZE(vmcs_field_to_offset_table) ||
>> -         vmcs_field_to_offset_table[field] == 0)
>> +     BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) >
>> +                  VMCS12_MAX_FIELD_INDEX + 1);
>> +     if (VMCS_FIELD_HIGH(field) && width != VMCS_FIELD_WIDTH_U64)
>>               return -ENOENT;
>>
>> -     return vmcs_field_to_offset_table[field];
>> +     if (index >= ARRAY_SIZE(vmcs_field_to_offset_table))
>> +             return -ENOENT;
>> +
>> +     offset = vmcs_field_to_offset_table[index][type][width];
>> +
>> +     if (offset == 0)
>> +             return -ENOENT;
>> +
>> +     if (VMCS_FIELD_HIGH(field))
>> +             offset += sizeof(u32);
>> +
>> +     return offset;
>>  }
>>
>>  static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
>> @@ -4050,23 +4095,9 @@ static void free_kvm_area(void)
>>       }
>>  }
>>
>> -enum vmcs_field_width {
>> -     VMCS_FIELD_WIDTH_U16 = 0,
>> -     VMCS_FIELD_WIDTH_U64 = 1,
>> -     VMCS_FIELD_WIDTH_U32 = 2,
>> -     VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
>> -};
>> -
>> -static inline int vmcs_field_width(unsigned long field)
>> -{
>> -     if (0x1 & field)        /* the *_HIGH fields are all 32 bit */
>> -             return VMCS_FIELD_WIDTH_U32;
>> -     return (field >> 13) & 0x3 ;
>> -}
>> -
>>  static inline int vmcs_field_readonly(unsigned long field)
>>  {
>> -     return (((field >> 10) & 0x3) == 1);
>> +     return VMCS_FIELD_TYPE(field) == VMCS_FIELD_TYPE_INFO;
>>  }
>>
>>  static void init_vmcs_shadow_fields(void)
>>
>

[PATCH v2 0/3] Reduce the size of the vmcs_field_to_offset_table

[Jim Mattson]

The vmcs_field_to_offset_table is quite sparse, with its size determined
by the highest VMCS field encoding (0x6c16 for HOST_RIP). To support multiple
VMCS12 formats, we will need multiple vmcs_field_to_offset_tables, and
it would be a shame to replicate this sparse table.

Patches 1 and 2 are no longer necessary for v2, but I'm going to leave them
in the patch set, because I still think they are worthwhile.

Jim Mattson (3):
  kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX
  kvm: vmx: Change vmcs_field_type to vmcs_field_width
  kvm: vmx: Reduce size of vmcs_field_to_offset_table

 arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 37 deletions(-)

-- 
2.15.1.620.gb9897f4670-goog

[PATCH v2 1/3] kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX

[Jim Mattson]

This is the highest index value used in any supported VMCS12 field
encoding. It is used to populate the IA32_VMX_VMCS_ENUM MSR.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 669f5f74857d..dfce28498636 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -418,6 +418,12 @@ struct __packed vmcs12 {
  */
 #define VMCS12_SIZE 0x1000
 
+/*
+ * VMCS12_MAX_FIELD_INDEX is the highest index value used in any
+ * supported VMCS12 field encoding.
+ */
+#define VMCS12_MAX_FIELD_INDEX 0x17
+
 /*
  * The nested_vmx structure is part of vcpu_vmx, and holds information we need
  * for correct emulation of VMX (i.e., nested VMX) on this vcpu.
@@ -3005,7 +3011,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 	rdmsrl(MSR_IA32_VMX_CR4_FIXED1, vmx->nested.nested_vmx_cr4_fixed1);
 
 	/* highest index: VMX_PREEMPTION_TIMER_VALUE */
-	vmx->nested.nested_vmx_vmcs_enum = 0x2e;
+	vmx->nested.nested_vmx_vmcs_enum = VMCS12_MAX_FIELD_INDEX << 1;
 }
 
 /*
-- 
2.15.1.620.gb9897f4670-goog

[PATCH v2 2/3] kvm: vmx: Change vmcs_field_type to vmcs_field_width

[Jim Mattson]

Per the SDM, "[VMCS] Fields are grouped by width (16-bit, 32-bit,
etc.) and type (guest-state, host-state, etc.)." Previously, the width
was indicated by vmcs_field_type. To avoid confusion when we start
dealing with both field width and field type, change vmcs_field_type
to vmcs_field_width.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 58 +++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index dfce28498636..1847000fbb0c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4050,17 +4050,17 @@ static void free_kvm_area(void)
 	}
 }
 
-enum vmcs_field_type {
-	VMCS_FIELD_TYPE_U16 = 0,
-	VMCS_FIELD_TYPE_U64 = 1,
-	VMCS_FIELD_TYPE_U32 = 2,
-	VMCS_FIELD_TYPE_NATURAL_WIDTH = 3
+enum vmcs_field_width {
+	VMCS_FIELD_WIDTH_U16 = 0,
+	VMCS_FIELD_WIDTH_U64 = 1,
+	VMCS_FIELD_WIDTH_U32 = 2,
+	VMCS_FIELD_WIDTH_NATURAL_WIDTH = 3
 };
 
-static inline int vmcs_field_type(unsigned long field)
+static inline int vmcs_field_width(unsigned long field)
 {
 	if (0x1 & field)	/* the *_HIGH fields are all 32 bit */
-		return VMCS_FIELD_TYPE_U32;
+		return VMCS_FIELD_WIDTH_U32;
 	return (field >> 13) & 0x3 ;
 }
 
@@ -4098,7 +4098,7 @@ static void init_vmcs_shadow_fields(void)
 
 		clear_bit(field, vmx_vmwrite_bitmap);
 		clear_bit(field, vmx_vmread_bitmap);
-		if (vmcs_field_type(field) == VMCS_FIELD_TYPE_U64) {
+		if (vmcs_field_width(field) == VMCS_FIELD_WIDTH_U64) {
 			clear_bit(field + 1, vmx_vmwrite_bitmap);
 			clear_bit(field + 1, vmx_vmread_bitmap);
 		}
@@ -4107,7 +4107,7 @@ static void init_vmcs_shadow_fields(void)
 		unsigned long field = shadow_read_only_fields[i];
 
 		clear_bit(field, vmx_vmread_bitmap);
-		if (vmcs_field_type(field) == VMCS_FIELD_TYPE_U64)
+		if (vmcs_field_width(field) == VMCS_FIELD_WIDTH_U64)
 			clear_bit(field + 1, vmx_vmread_bitmap);
 	}
 }
@@ -7751,17 +7751,17 @@ static inline int vmcs12_read_any(struct kvm_vcpu *vcpu,
 
 	p = ((char *)(get_vmcs12(vcpu))) + offset;
 
-	switch (vmcs_field_type(field)) {
-	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+	switch (vmcs_field_width(field)) {
+	case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 		*ret = *((natural_width *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U16:
+	case VMCS_FIELD_WIDTH_U16:
 		*ret = *((u16 *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U32:
+	case VMCS_FIELD_WIDTH_U32:
 		*ret = *((u32 *)p);
 		return 0;
-	case VMCS_FIELD_TYPE_U64:
+	case VMCS_FIELD_WIDTH_U64:
 		*ret = *((u64 *)p);
 		return 0;
 	default:
@@ -7778,17 +7778,17 @@ static inline int vmcs12_write_any(struct kvm_vcpu *vcpu,
 	if (offset < 0)
 		return offset;
 
-	switch (vmcs_field_type(field)) {
-	case VMCS_FIELD_TYPE_U16:
+	switch (vmcs_field_width(field)) {
+	case VMCS_FIELD_WIDTH_U16:
 		*(u16 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_U32:
+	case VMCS_FIELD_WIDTH_U32:
 		*(u32 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_U64:
+	case VMCS_FIELD_WIDTH_U64:
 		*(u64 *)p = field_value;
 		return 0;
-	case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+	case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 		*(natural_width *)p = field_value;
 		return 0;
 	default:
@@ -7813,17 +7813,17 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
 
 	for (i = 0; i < num_fields; i++) {
 		field = fields[i];
-		switch (vmcs_field_type(field)) {
-		case VMCS_FIELD_TYPE_U16:
+		switch (vmcs_field_width(field)) {
+		case VMCS_FIELD_WIDTH_U16:
 			field_value = vmcs_read16(field);
 			break;
-		case VMCS_FIELD_TYPE_U32:
+		case VMCS_FIELD_WIDTH_U32:
 			field_value = vmcs_read32(field);
 			break;
-		case VMCS_FIELD_TYPE_U64:
+		case VMCS_FIELD_WIDTH_U64:
 			field_value = vmcs_read64(field);
 			break;
-		case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+		case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 			field_value = vmcs_readl(field);
 			break;
 		default:
@@ -7861,17 +7861,17 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
 			field = fields[q][i];
 			vmcs12_read_any(&vmx->vcpu, field, &field_value);
 
-			switch (vmcs_field_type(field)) {
-			case VMCS_FIELD_TYPE_U16:
+			switch (vmcs_field_width(field)) {
+			case VMCS_FIELD_WIDTH_U16:
 				vmcs_write16(field, (u16)field_value);
 				break;
-			case VMCS_FIELD_TYPE_U32:
+			case VMCS_FIELD_WIDTH_U32:
 				vmcs_write32(field, (u32)field_value);
 				break;
-			case VMCS_FIELD_TYPE_U64:
+			case VMCS_FIELD_WIDTH_U64:
 				vmcs_write64(field, (u64)field_value);
 				break;
-			case VMCS_FIELD_TYPE_NATURAL_WIDTH:
+			case VMCS_FIELD_WIDTH_NATURAL_WIDTH:
 				vmcs_writel(field, (long)field_value);
 				break;
 			default:
-- 
2.15.1.620.gb9897f4670-goog

[PATCH v2 3/3] kvm: vmx: Reduce size of vmcs_field_to_offset_table

[Jim Mattson]

The vmcs_field_to_offset_table was a rather sparse table of short
integers with a maximum index of 0x6c16, amounting to 55342 bytes. Now
that we are considering support for multiple VMCS12 formats, it would
be unfortunate to replicate that large, sparse table. Rotating the
field encoding (as a 16-bit integer) left by 6 reduces that table to
5926 bytes.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1847000fbb0c..fec38a1614b5 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -714,10 +714,12 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
 	return &(to_vmx(vcpu)->pi_desc);
 }
 
+#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
 #define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
-#define FIELD(number, name)	[number] = VMCS12_OFFSET(name)
-#define FIELD64(number, name)	[number] = VMCS12_OFFSET(name), \
-				[number##_HIGH] = VMCS12_OFFSET(name)+4
+#define FIELD(number, name)	[ROL16(number, 6)] = VMCS12_OFFSET(name)
+#define FIELD64(number, name)						\
+	FIELD(number, name),						\
+	[ROL16(number##_HIGH, 6)] = VMCS12_OFFSET(name) + sizeof(u32)
 
 
 static unsigned long shadow_read_only_fields[] = {
@@ -925,13 +927,17 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 
 static inline short vmcs_field_to_offset(unsigned long field)
 {
-	BUILD_BUG_ON(ARRAY_SIZE(vmcs_field_to_offset_table) > SHRT_MAX);
+	unsigned index;
+
+	if (field >> 15)
+		return -ENOENT;
 
-	if (field >= ARRAY_SIZE(vmcs_field_to_offset_table) ||
-	    vmcs_field_to_offset_table[field] == 0)
+	index = ROL16(field, 6);
+	if (index >= ARRAY_SIZE(vmcs_field_to_offset_table) ||
+	    vmcs_field_to_offset_table[index] == 0)
 		return -ENOENT;
 
-	return vmcs_field_to_offset_table[field];
+	return vmcs_field_to_offset_table[index];
 }
 
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
-- 
2.15.1.620.gb9897f4670-goog

Re: [PATCH v2 0/3] Reduce the size of the vmcs_field_to_offset_table

[Paolo Bonzini]

On 22/12/2017 21:09, Jim Mattson wrote:
> The vmcs_field_to_offset_table is quite sparse, with its size determined
> by the highest VMCS field encoding (0x6c16 for HOST_RIP). To support multiple
> VMCS12 formats, we will need multiple vmcs_field_to_offset_tables, and
> it would be a shame to replicate this sparse table.
> 
> Patches 1 and 2 are no longer necessary for v2, but I'm going to leave them
> in the patch set, because I still think they are worthwhile.
> 
> Jim Mattson (3):
>   kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX
>   kvm: vmx: Change vmcs_field_type to vmcs_field_width
>   kvm: vmx: Reduce size of vmcs_field_to_offset_table
> 
>  arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 49 insertions(+), 37 deletions(-)
> 

Looks good, I'll queue it shortly (after I look a bit more into
Wanpeng's report).

Paolo

Re: [PATCH v2 1/3] kvm: vmx: Introduce VMCS12_MAX_FIELD_INDEX

[David Hildenbrand]

On 22.12.2017 21:11, Jim Mattson wrote:
> This is the highest index value used in any supported VMCS12 field
> encoding. It is used to populate the IA32_VMX_VMCS_ENUM MSR.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 669f5f74857d..dfce28498636 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -418,6 +418,12 @@ struct __packed vmcs12 {
>   */
>  #define VMCS12_SIZE 0x1000
>  
> +/*
> + * VMCS12_MAX_FIELD_INDEX is the highest index value used in any
> + * supported VMCS12 field encoding.
> + */
> +#define VMCS12_MAX_FIELD_INDEX 0x17
> +
>  /*
>   * The nested_vmx structure is part of vcpu_vmx, and holds information we need
>   * for correct emulation of VMX (i.e., nested VMX) on this vcpu.
> @@ -3005,7 +3011,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  	rdmsrl(MSR_IA32_VMX_CR4_FIXED1, vmx->nested.nested_vmx_cr4_fixed1);
>  
>  	/* highest index: VMX_PREEMPTION_TIMER_VALUE */
> -	vmx->nested.nested_vmx_vmcs_enum = 0x2e;
> +	vmx->nested.nested_vmx_vmcs_enum = VMCS12_MAX_FIELD_INDEX << 1;
>  }
>  
>  /*
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb