linux-kernel - Re: [PATCH] PCI/AER: update AER status string print to match other AER logs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <afd74efa-96eb-3597-7323-a110f53e221b@codeaurora.org>
Date:   Tue, 17 Oct 2017 13:13:31 -0400
From:   Tyler Baicar <tbaicar@...eaurora.org>
To:     David Laight <David.Laight@...LAB.COM>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "helgaas@...nel.org" <helgaas@...nel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] PCI/AER: update AER status string print to match other
 AER logs

On 10/17/2017 12:00 PM, David Laight wrote:
> From: Tyler Baicar
>> Sent: 17 October 2017 16:42
>> Currently the AER driver uses cper_print_bits() to print the AER status
>> string. This causes the status string to not include the proper PCI device
>> name prefix that the other AER prints include. Also, it has a different
>> print level than all the other AER prints.
>>
>> Update the AER driver to print the AER status string with the proper string
>> prefix and proper print level.
>>
>> Previous log example:
>>
>> e1000e 0003:01:00.1: aer_status: 0x00000041, aer_mask: 0x00000000
>> Receiver Error, Bad TLP
> ...
>> New log:
>>
>> e1000e 0003:01:00.1: aer_status: 0x00000041, aer_mask: 0x00000000
>> e1000e 0003:01:00.1: Receiver Error
>> e1000e 0003:01:00.1: Bad TLP
> Wouldn't it be better to manage to print the above all on 1 line?
Hello David,

I broke them up into separate lines to simplify the code. If you look at 
cper_print_bits(),
it is not a clean solution and involves some hard coded values to try to limit 
the lines to
80 characters.

http://elixir.free-electrons.com/linux/v4.14-rc5/source/drivers/firmware/efi/cper.c#L85

I think printing one error per line in this case is a better solution since the 
code is much
cleaner. If you would like me to add this code to print them in a list and limit 
the lines
to 80 characters I can add that in though.
>
> ...
>> index 54c4b69..b718daa 100644
>> --- a/drivers/pci/pcie/aer/aerdrv_errprint.c
>> +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
>> @@ -206,6 +206,19 @@ void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
>>   }
>>
>>   #ifdef CONFIG_ACPI_APEI_PCIEAER
>> +void dev_print_bits(struct pci_dev *dev, unsigned int bits,
>> +		    const char * const strs[], unsigned int strs_size)
> static and rename to aer_print_bits since this isn't a generic 'dev'
> function.
Will do.

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.