diff mbox

nvmx: Check exit qualification RD/WR permission for MMIO accesses

Message ID 1520158592-16952-1-git-send-email-karahmed@amazon.de (mailing list archive)
State New, archived
Headers show

Commit Message

KarimAllah Ahmed March 4, 2018, 10:16 a.m. UTC 
Validate that a write MMIO access that follows a read MMIO access would
have the correct access captured in the exit qualification.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
Message-Id: <1519841208-23349-1-git-send-email-karahmed@amazon.de>
---
 x86/vmx_tests.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 4 deletions(-)

Comments

KarimAllah Ahmed April 12, 2018, 12:11 p.m. UTC | #1 
On Sun, 2018-03-04 at 11:16 +0100, KarimAllah Ahmed wrote:
> Validate that a write MMIO access that follows a read MMIO access would

> have the correct access captured in the exit qualification.

> 

> Cc: Paolo Bonzini <pbonzini@redhat.com>

> Cc: Radim Krčmář <rkrcmar@redhat.com>

> Cc: kvm@vger.kernel.org

> Cc: linux-kernel@vger.kernel.org

> Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>

> Message-Id: <1519841208-23349-1-git-send-email-karahmed@amazon.de>

> ---

>  x86/vmx_tests.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----

>  1 file changed, 48 insertions(+), 4 deletions(-)

> 

> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c

> index 598dd88..a72af1a 100644

> --- a/x86/vmx_tests.c

> +++ b/x86/vmx_tests.c

> @@ -7,6 +7,7 @@

>  #include "msr.h"

>  #include "processor.h"

>  #include "vm.h"

> +#include "pci.h"

>  #include "fwcfg.h"

>  #include "isr.h"

>  #include "desc.h"

> @@ -28,6 +29,8 @@ unsigned long *pml4;

>  u64 eptp;

>  void *data_page1, *data_page2;

>  

> +phys_addr_t pci_physaddr;

> +

>  void *pml_log;

>  #define PML_INDEX 512

>  

> @@ -1041,6 +1044,9 @@ static int apic_version;

>  

>  static int ept_init_common(bool have_ad)

>  {

> +	int ret;

> +	struct pci_dev pcidev;

> +

>  	if (setup_ept(have_ad))

>  		return VMX_TEST_EXIT;

>  	data_page1 = alloc_page();

> @@ -1053,6 +1059,13 @@ static int ept_init_common(bool have_ad)

>  			EPT_RA | EPT_WA | EPT_EA);

>  

>  	apic_version = apic_read(APIC_LVR);

> +

> +	ret = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);

> +	if (ret != PCIDEVADDR_INVALID) {

> +		pci_dev_init(&pcidev, ret);

> +		pci_physaddr = pcidev.resource[PCI_TESTDEV_BAR_MEM];

> +	}

> +

>  	return VMX_TEST_START;

>  }

>  

> @@ -1101,6 +1114,16 @@ t1:

>  	vmcall();

>  	*((u32 *)data_page1) = MAGIC_VAL_2;

>  	report("EPT violation - paging structure", vmx_get_test_stage() == 5);

> +

> +	// MMIO Read/Write

> +	vmx_set_test_stage(5);

> +	vmcall();

> +

> +	*(u32 volatile *)pci_physaddr;

> +	report("MMIO EPT violation - read", vmx_get_test_stage() == 6);

> +

> +	*(u32 volatile *)pci_physaddr = MAGIC_VAL_1;

> +	report("MMIO EPT violation - write", vmx_get_test_stage() == 7);

>  }

>  

>  static void ept_main()

> @@ -1108,12 +1131,12 @@ static void ept_main()

>  	ept_common();

>  

>  	// Test EPT access to L1 MMIO

> -	vmx_set_test_stage(6);

> +	vmx_set_test_stage(7);

>  	report("EPT - MMIO access", *((u32 *)0xfee00030UL) == apic_version);

>  

>  	// Test invalid operand for INVEPT

>  	vmcall();

> -	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 7);

> +	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 8);

>  }

>  

>  bool invept_test(int type, u64 eptp)

> @@ -1187,7 +1210,7 @@ static int ept_exit_handler_common(bool have_ad)

>  	ulong reason;

>  	u32 insn_len;

>  	u32 exit_qual;

> -	static unsigned long data_page1_pte, data_page1_pte_pte;

> +	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;

>  

>  	guest_rip = vmcs_read(GUEST_RIP);

>  	guest_cr3 = vmcs_read(GUEST_CR3);

> @@ -1249,7 +1272,12 @@ static int ept_exit_handler_common(bool have_ad)

>  				data_page1_pte_pte & ~EPT_PRESENT);

>  			ept_sync(INVEPT_SINGLE, eptp);

>  			break;

> -		case 6:

> +		case 5:

> +			install_ept(pml4, (unsigned long)pci_physaddr,

> +				(unsigned long)pci_physaddr, 0);

> +			ept_sync(INVEPT_SINGLE, eptp);

> +			break;

> +		case 7:

>  			if (!invept_test(0, eptp))

>  				vmx_inc_test_stage();

>  			break;

