| Message ID | 20191010131943.26822-1-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: cpufeature: Fix truncating a feature value | expand |
On Thu, Oct 10, 2019 at 02:19:43PM +0100, Suzuki K Poulose wrote: > A signed feature value is truncated to turn to an unsigned value > causing bad state in the system wide infrastructure. This affects > the discovery of FP/ASIMD support on arm64. Fix this by making sure > we cast it properly. > > This was inadvertently fixed upstream in v4.6 onwards with the following : > commit 28c5dcb22f90113dea ("arm64: Rename cpuid_feature field extract routines") What prevents us from just taking that commit instead? You did not document that here at all, which I thought I asked for. Also, you only need 12 digits for a sha1, 28c5dcb22f90 ("arm64: Rename cpuid_feature field extract routines") would be just fine :) thanks, greg k-h
Hi Greg, On 11/10/2019 05:55, Greg KH wrote: > On Thu, Oct 10, 2019 at 02:19:43PM +0100, Suzuki K Poulose wrote: >> A signed feature value is truncated to turn to an unsigned value >> causing bad state in the system wide infrastructure. This affects >> the discovery of FP/ASIMD support on arm64. Fix this by making sure >> we cast it properly. >> >> This was inadvertently fixed upstream in v4.6 onwards with the following : >> commit 28c5dcb22f90113dea ("arm64: Rename cpuid_feature field extract routines") > > What prevents us from just taking that commit instead? You did not > document that here at all, which I thought I asked for. Sorry, I missed that part. So, that change introduces helpers to extract feature fields based on the sign. And it also depends on commit ff96f7bc7bf6 ("arm64: capabilities: Handle sign of the feature bit") which introduces "sign" bit for the "capability" list and modifies the generic capability->matches() helpers to use the hint to switch to the appropriate helpers. I could backport parts of the commit 28c5dcb22f90 dropping the bits that affect the changes mentioned above. > > Also, you only need 12 digits for a sha1, 28c5dcb22f90 ("arm64: Rename > cpuid_feature field extract routines") would be just fine :) Yea, I understand. Its simply a pain to count the numbers, so I make sure to pickup something that looks larger than the 12 ;-). I will try to stick to that :-) Cheers Suzuki
On Fri, Oct 11, 2019 at 11:31:30AM +0100, Suzuki K Poulose wrote: > Hi Greg, > > On 11/10/2019 05:55, Greg KH wrote: > > On Thu, Oct 10, 2019 at 02:19:43PM +0100, Suzuki K Poulose wrote: > > > A signed feature value is truncated to turn to an unsigned value > > > causing bad state in the system wide infrastructure. This affects > > > the discovery of FP/ASIMD support on arm64. Fix this by making sure > > > we cast it properly. > > > > > > This was inadvertently fixed upstream in v4.6 onwards with the following : > > > commit 28c5dcb22f90113dea ("arm64: Rename cpuid_feature field extract routines") > > > > What prevents us from just taking that commit instead? You did not > > document that here at all, which I thought I asked for. > > Sorry, I missed that part. So, that change introduces helpers to > extract feature fields based on the sign. And it also depends on > > commit ff96f7bc7bf6 ("arm64: capabilities: Handle sign of the feature bit") > > which introduces "sign" bit for the "capability" list and modifies > the generic capability->matches() helpers to use the hint to switch to the > appropriate helpers. That's ok, does that cause any problems? We always want the original patch instead of a one-off patch as that way we do not diverge. > I could backport parts of the commit 28c5dcb22f90 dropping the bits > that affect the changes mentioned above. Please do, that is always prefered as well, but do the first thing above if at all possible. > > > > Also, you only need 12 digits for a sha1, 28c5dcb22f90 ("arm64: Rename > > cpuid_feature field extract routines") would be just fine :) > > Yea, I understand. Its simply a pain to count the numbers, so I make sure > to pickup something that looks larger than the 12 ;-). I will try to stick > to that :-) git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n" will give you the correct format. I suggest making it a git alias :) Or, use: [core] abbrev = 12 in your .gitconfig file. thanks, greg k-h
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 0a66f8241f18..9eb0d8072dd9 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -151,8 +151,8 @@ static inline u64 arm64_ftr_mask(struct arm64_ftr_bits *ftrp) static inline s64 arm64_ftr_value(struct arm64_ftr_bits *ftrp, u64 val) { return ftrp->sign ? - cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width) : - cpuid_feature_extract_unsigned_field_width(val, ftrp->shift, ftrp->width); + (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width) : + (s64)cpuid_feature_extract_unsigned_field_width(val, ftrp->shift, ftrp->width); } static inline bool id_aa64mmfr0_mixed_endian_el0(u64 mmfr0)
A signed feature value is truncated to turn to an unsigned value causing bad state in the system wide infrastructure. This affects the discovery of FP/ASIMD support on arm64. Fix this by making sure we cast it properly. This was inadvertently fixed upstream in v4.6 onwards with the following : commit 28c5dcb22f90113dea ("arm64: Rename cpuid_feature field extract routines") Cc: stable@vger.kernel.org # v4.4 Cc: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- arch/arm64/include/asm/cpufeature.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)