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

[Peter Maydell]

On Wed, 28 Sept 2022 at 17:42, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 9/27/22 07:14, Alex Bennée wrote:
> > --- a/hw/misc/tz-msc.c
> > +++ b/hw/misc/tz-msc.c
> > @@ -137,11 +137,11 @@ static MemTxResult tz_msc_read(void *opaque, hwaddr 
> > addr, uint64_t *pdata,
> >           return MEMTX_OK;
> >       case MSCAllowSecure:
> >           attrs.secure = 1;
> > -        attrs.unspecified = 0;
> > +        attrs.requester_type = MTRT_CPU;
> >           break;
> >       case MSCAllowNonSecure:
> >           attrs.secure = 0;
> > -        attrs.unspecified = 0;
> > +        attrs.requester_type = MTRT_CPU;
> >           break;
>
> This is surely incomplete.  You can't just set "cpu" without saying where 
> it's from.
>
> Since this device is only used by the ARMSSE machine, I would hope that 
> attrs.unspecified
> should never be set before the patch, and thus MTRT_CPU should be set 
> afterward.

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.

thanks
-- PMM