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: <DM6PR11MB3819828C1C193087AC6A4629857B0@DM6PR11MB3819.namprd11.prod.outlook.com>
Date:   Mon, 20 Jul 2020 09:21:43 +0000
From:   "Wu, Hao" <hao.wu@...el.com>
To:     Tom Rix <trix@...hat.com>, "Xu, Yilun" <yilun.xu@...el.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>
CC:     "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 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?

Thanks
Hao