Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-26 Thread Ard Biesheuvel 
On 26 February 2018 at 16:00, Ghannam, Yazen <yazen.ghan...@amd.com> wrote:
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Saturday, February 24, 2018 11:40 AM
>> To: Ghannam, Yazen <yazen.ghan...@amd.com>
>> Cc: linux-...@vger.kernel.org; Linux Kernel Mailing List > ker...@vger.kernel.org>; Borislav Petkov <b...@suse.de>; the arch/x86
>> maintainers <x...@kernel.org>
>> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
>>
> ...
>> >
>> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
>> x86.c
>> > index b50ee3cdf637..9d608f742c98 100644
>> > --- a/drivers/firmware/efi/cper-x86.c
>> > +++ b/drivers/firmware/efi/cper-x86.c
>> > @@ -4,15 +4,28 @@
>> >
>> >  #include 
>> >
>> > +#define INDENT_SP  " "
>> > +
>> >  /*
>> >   * We don't need a "CPER_IA" prefix since these are all locally defined.
>> >   * This will save us a lot of line space.
>> >   */
>> >  #define VALID_LAPIC_ID BIT_ULL(0)
>> >  #define VALID_CPUID_INFO   BIT_ULL(1)
>> > +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2))
>> >> 2)
>> > +
>>
>> Parens around 'bits' please
>>
>
> Like this?
>
> #define VALID_PROC_ERR_INFO_NUM(bits)  (((bits) & GENMASK_ULL(7, 2)) >> 2)
>

Yes. Your code currently does not pass expressions into these, but it
is good form to use parens here to make them future proof

> I'll do the same for the others.
>

Cheers

>> > +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
>> > +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
>> > +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
>> > +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
>> > +#define INFO_VALID_IP  BIT_ULL(4)
>> >
>> >  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
>> *proc)
>> >  {
>> > +   int i;
>> > +   struct cper_ia_err_info *err_info;
>> > +   char newpfx[64];
>> > +
>> > printk("%sValidation Bits: 0x%016llx\n", pfx, 
>> > proc->validation_bits);
>> >
>> > if (proc->validation_bits & VALID_LAPIC_ID)
>> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
>> cper_sec_proc_ia *proc)
>> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
>> >cpuid,
>> >sizeof(proc->cpuid), 0);
>> > }
>> > +
>> > +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> > +
>> > +   err_info = (struct cper_ia_err_info *)(proc + 1);
>> > +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); 
>> > i++)
>> {
>> > +   printk("%sError Information Structure %d:\n", pfx, i);
>> > +
>> > +   printk("%sError Structure Type: %pUl\n", newpfx,
>> > +_info->err_type);
>> > +
>>
>> The indentation is a bit awkward here. Could you please align followup
>> lines with the character following the ( on the first line?
>>
>
> Yes, will do.
>
>> > +   printk("%sValidation Bits: 0x%016llx\n",
>> > +newpfx, err_info->validation_bits);
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
>> > +   printk("%sCheck Information: 0x%016llx\n", newpfx,
>> > +err_info->check_info);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
>> > +   printk("%sTarget Identifier: 0x%016llx\n",
>> > +newpfx, err_info->target_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
>> > +   printk("%sRequestor Identifier: 0x%016llx\n",
>> > +newpfx, err_info->requestor_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
>> > +   printk("%sResponder Identifier: 0x%016llx\n",
>> > +newpfx, err_info->responder_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_IP) {
>> > +   printk("%sInstruction Pointer: 0x%016llx\n",
>> > +newpfx, err_info->ip);
>> > +   }
>> > +
>> > +   err_info++;
>> > +   }
>> >  }
>> > --
>> > 2.14.1
>> >

Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-26 Thread Ard Biesheuvel 
On 26 February 2018 at 16:00, Ghannam, Yazen  wrote:
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Saturday, February 24, 2018 11:40 AM
>> To: Ghannam, Yazen 
>> Cc: linux-...@vger.kernel.org; Linux Kernel Mailing List > ker...@vger.kernel.org>; Borislav Petkov ; the arch/x86
>> maintainers 
>> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
>>
> ...
>> >
>> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
>> x86.c
>> > index b50ee3cdf637..9d608f742c98 100644
>> > --- a/drivers/firmware/efi/cper-x86.c
>> > +++ b/drivers/firmware/efi/cper-x86.c
>> > @@ -4,15 +4,28 @@
>> >
>> >  #include 
>> >
>> > +#define INDENT_SP  " "
>> > +
>> >  /*
>> >   * We don't need a "CPER_IA" prefix since these are all locally defined.
>> >   * This will save us a lot of line space.
>> >   */
>> >  #define VALID_LAPIC_ID BIT_ULL(0)
>> >  #define VALID_CPUID_INFO   BIT_ULL(1)
>> > +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2))
>> >> 2)
>> > +
>>
>> Parens around 'bits' please
>>
>
> Like this?
>
> #define VALID_PROC_ERR_INFO_NUM(bits)  (((bits) & GENMASK_ULL(7, 2)) >> 2)
>

