On Mon, 14 Sept 2026 at 19:39, Richard Henderson
<[email protected]> wrote:
>
> On 9/14/26 08:21, Peter Maydell wrote:
> > On Mon, 14 Sept 2026 at 19:12, Richard Henderson
> > <[email protected]> wrote:
> >>
> >> On 9/14/26 07:51, Peter Maydell wrote:
> >>> On Mon, 14 Sept 2026 at 18:21, Richard Henderson
> >>> <[email protected]> wrote:
> >>>>
> >>>> On 9/13/26 23:16, Peter Maydell wrote:
> >>>>> On Thu, 6 Aug 2026 at 18:32, Richard Henderson
> >>>>> <[email protected]> wrote:
> >>>>>>
> >>>>>> It is IMPLEMENTATION DEFINED whether HCR_EL2.TID3 traps on registers
> >>>>>> within the range which are not explicitly listed. Previously we chose
> >>>>>> not to honor the TID3 bit.
> >>>>>>
> >>>>>> However, with FEAT_IDTE3 it becomes mandatory to honor SCR_EL3.TID3
> >>>>>> on these registers, so we might as well honor HCR_EL2.TID3 too.
> >>>>>
> >>>>> I thought we agreed not to change the behaviour here? Is this
> >>>>> just a "forgot to update commit message" thing?
> >>>> Oh, yes. Because I did update the code to test for v8 in translate.c.
> >>>
> >>> So what does this patch do? Is it just a no-behaviour-change
> >>> refactor so we have a place to add the SCR_EL3.TID3 check
> >>> in the following patch ?
> >> It does what it says on the tin -- it honors HCR_EL2.TID3 for undefined
> >> registers. We currently only do this via access_tid3 for defined
> >> registers.
> >
> > But there aren't any genuinely UNDEFINED encodings in the TID3
> > space, are there? The ID register space has to RES0, not UNDEF,
> > for reserved encodings, and so we have cpreg structs for
> > all the reserved encodings in the ID space.
> Since we don't do this for all arm_cpreg_encoding_in_idspace(), I
> assumed that we didn't do it for all arm_cpreg_encoding_in_tid3() either.
>
> Either that, or we have existing dead code testing
> arm_cpreg_encoding_in_idspace()?
Looking more closely at the Arm ARM (DDI0487M.c section D23.3.1)
I wasn't quite right in vaguely referring to "the ID register
space". The whole Feature ID space covers:
op0 == 3, op1 == {0,1,3}, CRn == 0, CRm == {0-7}, op2 == {0-7}
The "unallocated encodings RES0 from EL1 and above" and "TID3 traps
apply" space is only a subset of that, where op1 == 0 and CRm == {2-7}.
For the rest of ID space, unallocated encodings UNDEF (or trap
as "sysreg trap" if FEAT_IDST is present), and TID3 traps don't apply.
So we need arm_cpreg_encoding_in_idspace() for the areas
where unallocated encodings UNDEF; we also need it for
the "access from EL0" case, which fails the cpreg .access
check. But we shouldn't need to jump through the same hoops
for TID3 traps, because they should only apply to the smaller
part of the space where we have complete cpreg struct coverage
and the .access trap doesn't forbid EL1-and-up accesses.
I think the upshot of that is that we can just add the
SCR_EL3.TID3 check to where we currently check HCR_EL2.TID3.
thanks
-- PMM