Re: [PATCH v3 01/15] hw: encode accessing CPU index in MemTxAttrs

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 31/10/22 14:03, Peter Maydell wrote:
> On Mon, 31 Oct 2022 at 12:08, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 4/10/22 16:54, Peter Maydell wrote:
>>> On Tue, 4 Oct 2022 at 14:33, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>>
>>>>
>>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>>> The MSC is in the address map like most other stuff, and thus there is
>>>>> no restriction on whether it can be accessed by other things than CPUs
>>>>> (DMAing to it would be silly but is perfectly possible).
>>>>>
>>>>> The intent of the code is "pass this transaction through, but force
>>>>> it to be Secure/NonSecure regardless of what it was before". That
>>>>> should not involve a change of the requester type.
>>>>
>>>> Should we assert (or warn) when the requester_type is unspecified?
>>>
>>> Not in the design of MemTxAttrs that's currently in git, no:
>>> in that design it's perfectly fine for something generating
>>> memory transactions to use MEMTXATTRS_UNSPECIFIED (which defaults
>>> to meaning a bunch of things including "not secure").
>>
>> In tz_mpc_handle_block():
>>
>> 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) {
>>
>>           s->int_info1 = addr;
>>           s->int_info2 = 0;
>>           s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, HMASTER,
>>                                     attrs.requester_id & 0xffff);
>>           s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, HNONSEC,
>>                                     ~attrs.secure);
>>           s->int_info2 = FIELD_DP32(s->int_info2, INT_INFO2, CFG_NS,
>>                                     tz_mpc_cfg_ns(s, addr));
>>
>>
>> Should we check whether the requester is MTRT_CPU?
> 
> That code is basically assuming that the requester_id is the AMBA AHB
> 'HMASTER' field (i.e. something hopefully unique to all things that
> send out transactions, not necessarily limited only to CPUs), which is a
> somewhat bogus assumption given that it isn't currently any such thing...
> 
> I'm not sure if/how this patchset plans to model generic "ID of transaction
> generator".

Does your 'generic "ID of transaction generator"' fit into MTRT_MACHINE 
described as "for more complex encoding":

   'MACHINE indicates a machine specific encoding which needs further
    processing to decode into its constituent parts.'

?

> 
> -- PMM