Re: [Qemu-devel] [PATCH v3 3/6] target-arm: add hvc and smc exception emulation handling infrastructure

[Ard Biesheuvel]

On 9 September 2014 23:59, Peter Maydell <address@hidden> wrote:
> On 9 September 2014 22:51, Ard Biesheuvel <address@hidden> wrote:
>> On 9 September 2014 19:45, Peter Maydell <address@hidden> wrote:
>>> This is my suggestion for the best compromise between
>>> "theoretical perfect fidelity to the architecture" and
>>> "not too painful to implement":
>>> at translate time, do:
>>>
>>>   if (psci enabled via HVC || EL2 implemented) {
>>>       gen_ss_advance(s);
>>>       gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16));
>>>   } else {
>>>       unallocated_encoding();
>>>   }
>>> and ditto for SMC.
>>>
>>
>> OK, so does that mean I need to add fields to DisasContext for these
>> functions to inspect at the translation stage, and copy the
>> PSCI_METHOD_[SVC|HVC|NONE] values in it?
>
> You only need one field in DisasContext, but yes.
> (The idea of DisasContext and not giving most of translate-a64.c
> access to the CPU object is that it makes it hard to accidentally
> access stuff in the CPU object that's not valid to depend on at
> translate time, because it's an easy to spot and easy to review
> change if something new gets added. PSCI method type is
> OK because it's constant for the life of the simulation.)
>

OK

>> Yeah, makes sense. I will also add ARCH(6K) and ARCH(7) checks, for
>> SMC and HVC respectively.
>> (I don't suppose there is any point in adding TZ and VIRT feature bits
>> for this atm)
>
> We already have ARM_FEATURE_EL2 and ARM_FEATURE_EL3,
> actually. You should probably look at Edgar's patchset on list which
> adds proper SMC/HVC support -- that has failed review on a
> few of the early patches but the middle of the set includes
> some which also change this area:
> http://lists.gnu.org/archive/html/qemu-devel/2014-08/msg02865.html
> http://lists.gnu.org/archive/html/qemu-devel/2014-08/msg02866.html
> I don't want this patchset to depend on that one but you
> might find the shape of the code useful. (It doesn't do anything
> in the 32 bit code, though.)
>

I had a peek, and there is indeed substantial overlap, but not in an
incompatible way afaict
Setting ARM_FEATURE_EL[2|3] and then only emulate some HVC/SMC calls
under some circumstances is probably not the right way to go.
In fact, I think they are mutually exclusive, i.e., it probably makes
little sense to fully emulate EL2 /and/ use the HVC conduit for PSCI
at the same time.