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

[Ard Biesheuvel]

On 10 September 2014 17:42, Greg Bellows <address@hidden> wrote:
> We also have v4 of the TZ patches which do provide 32-bit EL3 support, but
> not EL2.  Maybe good to align with this code as well.
>
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg07347.html
>

As far as I can tell, there is very little overlap, only patch 8/33
contains some stuff that we added as well.
I expect PSCI handling and emulation of hvc and/or smc to be mutually
exclusive, nothing we won't be able to handle with a couple of
conditionals.

My apologies for not aligning with you beforehand, I just adopted some
patches from Rob that I needed for reset and poweroff under UEFI, and
I had no idea there was so much in flight already.

-- 
Ard.

>
> On 9 September 2014 16: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.)
>>
>> > 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.)
>>
>> thanks
>> -- PMM
>>
>