diff mbox series

[kvm-unit-test,1/3] KVM nVMX: test_vmcs_page_* functions need to accept alignment size as a parameter

Message ID 20181206182055.16927-2-krish.sadhukhan@oracle.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-test,1/3] KVM nVMX: test_vmcs_page_* functions need to accept alignment size as a parameter | expand

Commit Message

Krish Sadhukhan Dec. 6, 2018, 6:20 p.m. UTC 
.. because not all alignments fall on page size boundary.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 x86/vmx_tests.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

Comments

Jim Mattson Dec. 6, 2018, 7:55 p.m. UTC | #1 
On Thu, Dec 6, 2018 at 10:46 AM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
> .. because not all alignments fall on page size boundary.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>

But page references, by definition, do fall on a page size boundary.
If you're going to relax that constraint, perhaps you should drop
'page' from the function names?
Krish Sadhukhan Dec. 6, 2018, 10:10 p.m. UTC | #2 
On 12/06/2018 11:55 AM, Jim Mattson wrote:
> On Thu, Dec 6, 2018 at 10:46 AM Krish Sadhukhan
> <krish.sadhukhan@oracle.com> wrote:
>> .. because not all alignments fall on page size boundary.
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
> But page references, by definition, do fall on a page size boundary.
> If you're going to relax that constraint, perhaps you should drop
> 'page' from the function names?

Yes, I agree that we should drop 'page' from the names.  I propose 
test_vmcs_addr_*.
diff mbox series

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index b105b23..76bf1f4 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3464,16 +3464,17 @@  static void test_vmcs_page_addr(const char *name,
 				enum Encoding encoding,
 				bool ignored,
 				bool xfail_beyond_mapped_ram,
-				u64 addr)
+				u64 addr,
+				u64 align)
 {
 	bool xfail =
 		(xfail_beyond_mapped_ram &&
-		 addr > fwcfg_get_u64(FW_CFG_RAM_SIZE) - PAGE_SIZE &&
+		 addr > fwcfg_get_u64(FW_CFG_RAM_SIZE) - align &&
 		 addr < (1ul << cpuid_maxphyaddr()));
 
 	report_prefix_pushf("%s = %lx", name, addr);
 	vmcs_write(encoding, addr);
-	test_vmx_controls(ignored || (IS_ALIGNED(addr, PAGE_SIZE) &&
+	test_vmx_controls(ignored || (IS_ALIGNED(addr, align) &&
 				  addr < (1ul << cpuid_maxphyaddr())),
 			  xfail);
 	report_prefix_pop();
@@ -3486,25 +3487,30 @@  static void test_vmcs_page_addr(const char *name,
 static void test_vmcs_page_values(const char *name,
 				  enum Encoding encoding,
 				  bool ignored,
-				  bool xfail_beyond_mapped_ram)
+				  bool xfail_beyond_mapped_ram,
+				  u64 align)
 {
 	unsigned i;
 	u64 orig_val = vmcs_read(encoding);
 
 	for (i = 0; i < 64; i++)
 		test_vmcs_page_addr(name, encoding, ignored,
-				    xfail_beyond_mapped_ram, 1ul << i);
+				    xfail_beyond_mapped_ram, 1ul << i,
+				    align);
 
 	test_vmcs_page_addr(name, encoding, ignored,
-			    xfail_beyond_mapped_ram, PAGE_SIZE - 1);
+			    xfail_beyond_mapped_ram, PAGE_SIZE - 1,
+			    align);
 	test_vmcs_page_addr(name, encoding, ignored,
-			    xfail_beyond_mapped_ram, PAGE_SIZE);
+			    xfail_beyond_mapped_ram, PAGE_SIZE,
+			    align);
 	test_vmcs_page_addr(name, encoding, ignored,
 			    xfail_beyond_mapped_ram,
-			    (1ul << cpuid_maxphyaddr()) - PAGE_SIZE);
+			    (1ul << cpuid_maxphyaddr()) - PAGE_SIZE,
+			    align);
 	test_vmcs_page_addr(name, encoding, ignored,
 			    xfail_beyond_mapped_ram,
-			    -1ul);
+			    -1ul, align);
 
 	vmcs_write(encoding, orig_val);
 }
