Re: [RFC PATCH 00/25] Upstream kvx Linux port

["Arnd Bergmann" <arnd@arndb.de>]

On Thu, Jan 5, 2023, at 11:40, Jules Maselbas wrote:
> On Wed, Jan 04, 2023 at 04:58:25PM +0100, Arnd Bergmann wrote:
>> On Tue, Jan 3, 2023, at 17:43, Yann Sionneau wrote:

>> I commented on the syscall patch directly, I think it's
>> important to stop using the deprecated syscalls as soon
>> as possible to avoid having dependencies in too many
>> libc binaries. Almost everything else can be changed
>> easily as you get closer to upstream inclusion.
>> 
>> I did not receive most of the other patches as I'm
>> not subscribed to all the mainline lists. For future 
>> submissions, can you add the linux-arch list to Cc for
>> all patches?
>
> We misused get_maintainers.pl, running it on each patch instead
> of using it on the whole series. next time every one will be in
> copy of every patch in the series and including linux-arch.

Be careful not to make the list too long though, there is
usually a limit of 1024 characters for the entire Cc list,
above this your mails may get dropped by the mailing lists.

>> Reading the rest of the series through lore.kernel.org,
>> most of the comments I have are for improvements that
>> you may find valuable rather than serious mistakes:
>> 
>> - the {copy_to,copy_from,clear}_user functions are
>>   well worth optimizing better than the byte-at-a-time
>>   version you have, even just a C version built around
>>   your __get_user/__put_user inline asm should help, and
>>   could be added to lib/usercopy.c.
>
> right, we are using memcpy for {copy_to,copy_from}_user_page
> which has a simple optimized version introduced in
> (kvx: Add some library functions).
> I wonder if it is possible to do the same for copy_*_user functions.

copy_from_user_page() is only about managing cache aliases,
not user access, and it's not used anywhere in the fast
path, so it's a bit different.

arch/arm/lib/copy_template.S has an example for how
you can share the same assembler implementation between
memcpy() and copy_from_user(), but it's not very
intuitive.

The tricky bit with copy_from_user() is the partial copy
that relies on copying the exact amount of data that can
be accessed including the last byte before the end of
the mapping, and returning the correct count of non-copied
bytes.

If you want a C version, you can probably use the
copy_from_kernel_nofault implementation mm/maccess.c
as a template, but have to add code for the correct
return value.

    Arnd