Re: [PATCH 2/3] target/arm/ptw.c: Add two new spaces to the granule protection case

[Jim MacArthur <jim.macarthur@linaro.org>]

On Fri, Apr 17, 2026 at 08:36:52AM +1000, Richard Henderson wrote:
> On 4/16/26 21:12, Jim MacArthur wrote:
> > This makes no functional difference, it just marks out two cases for
> > future use.
> > 
> > Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> > ---
> >   target/arm/ptw.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> > index 7b993bb5b3..d4b63a365a 100644
> > --- a/target/arm/ptw.c
> > +++ b/target/arm/ptw.c
> > @@ -510,6 +510,11 @@ bool arm_granule_protection_check(ARMGranuleProtectionConfig config,
> >           break;
> >       case 0b1111: /* all access */
> >           return true;
> > +    case 0b0100: /* system agent only */
> > +    case 0b0101: /* non-secure protected */
> > +        /* System Agent and Non-secure Protected are GDI extensions. */
> > +        /* No processing element should have access to these. */
> > +        goto fault_walk;
> >       case 0b1000: /* secure */
> >           if (!config.support_sel2) {
> >               goto fault_walk;
> > 
> 
> There's no real point to this patch, since we get the same effect from the default case.
> 
> It should be trivial to "properly" implement these two cases for the CPU, since
> 
>     R_QCLPTA:  PE is not permitted to access memory within the NSP
>     or the SA PA spaces, from any Security state.
> 
> Access to these spaces is only allowed from external devices via the SMMU.
> 
> The only effect on the PE is to check the various GPCCR fields (SA, NSP,
> NA6, NA7) to transform them from one type of fault to another: fault_walk ->
> fault_fail (i.e. break).

The thing that held me back here was the mapping from 'Reserved' in the table in D9.6.5 to fault_walk, which wasn't clear in the documentation, and hence leaving the behaviour unchanged. However, I would agree that we can map (GPCCR.SA=1, gpi=0x0x100) to fault_fail and so on and leave the (SA=0, 0b0100) case to what it would have been before (fault_walk).

Thanks for your feedback,

Jim