[v3,01/26] tcg-aarch64: Properly detect SIGSEGV writes

Since the kernel doesn't pass any info on the reason for the fault,
disassemble the instruction to detect a store.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 user-exec.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

On 03.04.2014 21:56, Richard Henderson wrote:
> Since the kernel doesn't pass any info on the reason for the fault,
> disassemble the instruction to detect a store.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  user-exec.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/user-exec.c b/user-exec.c
> index bc58056..52f76c9 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -465,16 +465,33 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>  
>  #elif defined(__aarch64__)
>  
> -int cpu_signal_handler(int host_signum, void *pinfo,
> -                       void *puc)
> +int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
>  {
>      siginfo_t *info = pinfo;
>      struct ucontext *uc = puc;
> -    uint64_t pc;
> -    int is_write = 0; /* XXX how to determine? */
> +    uintptr_t pc = uc->uc_mcontext.pc;
> +    uint32_t insn = *(uint32_t *)pc;
> +    bool is_write;
>  
> -    pc = uc->uc_mcontext.pc;
> -    return handle_cpu_signal(pc, (uint64_t)info->si_addr,
> +    /* XXX: need kernel patch to get write flag faster.  */
> +    /* XXX: several of these could be combined.  */
> +    is_write = (   (insn & 0xbfff0000) == 0x0c000000   /* C3.3.1 */
> +                || (insn & 0xbfe00000) == 0x0c800000   /* C3.3.2 */
> +                || (insn & 0xbfdf0000) == 0x0d000000   /* C3.3.3 */
> +                || (insn & 0xbfc00000) == 0x0d800000   /* C3.3.4 */
> +                || (insn & 0x3f400000) == 0x08000000   /* C3.3.6 */
> +                || (insn & 0x3bc00000) == 0x28400000   /* C3.3.7 */

I think the Load (L) bit should be 0 here so

== 0x28000000

> +                || (insn & 0x3be00c00) == 0x38000400   /* C3.3.8 */

With V=1, an opc of 0b10 is also a write, I think. It's the 128bit FP/SIMD STR.

> +                || (insn & 0x3be00c00) == 0x38000c00   /* C3.3.9 */

Same here.

> +                || (insn & 0x3be00c00) == 0x38200800   /* C3.3.10 */

Same.

> +                || (insn & 0x3be00c00) == 0x38000800   /* C3.3.11 */
> +                || (insn & 0x3be00c00) == 0x38000000   /* C3.3.12 */

Same.

> +                || (insn & 0x3bc00000) == 0x39000000   /* C3.3.13 */

Same.

> +                || (insn & 0x3bc00000) == 0x29000000   /* C3.3.14 */
> +                || (insn & 0x3bc00000) == 0x28800000   /* C3.3.15 */
> +                || (insn & 0x3bc00000) == 0x29800000); /* C3.3.16 */
> +
> +    return handle_cpu_signal(pc, (uintptr_t)info->si_addr,
>                               is_write, &uc->uc_sigmask, puc);
>  }
>  
> 

Thanks,

Claudio

On 04/07/2014 12:58 AM, Claudio Fontana wrote:
>> +                || (insn & 0x3bc00000) == 0x28400000   /* C3.3.7 */
> 
> I think the Load (L) bit should be 0 here so
> 
> == 0x28000000

Oops.  Fixed.

> 
>> +                || (insn & 0x3be00c00) == 0x38000400   /* C3.3.8 */
> 
> With V=1, an opc of 0b10 is also a write, I think. It's the 128bit FP/SIMD STR.

Exactly, that's why I'm masking it out, to ignore it.

 insn  =  size 1 1   1 v 0 0 ...
 mask  =   0 0 1 1   1 0 1 1 ...  = 0x3b...
 equal =   0 0 1 1   1 0 0 0 ...  = 0x38...


r~

On 3 April 2014 20:56, Richard Henderson <rth@twiddle.net> wrote:
> Since the kernel doesn't pass any info on the reason for the fault,

There are now patches proposed to the kernel to supply this:
http://www.spinics.net/lists/arm-kernel/msg320268.html

thanks
-- PMM

On 07.04.2014 18:33, Richard Henderson wrote:
> On 04/07/2014 12:58 AM, Claudio Fontana wrote:
>>> +                || (insn & 0x3bc00000) == 0x28400000   /* C3.3.7 */
>>
>> I think the Load (L) bit should be 0 here so
>>
>> == 0x28000000
> 
> Oops.  Fixed.
> 
>>
>>> +                || (insn & 0x3be00c00) == 0x38000400   /* C3.3.8 */
>>
>> With V=1, an opc of 0b10 is also a write, I think. It's the 128bit FP/SIMD STR.
> 
> Exactly, that's why I'm masking it out, to ignore it.
> 
>  insn  =  size 1 1   1 v 0 0 ...
>  mask  =   0 0 1 1   1 0 1 1 ...  = 0x3b...
>  equal =   0 0 1 1   1 0 0 0 ...  = 0x38...
> 
> 
> r~
> 

the problem is not in the two nibbles you show, but in the third nibble:
31 30 29 28  27 26 25 24  23 22 21 20
 size  1  1   1  v  0  0   opc   0  x

the third nibble in your mask is 'E' and the expected result is 0, which forces opc to be 0 for writes.
However, for 128bit SIMD STR, the opc is 2 (0b10).

Ciao,

Claudio

On 04/14/2014 04:32 AM, Claudio Fontana wrote:
> the problem is not in the two nibbles you show, but in the third nibble:
> 31 30 29 28  27 26 25 24  23 22 21 20
>  size  1  1   1  v  0  0   opc   0  x
> 
> the third nibble in your mask is 'E' and the expected result is 0, which forces opc to be 0 for writes.
> However, for 128bit SIMD STR, the opc is 2 (0b10).

Ah, I see what you mean.

Unfortunately, this means a simple mask won't work for this case;
it'll take two masks to decode that properly.


r~