Re: [Qemu-devel] [RFC PATCH 00/13] target/arm: Derive cpu id regs from features

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> This is something we talked about in the context of enabling sve in
> system mode.  We don't want to replicate info between these two locations.
>
> I'm not 100% happy with this, thus the RFC.  In particular, there are
> several places in id_isar0, id_isar2, and id_isar4 that expose micro-
> architectural details of the cpus.  We cannot infer these values.
> We'll not be able to replicate the exact id values without additional
> changes.

Can you remind me of which patch it was in the SVE system mode series
that prompted this as patches 1-3 of that series seem perfectly
reasonable to me. Without seeing a case where this is manifestly better
than the alternative it feels a bit like too much churn for little
direct benefit. OTOH it doesn't make things worse (not withstanding
fixing the final assert failures).

> But I'll also note that with ARM_FEATURE_SWP, we're now at 60 feature
> bits, which means that we only have 4 remaining before we have to come
> up with another solution there.

Do we? I seem to recall Peter had played with widening the feature field
without any major issue. Given they are static checks I would have
thought the compiler would just end up using a slightly different offset
to test the higher bits.

>
> I do wonder if we should instead introduce some little inline functions
> to test each of the current feature bits, and once that's done convert
> those to test cpu->id_* bits.
>
> Most, but not all, of the feature bits would go away.  We'd have the
> exact id values one would expect for a given cpu without having to
> replicate the info.
>
> Thoughts, one way or the other?

I haven't looked too closely at the individual bit patterns as reviewing
system registers on a single screen laptop is a little painful compared
to at home. However you can at least have an:

Acked-by: Alex Bennée <address@hidden>

for the series. I guess I'm ambivalent about this particular series.

>
>
> r~
>
>
> Richard Henderson (13):
>   target/arm: Add ARM_FEATURE_SWP
>   target/arm: Derive id_isar0 from features
>   target/arm: Derive id_isar1 from features
>   target/arm: Derive id_isar2 from features
>   target/arm: Derive id_isar3 from features
>   target/arm: Derive id_isar4 from features
>   target/arm: Derive id_isar5 and id_isar6 from features
>   target/arm: Derive id_pfr0 from features
>   target/arm: Derive id_pfr1 from features
>   target/arm: Derive id_aa64isar0 from features
>   target/arm: Derive id_aa64isar1 from features
>   target/arm: Derive id_aa64pfr0 from features
>   target/arm: Remove assertions from resolve_id_regs
>
>  target/arm/cpu.h       |   1 +
>  linux-user/elfload.c   |   3 +-
>  target/arm/cpu.c       | 381 +++++++++++++++++++++++++++++++++++++++++
>  target/arm/translate.c |   4 +
>  4 files changed, 388 insertions(+), 1 deletion(-)


--
Alex Bennée