linux-kernel - Re: [RFC v2 1/1] fpga-region: Add generic IOCTL interface for runtime FPGA programming

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: <Z0Z6socXrmHQ26C0@yilunxu-OptiPlex-7050>
Date: Wed, 27 Nov 2024 09:49:38 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: "Manne, Nava kishore" <nava.kishore.manne@....com>
Cc: "git (AMD-Xilinx)" <git@....com>, "mdf@...nel.org" <mdf@...nel.org>,
	"hao.wu@...el.com" <hao.wu@...el.com>,
	"yilun.xu@...el.com" <yilun.xu@...el.com>,
	"trix@...hat.com" <trix@...hat.com>,
	"robh@...nel.org" <robh@...nel.org>,
	"saravanak@...gle.com" <saravanak@...gle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [RFC v2 1/1] fpga-region: Add generic IOCTL interface for
 runtime FPGA programming

> > > + * struct fpga_region_ops - ops for low level FPGA region ops for
> > > +device
> > > + * enumeration/removal
> > > + * @region_status: returns the FPGA region status
> > > + * @region_config_enumeration: Configure and enumerate the FPGA region.
> > > + * @region_remove: Remove all devices within the FPGA region
> > > + * (which are added as part of the enumeration).
> > > + */
> > > +struct fpga_region_ops {
> > > +	int (*region_status)(struct fpga_region *region);
> > > +	int (*region_config_enumeration)(struct fpga_region *region,
> > > +					 struct fpga_region_config_info *config_info);
> > 
> > My current concern is still about this combined API, it just offloads all work to low
> > level, but we have some common flows. That's why we introduce a common FPGA
> > reprograming API.
> > 
> > I didn't see issue about the vendor specific pre configuration. They are generally
> > needed to initialize the struct fpga_image_info, which is a common structure for
> > fpga_region_program_fpga().
> > 
> > For port IDs(AFU) inputs for DFL, I think it could also be changed (Don't have to be
> > implemented in this patchset). Previously DFL provides an uAPI for the whole
> > device, so it needs a port_id input to position which fpga_region within the device for
> > programming. But now, we are introducing a per fpga_region programming interface,
> > IIUC port_id should not be needed anymore.
> > 
> > The combined API is truly simple for leveraging the existing of-fpga-region overlay
> > apply mechanism. But IMHO that flow doesn't fit our new uAPI well. That flow is to
> > adapt the generic configfs overlay interface, which comes to a dead end as you
> > mentioned.
> > 
> > My gut feeling for the generic programing flow should be:
> > 
> >  1. Program the image to HW.
> >  2. Enumerate the programmed image (apply the DT overlay)
> > 
> > Why we have to:
> > 
> >  1. Start enumeration.
> >  2. On pre enumeration, programe the image.
> >  3. Real enumeration.
> > 
> 
> I agree with the approach of leveraging vendor-specific callbacks to handle
> the distinct phases of the FPGA programming process. 
> Here's the proposed flow.
>  
> Pre-Configuration:
> A vendor-specific callback extracts the required pre-configuration details
> and initializes struct fpga_image_info. This ensures that all vendor-specific

Since we need to construct the fpga_image_info, initialize multiple
field as needed, I'm wondering if configfs could be a solution for the
uAPI?

> requirements are handled before proceeding to the programming phase.
>  
> Programming:
> The common API fpga_region_program_fpga() is used to program the image
> to hardware. This standardizes the programming logic, minimizing duplication
> and ensuring consistency across implementations.
>  
> Enumeration:
> A vendor-specific callback is used for real enumeration, enabling hardware
> specific customization while keeping the flow flexible and adaptable
> 
> This approach provides a clean separation of responsibilities, with
> vendor-specific logic confined to the pre-configuration and enumeration
> phases, while the programming phase leverages a common API.
> It simplifies maintenance and aligns well with the Program -> Enumerate flow.

Generally I'm good to this flow.

Thanks,
Yilun

>  
> Looking forward to your feedback.
> 
> 
> Regards,
> Navakishore.