On Tue, 3 Sep 2024 00:11:04 +0200 Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
> Hi Alireza, > > On 2/9/24 22:32, Alireza Sanaee wrote: > > This patch allows for easier manipulation of the cache description > > register, CCSIDR. Which is helpful for testing as well. Currently, > > numbers get hard-coded and might be prone to errors. > > > > Therefore, this patch adds a wrapper for different types of CPUs > > available in tcg to decribe caches. One function `make_ccsidr` > > supports two cases by carrying a parameter as FORMAT that can be > > LEGACY and CCIDX which determines the specification of the register. > > > > For CCSIDR register, 32 bit version follows specification [1]. > > Conversely, 64 bit version follows specification [2]. > > --- > > [cut] > > > Changes from v2 [3] -> v3: > > > > 1) add only one function instead of ccsidr32 and ccsidr64 > > 2) use deposit32 and deposit64 in ccsidr function > > > --- > > > [1] B4.1.19, ARM Architecture Reference Manual ARMv7-A and ARMv7-R > > edition, https://developer.arm.com/documentation/ddi0406 > > [2] D23.2.29, ARM Architecture Reference Manual for A-profile > > Architecture, https://developer.arm.com/documentation/ddi0487/latest/ > > --- > > [cut] > > > [3] > > https://lore.kernel.org/qemu-devel/20240830184713.224-1-alireza.san...@huawei.com/ > > > > --- > > > Signed-off-by: Alireza Sanaee <alireza.san...@huawei.com> > > --- > > target/arm/cpu-features.h | 50 ++++++++++++++++++ > > target/arm/cpu64.c | 19 ++++--- > > target/arm/tcg/cpu64.c | 108 > > +++++++++++++++++++------------------- 3 files changed, 117 > > insertions(+), 60 deletions(-) > > > > diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h > > index c59ca104fe..ef40b0dfdc 100644 > > --- a/target/arm/cpu-features.h > > +++ b/target/arm/cpu-features.h > > @@ -1022,6 +1022,56 @@ static inline bool > > isar_feature_any_evt(const ARMISARegisters *id) return > > isar_feature_aa64_evt(id) || isar_feature_aa32_evt(id); } > > > > +typedef enum { > > + CCSIDR_FORMAT_LEGACY, > > + CCSIDR_FORMAT_CCIDX, > > +} CCSIDRFormat; > > + > > +static inline uint64_t make_ccsidr(CCSIDRFormat format, unsigned > > assoc, > > + unsigned linesize, unsigned > > cachesize, > > + uint8_t flags) > > +{ > > + unsigned lg_linesize = ctz32(linesize); > > + unsigned sets; > > + uint32_t ccsidr32 = 0; > > + uint64_t ccsidr64 = 0; > > deposit32() works with unsigned so you can use 'uint64_t ccsidr' for > both cases and return once. > > > + > > + assert(assoc != 0); > > + assert(is_power_of_2(linesize)); > > As mentioned in v2, if you insist in using an inlined method, you have > to include "qemu/host-utils.h" to get is_power_of_2() declaration. > > If the inlining is proven problematic later we can still un-inline > it, so as this is an useful cleanup (preferably with one 'ccsidr' > variable and the include): > Hi. sure! that makes sense. applied those two changes in v4. sorry for missing out the comment on v2. > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > > > + assert(lg_linesize >= 4 && lg_linesize <= 7 + 4); > > + > > + /* sets * associativity * linesie == cachesize. */ > > + sets = cachesize / (assoc * linesize); > > + assert(cachesize % (assoc * linesize) == 0); > > + > > + if (format == CCSIDR_FORMAT_LEGACY) { > > + /* > > + * The 32-bit CCSIDR format is: > > + * [27:13] number of sets - 1 > > + * [12:3] associativity - 1 > > + * [2:0] log2(linesize) - 4 > > + * so 0 == 16 bytes, 1 == 32 bytes, 2 == 64 > > bytes, etc > > + */ > > + ccsidr32 = deposit32(ccsidr32, 28, 4, flags); > > + ccsidr32 = deposit32(ccsidr32, 13, 15, sets - 1); > > + ccsidr32 = deposit32(ccsidr32, 3, 10, assoc - 1); > > + ccsidr32 = deposit32(ccsidr32, 0, 3, lg_linesize - 4); > > + return (uint64_t)ccsidr32; > > + } else { > > + /* > > + * The 64-bit CCSIDR_EL1 format is: > > + * [55:32] number of sets - 1 > > + * [23:3] associativity - 1 > > + * [2:0] log2(linesize) - 4 > > + * so 0 == 16 bytes, 1 == 32 bytes, 2 == 64 > > bytes, etc > > + */ > > + ccsidr64 = deposit64(ccsidr64, 32, 24, sets - 1); > > + ccsidr64 = deposit64(ccsidr64, 3, 21, assoc - 1); > > + ccsidr64 = deposit64(ccsidr64, 0, 3, lg_linesize - 4); > > + return ccsidr64; > > + } > > +} >