[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/5] target-microblaze: Allow the stack prote
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] target-microblaze: Allow the stack protection to be disabled/enabled |
Date: |
Fri, 29 May 2015 15:42:15 +1000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, May 29, 2015 at 03:39:32PM +1000, Alistair Francis wrote:
> On Fri, May 29, 2015 at 3:35 PM, Alistair Francis
> <address@hidden> wrote:
> > On Thu, May 28, 2015 at 4:17 PM, Edgar E. Iglesias
> > <address@hidden> wrote:
> >> 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 <address@hidden>
> >>
> >> 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.
> >
> > Hey Edgar,
> >
> > Yeah I will. They get re-added straight away, which is why I left them in.
> >
> >>
> >>> +
> >>> 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.
> >
> > Fair enough. I will leave it here and add a patch at the end that disables
> > it.
>
> On that note, should the current machines enable it?
>
> It has always been enabled for them, but they do support MMU's so it
> should be disabled.
Right, stackprot should be disabled as the MMU is on.
I wasn't aware that the stackprot was not used in combination with
the MMU at the time...
Cheers,
Edgar
>
> Thanks,
>
> Alistair
>
> >
> > Thanks,
> >
> > Alistair
> >
> >>
> >>
> >>
> >>
> >>> 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
> >>>
> >>
[Qemu-devel] [PATCH v2 4/5] target-microblaze: Tidy up the base-vectors property, Alistair Francis, 2015/05/28
[Qemu-devel] [PATCH v2 5/5] target-microblaze: Convert use-fpu to a CPU property, Alistair Francis, 2015/05/28