linux-kernel - Re: [PATCH 0/2] Modularization of DFL private feature drivers

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: <20200721060446.GC17091@yilunxu-OptiPlex-7050>
Date:   Tue, 21 Jul 2020 14:04:46 +0800
From:   Xu Yilun <yilun.xu@...el.com>
To:     "Wu, Hao" <hao.wu@...el.com>
Cc:     Tom Rix <trix@...hat.com>, "mdf@...nel.org" <mdf@...nel.org>,
        "linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "lgoncalv@...hat.com" <lgoncalv@...hat.com>,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>
Subject: Re: [PATCH 0/2] Modularization of DFL private feature drivers

On Mon, Jul 20, 2020 at 05:21:43PM +0800, Wu, Hao wrote:
> > On 7/16/20 8:48 PM, Wu, Hao wrote:
> > >> Subject: Re: [PATCH 0/2] Modularization of DFL private feature drivers
> > >>
> > >> Generally i think this is a good approach.
> > >>
> > >> However I do have concern.
> > >>
> > >> The feature_id in dfl is magic number, similar to pci id but without a
> > vendor
> > >> id.
> > >>
> > >> Is it possible to add something like a vendor id so different vendors would
> > >> not have to be so careful to use a unique id ?
> > > Hi Tom,
> > >
> > > Thanks for the comments.
> > >
> > > Here is only one field defined in spec, that is feature id to distinguish one
> > > feature with another one. There is no vendor id here I think, and several
> > > cards are using this for now and seems not possible to change DFH format
> > > for these products. : (
> >
> > There looks like some unused bits in the first word, how about
> >
> > #define DFH_EOL            BIT_ULL(40)        /* End of list */
> >
> > +define DFH_VENDOR    GENMAKE_ULL(49,41) /* Vendor ID */
> >
> > #define DFH_TYPE        GENMASK_ULL(63, 60)    /* Feature type */
> >
> > And Intel gets id 0.
> >
> > >
> > > I fully understand the concern is the feature id management, and potential
> > > conflicts there from different vendors. One possible method to resolve this
> > > is managing the ids in a public place (web? Or just the driver header file for
> > > feature id definition), all vendors can request some feature ids, and other
> > > people can see which feature ids have been used so that they can avoid
> > > using conflict ones with other people.
> >
> > The conflict will come in development when two vendors use the same
> > unpublished feature id.
> >
> > Also keeping the truth of id's in the kernel source isn't that great because the
> > public kernel always lags development repositories.
> 
> I fully understand your point, and it's a good idea to me, but I am not sure if
> we can update the spec for DFHv0 at this moment. Let me check with spec
> owner about this. Actually I believe we don't want to add anything in driver
> code which is not defined in the spec at all. : (
> 
> >
> > >
> > > And in the later version DFH, this feature id will be replaced with GUID
> > > I think, so it can resolve the conflict problems from different vendors?
> > > But now, we still need to handle the existing ones. : )
> >
> > This is why I proposed needing to generalize the matching.
> 
> Personally I prefer that we can have standard matching functions per DFH
> specs.
> 
> Yilun, how do you think about this?

I also prefer we don't add anything now before DFL spec is updated.

The idea of extending the DFLv0 is good, but I'm not sure if it is necessary
now. It depends on how the customer is using the card, are they
developing more FME features themselves on current cards and want to make
them public.

I think we could discuss about the strategy. And we could add another
patchset if we finally decide and update the DFLv0 spec.

Thanks
Yilun

> 
> Thanks
> Hao
>