| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 7 Oct 2016 17:39:59 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Tyler Baicar <tbaicar@...eaurora.org>
Cc: christoffer.dall@...aro.org, marc.zyngier@....com,
pbonzini@...hat.com, rkrcmar@...hat.com, linux@...linux.org.uk,
catalin.marinas@....com, will.deacon@....com, rjw@...ysocki.net,
lenb@...nel.org, matt@...eblueprint.co.uk, robert.moore@...el.com,
lv.zheng@...el.com, mark.rutland@....com, james.morse@....com,
akpm@...ux-foundation.org, sandeepa.s.prabhu@...il.com,
shijie.huang@....com, paul.gortmaker@...driver.com,
tomasz.nowicki@...aro.org, fu.wei@...aro.org, bristot@...hat.com,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
Dkvm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, linux-efi@...r.kernel.org,
devel@...ica.org
Subject: Re: [PATCH V3 09/10] trace, ras: add ARM processor error trace
event
On Fri, 7 Oct 2016 15:31:21 -0600
Tyler Baicar <tbaicar@...eaurora.org> wrote:
> Currently there are trace events for the various RAS
> errors with the exception of ARM processor type errors.
> Add a new trace event for such errors so that the user
> will know when they occur. These trace events are
> consistent with the ARM processor error section type
> defined in UEFI 2.6 spec section N.2.4.4.
>
> Signed-off-by: Tyler Baicar <tbaicar@...eaurora.org>
> ---
> drivers/firmware/efi/cper.c | 9 ++++++
> drivers/ras/ras.c | 1 +
> include/ras/ras_event.h | 67 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 77 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index f9ffba6..21b8a6f 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -34,6 +34,7 @@
> #include <linux/aer.h>
> #include <linux/printk.h>
> #include <linux/bcd.h>
> +#include <ras/ras_event.h>
>
> #define INDENT_SP " "
>
> @@ -256,6 +257,14 @@ static void cper_print_proc_armv8(const char *pfx,
> CPER_ARMV8_INFO_VALID_PHYSICAL_ADDR)
> printk("%sphysical fault address: 0x%016llx\n",
> newpfx, err_info->physical_fault_addr);
> + trace_arm_event(proc->affinity_level, proc->mpidr, proc->midr,
> + proc->running_state, proc->psci_state,
> + err_info->version, err_info->type,
> + err_info->multiple_error,
> + err_info->validation_bits,
> + err_info->error_info,
> + err_info->virt_fault_addr,
> + err_info->physical_fault_addr);
Why waste all the effort into passing each individual field. Why not
just pass the structure in and sort it out in the TP_fast_assign()?
> err_info += 1;
> }
>
> diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
> index fb2500b..8ba5a94 100644
> --- a/drivers/ras/ras.c
> +++ b/drivers/ras/ras.c
> @@ -28,3 +28,4 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(extlog_mem_event);
> #endif
> EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
> EXPORT_TRACEPOINT_SYMBOL_GPL(unknown_sec_event);
> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_event);
> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
> index 5861b6f..eb2719a 100644
> --- a/include/ras/ras_event.h
> +++ b/include/ras/ras_event.h
> @@ -162,6 +162,73 @@ TRACE_EVENT(mc_event,
> );
>
> /*
> + * ARM Processor Events Report
> + *
> + * This event is generated when hardware detects an ARM processor error
> + * has occurred. UEFI 2.6 spec section N.2.4.4.
> + */
> +TRACE_EVENT(arm_event,
> +
> + TP_PROTO(const u8 affinity,
> + const u64 mpidr,
> + const u64 midr,
> + const u32 running_state,
> + const u32 psci_state,
> + const u8 version,
> + const u8 type,
> + const u16 err_count,
> + const u8 flags,
> + const u64 info,
> + const u64 virt_fault_addr,
> + const u64 phys_fault_addr),
> +
> + TP_ARGS(affinity, mpidr, midr, running_state, psci_state,
> + version, type, err_count, flags, info, virt_fault_addr,
> + phys_fault_addr),
> +
> + TP_STRUCT__entry(
> + __field(u8, affinity)
> + __field(u64, mpidr)
> + __field(u64, midr)
> + __field(u32, running_state)
> + __field(u32, psci_state)
> + __field(u8, version)
> + __field(u8, type)
> + __field(u16, err_count)
> + __field(u8, flags)
> + __field(u64, info)
> + __field(u64, virt_fault_addr)
> + __field(u64, phys_fault_addr)
The above creates a structure with lots of holes in it. Pack it better.
You want something like:
__field(u64, mpidr)
__field(u64, midr)
__field(u64, info)
__field(u64, virt_fault_addr)
__field(u64, phys_fault_addr)
__field(u32, running_state)
__field(u32, psci_state)
__field(u16, err_count)
__field(u8, affinity)
__field(u8, version)
__field(u8, type)
__field(u8, flags)
The above is a total of 54 bytes. Your original was at a minimum, 64
bytes.
-- Steve
> + ),
> +
> + TP_fast_assign(
> + __entry->affinity = affinity;
> + __entry->mpidr = mpidr;
> + __entry->midr = midr;
> + __entry->running_state = running_state;
> + __entry->psci_state = psci_state;
> + __entry->version = version;
> + __entry->type = type;
> + __entry->err_count = err_count;
> + __entry->flags = flags;
> + __entry->info = info;
> + __entry->virt_fault_addr = virt_fault_addr;
> + __entry->phys_fault_addr = phys_fault_addr;
> + ),
> +
> + TP_printk("affinity level: %d; MPIDR: %016llx; MIDR: %016llx; "
> + "running state: %d; PSCI state: %d; version: %d; type: %d; "
> + "error count: 0x%04x; flags: 0x%02x; info: %016llx; "
> + "virtual fault address: %016llx; "
> + "physical fault address: %016llx",
> + __entry->affinity, __entry->mpidr, __entry->midr,
> + __entry->running_state, __entry->psci_state, __entry->version,
> + __entry->type, __entry->err_count, __entry->flags,
> + __entry->info, __entry->virt_fault_addr,
> + __entry->phys_fault_addr)
> +);
> +
> +/*
> * Unknown Section Report
> *
> * This event is generated when hardware detected a hardware
Powered by blists - more mailing lists