Re: [PATCH] target/arm: Honor HCR_EL2.TID3 trapping requirements

[Marc Zyngier <maz@kernel.org>]

On 2019-11-25 17:27, Peter Maydell wrote:
> On Mon, 25 Nov 2019 at 17:08, Marc Zyngier <maz@kernel.org> wrote:
>>
>> On 2019-11-25 16:21, Peter Maydell wrote:
>> > Missing .accessfn for ID_AA4PFR2_EL1_RESERVED ?
>>
>> Indeed, I oversaw that one. I'll fix it and repost it together with
>> the extra patches to handle TID1 and TID2.
>
> Given that rc3 (last chance, more or less, for bugfixes for 4.2)
> is tomorrow, I propose that I take this patch with the
> +              .accessfn = access_aa64idreg,
> line for ID_AA64PFR2_EL1_RESERVED squashed in. I think
> TID1/TID2 and the MMFR-from-aarch32 parts are less urgent ?

That'd be great, thank you.

TID2 is only a nice to have (it allows us to virtualize the cache
hierarchy, only a performance optimization for fairly stupid 32bit
guests), and TID1 is so far unused by Linux.

I also had a look at TID0, but couldn't find any of the Jazelle
stuff in QEMU...

>> > These are the AArch64 accessors for the aarch32 MVFR registers,
>> > but this trap bit is also supposed to trap aarch32 accesses to
>> > the MVFR registers, which are via the vmrs insn. That needs
>> > to be done in trans_VMSR_VMRS(); we can do that with a
>> > followup patch, since the mechanism there is completely different.
>>
>> Holy cow! I'm afraid that my newly acquired QEMU-foo is missing
>> a few key options. Does it mean we need to generate a trapping
>> instruction, as opposed to take the exception on access?
>
> We will need to generate a call to a helper function which
> does the access check (and possibly raises an exception).
> We can't determine at translate time whether the insn is
> going to cause an exception, because the TID3 bit is not one
> of the fairly limited set of information which we put into
> the TB flags and allow to be baked into the generated code.
> (In theory we could add it, but we don't have very many TB
> flags so we only do that for cases where the overhead of doing
> the check at runtime matters.)

Ah! I guess you're referring to the HELPER() stuff in the various
target/arm/*_helper.c files? If so, I think I see how to do it.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...