Re: [Qemu-devel] [PATCH v2 11/35] target-arm: Split cpreg access checks out from read/write functions

[Peter Maydell]

On 9 February 2014 02:50, Peter Crosthwaite
<address@hidden> wrote:
> On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <address@hidden> wrote:
>> +typedef enum CPAccessResult {
>> +    /* Access is permitted */
>> +    CP_ACCESS_OK = 0,
>> +    /* Access fails due to a configurable trap or enable which would
>> +     * result in a categorized exception syndrome giving information about
>> +     * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6,
>> +     * 0xc or 0x18).
>> +     */
>> +    CP_ACCESS_TRAP = 1,
>> +    /* Access fails and results in an exception syndrome 0x0 
>> ("uncategorized").
>> +     * Note that this is not a catch-all case -- the set of cases which may
>> +     * result in this failure is specifically defined by the architecture.
>> +     */
>> +    CP_ACCESS_TRAP_UNCATEGORIZED = 2,
>
> Hi Peter,
>
> So its not obvious to me just yet why these two types or traps needs
> separate encoding on this level. From your commentary, the two
> different types of traps are mutually exclusive and identifyable by
> the syndrome information. I.E _TRAP is syndrome != 0 and _TRAP_UNCAT
> is syndrome == 0. Cant the access checkers return boolean if is_trap
> then the syndrome can be used to make this distinction?

No, because the access checker is the thing that tells us what the
syndrome register is supposed to be. That is, the caller of the access
functions will do something like:

     ret = ri->accessfn(env, ri);
     switch (ret) {
          case CP_ACCESS_OK:
                 return;
          case CP_ACCESS_TRAP:
                 env->exception.syndrome = syn_cp_trap(stuff);
                 break;
          case CP_ACCESS_TRAP_UNCATEGORIZED:
                 env->exception_syndrome = syn_uncategorized();
                 break;
     }
     raise_exception(EXCP_UDEF);

(This code is in the syndrome work I have which sits on top of this
patchset; it's pretty nearly ready to post but I have a few odds and
ends to tie off first.)

The actual value of the syndrome register depends on a pile
of stuff that includes various fields from the instruction, so we
don't want to burden the access checkers with actually setting
or returning us a syndrome value. (Also the syndrome encoding
covers the whole uint32_t space so it's not possible to return
"syndrome or some value meaning no exception", which makes
that an awkward API choice anyway.) So we just have the checker
tell us "OK, or this set of syndromes, or that set?" and we can calculate
the syndrome from just that three-way bit of information.

The phrasing of the comments above is supposed to help somebody
implementing a cpreg know what they should be returning : the
ARM ARM docs will let them know which kind of syndrome category
the access failure is, which in turn indicates which CP_ value to
return to cause QEMU to generate the syndrome information that
is required.

thanks
-- PMM