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

[David Gibson]

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 <address@hidden>
> 
> 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);
> >      }
> > 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature