diff mbox

target-arm: Implement AArch64 OSLSR_EL1 sysreg dummy

Message ID 1442946912-15351-1-git-send-email-davorin.mista@aggios.com
State New
Headers show

Commit Message

Davorin Mista Sept. 22, 2015, 6:35 p.m. UTC
Define a dummy version of the AArch64 OSLAR_EL1 system register
which just ignores reads. 
Linux reads from this register during its suspend/resume procedure.

Signed-off-by: Davorin Mista <davorin.mista@aggios.com>

---
 target-arm/helper.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Alistair Francis Sept. 22, 2015, 9:16 p.m. UTC | #1
On Tue, Sep 22, 2015 at 11:35 AM, Davorin Mista
<davorin.mista@aggios.com> wrote:
> Define a dummy version of the AArch64 OSLAR_EL1 system register
> which just ignores reads.
> Linux reads from this register during its suspend/resume procedure.
>
> Signed-off-by: Davorin Mista <davorin.mista@aggios.com>

Looks good to me

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
> ---
>  target-arm/helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 454d666..8431181 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3085,6 +3085,10 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>      { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
>        .access = PL1_W, .type = ARM_CP_NOP },
> +    /* We define a dummy OSLSR_EL1, because Linux reads from it. */
> +    { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4 ,
> +      .access = PL1_R, .type = ARM_CP_NOP },
>      /* Dummy OSDLR_EL1: 32-bit Linux will read this */
>      { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,
> --
> 2.5.3
>
>
Peter Maydell Sept. 23, 2015, 4:07 p.m. UTC | #2
On 22 September 2015 at 11:35, Davorin Mista <davorin.mista@aggios.com> wrote:
> Define a dummy version of the AArch64 OSLAR_EL1 system register

Should read "OSLSR_EL1" :-)

> which just ignores reads.
> Linux reads from this register during its suspend/resume procedure.

> Signed-off-by: Davorin Mista <davorin.mista@aggios.com>
>
> ---
>  target-arm/helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 454d666..8431181 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3085,6 +3085,10 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>      { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
>        .access = PL1_W, .type = ARM_CP_NOP },
> +    /* We define a dummy OSLSR_EL1, because Linux reads from it. */
> +    { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4 ,
> +      .access = PL1_R, .type = ARM_CP_NOP },
>      /* Dummy OSDLR_EL1: 32-bit Linux will read this */
>      { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,
> --

I think we need to do better than reads-as-zero here if we're going
to implement this. The value read here should be 0b10x0, where the
value of 'x' is 1 on reset and is controlled by what is written to
the OSLAR_EL1.

This should be pretty straightforward to implement, we just need some
state for the OSLSR, implement OSLSR as a RO register backed by the
state field, and change OSLAR to have a write-function which alters
the OSLSR state field appropriately depending on what's written.

thanks
-- PMM
Davorin Mista Sept. 23, 2015, 7:12 p.m. UTC | #3
On 09/23/2015 09:07 AM, Peter Maydell wrote:
> On 22 September 2015 at 11:35, Davorin Mista <davorin.mista@aggios.com> wrote:
>> Define a dummy version of the AArch64 OSLAR_EL1 system register
>
> Should read "OSLSR_EL1" :-)

Yes, sorry about that :)

>
>> which just ignores reads.
>> Linux reads from this register during its suspend/resume procedure.
>
>> Signed-off-by: Davorin Mista <davorin.mista@aggios.com>
>>
>> ---
>>   target-arm/helper.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 454d666..8431181 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -3085,6 +3085,10 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>>       { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
>>         .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
>>         .access = PL1_W, .type = ARM_CP_NOP },
>> +    /* We define a dummy OSLSR_EL1, because Linux reads from it. */
>> +    { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
>> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4 ,
>> +      .access = PL1_R, .type = ARM_CP_NOP },
>>       /* Dummy OSDLR_EL1: 32-bit Linux will read this */
>>       { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
>>         .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,
>> --
>
> I think we need to do better than reads-as-zero here if we're going
> to implement this. The value read here should be 0b10x0, where the
> value of 'x' is 1 on reset and is controlled by what is written to
> the OSLAR_EL1.
>
> This should be pretty straightforward to implement, we just need some
> state for the OSLSR, implement OSLSR as a RO register backed by the
> state field, and change OSLAR to have a write-function which alters
> the OSLSR state field appropriately depending on what's written.

Okay, so we'd add a writefn for OSLAR_EL1, and a readfn for OSLSR_EL1.

Where do you suggest we keep the OSLSR state? Directly in the ARMCPU 
struct? As far as I can tell it's the first register from the PTM-A9 
block. None of the other debug registers have read/write functions so far.

Also, what type should the register be? Out of the available ARM_CP_xx 
types I'm not sure which one(s) apply.

Thanks,
Davorin

>
> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 454d666..8431181 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3085,6 +3085,10 @@  static const ARMCPRegInfo debug_cp_reginfo[] = {
     { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
       .access = PL1_W, .type = ARM_CP_NOP },
+    /* We define a dummy OSLSR_EL1, because Linux reads from it. */
+    { .name = "OSLSR_EL1", .state = ARM_CP_STATE_BOTH,
+      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 4 ,
+      .access = PL1_R, .type = ARM_CP_NOP },
     /* Dummy OSDLR_EL1: 32-bit Linux will read this */
     { .name = "OSDLR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 4,