diff mbox

[kvm-unit-tests,1/3] x86/vmx: fix EPT - MMIO access

Message ID 20170629172647.22188-2-rkrcmar@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář June 29, 2017, 5:26 p.m. UTC
Reading the memory mapped page with x2apic is a bug.  Use the generic reader
instead.  An alternative would be to disable x2apic.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 x86/vmx_tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Feiner June 29, 2017, 5:34 p.m. UTC | #1
On Thu, Jun 29, 2017 at 10:26 AM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>
> Reading the memory mapped page with x2apic is a bug.  Use the generic reader
> instead.  An alternative would be to disable x2apic.
>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>

> ---
>  x86/vmx_tests.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 1f439522cad8..7be016ce4fbc 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1031,7 +1031,7 @@ static int ept_init_common(bool have_ad)
>         install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
>                         EPT_RA | EPT_WA | EPT_EA);
>
> -       apic_version = *((u32 *)0xfee00030UL);
> +       apic_version = apic_read(APIC_LVR);
>         return VMX_TEST_START;
>  }
>
> --
> 2.13.2
>

Looks good!
Paolo Bonzini June 30, 2017, 10:22 a.m. UTC | #2
On 29/06/2017 19:34, Peter Feiner wrote:
> Reading the memory mapped page with x2apic is a bug.  Use the generic reader
> instead.  An alternative would be to disable x2apic.

Disabling x2apic would test what the test is supposed to test. :)

Paolo
Radim Krčmář July 3, 2017, 5:13 p.m. UTC | #3
2017-06-30 12:22+0200, Paolo Bonzini:
> On 29/06/2017 19:34, Peter Feiner wrote:
> > Reading the memory mapped page with x2apic is a bug.  Use the generic reader
> > instead.  An alternative would be to disable x2apic.
> 
> Disabling x2apic would test what the test is supposed to test. :)

We should still be doing the same --  we're interested in testing the
MMIO access from the guest, which is being done in ept_main().

ept_init_common() is used just to access the reference value and I think
that allowing x2APIC there gives us a bit more leeway,

thanks.
Paolo Bonzini July 3, 2017, 5:28 p.m. UTC | #4
----- Original Message -----
> From: "Radim Krčmář" <rkrcmar@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Peter Feiner" <pfeiner@google.com>, kvm@vger.kernel.org, "David Matlack" <dmatlack@google.com>
> Sent: Monday, July 3, 2017 7:13:44 PM
> Subject: Re: [kvm-unit-tests PATCH 1/3] x86/vmx: fix EPT - MMIO access
> 
> 2017-06-30 12:22+0200, Paolo Bonzini:
> > On 29/06/2017 19:34, Peter Feiner wrote:
> > > Reading the memory mapped page with x2apic is a bug.  Use the generic
> > > reader
> > > instead.  An alternative would be to disable x2apic.
> > 
> > Disabling x2apic would test what the test is supposed to test. :)
> 
> We should still be doing the same --  we're interested in testing the
> MMIO access from the guest, which is being done in ept_main().
> 
> ept_init_common() is used just to access the reference value and I think
> that allowing x2APIC there gives us a bit more leeway,

Thanks for the explanation.  You're obviously right, will apply tomorrow
with an improved commit message.

Paolo
diff mbox

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 1f439522cad8..7be016ce4fbc 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1031,7 +1031,7 @@  static int ept_init_common(bool have_ad)
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
 
-	apic_version = *((u32 *)0xfee00030UL);
+	apic_version = apic_read(APIC_LVR);
 	return VMX_TEST_START;
 }