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

[Peter Maydell]

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".

-- PMM