From: james.morse@arm.com (James Morse)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: rename the function arm64_is_ras_serror() to avoid confusion
Date: Thu, 15 Mar 2018 19:52:17 +0000 [thread overview]
Message-ID: <5AAACEF1.1060701@arm.com> (raw)
In-Reply-To: <CAMj-D2Cd7_fCwMuHKr-UQ3i4LEDkktxcg_GtO-QCqnj52mHyKA@mail.gmail.com>
Hi gengdongjiu,
On 26/02/18 16:13, gengdongjiu wrote:
> 2018-02-24 1:58 GMT+08:00 James Morse <james.morse@arm.com>:
>> 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
prev parent reply other threads:[~2018-03-15 19:52 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-22 18:02 [PATCH] arm64: rename the function arm64_is_ras_serror() to avoid confusion Dongjiu Geng
2018-02-23 17:58 ` James Morse
2018-02-26 16:13 ` gengdongjiu
2018-03-15 19:52 ` James Morse [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5AAACEF1.1060701@arm.com \
--to=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).