diff mbox

[7/8] tests: Use qpci_mem{read, write} in ivshmem-test

Message ID 1476787933-7180-8-git-send-email-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson Oct. 18, 2016, 10:52 a.m. UTC
ivshmem implements a block of shared memory in a PCI BAR.  Currently our
test case accesses this using qtest_mem{read,write}.  However, deducing
the correct addresses for these requires making assumptions about the
internel format returned by qpci_iomap(), along with some ugly casts.

This patch changes the test to use the new qpci_mem{read,write} interfaces
which is neater.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tests/ivshmem-test.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Laurent Vivier Oct. 18, 2016, 4:14 p.m. UTC | #1
On 18/10/2016 12:52, David Gibson wrote:
> ivshmem implements a block of shared memory in a PCI BAR.  Currently our
> test case accesses this using qtest_mem{read,write}.  However, deducing
> the correct addresses for these requires making assumptions about the
> internel format returned by qpci_iomap(), along with some ugly casts.
> 
> This patch changes the test to use the new qpci_mem{read,write} interfaces
> which is neater.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

I think your changes break this test.

There is two guest, so we need to access them through two sockets,
defined in s1->qtest and s2->qtest.

But qpci_memXXX() always use memread(...) and thus
qtest_memread(global_qtest, ...) , so always test the last guest started.

You must swap global_qtest as it is done in in_reg()/out_reg() before
your qpci_memXXX() in test_ivshmem_pair().

Laurent

