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.
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
>>
>
