| Message ID | 20181212010437.11129-3-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-test,v2,1/4] KVM nVMX: Change the names of the functions test_vmcs_page_* to test_vmcs_addr_* | expand |
On Tue, Dec 11, 2018 at 5:29 PM 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> > --- > x86/vmx_tests.c | 40 ++++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index bdd23df..3e6babe 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -3463,16 +3463,17 @@ static void test_vmcs_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(); > @@ -3485,25 +3486,26 @@ static void test_vmcs_addr(const char *name, > static void test_vmcs_addr_values(const char *name, > enum Encoding encoding, > bool ignored, > - bool xfail_beyond_mapped_ram) > + bool xfail_beyond_mapped_ram, > + u64 align) s/align/alignment/, and move before 'ignored'? > { > unsigned i; > u64 orig_val = vmcs_read(encoding); > > for (i = 0; i < 64; i++) > test_vmcs_addr(name, encoding, ignored, > - xfail_beyond_mapped_ram, 1ul << i); > + xfail_beyond_mapped_ram, 1ul << i, align); > > test_vmcs_addr(name, encoding, ignored, > - xfail_beyond_mapped_ram, PAGE_SIZE - 1); > + xfail_beyond_mapped_ram, PAGE_SIZE - 1, align); Should PAGE_SIZE, here and below, be changed to align? > test_vmcs_addr(name, encoding, ignored, > - xfail_beyond_mapped_ram, PAGE_SIZE); > + xfail_beyond_mapped_ram, PAGE_SIZE, align); > test_vmcs_addr(name, encoding, ignored, > xfail_beyond_mapped_ram, > - (1ul << cpuid_maxphyaddr()) - PAGE_SIZE); > + (1ul << cpuid_maxphyaddr()) - PAGE_SIZE, align); > test_vmcs_addr(name, encoding, ignored, > xfail_beyond_mapped_ram, > - -1ul); > + -1ul, align); > > vmcs_write(encoding, orig_val); > } > @@ -3516,7 +3518,7 @@ static void test_vmcs_addr_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) s/align/alignment/?
On 12/12/2018 11:01 AM, Jim Mattson wrote: > On Tue, Dec 11, 2018 at 5:29 PM 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> >> --- >> x86/vmx_tests.c | 40 ++++++++++++++++++++++------------------ >> 1 file changed, 22 insertions(+), 18 deletions(-) >> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >> index bdd23df..3e6babe 100644 >> --- a/x86/vmx_tests.c >> +++ b/x86/vmx_tests.c >> @@ -3463,16 +3463,17 @@ static void test_vmcs_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(); >> @@ -3485,25 +3486,26 @@ static void test_vmcs_addr(const char *name, >> static void test_vmcs_addr_values(const char *name, >> enum Encoding encoding, >> bool ignored, >> - bool xfail_beyond_mapped_ram) >> + bool xfail_beyond_mapped_ram, >> + u64 align) > s/align/alignment/, and move before 'ignored'? Using the full word is better, but I used 'align' to save some space :). If you feel strongly about it, I will change it. I will move the parameter before 'ignored'. >> { >> unsigned i; >> u64 orig_val = vmcs_read(encoding); >> >> for (i = 0; i < 64; i++) >> test_vmcs_addr(name, encoding, ignored, >> - xfail_beyond_mapped_ram, 1ul << i); >> + xfail_beyond_mapped_ram, 1ul << i, align); >> >> test_vmcs_addr(name, encoding, ignored, >> - xfail_beyond_mapped_ram, PAGE_SIZE - 1); >> + xfail_beyond_mapped_ram, PAGE_SIZE - 1, align); > Should PAGE_SIZE, here and below, be changed to align? The two parameters are semantically different even though both are using PAGE_SIZE. That's why I didn't parameterize the address parameter. >> test_vmcs_addr(name, encoding, ignored, >> - xfail_beyond_mapped_ram, PAGE_SIZE); >> + xfail_beyond_mapped_ram, PAGE_SIZE, align); >> test_vmcs_addr(name, encoding, ignored, >> xfail_beyond_mapped_ram, >> - (1ul << cpuid_maxphyaddr()) - PAGE_SIZE); >> + (1ul << cpuid_maxphyaddr()) - PAGE_SIZE, align); >> test_vmcs_addr(name, encoding, ignored, >> xfail_beyond_mapped_ram, >> - -1ul); >> + -1ul, align); >> >> vmcs_write(encoding, orig_val); >> } >> @@ -3516,7 +3518,7 @@ static void test_vmcs_addr_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) > s/align/alignment/?
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index bdd23df..3e6babe 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -3463,16 +3463,17 @@ static void test_vmcs_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(); @@ -3485,25 +3486,26 @@ static void test_vmcs_addr(const char *name, static void test_vmcs_addr_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_addr(name, encoding, ignored, - xfail_beyond_mapped_ram, 1ul << i); + xfail_beyond_mapped_ram, 1ul << i, align); test_vmcs_addr(name, encoding, ignored, - xfail_beyond_mapped_ram, PAGE_SIZE - 1); + xfail_beyond_mapped_ram, PAGE_SIZE - 1, align); test_vmcs_addr(name, encoding, ignored, - xfail_beyond_mapped_ram, PAGE_SIZE); + xfail_beyond_mapped_ram, PAGE_SIZE, align); test_vmcs_addr(name, encoding, ignored, xfail_beyond_mapped_ram, - (1ul << cpuid_maxphyaddr()) - PAGE_SIZE); + (1ul << cpuid_maxphyaddr()) - PAGE_SIZE, align); test_vmcs_addr(name, encoding, ignored, xfail_beyond_mapped_ram, - -1ul); + -1ul, align); vmcs_write(encoding, orig_val); } @@ -3516,7 +3518,7 @@ static void test_vmcs_addr_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); @@ -3539,7 +3541,8 @@ static void test_vmcs_addr_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_addr_values(field_name, field, false, xfail_beyond_mapped_ram); + test_vmcs_addr_values(field_name, field, false, + xfail_beyond_mapped_ram, align); report_prefix_pop(); report_prefix_pushf("%s disabled", control_name); @@ -3549,7 +3552,7 @@ static void test_vmcs_addr_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_addr_values(field_name, field, true, false); + test_vmcs_addr_values(field_name, field, true, false, align); report_prefix_pop(); vmcs_write(field, page_addr); @@ -3566,10 +3569,10 @@ static void test_io_bitmaps(void) { test_vmcs_addr_reference(CPU_IO_BITMAP, IO_BITMAP_A, "I/O bitmap A", "Use I/O bitmaps", false, - true); + true, PAGE_SIZE); test_vmcs_addr_reference(CPU_IO_BITMAP, IO_BITMAP_B, "I/O bitmap B", "Use I/O bitmaps", false, - true); + true, PAGE_SIZE); } /* @@ -3582,7 +3585,7 @@ static void test_msr_bitmap(void) { test_vmcs_addr_reference(CPU_MSR_BITMAP, MSR_BITMAP, "MSR bitmap", "Use MSR bitmaps", false, - true); + true, PAGE_SIZE); } /* @@ -3597,7 +3600,7 @@ static void test_apic_virt_addr(void) { test_vmcs_addr_reference(CPU_TPR_SHADOW, APIC_VIRT_ADDR, "virtual-APIC address", "Use TPR shadow", - true, true); + true, true, PAGE_SIZE); } /* @@ -3616,7 +3619,8 @@ static void test_apic_access_addr(void) test_vmcs_addr_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) @@ -4714,7 +4718,7 @@ static void test_pml(void) report_prefix_pop(); test_vmcs_addr_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);