Re: [PATCH 3/3] tests: Aspeed HACE Scatter-Gather tests

[Klaus Heinrich Kiwi]

Hi Joel,


On 3/24/2021 11:18 PM, Joel Stanley wrote:

> > +    const uint32_t src_addr_1 = src_addr + 0x1000000;
> > +    const uint32_t src_addr_2 = src_addr + 0x2000000;
> > +    const uint32_t src_addr_3 = src_addr + 0x3000000;
> > +    const uint32_t digest_addr = src_addr + 0x4000000;
> > +    uint8_t digest[32] = {0};
> > +    struct aspeed_sg_list array[] = {
> > +            { sizeof(test_vector_sg1),              src_addr_1},
> > +            { sizeof(test_vector_sg2),              src_addr_2},
> > +            { sizeof(test_vector_sg3) | 1u << 31,   src_addr_3},
>
> These sizeofs are always going to be 3.

I was trying to not hard-code 3 here and perhaps allow for more clarity and
extensibility, in case we ever decide to use larger vectors?

 
> I assume 1 << 31 is to indicate the final entry? Perhaps add a define for it.
ack

> > +        };
> > +
> > +    /* Check engine is idle, no busy or irq bits set */
> > +    g_assert_cmphex(qtest_readl(s, base + HACE_STS), ==, 0);
> > +
> > +    /* Write test vector into memory */
> > +    qtest_memwrite(s, src_addr_1, test_vector_sg1, sizeof(test_vector_sg1));
> > +    qtest_memwrite(s, src_addr_2, test_vector_sg2, sizeof(test_vector_sg2));
> > +    qtest_memwrite(s, src_addr_3, test_vector_sg3, sizeof(test_vector_sg3));
>
> It would simplify your test case if you wrote the test vector to the
> one memory location.

I guess I like that each vector is significantly displaced from each other, as 
this
is supported by the hardware.

> >   struct masks {
> > -    uint32_t src;
> > +    uint32_t src_direct;
>
> You could leave this one the same.
>
> > +    uint32_t src_sg;
>
> You add this, but haven't written a new test to use it.

Turns out there's nothing special about src/dst/len masking
when using HASH_SG_EN, so I'll remove those.

Thanks,

 -Klaus

--
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>