Date: Fri, 29 May 2015 16:46:53 +1000
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
Message-ID: <20150529064653.GB30952@toto>
References: <cover.1432879373.git.alistair.francis@xilinx.com>
	<386d1ca76a22f628d660feb134db9e87e76147f6.1432879373.git.alistair.francis@xilinx.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <386d1ca76a22f628d660feb134db9e87e76147f6.1432879373.git.alistair.francis@xilinx.com>
Subject: Re: [Qemu-devel] [PATCH v3 3/6] target-microblaze: Allow the stack
 protection to be disabled
To: Alistair Francis <alistair.francis@xilinx.com>
Cc: peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org, afaerber@suse.de, rth@twiddle.net

On Fri, May 29, 2015 at 04:30:43PM +1000, Alistair Francis 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>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
> V3:
>  - Enable stack protection by default
> V2:
>  - Change the variable name to stackprot
>  - Include protection for the second time stack protection
>    is enabled
>  - Disable stack protection by default
> Changes since RFC:
>  - Move the cfg.stackproc check into translate.c
>  - Set the PVR register
> 
>  target-microblaze/cpu-qom.h   |    5 +++++
>  target-microblaze/cpu.c       |    5 +++++
>  target-microblaze/cpu.h       |    1 +
>  target-microblaze/translate.c |    4 ++--
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h
> index e3e0701..e08adb9 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 stackprot;
> +    } cfg;
> +
>      CPUMBState env;
>  } MicroBlazeCPU;
>  
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 95be540..d3dad4a 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -114,6 +114,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
>                          | PVR2_USE_FPU2_MASK \
>                          | PVR2_FPU_EXC_MASK \
>                          | 0;
> +
> +    env->pvr.regs[0] |= cpu->cfg.stackprot ? PVR0_SPROT_MASK : 0;
> +
>      env->pvr.regs[10] = 0x0c000000; /* Default to spartan 3a dsp family.  */
>      env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17);
>  
> @@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = {
>  
>  static Property mb_properties[] = {
>      DEFINE_PROP_UINT32("xlnx.base-vectors", MicroBlazeCPU, base_vectors, 0),
> +    DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
> +                     true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index e4c1cde..481f463 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState;
>  #define PVR0_FAULT			0x00100000
>  #define PVR0_VERSION_MASK               0x0000FF00
>  #define PVR0_USER1_MASK                 0x000000FF
> +#define PVR0_SPROT_MASK                 0x00000001
>  
>  /* User 2 PVR mask */
>  #define PVR1_USER2_MASK                 0xFFFFFFFF
> diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
> index 4068946..bd10b40 100644
> --- a/target-microblaze/translate.c
> +++ b/target-microblaze/translate.c
> @@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
>      int stackprot = 0;
>  
>      /* All load/stores use ra.  */
> -    if (dc->ra == 1) {
> +    if (dc->ra == 1 && dc->cpu->cfg.stackprot) {
>          stackprot = 1;
>      }
>  
> @@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t)
>              return &cpu_R[dc->ra];
>          }
>  
> -        if (dc->rb == 1) {
> +        if (dc->rb == 1 && dc->cpu->cfg.stackprot) {
>              stackprot = 1;
>          }
>  
> -- 
> 1.7.1
>