From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52352) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxrCD-0004Bb-78 for qemu-devel@nongnu.org; Thu, 28 May 2015 02:21:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxrC7-0004Ew-3i for qemu-devel@nongnu.org; Thu, 28 May 2015 02:21:37 -0400 Received: from mail-bn1bon0095.outbound.protection.outlook.com ([157.56.111.95]:20480 helo=na01-bn1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxrC6-0004Ek-SH for qemu-devel@nongnu.org; Thu, 28 May 2015 02:21:31 -0400 Date: Thu, 28 May 2015 16:17:33 +1000 From: "Edgar E. Iglesias" Message-ID: <20150528061733.GQ30952@toto> References: <84e456db1be3f771dea4020092662a9e80c4cf54.1432790821.git.alistair.francis@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <84e456db1be3f771dea4020092662a9e80c4cf54.1432790821.git.alistair.francis@xilinx.com> Subject: Re: [Qemu-devel] [PATCH v2 3/5] target-microblaze: Allow the stack protection to be disabled/enabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alistair Francis Cc: peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org, afaerber@suse.de, rth@twiddle.net On Thu, May 28, 2015 at 03:37:42PM +1000, Alistair Francis wrote: > Microblaze stack protection is configurable and isn't always enabled. > This patch allows the stack protection to be disabled/enabled from the > CPU properties. > > The stack protection is disabled by default as by default the Microblaze > machines enable the MMU and stack protection can't be enabled if the > MMU is. > > Signed-off-by: Alistair Francis Hi Alistair, > --- > 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..ead2fcd 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); Could you please skip the parentheses. > + > 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, > + false), I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU. > 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 >