Yes. Your code currently does not pass expressions into these, but it
is good form to use parens here to make them future proof

> I'll do the same for the others.
>

Cheers

>> > +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
>> > +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
>> > +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
>> > +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
>> > +#define INFO_VALID_IP  BIT_ULL(4)
>> >
>> >  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
>> *proc)
>> >  {
>> > +   int i;
>> > +   struct cper_ia_err_info *err_info;
>> > +   char newpfx[64];
>> > +
>> > printk("%sValidation Bits: 0x%016llx\n", pfx, 
>> > proc->validation_bits);
>> >
>> > if (proc->validation_bits & VALID_LAPIC_ID)
>> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
>> cper_sec_proc_ia *proc)
>> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
>> >cpuid,
>> >sizeof(proc->cpuid), 0);
>> > }
>> > +
>> > +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> > +
>> > +   err_info = (struct cper_ia_err_info *)(proc + 1);
>> > +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); 
>> > i++)
>> {
>> > +   printk("%sError Information Structure %d:\n", pfx, i);
>> > +
>> > +   printk("%sError Structure Type: %pUl\n", newpfx,
>> > +_info->err_type);
>> > +
>>
>> The indentation is a bit awkward here. Could you please align followup
>> lines with the character following the ( on the first line?
>>
>
> Yes, will do.
>
>> > +   printk("%sValidation Bits: 0x%016llx\n",
>> > +newpfx, err_info->validation_bits);
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
>> > +   printk("%sCheck Information: 0x%016llx\n", newpfx,
>> > +err_info->check_info);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
>> > +   printk("%sTarget Identifier: 0x%016llx\n",
>> > +newpfx, err_info->target_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
>> > +   printk("%sRequestor Identifier: 0x%016llx\n",
>> > +newpfx, err_info->requestor_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
>> > +   printk("%sResponder Identifier: 0x%016llx\n",
>> > +newpfx, err_info->responder_id);
>> > +   }
>> > +
>> > +   if (err_info->validation_bits & INFO_VALID_IP) {
>> > +   printk("%sInstruction Pointer: 0x%016llx\n",
>> > +newpfx, err_info->ip);
>> > +   }
>> > +
>> > +   err_info++;
>> > +   }
>> >  }
>> > --
>> > 2.14.1
>> >

RE: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-26 Thread Ghannam, Yazen 
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Saturday, February 24, 2018 11:40 AM
> To: Ghannam, Yazen <yazen.ghan...@amd.com>
> Cc: linux-...@vger.kernel.org; Linux Kernel Mailing List  ker...@vger.kernel.org>; Borislav Petkov <b...@suse.de>; the arch/x86
> maintainers <x...@kernel.org>
> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
> 
...
> >
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > index b50ee3cdf637..9d608f742c98 100644
> > --- a/drivers/firmware/efi/cper-x86.c
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -4,15 +4,28 @@
> >
> >  #include 
> >
> > +#define INDENT_SP  " "
> > +
> >  /*
> >   * We don't need a "CPER_IA" prefix since these are all locally defined.
> >   * This will save us a lot of line space.
> >   */
> >  #define VALID_LAPIC_ID BIT_ULL(0)
> >  #define VALID_CPUID_INFO   BIT_ULL(1)
> > +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2))
> >> 2)
> > +
> 
> Parens around 'bits' please
> 

