diff mbox series

[kvm-unit-tests] x86: apic: pv ipis out-of-bounds access testing

Message ID 1535522087-5704-1-git-send-email-wanpengli@tencent.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] x86: apic: pv ipis out-of-bounds access testing | expand

Commit Message

Wanpeng Li Aug. 29, 2018, 5:54 a.m. UTC
From: Wanpeng Li <wanpengli@tencent.com>

Adds pv ipis out-of-bounds acess testing. 

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Liran Alon <liran.alon@oracle.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 x86/apic.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Liran Alon Aug. 29, 2018, 9:09 a.m. UTC | #1
> On 29 Aug 2018, at 8:54, Wanpeng Li <kernellwp@gmail.com> wrote:
> 
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> Adds pv ipis out-of-bounds acess testing. 

I believe it is worth elaborating a bit more in commit message…
At least refer to the kernel patch which fixes the issue you are testing here.

> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Liran Alon <liran.alon@oracle.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
> x86/apic.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/x86/apic.c b/x86/apic.c
> index c976e8d..460d7a5 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -566,6 +566,15 @@ static void test_apic_change_mode(void)
> 	report("TMCCT should stay at zero", !apic_read(APIC_TMCCT));
> }
> 
> +void test_pv_ipi()
> +{
> +    unsigned int nr = 10, ret;

Define KVM_HC_SEND_IPI instead of using hard-coded “10”.

> +    unsigned long p1 = 0xFFFFFFFF, p2 = 0, p3 = 0xFFFFFFFF, p4 = 0x0;

As arguments are documented in Documentation/virtual/kvm/hypercalls.txt from zero-index
(a0,a1,a2 and etc.) it is preferred you use here same naming convention.

> +
> +    asm volatile("vmcall" : "=a"(ret) :"a"(nr), "b"(p1), "c"(p2), "d"(p3), "S"(p4));
> +    report("PV IPIs testing", !ret);
> +}
> +
> int main(void)
> {
>     setup_vm();
> @@ -581,6 +590,7 @@ int main(void)
> 
>     test_self_ipi();
>     test_physical_broadcast();
> +    test_pv_ipi();
> 
>     test_sti_nmi();
>     test_multiple_nmi();
> -- 
> 2.7.4
> 

From semantics perspective:
Reviewed-By: Liran Alon <liran.alon@oracle.com>
Wanpeng Li Aug. 29, 2018, 10:11 a.m. UTC | #2
On Wed, 29 Aug 2018 at 17:10, Liran Alon <liran.alon@oracle.com> wrote:
>
>
>
> > On 29 Aug 2018, at 8:54, Wanpeng Li <kernellwp@gmail.com> wrote:
> >
> > From: Wanpeng Li <wanpengli@tencent.com>
> >
> > Adds pv ipis out-of-bounds acess testing.
>
> I believe it is worth elaborating a bit more in commit message…
> At least refer to the kernel patch which fixes the issue you are testing here.
>

Will do in v2.

> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Cc: Liran Alon <liran.alon@oracle.com>
> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > ---
> > x86/apic.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/x86/apic.c b/x86/apic.c
> > index c976e8d..460d7a5 100644
> > --- a/x86/apic.c
> > +++ b/x86/apic.c
> > @@ -566,6 +566,15 @@ static void test_apic_change_mode(void)
> >       report("TMCCT should stay at zero", !apic_read(APIC_TMCCT));
> > }
> >
> > +void test_pv_ipi()
> > +{
> > +    unsigned int nr = 10, ret;
>
> Define KVM_HC_SEND_IPI instead of using hard-coded “10”.

Ditto.

>
> > +    unsigned long p1 = 0xFFFFFFFF, p2 = 0, p3 = 0xFFFFFFFF, p4 = 0x0;
>
> As arguments are documented in Documentation/virtual/kvm/hypercalls.txt from zero-index
> (a0,a1,a2 and etc.) it is preferred you use here same naming convention.

Ditto.

Regards,
Wanpeng Li

>
> > +
> > +    asm volatile("vmcall" : "=a"(ret) :"a"(nr), "b"(p1), "c"(p2), "d"(p3), "S"(p4));
> > +    report("PV IPIs testing", !ret);
> > +}
> > +
> > int main(void)
> > {
> >     setup_vm();
> > @@ -581,6 +590,7 @@ int main(void)
> >
> >     test_self_ipi();
> >     test_physical_broadcast();
> > +    test_pv_ipi();
> >
> >     test_sti_nmi();
> >     test_multiple_nmi();
> > --
> > 2.7.4
> >
>
> From semantics perspective:
> Reviewed-By: Liran Alon <liran.alon@oracle.com>
>
>
diff mbox series

Patch

diff --git a/x86/apic.c b/x86/apic.c
index c976e8d..460d7a5 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -566,6 +566,15 @@  static void test_apic_change_mode(void)
 	report("TMCCT should stay at zero", !apic_read(APIC_TMCCT));
 }
 
+void test_pv_ipi()
+{
+    unsigned int nr = 10, ret;
+    unsigned long p1 = 0xFFFFFFFF, p2 = 0, p3 = 0xFFFFFFFF, p4 = 0x0;
+
+    asm volatile("vmcall" : "=a"(ret) :"a"(nr), "b"(p1), "c"(p2), "d"(p3), "S"(p4));
+    report("PV IPIs testing", !ret);
+}
+
 int main(void)
 {
     setup_vm();
@@ -581,6 +590,7 @@  int main(void)
 
     test_self_ipi();
     test_physical_broadcast();
+    test_pv_ipi();
 
     test_sti_nmi();
     test_multiple_nmi();