[Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.

Sandra Loosemore posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1536701379-106032-1-git-send-email-sandra@codesourcery.com
Test docker-clang@ubuntu failed
Test checkpatch passed
linux-user/nios2/cpu_loop.c | 3 +++
1 file changed, 3 insertions(+)
[Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.
Posted by Sandra Loosemore 7 years, 1 month ago
Without this patch, QEMU exits immediately when it execution stops at
a breakpoint, instead of reporting it to GDB.

Signed-off-by: Sandra Loosemore <sandra@codesourcery.com>
---
 linux-user/nios2/cpu_loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index dac7a06..a5ae37f 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
                 gdbsig = TARGET_SIGTRAP;
                 break;
             }
+        case EXCP_DEBUG:
+            gdbsig = TARGET_SIGTRAP;
+            break;
         case 0xaa:
             switch (env->regs[R_PC]) {
             /*case 0x1000:*/  /* TODO:__kuser_helper_version */
-- 
2.8.1


Re: [Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.
Posted by Richard Henderson 7 years, 1 month ago
On 09/11/2018 02:29 PM, Sandra Loosemore wrote:
> Without this patch, QEMU exits immediately when it execution stops at
> a breakpoint, instead of reporting it to GDB.
> 
> Signed-off-by: Sandra Loosemore <sandra@codesourcery.com>
> ---
>  linux-user/nios2/cpu_loop.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
> index dac7a06..a5ae37f 100644
> --- a/linux-user/nios2/cpu_loop.c
> +++ b/linux-user/nios2/cpu_loop.c
> @@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
>                  gdbsig = TARGET_SIGTRAP;
>                  break;
>              }
> +        case EXCP_DEBUG:
> +            gdbsig = TARGET_SIGTRAP;
> +            break;

This really isn't complete.  You set gdbsig from odd places instead of using
queue_signal; you fail to honor the return value from gdb_handlesig.

But I suppose those should be separate patches, so
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.
Posted by Sandra Loosemore 7 years, 1 month ago
On 09/12/2018 11:49 AM, Richard Henderson wrote:
> On 09/11/2018 02:29 PM, Sandra Loosemore wrote:
>> Without this patch, QEMU exits immediately when it execution stops at
>> a breakpoint, instead of reporting it to GDB.
>>
>> Signed-off-by: Sandra Loosemore <sandra@codesourcery.com>
>> ---
>>   linux-user/nios2/cpu_loop.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
>> index dac7a06..a5ae37f 100644
>> --- a/linux-user/nios2/cpu_loop.c
>> +++ b/linux-user/nios2/cpu_loop.c
>> @@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
>>                   gdbsig = TARGET_SIGTRAP;
>>                   break;
>>               }
>> +        case EXCP_DEBUG:
>> +            gdbsig = TARGET_SIGTRAP;
>> +            break;
> 
> This really isn't complete.  You set gdbsig from odd places instead of using
> queue_signal; you fail to honor the return value from gdb_handlesig.
> 
> But I suppose those should be separate patches, so
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Yes, I thought this code was rather ugly.  But I thought a minimal patch 
to un-break GDB use would be easier to get in than any kind of rewrite, 
particularly given that I'm not terribly familiar with current best 
practices in the QEMU development community, etc.

-Sandra

Re: [Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.
Posted by Alex Bennée 7 years, 1 month ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 09/11/2018 02:29 PM, Sandra Loosemore wrote:
>> Without this patch, QEMU exits immediately when it execution stops at
>> a breakpoint, instead of reporting it to GDB.
>>
>> Signed-off-by: Sandra Loosemore <sandra@codesourcery.com>
>> ---
>>  linux-user/nios2/cpu_loop.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
>> index dac7a06..a5ae37f 100644
>> --- a/linux-user/nios2/cpu_loop.c
>> +++ b/linux-user/nios2/cpu_loop.c
>> @@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
>>                  gdbsig = TARGET_SIGTRAP;
>>                  break;
>>              }
>> +        case EXCP_DEBUG:
>> +            gdbsig = TARGET_SIGTRAP;
>> +            break;
>
> This really isn't complete.  You set gdbsig from odd places instead of using
> queue_signal; you fail to honor the return value from gdb_handlesig.
>
> But I suppose those should be separate patches, so
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

At least the cpu_loops have been separated now.. I guess the next step
is to audit each one for common features? There do seem to be some magic
numbers in the nios loop which I find concerning.

>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.
Posted by Sandra Loosemore 7 years, 1 month ago
On 09/12/2018 12:39 PM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 09/11/2018 02:29 PM, Sandra Loosemore wrote:
>>> Without this patch, QEMU exits immediately when it execution stops at
>>> a breakpoint, instead of reporting it to GDB.
>>>
>>> Signed-off-by: Sandra Loosemore <sandra@codesourcery.com>
>>> ---
>>>   linux-user/nios2/cpu_loop.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
>>> index dac7a06..a5ae37f 100644
>>> --- a/linux-user/nios2/cpu_loop.c
>>> +++ b/linux-user/nios2/cpu_loop.c
>>> @@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
>>>                   gdbsig = TARGET_SIGTRAP;
>>>                   break;
>>>               }
>>> +        case EXCP_DEBUG:
>>> +            gdbsig = TARGET_SIGTRAP;
>>> +            break;
>>
>> This really isn't complete.  You set gdbsig from odd places instead of using
>> queue_signal; you fail to honor the return value from gdb_handlesig.
>>
>> But I suppose those should be separate patches, so
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> At least the cpu_loops have been separated now.. I guess the next step
> is to audit each one for common features? There do seem to be some magic
> numbers in the nios loop which I find concerning.

I'm not sure where the 0xaa value came from, but it is used to indicate 
syscalls through the kuser page mapped at address 0x1000 in user space. 
The addresses are a fixed ABI although not terribly well-documented 
AFAICT.  There's code in target/nios2 that catches attempts to execute 
code on that page, and in the Linux kernel the code that goes on the 
kuser page is at the end of arch/nios2/kernel/entry.S.  This all could 
certainly be better documented in the QEMU cpu_loop code too.

-Sandra