linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tbaicar@codeaurora.org (Baicar, Tyler)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/8] trace: ras: add ARM processor error information trace event
Date: Mon, 17 Apr 2017 11:18:43 -0600	[thread overview]
Message-ID: <f8ef4306-fd69-d8a3-3ca6-86be2825cab3@codeaurora.org> (raw)
In-Reply-To: <8aa30f6a-d18d-1cce-57dc-08efb52d822e@huawei.com>

On 4/16/2017 9:16 PM, Xie XiuQi wrote:
> On 2017/4/17 11:08, Xie XiuQi wrote:
>>> On 3/30/2017 4:31 AM, Xie XiuQi wrote:
>>>> Add a new trace event for ARM processor error information, so that
>>>> the user will know what error occurred. With this information the
>>>> user may take appropriate action.
>>>>
>>>> These trace events are consistent with the ARM processor error
>>>> information table which defined in UEFI 2.6 spec section N.2.4.4.1.
>>>>
>>>> ---
>>>> v2: add trace enabled condition as Steven's suggestion.
>>>>       fix a typo.
>>>> ---
>>>>
>>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>>> Cc: Tyler Baicar <tbaicar@codeaurora.org>
>>>> Signed-off-by: Xie XiuQi <xiexiuqi@huawei.com>
>>>> ---
>>>
...
>>>> +/*
>>>> + * First define the enums in MM_ACTION_RESULT to be exported to userspace
>>>> + * via TRACE_DEFINE_ENUM().
>>>> + */
>>>> +#undef EM
>>>> +#undef EMe
>>>> +#define EM(a, b) TRACE_DEFINE_ENUM(a);
>>>> +#define EMe(a, b)    TRACE_DEFINE_ENUM(a);
>>>> +
>>>> +ARM_PROC_ERR_TYPE
>>>> +ARM_PROC_ERR_FLAGS
>>> Are the above two lines supposed to be here?
>>>> +
>>>> +/*
>>>> + * Now redefine the EM() and EMe() macros to map the enums to the strings
>>>> + * that will be printed in the output.
>>>> + */
>>>> +#undef EM
>>>> +#undef EMe
>>>> +#define EM(a, b)        { a, b },
>>>> +#define EMe(a, b)    { a, b }
>>>> +
>>>> +TRACE_EVENT(arm_proc_err,
>>> I think it would be better to keep this similar to the naming of the current RAS trace events (right now we have mc_event, arm_event, aer_event, etc.). I would suggest using "arm_err_info_event" since this is handling the error information structures of the arm errors.
>>>> +
>>>> +    TP_PROTO(const struct cper_arm_err_info *err),
>>>> +
>>>> +    TP_ARGS(err),
>>>> +
>>>> +    TP_STRUCT__entry(
>>>> +        __field(u8, type)
>>>> +        __field(u16, multiple_error)
>>>> +        __field(u8, flags)
>>>> +        __field(u64, error_info)
>>>> +        __field(u64, virt_fault_addr)
>>>> +        __field(u64, physical_fault_addr)
>>> Validation bits should also be a part of this structure that way user space tools will know which of these fields are valid.
>> Could we use the default value to check the validation which we have checked in TP_fast_assign?
Yes, true...I guess we really don't need the validation bits then.
>>>> +    ),
>>>> +
>>>> +    TP_fast_assign(
>>>> +        __entry->type = err->type;
>>>> +
>>>> +        if (err->validation_bits & CPER_ARM_INFO_VALID_MULTI_ERR)
>>>> +            __entry->multiple_error = err->multiple_error;
>>>> +        else
>>>> +            __entry->multiple_error = ~0;
>>>> +
>>>> +        if (err->validation_bits & CPER_ARM_INFO_VALID_FLAGS)
>>>> +            __entry->flags = err->flags;
>>>> +        else
>>>> +            __entry->flags = ~0;
>>>> +
>>>> +        if (err->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO)
>>>> +            __entry->error_info = err->error_info;
>>>> +        else
>>>> +            __entry->error_info = 0ULL;
>>>> +
>>>> +        if (err->validation_bits & CPER_ARM_INFO_VALID_VIRT_ADDR)
>>>> +            __entry->virt_fault_addr = err->virt_fault_addr;
>>>> +        else
>>>> +            __entry->virt_fault_addr = 0ULL;
>>>> +
>>>> +        if (err->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR)
>>>> +            __entry->physical_fault_addr = err->physical_fault_addr;
>>>> +        else
>>>> +            __entry->physical_fault_addr = 0ULL;
>>>> +    ),
>>>> +
>>>> +    TP_printk("ARM Processor Error: type %s; count: %u; flags: %s;"
>>> I think the "ARM Processor Error:" part of this should just be removed. Here's the output with this removed and the trace event renamed to arm_err_info_event. I think this looks much cleaner and matches the style used with the arm_event.
>>>
>>>            <idle>-0     [020] .ns.   366.592434: arm_event: affinity level: 2; MPIDR: 0000000000000000; MIDR: 00000000510f8000; running state: 1; PSCI state: 0
>>>            <idle>-0     [020] .ns.   366.592437: arm_err_info_event: type cache error; count: 0; flags: 0x3; error info: 0000000000c20058; virtual address: 0000000000000000; physical address: 0000000000000000
> As this section is ARM Processor Error Section, how about use arm_proc_err_event?
This is not for the ARM Processor Error Section, that is what the 
arm_event is handling. What you are adding this trace support for here 
is called the ARM Processor Error Information (UEFI 2.6 spec section 
N.2.4.4.1). So I think your trace event here should be called 
arm_err_info_event. This will also be consistent with the other two 
trace events that I'm planning on adding:

arm_ctx_info_event: ARM Processor Context Information (UEFI 2.6 section 
N.2.4.4.2)
arm_vendor_info_event: This is the "Vendor Specific Error Information" 
in the ARM Processor Error Section (Table 260). It's possible I may just 
add this into the arm_event trace event, but I haven't looked into it 
enough yet.

Thanks,
Tyler

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

  reply	other threads:[~2017-04-17 17:18 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30 10:31 [PATCH v3 0/8] arm64: acpi: apei: handle SEI notification type for ARMv8 Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 1/8] trace: ras: add ARM processor error information trace event Xie XiuQi
2017-03-30 16:02   ` Steven Rostedt
2017-04-06  9:03     ` Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 2/8] acpi: apei: handle SEI notification type for ARMv8 Xie XiuQi
2017-03-31 16:20   ` James Morse
2017-04-06  9:11     ` Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 3/8] arm64: apei: add a per-cpu variable to indecate sei is processing Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 4/8] APEI: GHES: reserve a virtual page for SEI context Xie XiuQi
2017-03-31 16:22   ` James Morse
2017-04-06  9:25     ` Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 5/8] arm64: KVM: add guest SEI support Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 6/8] arm64: RAS: add ras extension runtime detection Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 7/8] arm64: exception: handle asynchronous SError interrupt Xie XiuQi
2017-04-13  8:44   ` Xiongfeng Wang
2017-04-13 10:51   ` Mark Rutland
2017-04-14  7:03     ` Xie XiuQi
2017-04-18  1:09     ` Xiongfeng Wang
2017-04-18 10:51       ` James Morse
2017-04-19  2:37         ` Xiongfeng Wang
2017-04-20  8:52           ` James Morse
2017-04-21 11:33             ` Xiongfeng Wang
2017-04-24 17:14               ` James Morse
2017-04-28  2:55                 ` Xiongfeng Wang
2017-05-08 17:27                   ` James Morse
2017-05-09  2:16                     ` Xiongfeng Wang
2017-04-21 10:46   ` Xiongfeng Wang
2017-03-30 10:31 ` [PATCH v3 8/8] arm64: exception: check shared writable page in SEI handler Xie XiuQi
2017-04-07 15:56   ` James Morse
2017-04-12  8:35     ` Xiongfeng Wang
2017-03-30 10:31 ` [PATCH v3 0/8] arm64: acpi: apei: handle SEI notification type for ARMv8 Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 1/8] trace: ras: add ARM processor error information trace event Xie XiuQi
2017-04-14 20:36   ` Baicar, Tyler
2017-04-17  3:08     ` Xie XiuQi
2017-04-17  3:16       ` Xie XiuQi
2017-04-17 17:18         ` Baicar, Tyler [this message]
2017-04-18  2:22           ` Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 2/8] acpi: apei: handle SEI notification type for ARMv8 Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 3/8] arm64: apei: add a per-cpu variable to indecate sei is processing Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 4/8] APEI: GHES: reserve a virtual page for SEI context Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 5/8] arm64: KVM: add guest SEI support Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 6/8] arm64: RAS: add ras extension runtime detection Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 7/8] arm64: exception: handle asynchronous SError interrupt Xie XiuQi
2017-03-30 10:31 ` [PATCH v3 8/8] arm64: exception: check shared writable page in SEI handler Xie XiuQi

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=f8ef4306-fd69-d8a3-3ca6-86be2825cab3@codeaurora.org \
    --to=tbaicar@codeaurora.org \
    --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).