diff mbox

[v3,2/6] target-arm: do not set do_interrupt handler for AArch64 user mode

Message ID 1409919897-360-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Sept. 5, 2014, 12:24 p.m. UTC 
From: Rob Herring <rob.herring@linaro.org>

User mode emulation should never get interrupts and thus should not
use the system emulation exception handler function. Remove the reference,
and '#ifndef USER_MODE_ONLY' the function itself as well, so that we can add
system mode only functionality to it.

Signed-off-by: Rob Herring <rob.herring@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 target-arm/cpu64.c      | 2 ++
 target-arm/helper-a64.c | 3 +++
 2 files changed, 5 insertions(+)

Comments

Peter Maydell Sept. 9, 2014, 3:25 p.m. UTC | #1 
On 5 September 2014 13:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> From: Rob Herring <rob.herring@linaro.org>
>
> User mode emulation should never get interrupts and thus should not
> use the system emulation exception handler function. Remove the reference,
> and '#ifndef USER_MODE_ONLY' the function itself as well, so that we can add
> system mode only functionality to it.
>
> Signed-off-by: Rob Herring <rob.herring@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  target-arm/cpu64.c      | 2 ++
>  target-arm/helper-a64.c | 3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index aa42803959be..9f88b9f4eea0 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -196,7 +196,9 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
>
> +#if !defined(CONFIG_USER_ONLY)
>      cc->do_interrupt = aarch64_cpu_do_interrupt;
> +#endif
>      cc->set_pc = aarch64_cpu_set_pc;
>      cc->gdb_read_register = aarch64_cpu_gdb_read_register;
>      cc->gdb_write_register = aarch64_cpu_gdb_write_register;
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 2e9ef64786ae..89b913ee9396 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -438,6 +438,8 @@ uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
>      return crc32c(acc, buf, bytes) ^ 0xffffffff;
>  }
>
> +#if !defined(CONFIG_USER_ONLY)
> +
>  /* Handle a CPU exception.  */
>  void aarch64_cpu_do_interrupt(CPUState *cs)
>  {
> @@ -512,3 +514,4 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      env->pc = addr;
>      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>  }
> +#endif

For consistency we need to not set do_interrupt in
the arm_cpu_class_init() function too. (Then we'll
have it be NULL in all cases, since v7m already
guards its assignment with an ifdef.)

thanks
-- PMM
Ard Biesheuvel Sept. 9, 2014, 3:27 p.m. UTC | #2 
On 9 September 2014 17:25, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 September 2014 13:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> From: Rob Herring <rob.herring@linaro.org>
>>
>> User mode emulation should never get interrupts and thus should not
>> use the system emulation exception handler function. Remove the reference,
>> and '#ifndef USER_MODE_ONLY' the function itself as well, so that we can add
>> system mode only functionality to it.
>>
>> Signed-off-by: Rob Herring <rob.herring@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  target-arm/cpu64.c      | 2 ++
>>  target-arm/helper-a64.c | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
>> index aa42803959be..9f88b9f4eea0 100644
>> --- a/target-arm/cpu64.c
>> +++ b/target-arm/cpu64.c
>> @@ -196,7 +196,9 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>>  {
>>      CPUClass *cc = CPU_CLASS(oc);
>>
>> +#if !defined(CONFIG_USER_ONLY)
>>      cc->do_interrupt = aarch64_cpu_do_interrupt;
>> +#endif
>>      cc->set_pc = aarch64_cpu_set_pc;
>>      cc->gdb_read_register = aarch64_cpu_gdb_read_register;
>>      cc->gdb_write_register = aarch64_cpu_gdb_write_register;
>> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
>> index 2e9ef64786ae..89b913ee9396 100644
>> --- a/target-arm/helper-a64.c
>> +++ b/target-arm/helper-a64.c
>> @@ -438,6 +438,8 @@ uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
>>      return crc32c(acc, buf, bytes) ^ 0xffffffff;
>>  }
>>
>> +#if !defined(CONFIG_USER_ONLY)
>> +
>>  /* Handle a CPU exception.  */
>>  void aarch64_cpu_do_interrupt(CPUState *cs)
>>  {
>> @@ -512,3 +514,4 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>>      env->pc = addr;
>>      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>>  }
>> +#endif
>
> For consistency we need to not set do_interrupt in
> the arm_cpu_class_init() function too. (Then we'll
> have it be NULL in all cases, since v7m already
> guards its assignment with an ifdef.)
>

Would you prefer a single patch that does both?
Or a followup patch?
Peter Maydell Sept. 9, 2014, 3:42 p.m. UTC | #3 
On 9 September 2014 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 September 2014 17:25, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For consistency we need to not set do_interrupt in
>> the arm_cpu_class_init() function too. (Then we'll
>> have it be NULL in all cases, since v7m already
>> guards its assignment with an ifdef.)
>>
>
> Would you prefer a single patch that does both?
> Or a followup patch?

I think a single patch doing both will be OK.
I don't care strongly either way.

-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index aa42803959be..9f88b9f4eea0 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -196,7 +196,9 @@  static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 {
     CPUClass *cc = CPU_CLASS(oc);
 
+#if !defined(CONFIG_USER_ONLY)
     cc->do_interrupt = aarch64_cpu_do_interrupt;
+#endif
     cc->set_pc = aarch64_cpu_set_pc;
     cc->gdb_read_register = aarch64_cpu_gdb_read_register;
     cc->gdb_write_register = aarch64_cpu_gdb_write_register;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2e9ef64786ae..89b913ee9396 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -438,6 +438,8 @@  uint64_t HELPER(crc32c_64)(uint64_t acc, uint64_t val, uint32_t bytes)
     return crc32c(acc, buf, bytes) ^ 0xffffffff;
 }
 
+#if !defined(CONFIG_USER_ONLY)
+
 /* Handle a CPU exception.  */
 void aarch64_cpu_do_interrupt(CPUState *cs)
 {
@@ -512,3 +514,4 @@  void aarch64_cpu_do_interrupt(CPUState *cs)
     env->pc = addr;
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
 }
+#endif