Re: [PATCH] efi: parse ARM error information value

2017-10-17 Thread Andy Shevchenko
On Tue, 2017-10-17 at 11:23 -0600, Tyler Baicar wrote:
> ARM errors just print out the error information value, then the
> value needs to be manually decoded as per the UEFI spec. Add
> decoding of the ARM error information value so that the kernel
> logs capture all of the valid information at first glance.
> 
> ARM error information value decoding is captured in UEFI 2.7
> spec tables 263-265.

Could it be located in separate file?

> + printk("%stransaction type: %s\n", pfx,
> +arm_err_trans_type_strs[trans_type]);

Plain printk():s?

> +#define CPER_ARM_ERR_VALID_TRANSACTION_TYPE  0x0001
> +#define CPER_ARM_ERR_VALID_OPERATION_TYPE0x0002
> +#define CPER_ARM_ERR_VALID_LEVEL 0x0004
> +#define CPER_ARM_ERR_VALID_PROC_CONTEXT_CORRUPT  0x0008
> +#define CPER_ARM_ERR_VALID_CORRECTED 0x0010
> +#define CPER_ARM_ERR_VALID_PRECISE_PC0x0020
> +#define CPER_ARM_ERR_VALID_RESTARTABLE_PC0x0040
> +#define CPER_ARM_ERR_VALID_PARTICIPATION_TYPE0x0080
> +#define CPER_ARM_ERR_VALID_TIME_OUT  0x0100
> +#define CPER_ARM_ERR_VALID_ADDRESS_SPACE 0x0200
> +#define CPER_ARM_ERR_VALID_MEM_ATTRIBUTES0x0400
> +#define CPER_ARM_ERR_VALID_ACCESS_MODE   0x0800

BIT() is already being used in this file.

> +
> +#define CPER_ARM_ERR_TRANSACTION_SHIFT   16
> +#define CPER_ARM_ERR_TRANSACTION_MASK0x3

Mask is mask, so GENMASK()

-- 
Andy Shevchenko 
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] efi: parse ARM error information value

2017-10-17 Thread Tyler Baicar
ARM errors just print out the error information value, then the
value needs to be manually decoded as per the UEFI spec. Add
decoding of the ARM error information value so that the kernel
logs capture all of the valid information at first glance.

ARM error information value decoding is captured in UEFI 2.7
spec tables 263-265.

Signed-off-by: Tyler Baicar 
---
 drivers/firmware/efi/cper.c | 213 +++-
 include/linux/cper.h|  44 +
 2 files changed, 255 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index d2fcafc..0a4ce9d 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -201,13 +201,218 @@ static void cper_print_proc_generic(const char *pfx,
"Misc. system register structure",
 };
 
