| 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: Thu, 26 Mar 2020 09:47:52 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"rjw@...ysocki.net" <rjw@...ysocki.net>,
"lenb@...nel.org" <lenb@...nel.org>, "bp@...en8.de" <bp@...en8.de>,
"james.morse@....com" <james.morse@....com>,
"tony.luck@...el.com" <tony.luck@...el.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"zhangliguang@...ux.alibaba.com" <zhangliguang@...ux.alibaba.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
Linuxarm <linuxarm@...wei.com>,
Jonathan Cameron <jonathan.cameron@...wei.com>,
tanxiaofei <tanxiaofei@...wei.com>,
yangyicong <yangyicong@...wei.com>,
Dan Carpenter <dan.carpenter@...cle.com>
Subject: RE: [PATCH v5 2/2] PCI: HIP: Add handling of HiSilicon HIP PCIe
controller errors
Hi Bjorn,
Thanks for the feedbacks.
>-----Original Message-----
>From: Bjorn Helgaas [mailto:helgaas@...nel.org]
>Sent: 25 March 2020 17:37
>To: Shiju Jose <shiju.jose@...wei.com>
>Cc: linux-acpi@...r.kernel.org; linux-pci@...r.kernel.org; linux-
>kernel@...r.kernel.org; rjw@...ysocki.net; lenb@...nel.org; bp@...en8.de;
>james.morse@....com; tony.luck@...el.com; gregkh@...uxfoundation.org;
>zhangliguang@...ux.alibaba.com; tglx@...utronix.de; Linuxarm
><linuxarm@...wei.com>; Jonathan Cameron
><jonathan.cameron@...wei.com>; tanxiaofei <tanxiaofei@...wei.com>;
>yangyicong <yangyicong@...wei.com>; Dan Carpenter
><dan.carpenter@...cle.com>
>Subject: Re: [PATCH v5 2/2] PCI: HIP: Add handling of HiSilicon HIP PCIe
>controller errors
>
>[+cc Dan]
>
>On Wed, Mar 25, 2020 at 01:55:18PM +0000, Shiju Jose wrote:
>> The HiSilicon HIP PCIe controller is capable of handling errors on
>> root port and perform port reset separately at each root port.
>>
>> This patch add error handling driver for HIP PCIe controller to log
>> and report recoverable errors. Perform root port reset and restore
>> link status after the recovery.
>>
>> Following are some of the PCIe controller's recoverable errors 1.
>> completion transmission timeout error.
>> 2. CRS retry counter over the threshold error.
>> 3. ECC 2 bit errors
>> 4. AXI bresponse/rresponse errors etc.
>>
>> Also fix the following Smatch warning:
>> warn: should '((((1))) << (9 + i))' be a 64 bit type?
>> if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i))
>> ^^^ This should be BIT_ULL() because it goes up to 9 + 32.
>> Reported-by: kbuild test robot <lkp@...el.com>
>> Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
>
>I'm glad you did this fix, and thanks for acknowledging Dan, but I don't think it's
>necessary to mention it in the commit log here because it won't really be useful
>in the future. It's only relevant when comparing the unmerged versions of this
>series, e.g., v4 compared to v3.
Sure. We will delete this.
>
>If we were fixing something that's already been merged upstream, we should
>absolutely include this, but since this hasn't been merged yet Dan's report is
>basically the same as other review comments, which we normally just address
>and mention in the change history in the [0/n] cover letter (as you're already
>doing, thanks for that!).
>
>Also, I think it's nice to CC: anybody who has commented on previous versions
>of the patch series, so I added Dan to the CC: list here.
>That way he can chime in if we're not addressing his report correctly.
>
>> Signed-off-by: Yicong Yang <yangyicong@...ilicon.com>
>> Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
>> --
>> drivers/pci/controller/Kconfig | 8 +
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-hisi-error.c | 336
>> +++++++++++++++++++++++++++++++
>> 3 files changed, 345 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-hisi-error.c
>
>As I mentioned in the other message, I think this file should be
>drivers/pci/controller/dwc/pcie-hisi-error.c so it's right next to pcie-hisi.c. If
>there's some reason it needs to be here instead, please mention that in the
>commit log.
>
>> ---
>> drivers/pci/controller/Kconfig | 8 +
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-hisi-error.c | 357
>> +++++++++++++++++++++++
>> 3 files changed, 366 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-hisi-error.c
>
>> +struct hisi_pcie_err_data {
>> + u64 val_bits;
>> + u8 version;
>> + u8 soc_id;
>> + u8 socket_id;
>> + u8 nimbus_id;
>> + u8 sub_module_id;
>> + u8 core_id;
>> + u8 port_id;
>> + u8 err_severity;
>> + u16 err_type;
>> + u8 reserv[2];
>> + u32 err_misc[HISI_PCIE_ERR_MISC_REGS];
>> +};
>> +
>> +struct hisi_pcie_err_info {
>> + struct hisi_pcie_err_data err_data;
>> + struct platform_device *pdev;
>> +};
>> +
>> +struct hisi_pcie_err_private {
>> + struct notifier_block nb;
>> + struct platform_device *pdev;
>> +};
>
>Either align all the struct members or none of them. Currently
>hisi_pcie_err_data is aligned but hisi_pcie_err_info and hisi_pcie_err_private
>are not.
We will align the structures.
>
>> + /* Call the ACPI handle to reset root port */
>
>Superfluous comment.
We will remove this comment.
>
>> + s = acpi_evaluate_integer(handle, "RST", &arg_list, &data);
>> + if (ACPI_FAILURE(s)) {
>> + dev_err(dev, "No RST method\n");
>> + return -EIO;
>> + }
>
>> +static void hisi_pcie_handle_one_error(const struct
>> +hisi_pcie_err_data
>> *err,
>> + struct platform_device *pdev)
>
>Align "struct platform_device ..." under "const struct hisi_pcie_err_data ...".
sure.
>
>Bjorn
Thanks,
Shiju
Powered by blists - more mailing lists