[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/9] arm: add missing scu registers
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/9] arm: add missing scu registers |
Date: |
Sat, 24 Dec 2011 00:23:32 +0000 |
On 22 December 2011 18:20, Mark Langsdorf <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Add power control and non-secure access ctrl registers
Commit message says it's adding the non-secure
access control register, but the patch is only
doing power control.
>
> Signed-off-by: Rob Herring <address@hidden>
> Signed-off-by: Mark Langsdorf <address@hidden>
> ---
> Changes from v1:
> Added VMState support
> Checked alignment of writes to the power control register
>
> hw/a9mpcore.c | 34 ++++++++++++++++++++++++++++++++--
> 1 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/hw/a9mpcore.c b/hw/a9mpcore.c
> index cd2985f..875ae98 100644
> --- a/hw/a9mpcore.c
> +++ b/hw/a9mpcore.c
> @@ -29,6 +29,7 @@ gic_get_current_cpu(void)
> typedef struct a9mp_priv_state {
> gic_state gic;
> uint32_t scu_control;
> + uint32_t scu_status;
> uint32_t old_timer_status[8];
> uint32_t num_cpu;
> qemu_irq *timer_irq;
> @@ -48,7 +49,13 @@ static uint64_t a9_scu_read(void *opaque,
> target_phys_addr_t offset,
> case 0x04: /* Configuration */
> return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1);
> case 0x08: /* CPU Power Status */
> - return 0;
> + return s->scu_status;
> + case 0x09: /* CPU status. */
> + return s->scu_status >> 8;
> + case 0x0a: /* CPU status. */
> + return s->scu_status >> 16;
> + case 0x0b: /* CPU status. */
> + return s->scu_status >> 24;
> case 0x0c: /* Invalidate All Registers In Secure State */
> return 0;
> case 0x40: /* Filtering Start Address Register */
> @@ -73,6 +80,29 @@ static void a9_scu_write(void *opaque, target_phys_addr_t
> offset,
> break;
> case 0x4: /* Configuration: RO */
> break;
> + case 0x08: /* Power Control */
> + s->scu_status = value;
> + break;
This does the wrong thing for a byte or halfword
write to 0x8. (Byte writes in particular are going to be the
common case for this register for obvious reasons.)
> + case 0x09: /* Power Control */
> + s->scu_status &= ~(0xff << 8);
> + s->scu_status |= (value & 0xff) << 8;
> + break;
> + case 0x0A: /* Power Control */
> + if (size == 1) {
> + s->scu_status &= ~(0xff << 16);
> + s->scu_status |= (value & 0xff) << 16;
> + } else if (size == 2) {
> + s->scu_status &= ~(0xffff << 16);
> + s->scu_status |= (value & 0xffff) << 16;
> + } else {
> + /* illegal number of bytes */
> + break;
> + }
> + break;
> + case 0x0B: /* Power Control */
> + s->scu_status &= ~(0xff << 24);
> + s->scu_status |= (value & 0xff) << 24;
> + break;
How about:
switch (size) {
case 1:
mask = 0xff;
break;
case 2:
mask = 0xffff;
break;
case 4:
mask = 0xffffffff;
break;
}
Then the register write code simplifies to:
case 0x08: case 0x09: case 0xa: case 0xb:
shift = (offset - 0x8) * 8;
s->scu_status &= ~(mask << shift);
s->scu_status |= ((value & mask) << shift);
break;
(written on the hoof and untested!)
> case 0x0c: /* Invalidate All Registers In Secure State */
> /* no-op as we do not implement caches */
> break;
> @@ -80,7 +110,6 @@ static void a9_scu_write(void *opaque, target_phys_addr_t
> offset,
> case 0x44: /* Filtering End Address Register */
> /* RAZ/WI, like an implementation with only one AXI master */
> break;
> - case 0x8: /* CPU Power Status */
> case 0x50: /* SCU Access Control Register */
> case 0x54: /* SCU Non-secure Access Control Register */
> /* unimplemented, fall through */
> @@ -173,6 +202,7 @@ static const VMStateDescription vmstate_a9mp_priv = {
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> VMSTATE_UINT32(scu_control, a9mp_priv_state),
> + VMSTATE_UINT32(scu_status, a9mp_priv_state),
> VMSTATE_UINT32_ARRAY(old_timer_status, a9mp_priv_state, 8),
> VMSTATE_END_OF_LIST()
> }
This breaks migration compatibility. You want:
* set .version_id to 2
* leave .minimum_version_id as 1
* the new field should go at the end of the list and be:
VMSTATE_UINT32_V(scu_status, a9mp_priv_state, 2),
(which says it only exists in version 2).
-- PMM
- [Qemu-devel] [PATCH v2 0/9] various ARM fixes, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 4/9] arm: add dummy gic security registers, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 2/9] arm: Set frequencies for arm_timer, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH 5/9] ahci: convert ahci_reset to use AHCIState, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 1/9] arm: add missing scu registers, Mark Langsdorf, 2011/12/22
- Re: [Qemu-devel] [PATCH v2 1/9] arm: add missing scu registers,
Peter Maydell <=
- [Qemu-devel] [PATCH 9/9] arm: increase a9mp interrupts to 160, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 3/9] arm: add dummy v7 cp15 config_base_register, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 8/9] Add xgmac ethernet model, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH 6/9] ahci: add support for non-PCI based controllers, Mark Langsdorf, 2011/12/22
- [Qemu-devel] [PATCH v2 7/9] add L2x0/PL310 cache controller device, Mark Langsdorf, 2011/12/22