linux-kernel - Re: [PATCH V2 1/4] drivers/fpga/amd: Add new driver amd versal-pci

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] [day] [month] [year] [list]

Message-ID: <Z6WO2Ktc9HoqdUSU@yilunxu-OptiPlex-7050>
Date: Fri, 7 Feb 2025 12:40:56 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Yidong Zhang <yidong.zhang@....com>
Cc: linux-kernel@...r.kernel.org, linux-fpga@...r.kernel.org,
	mdf@...nel.org, hao.wu@...el.com, yilun.xu@...el.com,
	lizhi.hou@....com, DMG Karthik <Karthik.DMG@....com>,
	Nishad Saraf <nishads@....com>,
	Prapul Krishnamurthy <prapulk@....com>,
	Hayden Laccabue <hayden.laccabue@....com>
Subject: Re: [PATCH V2 1/4] drivers/fpga/amd: Add new driver amd versal-pci

On Thu, Feb 06, 2025 at 07:16:25PM -0800, Yidong Zhang wrote:
> 
> 
> On 2/6/25 18:19, Xu Yilun wrote:
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> > 
> > 
> > > > No need to detach a specific driver, remove all devices in the
> > > > fpga-region. I imagine this could also be a generic flow for all PCI/e
> > > > based FPGA cards.
> > > 
> > > I see your point. Is there an existing example in current fpga driver for
> > > PCI/e based cards?
> > 
> > No. The fpga-region re-enumeration arch is still WIP, so no existing
> > implementation.
> 
> Got you. I can also help to test or provide feedback.
> Actually, I had sent Nava my protype of using configfs for the non-OF
> driver. He should have the updated patch soon.
> 
> > 
> > > 
> > > We will need to talk to our management team and re-design our driver.
> > > I think we have 2 approaches:
> > > 1) Align with linux fpga, having one driver for both userPF and mgmtPF; or
> > 
> > Don't get you. Linux FPGA doesn't require one driver for both PFs.
> 
> I assume when you said "generic flow for removing all devices in
> fpga-region" means that there is a single driver which use the fpga-region
> callbacks to remove all devices and then re-progream the FPGA.
> 
> On AMD V70 hardware, the userPF has different deviceid than the mgmtPF.
> Thus, our current design is having 2 separate pcie drivers for each
> different deviceid.
> 
> Thus, in our current design, the fpga-region should be in the userPF driver
> which has callbacks to remove all devices. But in mgmtPF, it is more like a

A question, if the FPGA logic is cleared, does the userPF still exist on
PCIe bus?

> utility which only handles request from the userPF but it has the management
> privilege. Usually, cloud vendors require the mgmtPF deployed in their
> secured domain (can be a separate physical machine).

I though mgmtPF & userPF are just 2 functions on one PCIe card, they are not?
Then how the mgmtPF writes data to another physical machine and
influence the userPF?

Thanks,
Yilun

> 
> We can keep 2 drivers, one for userPF, one for mgmtPF. The userPF and mgmtPF
> communicate each other via the communication channel because they can be
> physically on different machine.
> 
> Are you looking for us to upstream both of them together?
> If yes, I still think that the fpga-region should be in the userPF driver.
> The mgmtPF driver is still a utility driver.
> 
> Please correct me if I am wrong.:)
> 
> > 
> > > 2) find a different location (maybe driver/misc) for the version-pci driver,
> > > because it is an utility driver, not need to be tied with fpga framework.
> > 
> > I'm not the misc maintainer, but I assume in-tree utility driver +
> > out-of-tree client driver is not generally welcomed.
> 
> Great info! Thanks! I will have to discuss this with my team too.
> 
> My understanding, so far based on your comments above, the drivers/fpga
> prefer to use fpga-region ops to handle FPGA re-program changes.
> 
> The current versal-pci driver is a utility driver.
> The fpga-region should be within the userPF driver.
> 
> We can try to make our userPF driver in-tree as well. But the current plan
> is to do it later.
> 
> If you prefer we do both of them together. I can talk to my team.
> 
> Thanks,
> David
> > 
> > Thanks,
> > Yilun
> > 
> > > 
> > > Please let me know you thoughts. Which way is acceptable by you.
> > > 
> > > Thanks,
> > > David
> > > > 
> > > > Thanks,
> > > > Yilun
> > > > 
> > > > > driver and allow the userPF driver re-enumerate all to match the new
> > > > > hardware.
> > > > > 
> > > > > I think my understanding is correct, it is doable.
> > > > > 
> > > > > As long as we can keep our userPF driver as separate driver, the code change
> > > > > won't be too big.
> > > > > 
> > > > > > 
> > > > > > Thanks,
> > > > > > Yilun