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

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> 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.'
>
> ?

Yes - I've just done something similar to model the IOAPIC on x86.
Currently that uses a magic number of requester_id that is unique to the
"machine bus" but it could multiplex multiple bits of data on more
complex topologies.

I'll post v5 soon now I have x86 working.

-- 
Alex Bennée