Like this?

#define VALID_PROC_ERR_INFO_NUM(bits)  (((bits) & GENMASK_ULL(7, 2)) >> 2)

I'll do the same for the others.

> > +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
> > +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
> > +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
> > +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
> > +#define INFO_VALID_IP  BIT_ULL(4)
> >
> >  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
> *proc)
> >  {
> > +   int i;
> > +   struct cper_ia_err_info *err_info;
> > +   char newpfx[64];
> > +
> > printk("%sValidation Bits: 0x%016llx\n", pfx, 
> > proc->validation_bits);
> >
> > if (proc->validation_bits & VALID_LAPIC_ID)
> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
> cper_sec_proc_ia *proc)
> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
> >cpuid,
> >sizeof(proc->cpuid), 0);
> > }
> > +
> > +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> > +
> > +   err_info = (struct cper_ia_err_info *)(proc + 1);
> > +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++)
> {
> > +   printk("%sError Information Structure %d:\n", pfx, i);
> > +
> > +   printk("%sError Structure Type: %pUl\n", newpfx,
> > +_info->err_type);
> > +
> 
> The indentation is a bit awkward here. Could you please align followup
> lines with the character following the ( on the first line?
> 

Yes, will do.

> > +   printk("%sValidation Bits: 0x%016llx\n",
> > +newpfx, err_info->validation_bits);
> > +
> > +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> > +   printk("%sCheck Information: 0x%016llx\n", newpfx,
> > +err_info->check_info);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> > +   printk("%sTarget Identifier: 0x%016llx\n",
> > +newpfx, err_info->target_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> > +   printk("%sRequestor Identifier: 0x%016llx\n",
> > +newpfx, err_info->requestor_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> > +   printk("%sResponder Identifier: 0x%016llx\n",
> > +newpfx, err_info->responder_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_IP) {
> > +   printk("%sInstruction Pointer: 0x%016llx\n",
> > +newpfx, err_info->ip);
> > +   }
> > +
> > +   err_info++;
> > +   }
> >  }
> > --
> > 2.14.1
> >

RE: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-26 Thread Ghannam, Yazen 
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Saturday, February 24, 2018 11:40 AM
> To: Ghannam, Yazen 
> Cc: linux-...@vger.kernel.org; Linux Kernel Mailing List  ker...@vger.kernel.org>; Borislav Petkov ; the arch/x86
> maintainers 
> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
> 
...
> >
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > index b50ee3cdf637..9d608f742c98 100644
> > --- a/drivers/firmware/efi/cper-x86.c
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -4,15 +4,28 @@
> >
> >  #include 
> >
> > +#define INDENT_SP  " "
> > +
> >  /*
> >   * We don't need a "CPER_IA" prefix since these are all locally defined.
> >   * This will save us a lot of line space.
> >   */
> >  #define VALID_LAPIC_ID BIT_ULL(0)
> >  #define VALID_CPUID_INFO   BIT_ULL(1)
> > +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2))
> >> 2)
> > +
> 
> Parens around 'bits' please
> 

Like this?

#define VALID_PROC_ERR_INFO_NUM(bits)  (((bits) & GENMASK_ULL(7, 2)) >> 2)

I'll do the same for the others.

