Re: Dovetail on risc-v: Testing strategy, managing efforts for forward porting

"Bezdeka, Florian" <[email protected]> writes:

> On Fri, 2026-05-29 at 12:49 +0200, Florian Bezdeka wrote:
>> On Fri, 2026-05-29 at 12:43 +0200, Philippe Gerum wrote:
>> > Jan Kiszka <[email protected]> writes:
>> > 
>> > > On 29.05.26 12:08, Philippe Gerum wrote:
>> > > > 
>> > > > Florian Bezdeka <[email protected]> writes:
>> > > > 
>> > > > > Hi Philippe,
>> > > > > 
>> > > > > I'm currently waiting for a v8 of Tobias' risc-v implementation. A
>> > > > > couple of minor things still need fixing, but overall I would say that
>> > > > > we are looking good - ready for merging - assuming that currently
>> > > > > running tests are successful.
>> > > > > 
>> > > > 
>> > > > Really nice stuff, thanks to anyone involved.
>> > > > 
>> > > > > From my point of view there is one issue left, that needs a discussion:
>> > > > > Testing and forward porting in the future.
>> > > > > 
>> > > > > Up to now I was able to test all architectures using cobalt / x3. That
>> > > > > will change now, as we don't have risc-v support in x3.
>> > > > > 
>> > > > > I see the following options:
>> > > > > 
>> > > > > 1: Additionally forward port linux-evl in the future, so that we can run
>> > > > > tests against risc-v using x4. Additional effort for me, especially
>> > > > > because we don't have an in-house user for that. Can you estimate how
>> > > > > much time it takes you to forward port linux-evl in comparison to linux-
>> > > > > dovetail?
>> > > > > 
>> > > > 
>> > > > Porting evl to a new arch kernel-wise is fundamentally trivial, the real
>> > > > work is done by Dovetail. For every currently supported arch, the amount
>> > > > of arch-specific evl code is 60 lines of C, merely wrappers in headers
>> > > > (arch/*/include/asm/evl/*.h). Tobias already contributed the few bits
>> > > > libevl needs to support risc-v.
>> > > > 
>> > > > > 2: Add risc-v support to x3 as well. To my understanding there were no
>> > > > > plans yet, but it might be worth it to limit forward porting efforts. I
>> > > > > could focus on x3 for testing. This might also help to compare risc-v
>> > > > > against the other architectures.
>> > > > > 
>> > > > 
>> > > > Although this may require a little more effort compared to x4, such port
>> > > > would still be tractable since Dovetail does most of the work here as
>> > > > well. Adding risc-v support to x3 is Jan's call.
>> > > > 
>> > > > > 3: Ignore risc-v during forward porting. Yes, this option exists, but I
>> > > > > don't consider it as a real / valid option.
>> > > > > 
>> > > > 
>> > > > Neither do I. Merged implies maintained.
>> > > > 
>> > > > > 4: Merge the risc-v support into linux-evl. That would make risc-v
>> > > > > support exclusive to evl, where we could already test it. Would that be
>> > > > > an valid option for the evl side?
>> > > > > 
>> > > > 
>> > > > No, I believe that we can deal with one arch more such as risc-v in the
>> > > > Dovetail maintenance process. Besides, we really want the generic
>> > > > Dovetail code to be shared by all architectures, otherwise this would
>> > > > become a maintenance nightmare for the risc-v port.
>> > > > 
>> > > > > Any additional option available, that I missed?
>> > > > > 
>> > > > 
>> > > > I'd say, the simpler the better. Today, enabling risc-v for x4 would be
>> > > > straightforward while keeping the current maintenance model for Dovetail
>> > > > mostly unaffected.  Granted, forward porting to the latest kernel
>> > > > release may require a bit more work in order to maintain the risc-v part
>> > > > too, but my understanding is that Tobias is willing to help with this in
>> > > > the long term. AFAICS, the arch-specific part in risc-v for Dovetail is
>> > > > fairly reasonable in size as well.
>> > > > 
>> > > > So, first step would be to merge the Dovetail risc-v support into
>> > > > linux-dovetail like other supported archs, enabling it for x4/evl in the
>> > > > same move. It could be enabled for x3 next, when time allows on your end
>> > > > if desired.
>> > > > 
>> > > 
>> > > To my understanding, Florian's concerns are not about the first-time
>> > > enabling or risc-v but its future testing when going to a new kernel
>> > > version. While arch-wise, most work is done in dovetail, testing would
>> > > only be possible once linux-evl is ported to a new kernel as well. And
>> > > that porting is widely arch-agnostic but for other reasons (drivers
>> > > e.g.) quite some generic effort.
>> > 
>> > This proved to be a limited effort over time. Porting issues in generic
>> > driver code for linux-evl can only affect the GPIO core, all the rest is
>> > on the Dovetail side (NIC drivers, SPI and DMA cores), so already ported
>> > actually.
>> > 
>> > Those issues did happen but rarely, and the complexity to address them
>> > always were an order of magnitude smaller than it has been for Dovetail
>> > over time. Most importantly, those merge conflicts were all due to
>> > changes in symbol names and routines moving around due to upstream
>> > refactoring/cleaning up the code, still keeping the original logic
>> > untouched though. I'd say such porting work still qualifies as simple.
>> 
>> With that: Let me try it. Which branch should be the "source" for v7.1-
>> evl-rebase? next/v7.0-evl-rebase or v7.0-evl-rebase? (with or without
>> next/)
>> 
>> I would do it the same way as for Dovetail: I would push to something
>> like wip/<release>-rebase and ask you to take over / release. Right?
>> 
>> Florian
>
> So, first evl rebase done. Result available at [1], please review and
> spawn tests.
>
> I had to fix a merge conflict in drivers/pio/gpiolib-cdev.c,
> lineevent_create, where we had the following code flow:
>
> 	if (oob_handling_requested(lflags)) {
> 		ret = evl_open_file(&le->oob_state.efile, file);
> 		if (ret)
> 			goto out_put_file;
> 	}
>
> 	eventreq.fd = fd;
> 	if (copy_to_user(ip, &eventreq, sizeof(eventreq))) {
> 		if (oob_handling_requested(lflags))
> 			evl_release_file(&le->oob_state.efile);
>  
> 		ret = -EFAULT;
> 		goto out_put_file;
> 	}
>
> 	return ret;
>
> out_put_file:
> 	fput(file)
>
>
> I completely removed the fput() call as I considered that a resource-
> count underflow. evl_open_file() will always return 0 -

In the current implementation, yes. This might change.

> still keeping
> the error handling - and if copy_to_user() failed, I think there is no
> reason to call it. Right?

Since 7.1 uses RAII-style management via FD_PREPARE() and friends, no
need for explicit fput() anymore indeed.

>
> [1] https://gitlab.com/Xenomai/xenomai4/linux-evl/-/tree/wip/flo/v7.1-evl-rebase

-- 
Philippe.