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: <CAHp75Vf16mH4KQ232rip9MPLoSE1TmJ_jeiwVUzqxOH5b0RFJQ@mail.gmail.com>
Date:   Wed, 10 Nov 2021 18:59:25 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Tom Rix <trix@...hat.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Moritz Fischer <mdf@...nel.org>,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>,
        linux-fpga@...r.kernel.org,
        Linux Kernel Mailing List <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 Wed, Nov 10, 2021 at 2:28 PM Tom Rix <trix@...hat.com> wrote:
> On 11/10/21 12:24 AM, Andy Shevchenko wrote:
> > On Tue, Nov 09, 2021 at 10:27:58AM -0800, Tom Rix wrote:
> >> On 11/9/21 10:05 AM, Andy Shevchenko wrote:
> >>> On Tue, Nov 09, 2021 at 07:55:43AM -0800, Tom Rix wrote:
> >>>> On 11/9/21 7:41 AM, Andy Shevchenko wrote:
> > ...
> >
> >>>>> + 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.
> >> I would rather see this fixed now or explained why this isn't a problem.
> > This is out of scope of this change in a few ways:
> >   - we don't do 2+ things in one patch
> >   - the change doesn't change behaviour
> >   - the change is a simple cleanup
> >   - another vendor may well have quite different VSEC ID for DFL
> >
> > If you think that it should be needed, one can come up with it later on.
>
> Fixing a problem is more useful than a cleanup. The fix should come first.

What do you mean by that? The original code never worked with what you
are suggesting. There is nothing to fix in terms of "fix". What you
are proposing is a feature. And as we know the features are going into
the kernel in a natural order, means fixes - priority 1, cleanups /
refactoring as prerequisites to the feature enabling - priority 2,
feature - priority 3, other cleanups and code improvements - priority
4.

That said, the proposed change definitely falls into category 2. It
makes the proposed feature to be easily realized.

Also, do not forget that vendor specific stuff is _by definition_
vendor specific, and the proposed feature is doubtful until you prove
there is another vendor-id pair.

-- 
With Best Regards,
Andy Shevchenko