On 28 January 2014 14:05, Peter Crosthwaite
<peter.crosthwaite@xxxxxxxxxx> wrote:
> On Tue, Jan 28, 2014 at 6:45 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
>> On 28 January 2014 01:46, Peter Crosthwaite
>> <peter.crosthwaite@xxxxxxxxxx> wrote:
>>> There seem to be multiple instances in this series where you fallback
>>> to open coded R/W accessor functions for the sake of access checks. Is
>>> it better to define a bool check_access() fn hook in ARMCPRegInfo and
>>> leave the actual write/read behaviour to the data driven mechanisms?
>>> This may also minimise the need for raw_write hook usages as it serves
>>> to isolate the actual state change into its own self contained
>>> definition (whether open coded or not).
>>
>> Yes, I think it's probably going to be better to do that. We may need
>> to make it more than just bool, though since for AArch64 the
>> kind of exception can be different I think -- the specific syndrome
>> information can vary.
>>
>
> I guess then it's simplest to just return in same format as read/write
> accessors.
Well, AArch64 means we also can't just return EXCP_UDEF or 0,
but ideally return at least some syndrome information.
I haven't quite figured out the nicest way to do this yet.
My current work in progress has:
* retain all the AArch32 EXCP_* classes as values for
env->exception_index
* whenever we generate an exception, also set up syndrome
register information in the cpu state struct; the
syndrome register high 6 bits happen to contain a category
code, but we don't map this to an env->exception_index
So for AArch64, access functions need to be able to return one
of three things:
(1) no exception, ie access OK
(2) exception which in UNDEF if taken to AArch32, or
a synchronous exception with ESR_ELx.EC = 0 ("uncategorized")
if taken to AArch64
(3) exception which is UNDEF if taken to AArch32, or a
synchronous exception with ESR_ELx.EC == 0x3/4/5/6/C/18
if taken to AArch64 (exact category depends on the
insn used to access the register, and ESR_ELx
includes all the opc1/2/crn/crm fields from the insn)
-- this generally is used for "access failed because of
a configurable trap or enable bit"
so we need to at least return a three-way "ok/disabled/undef"
code. I think actually most if not all of our current access
checks done in read/write fns should be type (2), but I don't
want to leave us stuck if we find one that needs to be type
(1) now or in the future. Failures due to the ri->access
permissions are always type (1).
We could return a complete EXCP_* value plus syndrome,
but that seems like overkill especially since the calling
code has most of the info needed for putting the syndrome
together properly. So I think:
typedef enum CPAccessResult {
/* Access is permitted */
CP_ACCESS_OK = 0,
/* Access fails due to a configurable trap or enable which would
* result in an exception syndrome other than 'uncategorized'
*/
CP_ACCESS_TRAP = 1,
/* Access fails and should result in an exception syndrome of
* 'uncategorized'
*/
CP_ACCESS_TRAP = 2,
} CPAccessResult;
typedef CPAccessResult CPAccessFn(CPUARMState *env,
const ARMCPRegInfo *opaque);
and an entry in ARMCPRegInfo:
/* Function for checking access permissions for the register
* which should be done in addition to those specified by
* the access field. If NULL, no extra checks required.
*/
CPAccessFn accessfn;
and CPReadFn and CPWriteFn both go from 'int' return to
'void' return.
(Anybody got a better name than CPAccessResult for the enum type?)
thanks
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