+static const char * const arm_err_trans_type_strs[] = {
+   "Instruction",
+   "Data Access",
+   "Generic",
+};
+
+static const char * const arm_bus_err_op_strs[] = {
+   "Generic error (type cannot be determined)",
+   "Generic read (type of instruction or data request cannot be 
determined)",
+   "Generic write (type of instruction of data request cannot be 
determined)",
+   "Data read",
+   "Data write",
+   "Instruction fetch",
+   "Prefetch",
+};
+
+static const char * const arm_cache_err_op_strs[] = {
+   "Generic error (type cannot be determined)",
+   "Generic read (type of instruction or data request cannot be 
determined)",
+   "Generic write (type of instruction of data request cannot be 
determined)",
+   "Data read",
+   "Data write",
+   "Instruction fetch",
+   "Prefetch",
+   "Eviction",
+   "Snooping (processor initiated a cache snoop that resulted in an 
error)",
+   "Snooped (processor raised a cache error caused by another processor or 
device snooping its cache)",
+   "Management",
+};
+
+static const char * const arm_tlb_err_op_strs[] = {
+   "Generic error (type cannot be determined)",
+   "Generic read (type of instruction or data request cannot be 
determined)",
+   "Generic write (type of instruction of data request cannot be 
determined)",
+   "Data read",
+   "Data write",
+   "Instruction fetch",
+   "Prefetch",
+   "Local management operation (processor initiated a TLB management 
operation that resulted in an error)",
+   "External management operation (processor raised a TLB error caused by 
another processor or device broadcasting TLB operations)",
+};
+
+static const char * const arm_bus_err_part_type_strs[] = {
+   "Local processor originated request",
+   "Local processor responded to request",
+   "Local processor observed",
+   "Generic",
+};
+
+static const char * const arm_bus_err_addr_space_strs[] = {
+   "External Memory Access",
+   "Internal Memory Access",
+   "Unknown",
+   "Device Memory Access",
+};
+
+static void cper_print_arm_err_info(const char *pfx, u32 type,
+   u64 error_info)
+{
+   u8 trans_type, op_type, level, participation_type, address_space;
+   u16 mem_attributes;
+   bool proc_context_corrupt, corrected, precise_pc, restartable_pc;
+   bool time_out, access_mode;
+
+   /* If the type is unknown, bail. */
+   if (type > CPER_ARM_MAX_TYPE)
+   return;
+
+   /*
+* Vendor type errors have error information values that are vendor
+* specific.
+*/
+   if (type == CPER_ARM_VENDOR_ERROR)
+   return;
+
+   if (error_info & CPER_ARM_ERR_VALID_TRANSACTION_TYPE) {
+   trans_type = ((error_info >> CPER_ARM_ERR_TRANSACTION_SHIFT)
+ & CPER_ARM_ERR_TRANSACTION_MASK);
+   if (trans_type < ARRAY_SIZE(arm_err_trans_type_strs)) {
+   printk("%stransaction type: %s\n", pfx,
+  arm_err_trans_type_strs[trans_type]);
+   }
+   }
+
+   if (error_info & CPER_ARM_ERR_VALID_OPERATION_TYPE) {
+   op_type = ((error_info >> CPER_ARM_ERR_OPERATION_SHIFT)
+  & CPER_ARM_ERR_OPERATION_MASK);
+   switch (type) {
+   case CPER_ARM_CACHE_ERROR:
+   if (op_type < ARRAY_SIZE(arm_cache_err_op_strs)) {
+   printk("%soperation type: %s\n", pfx,
+  arm_cache_err_op_strs[op_type]);
+   }
+   break;
+   case CPER_ARM_TLB_ERROR:
+   if (op_type < ARRAY_SIZE(arm_tlb_err_op_strs)) {
+   printk("%soperation type: %s\n", pfx,
+  arm_tlb_err_op_strs[op_type]);
+   }
+   break;
+   case CPER_ARM_BUS_ERROR:
+   if (op_type 

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

2017-10-17 Thread Thiebaud Weksteen
On Mon, Oct 16, 2017 at 1:49 PM, Jarkko Sakkinen
 wrote:
> On Mon, Oct 16, 2017 at 02:28:33PM +0300, Jarkko Sakkinen wrote:
>> On Wed, Oct 11, 2017 at 02:52:54PM +0300, Jarkko Sakkinen wrote:
>> > On Wed, Oct 11, 2017 at 12:54:26PM +1100, James Morris wrote:
>> > > On Tue, 10 Oct 2017, Jarkko Sakkinen wrote:
>> > >
>> > > > The way I've agreed with James Morris to have my tree is to be rooted 
>> > > > to
>> > > > security trees next branch.
>> > > >
>> > > > James, what actions should we take?
>> > >
>> > > This process has changed recently -- I posted to lsm but forgot to post 
>> > > to
>> > > linux-integrity.
>> > >
>> > > http://kernsec.org/pipermail/linux-security-module-archive/2017-September/003356.html
>> > >
>> > > Summary: please track the next-general branch in my tree for your
>> > > development, it replaces 'next'.
>> > >
>> > >
>> > > - James
>> > > --
>> > > James Morris
>> > > 
>> >
>> > Ah I'm subscribed to that list but lately been busy getting a huge patch
>> > set to platform-driver-x86 [1] for review, which has prioritized out
>> > reading much else than linux-integrity.
>> >
>> > Thank you. I'll retry the patches tomorrow.
>> >
>> > /Jarkko
>>
>> Cannot observer binary_bios_measuremens file.
>>
>> What kind of hardware was used to develop/test this?
>>
>> I tried it with Kabylake and PTT (firmware TPM).
>>
>> /Jarkko
>
> My guess would be wrong event log format.
>
> At minimum this patch set should add a klog (info level) message to tell
> that unsupported event log format is being used.
>
> /Jarkko

This patch was mainly developed and tested on Kabylake with PTT as well.

It could be a few things. Are you booting with the EFI stub? Is the
TPM enabled within the BIOS? Does tpm_tis get loaded? Does it produce
any log?
If the logs are recovered (but not parsed), you should already see an
entry in the logs like:

efi:  SMBIOS=0x7fed6000  ACPI=0x7ff0  TPMEventLog=0x.

Can you see the TPMEventLog part?

The issue with extra logging is that the log recovery happens within
the EFI stub phase where limited logging is available (which I think
has been limited to error and fatal message only).
For now, it cannot be a version mismatch as the stub will only request
the version 1.2 format.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html