linux-kernel - Re: [PATCH v1 1/1] fpga: dfl: pci: Use pci_find_vsec

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: <YYq4fSRoyzFE4Vei@smile.fi.intel.com>
Date:   Tue, 9 Nov 2021 20:05:49 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Tom Rix <trix@...hat.com>
Cc:     Moritz Fischer <mdf@...nel.org>,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>,
        linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wu Hao <hao.wu@...el.com>, Xu Yilun <yilun.xu@...el.com>
Subject: Re: [PATCH v1 1/1] fpga: dfl: pci: Use pci_find_vsec_capability()
 when looking for DFL

On Tue, Nov 09, 2021 at 07:55:43AM -0800, Tom Rix wrote:
> On 11/9/21 7:41 AM, Andy Shevchenko wrote:
> > Currently the find_dfls_by_vsec() opens code pci_find_vsec_capability().
> > Refactor the former to use the latter. No functional change intended.

Thanks for review, my answers below.

...

> > +	u16 voff;
> The later use of voff in pci_read_config_dword is of type 'int', it may be
> better to keep voff as an int.

I don't think so. The rule of thumb that the types should match the value they
got in the first place. In this case it's u16. Compiler will implicitly cast it
to whatever is needed as long as the type is good for integer promotion.

...

> > +	voff = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_DFLS);
> 
> This may be a weakness in the origin code, but intel isn't the exclusive
> user of DFL.

This does not change the original code. If you think so, this can be extended
later on.

> >   	if (!voff) {
> >   		dev_dbg(&pcidev->dev, "%s no DFL VSEC found\n", __func__);
> >   		return -ENODEV;

-- 
With Best Regards,
Andy Shevchenko