Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Sat, Dec 21, 2013 at 8:07 AM, Peter Maydell <address@hidden> wrote:
> On 20 December 2013 21:41, Peter Crosthwaite
> <address@hidden> wrote:
>> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <address@hidden> wrote:
>>> On 20 December 2013 10:00, Peter Maydell <address@hidden> wrote:
>>>> I'll have a think about this, because there are some wrinkles relating
>>>> to reset that might be usefully solved this way.
>>>
>>> So my conclusion here is that really this boils down to how we
>>> want to deal with sysregs for AArch64 capable CPUs in general.
>>> There are two models I can think of:
>>>
>>> Option 1:
>>>  * we provide new reginfo definitions that describe the AArch64
>>>    views of the system registers
>>>  * for the AArch32 views, we just fall through and use the existing
>>>    definitions we've written for AArch32-only CPUs
>>>
>>> Option 2:
>>>  * we provide new reginfo definitions for the AArch64 views of
>>>    the registers, which include annotations to say whether there
>>>    is an AArch32 view of this register
>>
>> So to clarify, would MIDR and friends be in this bucket? And does t
>> obsolete the old MIDR def such there is only one CPRegInfo globally?
>
> I think we end up with a MIDR def for old CPUs and a MIDR
> for v8-and-up CPUs which handles both AArch64 and AArch32 views:
>             { .name = "MIDR_EL1",
>               .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
>               .access = PL1_R,
>              .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW,
>              .resetvalue = cpu->midr },
>
> because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x
> registers read as MIDR"

Ok so MIDR is an annoying special case (Some of the "friends" I listed
early are probably better examples).

> and (b) we can ignore the annoying
> special case handling for the TI925.
>
> (I wonder if it would be nicer to have a .state field (values

Ideally, I think its about which encoding schemes have visibility of
this reg. From what we discussed so far, it seems that there are three
separate encoding schemes:

32/MRC - the current 99% case, but needs to now support access to
widened-to-64 registers (upper RES0 policy)
32/MRRC - 64bit regs accessible from 32
64/MRS - What you are adding here

One hot encode accessibility via each of these in a ".encodings"
field, then the implementations or MRC, MRRC and MRS just check
independently whether they are allowed or not.

> CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH)
> rather than shoehorning it into the flags field...)
>

Agreed - new field.

> For cases where we don't have annoying v7-and-earlier legacy
> to deal with we can just have one reginfo struct I think.
>
> I think it's the weird stuff that's going to get irritating because
> we're going to end up with "not for v8" logic to suppress half of
> them.
>
>>>  * for the few registers which aren't neatly arranged so the
>>>    crn/crm/opc1/opc2 line up, we just split up into a separate
>>>    reginfo for AArch64 and AArch32
>>
>> ACK, that sounds awkward but there nothing we can do abt it. how many
>> are there? The few I checked always line up.
>
> Well, everything in cp14 doesn't have architectural equivalents,
> for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2
> but that is OSDTRRX_EL1 in AArch64. The TLB and cache
> maintenance ops don't always match up because they've been
> tidied up, I think. The docs don't make it terribly easy to compile
> a list of mismatches though.
>
>>>  * register_cp_regs_for_features() has separate AArch32
>>>    and AArch64 versions
>>>
>>
>> Do we need this? With this scheme its still possible to enforce
>> exclusivity in the CPRegInfo themselves.
>
> Mmm, I guess we don't really, though it does mean we're
> a bit more likely to have dubious stuff lurking around in
> the AArch32-visible sysregs. (OTOH that's pretty irrelevant
> until we have support for either Hyp or TrustZone, since
> otherwise EL1 is always AArch64...)

Is there more to it that just enforcing MRS/MRC/MRRC perms? An MRS
only defined CP reg should never have AArch32 visibility unless I am
missing something.

Regards,
Peter

>
> -- PMM
>