[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/7] target-arm: add hvc and smc exception emula
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 3/7] target-arm: add hvc and smc exception emulation handling infrastructure |
Date: |
Thu, 15 May 2014 14:06:54 +0100 |
On 14 May 2014 21:55, Rob Herring <address@hidden> wrote:
> On Wed, May 14, 2014 at 12:44 PM, Peter Maydell
> <address@hidden> wrote:
>> We can't cpu_abort() just because the guest hands us an HVC
>> or SMC that we don't happen to handle in QEMU. We should
>> just qemu_log_mask(LOG_GUEST_ERROR, ...) and then do the
>> architecturally mandated behaviour for SMC or HVC instructions
>> on a CPU which doesn't implement EL2/EL3, ie treat as
>> UnallocatedEncoding(). (Don't forget to fix up exception.syndrome
>> in this case, since it should be syn_uncategorized(), not
>> the SMC/HVC value.)
>
> So I think this and shifting setting ESR/FAR after the switch should
> be sufficient:
>
> case EXCP_SMC:
> if (arm_cpu_do_smc(cs)) {
> return;
> }
> env->exception.syndrome = syn_uncategorized();
> if (is_a64(env)) {
> env->pc -= 4;
> } else {
> env->regs[15] -= env->thumb ? 2 : 4;
> }
> break;
>
> I'm not completely sure the PC adjustment is correct.
I don't think you want that env->thumb conditional:
the SMC instruction is 4 bytes in both the A32 and
T32 encodings, so we always need to rewind by 4 bytes.
>>> +static inline uint32_t syn_aa64_hvc(uint32_t imm16)
>>> +{
>>> + return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
>>> +}
>>> +
>>> +static inline uint32_t syn_aa32_hvc(uint32_t imm16)
>>> +{
>>> + return (EC_AA32_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
>>> +}
>>> +
>>> +static inline uint32_t syn_aa64_smc(uint32_t imm16)
>>> +{
>>> + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff);
>>> +}
>>> +
>>
>> You want a syn_aa32_smc() as well [note that it doesn't
>> take an immediate].
>
> I also noticed I need to set ARM_EL_IL based on thumb or not on the
> aa32 variants.
No, for HVC and SMC the Thumb instructions are 32 bit so
ARM_EL_IL should be set. (Breakpoint and SVC are different because
the T32 BKPT and SVC insn encodings are only 16 bits long.
In retrospect calling the syn_aa32_* parameter "is_thumb" rather
than "is_16bit" was perhaps misleading.)
thanks
-- PMM