linux-kernel - RE: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

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: <BYAPR02MB471005801CE9F16523516611C22D0@BYAPR02MB4710.namprd02.prod.outlook.com>
Date:   Tue, 9 Apr 2019 08:54:34 +0000
From:   Nava kishore Manne <navam@...inx.com>
To:     Michal Simek <michals@...inx.com>, Moritz Fischer <mdf@...nel.org>
CC:     "atull@...nel.org" <atull@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        Michal Simek <michals@...inx.com>,
        Rajan Vaja <RAJANV@...inx.com>, Jolly Shah <JOLLYS@...inx.com>,
        "linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "chinnikishore369@...il.com" <chinnikishore369@...il.com>
Subject: RE: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

Hi Moritz,

Thanks for the quick response.
Please find my response inline

> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@...inx.com]
> Sent: Tuesday, April 9, 2019 12:04 PM
> To: Moritz Fischer <mdf@...nel.org>; Nava kishore Manne
> <navam@...inx.com>
> Cc: atull@...nel.org; robh+dt@...nel.org; mark.rutland@....com; Michal
> Simek <michals@...inx.com>; Rajan Vaja <RAJANV@...inx.com>; Jolly Shah
> <JOLLYS@...inx.com>; linux-fpga@...r.kernel.org; devicetree@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> chinnikishore369@...il.com
> Subject: Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's
> 
> On 08. 04. 19 19:14, Moritz Fischer wrote:
> > Hi Nava,
> >
> > On Tue, Apr 02, 2019 at 06:01:21PM +0530, Nava kishore Manne wrote:
> >> This Patch Adds fpga API's to support the Bitstream loading by using
> >> firmware interface.
> >>
> >> Signed-off-by: Nava kishore Manne <nava.manne@...inx.com>
> >> ---
> >> Changes for v4:
> >> 		-None.
> >>
> >> Chnages for v3:
> >> 		-Created patches on top of 5.0-rc5.
> >> 		 No functional changes.
> >>
> >> Changes for v2:
> >> 		-Added Firmware FPGA Manager flags As suggested by
> >> 		 Moritz.
> >>
> >> Changes for v1:
> >> 		-None.
> >>
> >> Changes for RFC-V2:
> >> 		-New Patch
> >>
> >>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
> >>  include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
> >>  2 files changed, 56 insertions(+)
> >>
> >> diff --git a/drivers/firmware/xilinx/zynqmp.c
> >> b/drivers/firmware/xilinx/zynqmp.c
> >> index 98f936125643..7159a90abc44 100644
> >> --- a/drivers/firmware/xilinx/zynqmp.c
> >> +++ b/drivers/firmware/xilinx/zynqmp.c
> >> @@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum
> zynqmp_pm_reset reset,
> >>  	return ret;
> >>  }
> >>
> >> +/*
> >> + * zynqmp_pm_fpga_load - Perform the fpga load
> >> + * @address:	Address to write to
> >> + * @size:	pl bitstream size
> >> + * @flags:
> >> + *	BIT(0) - Bit-stream type.
> >> + *		 0 - Full Bitstream.
> >> + *		 1 - Partial Bitstream.
> >> + *
> >> + * This function provides access to pmufw. To transfer
> >> + * the required bitstream into PL.
> >> + *
> >> + * Return: Returns status, either success or error+reason  */ static
> >> +int zynqmp_pm_fpga_load(const u64 address, const u32 size,
> >> +			       const u32 flags)
> >> +{
> >> +	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
> >> +				   upper_32_bits(address), size, flags, NULL); }
> >> +
> >> +/**
> >> + * zynqmp_pm_fpga_get_status - Read value from PCAP status register
> >> + * @value: Value to read
> >> + *
> >> + * This function provides access to the xilfpga library to get
> >
> > xilfpga? Is that PMU firmware you're talking about?

Xilfpga is a library and it’s a part of PMUFW BSP.
It will follow the below flow to configure the PL from Linux environment.
Linux -Fpga Manager framework <--> Linux-Firmware Driver <- -smc call-->ATF <--IPI call--> PMUFW<--> Xilfpga library.

> >
> >> + * the PCAP status
> >> + *
> >> + * Return: Returns status, either success or error+reason  */ static
> >> +int zynqmp_pm_fpga_get_status(u32 *value) {
> >> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> >> +	int ret;
> >> +
> >> +	if (!value)
> >> +		return -EINVAL;
> >> +
> >> +	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0,
> ret_payload);
> >> +	*value = ret_payload[1];
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  /**
> >>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
> >>   *			       master has initialized its own power management
> >> @@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
> >>  	.request_node = zynqmp_pm_request_node,
> >>  	.release_node = zynqmp_pm_release_node,
> >>  	.set_requirement = zynqmp_pm_set_requirement,
> >> +	.fpga_load = zynqmp_pm_fpga_load,
> >> +	.fpga_get_status = zynqmp_pm_fpga_get_status,
> >>  };
> >>
> >>  /**
> >> diff --git a/include/linux/firmware/xlnx-zynqmp.h
> >> b/include/linux/firmware/xlnx-zynqmp.h
> >> index 642dab10f65d..4df226b6ab0f 100644
> >> --- a/include/linux/firmware/xlnx-zynqmp.h
> >> +++ b/include/linux/firmware/xlnx-zynqmp.h
> >> @@ -48,6 +48,12 @@
> >>  #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
> >>  #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
> >>
> >> +/*
> >> + * Firmware FPGA Manager flags
> >> + * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration  */
> >> +#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
> >> +
> >>  enum pm_api_id {
> >>  	PM_GET_API_VERSION = 1,
> >>  	PM_REQUEST_NODE = 13,
> >> @@ -56,6 +62,8 @@ enum pm_api_id {
> >>  	PM_RESET_ASSERT = 17,
> >>  	PM_RESET_GET_STATUS,
> >>  	PM_PM_INIT_FINALIZE = 21,
> >> +	PM_FPGA_LOAD = 22,
> >> +	PM_FPGA_GET_STATUS,
> >
> > Any reason you can't do 'PM_FPGA_GET_STATUS = 23' here? Trying to
> > understand your reasoning. Are you planning to move them around?
> 
> It is 23 by design. In xilinx repo there was only the first value which is
> recommended practice. But upstreaming is not done in the same order that's
> why if there is a gap you need to assign values there.
> Even that 22 can be removed in this case but it is just nit.
> 
 As Michal said here Assigning value of 22 is not needed. Will fix this issue in the next version.

Regards,
Navakishore.