Peter Maydell <peter.mayd...@linaro.org> writes:
> On Wed, 28 Sept 2022 at 17:42, Richard Henderson > <richard.hender...@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. Should we assert (or warn) when the requester_type is unspecified? > > thanks > -- PMM -- Alex Bennée
