Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

2017-07-28 Thread Philippe Mathieu-Daudé 

On 07/28/2017 05:42 AM, Peter Maydell wrote:

On 27 July 2017 at 23:58, Philippe Mathieu-Daudé  wrote:

On 07/27/2017 07:43 PM, Philippe Mathieu-Daudé wrote:


On 07/27/2017 07:59 AM, Peter Maydell wrote:


[...]


-u32p += env->cp15.c6_rgnr;
+u32p += env->pmsav7.rnr;
   tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
   *u32p = value;
   }
@@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
 .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn =
pmsav7_reset },
   { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 =
0,



"RGNR" -> "RNR"



Ah "RGNR" for -R and "RNR" for -M hmmm... still better keep the name
matching the field, "rnr".


It's a bit awkward, yes -- we're going to get a mismatch one way
or the other.

In this patch I wanted only to change the field name, not
anything else (it's already a bit borderline for 2.10).


Fine by me for what's worth. So either ways:
Reviewed-by: Philippe Mathieu-Daudé 



thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

2017-07-28 Thread Peter Maydell 
On 27 July 2017 at 23:58, Philippe Mathieu-Daudé  wrote:
> On 07/27/2017 07:43 PM, Philippe Mathieu-Daudé wrote:
>>
>> On 07/27/2017 07:59 AM, Peter Maydell wrote:
>
> [...]
>>>
>>> -u32p += env->cp15.c6_rgnr;
>>> +u32p += env->pmsav7.rnr;
>>>   tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
>>>   *u32p = value;
>>>   }
>>> @@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
>>> .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn =
>>> pmsav7_reset },
>>>   { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 =
>>> 0,
>>
>>
>> "RGNR" -> "RNR"
>
>
> Ah "RGNR" for -R and "RNR" for -M hmmm... still better keep the name
> matching the field, "rnr".

It's a bit awkward, yes -- we're going to get a mismatch one way
or the other.

In this patch I wanted only to change the field name, not
anything else (it's already a bit borderline for 2.10).

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

2017-07-27 Thread Philippe Mathieu-Daudé 

On 07/27/2017 07:43 PM, Philippe Mathieu-Daudé wrote:

On 07/27/2017 07:59 AM, Peter Maydell wrote:

[...]

-u32p += env->cp15.c6_rgnr;
+u32p += env->pmsav7.rnr;
  tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
  *u32p = value;
  }
@@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
.readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = 
pmsav7_reset },
  { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 
= 0,


"RGNR" -> "RNR"


Ah "RGNR" for -R and "RNR" for -M hmmm... still better keep the name 
matching the field, "rnr".





.access = PL1_RW,
-  .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),
+  .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
.writefn = pmsav7_rgnr_write },


pmsav7_rnr_write


  REGINFO_SENTINEL
  };
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1a40469..93c1a78 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -151,7 +151,7 @@ static bool pmsav7_rgnr_vmstate_validate(void 
*opaque, int version_id)


pmsav7_rnr_vmstate_validate()


  {
  ARMCPU *cpu = opaque;
-return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;
+return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;
  }
  static const VMStateDescription vmstate_pmsav7 = {


[...]
 VMSTATE_VALIDATE("rgnr is valid", pmsav7_rgnr_vmstate_validate),

also this one "rnr is valid" ^^^

once fixed:
Reviewed-by: Philippe Mathieu-Daudé

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.10 3/5] target/arm: Rename cp15.c6_rgnr to pmsav7.rnr

2017-07-27 Thread Philippe Mathieu-Daudé 

Hi Peter,

you missed some rgnr -> rnr :|

On 07/27/2017 07:59 AM, Peter Maydell wrote:

Almost all of the PMSAv7 state is in the pmsav7 substruct of
the ARM CPU state structure. The exception is the region
number register, which is in cp15.c6_rgnr. This exception
is a bit odd for M profile, which otherwise generally does
not store state in the cp15 substruct.

Rename cp15.c6_rgnr to pmsav7.rnr accordingly.

Signed-off-by: Peter Maydell 
---
  hw/intc/armv7m_nvic.c | 14 +++---
  target/arm/cpu.h  |  3 +--
  target/arm/helper.c   |  6 +++---
  target/arm/machine.c  |  2 +-
  4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 26a4b2d..323e2d4 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -536,13 +536,13 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
  case 0xd94: /* MPU_CTRL */
  return cpu->env.v7m.mpu_ctrl;
  case 0xd98: /* MPU_RNR */
-return cpu->env.cp15.c6_rgnr;
+return cpu->env.pmsav7.rnr;
  case 0xd9c: /* MPU_RBAR */
  case 0xda4: /* MPU_RBAR_A1 */
  case 0xdac: /* MPU_RBAR_A2 */
  case 0xdb4: /* MPU_RBAR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return 0;
@@ -554,7 +554,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
  case 0xdb0: /* MPU_RASR_A2 */
  case 0xdb8: /* MPU_RASR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return 0;
@@ -681,7 +681,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
PRIu32 "/%" PRIu32 "\n",
value, cpu->pmsav7_dregion);
  } else {
-cpu->env.cp15.c6_rgnr = value;
+cpu->env.pmsav7.rnr = value;
  }
  break;
  case 0xd9c: /* MPU_RBAR */
@@ -702,9 +702,9 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
region, cpu->pmsav7_dregion);
  return;
  }
-cpu->env.cp15.c6_rgnr = region;
+cpu->env.pmsav7.rnr = region;
  } else {
-region = cpu->env.cp15.c6_rgnr;
+region = cpu->env.pmsav7.rnr;
  }
  
  if (region >= cpu->pmsav7_dregion) {

@@ -720,7 +720,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value)
  case 0xdb0: /* MPU_RASR_A2 */
  case 0xdb8: /* MPU_RASR_A3 */
  {
-int region = cpu->env.cp15.c6_rgnr;
+int region = cpu->env.pmsav7.rnr;
  
  if (region >= cpu->pmsav7_dregion) {

  return;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 102c58a..b39d64a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -305,8 +305,6 @@ typedef struct CPUARMState {
  uint64_t par_el[4];
  };
  
-uint32_t c6_rgnr;

-
  uint32_t c9_insn; /* Cache lockdown registers.  */
  uint32_t c9_data;
  uint64_t c9_pmcr; /* performance monitor control register */
@@ -519,6 +517,7 @@ typedef struct CPUARMState {
  uint32_t *drbar;
  uint32_t *drsr;
  uint32_t *dracr;
+uint32_t rnr;
  } pmsav7;
  
  void *nvic;

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 169c361..63187de 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2385,7 +2385,7 @@ static uint64_t pmsav7_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
  return 0;
  }
  
-u32p += env->cp15.c6_rgnr;

+u32p += env->pmsav7.rnr;
  return *u32p;
  }
  
@@ -2399,7 +2399,7 @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,

  return;
  }
  
-u32p += env->cp15.c6_rgnr;

+u32p += env->pmsav7.rnr;
  tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
  *u32p = value;
  }
@@ -2447,7 +2447,7 @@ static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
.readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset 
},
  { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,


"RGNR" -> "RNR"


.access = PL1_RW,
-  .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),
+  .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
.writefn = pmsav7_rgnr_write },


pmsav7_rnr_write


  REGINFO_SENTINEL
  };
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1a40469..93c1a78 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -151,7 +151,7 @@ static bool pmsav7_rgnr_vmstate_validate(void *opaque, int 
version_id)


pmsav7_rnr_vmstate_validate()


  {
  ARMCPU *cpu = opaque;
  
-return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;

+return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;
  }