> ---
>  tests/ivshmem-test.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> index f36bfe7..97a887e 100644
> --- a/tests/ivshmem-test.c
> +++ b/tests/ivshmem-test.c
> @@ -169,7 +169,7 @@ static void test_ivshmem_single(void)
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
>          data[i] = i;
>      }
> -    qtest_memwrite(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> +    qpci_memwrite(s->dev, s->mem_base, data, sizeof(data));
>  
>      /* verify write */
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
> @@ -178,7 +178,7 @@ static void test_ivshmem_single(void)
>  
>      /* read it back and verify read */
>      memset(data, 0, sizeof(data));
> -    qtest_memread(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> +    qpci_memread(s->dev, s->mem_base, data, sizeof(data));
>      for (i = 0; i < G_N_ELEMENTS(data); i++) {
>          g_assert_cmpuint(data[i], ==, i);
>      }
> @@ -201,29 +201,29 @@ static void test_ivshmem_pair(void)
>  
>      /* host write, guest 1 & 2 read */
>      memset(tmpshmem, 0x42, TMPSHMSIZE);
> -    qtest_memread(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s1->dev, s1->mem_base, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x42);
>      }
> -    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x42);
>      }
>  
>      /* guest 1 write, guest 2 read */
>      memset(data, 0x43, TMPSHMSIZE);
> -    qtest_memwrite(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> +    qpci_memwrite(s1->dev, s1->mem_base, data, TMPSHMSIZE);
>      memset(data, 0, TMPSHMSIZE);
> -    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x43);
>      }
>  
>      /* guest 2 write, guest 1 read */
>      memset(data, 0x44, TMPSHMSIZE);
> -    qtest_memwrite(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memwrite(s2->dev, s2->mem_base, data, TMPSHMSIZE);
>      memset(data, 0, TMPSHMSIZE);
> -    qtest_memread(s1->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> +    qpci_memread(s1->dev, s2->mem_base, data, TMPSHMSIZE);
>      for (i = 0; i < TMPSHMSIZE; i++) {
>          g_assert_cmpuint(data[i], ==, 0x44);
>      }
>
David Gibson Oct. 19, 2016, 4:13 a.m. UTC | #2
On Tue, Oct 18, 2016 at 06:14:09PM +0200, Laurent Vivier wrote:
> 
> 
> On 18/10/2016 12:52, David Gibson wrote:
> > ivshmem implements a block of shared memory in a PCI BAR.  Currently our
> > test case accesses this using qtest_mem{read,write}.  However, deducing
> > the correct addresses for these requires making assumptions about the
> > internel format returned by qpci_iomap(), along with some ugly casts.
> > 
> > This patch changes the test to use the new qpci_mem{read,write} interfaces
> > which is neater.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> I think your changes break this test.
> 
> There is two guest, so we need to access them through two sockets,
> defined in s1->qtest and s2->qtest.
> 
> But qpci_memXXX() always use memread(...) and thus
> qtest_memread(global_qtest, ...) , so always test the last guest started.
> 
> You must swap global_qtest as it is done in in_reg()/out_reg() before
> your qpci_memXXX() in test_ivshmem_pair().

Ah.. good point.  I'll fix that in the next spin.

> 
> Laurent
> 
> > ---
> >  tests/ivshmem-test.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> > index f36bfe7..97a887e 100644
> > --- a/tests/ivshmem-test.c
> > +++ b/tests/ivshmem-test.c
> > @@ -169,7 +169,7 @@ static void test_ivshmem_single(void)
> >      for (i = 0; i < G_N_ELEMENTS(data); i++) {
> >          data[i] = i;
> >      }
> > -    qtest_memwrite(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> > +    qpci_memwrite(s->dev, s->mem_base, data, sizeof(data));
> >  
> >      /* verify write */
> >      for (i = 0; i < G_N_ELEMENTS(data); i++) {
> > @@ -178,7 +178,7 @@ static void test_ivshmem_single(void)
> >  
> >      /* read it back and verify read */
> >      memset(data, 0, sizeof(data));
> > -    qtest_memread(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> > +    qpci_memread(s->dev, s->mem_base, data, sizeof(data));
> >      for (i = 0; i < G_N_ELEMENTS(data); i++) {
> >          g_assert_cmpuint(data[i], ==, i);
> >      }
> > @@ -201,29 +201,29 @@ static void test_ivshmem_pair(void)
> >  
> >      /* host write, guest 1 & 2 read */
> >      memset(tmpshmem, 0x42, TMPSHMSIZE);
> > -    qtest_memread(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> > +    qpci_memread(s1->dev, s1->mem_base, data, TMPSHMSIZE);
> >      for (i = 0; i < TMPSHMSIZE; i++) {
> >          g_assert_cmpuint(data[i], ==, 0x42);
> >      }
> > -    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> >      for (i = 0; i < TMPSHMSIZE; i++) {
> >          g_assert_cmpuint(data[i], ==, 0x42);
> >      }
> >  
> >      /* guest 1 write, guest 2 read */
> >      memset(data, 0x43, TMPSHMSIZE);
> > -    qtest_memwrite(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> > +    qpci_memwrite(s1->dev, s1->mem_base, data, TMPSHMSIZE);
> >      memset(data, 0, TMPSHMSIZE);
> > -    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> >      for (i = 0; i < TMPSHMSIZE; i++) {
> >          g_assert_cmpuint(data[i], ==, 0x43);
> >      }
> >  
> >      /* guest 2 write, guest 1 read */
> >      memset(data, 0x44, TMPSHMSIZE);
> > -    qtest_memwrite(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    qpci_memwrite(s2->dev, s2->mem_base, data, TMPSHMSIZE);
> >      memset(data, 0, TMPSHMSIZE);
> > -    qtest_memread(s1->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    qpci_memread(s1->dev, s2->mem_base, data, TMPSHMSIZE);
> >      for (i = 0; i < TMPSHMSIZE; i++) {
> >          g_assert_cmpuint(data[i], ==, 0x44);
> >      }
> > 
>
diff mbox

Patch

diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
index f36bfe7..97a887e 100644
--- a/tests/ivshmem-test.c
+++ b/tests/ivshmem-test.c
@@ -169,7 +169,7 @@  static void test_ivshmem_single(void)
     for (i = 0; i < G_N_ELEMENTS(data); i++) {
         data[i] = i;
     }
-    qtest_memwrite(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
+    qpci_memwrite(s->dev, s->mem_base, data, sizeof(data));
 
     /* verify write */
     for (i = 0; i < G_N_ELEMENTS(data); i++) {
@@ -178,7 +178,7 @@  static void test_ivshmem_single(void)
 
     /* read it back and verify read */
     memset(data, 0, sizeof(data));
-    qtest_memread(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
+    qpci_memread(s->dev, s->mem_base, data, sizeof(data));
     for (i = 0; i < G_N_ELEMENTS(data); i++) {
         g_assert_cmpuint(data[i], ==, i);
     }
@@ -201,29 +201,29 @@  static void test_ivshmem_pair(void)
 
     /* host write, guest 1 & 2 read */
     memset(tmpshmem, 0x42, TMPSHMSIZE);
-    qtest_memread(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
+    qpci_memread(s1->dev, s1->mem_base, data, TMPSHMSIZE);
     for (i = 0; i < TMPSHMSIZE; i++) {
         g_assert_cmpuint(data[i], ==, 0x42);
     }
-    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
+    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
     for (i = 0; i < TMPSHMSIZE; i++) {
         g_assert_cmpuint(data[i], ==, 0x42);
     }
 
     /* guest 1 write, guest 2 read */
     memset(data, 0x43, TMPSHMSIZE);
-    qtest_memwrite(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
+    qpci_memwrite(s1->dev, s1->mem_base, data, TMPSHMSIZE);
     memset(data, 0, TMPSHMSIZE);
-    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
+    qpci_memread(s2->dev, s2->mem_base, data, TMPSHMSIZE);
     for (i = 0; i < TMPSHMSIZE; i++) {
         g_assert_cmpuint(data[i], ==, 0x43);
     }
 
     /* guest 2 write, guest 1 read */
     memset(data, 0x44, TMPSHMSIZE);
-    qtest_memwrite(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
+    qpci_memwrite(s2->dev, s2->mem_base, data, TMPSHMSIZE);
     memset(data, 0, TMPSHMSIZE);
-    qtest_memread(s1->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
+    qpci_memread(s1->dev, s2->mem_base, data, TMPSHMSIZE);
     for (i = 0; i < TMPSHMSIZE; i++) {
         g_assert_cmpuint(data[i], ==, 0x44);
     }