> @@ -1305,6 +1333,22 @@ static int ept_exit_handler_common(bool have_ad)

>  				data_page1_pte_pte | (EPT_PRESENT));

>  			ept_sync(INVEPT_SINGLE, eptp);

>  			break;

> +		case 5:

> +			if (exit_qual & EPT_VLT_RD)

> +				vmx_inc_test_stage();

> +			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,

> +						1, &memaddr_pte));

> +			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA);

> +			ept_sync(INVEPT_SINGLE, eptp);

> +			break;

> +		case 6:

> +			if (exit_qual & EPT_VLT_WR)

> +				vmx_inc_test_stage();

> +			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,

> +						1, &memaddr_pte));

> +			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA | EPT_WA);

> +			ept_sync(INVEPT_SINGLE, eptp);

> +			break;

>  		default:

>  			// Should not reach here

>  			report("ERROR : unexpected stage, %d", false,


Any comments on this one?

I forgot to add 'kvm-unit-tests' in the subject! :)
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
Paolo Bonzini April 12, 2018, 1:08 p.m. UTC | #2 
On 04/03/2018 11:16, KarimAllah Ahmed wrote:
> Validate that a write MMIO access that follows a read MMIO access would
> have the correct access captured in the exit qualification.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
> Message-Id: <1519841208-23349-1-git-send-email-karahmed@amazon.de>
> ---
>  x86/vmx_tests.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 598dd88..a72af1a 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -7,6 +7,7 @@
>  #include "msr.h"
>  #include "processor.h"
>  #include "vm.h"
> +#include "pci.h"
>  #include "fwcfg.h"
>  #include "isr.h"
>  #include "desc.h"
> @@ -28,6 +29,8 @@ unsigned long *pml4;
>  u64 eptp;
>  void *data_page1, *data_page2;
>  
> +phys_addr_t pci_physaddr;
> +
>  void *pml_log;
>  #define PML_INDEX 512
>  
> @@ -1041,6 +1044,9 @@ static int apic_version;
>  
>  static int ept_init_common(bool have_ad)
>  {
> +	int ret;
> +	struct pci_dev pcidev;
> +
>  	if (setup_ept(have_ad))
>  		return VMX_TEST_EXIT;
>  	data_page1 = alloc_page();
> @@ -1053,6 +1059,13 @@ static int ept_init_common(bool have_ad)
>  			EPT_RA | EPT_WA | EPT_EA);
>  
>  	apic_version = apic_read(APIC_LVR);
> +
> +	ret = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);
> +	if (ret != PCIDEVADDR_INVALID) {
> +		pci_dev_init(&pcidev, ret);
> +		pci_physaddr = pcidev.resource[PCI_TESTDEV_BAR_MEM];
> +	}
> +
>  	return VMX_TEST_START;
>  }
>  
> @@ -1101,6 +1114,16 @@ t1:
>  	vmcall();
>  	*((u32 *)data_page1) = MAGIC_VAL_2;
>  	report("EPT violation - paging structure", vmx_get_test_stage() == 5);
> +
> +	// MMIO Read/Write
> +	vmx_set_test_stage(5);
> +	vmcall();
> +
> +	*(u32 volatile *)pci_physaddr;
> +	report("MMIO EPT violation - read", vmx_get_test_stage() == 6);
> +
> +	*(u32 volatile *)pci_physaddr = MAGIC_VAL_1;
> +	report("MMIO EPT violation - write", vmx_get_test_stage() == 7);
>  }
>  
>  static void ept_main()
> @@ -1108,12 +1131,12 @@ static void ept_main()
>  	ept_common();
>  
>  	// Test EPT access to L1 MMIO
> -	vmx_set_test_stage(6);
> +	vmx_set_test_stage(7);
>  	report("EPT - MMIO access", *((u32 *)0xfee00030UL) == apic_version);
>  
>  	// Test invalid operand for INVEPT
>  	vmcall();
> -	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 7);
> +	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 8);
>  }
>  
>  bool invept_test(int type, u64 eptp)
> @@ -1187,7 +1210,7 @@ static int ept_exit_handler_common(bool have_ad)
>  	ulong reason;
>  	u32 insn_len;
>  	u32 exit_qual;
> -	static unsigned long data_page1_pte, data_page1_pte_pte;
> +	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
>  
>  	guest_rip = vmcs_read(GUEST_RIP);
>  	guest_cr3 = vmcs_read(GUEST_CR3);
> @@ -1249,7 +1272,12 @@ static int ept_exit_handler_common(bool have_ad)
>  				data_page1_pte_pte & ~EPT_PRESENT);
>  			ept_sync(INVEPT_SINGLE, eptp);
>  			break;
> -		case 6:
> +		case 5:
> +			install_ept(pml4, (unsigned long)pci_physaddr,
> +				(unsigned long)pci_physaddr, 0);
> +			ept_sync(INVEPT_SINGLE, eptp);
> +			break;
> +		case 7:
>  			if (!invept_test(0, eptp))
>  				vmx_inc_test_stage();
>  			break;
> @@ -1305,6 +1333,22 @@ static int ept_exit_handler_common(bool have_ad)
>  				data_page1_pte_pte | (EPT_PRESENT));
>  			ept_sync(INVEPT_SINGLE, eptp);
>  			break;
> +		case 5:
> +			if (exit_qual & EPT_VLT_RD)
> +				vmx_inc_test_stage();
> +			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
> +						1, &memaddr_pte));
> +			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA);
> +			ept_sync(INVEPT_SINGLE, eptp);
> +			break;
> +		case 6:
> +			if (exit_qual & EPT_VLT_WR)
> +				vmx_inc_test_stage();
> +			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
> +						1, &memaddr_pte));
> +			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA | EPT_WA);
> +			ept_sync(INVEPT_SINGLE, eptp);
> +			break;
>  		default:
>  			// Should not reach here
>  			report("ERROR : unexpected stage, %d", false,
> 

Pushed, thanks!

Paolo
diff mbox

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 598dd88..a72af1a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7,6 +7,7 @@ 
 #include "msr.h"
 #include "processor.h"
 #include "vm.h"
+#include "pci.h"
 #include "fwcfg.h"
 #include "isr.h"
 #include "desc.h"
@@ -28,6 +29,8 @@  unsigned long *pml4;
 u64 eptp;
 void *data_page1, *data_page2;
 
+phys_addr_t pci_physaddr;
+
 void *pml_log;
 #define PML_INDEX 512
 
@@ -1041,6 +1044,9 @@  static int apic_version;
 
 static int ept_init_common(bool have_ad)
 {
+	int ret;
+	struct pci_dev pcidev;
+
 	if (setup_ept(have_ad))
 		return VMX_TEST_EXIT;
 	data_page1 = alloc_page();
@@ -1053,6 +1059,13 @@  static int ept_init_common(bool have_ad)
 			EPT_RA | EPT_WA | EPT_EA);
 
 	apic_version = apic_read(APIC_LVR);
+
+	ret = pci_find_dev(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_TEST);
+	if (ret != PCIDEVADDR_INVALID) {
+		pci_dev_init(&pcidev, ret);
+		pci_physaddr = pcidev.resource[PCI_TESTDEV_BAR_MEM];
+	}
+
 	return VMX_TEST_START;
 }
 
