From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932328AbeCOTy7 (ORCPT ); Thu, 15 Mar 2018 15:54:59 -0400 Received: from foss.arm.com ([217.140.101.70]:47050 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751546AbeCOTy6 (ORCPT ); Thu, 15 Mar 2018 15:54:58 -0400 Message-ID: <5AAACEF1.1060701@arm.com> Date: Thu, 15 Mar 2018 19:52:17 +0000 From: James Morse User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: gengdongjiu CC: Dongjiu Geng , Catalin Marinas , Will Deacon , Christoffer Dall , Marc Zyngier , Linux Kernel Mailing List , arm-mail-list , kvmarm@lists.cs.columbia.edu, Huangshaoyu Subject: Re: [PATCH] arm64: rename the function arm64_is_ras_serror() to avoid confusion References: <1519322552-7374-1-git-send-email-gengdongjiu@huawei.com> <5A90563E.4020608@arm.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi gengdongjiu, On 26/02/18 16:13, gengdongjiu wrote: > 2018-02-24 1:58 GMT+08:00 James Morse : >> On 22/02/18 18:02, Dongjiu Geng wrote: >>> The RAS SError Syndrome can be Implementation-Defined, >>> arm64_is_ras_serror() is used to judge whether it is RAS SError, >>> but arm64_is_ras_serror() does not include this judgement. In order >>> to avoid function name confusion, we rename the arm64_is_ras_serror() >>> to arm64_is_categorized_ras_serror(), this function is used to >>> judge whether it is categorized RAS Serror. >> >> I don't see how 'categorized' is relevant. The most significant ISS bit is used >> to determine if this is an IMP-DEF ESR, or one that uses the architected layout. > > From the name arm64_is_ras_serror(), it used to judge whether this is > RAS Serror, > but arm64_is_ras_serror() think the IMP-DEF SError is not RAS SError, > as shown the code note and code in[1]. > In fact the IMP-DEF SError is also RAS SError, so when I read the > code, it looks like This is just you then. No-one else has your imp-def:RAS error ESR values. This would be like me adding some impdef branch instruction, then claiming aarch64_insn_is_branch() doesn't take account of my private additions. I agree the name is assuming all architected ESR are RAS-errors, and that impdef ESR are just that: impdef, that's all we know about them. Unless this causes us to do the wrong thing, I don't think it matters. Obviously we would need to change it if a new architected ESR is added. > confusion, so I rename it to arm64_is_categorized_ras_serror(), then This is actually worse, because there is an architected ESR for 'uncategorized', that the helper papers-over and treats as uncontained. Calling it 'categorized' means we now have three states, not two. > this function is only used to > judge whether this is categorized RAS SError, > if it is categorized, the code will continue judge its Asynchronous Error Type. > if it is uncategorized, the code will panic(this is the original code > logic) or not panic when we support kernel-first or can isolate the > SError Thanks, James From mboxrd@z Thu Jan 1 00:00:00 1970 From: james.morse@arm.com (James Morse) Date: Thu, 15 Mar 2018 19:52:17 +0000 Subject: [PATCH] arm64: rename the function arm64_is_ras_serror() to avoid confusion In-Reply-To: References: <1519322552-7374-1-git-send-email-gengdongjiu@huawei.com> <5A90563E.4020608@arm.com> Message-ID: <5AAACEF1.1060701@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi gengdongjiu, On 26/02/18 16:13, gengdongjiu wrote: > 2018-02-24 1:58 GMT+08:00 James Morse : >> On 22/02/18 18:02, Dongjiu Geng wrote: >>> The RAS SError Syndrome can be Implementation-Defined, >>> arm64_is_ras_serror() is used to judge whether it is RAS SError, >>> but arm64_is_ras_serror() does not include this judgement. In order >>> to avoid function name confusion, we rename the arm64_is_ras_serror() >>> to arm64_is_categorized_ras_serror(), this function is used to >>> judge whether it is categorized RAS Serror. >> >> I don't see how 'categorized' is relevant. The most significant ISS bit is used >> to determine if this is an IMP-DEF ESR, or one that uses the architected layout. > > From the name arm64_is_ras_serror(), it used to judge whether this is > RAS Serror, > but arm64_is_ras_serror() think the IMP-DEF SError is not RAS SError, > as shown the code note and code in[1]. > In fact the IMP-DEF SError is also RAS SError, so when I read the > code, it looks like This is just you then. No-one else has your imp-def:RAS error ESR values. This would be like me adding some impdef branch instruction, then claiming aarch64_insn_is_branch() doesn't take account of my private additions. I agree the name is assuming all architected ESR are RAS-errors, and that impdef ESR are just that: impdef, that's all we know about them. Unless this causes us to do the wrong thing, I don't think it matters. Obviously we would need to change it if a new architected ESR is added. > confusion, so I rename it to arm64_is_categorized_ras_serror(), then This is actually worse, because there is an architected ESR for 'uncategorized', that the helper papers-over and treats as uncontained. Calling it 'categorized' means we now have three states, not two. > this function is only used to > judge whether this is categorized RAS SError, > if it is categorized, the code will continue judge its Asynchronous Error Type. > if it is uncategorized, the code will panic(this is the original code > logic) or not panic when we support kernel-first or can isolate the > SError Thanks, James