diff mbox

[RFC,v1,2/3] target-microblaze: Allow the stack protection to be disabled

Message ID b4018252e8e406d77d818146b47dd84c7af77220.1431583229.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis May 14, 2015, 6:08 a.m. UTC 
Microblaze stack protection is configurable and isn't always enabled.
This patch allows the stack protection to be disabled from the CPU
properties.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 target-microblaze/cpu-qom.h   |    5 +++++
 target-microblaze/cpu.c       |    2 ++
 target-microblaze/op_helper.c |    4 +++-
 3 files changed, 10 insertions(+), 1 deletions(-)

Comments

Richard Henderson May 14, 2015, 6:36 p.m. UTC | #1 
On 05/13/2015 11:08 PM, Alistair Francis wrote:
>  void helper_stackprot(CPUMBState *env, uint32_t addr)
>  {
> -    if (addr < env->slr || addr > env->shr) {
> +    MicroBlazeCPU *cpu = mb_env_get_cpu(env);
> +
> +    if (cpu->cfg.stackproc && (addr < env->slr || addr > env->shr)) {

This check should be done in translate.c to avoid calling this function in the
first place.


r~
Alistair Francis May 14, 2015, 10:53 p.m. UTC | #2 
On Fri, May 15, 2015 at 4:36 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/13/2015 11:08 PM, Alistair Francis wrote:
>>  void helper_stackprot(CPUMBState *env, uint32_t addr)
>>  {
>> -    if (addr < env->slr || addr > env->shr) {
>> +    MicroBlazeCPU *cpu = mb_env_get_cpu(env);
>> +
>> +    if (cpu->cfg.stackproc && (addr < env->slr || addr > env->shr)) {
>
> This check should be done in translate.c to avoid calling this function in the
> first place.

Looking at it again I agree, I will move it.

Thanks,

Alistair

>
>
> r~
>
Peter Crosthwaite May 15, 2015, 5:26 a.m. UTC | #3 
On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Microblaze stack protection is configurable and isn't always enabled.
> This patch allows the stack protection to be disabled from the CPU
> properties.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  target-microblaze/cpu-qom.h   |    5 +++++
>  target-microblaze/cpu.c       |    2 ++
>  target-microblaze/op_helper.c |    4 +++-
>  3 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index e3e0701..7bc5b81 100644
> --- a/target-microblaze/cpu-qom.h
> +++ b/target-microblaze/cpu-qom.h
> @@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
>      uint32_t base_vectors;
>      /*< public >*/
>
> +    /* Microblaze Configuration Settings */
> +    struct {
> +        bool stackproc;
> +    } cfg;
> +
>      CPUMBState env;
>  } MicroBlazeCPU;
>
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 67e3182..c08da19 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -152,6 +152,8 @@ static const VMStateDescription vmstate_mb_cpu = {
>
>  static Property mb_properties[] = {
>      DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
> +    DEFINE_PROP_BOOL("xlnx.use-stack-protection", MicroBlazeCPU, cfg.stackproc,
> +                     true),

This should deposit (at realize time) into pvr[0].SPROT bit.

Regards,
Peter

>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
> index d2b3624..24df538 100644
> --- a/target-microblaze/op_helper.c
> +++ b/target-microblaze/op_helper.c
> @@ -467,7 +467,9 @@ void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
>
>  void helper_stackprot(CPUMBState *env, uint32_t addr)
>  {
> -    if (addr < env->slr || addr > env->shr) {
> +    MicroBlazeCPU *cpu = mb_env_get_cpu(env);
> +
> +    if (cpu->cfg.stackproc && (addr < env->slr || addr > env->shr)) {
>          qemu_log("Stack protector violation at %x %x %x\n",
>                   addr, env->slr, env->shr);
>          env->sregs[SR_EAR] = addr;
> --
> 1.7.1
>
>
Alistair Francis May 15, 2015, 6:51 a.m. UTC | #4 
On Fri, May 15, 2015 at 3:26 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> Microblaze stack protection is configurable and isn't always enabled.
>> This patch allows the stack protection to be disabled from the CPU
>> properties.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  target-microblaze/cpu-qom.h   |    5 +++++
>>  target-microblaze/cpu.c       |    2 ++
>>  target-microblaze/op_helper.c |    4 +++-
>>  3 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
>> index e3e0701..7bc5b81 100644
>> --- a/target-microblaze/cpu-qom.h
>> +++ b/target-microblaze/cpu-qom.h
>> @@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU {
>>      uint32_t base_vectors;
>>      /*< public >*/
>>
>> +    /* Microblaze Configuration Settings */
>> +    struct {
>> +        bool stackproc;
>> +    } cfg;
>> +
>>      CPUMBState env;
>>  } MicroBlazeCPU;
>>
>> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
>> index 67e3182..c08da19 100644
>> --- a/target-microblaze/cpu.c
>> +++ b/target-microblaze/cpu.c
>> @@ -152,6 +152,8 @@ static const VMStateDescription vmstate_mb_cpu = {
>>
>>  static Property mb_properties[] = {
>>      DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
>> +    DEFINE_PROP_BOOL("xlnx.use-stack-protection", MicroBlazeCPU, cfg.stackproc,
>> +                     true),
>
> This should deposit (at realize time) into pvr[0].SPROT bit.

Thanks Peter. I will add that.

Thanks,

Alistair

>
> Regards,
> Peter
>
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
>> index d2b3624..24df538 100644
>> --- a/target-microblaze/op_helper.c
>> +++ b/target-microblaze/op_helper.c
>> @@ -467,7 +467,9 @@ void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
>>
>>  void helper_stackprot(CPUMBState *env, uint32_t addr)
>>  {
>> -    if (addr < env->slr || addr > env->shr) {
>> +    MicroBlazeCPU *cpu = mb_env_get_cpu(env);
>> +
>> +    if (cpu->cfg.stackproc && (addr < env->slr || addr > env->shr)) {
>>          qemu_log("Stack protector violation at %x %x %x\n",
>>                   addr, env->slr, env->shr);
>>          env->sregs[SR_EAR] = addr;
>> --
>> 1.7.1
>>
>>
>
diff mbox

Patch

diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
index e3e0701..7bc5b81 100644
--- a/target-microblaze/cpu-qom.h
+++ b/target-microblaze/cpu-qom.h
@@ -59,6 +59,11 @@  typedef struct MicroBlazeCPU {
     uint32_t base_vectors;
     /*< public >*/
 
+    /* Microblaze Configuration Settings */
+    struct {
+        bool stackproc;
+    } cfg;
+
     CPUMBState env;
 } MicroBlazeCPU;
 
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 67e3182..c08da19 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -152,6 +152,8 @@  static const VMStateDescription vmstate_mb_cpu = {
 
 static Property mb_properties[] = {
     DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
+    DEFINE_PROP_BOOL("xlnx.use-stack-protection", MicroBlazeCPU, cfg.stackproc,
+                     true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index d2b3624..24df538 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -467,7 +467,9 @@  void helper_memalign(CPUMBState *env, uint32_t addr, uint32_t dr, uint32_t wr,
 
 void helper_stackprot(CPUMBState *env, uint32_t addr)
 {
-    if (addr < env->slr || addr > env->shr) {
+    MicroBlazeCPU *cpu = mb_env_get_cpu(env);
+
+    if (cpu->cfg.stackproc && (addr < env->slr || addr > env->shr)) {
         qemu_log("Stack protector violation at %x %x %x\n",
                  addr, env->slr, env->shr);
         env->sregs[SR_EAR] = addr;