On Fri, May 29, 2015 at 3:42 PM, Edgar E. Iglesias
<edgar.igles...@xilinx.com> wrote:
> On Fri, May 29, 2015 at 03:39:32PM +1000, Alistair Francis wrote:
>> On Fri, May 29, 2015 at 3:35 PM, Alistair Francis
>> <alistair.fran...@xilinx.com> wrote:
>> > On Thu, May 28, 2015 at 4:17 PM, Edgar E. Iglesias
>> > <edgar.igles...@xilinx.com> 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 <alistair.fran...@xilinx.com>
>> >>
>> >> 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...
Ok, I will disable it by default and for all of the current machines.
Thanks,
Alistair
>
> 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
>> >>>
>> >>
>