> > +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
> > +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
> > +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
> > +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
> > +#define INFO_VALID_IP  BIT_ULL(4)
> >
> >  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
> *proc)
> >  {
> > +   int i;
> > +   struct cper_ia_err_info *err_info;
> > +   char newpfx[64];
> > +
> > printk("%sValidation Bits: 0x%016llx\n", pfx, 
> > proc->validation_bits);
> >
> > if (proc->validation_bits & VALID_LAPIC_ID)
> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
> cper_sec_proc_ia *proc)
> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
> >cpuid,
> >sizeof(proc->cpuid), 0);
> > }
> > +
> > +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> > +
> > +   err_info = (struct cper_ia_err_info *)(proc + 1);
> > +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++)
> {
> > +   printk("%sError Information Structure %d:\n", pfx, i);
> > +
> > +   printk("%sError Structure Type: %pUl\n", newpfx,
> > +_info->err_type);
> > +
> 
> The indentation is a bit awkward here. Could you please align followup
> lines with the character following the ( on the first line?
> 

Yes, will do.

> > +   printk("%sValidation Bits: 0x%016llx\n",
> > +newpfx, err_info->validation_bits);
> > +
> > +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> > +   printk("%sCheck Information: 0x%016llx\n", newpfx,
> > +err_info->check_info);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> > +   printk("%sTarget Identifier: 0x%016llx\n",
> > +newpfx, err_info->target_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> > +   printk("%sRequestor Identifier: 0x%016llx\n",
> > +newpfx, err_info->requestor_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> > +   printk("%sResponder Identifier: 0x%016llx\n",
> > +newpfx, err_info->responder_id);
> > +   }
> > +
> > +   if (err_info->validation_bits & INFO_VALID_IP) {
> > +   printk("%sInstruction Pointer: 0x%016llx\n",
> > +newpfx, err_info->ip);
> > +   }
> > +
> > +   err_info++;
> > +   }
> >  }
> > --
> > 2.14.1
> >

Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-24 Thread Ard Biesheuvel 
On 23 February 2018 at 20:03, Yazen Ghannam  wrote:
> From: Yazen Ghannam 
>
> Print the fields in the IA32/X64 Processor Error Info Structure.
>
> Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
> Structure.
>
> Cc:  # 4.16.x
> Signed-off-by: Yazen Ghannam 
> ---
>  drivers/firmware/efi/cper-x86.c | 53 
> +
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index b50ee3cdf637..9d608f742c98 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -4,15 +4,28 @@
>
>  #include 
>
> +#define INDENT_SP  " "
> +
>  /*
>   * We don't need a "CPER_IA" prefix since these are all locally defined.
>   * This will save us a lot of line space.
>   */
>  #define VALID_LAPIC_ID BIT_ULL(0)
>  #define VALID_CPUID_INFO   BIT_ULL(1)
> +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2)) >> 2)
> +

Parens around 'bits' please

> +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
> +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
> +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
> +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
> +#define INFO_VALID_IP  BIT_ULL(4)
>
>  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
>  {
> +   int i;
> +   struct cper_ia_err_info *err_info;
> +   char newpfx[64];
> +
> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>
> if (proc->validation_bits & VALID_LAPIC_ID)
> @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct 
> cper_sec_proc_ia *proc)
> print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, 
> proc->cpuid,
>sizeof(proc->cpuid), 0);
> }
> +
> +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> +
> +   err_info = (struct cper_ia_err_info *)(proc + 1);
> +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
> +   printk("%sError Information Structure %d:\n", pfx, i);
> +
> +   printk("%sError Structure Type: %pUl\n", newpfx,
> +_info->err_type);
> +

