On 2/7/19 8:47 AM, Peter Maydell wrote:
> On Mon, 14 Jan 2019 at 01:11, Richard Henderson
> <richard.hender...@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> target/arm/helper-a64.h | 1 +
>> target/arm/mte_helper.c | 55 ++++++++++++++++++++++++++++++++++++++
>> target/arm/translate-a64.c | 7 +++++
>> 3 files changed, 63 insertions(+)
>>
>> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
>> index fa4c371a47..7a6051fdab 100644
>> --- a/target/arm/helper-a64.h
>> +++ b/target/arm/helper-a64.h
>> @@ -104,3 +104,4 @@ DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env,
>> i64)
>> DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>>
>> DEF_HELPER_FLAGS_2(mte_check, TCG_CALL_NO_WG, i64, env, i64)
>> +DEF_HELPER_FLAGS_3(irg, TCG_CALL_NO_RWG, i64, env, i64, i64)
>> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
>> index 6f4bc0aa04..1878393fc4 100644
>> --- a/target/arm/mte_helper.c
>> +++ b/target/arm/mte_helper.c
>> @@ -36,6 +36,48 @@ static int allocation_tag_from_addr(uint64_t ptr)
>> return (extract64(ptr, 56, 4) + extract64(ptr, 55, 1)) & 15;
>> }
>>
>> +/* Like ChooseNonExcludedTag, except that GCR_EL1 is already in. */
>
> I don't understand this comment -- neither the pseudocode
> function nor this code refer to GCR_EL1.
That's changed since the first rev. Now ChooseNonExcludedTag just has an
exclude argument, and the caller includes GCR_EL1.Exclude.
>> +static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
>> +{
>> + if (exclude != 0xffff) {
>> + int i;
>> + for (i = 0; i < offset; ++i) {
>> + do {
>> + tag = (tag + 1) & 15;
>> + } while (exclude & (1 << tag));
>> + }
>> + }
>> + return tag;
>
> This doesn't seem to do the same thing as ChooseNonExcludedTag()
> for the offset==0 case, or for the exclude == 0xffff case.
All of this has changed since the first rev too.
> This comment is trying to say that our IMPDEF choice for GCR_EL1.RRND==1
> is "behave the same as if RRND==0", right? I think that would be
> clearer as a comment at the callsite, because if you're following
> along with the pseudocode you expect the IRG code to do an
> "if RRND == 1 then { choose_random_nonexcluded_tag(); } else { ... }".
Thanks for the verbage.
r~
