Re: [PATCH RFC v2 1/2] m68k: Handle __generic_copy_to_user faults more carefully

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Finn,

Am 25.04.2024 um 18:47 schrieb Finn Thain:
> On Thu, 25 Apr 2024, Michael Schmitz wrote:
>
>> --- a/arch/m68k/lib/uaccess.c
>> +++ b/arch/m68k/lib/uaccess.c
>> @@ -68,23 +68,25 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from,
>>  		"3:	subq.l	#1,%0\n"
>>  		"4:	jne	1b\n"
>>  		"5:	btst	#1,%5\n"
>> -		"	jeq	7f\n"
>> -		"	move.w	(%1)+,%3\n"
>> -		"6:	"MOVES".w	%3,(%2)+\n"
>> -		"7:	btst	#0,%5\n"
>> -		"8:	jeq	10f\n"
>> -		"	move.b	(%1)+,%3\n"
>> -		"9:	"MOVES".b  %3,(%2)+\n"
>> -		"10:\n"
>> +		"	jeq	8f\n"
>> +		"6:	move.w	(%1)+,%3\n"
>> +		"7:	"MOVES".w	%3,(%2)+\n"
>> +		"8:	btst	#0,%5\n"
>
> I understand why you put the MOVE.W and MOVE.B into the exception table.
>
>> +		"9:	jeq	13f\n"
>> +		"10:	move.b	(%1)+,%3\n"
>> +		"11:	"MOVES".b  %3,(%2)+\n"
>> +		"12:	nop\n"
>> +		"13:\n"
>>  		"	.section .fixup,\"ax\"\n"
>>  		"	.even\n"
>>  		"20:	lsl.l	#2,%0\n"
>>  		"50:	add.l	%5,%0\n"
>> -		"	jra	10b\n"
>> +		"	jra	13b\n"
>>  		"	.previous\n"
>>  		"\n"
>>  		"	.section __ex_table,\"a\"\n"
>>  		"	.align	4\n"
>> +		"	.long	1b,20b\n"
>
> ... but I don't see why you need the MOVE.L in there (?)

It's part of a loop - if there's enough longwords to copy, that 
instruction may follow the fault instruction eventually.

> Also, it is odd to see the third JEQ added to the table but not the second.
> Perhaps we don't need either one in there (?)

Faults on 030 appear to happen two instructions after a moves. But I may 
be misremembering that...

I'll have to verify that.

Cheers,

	Michael

>>  		"	.long	2b,20b\n"
>>  		"	.long	3b,20b\n"
>>  		"	.long	4b,20b\n"
>> @@ -93,6 +95,8 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from,
>>  		"	.long	8b,50b\n"
>>  		"	.long	9b,50b\n"
>>  		"	.long	10b,50b\n"
>> +		"	.long	11b,50b\n"
>> +		"	.long	12b,50b\n"
>>  		"	.previous"
>>  		: "=d" (res), "+a" (from), "+a" (to), "=&d" (tmp)
>>  		: "0" (n / 4), "d" (n & 3));