@@ -3517,7 +3523,7 @@  static void test_vmcs_page_reference(u32 control_bit, enum Encoding field,
 				     const char *field_name,
 				     const char *control_name,
 				     bool xfail_beyond_mapped_ram,
-				     bool control_primary)
+				     bool control_primary, u64 align)
 {
 	u32 primary = vmcs_read(CPU_EXEC_CTRL0);
 	u32 secondary = vmcs_read(CPU_EXEC_CTRL1);
@@ -3540,7 +3546,8 @@  static void test_vmcs_page_reference(u32 control_bit, enum Encoding field,
 		vmcs_write(CPU_EXEC_CTRL0, primary | CPU_SECONDARY);
 		vmcs_write(CPU_EXEC_CTRL1, secondary | control_bit);
 	}
-	test_vmcs_page_values(field_name, field, false, xfail_beyond_mapped_ram);
+	test_vmcs_page_values(field_name, field, false,
+			      xfail_beyond_mapped_ram, align);
 	report_prefix_pop();
 
 	report_prefix_pushf("%s disabled", control_name);
@@ -3550,7 +3557,7 @@  static void test_vmcs_page_reference(u32 control_bit, enum Encoding field,
 		vmcs_write(CPU_EXEC_CTRL0, primary & ~CPU_SECONDARY);
 		vmcs_write(CPU_EXEC_CTRL1, secondary & ~control_bit);
 	}
-	test_vmcs_page_values(field_name, field, true, false);
+	test_vmcs_page_values(field_name, field, true, false, align);
 	report_prefix_pop();
 
 	vmcs_write(field, page_addr);
@@ -3567,10 +3574,10 @@  static void test_io_bitmaps(void)
 {
 	test_vmcs_page_reference(CPU_IO_BITMAP, IO_BITMAP_A,
 				 "I/O bitmap A", "Use I/O bitmaps", false,
-				 true);
+				 true, PAGE_SIZE);
 	test_vmcs_page_reference(CPU_IO_BITMAP, IO_BITMAP_B,
 				 "I/O bitmap B", "Use I/O bitmaps", false,
-				 true);
+				 true, PAGE_SIZE);
 }
 
 /*
@@ -3583,7 +3590,7 @@  static void test_msr_bitmap(void)
 {
 	test_vmcs_page_reference(CPU_MSR_BITMAP, MSR_BITMAP,
 				 "MSR bitmap", "Use MSR bitmaps", false,
-				 true);
+				 true, PAGE_SIZE);
 }
 
 /*
@@ -3598,7 +3605,7 @@  static void test_apic_virt_addr(void)
 {
 	test_vmcs_page_reference(CPU_TPR_SHADOW, APIC_VIRT_ADDR,
 				 "virtual-APIC address", "Use TPR shadow",
-				 true, true);
+				 true, true, PAGE_SIZE);
 }
 
 /*
@@ -3617,7 +3624,8 @@  static void test_apic_access_addr(void)
 
 	test_vmcs_page_reference(CPU_VIRT_APIC_ACCESSES, APIC_ACCS_ADDR,
 				 "APIC-access address",
-				 "virtualize APIC-accesses", false, false);
+				 "virtualize APIC-accesses", false, false,
+				 PAGE_SIZE);
 }
 
 static bool set_bit_pattern(u8 mask, u32 *secondary)
@@ -4715,7 +4723,7 @@  static void test_pml(void)
 	report_prefix_pop();
 
 	test_vmcs_page_reference(CPU_PML, PMLADDR, "PML address",
-	    "PML", false, false);
+	    "PML", false, false, PAGE_SIZE);
 
 	vmcs_write(CPU_EXEC_CTRL0, primary_saved);
 	vmcs_write(CPU_EXEC_CTRL1, secondary_saved);