Implement the missing registers for the TZ MPC.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/misc/tz-mpc.h | 10 +++
hw/misc/tz-mpc.c | 137 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
index b5eaf1699ea..1fff4d6029a 100644
--- a/include/hw/misc/tz-mpc.h
+++ b/include/hw/misc/tz-mpc.h
@@ -48,6 +48,16 @@ struct TZMPC {
/*< public >*/
+ /* State */
+ uint32_t ctrl;
+ uint32_t blk_idx;
+ uint32_t int_stat;
+ uint32_t int_en;
+ uint32_t int_info1;
+ uint32_t int_info2;
+
+ uint32_t *blk_lut;
+
qemu_irq irq;
/* Properties */
diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
index d4467ccc3b2..9db55e23daf 100644
--- a/hw/misc/tz-mpc.c
+++ b/hw/misc/tz-mpc.c
@@ -28,16 +28,23 @@ enum {
/* Config registers */
REG32(CTRL, 0x00)
+ FIELD(CTRL, SEC_RESP, 4, 1)
+ FIELD(CTRL, AUTOINC, 8, 1)
+ FIELD(CTRL, LOCKDOWN, 31, 1)
REG32(BLK_MAX, 0x10)
REG32(BLK_CFG, 0x14)
REG32(BLK_IDX, 0x18)
REG32(BLK_LUT, 0x1c)
REG32(INT_STAT, 0x20)
+ FIELD(INT_STAT, IRQ, 0, 1)
REG32(INT_CLEAR, 0x24)
+ FIELD(INT_CLEAR, IRQ, 0, 1)
REG32(INT_EN, 0x28)
+ FIELD(INT_EN, IRQ, 0, 1)
REG32(INT_INFO1, 0x2c)
REG32(INT_INFO2, 0x30)
REG32(INT_SET, 0x34)
+ FIELD(INT_SET, IRQ, 0, 1)
REG32(PIDR4, 0xfd0)
REG32(PIDR5, 0xfd4)
REG32(PIDR6, 0xfd8)
@@ -57,14 +64,55 @@ static const uint8_t tz_mpc_idregs[] = {
0x0d, 0xf0, 0x05, 0xb1,
};
+static void tz_mpc_irq_update(TZMPC *s)
+{
+ qemu_set_irq(s->irq, s->int_stat && s->int_en);
+}
+
static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
uint64_t *pdata,
unsigned size, MemTxAttrs attrs)
{
+ TZMPC *s = TZ_MPC(opaque);
uint64_t r;
uint32_t offset = addr & ~0x3;
switch (offset) {
+ case A_CTRL:
+ r = s->ctrl;
+ break;
+ case A_BLK_MAX:
+ r = s->blk_max;
+ break;
+ case A_BLK_CFG:
+ /* We are never in "init in progress state", so this just indicates
+ * the block size. s->blocksize == (1 << BLK_CFG + 5), so
+ * BLK_CFG == ctz32(s->blocksize) - 5
+ */
+ r = ctz32(s->blocksize) - 5;
+ break;
+ case A_BLK_IDX:
+ r = s->blk_idx;
+ break;
+ case A_BLK_LUT:
+ r = s->blk_lut[s->blk_idx];
+ if (size == 4) {
+ s->blk_idx++;
+ s->blk_idx %= s->blk_max;
+ }
+ break;
+ case A_INT_STAT:
+ r = s->int_stat;
+ break;
+ case A_INT_EN:
+ r = s->int_en;
+ break;
+ case A_INT_INFO1:
+ r = s->int_info1;
+ break;
+ case A_INT_INFO2:
+ r = s->int_info2;
+ break;
case A_PIDR4:
case A_PIDR5:
case A_PIDR6:
@@ -110,6 +158,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
uint64_t value,
unsigned size, MemTxAttrs attrs)
{
+ TZMPC *s = TZ_MPC(opaque);
uint32_t offset = addr & ~0x3;
trace_tz_mpc_reg_write(addr, value, size);
@@ -122,9 +171,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
uint32_t oldval;
switch (offset) {
- /* As we add support for registers which need expansions
- * other than zeroes we'll fill in cases here.
- */
+ case A_CTRL:
+ oldval = s->ctrl;
+ break;
+ case A_BLK_IDX:
+ oldval = s->blk_idx;
+ break;
+ case A_BLK_LUT:
+ oldval = s->blk_lut[s->blk_idx];
+ break;
default:
oldval = 0;
break;
@@ -132,7 +187,51 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
}
+ if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
+ (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
+ /* Lockdown mode makes these three registers read-only, and
+ * the only way out of it is to reset the device.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
+ "while MPC is in lockdown mode\n", offset);
+ return MEMTX_OK;
+ }
+
switch (offset) {
+ case A_CTRL:
+ /* We don't implement the 'data gating' feature so all other bits
+ * are reserved and we make them RAZ/WI.
+ */
+ s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
+ R_CTRL_AUTOINC_MASK |
+ R_CTRL_LOCKDOWN_MASK);
+ break;
+ case A_BLK_IDX:
+ s->blk_idx = value % s->blk_max;
+ break;
+ case A_BLK_LUT:
+ s->blk_lut[s->blk_idx] = value;
+ if (size == 4) {
+ s->blk_idx++;
+ s->blk_idx %= s->blk_max;
+ }
+ break;
+ case A_INT_CLEAR:
+ if (value & R_INT_CLEAR_IRQ_MASK) {
+ s->int_stat = 0;
+ tz_mpc_irq_update(s);
+ }
+ break;
+ case A_INT_EN:
+ s->int_en = value & R_INT_EN_IRQ_MASK;
+ tz_mpc_irq_update(s);
+ break;
+ case A_INT_SET:
+ if (value & R_INT_SET_IRQ_MASK) {
+ s->int_stat = R_INT_STAT_IRQ_MASK;
+ tz_mpc_irq_update(s);
+ }
+ break;
case A_PIDR4:
case A_PIDR5:
case A_PIDR6:
@@ -248,6 +347,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
static void tz_mpc_reset(DeviceState *dev)
{
+ TZMPC *s = TZ_MPC(dev);
+
+ s->ctrl = 0x00000100;
+ s->blk_idx = 0;
+ s->int_stat = 0;
+ s->int_en = 1;
+ s->int_info1 = 0;
+ s->int_info2 = 0;
+
+ memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
}
static void tz_mpc_init(Object *obj)
@@ -321,13 +430,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
"tz-mpc-downstream");
address_space_init(&s->blocked_io_as, &s->blocked_io,
"tz-mpc-blocked-io");
+
+ s->blk_lut = g_new(uint32_t, s->blk_max);
+}
+
+static int tz_mpc_post_load(void *opaque, int version_id)
+{
+ TZMPC *s = TZ_MPC(opaque);
+
+ /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
+ if (s->blk_idx >= s->blk_max) {
+ return -1;
+ }
+ return 0;
}
static const VMStateDescription tz_mpc_vmstate = {
.name = "tz-mpc",
.version_id = 1,
.minimum_version_id = 1,
+ .post_load = tz_mpc_post_load,
.fields = (VMStateField[]) {
+ VMSTATE_UINT32(ctrl, TZMPC),
+ VMSTATE_UINT32(blk_idx, TZMPC),
+ VMSTATE_UINT32(int_stat, TZMPC),
+ VMSTATE_UINT32(int_en, TZMPC),
+ VMSTATE_UINT32(int_info1, TZMPC),
+ VMSTATE_UINT32(int_info2, TZMPC),
+ VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
+ 0, vmstate_info_uint32, uint32_t),
VMSTATE_END_OF_LIST()
}
};
--
2.17.1
Hi Peter,
On 06/04/2018 05:29 PM, Peter Maydell wrote:
> Implement the missing registers for the TZ MPC.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/misc/tz-mpc.h | 10 +++
> hw/misc/tz-mpc.c | 137 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 144 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
> index b5eaf1699ea..1fff4d6029a 100644
> --- a/include/hw/misc/tz-mpc.h
> +++ b/include/hw/misc/tz-mpc.h
> @@ -48,6 +48,16 @@ struct TZMPC {
>
> /*< public >*/
>
> + /* State */
> + uint32_t ctrl;
> + uint32_t blk_idx;
> + uint32_t int_stat;
> + uint32_t int_en;
> + uint32_t int_info1;
> + uint32_t int_info2;
> +
> + uint32_t *blk_lut;
> +
> qemu_irq irq;
>
> /* Properties */
> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
> index d4467ccc3b2..9db55e23daf 100644
> --- a/hw/misc/tz-mpc.c
> +++ b/hw/misc/tz-mpc.c
> @@ -28,16 +28,23 @@ enum {
>
> /* Config registers */
> REG32(CTRL, 0x00)
> + FIELD(CTRL, SEC_RESP, 4, 1)
> + FIELD(CTRL, AUTOINC, 8, 1)
> + FIELD(CTRL, LOCKDOWN, 31, 1)
> REG32(BLK_MAX, 0x10)
> REG32(BLK_CFG, 0x14)
> REG32(BLK_IDX, 0x18)
> REG32(BLK_LUT, 0x1c)
> REG32(INT_STAT, 0x20)
> + FIELD(INT_STAT, IRQ, 0, 1)
> REG32(INT_CLEAR, 0x24)
> + FIELD(INT_CLEAR, IRQ, 0, 1)
> REG32(INT_EN, 0x28)
> + FIELD(INT_EN, IRQ, 0, 1)
> REG32(INT_INFO1, 0x2c)
> REG32(INT_INFO2, 0x30)
> REG32(INT_SET, 0x34)
> + FIELD(INT_SET, IRQ, 0, 1)
> REG32(PIDR4, 0xfd0)
> REG32(PIDR5, 0xfd4)
> REG32(PIDR6, 0xfd8)
> @@ -57,14 +64,55 @@ static const uint8_t tz_mpc_idregs[] = {
> 0x0d, 0xf0, 0x05, 0xb1,
> };
>
> +static void tz_mpc_irq_update(TZMPC *s)
> +{
> + qemu_set_irq(s->irq, s->int_stat && s->int_en);
> +}
> +
> static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
> uint64_t *pdata,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint64_t r;
> uint32_t offset = addr & ~0x3;
I don't get where do we check if the access is secure or not. Shouldn't
we test attrs.secure somewhere (PPROT[1] == 0)? Also the spec says
IDregs can be read by any type of access. Where is this differentiation
done?
Thanks
Eric
>
> switch (offset) {
> + case A_CTRL:
> + r = s->ctrl;
> + break;
> + case A_BLK_MAX:
> + r = s->blk_max;
> + break;
> + case A_BLK_CFG:
> + /* We are never in "init in progress state", so this just indicates
> + * the block size. s->blocksize == (1 << BLK_CFG + 5), so
> + * BLK_CFG == ctz32(s->blocksize) - 5
> + */
> + r = ctz32(s->blocksize) - 5;
> + break;
> + case A_BLK_IDX:
> + r = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + r = s->blk_lut[s->blk_idx];
> + if (size == 4) {
> + s->blk_idx++;
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_STAT:
> + r = s->int_stat;
> + break;
> + case A_INT_EN:
> + r = s->int_en;
> + break;
> + case A_INT_INFO1:
> + r = s->int_info1;
> + break;
> + case A_INT_INFO2:
> + r = s->int_info2;
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -110,6 +158,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint64_t value,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint32_t offset = addr & ~0x3;
>
> trace_tz_mpc_reg_write(addr, value, size);
> @@ -122,9 +171,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint32_t oldval;
>
> switch (offset) {
> - /* As we add support for registers which need expansions
> - * other than zeroes we'll fill in cases here.
> - */
> + case A_CTRL:
> + oldval = s->ctrl;
> + break;
> + case A_BLK_IDX:
> + oldval = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + oldval = s->blk_lut[s->blk_idx];
> + break;
> default:
> oldval = 0;
> break;
> @@ -132,7 +187,51 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
> }
>
> + if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
> + (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
> + /* Lockdown mode makes these three registers read-only, and
> + * the only way out of it is to reset the device.
> + */
> + qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
> + "while MPC is in lockdown mode\n", offset);
> + return MEMTX_OK;
> + }
> +
> switch (offset) {
> + case A_CTRL:
> + /* We don't implement the 'data gating' feature so all other bits
> + * are reserved and we make them RAZ/WI.
> + */
> + s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
> + R_CTRL_AUTOINC_MASK |
> + R_CTRL_LOCKDOWN_MASK);
> + break;
> + case A_BLK_IDX:
> + s->blk_idx = value % s->blk_max;
> + break;
> + case A_BLK_LUT:
> + s->blk_lut[s->blk_idx] = value;
> + if (size == 4) {
> + s->blk_idx++;
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_CLEAR:
> + if (value & R_INT_CLEAR_IRQ_MASK) {
> + s->int_stat = 0;
> + tz_mpc_irq_update(s);
> + }
> + break;
> + case A_INT_EN:
> + s->int_en = value & R_INT_EN_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + break;
> + case A_INT_SET:
> + if (value & R_INT_SET_IRQ_MASK) {
> + s->int_stat = R_INT_STAT_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + }
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -248,6 +347,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
>
> static void tz_mpc_reset(DeviceState *dev)
> {
> + TZMPC *s = TZ_MPC(dev);
> +
> + s->ctrl = 0x00000100;
> + s->blk_idx = 0;
> + s->int_stat = 0;
> + s->int_en = 1;
> + s->int_info1 = 0;
> + s->int_info2 = 0;
> +
> + memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
> }
>
> static void tz_mpc_init(Object *obj)
> @@ -321,13 +430,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
> "tz-mpc-downstream");
> address_space_init(&s->blocked_io_as, &s->blocked_io,
> "tz-mpc-blocked-io");
> +
> + s->blk_lut = g_new(uint32_t, s->blk_max);
> +}
> +
> +static int tz_mpc_post_load(void *opaque, int version_id)
> +{
> + TZMPC *s = TZ_MPC(opaque);
> +
> + /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
> + if (s->blk_idx >= s->blk_max) {
> + return -1;
> + }
> + return 0;
> }
>
> static const VMStateDescription tz_mpc_vmstate = {
> .name = "tz-mpc",
> .version_id = 1,
> .minimum_version_id = 1,
> + .post_load = tz_mpc_post_load,
> .fields = (VMStateField[]) {
> + VMSTATE_UINT32(ctrl, TZMPC),
> + VMSTATE_UINT32(blk_idx, TZMPC),
> + VMSTATE_UINT32(int_stat, TZMPC),
> + VMSTATE_UINT32(int_en, TZMPC),
> + VMSTATE_UINT32(int_info1, TZMPC),
> + VMSTATE_UINT32(int_info2, TZMPC),
> + VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
> + 0, vmstate_info_uint32, uint32_t),
> VMSTATE_END_OF_LIST()
> }
> };
>
On 14 June 2018 at 21:14, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Peter,
>
> On 06/04/2018 05:29 PM, Peter Maydell wrote:
>> Implement the missing registers for the TZ MPC.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
>> uint64_t *pdata,
>> unsigned size, MemTxAttrs attrs)
>> {
>> + TZMPC *s = TZ_MPC(opaque);
>> uint64_t r;
>> uint32_t offset = addr & ~0x3;
> I don't get where do we check if the access is secure or not. Shouldn't
> we test attrs.secure somewhere (PPROT[1] == 0)? Also the spec says
> IDregs can be read by any type of access. Where is this differentiation
> done?
I'd missed that bit of the spec -- yes, we should be checking
attrs.secure to see if we should allow access to the non-ID registers.
For the MPS2 boards it doesn't matter because the registers are
in a part of the memory map that is enforced as secure-access-only
by the IDAU.
thanks
-- PMM
Hi Peter,
On 06/04/2018 05:29 PM, Peter Maydell wrote:
> Implement the missing registers for the TZ MPC.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/misc/tz-mpc.h | 10 +++
> hw/misc/tz-mpc.c | 137 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 144 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
> index b5eaf1699ea..1fff4d6029a 100644
> --- a/include/hw/misc/tz-mpc.h
> +++ b/include/hw/misc/tz-mpc.h
> @@ -48,6 +48,16 @@ struct TZMPC {
>
> /*< public >*/
>
> + /* State */
> + uint32_t ctrl;
> + uint32_t blk_idx;
> + uint32_t int_stat;
> + uint32_t int_en;
> + uint32_t int_info1;
> + uint32_t int_info2;
> +
> + uint32_t *blk_lut;
> +
> qemu_irq irq;
>
> /* Properties */
> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
> index d4467ccc3b2..9db55e23daf 100644
> --- a/hw/misc/tz-mpc.c
> +++ b/hw/misc/tz-mpc.c
> @@ -28,16 +28,23 @@ enum {
>
> /* Config registers */
> REG32(CTRL, 0x00)
> + FIELD(CTRL, SEC_RESP, 4, 1)
> + FIELD(CTRL, AUTOINC, 8, 1)
> + FIELD(CTRL, LOCKDOWN, 31, 1)
> REG32(BLK_MAX, 0x10)
> REG32(BLK_CFG, 0x14)
> REG32(BLK_IDX, 0x18)
> REG32(BLK_LUT, 0x1c)
> REG32(INT_STAT, 0x20)
> + FIELD(INT_STAT, IRQ, 0, 1)
> REG32(INT_CLEAR, 0x24)
> + FIELD(INT_CLEAR, IRQ, 0, 1)
> REG32(INT_EN, 0x28)
> + FIELD(INT_EN, IRQ, 0, 1)
> REG32(INT_INFO1, 0x2c)
> REG32(INT_INFO2, 0x30)
> REG32(INT_SET, 0x34)
> + FIELD(INT_SET, IRQ, 0, 1)
> REG32(PIDR4, 0xfd0)
> REG32(PIDR5, 0xfd4)
> REG32(PIDR6, 0xfd8)
> @@ -57,14 +64,55 @@ static const uint8_t tz_mpc_idregs[] = {
> 0x0d, 0xf0, 0x05, 0xb1,
> };
>
> +static void tz_mpc_irq_update(TZMPC *s)
> +{
> + qemu_set_irq(s->irq, s->int_stat && s->int_en);
> +}
> +
> static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
> uint64_t *pdata,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint64_t r;
> uint32_t offset = addr & ~0x3;
>
> switch (offset) {
> + case A_CTRL:
> + r = s->ctrl;
> + break;
> + case A_BLK_MAX:
> + r = s->blk_max;
> + break;
> + case A_BLK_CFG:
> + /* We are never in "init in progress state", so this just indicates
> + * the block size. s->blocksize == (1 << BLK_CFG + 5), so
> + * BLK_CFG == ctz32(s->blocksize) - 5
> + */
> + r = ctz32(s->blocksize) - 5;
> + break;
> + case A_BLK_IDX:
> + r = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + r = s->blk_lut[s->blk_idx];
> + if (size == 4) {
> + s->blk_idx++;
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_STAT:
> + r = s->int_stat;
> + break;
> + case A_INT_EN:
> + r = s->int_en;
> + break;
> + case A_INT_INFO1:
> + r = s->int_info1;
> + break;
> + case A_INT_INFO2:
> + r = s->int_info2;
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -110,6 +158,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint64_t value,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint32_t offset = addr & ~0x3;
>
> trace_tz_mpc_reg_write(addr, value, size);
> @@ -122,9 +171,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint32_t oldval;
>
> switch (offset) {
> - /* As we add support for registers which need expansions
> - * other than zeroes we'll fill in cases here.
> - */
> + case A_CTRL:
> + oldval = s->ctrl;
> + break;
> + case A_BLK_IDX:
> + oldval = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + oldval = s->blk_lut[s->blk_idx];
> + break;
> default:
> oldval = 0;
> break;
> @@ -132,7 +187,51 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
> }
>
> + if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
> + (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
> + /* Lockdown mode makes these three registers read-only, and
> + * the only way out of it is to reset the device.
> + */
> + qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
> + "while MPC is in lockdown mode\n", offset);
> + return MEMTX_OK;
> + }
> +
> switch (offset) {
> + case A_CTRL:
> + /* We don't implement the 'data gating' feature so all other bits
> + * are reserved and we make them RAZ/WI.
> + */
> + s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
> + R_CTRL_AUTOINC_MASK |
> + R_CTRL_LOCKDOWN_MASK);
> + break;
> + case A_BLK_IDX:
> + s->blk_idx = value % s->blk_max;
> + break;
> + case A_BLK_LUT:
> + s->blk_lut[s->blk_idx] = value;
> + if (size == 4) {
> + s->blk_idx++;
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_CLEAR:
> + if (value & R_INT_CLEAR_IRQ_MASK) {
> + s->int_stat = 0;
> + tz_mpc_irq_update(s);
don't you need to clear the info regs. spec says:
the [info] register retains its value until mpc_irq is cleared.
Thanks
Eric
> + }
> + break;
> + case A_INT_EN:
> + s->int_en = value & R_INT_EN_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + break;
> + case A_INT_SET:
> + if (value & R_INT_SET_IRQ_MASK) {
> + s->int_stat = R_INT_STAT_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + }
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -248,6 +347,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
>
> static void tz_mpc_reset(DeviceState *dev)
> {
> + TZMPC *s = TZ_MPC(dev);
> +
> + s->ctrl = 0x00000100;
> + s->blk_idx = 0;
> + s->int_stat = 0;
> + s->int_en = 1;
> + s->int_info1 = 0;
> + s->int_info2 = 0;
> +
> + memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
> }
>
> static void tz_mpc_init(Object *obj)
> @@ -321,13 +430,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
> "tz-mpc-downstream");
> address_space_init(&s->blocked_io_as, &s->blocked_io,
> "tz-mpc-blocked-io");
> +
> + s->blk_lut = g_new(uint32_t, s->blk_max);
> +}
> +
> +static int tz_mpc_post_load(void *opaque, int version_id)
> +{
> + TZMPC *s = TZ_MPC(opaque);
> +
> + /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
> + if (s->blk_idx >= s->blk_max) {
> + return -1;
> + }
> + return 0;
> }
>
> static const VMStateDescription tz_mpc_vmstate = {
> .name = "tz-mpc",
> .version_id = 1,
> .minimum_version_id = 1,
> + .post_load = tz_mpc_post_load,
> .fields = (VMStateField[]) {
> + VMSTATE_UINT32(ctrl, TZMPC),
> + VMSTATE_UINT32(blk_idx, TZMPC),
> + VMSTATE_UINT32(int_stat, TZMPC),
> + VMSTATE_UINT32(int_en, TZMPC),
> + VMSTATE_UINT32(int_info1, TZMPC),
> + VMSTATE_UINT32(int_info2, TZMPC),
> + VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
> + 0, vmstate_info_uint32, uint32_t),
> VMSTATE_END_OF_LIST()
> }
> };
>
On 14 June 2018 at 21:36, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Peter,
>
> On 06/04/2018 05:29 PM, Peter Maydell wrote:
>> Implement the missing registers for the TZ MPC.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> + case A_INT_CLEAR:
>> + if (value & R_INT_CLEAR_IRQ_MASK) {
>> + s->int_stat = 0;
>> + tz_mpc_irq_update(s);
> don't you need to clear the info regs. spec says:
> the [info] register retains its value until mpc_irq is cleared.
The full sentence is "Subsequent security violating transfers
remain blocked, that is, not captured in this register
and the register retains its value until mpc_irq is cleared."
I interpret "until mpc_irq is cleared" as applying to the
entire thing, ie mpc_irq being cleared is what allows a
subsequent transfer to be captured in this register.
(Hardware actively clearing itself to zero is unlikely,
because that costs extra gates which designers don't tend
to do unless there's a reason for it.)
From a guest point of view (which is kind of the pov the
docs are written from), the guest can't rely on the register
value once mpc_irq is cleared (because another transaction
might come along and cause the value to be overwritten).
thanks
-- PMM
Hi Peter,
On 06/15/2018 11:04 AM, Peter Maydell wrote:
> On 14 June 2018 at 21:36, Auger Eric <eric.auger@redhat.com> wrote:
>> Hi Peter,
>>
>> On 06/04/2018 05:29 PM, Peter Maydell wrote:
>>> Implement the missing registers for the TZ MPC.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>>> + case A_INT_CLEAR:
>>> + if (value & R_INT_CLEAR_IRQ_MASK) {
>>> + s->int_stat = 0;
>>> + tz_mpc_irq_update(s);
>> don't you need to clear the info regs. spec says:
>> the [info] register retains its value until mpc_irq is cleared.
>
> The full sentence is "Subsequent security violating transfers
> remain blocked, that is, not captured in this register
> and the register retains its value until mpc_irq is cleared."
> I interpret "until mpc_irq is cleared" as applying to the
> entire thing, ie mpc_irq being cleared is what allows a
> subsequent transfer to be captured in this register.
> (Hardware actively clearing itself to zero is unlikely,
> because that costs extra gates which designers don't tend
> to do unless there's a reason for it.)
>
> From a guest point of view (which is kind of the pov the
> docs are written from), the guest can't rely on the register
> value once mpc_irq is cleared (because another transaction
> might come along and cause the value to be overwritten).
OK
Thanks
Eric
>
> thanks
> -- PMM
>
Hi Peter,
On 06/04/2018 05:29 PM, Peter Maydell wrote:
> Implement the missing registers for the TZ MPC.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/misc/tz-mpc.h | 10 +++
> hw/misc/tz-mpc.c | 137 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 144 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/misc/tz-mpc.h b/include/hw/misc/tz-mpc.h
> index b5eaf1699ea..1fff4d6029a 100644
> --- a/include/hw/misc/tz-mpc.h
> +++ b/include/hw/misc/tz-mpc.h
> @@ -48,6 +48,16 @@ struct TZMPC {
>
> /*< public >*/
>
> + /* State */
> + uint32_t ctrl;
> + uint32_t blk_idx;
> + uint32_t int_stat;
> + uint32_t int_en;
> + uint32_t int_info1;
> + uint32_t int_info2;
> +
> + uint32_t *blk_lut;
> +
> qemu_irq irq;
>
> /* Properties */
> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
> index d4467ccc3b2..9db55e23daf 100644
> --- a/hw/misc/tz-mpc.c
> +++ b/hw/misc/tz-mpc.c
> @@ -28,16 +28,23 @@ enum {
>
> /* Config registers */
> REG32(CTRL, 0x00)
> + FIELD(CTRL, SEC_RESP, 4, 1)
> + FIELD(CTRL, AUTOINC, 8, 1)
> + FIELD(CTRL, LOCKDOWN, 31, 1)
> REG32(BLK_MAX, 0x10)
> REG32(BLK_CFG, 0x14)
> REG32(BLK_IDX, 0x18)
> REG32(BLK_LUT, 0x1c)
> REG32(INT_STAT, 0x20)
> + FIELD(INT_STAT, IRQ, 0, 1)
> REG32(INT_CLEAR, 0x24)
> + FIELD(INT_CLEAR, IRQ, 0, 1)
> REG32(INT_EN, 0x28)
> + FIELD(INT_EN, IRQ, 0, 1)
> REG32(INT_INFO1, 0x2c)
> REG32(INT_INFO2, 0x30)
> REG32(INT_SET, 0x34)
> + FIELD(INT_SET, IRQ, 0, 1)
> REG32(PIDR4, 0xfd0)
> REG32(PIDR5, 0xfd4)
> REG32(PIDR6, 0xfd8)
> @@ -57,14 +64,55 @@ static const uint8_t tz_mpc_idregs[] = {
> 0x0d, 0xf0, 0x05, 0xb1,
> };
>
> +static void tz_mpc_irq_update(TZMPC *s)
> +{
> + qemu_set_irq(s->irq, s->int_stat && s->int_en);
> +}
> +
> static MemTxResult tz_mpc_reg_read(void *opaque, hwaddr addr,
> uint64_t *pdata,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint64_t r;
> uint32_t offset = addr & ~0x3;
>
> switch (offset) {
> + case A_CTRL:
> + r = s->ctrl;
> + break;
> + case A_BLK_MAX:
> + r = s->blk_max;
> + break;
> + case A_BLK_CFG:
> + /* We are never in "init in progress state", so this just indicates
> + * the block size. s->blocksize == (1 << BLK_CFG + 5), so
> + * BLK_CFG == ctz32(s->blocksize) - 5
> + */
> + r = ctz32(s->blocksize) - 5;
> + break;
> + case A_BLK_IDX:
> + r = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + r = s->blk_lut[s->blk_idx];
> + if (size == 4) {
don't you need to test CTRL[8] before doing the auto-increment?
spec says:
"A full word write or read to this register
automatically increments the BLK_IDX by one if
enabled by CTRL[8]."
> + s->blk_idx++;
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_STAT:
> + r = s->int_stat;
> + break;
> + case A_INT_EN:
> + r = s->int_en;
> + break;
> + case A_INT_INFO1:
> + r = s->int_info1;
> + break;
> + case A_INT_INFO2:
> + r = s->int_info2;
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -110,6 +158,7 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint64_t value,
> unsigned size, MemTxAttrs attrs)
> {
> + TZMPC *s = TZ_MPC(opaque);
> uint32_t offset = addr & ~0x3;
>
> trace_tz_mpc_reg_write(addr, value, size);
> @@ -122,9 +171,15 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> uint32_t oldval;
>
> switch (offset) {
> - /* As we add support for registers which need expansions
> - * other than zeroes we'll fill in cases here.
> - */
> + case A_CTRL:
> + oldval = s->ctrl;
> + break;
> + case A_BLK_IDX:
> + oldval = s->blk_idx;
> + break;
> + case A_BLK_LUT:
> + oldval = s->blk_lut[s->blk_idx];
> + break;
> default:
> oldval = 0;
> break;
> @@ -132,7 +187,51 @@ static MemTxResult tz_mpc_reg_write(void *opaque, hwaddr addr,
> value = deposit32(oldval, (addr & 3) * 8, size * 8, value);
> }
>
> + if ((s->ctrl & R_CTRL_LOCKDOWN_MASK) &&
> + (offset == A_CTRL || offset == A_BLK_LUT || offset == A_INT_EN)) {
> + /* Lockdown mode makes these three registers read-only, and
> + * the only way out of it is to reset the device.
> + */
> + qemu_log_mask(LOG_GUEST_ERROR, "TZ MPC register write to offset 0x%x "
> + "while MPC is in lockdown mode\n", offset);
> + return MEMTX_OK;
> + }
> +
> switch (offset) {
> + case A_CTRL:
> + /* We don't implement the 'data gating' feature so all other bits
> + * are reserved and we make them RAZ/WI.
> + */
> + s->ctrl = value & (R_CTRL_SEC_RESP_MASK |
> + R_CTRL_AUTOINC_MASK |
> + R_CTRL_LOCKDOWN_MASK);
> + break;
> + case A_BLK_IDX:
> + s->blk_idx = value % s->blk_max;
> + break;
> + case A_BLK_LUT:
> + s->blk_lut[s->blk_idx] = value;
> + if (size == 4) {
> + s->blk_idx++;
same?
Thanks
Eric
> + s->blk_idx %= s->blk_max;
> + }
> + break;
> + case A_INT_CLEAR:
> + if (value & R_INT_CLEAR_IRQ_MASK) {
> + s->int_stat = 0;
> + tz_mpc_irq_update(s);
> + }
> + break;
> + case A_INT_EN:
> + s->int_en = value & R_INT_EN_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + break;
> + case A_INT_SET:
> + if (value & R_INT_SET_IRQ_MASK) {
> + s->int_stat = R_INT_STAT_IRQ_MASK;
> + tz_mpc_irq_update(s);
> + }
> + break;
> case A_PIDR4:
> case A_PIDR5:
> case A_PIDR6:
> @@ -248,6 +347,16 @@ static int tz_mpc_num_indexes(IOMMUMemoryRegion *iommu)
>
> static void tz_mpc_reset(DeviceState *dev)
> {
> + TZMPC *s = TZ_MPC(dev);
> +
> + s->ctrl = 0x00000100;
> + s->blk_idx = 0;
> + s->int_stat = 0;
> + s->int_en = 1;
> + s->int_info1 = 0;
> + s->int_info2 = 0;
> +
> + memset(s->blk_lut, 0, s->blk_max * sizeof(uint32_t));
> }
>
> static void tz_mpc_init(Object *obj)
> @@ -321,13 +430,35 @@ static void tz_mpc_realize(DeviceState *dev, Error **errp)
> "tz-mpc-downstream");
> address_space_init(&s->blocked_io_as, &s->blocked_io,
> "tz-mpc-blocked-io");
> +
> + s->blk_lut = g_new(uint32_t, s->blk_max);
> +}
> +
> +static int tz_mpc_post_load(void *opaque, int version_id)
> +{
> + TZMPC *s = TZ_MPC(opaque);
> +
> + /* Check the incoming data doesn't point blk_idx off the end of blk_lut. */
> + if (s->blk_idx >= s->blk_max) {
> + return -1;
> + }
> + return 0;
> }
>
> static const VMStateDescription tz_mpc_vmstate = {
> .name = "tz-mpc",
> .version_id = 1,
> .minimum_version_id = 1,
> + .post_load = tz_mpc_post_load,
> .fields = (VMStateField[]) {
> + VMSTATE_UINT32(ctrl, TZMPC),
> + VMSTATE_UINT32(blk_idx, TZMPC),
> + VMSTATE_UINT32(int_stat, TZMPC),
> + VMSTATE_UINT32(int_en, TZMPC),
> + VMSTATE_UINT32(int_info1, TZMPC),
> + VMSTATE_UINT32(int_info2, TZMPC),
> + VMSTATE_VARRAY_UINT32(blk_lut, TZMPC, blk_max,
> + 0, vmstate_info_uint32, uint32_t),
> VMSTATE_END_OF_LIST()
> }
> };
>
On 15 June 2018 at 08:23, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Peter,
>
> On 06/04/2018 05:29 PM, Peter Maydell wrote:
>> Implement the missing registers for the TZ MPC.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> + case A_BLK_LUT:
>> + r = s->blk_lut[s->blk_idx];
>> + if (size == 4) {
> don't you need to test CTRL[8] before doing the auto-increment?
>
> spec says:
> "A full word write or read to this register
> automatically increments the BLK_IDX by one if
> enabled by CTRL[8]."
Oops, yes, definitely.
thanks
-- PMM
© 2016 - 2026 Red Hat, Inc.