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

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

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>