linux-kernel - Re: [PATCH 1/2] dt-bindings: fpga: zynq: Document ICAP on boot

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: <2ccsnpv67gsu354uo7xe7syrxs265ncj6hl26v3cwf2dfm7hyu@ihkemyajuiag>
Date: Mon, 31 Mar 2025 09:07:03 -0400
From: Sam Winchenbach <sam.winchenbach@...mepointer.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-kernel@...r.kernel.org, mdf@...nel.org, hao.wu@...el.com, 
	yilun.xu@...el.com, trix@...hat.com, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, michal.simek@....com, linux-fpga@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	Sam Winchenbach <swinchenbach@...a.org>
Subject: Re: [PATCH 1/2] dt-bindings: fpga: zynq: Document ICAP on boot

On Mon, Mar 31, 2025 at 02:43:59PM +0200, Krzysztof Kozlowski wrote:
> On 31/03/2025 14:30, Sam Winchenbach wrote:
> >>
> >>> +    type: boolean
> >>> +    description: If present, the ICAP controller will be enabled when
> >>> +      the driver probes. This is useful if the fabric is loaded
> >>> +      during the boot process and contains a core, such as the SEM,
> >>
> >> I don't get how this is suitable for DT. If you decide to load the
> >> fabric from driver, that's driver decision so not DT.
> > 
> > Before writing the fabric to the FPGA the driver disables the ICAP, enabling
> > the PCAP. Once writing is complete it unconditionally disables the PCAP,
> > enabling the ICAP. This patch just makes it so, depending on the use case,
> > the ICAP can be enabled at boot. This will not prevent the system from being
> > able to load a fabric through the driver. I added in this boolean so existing
> > behavior would be maintained.
> > 
> > Do you recommend another approach such as writing to a sysfs attribute to
> > switch from PCAP to ICAP?
> Not sure yet. Can't you check the status of ICAP before programming and
> then enable it only if was enabled before?

I am having a bit of difficulty understanding this so let's talk about cases
where the ICAP is enabled/disabled -

1. When writing the fabric from the driver
   In this situation it might make sense to read the state of the ICAP
   interface when preparing the fabric, before enabling PCAP. When the write
   completes you could re-enable the ICAP if it was previously enabled.

   This might be outside the scope of this change - and I am not comfortable
   enough with this use-case to understand potential side effects from doing
   this. Logically it makes sense, but there may be a very specific reason that
   the ICAP must be enabled after doing a fabric load or partial
   reconfiguration.

2. When the FPGA driver loads and is probed by the DTS
   In this situation, which is covered by this patch, the FPGA is loaded by
   BootROM/FSBL but contains functionality that requires the ICAP. Unless the
   user has made modifications to the FSBL or 3rd stage bootloader there is no
   clear way to enable the ICAP interface. Checking to see if it had been
   enabled prior to loading this driver does not (in my opinion) make a lot of
   sense here.

   Perhaps the name of the DTS is confusing? The suffix '-on-load' was meant to
   indicate when the driver was loaded, not the fabric. Would the suffix
   '-on-probe' be more clear?

Let me know your thoughts,
-Sam

>
> Best regards,
> Krzysztof