Richard Henderson <richard.hender...@linaro.org> 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 <alex.ben...@linaro.org> 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