Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Subject: Re: [PATCH v2 1/2] Documentation: fpga: dfl: add PCI Identification
 documentation
To:     matthew.gerlach@linux.intel.com,
        Russ Weight <russell.h.weight@intel.com>
Cc:     hao.wu@intel.com, yilun.xu@intel.com,
        basheer.ahmed.muddebihal@intel.com, mdf@kernel.org, corbet@lwn.net,
        linux-fpga@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, dan.j.williams@intel.com,
        ashok.raj@intel.com, tianfei.zhang@intel.com
References: <20220303003534.3307971-1-matthew.gerlach@linux.intel.com>
 <20220303003534.3307971-2-matthew.gerlach@linux.intel.com>
 <6448f21f-7ce1-d9ce-1048-29aca14f9d3d@redhat.com>
 <bc41bc12-0d88-771f-7e78-4e29361fcfd8@intel.com>
 <alpine.DEB.2.22.394.2203041021240.3408681@rhweight-WRK1>
From:   Tom Rix <trix@redhat.com>
Message-ID: <6c943903-0fb2-324c-5831-d569d53a7a4c@redhat.com>
Date:   Tue, 22 Mar 2022 10:11:47 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.22.394.2203041021240.3408681@rhweight-WRK1>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 3/4/22 10:30 AM, matthew.gerlach@linux.intel.com wrote:
>
>
> On Fri, 4 Mar 2022, Russ Weight wrote:
>
>>
>>
>> On 3/3/22 14:04, Tom Rix wrote:
>>>
>>> On 3/2/22 4:35 PM, matthew.gerlach@linux.intel.com wrote:
>>>> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>>>>
>>>> Add documentation on identifying FPGA based PCI cards prompted
>>>> by discussion on the linux-fpga@vger.kernel.org mailing list.
>>>>
>>>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>>>> ---
>>>> v2: Introduced in v2.
>>>> ---
>>>>   Documentation/fpga/dfl.rst | 20 ++++++++++++++++++++
>>>>   1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
>>>> index ef9eec71f6f3..5fb2ca8e76d7 100644
>>>> --- a/Documentation/fpga/dfl.rst
>>>> +++ b/Documentation/fpga/dfl.rst
>>>> @@ -502,6 +502,26 @@ Developer only needs to provide a sub feature 
>>>> driver with matched feature id.
>>>>   FME Partial Reconfiguration Sub Feature driver (see 
>>>> drivers/fpga/dfl-fme-pr.c)
>>>>   could be a reference.
>>>>   +PCI Device Identification
>>>> +================================
>>>> +Since FPGA based PCI cards can be reconfigured to a perform a 
>>>> completely
>>>> +new function at runtime, properly identifying such cards and 
>>>> binding the
>>>> +correct driver can be challenging. In many use cases, deployed 
>>>> FPGA based
>>>> +PCI cards are essentially static and the PCI Product ID and Vendor 
>>>> ID pair
>>>> +is sufficient to identify the card.  The DFL framework helps with the
>>>> +dynamic case of deployed FPGA cards changing at run time by providing
>>>> +more detailed information about card discoverable at runtime.
>>>> +
>>>> +At one level, the DFL on a PCI card describes the function of the 
>>>> card.
>>>> +However, the same DFL could be instantiated on different physical 
>>>> cards.
>>>> +Conversely, different DFLs could be instantiated on the same 
>>>> physical card.
>>>> +Practical management of a cloud containing a heterogeneous set of 
>>>> such cards
>>>> +requires a PCI level of card identification. While the PCI Product 
>>>> ID and
>>>> +Vendor ID may be sufficient to bind the dfl-pci driver, it is 
>>>> expected
>>>> +that FPGA PCI cards would advertise suitable Subsystem ID and 
>>>> Subsystem
>>>> +Vendor ID values. PCI Vital Product Data (VPD) can also be used for
>>>> +more granular information about the board.
>>>
>>> This describes a bit more of the problem, it should describe it wrt 
>>> ofs dev id. The introduction of the ofs dev should be explicitly 
>>> called out as a generic pci id.
>
> The problem I'm describing exists for all FPGA based PCI cards; so I 
> am purposely trying to be abstract as much as possible.
>
>>>
>>> Why couldn't one of the old pci id's be reused ?
>
> Yes, old pci id's could be reused, and people have done just that.  We 
> thought a new PCI ID would minimize confusion with cards that have 
> already been deployed.
>
>>>
>>> How will the subvendor/subid be enforced ?
>
> Subvendor and Subid are managed just like any other PCI card with or 
> without a FPGA.

Reviewing how the kernel uses subvendor and subsystem shows it is not 
widely used.

And when it is, it is used to isolate small variations or hw problems in 
the device, not completely unique cards

There are very few subsytem/subvendor's in pci_id.h, the usual case 
seems to be hardcoded hex.

So there is no enforcement.

I can not see how this generic id would not be abused by vendors nor how 
it would not be confusing to the end users.

Tom

>
>>>
>>> Is the current security manager patchset smart enough to save the 
>>> board from being bricked when a user doesn't look beyond the pci id ?
>>
>> Yes - the security manager is invoked based of DFL feature ID and 
>> revision, and the functionality is differentiated based on the same 
>> information.
>>
>>>
>>> What happens if a board uses this device id but doesn't have a max10 
>>> to do the update ?
>
> If a board doesn't have a max10, then there will be no DFH for a max10 
> in the board's DFLs.  Presumeably, the board would need some update 
> process, and an approprate DFH would be in that board's DFL.
>
>>>
>>> Tom
>>>
>>>> +
>>>>   Location of DFLs on a PCI Device
>>>>   ================================
>>>>   The original method for finding a DFL on a PCI device assumed the 
>>>> start of the
>>>
>>
>>