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

From: Marc Zyngier <maz@kernel.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Will Deacon <will@kernel.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH] target/arm: Honor HCR_EL2.TID3 trapping requirements
Date: Mon, 25 Nov 2019 17:49:07 +0000	[thread overview]
Message-ID: <15acc2b5111716b10ab7624a1ee45ce7@www.loen.fr> (raw)
In-Reply-To: <CAFEAcA87ceF_0ifLn1t9CxyEwJ-rwW8h4QauJGk+ATphJaWa6Q@mail.gmail.com>

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...