The indentation is a bit awkward here. Could you please align followup
lines with the character following the ( on the first line?

> +   printk("%sValidation Bits: 0x%016llx\n",
> +newpfx, err_info->validation_bits);
> +
> +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> +   printk("%sCheck Information: 0x%016llx\n", newpfx,
> +err_info->check_info);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> +   printk("%sTarget Identifier: 0x%016llx\n",
> +newpfx, err_info->target_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> +   printk("%sRequestor Identifier: 0x%016llx\n",
> +newpfx, err_info->requestor_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> +   printk("%sResponder Identifier: 0x%016llx\n",
> +newpfx, err_info->responder_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_IP) {
> +   printk("%sInstruction Pointer: 0x%016llx\n",
> +newpfx, err_info->ip);
> +   }
> +
> +   err_info++;
> +   }
>  }
> --
> 2.14.1
>

Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-24 Thread Ard Biesheuvel 
On 23 February 2018 at 20:03, Yazen Ghannam  wrote:
> From: Yazen Ghannam 
>
> Print the fields in the IA32/X64 Processor Error Info Structure.
>
> Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
> Structure.
>
> Cc:  # 4.16.x
> Signed-off-by: Yazen Ghannam 
> ---
>  drivers/firmware/efi/cper-x86.c | 53 
> +
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index b50ee3cdf637..9d608f742c98 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -4,15 +4,28 @@
>
>  #include 
>
> +#define INDENT_SP  " "
> +
>  /*
>   * We don't need a "CPER_IA" prefix since these are all locally defined.
>   * This will save us a lot of line space.
>   */
>  #define VALID_LAPIC_ID BIT_ULL(0)
>  #define VALID_CPUID_INFO   BIT_ULL(1)
> +#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2)) >> 2)
> +

Parens around 'bits' please

> +#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
> +#define INFO_VALID_TARGET_ID   BIT_ULL(1)
> +#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
> +#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
> +#define INFO_VALID_IP  BIT_ULL(4)
>
>  void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
>  {
> +   int i;
> +   struct cper_ia_err_info *err_info;
> +   char newpfx[64];
> +
> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>
> if (proc->validation_bits & VALID_LAPIC_ID)
> @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct 
> cper_sec_proc_ia *proc)
> print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, 
> proc->cpuid,
>sizeof(proc->cpuid), 0);
> }
> +
> +   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> +
> +   err_info = (struct cper_ia_err_info *)(proc + 1);
> +   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
> +   printk("%sError Information Structure %d:\n", pfx, i);
> +
> +   printk("%sError Structure Type: %pUl\n", newpfx,
> +_info->err_type);
> +

The indentation is a bit awkward here. Could you please align followup
lines with the character following the ( on the first line?

> +   printk("%sValidation Bits: 0x%016llx\n",
> +newpfx, err_info->validation_bits);
> +
> +   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> +   printk("%sCheck Information: 0x%016llx\n", newpfx,
> +err_info->check_info);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> +   printk("%sTarget Identifier: 0x%016llx\n",
> +newpfx, err_info->target_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> +   printk("%sRequestor Identifier: 0x%016llx\n",
> +newpfx, err_info->requestor_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> +   printk("%sResponder Identifier: 0x%016llx\n",
> +newpfx, err_info->responder_id);
> +   }
> +
> +   if (err_info->validation_bits & INFO_VALID_IP) {
> +   printk("%sInstruction Pointer: 0x%016llx\n",
> +newpfx, err_info->ip);
> +   }
> +
> +   err_info++;
> +   }
>  }
> --
> 2.14.1
>

[PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-23 Thread Yazen Ghannam 
From: Yazen Ghannam 

Print the fields in the IA32/X64 Processor Error Info Structure.

Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
Structure.

Cc:  # 4.16.x
Signed-off-by: Yazen Ghannam 
---
 drivers/firmware/efi/cper-x86.c | 53 +
 1 file changed, 53 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index b50ee3cdf637..9d608f742c98 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -4,15 +4,28 @@
 
 #include 
 
+#define INDENT_SP  " "
+
 /*
  * We don't need a "CPER_IA" prefix since these are all locally defined.
  * This will save us a lot of line space.
  */
 #define VALID_LAPIC_ID BIT_ULL(0)
 #define VALID_CPUID_INFO   BIT_ULL(1)
+#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2)) >> 2)
+
+#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
+#define INFO_VALID_TARGET_ID   BIT_ULL(1)
+#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
+#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
+#define INFO_VALID_IP  BIT_ULL(4)
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
+   int i;
+   struct cper_ia_err_info *err_info;
+   char newpfx[64];
+
printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
 
if (proc->validation_bits & VALID_LAPIC_ID)
@@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct 
cper_sec_proc_ia *proc)
print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
   sizeof(proc->cpuid), 0);
}
+
+   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+
+   err_info = (struct cper_ia_err_info *)(proc + 1);
+   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
+   printk("%sError Information Structure %d:\n", pfx, i);
+
+   printk("%sError Structure Type: %pUl\n", newpfx,
+_info->err_type);
+
+   printk("%sValidation Bits: 0x%016llx\n",
+newpfx, err_info->validation_bits);
+
+   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
+   printk("%sCheck Information: 0x%016llx\n", newpfx,
+err_info->check_info);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
+   printk("%sTarget Identifier: 0x%016llx\n",
+newpfx, err_info->target_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
+   printk("%sRequestor Identifier: 0x%016llx\n",
+newpfx, err_info->requestor_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
+   printk("%sResponder Identifier: 0x%016llx\n",
+newpfx, err_info->responder_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_IP) {
+   printk("%sInstruction Pointer: 0x%016llx\n",
+newpfx, err_info->ip);
+   }
+
+   err_info++;
+   }
 }
-- 
2.14.1

[PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

2018-02-23 Thread Yazen Ghannam 
From: Yazen Ghannam 

Print the fields in the IA32/X64 Processor Error Info Structure.

Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
Structure.

Cc:  # 4.16.x
Signed-off-by: Yazen Ghannam 
---
 drivers/firmware/efi/cper-x86.c | 53 +
 1 file changed, 53 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index b50ee3cdf637..9d608f742c98 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -4,15 +4,28 @@
 
 #include 
 
+#define INDENT_SP  " "
+
 /*
  * We don't need a "CPER_IA" prefix since these are all locally defined.
  * This will save us a lot of line space.
  */
 #define VALID_LAPIC_ID BIT_ULL(0)
 #define VALID_CPUID_INFO   BIT_ULL(1)
+#define VALID_PROC_ERR_INFO_NUM(bits)  ((bits & GENMASK_ULL(7, 2)) >> 2)
+
+#define INFO_VALID_CHECK_INFO  BIT_ULL(0)
+#define INFO_VALID_TARGET_ID   BIT_ULL(1)
+#define INFO_VALID_REQUESTOR_IDBIT_ULL(2)
+#define INFO_VALID_RESPONDER_IDBIT_ULL(3)
+#define INFO_VALID_IP  BIT_ULL(4)
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
+   int i;
+   struct cper_ia_err_info *err_info;
+   char newpfx[64];
+
printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
 
if (proc->validation_bits & VALID_LAPIC_ID)
@@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct 
cper_sec_proc_ia *proc)
print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
   sizeof(proc->cpuid), 0);
}
+
+   snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+
+   err_info = (struct cper_ia_err_info *)(proc + 1);
+   for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
+   printk("%sError Information Structure %d:\n", pfx, i);
+
+   printk("%sError Structure Type: %pUl\n", newpfx,
+_info->err_type);
+
+   printk("%sValidation Bits: 0x%016llx\n",
+newpfx, err_info->validation_bits);
+
+   if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
+   printk("%sCheck Information: 0x%016llx\n", newpfx,
+err_info->check_info);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
+   printk("%sTarget Identifier: 0x%016llx\n",
+newpfx, err_info->target_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
+   printk("%sRequestor Identifier: 0x%016llx\n",
+newpfx, err_info->requestor_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
+   printk("%sResponder Identifier: 0x%016llx\n",
+newpfx, err_info->responder_id);
+   }
+
+   if (err_info->validation_bits & INFO_VALID_IP) {
+   printk("%sInstruction Pointer: 0x%016llx\n",
+newpfx, err_info->ip);
+   }
+
+   err_info++;
+   }
 }
-- 
2.14.1