On 9 February 2014 03:27, Peter Crosthwaite
<peter.crosthwaite@xxxxxxxxxx> wrote:
> On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
>> -/* Access functions for coprocessor registers. These should always succeed. */
>> -typedef int CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque,
>> - uint64_t *value);
>> -typedef int CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque,
>> - uint64_t value);
>> +/* Access functions for coprocessor registers. These cannot fail and
>> + * may not raise exceptions.
>
> Is this based on an assumption that the only possible reason for an
> exception is access violation? This series quite validly obsoletes the
> need for exception-via-return-path, but my understanding is that CP
> accessor functions are able to implement any form of side affects. If
> an exception is thus generated from the side effects of a CP access
> then thats fair game - it just happens via the already existing
> mechanisms for generating an exception from helper context. Long story
> short, I think you can just drop second sentence from this comment.
It's based on the fact that the translate.c code no longer supports
calling raise_exception() from within a read/write function -- if you
try it the PC will be wrong because we haven't synchronised it
before calling the helper function. You're right that in theory a
coprocessor access could raise an arbitrary exception, but if
anybody needs that they'll need to add the support (probably by
adding an extra ARM_CP_ flag for "can raise exceptions" so we
don't take the hit of synchronising PC except in the odd case
where it's necessary.) In practice I think it is unlikely that there
will be any such situation which couldn't be handled by a suitable
accessfn.
>> uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
>> {
>> const ARMCPRegInfo *ri = rip;
>> - uint64_t value;
>> - int excp = ri->readfn(env, ri, &value);
>> - if (excp) {
>> - raise_exception(env, excp);
>> - }
>> - return value;
>
> Should ideally be a blank line here, but ..
>
>> + return ri->readfn(env, ri);
>
> ... you could just drop the single use variable completely with:
>
> return ri->readfn(env, (const ARMCPRegInfo *)rip);
I dislike casts like that. I'll put in the blank line if you like.
thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
