linux-kernel - Re: [PATCH v8 1/4] fpga: add an initial KUnit suite for the FPGA Manager

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: <1d03a695-d603-a4d5-b726-4b0ff6bc33f8@redhat.com>
Date:   Tue, 11 Jul 2023 16:02:44 +0200
From:   Marco Pagani <marpagan@...hat.com>
To:     Xu Yilun <yilun.xu@...el.com>
Cc:     Moritz Fischer <mdf@...nel.org>, Wu Hao <hao.wu@...el.com>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        linux-fpga@...r.kernel.org
Subject: Re: [PATCH v8 1/4] fpga: add an initial KUnit suite for the FPGA
 Manager



On 2023-07-10 06:44, Xu Yilun wrote:
> On 2023-06-30 at 17:25:04 +0200, Marco Pagani wrote:
>> The suite tests the basic behaviors of the FPGA Manager including
>> programming using a single contiguous buffer and a scatter gather table.
>>
>> Signed-off-by: Marco Pagani <marpagan@...hat.com>
>> ---
>>  drivers/fpga/tests/fpga-mgr-test.c | 311 +++++++++++++++++++++++++++++
>>  1 file changed, 311 insertions(+)
>>  create mode 100644 drivers/fpga/tests/fpga-mgr-test.c
>>
>> diff --git a/drivers/fpga/tests/fpga-mgr-test.c b/drivers/fpga/tests/fpga-mgr-test.c
>> new file mode 100644
>> index 000000000000..6fd2e235f195
>> --- /dev/null
>> +++ b/drivers/fpga/tests/fpga-mgr-test.c
>> @@ -0,0 +1,311 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KUnit test for the FPGA Manager
>> + *
>> + * Copyright (C) 2023 Red Hat, Inc.
>> + *
>> + * Author: Marco Pagani <marpagan@...hat.com>
>> + */
>> +

[...]

>> +static int op_write(struct fpga_manager *mgr, const char *buf, size_t count)
>> +{
>> +	struct mgr_stats *stats = mgr->priv;
>> +	size_t i;
>> +
>> +	/* Check the image */
>> +	stats->image_match = true;
>> +	for (i = 0; i < count; i++)
>> +		if (buf[i] != IMAGE_FILL)
>> +			stats->image_match = false;
>> +
>> +	stats->op_write_state = mgr->state;
>> +	stats->op_write_seq = stats->seq_num++;
>> +
>> +	return 0;
>> +}
>> +
>> +static int op_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
>> +{
>> +	struct mgr_stats *stats = mgr->priv;
>> +	struct sg_mapping_iter miter;
>> +	char *img;
>> +	size_t i;
>> +
>> +	/*
>> +	 * Check the image, but first skip the header since write_sg will get
>> +	 * the whole image in sg_table.
>> +	 */
>> +	stats->image_match = true;
>> +	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
>> +
>> +	if (!sg_miter_skip(&miter, HEADER_SIZE))
>> +		stats->image_match = false;
> 
> If this fails, should we continue?

Would it be okay to set the image_match flag to false and then
return 0 if sg_miter_skip() fails?

I think returning an error code to the FPGA manager would not
be beneficial in this case since if an op fails, it is a failure
of the FPGA manager itself, not the low-level driver that tests
the FPGA manager.


> 
>> +
>> +	while (sg_miter_next(&miter)) {
>> +		img = miter.addr;
>> +		for (i = 0; i < miter.length; i++) {
>> +			if (img[i] != IMAGE_FILL)
>> +				stats->image_match = false;
>> +		}
>> +	}
>> +
>> +	sg_miter_stop(&miter);
>> +
>> +	stats->op_write_sg_state = mgr->state;
>> +	stats->op_write_sg_seq = stats->seq_num++;
>> +
>> +	return 0;
>> +}
>