| Message ID | 1588426445-24344-4-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/cpufeature: Introduce ID_PFR2, ID_DFR1, ID_MMFR5 and other changes | expand |
On Sat, May 02, 2020 at 07:03:52PM +0530, Anshuman Khandual wrote: > Double lock feature can have the following possible values. > > 0b0000 - Double lock implemented > 0b1111 - Double lock not implemented > > But in case of a conflict the safe value should be 0b1111. Hence this must > be a signed feature instead. Also change FTR_EXACT to FTR_LOWER_SAFE. > > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > arch/arm64/kernel/cpufeature.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 51386dade423..cba43e4a5c79 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -338,7 +338,7 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = { > }; > > static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { > - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 36, 28, 0), > + S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 28, 0), Wait, isn't this buggered today? Shouldn't that 28 be a 4? I think we really need to: 1. Make it impossible to describe overlapping fields, incomplete registers etc (ideally at build-time) 2. Have a macro that for 4-bit fields so you don't have to type '4' all the time Suzuki, any ideas how we can make this a bit more robust? Will
On 05/05/2020 04:40 PM, Will Deacon wrote: > On Sat, May 02, 2020 at 07:03:52PM +0530, Anshuman Khandual wrote: >> Double lock feature can have the following possible values. >> >> 0b0000 - Double lock implemented >> 0b1111 - Double lock not implemented >> >> But in case of a conflict the safe value should be 0b1111. Hence this must >> be a signed feature instead. Also change FTR_EXACT to FTR_LOWER_SAFE. >> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> >> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> arch/arm64/kernel/cpufeature.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c >> index 51386dade423..cba43e4a5c79 100644 >> --- a/arch/arm64/kernel/cpufeature.c >> +++ b/arch/arm64/kernel/cpufeature.c >> @@ -338,7 +338,7 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = { >> }; >> >> static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { >> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 36, 28, 0), >> + S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 28, 0), > > Wait, isn't this buggered today? Shouldn't that 28 be a 4? I think we really Ahh, right. Will fix it. > need to: > > 1. Make it impossible to describe overlapping fields, incomplete > registers etc (ideally at build-time) AFICS the _SHIFT defines for a given register must be placed sequentially with dummy place holders (4 bit wide) for any missing fields. In that case we could just call BUILD_BUG_ON() for any possible breakage or overlap. But wondering how and where to loop over these SHIFT values for these registers. Another way (not build time though) will be to scan through ftr_id_xxx[], fetch individual arm64_ftr_bits (assuming there are dummy entries for non existing fields) and assert that arm6r_ftr_bits[shift, width] validates against the previous arm6r_ftr_bits[shift, width] in an increasing manner. Either of these methods will require some more thoughts. > > 2. Have a macro that for 4-bit fields so you don't have to type '4' > all the time Except for ftr_single32[], all other arm64_ftr_bits entries have the exact same shift value (i.e 4). ARM64_FTR_WIDTH sounds good ? > > Suzuki, any ideas how we can make this a bit more robust? > > Will >
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 51386dade423..cba43e4a5c79 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -338,7 +338,7 @@ static const struct arm64_ftr_bits ftr_id_mmfr0[] = { }; static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = { - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 36, 28, 0), + S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 36, 28, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_PMSVER_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
Double lock feature can have the following possible values. 0b0000 - Double lock implemented 0b1111 - Double lock not implemented But in case of a conflict the safe value should be 0b1111. Hence this must be a signed feature instead. Also change FTR_EXACT to FTR_LOWER_SAFE. Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- arch/arm64/kernel/cpufeature.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)