@@ -1101,6 +1114,16 @@  t1:
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_2;
 	report("EPT violation - paging structure", vmx_get_test_stage() == 5);
+
+	// MMIO Read/Write
+	vmx_set_test_stage(5);
+	vmcall();
+
+	*(u32 volatile *)pci_physaddr;
+	report("MMIO EPT violation - read", vmx_get_test_stage() == 6);
+
+	*(u32 volatile *)pci_physaddr = MAGIC_VAL_1;
+	report("MMIO EPT violation - write", vmx_get_test_stage() == 7);
 }
 
 static void ept_main()
@@ -1108,12 +1131,12 @@  static void ept_main()
 	ept_common();
 
 	// Test EPT access to L1 MMIO
-	vmx_set_test_stage(6);
+	vmx_set_test_stage(7);
 	report("EPT - MMIO access", *((u32 *)0xfee00030UL) == apic_version);
 
 	// Test invalid operand for INVEPT
 	vmcall();
-	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 7);
+	report("EPT - unsupported INVEPT", vmx_get_test_stage() == 8);
 }
 
 bool invept_test(int type, u64 eptp)
@@ -1187,7 +1210,7 @@  static int ept_exit_handler_common(bool have_ad)
 	ulong reason;
 	u32 insn_len;
 	u32 exit_qual;
-	static unsigned long data_page1_pte, data_page1_pte_pte;
+	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
 
 	guest_rip = vmcs_read(GUEST_RIP);
 	guest_cr3 = vmcs_read(GUEST_CR3);
@@ -1249,7 +1272,12 @@  static int ept_exit_handler_common(bool have_ad)
 				data_page1_pte_pte & ~EPT_PRESENT);
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
-		case 6:
+		case 5:
+			install_ept(pml4, (unsigned long)pci_physaddr,
+				(unsigned long)pci_physaddr, 0);
+			ept_sync(INVEPT_SINGLE, eptp);
+			break;
+		case 7:
 			if (!invept_test(0, eptp))
 				vmx_inc_test_stage();
 			break;
@@ -1305,6 +1333,22 @@  static int ept_exit_handler_common(bool have_ad)
 				data_page1_pte_pte | (EPT_PRESENT));
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
+		case 5:
+			if (exit_qual & EPT_VLT_RD)
+				vmx_inc_test_stage();
+			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
+						1, &memaddr_pte));
+			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA);
+			ept_sync(INVEPT_SINGLE, eptp);
+			break;
+		case 6:
+			if (exit_qual & EPT_VLT_WR)
+				vmx_inc_test_stage();
+			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)pci_physaddr,
+						1, &memaddr_pte));
+			set_ept_pte(pml4, memaddr_pte, 1, memaddr_pte | EPT_RA | EPT_WA);
+			ept_sync(INVEPT_SINGLE, eptp);
+			break;
 		default:
 			// Should not reach here
 			report("ERROR : unexpected stage, %d", false,