netdev - Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cfcbb987-89b9-dff2-bd88-abffb9c8cbc6@codeaurora.org>
Date:   Fri, 20 Nov 2020 19:44:30 -0800
From:   Hemant Kumar <hemantk@...eaurora.org>
To:     Shuah Khan <skhan@...uxfoundation.org>,
        manivannan.sadhasivam@...aro.org, gregkh@...uxfoundation.org
Cc:     linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        jhugo@...eaurora.org, bbhatt@...eaurora.org,
        loic.poulain@...aro.org, netdev@...r.kernel.org
Subject: Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK
 channel

Hi Shuah,

On 11/20/20 7:26 AM, Shuah Khan wrote:
> On 11/16/20 3:46 PM, Hemant Kumar wrote:
>> Loopback test opens the MHI device file node and writes
>> a data buffer to it. MHI UCI kernel space driver copies
>> the data and sends it to MHI uplink (Tx) LOOPBACK channel.
>> MHI device loops back the same data to MHI downlink (Rx)
>> LOOPBACK channel. This data is read by test application
>> and compared against the data sent. Test passes if data
>> buffer matches between Tx and Rx. Test application performs
>> open(), poll(), write(), read() and close() file operations.
>>
>> Signed-off-by: Hemant Kumar <hemantk@...eaurora.org>
>> ---
>>   Documentation/mhi/uci.rst                          |  32 +
>>   tools/testing/selftests/Makefile                   |   1 +
>>   tools/testing/selftests/drivers/.gitignore         |   1 +
>>   tools/testing/selftests/drivers/mhi/Makefile       |   8 +
>>   tools/testing/selftests/drivers/mhi/config         |   2 +
>>   .../testing/selftests/drivers/mhi/loopback_test.c  | 802 
>> +++++++++++++++++++++
>>   6 files changed, 846 insertions(+)
>>   create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
>>   create mode 100644 tools/testing/selftests/drivers/mhi/config
>>   create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c
>>
>> diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
>> index ce8740e..0a04afe 100644
>> --- a/Documentation/mhi/uci.rst
>> +++ b/Documentation/mhi/uci.rst
>> @@ -79,6 +79,38 @@ MHI client driver performs read operation, same 
>> data gets looped back to MHI
>>   host using LOOPBACK channel 1. LOOPBACK channel is used to verify 
>> data path
>>   and data integrity between MHI Host and MHI device.
> 
> Nice.
[..]
>> +
>> +enum debug_level {
>> +    DBG_LVL_VERBOSE,
>> +    DBG_LVL_INFO,
>> +    DBG_LVL_ERROR,
>> +};
>> +
>> +enum test_status {
>> +    TEST_STATUS_SUCCESS,
>> +    TEST_STATUS_ERROR,
>> +    TEST_STATUS_NO_DEV,
>> +};
>> +
> 
> Since you are running this test as part of kselftest, please use
> ksft errors nd returns.
Are you suggesting to use following macros instead of test_status enum ?
#define KSFT_PASS  0
#define KSFT_FAIL  1

> 
>> +struct lb_test_ctx {
>> +    char dev_node[256];
>> +    unsigned char *tx_buff;
>> +    unsigned char *rx_buff;
>> +    unsigned int rx_pkt_count;
>> +    unsigned int tx_pkt_count;
>> +    int iterations;
>> +    bool iter_complete;
>> +    bool comp_complete;
>> +    bool test_complete;
>> +    bool all_complete;
>> +    unsigned long buff_size;
>> +    long byte_recvd;
>> +    long byte_sent;
>> +};
>> +
>> +bool force_exit;
>> +char write_data = 'a';
>> +int completed_iterations;
>> +
>> +struct lb_test_ctx test_ctxt;
>> +enum debug_level msg_lvl;
>> +struct pollfd read_fd;
>> +struct pollfd write_fd;
>> +enum test_status mhi_test_return_value;
>> +enum test_status tx_status;
>> +enum test_status rx_status;
>> +enum test_status cmp_rxtx_status;
>> +
>> +#define test_log(test_msg_lvl, format, ...) do { \
>> +        if (test_msg_lvl >= msg_lvl) \
>> +            fprintf(stderr, format, ##__VA_ARGS__); \
>> +} while (0)
>> +
>> +static void loopback_test_sleep_ms(int ms)
>> +{
>> +    usleep(1000 * ms);
>> +}
>> +
> 
> Have you run this as part of "make kselftest" run. How does this
> sleep work in that env.?
Looks like kselftest runs this test application by directly executing 
the binary, but this test application requires a valid mhi device file 
node as a required parameter. So considering that requirement, is this 
test application qualifies to run using kselftest ? Without a valid 
device file node test would fail. Is there an option to run this test as 
standalone test which can only be run when a mhi device file node is 
present ? Having said that i tested this driver by
directly executing it using the test binary which is compiled using
make loopback_test under mhi dir.

> Are there any cases where this test can't run and have to - those
> cases need to be skips.
Yes, as this test application can not run by itself it needs a valid mhi 
device file node to write and test reads the same device node to get the 
data back.
So test can not be run without having a MHI device connected over a 
transport (in my testing MHI device is connected over PCIe). Could you 
please suggest an option to use this test application as a standalone 
test instead of being part of kselftest?
> 
> thanks,
> -- Shuah

Thanks,
Hemant
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project