linux-kernel - RE: [PATCH v9 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe controller errors

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]

Date:   Tue, 16 Jun 2020 12:46:32 +0000
From:   Shiju Jose <shiju.jose@...wei.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
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>,
        "bp@...en8.de" <bp@...en8.de>,
        "james.morse@....com" <james.morse@....com>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "dan.carpenter@...cle.com" <dan.carpenter@...cle.com>,
        "zhangliguang@...ux.alibaba.com" <zhangliguang@...ux.alibaba.com>,
        "Wangkefeng (OS Kernel Lab)" <wangkefeng.wang@...wei.com>,
        "jroedel@...e.de" <jroedel@...e.de>,
        yangyicong <yangyicong@...wei.com>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        tanxiaofei <tanxiaofei@...wei.com>,
        Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH v9 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe
 controller errors

Hi Andy,

>-----Original Message-----
>From: Andy Shevchenko [mailto:andriy.shevchenko@...ux.intel.com]
>Sent: 16 June 2020 13:41
>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; bp@...en8.de;
>james.morse@....com; lenb@...nel.org; tony.luck@...el.com;
>dan.carpenter@...cle.com; zhangliguang@...ux.alibaba.com; Wangkefeng
>(OS Kernel Lab) <wangkefeng.wang@...wei.com>; jroedel@...e.de;
>yangyicong <yangyicong@...wei.com>; Jonathan Cameron
><jonathan.cameron@...wei.com>; tanxiaofei <tanxiaofei@...wei.com>;
>Linuxarm <linuxarm@...wei.com>
>Subject: Re: [PATCH v9 2/2] PCI: hip: Add handling of HiSilicon HIP PCIe
>controller errors
>
>On Tue, Jun 16, 2020 at 11:55:46AM +0000, Shiju Jose wrote:
>> >From: linux-acpi-owner@...r.kernel.org [mailto:linux-acpi-
>> >owner@...r.kernel.org] On Behalf Of Andy Shevchenko On Tue, Jun 16,
>> >2020 at 09:12:56AM +0000, Shiju Jose wrote:
>> >> >From: Andy Shevchenko [mailto:andriy.shevchenko@...ux.intel.com]
>> >> >On Mon, Jun 15, 2020 at 11:15:52AM +0100, Shiju Jose wrote:
>
>...
>
>> >> >> +#define HISI_PCIE_CORE_PORT_ID(v)        (((v) % 8) << 1)
>> >> >
>> >> >% -> & ?
>> >> (((v) % 8) << 1) is correct. We can make bit operation instead.
>> >
>> >y % x is usually being used when we consume y / x or in cases when y
>> >is advanced and we need to keep it under some threshold.
>> >
>> >Here it's not obvious to me, and usual pattern is to use bitwise
>operations.
>> >
>> >In any case some clarification is needed.
>> We want (v % 8) * 2 here to get the core port id, a numerical value but not
>a bit mask.
>> Maybe you want us to use ((v) & 7) << 1?
>> please point it out if I understand wrong.
>
>I understand the result, I do not understand the properties of v.
>So, looks like
>a) (v & 7) << 1 // take 3 LSBs from v and shift right to get port id pair (looks
>like)
>b) (v % 8) * 2 // get next free port or circle over 0 if no free pair found
>
>Add some comment explaining what's going on.
Sure.

>
>...
>
>> >> >> +	switch (id) {
>> >> >> +	case HISI_PCIE_SUB_MODULE_ID_AP: return "AP Layer";
>> >> >> +	case HISI_PCIE_SUB_MODULE_ID_TL: return "TL Layer";
>> >> >> +	case HISI_PCIE_SUB_MODULE_ID_MAC: return "MAC Layer";
>> >> >> +	case HISI_PCIE_SUB_MODULE_ID_DL: return "DL Layer";
>> >> >> +	case HISI_PCIE_SUB_MODULE_ID_SDI: return "SDI Layer";
>> >> >> +	}
>> >> >
>> >> >match_string() ?
>> >>
>> >> match_string() does not work here because we need sub module id ->
>> >> string conversion.
>> >
>> >Why? Are you using non-sequential (a.k.a. sparse) values?
>> These are the sequential values.
>> I mean in this case we do not have the third parameter to the
>> match_string(), string to match with the strings in the array, we just
>> have the value for the sub module id.
>> Can you suggest some example of match_string() for the similar case?
>
>Ah, I realize, this is the opposite, but still perhaps better to have like this:
>
>static const char * const foo[] = {
>	"AB",
>	"CD",
>};
>
>const char *bar(int id)
>{
>	if (id >= ARRAY_SIZE(foo))
>		return "unknown"; // whatever
>	return foo[id];
>}
Sure.
  
>
>--
>With Best Regards,
>Andy Shevchenko
>
Thanks,
Shiju