Re: [Qemu-arm] [PATCH 20/27] hw/misc/tz-mpc.c: Implement correct blocked-access behaviour

[Peter Maydell]

On 23 May 2018 at 11:49, Alex Bennée <address@hidden> wrote:
>
> Peter Maydell <address@hidden> writes:
>
>> The MPC is guest-configurable for whether blocked accesses:
>>  * should be RAZ/WI or cause a bus error
>>  * should generate an interrupt or not
>>
>> Implement this behaviour in the blocked-access handlers.
>>
>> Signed-off-by: Peter Maydell <address@hidden>
>> ---
>>  hw/misc/tz-mpc.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/misc/tz-mpc.c b/hw/misc/tz-mpc.c
>> index 93453cbef2..39a72563b7 100644
>> --- a/hw/misc/tz-mpc.c
>> +++ b/hw/misc/tz-mpc.c
>> @@ -43,6 +43,9 @@ REG32(INT_EN, 0x28)
>>      FIELD(INT_EN, IRQ, 0, 1)
>>  REG32(INT_INFO1, 0x2c)
>>  REG32(INT_INFO2, 0x30)
>> +    FIELD(INT_INFO2, HMASTER, 0, 16)
>> +    FIELD(INT_INFO2, HNONSEC, 16, 1)
>> +    FIELD(INT_INFO2, CFG_NS, 17, 1)
>>  REG32(INT_SET, 0x34)
>>      FIELD(INT_SET, IRQ, 0, 1)
>>  REG32(PIDR4, 0xfd0)
>> @@ -266,6 +269,45 @@ static const MemoryRegionOps tz_mpc_reg_ops = {
>>      .impl.max_access_size = 4,
>>  };
>>
>> +static inline bool tz_mpc_cfg_ns(TZMPC *s, hwaddr addr)
>> +{
>> +    /* Return the cfg_ns bit from the LUT for the specified address */
>> +    hwaddr blknum = addr / s->blocksize;
>> +    hwaddr blkword = blknum / 32;
>> +    uint32_t blkbit = 1U << (blknum % 32);
>> +
>> +    /* This would imply the address was larger than the size we
>> +     * defined this memory region to be, so it can't happen.
>> +     */
>> +    assert(blkword < s->blk_max);
>> +    return s->blk_lut[blkword] & blkbit;
>> +}
>> +
>> +static MemTxResult tz_mpc_handle_block(TZMPC *s, hwaddr addr, MemTxAttrs 
>> attrs)
>> +{
>> +    /* Handle a blocked transaction: raise IRQ, capture info, etc */
>> +    if (!s->int_stat) {
>> +        /* First blocked transfer: capture information into INT_INFO1 and
>> +         * INT_INFO2. Subsequent transfers are still blocked but don't
>> +         * capture information until the guest clears the interrupt.
>> +         */
>> +
>> +        s->int_info1 = addr;
>> +        s->int_info2 = 0;
>> +        s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, HMASTER,
>> +                                  attrs.requester_id & 0xffff);
>
> Does this actually need masking given the source is a 16 bit wide bitfield?

That might change in future (eg Eric was talking about wanting
to put SMMUv3 substream IDs in transaction attributes, and those
are 20 bits wide, so we might want to widen requester_id to fit them).

thanks
-- PMM